Skip to content

Commit

Permalink
fix: expose invalid_dpop_proof error code and set it to 401 on userinfo
Browse files Browse the repository at this point in the history
  • Loading branch information
panva committed Dec 3, 2021
1 parent 008072c commit 2628d7e
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 38 deletions.
5 changes: 3 additions & 2 deletions lib/actions/userinfo.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const { InvalidDpopProof, InvalidToken, InsufficientScope } = require('../helpers/errors');
const { InvalidToken, InsufficientScope, InvalidDpopProof } = require('../helpers/errors');
const difference = require('../helpers/_/difference');
const setWWWAuthenticate = require('../helpers/set_www_authenticate');
const bodyParser = require('../shared/conditional_body');
Expand Down Expand Up @@ -43,7 +43,8 @@ module.exports = [
}

if (err instanceof InvalidDpopProof) {
err.error = err.message = 'invalid_token'; // eslint-disable-line no-multi-assign
// eslint-disable-next-line no-multi-assign
err.status = err.statusCode = 401;
}

setWWWAuthenticate(ctx, scheme, {
Expand Down
11 changes: 7 additions & 4 deletions lib/helpers/validate_dpop.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,28 +44,31 @@ module.exports = async (ctx, accessToken) => {
);

if (typeof payload.jti !== 'string' || !payload.jti) {
throw new Error('must have a jti string property');
throw new InvalidDpopProof('DPoP Proof must have a jti string property');
}

if (payload.htm !== ctx.method) {
throw new Error('htm mismatch');
throw new InvalidDpopProof('DPoP Proof htm mismatch');
}

if (payload.htu !== ctx.oidc.urlFor(ctx.oidc.route)) {
throw new Error('htu mismatch');
throw new InvalidDpopProof('DPoP Proof htu mismatch');
}

if (accessToken) {
const ath = base64url.encode(createHash('sha256').update(accessToken).digest());
if (payload.ath !== ath) {
throw new Error('ath mismatch');
throw new InvalidDpopProof('DPoP Proof ath mismatch');
}
}

const thumbprint = await calculateJwkThumbprint(jwk);

return { thumbprint, jti: payload.jti, iat: payload.iat };
} catch (err) {
if (err instanceof InvalidDpopProof) {
throw err;
}
throw new InvalidDpopProof('invalid DPoP key binding', err.message);
}
};
76 changes: 44 additions & 32 deletions test/dpop/dpop.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,100 +118,100 @@ describe('features.dPoP', () => {
await this.agent.get('/me') // eslint-disable-line no-await-in-loop
.set('DPoP', JWT.sign({}, key, { kid: false, header: { jwk: key, typ: value } }))
.set('Authorization', `DPoP ${dpop}`)
.expect(400)
.expect({ error: 'invalid_token', error_description: 'invalid DPoP key binding' })
.expect(401)
.expect({ error: 'invalid_dpop_proof', error_description: 'invalid DPoP key binding' })
.expect('WWW-Authenticate', /^DPoP /)
.expect('WWW-Authenticate', /error="invalid_token"/)
.expect('WWW-Authenticate', /error="invalid_dpop_proof"/)
.expect('WWW-Authenticate', /algs="ES256 PS256"/);
}

for (const value of [1, true, 'none', 'HS256', 'unsupported']) { // eslint-disable-line no-restricted-syntax
await this.agent.get('/me') // eslint-disable-line no-await-in-loop
.set('DPoP', `${base64url.encode(JSON.stringify({ jwk: key, typ: 'dpop+jwt', alg: value }))}.e30.`)
.set('Authorization', `DPoP ${dpop}`)
.expect(400)
.expect({ error: 'invalid_token', error_description: 'invalid DPoP key binding' })
.expect(401)
.expect({ error: 'invalid_dpop_proof', error_description: 'invalid DPoP key binding' })
.expect('WWW-Authenticate', /^DPoP /)
.expect('WWW-Authenticate', /error="invalid_token"/)
.expect('WWW-Authenticate', /error="invalid_dpop_proof"/)
.expect('WWW-Authenticate', /algs="ES256 PS256"/);
}

for (const value of [undefined, '', 1, true, null, 'foo', []]) { // eslint-disable-line no-restricted-syntax
await this.agent.get('/me') // eslint-disable-line no-await-in-loop
.set('DPoP', JWT.sign({}, key, { kid: false, header: { typ: 'dpop+jwt', jwk: value } }))
.set('Authorization', `DPoP ${dpop}`)
.expect(400)
.expect({ error: 'invalid_token', error_description: 'invalid DPoP key binding' })
.expect(401)
.expect({ error: 'invalid_dpop_proof', error_description: 'invalid DPoP key binding' })
.expect('WWW-Authenticate', /^DPoP /)
.expect('WWW-Authenticate', /error="invalid_token"/)
.expect('WWW-Authenticate', /error="invalid_dpop_proof"/)
.expect('WWW-Authenticate', /algs="ES256 PS256"/);
}

await this.agent.get('/me') // eslint-disable-line no-await-in-loop
.set('DPoP', JWT.sign({}, key, { kid: false, header: { typ: 'dpop+jwt', jwk: key.toJWK(true) } }))
.set('Authorization', `DPoP ${dpop}`)
.expect(400)
.expect({ error: 'invalid_token', error_description: 'invalid DPoP key binding' })
.expect(401)
.expect({ error: 'invalid_dpop_proof', error_description: 'invalid DPoP key binding' })
.expect('WWW-Authenticate', /^DPoP /)
.expect('WWW-Authenticate', /error="invalid_token"/)
.expect('WWW-Authenticate', /error="invalid_dpop_proof"/)
.expect('WWW-Authenticate', /algs="ES256 PS256"/);

await this.agent.get('/me') // eslint-disable-line no-await-in-loop
.set('DPoP', JWT.sign({}, key, { kid: false, header: { typ: 'dpop+jwt', jwk: await JWK.generate('oct') } }))
.set('Authorization', `DPoP ${dpop}`)
.expect(400)
.expect({ error: 'invalid_token', error_description: 'invalid DPoP key binding' })
.expect(401)
.expect({ error: 'invalid_dpop_proof', error_description: 'invalid DPoP key binding' })
.expect('WWW-Authenticate', /^DPoP /)
.expect('WWW-Authenticate', /error="invalid_token"/)
.expect('WWW-Authenticate', /error="invalid_dpop_proof"/)
.expect('WWW-Authenticate', /algs="ES256 PS256"/);

await this.agent.get('/me') // eslint-disable-line no-await-in-loop
.set('DPoP', JWT.sign({ htm: 'POST', htu: `${this.provider.issuer}${this.suitePath('/me')}` }, key, { kid: false, header: { typ: 'dpop+jwt', jwk: key } }))
.set('Authorization', `DPoP ${dpop}`)
.expect(400)
.expect({ error: 'invalid_token', error_description: 'invalid DPoP key binding' })
.expect(401)
.expect({ error: 'invalid_dpop_proof', error_description: 'DPoP Proof must have a jti string property' })
.expect('WWW-Authenticate', /^DPoP /)
.expect('WWW-Authenticate', /error="invalid_token"/)
.expect('WWW-Authenticate', /error="invalid_dpop_proof"/)
.expect('WWW-Authenticate', /algs="ES256 PS256"/);

await this.agent.get('/me') // eslint-disable-line no-await-in-loop
.set('DPoP', JWT.sign({ jti: 'foo', htm: 'POST' }, key, { kid: false, header: { typ: 'dpop+jwt', jwk: key } }))
.set('Authorization', `DPoP ${dpop}`)
.expect(400)
.expect({ error: 'invalid_token', error_description: 'invalid DPoP key binding' })
.expect(401)
.expect({ error: 'invalid_dpop_proof', error_description: 'DPoP Proof htm mismatch' })
.expect('WWW-Authenticate', /^DPoP /)
.expect('WWW-Authenticate', /error="invalid_token"/)
.expect('WWW-Authenticate', /error="invalid_dpop_proof"/)
.expect('WWW-Authenticate', /algs="ES256 PS256"/);

await this.agent.get('/me') // eslint-disable-line no-await-in-loop
.set('DPoP', JWT.sign({ jti: 'foo', htm: 'GET', htu: 'foo' }, key, { kid: false, header: { typ: 'dpop+jwt', jwk: key } }))
.set('Authorization', `DPoP ${dpop}`)
.expect(400)
.expect({ error: 'invalid_token', error_description: 'invalid DPoP key binding' })
.expect(401)
.expect({ error: 'invalid_dpop_proof', error_description: 'DPoP Proof htu mismatch' })
.expect('WWW-Authenticate', /^DPoP /)
.expect('WWW-Authenticate', /error="invalid_token"/)
.expect('WWW-Authenticate', /error="invalid_dpop_proof"/)
.expect('WWW-Authenticate', /algs="ES256 PS256"/);

await this.agent.get('/me') // eslint-disable-line no-await-in-loop
.set('DPoP', JWT.sign({
jti: 'foo', htm: 'GET', htu: `${this.provider.issuer}${this.suitePath('/me')}`, iat: epochTime() - 61,
}, key, { kid: false, iat: false, header: { typ: 'dpop+jwt', jwk: key } }))
.set('Authorization', `DPoP ${dpop}`)
.expect(400)
.expect({ error: 'invalid_token', error_description: 'invalid DPoP key binding' })
.expect(401)
.expect({ error: 'invalid_dpop_proof', error_description: 'invalid DPoP key binding' })
.expect('WWW-Authenticate', /^DPoP /)
.expect('WWW-Authenticate', /error="invalid_token"/)
.expect('WWW-Authenticate', /error="invalid_dpop_proof"/)
.expect('WWW-Authenticate', /algs="ES256 PS256"/);

await this.agent.get('/me') // eslint-disable-line no-await-in-loop
.set('DPoP', JWT.sign({
jti: 'foo', htm: 'GET', htu: `${this.provider.issuer}${this.suitePath('/me')}`,
}, key, { kid: false, header: { typ: 'dpop+jwt', jwk: await JWK.generate('EC') } }))
.set('Authorization', `DPoP ${dpop}`)
.expect(400)
.expect({ error: 'invalid_token', error_description: 'invalid DPoP key binding' })
.expect(401)
.expect({ error: 'invalid_dpop_proof', error_description: 'invalid DPoP key binding' })
.expect('WWW-Authenticate', /^DPoP /)
.expect('WWW-Authenticate', /error="invalid_token"/)
.expect('WWW-Authenticate', /error="invalid_dpop_proof"/)
.expect('WWW-Authenticate', /algs="ES256 PS256"/);
});

