Skip to content

Commit a2a54c4

Browse files
committed
fix: scope in body responses is always a string
1 parent e01a5e4 commit a2a54c4

File tree

9 files changed

+44
-107
lines changed

9 files changed

+44
-107
lines changed

lib/handlers/token-handler.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,12 @@ class TokenHandler {
266266
updateSuccessResponse (response, tokenType) {
267267
response.body = tokenType.valueOf();
268268

269+
// for compliance reasons we rebuild the internal scope to be a string
270+
// https://datatracker.ietf.org/doc/html/rfc6749.html#section-5.1
271+
if (response.body.scope) {
272+
response.body.scope = response.body.scope.join(' ');
273+
}
274+
269275
response.set('Cache-Control', 'no-store');
270276
response.set('Pragma', 'no-cache');
271277
}

lib/utils/scope-util.js

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,25 @@
11
const isFormat = require('@node-oauth/formats');
22
const InvalidScopeError = require('../errors/invalid-scope-error');
3+
const toArray = s => Array.isArray(s) ? s : s.split(' ');
34

45
module.exports = {
56
parseScope: function (requestedScope) {
6-
if (!isFormat.nqschar(requestedScope)) {
7-
throw new InvalidScopeError('Invalid parameter: `scope`');
7+
if (typeof requestedScope === 'undefined' || requestedScope === null) {
8+
return undefined;
89
}
910

10-
if (requestedScope == null) {
11-
return undefined;
11+
const internalScope = toArray(requestedScope);
12+
13+
if (internalScope.length === 0) {
14+
throw new InvalidScopeError('Invalid parameter: `scope`');
1215
}
1316

14-
return requestedScope.split(' ');
17+
internalScope.forEach(value => {
18+
if (!isFormat.nqschar(value)) {
19+
throw new InvalidScopeError('Invalid parameter: `scope`');
20+
}
21+
});
22+
23+
return internalScope;
1524
}
1625
};

lib/validator/is.js

Lines changed: 0 additions & 90 deletions
This file was deleted.

test/compliance/client-authentication_test.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,14 @@ const client = db.saveClient({ id: 'a', secret: 'b', grants: ['password'] });
3737
const scope = 'read write';
3838

3939
function createDefaultRequest () {
40+
const dice = Math.random() > 0.5;
41+
const currentScope = dice ? scope : scope.split(' ');
4042
return createRequest({
4143
body: {
4244
grant_type: 'password',
4345
username: user.username,
4446
password: user.password,
45-
scope
47+
scope: currentScope
4648
},
4749
headers: {
4850
'authorization': 'Basic ' + Buffer.from(client.id + ':' + client.secret).toString('base64'),

test/compliance/client-credential-workflow_test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ describe('ClientCredentials Workflow Compliance (4.4)', function () {
9090
response.body.token_type.should.equal('Bearer');
9191
response.body.access_token.should.equal(token.accessToken);
9292
response.body.expires_in.should.be.a('number');
93-
response.body.scope.should.eql(['read', 'write']);
93+
response.body.scope.should.eql('read write');
9494
('refresh_token' in response.body).should.equal(false);
9595

9696
token.accessToken.should.be.a('string');

test/compliance/password-grant-type_test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ describe('PasswordGrantType Compliance', function () {
101101
response.body.access_token.should.equal(token.accessToken);
102102
response.body.refresh_token.should.equal(token.refreshToken);
103103
response.body.expires_in.should.be.a('number');
104-
response.body.scope.should.eql(['read', 'write']);
104+
response.body.scope.should.eql('read write');
105105

106106
token.accessToken.should.be.a('string');
107107
token.refreshToken.should.be.a('string');

test/compliance/refresh-token-grant-type_test.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ describe('RefreshTokenGrantType Compliance', function () {
124124
refreshResponse.body.access_token.should.equal(token.accessToken);
125125
refreshResponse.body.refresh_token.should.equal(token.refreshToken);
126126
refreshResponse.body.expires_in.should.be.a('number');
127-
refreshResponse.body.scope.should.eql(['read', 'write']);
127+
refreshResponse.body.scope.should.eql('read write');
128128

129129
token.accessToken.should.be.a('string');
130130
token.refreshToken.should.be.a('string');
@@ -223,7 +223,7 @@ describe('RefreshTokenGrantType Compliance', function () {
223223
refreshResponse.body.access_token.should.equal(token.accessToken);
224224
refreshResponse.body.refresh_token.should.equal(token.refreshToken);
225225
refreshResponse.body.expires_in.should.be.a('number');
226-
refreshResponse.body.scope.should.eql(['read']);
226+
refreshResponse.body.scope.should.eql('read');
227227
});
228228
});
229229
});

test/helpers/model.js

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@ function createModel (db) {
1010
}
1111

1212
async function saveToken (token, client, user) {
13+
if (token.scope && !Array.isArray(token.scope)) {
14+
throw new Error('Scope should internally be an array');
15+
}
1316
const meta = {
1417
clientId: client.id,
1518
userId: user.id,
@@ -38,7 +41,9 @@ function createModel (db) {
3841
if (!meta) {
3942
return false;
4043
}
41-
44+
if (meta.scope && !Array.isArray(meta.scope)) {
45+
throw new Error('Scope should internally be an array');
46+
}
4247
return {
4348
accessToken,
4449
accessTokenExpiresAt: meta.accessTokenExpiresAt,
@@ -54,7 +59,9 @@ function createModel (db) {
5459
if (!meta) {
5560
return false;
5661
}
57-
62+
if (meta.scope && !Array.isArray(meta.scope)) {
63+
throw new Error('Scope should internally be an array');
64+
}
5865
return {
5966
refreshToken,
6067
refreshTokenExpiresAt: meta.refreshTokenExpiresAt,
@@ -71,6 +78,9 @@ function createModel (db) {
7178
}
7279

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

test/integration/handlers/token-handler_test.js

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ describe('TokenHandler integration', function() {
329329
});
330330

331331
it('should not return custom attributes in a bearer token if the allowExtendedTokenAttributes is not set', function() {
332-
const token = { accessToken: 'foo', client: {}, refreshToken: 'bar', scope: ['foobar'], user: {}, foo: 'bar' };
332+
const token = { accessToken: 'foo', client: {}, refreshToken: 'bar', scope: ['baz'], user: {}, foo: 'bar' };
333333
const model = {
334334
getClient: function() { return { grants: ['password'] }; },
335335
getUser: function() { return {}; },
@@ -344,7 +344,7 @@ describe('TokenHandler integration', function() {
344344
username: 'foo',
345345
password: 'bar',
346346
grant_type: 'password',
347-
scope: 'baz'
347+
scope: ['baz']
348348
},
349349
headers: { 'content-type': 'application/x-www-form-urlencoded', 'transfer-encoding': 'chunked' },
350350
method: 'POST',
@@ -357,14 +357,14 @@ describe('TokenHandler integration', function() {
357357
should.exist(response.body.access_token);
358358
should.exist(response.body.refresh_token);
359359
should.exist(response.body.token_type);
360-
should.exist(response.body.scope);
360+
response.body.scope.should.eql('baz');
361361
should.not.exist(response.body.foo);
362362
})
363363
.catch(should.fail);
364364
});
365365

366366
it('should return custom attributes in a bearer token if the allowExtendedTokenAttributes is set', function() {
367-
const token = { accessToken: 'foo', client: {}, refreshToken: 'bar', scope: ['foobar'], user: {}, foo: 'bar' };
367+
const token = { accessToken: 'foo', client: {}, refreshToken: 'bar', scope: ['baz'], user: {}, foo: 'bar' };
368368
const model = {
369369
getClient: function() { return { grants: ['password'] }; },
370370
getUser: function() { return {}; },
@@ -392,7 +392,7 @@ describe('TokenHandler integration', function() {
392392
should.exist(response.body.access_token);
393393
should.exist(response.body.refresh_token);
394394
should.exist(response.body.token_type);
395-
should.exist(response.body.scope);
395+
response.body.scope.should.eql('baz');
396396
should.exist(response.body.foo);
397397
})
398398
.catch(should.fail);

0 commit comments

Comments
 (0)