Skip to content

Commit

Permalink
refactor: push id token required claims straight to jwt validation
Browse files Browse the repository at this point in the history
  • Loading branch information
panva committed Oct 7, 2024
1 parent f3a3fa4 commit ec45b61
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 54 deletions.
10 changes: 2 additions & 8 deletions conformance/fapi/invalid-missing-nonce.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,5 @@
import { test, rejects, flow, modules, plan, variant } from '../runner.js'
import { test, rejects, flow, modules } from '../runner.js'

for (const module of modules('invalid-missing-nonce')) {
test.serial(
rejects(flow({ useNonce: true })),
module,
plan.name.startsWith('fapi1') && variant.fapi_response_mode !== 'jarm'
? 'JWT "nonce" (nonce) claim missing'
: 'ID Token "nonce" claim missing',
)
test.serial(rejects(flow({ useNonce: true })), module, 'JWT "nonce" (nonce) claim missing')
}
137 changes: 92 additions & 45 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3297,8 +3297,9 @@ async function processGenericAccessTokenResponse(
as: AuthorizationServer,
client: Client,
response: Response,
ignoreIdToken = false,
ignoreRefreshToken = false,
ignoreIdToken: boolean,
ignoreRefreshToken: boolean,
additionalRequiredIdTokenClaims: (keyof typeof jwtClaimNames)[] | undefined,
): Promise<TokenEndpointResponse> {
assertAs(as)
assertClient(client)
Expand Down Expand Up @@ -3388,6 +3389,12 @@ async function processGenericAccessTokenResponse(
}

if (json.id_token) {
const requiredClaims: (keyof typeof jwtClaimNames)[] = ['aud', 'exp', 'iat', 'iss', 'sub']

if (additionalRequiredIdTokenClaims?.length) {
requiredClaims.push(...additionalRequiredIdTokenClaims)
}

const { claims, jwt } = await validateJwt(
json.id_token,
checkSigningAlgorithm.bind(
Expand All @@ -3401,7 +3408,7 @@ async function processGenericAccessTokenResponse(
getClockTolerance(client),
client[jweDecrypt],
)
.then(validatePresence.bind(undefined, ['aud', 'exp', 'iat', 'iss', 'sub']))
.then(validatePresence.bind(undefined, requiredClaims))
.then(validateIssuer.bind(undefined, as.issuer))
.then(validateAudience.bind(undefined, client.client_id))

Expand Down Expand Up @@ -3461,7 +3468,20 @@ export async function processRefreshTokenResponse(
client: Client,
response: Response,
): Promise<TokenEndpointResponse> {
return processGenericAccessTokenResponse(as, client, response)
const additionalRequiredClaims: (keyof typeof jwtClaimNames)[] = []

if (client.default_max_age !== undefined) {
assertNumber(client.default_max_age, false, '"client.default_max_age"')
additionalRequiredClaims.push('auth_time')
}
return processGenericAccessTokenResponse(
as,
client,
response,
false,
false,
additionalRequiredClaims,
)
}

function validateOptionalAudience(
Expand Down Expand Up @@ -3754,29 +3774,41 @@ export async function processAuthorizationCodeOpenIDResponse(
expectedNonce?: string | typeof expectNoNonce,
maxAge?: number | typeof skipAuthTimeCheck,
): Promise<OpenIDTokenEndpointResponse> {
const result = await processGenericAccessTokenResponse(as, client, response)
const additionalRequiredClaims: (keyof typeof jwtClaimNames)[] = []

assertString(result.id_token, '"response" body "id_token" property', USE_OAUTH_CALLBACK, {
body: result,
})
switch (expectedNonce) {
case undefined:
case expectNoNonce:
break
default:
assertString(expectedNonce, '"expectedNonce" argument')
additionalRequiredClaims.push('nonce')
}

if (maxAge !== undefined) {
assertNumber(maxAge, false, '"maxAge" argument')
} else if (client.default_max_age !== undefined) {
assertNumber(client.default_max_age, false, '"client.default_max_age"')
}

maxAge ??= client.default_max_age ?? skipAuthTimeCheck
const claims = getValidatedIdTokenClaims(result)!
if (
(client.require_auth_time || maxAge !== skipAuthTimeCheck) &&
claims.auth_time === undefined
) {
throw OPE('ID Token "auth_time" (authentication time) claim missing', INVALID_RESPONSE, {
claims,
})
if (maxAge !== skipAuthTimeCheck) {
additionalRequiredClaims.push('auth_time')
}

const result = await processGenericAccessTokenResponse(
as,
client,
response,
false,
false,
additionalRequiredClaims,
)

assertString(result.id_token, '"response" body "id_token" property', USE_OAUTH_CALLBACK, {
body: result,
})

const claims = getValidatedIdTokenClaims(result)!
if (maxAge !== skipAuthTimeCheck) {
const now = epochTime() + getClockSkew(client)
const tolerance = getClockTolerance(client)
Expand All @@ -3789,31 +3821,18 @@ export async function processAuthorizationCodeOpenIDResponse(
}
}

switch (expectedNonce) {
case undefined:
case expectNoNonce:
if (claims.nonce !== undefined) {
throw OPE('unexpected ID Token "nonce" claim value', JWT_CLAIM_COMPARISON, {
expected: undefined,
claims,
})
}
break
default:
assertString(expectedNonce, '"expectedNonce" argument')

if (claims.nonce === undefined) {
throw OPE('ID Token "nonce" claim missing', INVALID_RESPONSE, {
expected: expectedNonce,
claims,
})
}
if (claims.nonce !== expectedNonce) {
throw OPE('unexpected ID Token "nonce" claim value', JWT_CLAIM_COMPARISON, {
expected: expectedNonce,
claims,
})
}
if (expectedNonce === expectNoNonce) {
if (claims.nonce !== undefined) {
throw OPE('unexpected ID Token "nonce" claim value', JWT_CLAIM_COMPARISON, {
expected: undefined,
claims,
})
}
} else if (claims.nonce !== expectedNonce) {
throw OPE('unexpected ID Token "nonce" claim value', JWT_CLAIM_COMPARISON, {
expected: expectedNonce,
claims,
})
}

return result as OpenIDTokenEndpointResponse
Expand Down Expand Up @@ -3841,7 +3860,14 @@ export async function processAuthorizationCodeOAuth2Response(
client: Client,
response: Response,
): Promise<OAuth2TokenEndpointResponse> {
const result = await processGenericAccessTokenResponse(as, client, response, true)
const result = await processGenericAccessTokenResponse(
as,
client,
response,
true,
false,
undefined,
)

if (result.id_token !== undefined) {
throw OPE(
Expand Down Expand Up @@ -4078,7 +4104,14 @@ export async function processClientCredentialsResponse(
client: Client,
response: Response,
): Promise<ClientCredentialsGrantResponse> {
const result = await processGenericAccessTokenResponse(as, client, response, true, true)
const result = await processGenericAccessTokenResponse(
as,
client,
response,
true,
true,
undefined,
)

return result as ClientCredentialsGrantResponse
}
Expand Down Expand Up @@ -5421,7 +5454,21 @@ export async function processDeviceCodeResponse(
client: Client,
response: Response,
): Promise<TokenEndpointResponse> {
return processGenericAccessTokenResponse(as, client, response)
const additionalRequiredClaims: (keyof typeof jwtClaimNames)[] = []

if (client.default_max_age !== undefined) {
assertNumber(client.default_max_age, false, '"client.default_max_age"')
additionalRequiredClaims.push('auth_time')
}

return processGenericAccessTokenResponse(
as,
client,
response,
false,
false,
additionalRequiredClaims,
)
}

export interface GenerateKeyPairOptions {
Expand Down
2 changes: 1 addition & 1 deletion test/authorization_code.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -735,7 +735,7 @@ test('processAuthorizationCodeOpenIDResponse() nonce checks', async (t) => {
),
'anotherrandom-value',
),
{ message: 'ID Token "nonce" claim missing' },
{ message: 'JWT "nonce" (nonce) claim missing' },
)

await t.notThrowsAsync(
Expand Down

0 comments on commit ec45b61

Please sign in to comment.