From 47ee118523b2c185f622fa6386b3c1a30f928aa0 Mon Sep 17 00:00:00 2001 From: Daniel Hensby Date: Wed, 24 Jul 2024 15:14:15 +0100 Subject: [PATCH] fix: explicitly support array of strings for AuthenticateHandler.options In #267 some changes were made for compliance reasons where scopes should be treated as strings for our OAuth interfaces but as arrays of strings for internal purposes. As part of that change, we accidentally made the `scope` prop for the `authenticate` method only support strings. This change remediates any potential backwards compatibility changes from v5.0 to v5.1 --- index.d.ts | 2 +- lib/handlers/authenticate-handler.js | 2 +- .../handlers/authenticate-handler_test.js | 104 +++++++++++++++++- 3 files changed, 102 insertions(+), 6 deletions(-) diff --git a/index.d.ts b/index.d.ts index d2a705c..a8e1e7c 100644 --- a/index.d.ts +++ b/index.d.ts @@ -168,7 +168,7 @@ declare namespace OAuth2Server { /** * The scope(s) to authenticate. */ - scope?: string | undefined; + scope?: string[]; /** * Set the X-Accepted-OAuth-Scopes HTTP header on response objects. diff --git a/lib/handlers/authenticate-handler.js b/lib/handlers/authenticate-handler.js index a930c7c..9faaf0d 100644 --- a/lib/handlers/authenticate-handler.js +++ b/lib/handlers/authenticate-handler.js @@ -47,7 +47,7 @@ class AuthenticateHandler { this.addAuthorizedScopesHeader = options.addAuthorizedScopesHeader; this.allowBearerTokensInQueryString = options.allowBearerTokensInQueryString; this.model = options.model; - this.scope = parseScope(options.scope); + this.scope = Array.isArray(options.scope) ? options.scope : parseScope(options.scope); } /** diff --git a/test/integration/handlers/authenticate-handler_test.js b/test/integration/handlers/authenticate-handler_test.js index 8684ce8..f7c5323 100644 --- a/test/integration/handlers/authenticate-handler_test.js +++ b/test/integration/handlers/authenticate-handler_test.js @@ -242,6 +242,35 @@ describe('AuthenticateHandler integration', function() { }); it('should return an access token', function() { + const accessToken = { + user: {}, + accessTokenExpiresAt: new Date(new Date().getTime() + 10000) + }; + const model = { + getAccessToken: function() { + return accessToken; + }, + verifyScope: function() { + return true; + } + }; + const handler = new AuthenticateHandler({ addAcceptedScopesHeader: true, addAuthorizedScopesHeader: true, model: model, scope: ['foo'] }); + const request = new Request({ + body: {}, + headers: { 'Authorization': 'Bearer foo' }, + method: {}, + query: {} + }); + const response = new Response({ body: {}, headers: {} }); + + return handler.handle(request, response) + .then(function(data) { + data.should.equal(accessToken); + }) + .catch(should.fail); + }); + + it('should return an access token (deprecated)', function() { const accessToken = { user: {}, accessTokenExpiresAt: new Date(new Date().getTime() + 10000) @@ -515,7 +544,7 @@ describe('AuthenticateHandler integration', function() { }); describe('verifyScope()', function() { - it('should throw an error if `scope` is insufficient', function() { + it('should throw an error if `scope` is insufficient (deprecated)', function() { const model = { getAccessToken: function() {}, verifyScope: function() { @@ -532,7 +561,48 @@ describe('AuthenticateHandler integration', function() { }); }); + it('should throw an error if `scope` is insufficient', function() { + const model = { + getAccessToken: function() {}, + verifyScope: function() { + return false; + } + }; + const handler = new AuthenticateHandler({ addAcceptedScopesHeader: true, addAuthorizedScopesHeader: true, model: model, scope: ['foo'] }); + + return handler.verifyScope(['foo']) + .then(should.fail) + .catch(function(e) { + e.should.be.an.instanceOf(InsufficientScopeError); + e.message.should.equal('Insufficient scope: authorized scope is insufficient'); + }); + }); + + it('should support promises (deprecated)', function() { + const model = { + getAccessToken: function() {}, + verifyScope: function() { + return true; + } + }; + const handler = new AuthenticateHandler({ addAcceptedScopesHeader: true, addAuthorizedScopesHeader: true, model: model, scope: 'foo' }); + + handler.verifyScope(['foo']).should.be.an.instanceOf(Promise); + }); + it('should support promises', function() { + const model = { + getAccessToken: function() {}, + verifyScope: function() { + return true; + } + }; + const handler = new AuthenticateHandler({ addAcceptedScopesHeader: true, addAuthorizedScopesHeader: true, model: model, scope: ['foo'] }); + + handler.verifyScope(['foo']).should.be.an.instanceOf(Promise); + }); + + it('should support non-promises (deprecated)', function() { const model = { getAccessToken: function() {}, verifyScope: function() { @@ -551,7 +621,7 @@ describe('AuthenticateHandler integration', function() { return true; } }; - const handler = new AuthenticateHandler({ addAcceptedScopesHeader: true, addAuthorizedScopesHeader: true, model: model, scope: 'foo' }); + const handler = new AuthenticateHandler({ addAcceptedScopesHeader: true, addAuthorizedScopesHeader: true, model: model, scope: ['foo'] }); handler.verifyScope(['foo']).should.be.an.instanceOf(Promise); }); @@ -571,7 +641,7 @@ describe('AuthenticateHandler integration', function() { response.headers.should.not.have.property('x-accepted-oauth-scopes'); }); - it('should set the `X-Accepted-OAuth-Scopes` header if `scope` is specified', function() { + it('should set the `X-Accepted-OAuth-Scopes` header if `scope` is specified (deprecated)', function() { const model = { getAccessToken: function() {}, verifyScope: function() {} @@ -584,6 +654,19 @@ describe('AuthenticateHandler integration', function() { response.get('X-Accepted-OAuth-Scopes').should.equal('foo bar'); }); + it('should set the `X-Accepted-OAuth-Scopes` header if `scope` is specified', function() { + const model = { + getAccessToken: function() {}, + verifyScope: function() {} + }; + const handler = new AuthenticateHandler({ addAcceptedScopesHeader: true, addAuthorizedScopesHeader: false, model: model, scope: ['foo', 'bar'] }); + const response = new Response({ body: {}, headers: {} }); + + handler.updateResponse(response, { scope: ['foo', 'biz'] }); + + response.get('X-Accepted-OAuth-Scopes').should.equal('foo bar'); + }); + it('should not set the `X-Authorized-OAuth-Scopes` header if `scope` is not specified', function() { const model = { getAccessToken: function() {}, @@ -597,7 +680,7 @@ describe('AuthenticateHandler integration', function() { response.headers.should.not.have.property('x-oauth-scopes'); }); - it('should set the `X-Authorized-OAuth-Scopes` header', function() { + it('should set the `X-Authorized-OAuth-Scopes` header (deprecated)', function() { const model = { getAccessToken: function() {}, verifyScope: function() {} @@ -609,5 +692,18 @@ describe('AuthenticateHandler integration', function() { response.get('X-OAuth-Scopes').should.equal('foo biz'); }); + + it('should set the `X-Authorized-OAuth-Scopes` header', function() { + const model = { + getAccessToken: function() {}, + verifyScope: function() {} + }; + const handler = new AuthenticateHandler({ addAcceptedScopesHeader: false, addAuthorizedScopesHeader: true, model: model, scope: ['foo', 'bar'] }); + const response = new Response({ body: {}, headers: {} }); + + handler.updateResponse(response, { scope: ['foo', 'biz'] }); + + response.get('X-OAuth-Scopes').should.equal('foo biz'); + }); }); });