Skip to content

Commit

Permalink
Merge branch 'development' into dependabot/npm_and_yarn/eslint-8.54.0
Browse files Browse the repository at this point in the history
  • Loading branch information
jankapunkt authored Nov 30, 2023
2 parents 32fe5fe + 9562aa9 commit 6c670e2
Show file tree
Hide file tree
Showing 13 changed files with 98 additions and 117 deletions.
8 changes: 4 additions & 4 deletions index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ declare namespace OAuth2Server {
* Validate requested scope. Calls Model#validateScope() if implemented.
*
*/
validateScope(user: User, client: Client, scope: string[]): Promise<string[] | Falsey>;
validateScope(user: User, client: Client, scope?: string[]): Promise<string[] | Falsey>;

/**
* Retrieve info from the request and client and return token
Expand Down Expand Up @@ -314,7 +314,7 @@ declare namespace OAuth2Server {
* Invoked to check if the requested scope is valid for a particular client/user combination.
*
*/
validateScope?(user: User, client: Client, scope: string[]): Promise<string[] | Falsey>;
validateScope?(user: User, client: Client, scope?: string[]): Promise<string[] | Falsey>;

/**
* Invoked to check if the provided `redirectUri` is valid for a particular `client`.
Expand All @@ -340,7 +340,7 @@ declare namespace OAuth2Server {
* Invoked to check if the requested scope is valid for a particular client/user combination.
*
*/
validateScope?(user: User, client: Client, scope: string[]): Promise<string[] | Falsey>;
validateScope?(user: User, client: Client, scope?: string[]): Promise<string[] | Falsey>;
}

