Add support for HTTP Bearer authentication (RFC6750)

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;

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;

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,

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"/);
+    });
   });
 });