Expand Down Expand Up @@ -258,8 +258,8 @@ describe('features.dPoP', () => {
await this.agent.get('/me')
.set('Authorization', `DPoP ${dpop}`)
.set('DPoP', this.proof(`${this.provider.issuer}${this.suitePath('/me')}`, 'GET', 'anotherAccessTokenValue'))
.expect({ error: 'invalid_token', error_description: 'invalid DPoP key binding' })
.expect(400);
.expect({ error: 'invalid_dpop_proof', error_description: 'DPoP Proof ath mismatch' })
.expect(401);

expect(spy).to.have.property('calledOnce', true);
expect(spy.args[0][1]).to.have.property('error_detail', 'failed jkt verification');
Expand Down Expand Up @@ -626,4 +626,16 @@ describe('features.dPoP', () => {
expect(ClientCredentials).to.have.property('jkt', expectedS256);
});
});

describe('status codes at the token endpoint', () => {
it('should be 400 for invalid_dpop_proof', async function () {
return this.agent.post('/token')
.auth('client', 'secret')
.send({ grant_type: 'client_credentials' })
.set('DPoP', 'invalid')
.type('form')
.expect(400)
.expect({ error: 'invalid_dpop_proof', error_description: 'invalid DPoP key binding' });
});
});
});

0 comments on commit 2628d7e

Please sign in to comment.