interface RefreshTokenModel extends BaseModel, RequestAuthenticationModel {
Expand Down Expand Up @@ -374,7 +374,7 @@ declare namespace OAuth2Server {
* Invoked to check if the requested scope is valid for a particular client/user combination.
*
*/
validateScope?(user: User, client: Client, scope: string[]): Promise<string[] | Falsey>;
validateScope?(user: User, client: Client, scope?: string[]): Promise<string[] | Falsey>;
}

interface ExtensionModel extends BaseModel, RequestAuthenticationModel {}
Expand Down
3 changes: 2 additions & 1 deletion lib/handlers/authenticate-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const Request = require('../request');
const Response = require('../response');
const ServerError = require('../errors/server-error');
const UnauthorizedRequestError = require('../errors/unauthorized-request-error');
const { parseScope } = require('../utils/scope-util');

/**
* Constructor.
Expand Down Expand Up @@ -46,7 +47,7 @@ class AuthenticateHandler {
this.addAuthorizedScopesHeader = options.addAuthorizedScopesHeader;
this.allowBearerTokensInQueryString = options.allowBearerTokensInQueryString;
this.model = options.model;
this.scope = options.scope;
this.scope = parseScope(options.scope);
}

/**
Expand Down
6 changes: 6 additions & 0 deletions lib/handlers/token-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,12 @@ class TokenHandler {
updateSuccessResponse (response, tokenType) {
response.body = tokenType.valueOf();

// for compliance reasons we rebuild the internal scope to be a string
// https://datatracker.ietf.org/doc/html/rfc6749.html#section-5.1
if (response.body.scope) {
response.body.scope = response.body.scope.join(' ');
}

response.set('Cache-Control', 'no-store');
response.set('Pragma', 'no-cache');
}
Expand Down
17 changes: 13 additions & 4 deletions lib/utils/scope-util.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,25 @@
const isFormat = require('@node-oauth/formats');
const InvalidScopeError = require('../errors/invalid-scope-error');
const whiteSpace = /\s+/g;

module.exports = {
parseScope: function (requestedScope) {
if (!isFormat.nqschar(requestedScope)) {
if (requestedScope == null) {
return undefined;
}

if (typeof requestedScope !== 'string') {
throw new InvalidScopeError('Invalid parameter: `scope`');
}

if (requestedScope == null) {
return undefined;
// XXX: this prevents spaced-only strings to become
// treated as valid nqchar by making them empty strings
requestedScope = requestedScope.trim();

if(!isFormat.nqschar(requestedScope)) {
throw new InvalidScopeError('Invalid parameter: `scope`');
}

return requestedScope.split(' ');
return requestedScope.split(whiteSpace);
}
};
90 changes: 0 additions & 90 deletions lib/validator/is.js

This file was deleted.

2 changes: 1 addition & 1 deletion test/compliance/client-credential-workflow_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ describe('ClientCredentials Workflow Compliance (4.4)', function () {
response.body.token_type.should.equal('Bearer');
response.body.access_token.should.equal(token.accessToken);
response.body.expires_in.should.be.a('number');
response.body.scope.should.eql(['read', 'write']);
response.body.scope.should.eql('read write');
('refresh_token' in response.body).should.equal(false);

token.accessToken.should.be.a('string');
Expand Down
2 changes: 1 addition & 1 deletion test/compliance/password-grant-type_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ describe('PasswordGrantType Compliance', function () {
response.body.access_token.should.equal(token.accessToken);
response.body.refresh_token.should.equal(token.refreshToken);
response.body.expires_in.should.be.a('number');
response.body.scope.should.eql(['read', 'write']);
response.body.scope.should.eql('read write');

token.accessToken.should.be.a('string');
token.refreshToken.should.be.a('string');
Expand Down
4 changes: 2 additions & 2 deletions test/compliance/refresh-token-grant-type_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ describe('RefreshTokenGrantType Compliance', function () {
refreshResponse.body.access_token.should.equal(token.accessToken);
refreshResponse.body.refresh_token.should.equal(token.refreshToken);
refreshResponse.body.expires_in.should.be.a('number');
refreshResponse.body.scope.should.eql(['read', 'write']);
refreshResponse.body.scope.should.eql('read write');

token.accessToken.should.be.a('string');
token.refreshToken.should.be.a('string');
Expand Down Expand Up @@ -223,7 +223,7 @@ describe('RefreshTokenGrantType Compliance', function () {
refreshResponse.body.access_token.should.equal(token.accessToken);
refreshResponse.body.refresh_token.should.equal(token.refreshToken);
refreshResponse.body.expires_in.should.be.a('number');
refreshResponse.body.scope.should.eql(['read']);
refreshResponse.body.scope.should.eql('read');
});
});
});
14 changes: 12 additions & 2 deletions test/helpers/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ function createModel (db) {
}

async function saveToken (token, client, user) {
if (token.scope && !Array.isArray(token.scope)) {
throw new Error('Scope should internally be an array');
}
const meta = {
clientId: client.id,
userId: user.id,
Expand Down Expand Up @@ -38,7 +41,9 @@ function createModel (db) {
if (!meta) {
return false;
}

if (meta.scope && !Array.isArray(meta.scope)) {
throw new Error('Scope should internally be an array');
}
return {
accessToken,
accessTokenExpiresAt: meta.accessTokenExpiresAt,
Expand All @@ -54,7 +59,9 @@ function createModel (db) {
if (!meta) {
return false;
}

if (meta.scope && !Array.isArray(meta.scope)) {
throw new Error('Scope should internally be an array');
}
return {
refreshToken,
refreshTokenExpiresAt: meta.refreshTokenExpiresAt,
Expand All @@ -71,6 +78,9 @@ function createModel (db) {
}

async function verifyScope (token, scope) {
if (!Array.isArray(scope)) {
throw new Error('Scope should internally be an array');
}
return scope.every(s => scopes.includes(s));
}

Expand Down
14 changes: 7 additions & 7 deletions test/integration/handlers/authenticate-handler_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ describe('AuthenticateHandler integration', function() {
addAcceptedScopesHeader: true,
addAuthorizedScopesHeader: true,
model: model,
scope: ['foobar']
scope: 'foobar'
});

grantType.scope.should.eql(['foobar']);
Expand Down Expand Up @@ -254,7 +254,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' });
const request = new Request({
body: {},
headers: { 'Authorization': 'Bearer foo' },
Expand Down Expand Up @@ -522,7 +522,7 @@ describe('AuthenticateHandler integration', function() {
return false;
}
};
const handler = new AuthenticateHandler({ addAcceptedScopesHeader: true, addAuthorizedScopesHeader: true, model: model, scope: ['foo'] });
const handler = new AuthenticateHandler({ addAcceptedScopesHeader: true, addAuthorizedScopesHeader: true, model: model, scope: 'foo' });

return handler.verifyScope(['foo'])
.then(should.fail)
Expand All @@ -539,7 +539,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 @@ -551,7 +551,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 @@ -576,7 +576,7 @@ describe('AuthenticateHandler integration', function() {
getAccessToken: function() {},
verifyScope: function() {}
};
const handler = new AuthenticateHandler({ addAcceptedScopesHeader: true, addAuthorizedScopesHeader: false, model: model, scope: ['foo', 'bar'] });
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'] });
Expand All @@ -602,7 +602,7 @@ describe('AuthenticateHandler integration', function() {
getAccessToken: function() {},
verifyScope: function() {}
};
const handler = new AuthenticateHandler({ addAcceptedScopesHeader: false, addAuthorizedScopesHeader: true, model: model, scope: ['foo', 'bar'] });
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'] });
Expand Down
8 changes: 4 additions & 4 deletions test/integration/handlers/token-handler_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ describe('TokenHandler integration', function() {
});

it('should not return custom attributes in a bearer token if the allowExtendedTokenAttributes is not set', function() {
const token = { accessToken: 'foo', client: {}, refreshToken: 'bar', scope: ['foobar'], user: {}, foo: 'bar' };
const token = { accessToken: 'foo', client: {}, refreshToken: 'bar', scope: ['baz'], user: {}, foo: 'bar' };
const model = {
getClient: function() { return { grants: ['password'] }; },
getUser: function() { return {}; },
Expand Down Expand Up @@ -357,14 +357,14 @@ describe('TokenHandler integration', function() {
should.exist(response.body.access_token);
should.exist(response.body.refresh_token);
should.exist(response.body.token_type);
should.exist(response.body.scope);
response.body.scope.should.eql('baz');
should.not.exist(response.body.foo);
})
.catch(should.fail);
});

it('should return custom attributes in a bearer token if the allowExtendedTokenAttributes is set', function() {
const token = { accessToken: 'foo', client: {}, refreshToken: 'bar', scope: ['foobar'], user: {}, foo: 'bar' };
const token = { accessToken: 'foo', client: {}, refreshToken: 'bar', scope: ['baz'], user: {}, foo: 'bar' };
const model = {
getClient: function() { return { grants: ['password'] }; },
getUser: function() { return {}; },
Expand Down Expand Up @@ -392,7 +392,7 @@ describe('TokenHandler integration', function() {
should.exist(response.body.access_token);
should.exist(response.body.refresh_token);
should.exist(response.body.token_type);
should.exist(response.body.scope);
response.body.scope.should.eql('baz');
should.exist(response.body.foo);
})
.catch(should.fail);
Expand Down
2 changes: 1 addition & 1 deletion test/unit/handlers/authenticate-handler_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ describe('AuthenticateHandler', function() {
getAccessToken: function() {},
verifyScope: sinon.stub().returns(true)
};
const handler = new AuthenticateHandler({ addAcceptedScopesHeader: true, addAuthorizedScopesHeader: true, model: model, scope: ['bar'] });
const handler = new AuthenticateHandler({ addAcceptedScopesHeader: true, addAuthorizedScopesHeader: true, model: model, scope: 'bar' });

return handler.verifyScope(['foo'])
.then(function() {
Expand Down
Loading

0 comments on commit 6c670e2

Please sign in to comment.