fix: use insufficient_scope instead of invalid_scope at userinfo_endp…

…oint
panva · Oct 12, 2021 · ba8a8f0 · ba8a8f0
1 parent 2d3cae0
commit ba8a8f0
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 7 deletions.
diff --git a/lib/actions/userinfo.js b/lib/actions/userinfo.js
@@ -1,4 +1,4 @@
-const { InvalidDpopProof, InvalidToken, InvalidScope } = require('../helpers/errors');
+const { InvalidDpopProof, InvalidToken, InsufficientScope } = require('../helpers/errors');
 const difference = require('../helpers/_/difference');
 const setWWWAuthenticate = require('../helpers/set_www_authenticate');
 const bodyParser = require('../shared/conditional_body');
@@ -74,8 +74,9 @@ module.exports = [
 
     ctx.oidc.entity('AccessToken', accessToken);
 
-    if (!accessToken.scope || !accessToken.scope.split(' ').includes('openid')) {
-      throw new InvalidToken('access token missing openid scope');
+    const { scopes } = accessToken;
+    if (!scopes.size || !scopes.has('openid')) {
+      throw new InsufficientScope('access token missing openid scope', 'openid');
     }
 
     if (accessToken['x5t#S256']) {
@@ -115,11 +116,10 @@ module.exports = [
 
   async function validateScope(ctx, next) {
     if (ctx.oidc.params.scope) {
-      const accessTokenScopes = ctx.oidc.accessToken.scope.split(' ');
-      const missing = difference(ctx.oidc.params.scope.split(' '), accessTokenScopes);
+      const missing = difference(ctx.oidc.params.scope.split(' '), [...ctx.oidc.accessToken.scopes]);
 
       if (missing.length !== 0) {
-        throw new InvalidScope('access token missing requested scope', missing.join(' '));
+        throw new InsufficientScope('access token missing requested scope', missing.join(' '));
       }
     }
     await next();

diff --git a/lib/helpers/errors.js b/lib/helpers/errors.js
@@ -54,6 +54,14 @@ class InvalidScope extends OIDCProviderError {
   }
 }
 
+class InsufficientScope extends OIDCProviderError {
+  constructor(description, scope, detail) {
+    super(403, 'insufficient_scope');
+    Error.captureStackTrace(this, this.constructor);
+    Object.assign(this, { scope, error_description: description, error_detail: detail });
+  }
+}
+
 class InvalidRequest extends OIDCProviderError {
   constructor(description, code = 400, detail) {
     super(code, 'invalid_request');
@@ -168,6 +176,7 @@ module.exports.InvalidGrant = InvalidGrant;
 module.exports.InvalidRedirectUri = InvalidRedirectUri;
 module.exports.InvalidRequest = InvalidRequest;
 module.exports.InvalidScope = InvalidScope;
+module.exports.InsufficientScope = InsufficientScope;
 module.exports.InvalidToken = InvalidToken;
 module.exports.OIDCProviderError = OIDCProviderError;
 module.exports.SessionNotFound = SessionNotFound;

diff --git a/test/userinfo/userinfo.test.js b/test/userinfo/userinfo.test.js
@@ -77,9 +77,23 @@ describe('userinfo /me', () => {
       .expect(this.failWith(400, 'invalid_request', 'no access token provided'));
   });
 
+  it('validates the openid scope is present', async function () {
+    const at = await new this.provider.AccessToken({
+      client: await this.provider.Client.find('client'),
+    }).save();
+    sinon.stub(this.provider.Client, 'find').callsFake(async () => undefined);
+    return this.agent.get('/me')
+      .auth(at, { type: 'bearer' })
+      .expect(() => {
+        this.provider.Client.find.restore();
+      })
+      .expect(this.failWith(403, 'insufficient_scope', 'access token missing openid scope', 'openid'));
+  });
+
   it('validates a client is still valid for a found token', async function () {
     const at = await new this.provider.AccessToken({
       client: await this.provider.Client.find('client'),
+      scope: 'openid',
     }).save();
     sinon.stub(this.provider.Client, 'find').callsFake(async () => undefined);
     return this.agent.get('/me')
@@ -93,6 +107,7 @@ describe('userinfo /me', () => {
   it('validates an account still valid for a found token', async function () {
     const at = await new this.provider.AccessToken({
       client: await this.provider.Client.find('client'),
+      scope: 'openid',
       accountId: 'notfound',
     }).save();
     return this.agent.get('/me')
@@ -119,6 +134,6 @@ describe('userinfo /me', () => {
         scope: 'openid profile',
       })
       .auth(this.access_token, { type: 'bearer' })
-      .expect(this.failWith(400, 'invalid_scope', 'access token missing requested scope', 'profile'));
+      .expect(this.failWith(403, 'insufficient_scope', 'access token missing requested scope', 'profile'));
   });
 });