Skip to content

Commit

Permalink
fix: explicitly support array of strings for AuthenticateHandler.options
Browse files Browse the repository at this point in the history
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
  • Loading branch information
dhensby committed Jul 28, 2024
1 parent f511c32 commit 47ee118
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 6 deletions.
2 changes: 1 addition & 1 deletion index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion lib/handlers/authenticate-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down
104 changes: 100 additions & 4 deletions test/integration/handlers/authenticate-handler_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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() {
Expand All @@ -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() {
Expand All @@ -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);
});
Expand All @@ -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() {}
Expand All @@ -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() {},
Expand All @@ -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() {}
Expand All @@ -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');
});
});
});

0 comments on commit 47ee118

Please sign in to comment.