diff --git a/index.js b/index.js index 52d77e8..9844ad0 100644 --- a/index.js +++ b/index.js @@ -20,11 +20,17 @@ const ep = (endpoint) => `/ep_openid_connect/${endpoint}`; const endpointUrl = (endpoint) => new URL(ep(endpoint).substr(1), settings.base_url).toString(); const validateSubClaim = (sub) => { - if (typeof sub !== 'string' || // 'sub' claim must exist as a string per OIDC spec. - sub === '' || // Empty string doesn't make sense. - sub === '__proto__' || // Prevent prototype pollution. - settings.prohibited_usernames.includes(sub)) { - throw new Error('invalid sub claim'); + try { + if (typeof sub !== 'string' || // 'sub' claim must exist as a string per OIDC spec. + sub === '' || // Empty string doesn't make sense. + sub === '__proto__' || // Prevent prototype pollution. + settings.prohibited_usernames.includes(sub)) { + throw new Error('invalid sub claim'); + } + } catch (err) { + err.error = 'invalid_token'; // RFC6750 section 3.1. + err.error_description = err.message; + throw err; } }; @@ -178,11 +184,71 @@ exports.expressCreateServer = (hookName, {app}) => { }); }; -exports.authenticate = (hookName, {req, res, users}) => { +const userinfoFromBearerToken = async ({headers: {authorization = ''}}) => { + if (!/^Bearer(?: .*)?$/.test(authorization)) return null; + logger.debug('checking Bearer token'); + // RFC6749 section A.12 says that access tokens consist of characters 0x20 through 0x7E, but + // RFC6750 section 2.1 only allows a subset of those characters without explanation. ¯\_(ツ)_/¯ + // Because the original token string isn't encoded before putting it in the Authenticate header, + // just use whatever the client sends. If it contains invalid characters, introspection will fail. + const [, token] = /^Bearer +(.*)/.exec(authorization) || []; + try { + if (!token) throw new Error('missing Bearer token'); + } catch (err) { + err.error = 'invalid_request'; // RFC6750 section 3.1. + err.error_description = err.message; + throw err; + } + const [insp, userinfo] = await Promise.all([ + oidcClient.introspect(token, 'access_token'), + oidcClient.userinfo(token), + ]); + logger.debug('token introspection data:', insp); + logger.debug('userinfo:', userinfo); + try { + if (!insp.active) throw new Error('Bearer token not active'); + // RFC7662 says insp.token_type is optional, but check it if it's there. + if (insp.token_type != null && insp.token_type !== 'Bearer') { + throw new Error('token type is not Bearer'); + } + // RFC7662 says insp.sub is optional, but check it if it's there. + if (insp.sub != null && insp.sub !== userinfo.sub) { + throw new Error('sub claim mismatch'); + } + } catch (err) { + err.error = 'invalid_token'; // RFC6750 section 3.1. + err.error_description = err.message; + throw err; + } + try { + // RFC7662 says insp.scope is optional, but we require it so that we can check it. + if (insp.scope == null) throw new Error('unknown Bearer token scope'); + const scopes = insp.scope.split(/ +/); + for (const scope of settings.scope) { + if (!scopes.includes(scope)) throw new Error(`Bearer token lacks scope ${scope}`); + } + } catch (err) { + err.error = 'insufficient_scope'; // RFC6750 section 3.1. + err.error_description = err.message; + throw err; + } + validateSubClaim(userinfo.sub); + return userinfo; +}; + +exports.authenticate = async (hookName, {req, res, users}) => { if (oidcClient == null) return; logger.debug('authenticate hook for', req.url); - const {ep_openid_connect: {userinfo} = {}} = req.session; - if (userinfo == null) { // Nullish means the user isn't authenticated. + let {ep_openid_connect: {userinfo} = {}} = req.session; + try { + if (userinfo == null) userinfo = await userinfoFromBearerToken(req); + if (userinfo == null) throw new Error('not authenticated'); + } catch (err) { + logger.debug(`authentication failure: ${err}`); + // oidcClient sometimes throws errors with `.error*` properties that can (and should) be + // returned in the WWW-Authenticate header. The code above also sets those properties in thrown + // exceptions. Save the error so we can use those properties if present. + res.locals.ep_openid_connect = {err}; // Out of an abundance of caution, clear out the old state, nonce, and userinfo (if present) to // force regeneration. delete req.session.ep_openid_connect; @@ -206,34 +272,20 @@ exports.authenticate = (hookName, {req, res, users}) => { exports.authnFailure = (hookName, {req, res}) => { if (oidcClient == null) return; + const wwwAuthenticateFields = { + realm: 'ep_openid_connect', + scope: settings.scope.join(' '), + }; + // Include the special error properties from the thrown error object, if present. + const {ep_openid_connect: {err = {}} = {}} = res.locals; + for (const field of ['error', 'error_description', 'error_uri']) { + if (typeof err[field] === 'string') wwwAuthenticateFields[field] = err[field]; + } + const fieldsStr = Object.entries(wwwAuthenticateFields).map(([k, v]) => ` ${k}="${v}"`).join(''); + res.header('WWW-Authenticate', `Bearer${fieldsStr}`); // Normally the user is redirected to the login page which would then redirect the user back once // authenticated. For non-GET requests, send a 401 instead because users can't be redirected back. // Also send a 401 if an Authorization header is present to facilitate API error handling. - // - // 401 is the status that most closely matches the desired semantics. However, RFC7235 section - // 3.1 says, "The server generating a 401 response MUST send a WWW-Authenticate header field - // containing at least one challenge applicable to the target resource." Etherpad uses a token - // (signed session identifier) transmitted via cookie for authentication, but there is no - // standard authentication scheme name for that. So we use a non-standard name here. - // - // We could theoretically implement Bearer authorization (RFC6750), but it's unclear to me how - // to do this correctly and securely: - // * The userinfo endpoint is meant for the OAuth client, not the resource server, so it - // shouldn't be used to look up claims. - // * In general, access tokens might be opaque (not JWTs) so we can't get claims by parsing - // them. - // * The token introspection endpoint should return scope and subject (I think?), but probably - // not claims. - // * If claims can't be used to convey access level, how is it conveyed? Scope? Resource - // indicators (RFC8707)? - // * How is intended audience checked? Or is introspection guaranteed to do that for us? - // * Should tokens be limited to a particular pad? - // * Bearer tokens are only meant to convey authorization; authentication is handled by the - // authorization server. Should Bearer tokens be processed during the authorize hook? - // * How should bearer authentication interact with authorization plugins? - // * How should bearer authentication interact with plugins that add new endpoints? - // * Would we have to implement our own OAuth server to issue access tokens? - res.header('WWW-Authenticate', 'Etherpad'); if (!['GET', 'HEAD'].includes(req.method) || req.headers.authorization) { res.status(401).end(); return true; diff --git a/static/tests/backend/oidc-client.js b/static/tests/backend/oidc-client.js new file mode 100644 index 0000000..271b12c --- /dev/null +++ b/static/tests/backend/oidc-client.js @@ -0,0 +1,85 @@ +'use strict'; + +const assert = require('assert').strict; +const common = require('ep_etherpad-lite/tests/backend/common'); +const openidClient = require('openid-client'); +const http = require('http'); +const login = require('./login'); +const supertest = require('supertest'); +const util = require('util'); + +const logger = common.logger; + +class OidcClient { + get callbackUrl() { + return `http://localhost:${this._server.address().port}/callback`; + } + + async start(issuerUrl, settings = {}) { + await this.stop(); + this._issuer = issuerUrl; + this._server = http.createServer(); + this._server.on('request', (req, res) => this._handleRequest(req, res)); + await util.promisify(this._server.listen).call(this._server, 0, 'localhost'); + const issuer = await openidClient.Issuer.discover(issuerUrl); + this._client = await issuer.Client.register({ + ...settings, + redirect_uris: [this.callbackUrl], + response_types: ['code'], + }); + this._callbackChecks = new Map(); + this._callbackResults = new Map(); + logger.info(`Test OIDC client callback: ${this.callbackUrl}`); + } + + async stop() { + if (this._server == null) return; + await util.promisify(this._server.close).call(this._server); + this._server = null; + } + + async authenticate(scope, username) { + const agent = supertest.agent(''); + const state = openidClient.generators.state(); + const commonParams = {nonce: openidClient.generators.nonce(), scope, state}; + const codeVerifier = openidClient.generators.codeVerifier(); + this._callbackChecks.set(state, {...commonParams, code_verifier: codeVerifier}); + const results = new Promise((resolve) => this._callbackResults.set(state, resolve)); + const url = this._client.authorizationUrl({ + ...commonParams, + code_challenge: openidClient.generators.codeChallenge(codeVerifier), + code_challenge_method: 'S256', + }); + const res = await login(agent, this._issuer, url, username); + assert(res.request.url.startsWith(this.callbackUrl)); + assert.equal(res.status, 200); + this._callbackChecks.delete(state); + this._callbackResults.delete(state); + return await results; + } + + async _handleRequest(req, res) { + try { + const url = new URL(req.url, `http://${req.headers.host}/`); + if (req.method !== 'GET' || url.pathname !== '/callback') return res.end(); + const params = this._client.callbackParams(req); + if (!params.state) throw new Error('missing state from callback params'); + const checks = this._callbackChecks.get(params.state); + if (checks == null) throw new Error('missing callback checks'); + const tokenset = await this._client.callback(this.callbackUrl, params, checks); + const userinfo = await this._client.userinfo(tokenset); + this._callbackResults.get(params.state)({tokenset, userinfo}); + res.statusCode = 200; + res.write('success'); + } catch (err) { + logger.error(`Error while handling HTTP request in OidcClient: ${err.stack || err}`); + res.statusCode = 500; + } + res.end(); + } + + async revokeToken(token, tokenType) { + await this._client.revoke(token, tokenType); + } +} +module.exports = OidcClient; diff --git a/static/tests/backend/oidc-provider.js b/static/tests/backend/oidc-provider.js index 1f6c629..d57750e 100644 --- a/static/tests/backend/oidc-provider.js +++ b/static/tests/backend/oidc-provider.js @@ -33,6 +33,12 @@ class OidcProvider { }, features: { devInteractions: {enabled: false}, + introspection: { + enabled: true, + allowedPolicy: () => true, // Silence warning. + }, + registration: {enabled: true}, + revocation: {enabled: true}, }, findAccount: async (ctx, sub, token) => ({ accountId: sub, @@ -71,8 +77,9 @@ class OidcProvider { /* eslint-enable max-len */ }, ttl: { + // Short to test expiration. + AccessToken: 5, // Silence warnings. - AccessToken: 3600, Grant: 3600, IdToken: 3600, Interaction: 3600, diff --git a/static/tests/backend/specs/tests.js b/static/tests/backend/specs/tests.js index 1df7602..b7a6501 100644 --- a/static/tests/backend/specs/tests.js +++ b/static/tests/backend/specs/tests.js @@ -1,5 +1,6 @@ 'use strict'; +const OidcClient = require('../oidc-client'); const OidcProvider = require('../oidc-provider'); const assert = require('assert').strict; const common = require('ep_etherpad-lite/tests/backend/common'); @@ -216,13 +217,105 @@ describe(__filename, function () { it('HTTP PUT gives 401 instead of redirect', async function () { await agent.put(common.baseUrl) .expect(401) - .expect('www-authenticate', 'Etherpad'); + .expect('www-authenticate', /^Bearer /); }); it('Authorization header results in 401 instead of redirect', async function () { await agent.get(common.baseUrl) .set('Authorization', 'Basic dXNlcm5hbWU6cGFzc3dvcmQ=') .expect(401) - .expect('www-authenticate', 'Etherpad'); + .expect('www-authenticate', /^Bearer /); + }); + + describe('Bearer token authentication', function () { + let client; + + before(async function () { + client = new OidcClient(); + await client.start(issuer); + }); + + after(async function () { + if (client != null) await client.stop(); + client = null; + }); + + it('valid token', async function () { + const {tokenset: {access_token}} = + await client.authenticate(pluginSettings.scope.join(' '), 'normalUser'); + await agent.get(new URL('/', common.baseUrl)) + .set('Authorization', `Bearer ${access_token}`) + .expect(200); + }); + + it('claims apply', async function () { + const {tokenset: {access_token}} = + await client.authenticate(pluginSettings.scope.join(' '), 'adminUser'); + await agent.get(new URL('/admin/', common.baseUrl)) + .set('Authorization', `Bearer ${access_token}`) + .expect(200); + }); + + it('missing token', async function () { + await agent.get(new URL('/', common.baseUrl)) + .set('Authorization', 'Bearer ') + .expect(401) + .expect('WWW-Authenticate', /^Bearer.* error="invalid_request"/); + }); + + it('bad token value', async function () { + await agent.get(new URL('/', common.baseUrl)) + .set('Authorization', 'Bearer foobarbaz') + .expect(401) + .expect('WWW-Authenticate', /^Bearer.* error="invalid_token"/); + }); + + it('expired token', async function () { + const {tokenset: {access_token}} = + await client.authenticate(pluginSettings.scope.join(' '), 'normalUser'); + // OidcProvider is configured with 5s access token lifetime. Wait a bit over a second in case + // the expiration logic uses integer math with seconds instead of milliseconds. + await new Promise((resolve) => setTimeout(resolve, 6100)); + await agent.get(new URL('/', common.baseUrl)) + .set('Authorization', `Bearer ${access_token}`) + .expect(401) + .expect('WWW-Authenticate', /^Bearer.* error="invalid_token"/); + }); + + it('revoked token', async function () { + const {tokenset: {access_token}} = + await client.authenticate(pluginSettings.scope.join(' '), 'normalUser'); + await client.revokeToken(access_token, 'access_token'); + await agent.get(new URL('/', common.baseUrl)) + .set('Authorization', `Bearer ${access_token}`) + .expect(401) + .expect('WWW-Authenticate', /^Bearer.* error="invalid_token"/); + }); + + it('wrong token type (refresh token)', async function () { + const {tokenset: {refresh_token}} = + await client.authenticate(pluginSettings.scope.join(' '), 'normalUser'); + await agent.get(new URL('/', common.baseUrl)) + .set('Authorization', `Bearer ${refresh_token}`) + .expect(401) + .expect('WWW-Authenticate', /^Bearer.* error="invalid_token"/); + }); + + it('wrong token type (ID token)', async function () { + const {tokenset: {id_token}} = + await client.authenticate(pluginSettings.scope.join(' '), 'normalUser'); + await agent.get(new URL('/', common.baseUrl)) + .set('Authorization', `Bearer ${id_token}`) + .expect(401) + .expect('WWW-Authenticate', /^Bearer.* error="invalid_token"/); + }); + + it('insufficient scope', async function () { + const {tokenset: {access_token}} = await client.authenticate('openid', 'normalUser'); + await agent.get(new URL('/', common.baseUrl)) + .set('Authorization', `Bearer ${access_token}`) + .expect(401) + .expect('WWW-Authenticate', /^Bearer.* error="insufficient_scope"/); + }); }); });