From 8cdd4071a7624ff090c9e2a58b52a54eabcf51a9 Mon Sep 17 00:00:00 2001 From: Filip Skokan Date: Fri, 9 Nov 2018 20:45:28 +0100 Subject: [PATCH] feat: allow omitting redirect_uri in code exchange at the token endpoint when there is just one registered https://openid.net/specs/openid-connect-core-1_0.html#TokenRequestValidation Ensure that the redirect_uri parameter value is identical to the redirect_uri parameter value that was included in the initial Authorization Request. If the redirect_uri parameter value is not present when there is only one registered redirect_uri value, the Authorization Server MAY return an error (since the Client should have included the parameter) or MAY proceed without an error (since OAuth 2.0 permits the parameter to be omitted in this case). --- lib/actions/grants/authorization_code.js | 8 + .../authorization_code.config.js | 5 +- test/authorization_code/code.grant.test.js | 249 +++++++++++++++++- 3 files changed, 258 insertions(+), 4 deletions(-) diff --git a/lib/actions/grants/authorization_code.js b/lib/actions/grants/authorization_code.js index 7268aa701..6d2ed6f12 100644 --- a/lib/actions/grants/authorization_code.js +++ b/lib/actions/grants/authorization_code.js @@ -15,6 +15,14 @@ module.exports.handler = function getAuthorizationCodeHandler(provider) { const checkPKCE = getCheckPKCE(provider); return async function authorizationCodeResponse(ctx, next) { + if (ctx.oidc.params.redirect_uri === undefined) { + // It is permitted to omit the redirect_uri if only ONE is registered on the client + const { 0: uri, length } = ctx.oidc.client.redirectUris; + if (uri && length === 1) { + ctx.oidc.params.redirect_uri = uri; + } + } + presence(ctx, 'code', 'redirect_uri'); const code = await provider.AuthorizationCode.find(ctx.oidc.params.code, { diff --git a/test/authorization_code/authorization_code.config.js b/test/authorization_code/authorization_code.config.js index ce92aaa62..ab92af886 100644 --- a/test/authorization_code/authorization_code.config.js +++ b/test/authorization_code/authorization_code.config.js @@ -7,10 +7,11 @@ module.exports = { client_secret: 'secret', grant_types: ['authorization_code', 'refresh_token'], response_types: ['code'], - redirect_uris: ['https://client.example.com/cb'], + redirect_uris: ['https://client.example.com/cb', 'https://client.example.com/cb2'], }, { client_id: 'client2', client_secret: 'secret', - redirect_uris: ['https://client.example.com/cb'], + grant_types: ['authorization_code', 'refresh_token'], + redirect_uris: ['https://client.example.com/cb3'], }], }; diff --git a/test/authorization_code/code.grant.test.js b/test/authorization_code/code.grant.test.js index 051d29ae1..190af6581 100644 --- a/test/authorization_code/code.grant.test.js +++ b/test/authorization_code/code.grant.test.js @@ -18,7 +18,7 @@ describe('grant_type=authorization_code', () => { afterEach(() => timekeeper.reset()); - context('with real tokens', () => { + context('with real tokens (1/2) - more than two redirect_uris registered', () => { before(function () { return this.login(); }); after(function () { return this.logout(); }); @@ -273,6 +273,251 @@ describe('grant_type=authorization_code', () => { }); }); + context('with real tokens (2/2) - one redirect_uri registered', () => { + before(function () { return this.login(); }); + after(function () { return this.logout(); }); + + beforeEach(function () { + return this.agent.get('/auth') + .query({ + client_id: 'client2', + scope: 'openid', + response_type: 'code', + }) + .expect(302) + .expect((response) => { + const { query: { code } } = parseUrl(response.headers.location, true); + const jti = this.getTokenJti(code); + this.code = this.TestAdapter.for('AuthorizationCode').syncFind(jti); + this.ac = code; + }); + }); + + it('returns the right stuff', function () { + const spy = sinon.spy(); + this.provider.once('grant.success', spy); + + return this.agent.post(route) + .auth('client2', 'secret') + .type('form') + .send({ + code: this.ac, + grant_type: 'authorization_code', + }) + .expect(200) + .expect(() => { + expect(spy.calledOnce).to.be.true; + }) + .expect((response) => { + expect(response.body).to.have.keys('access_token', 'id_token', 'expires_in', 'token_type', 'scope'); + expect(response.body).not.to.have.key('refresh_token'); + }); + }); + + it('populates ctx.oidc.entities (no offline_access)', function (done) { + this.provider.use(this.assertOnce((ctx) => { + expect(ctx.oidc.entities).to.have.keys('Account', 'Client', 'AuthorizationCode', 'AccessToken'); + expect(ctx.oidc.entities.AccessToken).to.have.property('gty', 'authorization_code'); + }, done)); + + this.agent.post(route) + .auth('client2', 'secret') + .type('form') + .send({ + code: this.ac, + grant_type: 'authorization_code', + }) + .end(() => {}); + }); + + it('populates ctx.oidc.entities (w/ offline_access)', function (done) { + this.provider.use(this.assertOnce((ctx) => { + expect(ctx.oidc.entities).to.have.keys('Account', 'Client', 'AuthorizationCode', 'AccessToken', 'RefreshToken'); + expect(ctx.oidc.entities.AccessToken).to.have.property('gty', 'authorization_code'); + expect(ctx.oidc.entities.RefreshToken).to.have.property('gty', 'authorization_code'); + }, done)); + + this.TestAdapter.for('AuthorizationCode').syncUpdate(this.getTokenJti(this.ac), { + scope: 'openid offline_access', + }); + this.agent.post(route) + .auth('client2', 'secret') + .type('form') + .send({ + code: this.ac, + grant_type: 'authorization_code', + }) + .end(() => {}); + }); + + it('returns token-endpoint-like cache headers', function () { + return this.agent.post(route) + .auth('client2', 'secret') + .type('form') + .send({ + code: this.ac, + grant_type: 'authorization_code', + }) + .expect('pragma', 'no-cache') + .expect('cache-control', 'no-cache, no-store'); + }); + + context('', () => { + before(function () { + const ttl = i(this.provider).configuration('ttl'); + this.prev = ttl.AuthorizationCode; + ttl.AuthorizationCode = 5; + }); + + after(function () { + i(this.provider).configuration('ttl').AuthorizationCode = this.prev; + }); + + it('validates code is not expired', function () { + timekeeper.travel(Date.now() + (10 * 1000)); + const spy = sinon.spy(); + this.provider.once('grant.error', spy); + + return this.agent.post(route) + .auth('client2', 'secret') + .send({ + code: this.ac, + grant_type: 'authorization_code', + }) + .type('form') + .expect(400) + .expect(() => { + expect(spy.calledOnce).to.be.true; + expect(errorDetail(spy)).to.equal('authorization code is expired'); + }) + .expect((response) => { + expect(response.body).to.have.property('error', 'invalid_grant'); + }); + }); + }); + + it('validates code is not already used', function () { + const spy = sinon.spy(); + this.provider.once('grant.error', spy); + + this.code.consumed = epochTime(); + + return this.agent.post(route) + .auth('client2', 'secret') + .send({ + code: this.ac, + grant_type: 'authorization_code', + }) + .type('form') + .expect(400) + .expect(() => { + expect(spy.calledOnce).to.be.true; + expect(errorDetail(spy)).to.equal('authorization code already consumed'); + }) + .expect((response) => { + expect(response.body).to.have.property('error', 'invalid_grant'); + }); + }); + + it('consumes the code', function () { + return this.agent.post(route) + .auth('client2', 'secret') + .send({ + code: this.ac, + grant_type: 'authorization_code', + }) + .type('form') + .expect(() => { + expect(this.code).to.have.property('consumed').and.be.most(epochTime()); + }) + .expect(200); + }); + + it('validates code belongs to client', function () { + const spy = sinon.spy(); + this.provider.once('grant.error', spy); + + return this.agent.post(route) + .auth('client', 'secret') + .send({ + code: this.ac, + grant_type: 'authorization_code', + redirect_uri: 'https://client.example.com/cb3', + }) + .type('form') + .expect(400) + .expect(() => { + expect(spy.calledOnce).to.be.true; + expect(errorDetail(spy)).to.equal('authorization code client mismatch'); + }) + .expect((response) => { + expect(response.body).to.have.property('error', 'invalid_grant'); + }); + }); + + it('validates a grant type is supported', function () { + return this.agent.post(route) + .auth('client2', 'secret') + .send({ + code: this.ac, + grant_type: 'foobar', + }) + .type('form') + .expect(400) + .expect((response) => { + expect(response.body).to.have.property('error', 'unsupported_grant_type'); + }); + }); + + it('validates used redirect_uri (should it be provided)', function () { + const spy = sinon.spy(); + this.provider.once('grant.error', spy); + + return this.agent.post(route) + .auth('client2', 'secret') + .send({ + code: this.ac, + grant_type: 'authorization_code', + redirect_uri: 'https://client.example.com/cb?thensome', + }) + .type('form') + .expect(400) + .expect(() => { + expect(spy.calledOnce).to.be.true; + expect(errorDetail(spy)).to.equal('authorization code redirect_uri mismatch'); + }) + .expect((response) => { + expect(response.body).to.have.property('error', 'invalid_grant'); + }); + }); + + it('validates account is still there', function () { + sinon.stub(this.provider.Account, 'findById').callsFake(() => Promise.resolve()); + + const spy = sinon.spy(); + this.provider.once('grant.error', spy); + + return this.agent.post(route) + .auth('client2', 'secret') + .send({ + code: this.ac, + grant_type: 'authorization_code', + }) + .type('form') + .expect(() => { + this.provider.Account.findById.restore(); + }) + .expect(400) + .expect(() => { + expect(spy.calledOnce).to.be.true; + expect(errorDetail(spy)).to.equal('authorization code invalid (referenced account not found)'); + }) + .expect((response) => { + expect(response.body).to.have.property('error', 'invalid_grant'); + }); + }); + }); + describe('validates', () => { it('grant_type presence', function () { return this.agent.post(route) @@ -303,7 +548,7 @@ describe('grant_type=authorization_code', () => { }); }); - it('redirect_uri presence', function () { + it('redirect_uri presence (more then one registered)', function () { return this.agent.post(route) .auth('client', 'secret') .send({