Skip to content

Commit

Permalink
feat: allow omitting redirect_uri in code exchange at the token endpo…
Browse files Browse the repository at this point in the history 
…int 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).
  • Loading branch information
@panva
panva committed Nov 9, 2018
1 parent 3833296 commit 8cdd407
Show file tree
Hide file tree
Showing 3 changed files with 258 additions and 4 deletions.
8 changes: 8 additions & 0 deletions lib/actions/grants/authorization_code.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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, {
Expand Down
5 changes: 3 additions & 2 deletions test/authorization_code/authorization_code.config.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
}],
};
249 changes: 247 additions & 2 deletions test/authorization_code/code.grant.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(); });

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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({
Expand Down

0 comments on commit 8cdd407

Please sign in to comment.