Skip to content

Commit

Permalink
fix: passing null as checks.nonce should not disable it
Browse files Browse the repository at this point in the history
  • Loading branch information
panva committed Jan 13, 2022
1 parent cba11f2 commit 5120a07
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 22 deletions.
10 changes: 6 additions & 4 deletions lib/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ const [major, minor] = process.version

const rsaPssParams = major >= 17 || (major === 16 && minor >= 9);
const retryAttempt = Symbol();
const skipNonceCheck = Symbol();
const skipMaxAgeCheck = Symbol();

function pickCb(input) {
return pick(
Expand Down Expand Up @@ -762,7 +764,7 @@ class BaseClient {
const timestamp = now();
const { protected: header, payload, key } = await this.validateJWT(idToken, expectedAlg);

if (maxAge || (maxAge !== null && this.require_auth_time)) {
if (typeof maxAge === 'number' || (maxAge !== skipMaxAgeCheck && this.require_auth_time)) {
if (!payload.auth_time) {
throw new RPError({
message: 'missing required JWT property auth_time',
Expand All @@ -777,7 +779,7 @@ class BaseClient {
}
}

if (maxAge && payload.auth_time + maxAge < timestamp - this[CLOCK_TOLERANCE]) {
if (typeof maxAge === 'number' && payload.auth_time + maxAge < timestamp - this[CLOCK_TOLERANCE]) {
throw new RPError({
printf: [
'too much time has elapsed since the last End-User authentication, max_age %i, auth_time: %i, now %i',
Expand All @@ -792,7 +794,7 @@ class BaseClient {
});
}

if (nonce !== null && (payload.nonce || nonce !== undefined) && payload.nonce !== nonce) {
if (nonce !== skipNonceCheck && (payload.nonce || nonce !== undefined) && payload.nonce !== nonce) {
throw new RPError({
printf: ['nonce mismatch, expected %s, got: %s', nonce, payload.nonce],
jwt: idToken,
Expand Down Expand Up @@ -1090,7 +1092,7 @@ class BaseClient {

if (tokenset.id_token) {
await this.decryptIdToken(tokenset);
await this.validateIdToken(tokenset, null, 'token', null);
await this.validateIdToken(tokenset, skipNonceCheck, 'token', skipMaxAgeCheck);

if (refreshToken instanceof TokenSet && refreshToken.id_token) {
const expectedSub = refreshToken.claims().sub;
Expand Down
39 changes: 21 additions & 18 deletions test/client/client_instance.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3017,7 +3017,7 @@ describe('Client', () => {
};

return this.IdToken(this.keystore.get(), 'RS256', payload).then((token) =>
this.client.validateIdToken(token, null, null, 300),
this.client.validateIdToken(token, undefined, null, 300),
);
});

Expand Down Expand Up @@ -3054,7 +3054,7 @@ describe('Client', () => {
};

return this.IdToken(this.keystore.get(), 'RS256', payload).then((token) =>
this.client.validateIdToken(token, null, null, 300),
this.client.validateIdToken(token, undefined, null, 300),
);
});

Expand All @@ -3078,24 +3078,27 @@ describe('Client', () => {
});
});

it('ignores auth_time presence check when require_auth_time is true but null is passed', function () {
const client = new this.issuer.Client({
client_id: 'with-require_auth_time',
require_auth_time: true,
});
// const {skipMaxAgeCheck} = require('../../lib/client')
// console.log(skipMaxAgeCheck)

const payload = {
iss: this.issuer.issuer,
sub: 'userId',
aud: client.client_id,
exp: now() + 3600,
iat: now(),
};
// it.only('ignores auth_time presence check when require_auth_time is true but the private symbol is passed', function () {
// const client = new this.issuer.Client({
// client_id: 'with-require_auth_time',
// require_auth_time: true,
// });

return this.IdToken(this.keystore.get(), 'RS256', payload).then((token) =>
client.validateIdToken(token, null, null, null),
);
});
// const payload = {
// iss: this.issuer.issuer,
// sub: 'userId',
// aud: client.client_id,
// exp: now() + 3600,
// iat: now(),
// };

// return this.IdToken(this.keystore.get(), 'RS256', payload).then((token) =>
// client.validateIdToken(token, undefined, null, skipMaxAgeCheck),
// );
// });

it('verifies auth_time is present when require_auth_time is true', function () {
const client = new this.issuer.Client({
Expand Down

0 comments on commit 5120a07

Please sign in to comment.