Skip to content

Commit

Permalink
feat: always return scope with token implicit response
Browse files Browse the repository at this point in the history 
As it was not reliable to compare the scope parameter with scope
granted, due to AS policy changing scope being implemented by changing
the scope parameter, the server will now always return scope with
the implicit and hybrid authorization response types including `token`.
  • Loading branch information
@panva
panva committed Jun 9, 2019
1 parent 4fb513e commit ea7b394
Show file tree
Hide file tree
Showing 9 changed files with 17 additions and 19 deletions.
4 changes: 1 addition & 3 deletions lib/actions/authorization/process_response_types.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,9 @@ async function tokenHandler(ctx) {
access_token: await token.save(),
expires_in: token.expiration,
token_type: 'Bearer',
scope: token.scope,
};

if (token.scope !== ctx.oidc.params.scope) {
result.scope = token.scope;
}

return result;
}
Expand Down
4 changes: 2 additions & 2 deletions test/claims/claims.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ expire.setDate(expire.getDate() + 1);
this.wrap({ route, verb, auth })
.expect(302)
.expect(auth.validateFragment)
.expect(auth.validatePresence(['access_token'], false))
.expect(auth.validatePresence(['access_token', 'scope'], false))
.end((err, response) => {
if (err) {
return done(err);
Expand Down Expand Up @@ -188,7 +188,7 @@ expire.setDate(expire.getDate() + 1);
this.wrap({ route, verb, auth })
.expect(302)
.expect(auth.validateFragment)
.expect(auth.validatePresence(['id_token', 'access_token'], false))
.expect(auth.validatePresence(['id_token', 'access_token', 'scope'], false))
.expect((response) => {
const { query: { id_token } } = parseLocation(response.headers.location, true);
const { payload } = decodeJWT(id_token);
Expand Down
4 changes: 2 additions & 2 deletions test/core/hybrid/code+id_token+token.authorization.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ describe('HYBRID code+id_token+token', () => {
await this.wrap({ route, verb, auth })
.expect(302)
.expect(auth.validateFragment)
.expect(auth.validatePresence(['code', 'id_token', 'state', 'access_token', 'expires_in', 'token_type']))
.expect(auth.validatePresence(['code', 'id_token', 'state', 'access_token', 'expires_in', 'token_type', 'scope']))
.expect(auth.validateState)
.expect(auth.validateClientLocation);

Expand All @@ -39,7 +39,7 @@ describe('HYBRID code+id_token+token', () => {
await this.wrap({ route, verb, auth })
.expect(302)
.expect(auth.validateFragment)
.expect(auth.validatePresence(['code', 'id_token', 'state', 'access_token', 'expires_in', 'token_type']))
.expect(auth.validatePresence(['code', 'id_token', 'state', 'access_token', 'expires_in', 'token_type', 'scope']))
.expect(auth.validateState)
.expect(auth.validateClientLocation);

Expand Down
4 changes: 2 additions & 2 deletions test/core/hybrid/code+token.authorization.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ describe('HYBRID code+token', () => {
return this.wrap({ route, verb, auth })
.expect(302)
.expect(auth.validateFragment)
.expect(auth.validatePresence(['code', 'state', 'access_token', 'expires_in', 'token_type']))
.expect(auth.validatePresence(['code', 'state', 'access_token', 'expires_in', 'token_type', 'scope']))
.expect(auth.validateState)
.expect(auth.validateClientLocation);
});
Expand All @@ -37,7 +37,7 @@ describe('HYBRID code+token', () => {
return this.wrap({ route, verb, auth })
.expect(302)
.expect(auth.validateFragment)
.expect(auth.validatePresence(['code', 'state', 'access_token', 'expires_in', 'token_type']))
.expect(auth.validatePresence(['code', 'state', 'access_token', 'expires_in', 'token_type', 'scope']))
.expect(auth.validateState)
.expect(auth.validateClientLocation);
});
Expand Down
6 changes: 3 additions & 3 deletions test/core/implicit/id_token+token.authorization.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ describe('IMPLICIT id_token+token', () => {
await this.wrap({ route, verb, auth })
.expect(302)
.expect(auth.validateFragment)
.expect(auth.validatePresence(['id_token', 'state', 'access_token', 'expires_in', 'token_type']))
.expect(auth.validatePresence(['id_token', 'state', 'access_token', 'expires_in', 'token_type', 'scope']))
.expect(auth.validateState)
.expect(auth.validateClientLocation);

Expand All @@ -41,7 +41,7 @@ describe('IMPLICIT id_token+token', () => {
await this.wrap({ route, verb, auth })
.expect(302)
.expect(auth.validateFragment)
.expect(auth.validatePresence(['id_token', 'state', 'access_token', 'expires_in', 'token_type']))
.expect(auth.validatePresence(['id_token', 'state', 'access_token', 'expires_in', 'token_type', 'scope']))
.expect(auth.validateState)
.expect(auth.validateClientLocation);

Expand Down Expand Up @@ -70,7 +70,7 @@ describe('IMPLICIT id_token+token', () => {
return this.wrap({ route, verb, auth })
.expect(302)
.expect(auth.validateFragment)
.expect(auth.validatePresence(['id_token', 'state', 'access_token', 'expires_in', 'token_type']))
.expect(auth.validatePresence(['id_token', 'state', 'access_token', 'expires_in', 'token_type', 'scope']))
.expect(auth.validateState)
.expect(auth.validateClientLocation)
.then((response) => {
Expand Down
2 changes: 1 addition & 1 deletion test/dynamic_scopes/dynamic_scopes.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ describe('dynamic scopes', () => {
await this.wrap({ route: '/auth', verb: 'get', auth })
.expect(302)
.expect(auth.validateFragment)
.expect(auth.validatePresence(['code', 'state', 'access_token', 'expires_in', 'token_type']))
.expect(auth.validatePresence(['code', 'state', 'access_token', 'expires_in', 'token_type', 'scope']))
.expect(auth.validateState)
.expect(auth.validateClientLocation)
.expect(({ headers: { location } }) => {
Expand Down
6 changes: 3 additions & 3 deletions test/resource_indicators/resource_indicators.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ describe('features.resourceIndicators', () => {
await this.wrap({ route: '/auth', verb: 'get', auth })
.expect(302)
.expect(auth.validateFragment)
.expect(auth.validatePresence(['id_token', 'state', 'access_token', 'expires_in', 'token_type']))
.expect(auth.validatePresence(['id_token', 'state', 'access_token', 'expires_in', 'token_type', 'scope']))
.expect(auth.validateState)
.expect(auth.validateClientLocation);

Expand All @@ -380,7 +380,7 @@ describe('features.resourceIndicators', () => {
await this.wrap({ route: '/auth', verb: 'get', auth })
.expect(302)
.expect(auth.validateFragment)
.expect(auth.validatePresence(['id_token', 'state', 'access_token', 'expires_in', 'token_type']))
.expect(auth.validatePresence(['id_token', 'state', 'access_token', 'expires_in', 'token_type', 'scope']))
.expect(auth.validateState)
.expect(auth.validateClientLocation);

Expand All @@ -401,7 +401,7 @@ describe('features.resourceIndicators', () => {
await this.wrap({ route: '/auth', verb: 'get', auth })
.expect(302)
.expect(auth.validateFragment)
.expect(auth.validatePresence(['id_token', 'state', 'access_token', 'expires_in', 'token_type']))
.expect(auth.validatePresence(['id_token', 'state', 'access_token', 'expires_in', 'token_type', 'scope']))
.expect(auth.validateState)
.expect(auth.validateClientLocation);

Expand Down
2 changes: 1 addition & 1 deletion test/session_bound_tokens/session_bound_tokens.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ describe('session bound tokens behaviours', () => {
.query(auth)
.expect(302)
.expect(auth.validateFragment)
.expect(auth.validatePresence(['id_token', 'state', 'access_token', 'expires_in', 'token_type']))
.expect(auth.validatePresence(['id_token', 'state', 'access_token', 'expires_in', 'token_type', 'scope']))
.expect(auth.validateState)
.expect(auth.validateClientLocation)
.expect(assignAuthorizationResponseValues.bind(this));
Expand Down
4 changes: 2 additions & 2 deletions test/web_message/web_message.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ describe('configuration features.webMessageResponseMode', () => {
expect(response).to.have.property('redirect_uri', auth.redirect_uri);
expect(response).to.have.property('web_message_uri', null);
expect(response).to.have.property('web_message_target', null);
expect(response.response).to.have.keys('id_token', 'state', 'access_token', 'expires_in', 'token_type');
expect(response.response).to.have.keys('id_token', 'state', 'access_token', 'scope', 'expires_in', 'token_type');
expect(response.response.id_token).to.be.a('string');
expect(response.response.expires_in).to.be.a('number');
expect(response.response.access_token).to.be.a('string');
Expand Down Expand Up @@ -90,7 +90,7 @@ describe('configuration features.webMessageResponseMode', () => {
expect(response).to.have.property('redirect_uri', auth.redirect_uri);
expect(response).to.have.property('web_message_uri', 'https://auth.example.com');
expect(response).to.have.property('web_message_target', 'targetID');
expect(response.response).to.have.keys('id_token', 'state', 'access_token', 'expires_in', 'token_type');
expect(response.response).to.have.keys('id_token', 'state', 'access_token', 'scope', 'expires_in', 'token_type');
expect(response.response.id_token).to.be.a('string');
expect(response.response.expires_in).to.be.a('number');
expect(response.response.access_token).to.be.a('string');
Expand Down

0 comments on commit ea7b394

Please sign in to comment.