Skip to content

Commit

Permalink
fix: scope in body responses is always a string
Browse files Browse the repository at this point in the history
jankapunkt committed Nov 24, 2023
1 parent e01a5e4 commit a2a54c4
Showing 9 changed files with 44 additions and 107 deletions.
6 changes: 6 additions & 0 deletions lib/handlers/token-handler.js
Original file line number Diff line number Diff line change
@@ -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');
}
19 changes: 14 additions & 5 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 toArray = s => Array.isArray(s) ? s : s.split(' ');

module.exports = {
parseScope: function (requestedScope) {
if (!isFormat.nqschar(requestedScope)) {
throw new InvalidScopeError('Invalid parameter: `scope`');
if (typeof requestedScope === 'undefined' || requestedScope === null) {
return undefined;
}

if (requestedScope == null) {
return undefined;
const internalScope = toArray(requestedScope);

if (internalScope.length === 0) {
throw new InvalidScopeError('Invalid parameter: `scope`');
}

return requestedScope.split(' ');
internalScope.forEach(value => {
if (!isFormat.nqschar(value)) {
throw new InvalidScopeError('Invalid parameter: `scope`');
}
});

return internalScope;
}
};
90 changes: 0 additions & 90 deletions lib/validator/is.js

This file was deleted.

4 changes: 3 additions & 1 deletion test/compliance/client-authentication_test.js
Original file line number Diff line number Diff line change
@@ -37,12 +37,14 @@ const client = db.saveClient({ id: 'a', secret: 'b', grants: ['password'] });
const scope = 'read write';

function createDefaultRequest () {
const dice = Math.random() > 0.5;
const currentScope = dice ? scope : scope.split(' ');
return createRequest({
body: {
grant_type: 'password',
username: user.username,
password: user.password,
scope
scope: currentScope
},
headers: {
'authorization': 'Basic ' + Buffer.from(client.id + ':' + client.secret).toString('base64'),
2 changes: 1 addition & 1 deletion test/compliance/client-credential-workflow_test.js
Original file line number Diff line number Diff line change
@@ -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');
2 changes: 1 addition & 1 deletion test/compliance/password-grant-type_test.js
Original file line number Diff line number Diff line change
@@ -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');
4 changes: 2 additions & 2 deletions test/compliance/refresh-token-grant-type_test.js
Original file line number Diff line number Diff line change
@@ -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');
@@ -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
@@ -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,
@@ -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,
@@ -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,
@@ -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));
}

10 changes: 5 additions & 5 deletions test/integration/handlers/token-handler_test.js
Original file line number Diff line number Diff line change
@@ -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 {}; },
@@ -344,7 +344,7 @@ describe('TokenHandler integration', function() {
username: 'foo',
password: 'bar',
grant_type: 'password',
scope: 'baz'
scope: ['baz']
},
headers: { 'content-type': 'application/x-www-form-urlencoded', 'transfer-encoding': 'chunked' },
method: 'POST',
@@ -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 {}; },
@@ -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);

0 comments on commit a2a54c4

Please sign in to comment.