refactor: reconcile dpop and attestation challenge implementations

panva · panva · commit e31f63977c0f · 2025-07-17T22:34:49.000+02:00
diff --git a/lib/actions/authorization/check_dpop_jkt.js b/lib/actions/authorization/check_dpop_jkt.js
@@ -1,5 +1,5 @@
 import { InvalidRequest } from '../../helpers/errors.js';
-import dpopValidate, { DPOP_OK_WINDOW } from '../../helpers/validate_dpop.js';
+import dpopValidate, { CHALLENGE_OK_WINDOW } from '../../helpers/validate_dpop.js';
 import epochTime from '../../helpers/epoch_time.js';
 import instance from '../../helpers/weak_cache.js';
 
@@ -18,7 +18,7 @@ export default async function checkDpopJkt(ctx, next) {
       const unique = await ReplayDetection.unique(
         ctx.oidc.client.clientId,
         dPoP.jti,
-        epochTime() + DPOP_OK_WINDOW,
+        epochTime() + CHALLENGE_OK_WINDOW,
       );
 
       ctx.assert(unique, new InvalidRequest('DPoP proof JWT Replay detected'));
diff --git a/lib/actions/challenge.js b/lib/actions/challenge.js
@@ -8,7 +8,7 @@ export default [
 
     ctx.body = {};
 
-    const nextNonce = DPoPNonces?.nextNonce();
+    const nextNonce = DPoPNonces?.nextChallenge();
     if (nextNonce) {
       ctx.set('dpop-nonce', nextNonce);
     }
diff --git a/lib/actions/grants/authorization_code.js b/lib/actions/grants/authorization_code.js
@@ -4,7 +4,7 @@ import instance from '../../helpers/weak_cache.js';
 import checkPKCE from '../../helpers/pkce.js';
 import revoke from '../../helpers/revoke.js';
 import filterClaims from '../../helpers/filter_claims.js';
-import dpopValidate, { DPOP_OK_WINDOW } from '../../helpers/validate_dpop.js';
+import dpopValidate, { CHALLENGE_OK_WINDOW } from '../../helpers/validate_dpop.js';
 import resolveResource from '../../helpers/resolve_resource.js';
 import epochTime from '../../helpers/epoch_time.js';
 import checkRar from '../../shared/check_rar.js';
@@ -144,7 +144,7 @@ export const handler = async function authorizationCodeHandler(ctx) {
       const unique = await ReplayDetection.unique(
         ctx.oidc.client.clientId,
         dPoP.jti,
-        epochTime() + DPOP_OK_WINDOW,
+        epochTime() + CHALLENGE_OK_WINDOW,
       );
 
       ctx.assert(unique, new InvalidGrant('DPoP proof JWT Replay detected'));
diff --git a/lib/actions/grants/ciba.js b/lib/actions/grants/ciba.js
@@ -5,7 +5,7 @@ import presence from '../../helpers/validate_presence.js';
 import instance from '../../helpers/weak_cache.js';
 import filterClaims from '../../helpers/filter_claims.js';
 import revoke from '../../helpers/revoke.js';
-import dpopValidate, { DPOP_OK_WINDOW } from '../../helpers/validate_dpop.js';
+import dpopValidate, { CHALLENGE_OK_WINDOW } from '../../helpers/validate_dpop.js';
 import resolveResource from '../../helpers/resolve_resource.js';
 import epochTime from '../../helpers/epoch_time.js';
 import getCtxAccountClaims from '../../helpers/account_claims.js';
@@ -147,7 +147,7 @@ export const handler = async function cibaHandler(ctx) {
       const unique = await ReplayDetection.unique(
         ctx.oidc.client.clientId,
         dPoP.jti,
-        epochTime() + DPOP_OK_WINDOW,
+        epochTime() + CHALLENGE_OK_WINDOW,
       );
 
       ctx.assert(unique, new InvalidGrant('DPoP proof JWT Replay detected'));
diff --git a/lib/actions/grants/client_credentials.js b/lib/actions/grants/client_credentials.js
@@ -2,7 +2,7 @@ import instance from '../../helpers/weak_cache.js';
 import {
   InvalidGrant, InvalidTarget, InvalidScope, InvalidRequest,
 } from '../../helpers/errors.js';
-import dpopValidate, { DPOP_OK_WINDOW } from '../../helpers/validate_dpop.js';
+import dpopValidate, { CHALLENGE_OK_WINDOW } from '../../helpers/validate_dpop.js';
 import checkResource from '../../shared/check_resource.js';
 import epochTime from '../../helpers/epoch_time.js';
 
@@ -65,7 +65,7 @@ export const handler = async function clientCredentialsHandler(ctx) {
       const unique = await ReplayDetection.unique(
         client.clientId,
         dPoP.jti,
-        epochTime() + DPOP_OK_WINDOW,
+        epochTime() + CHALLENGE_OK_WINDOW,
       );
 
       ctx.assert(unique, new InvalidGrant('DPoP proof JWT Replay detected'));
diff --git a/lib/actions/grants/device_code.js b/lib/actions/grants/device_code.js
@@ -5,7 +5,7 @@ import presence from '../../helpers/validate_presence.js';
 import instance from '../../helpers/weak_cache.js';
 import filterClaims from '../../helpers/filter_claims.js';
 import revoke from '../../helpers/revoke.js';
-import dpopValidate, { DPOP_OK_WINDOW } from '../../helpers/validate_dpop.js';
+import dpopValidate, { CHALLENGE_OK_WINDOW } from '../../helpers/validate_dpop.js';
 import resolveResource from '../../helpers/resolve_resource.js';
 import epochTime from '../../helpers/epoch_time.js';
 import getCtxAccountClaims from '../../helpers/account_claims.js';
@@ -146,7 +146,7 @@ export const handler = async function deviceCodeHandler(ctx) {
       const unique = await ReplayDetection.unique(
         ctx.oidc.client.clientId,
         dPoP.jti,
-        epochTime() + DPOP_OK_WINDOW,
+        epochTime() + CHALLENGE_OK_WINDOW,
       );
 
       ctx.assert(unique, new InvalidGrant('DPoP proof JWT Replay detected'));
diff --git a/lib/actions/grants/refresh_token.js b/lib/actions/grants/refresh_token.js
@@ -6,7 +6,7 @@ import revoke from '../../helpers/revoke.js';
 import certificateThumbprint from '../../helpers/certificate_thumbprint.js';
 import * as formatters from '../../helpers/formatters.js';
 import filterClaims from '../../helpers/filter_claims.js';
-import dpopValidate, { DPOP_OK_WINDOW } from '../../helpers/validate_dpop.js';
+import dpopValidate, { CHALLENGE_OK_WINDOW } from '../../helpers/validate_dpop.js';
 import resolveResource from '../../helpers/resolve_resource.js';
 import epochTime from '../../helpers/epoch_time.js';
 import checkRar from '../../shared/check_rar.js';
@@ -105,7 +105,7 @@ export const handler = async function refreshTokenHandler(ctx) {
     const unique = await ReplayDetection.unique(
       client.clientId,
       dPoP.jti,
-      epochTime() + DPOP_OK_WINDOW,
+      epochTime() + CHALLENGE_OK_WINDOW,
     );
 
     ctx.assert(unique, new InvalidGrant('DPoP proof JWT Replay detected'));
diff --git a/lib/actions/userinfo.js b/lib/actions/userinfo.js
@@ -7,7 +7,7 @@ import noCache from '../shared/no_cache.js';
 import certificateThumbprint from '../helpers/certificate_thumbprint.js';
 import instance from '../helpers/weak_cache.js';
 import filterClaims from '../helpers/filter_claims.js';
-import dpopValidate, { DPOP_OK_WINDOW } from '../helpers/validate_dpop.js';
+import dpopValidate, { CHALLENGE_OK_WINDOW } from '../helpers/validate_dpop.js';
 import epochTime from '../helpers/epoch_time.js';
 import {
   InvalidToken, InsufficientScope, InvalidDpopProof, UseDpopNonce,
@@ -106,7 +106,7 @@ export default [
         const unique = await ctx.oidc.provider.ReplayDetection.unique(
           accessToken.clientId,
           dPoP.jti,
-          epochTime() + DPOP_OK_WINDOW,
+          epochTime() + CHALLENGE_OK_WINDOW,
         );
 
         ctx.assert(unique, new InvalidToken('DPoP proof JWT Replay detected'));
diff --git a/lib/helpers/configuration.js b/lib/helpers/configuration.js
@@ -59,7 +59,6 @@ class Configuration {
     this.checkCibaDeliveryModes();
     this.checkRichAuthorizationRequests();
     this.checkPostMethods();
-    this.checkAttestClientAuth();
 
     delete this.cookies.long.maxAge;
     delete this.cookies.long.expires;
@@ -394,15 +393,6 @@ class Configuration {
     });
   }
 
-  checkAttestClientAuth() {
-    if (this.features.attestClientAuth.enabled) {
-      const secret = this.features.attestClientAuth.challengeSecret;
-      if (!Buffer.isBuffer(secret) || secret.byteLength !== 32) {
-        throw new TypeError('features.attestClientAuth.challengeSecret secret must be a 32-byte Buffer instance');
-      }
-    }
-  }
-
   checkFapiProfile() {
     if (!this.features.fapi.enabled) {
       this.features.fapi.profile = () => undefined;
diff --git a/lib/helpers/dpop_nonces.js b/lib/helpers/dpop_nonces.js
diff --git a/lib/helpers/validate_dpop.js b/lib/helpers/validate_dpop.js
@@ -9,9 +9,9 @@ import {
 import { InvalidDpopProof, UseDpopNonce } from './errors.js';
 import instance from './weak_cache.js';
 import epochTime from './epoch_time.js';
-import { DPOP_OK_WINDOW } from './dpop_nonces.js';
+import { CHALLENGE_OK_WINDOW } from './challenge.js';
 
-export { DPOP_OK_WINDOW };
+export { CHALLENGE_OK_WINDOW };
 
 const weakMap = new WeakMap();
 
@@ -45,7 +45,7 @@ export default async (ctx, accessToken) => {
     throw new Error('features.dPoP.nonceSecret configuration is missing');
   }
 
-  const nextNonce = DPoPNonces?.nextNonce();
+  const nextNonce = DPoPNonces?.nextChallenge();
   let payload;
   let protectedHeader;
   try {
@@ -66,7 +66,7 @@ export default async (ctx, accessToken) => {
     if (!payload.nonce) {
       const now = epochTime();
       const diff = Math.abs(now - payload.iat);
-      if (diff > DPOP_OK_WINDOW) {
+      if (diff > CHALLENGE_OK_WINDOW) {
         if (nextNonce) {
           ctx.set('dpop-nonce', nextNonce);
           throw new UseDpopNonce('DPoP proof iat is not recent enough, use a DPoP nonce instead');
@@ -111,7 +111,7 @@ export default async (ctx, accessToken) => {
     throw new UseDpopNonce('nonce is required in the DPoP proof');
   }
 
-  if (payload.nonce && !DPoPNonces.checkNonce(payload.nonce)) {
+  if (payload.nonce && !DPoPNonces.checkChallenge(payload.nonce)) {
     ctx.set('dpop-nonce', nextNonce);
     throw new UseDpopNonce('invalid nonce in DPoP proof');
   }
diff --git a/lib/provider.js b/lib/provider.js
@@ -17,7 +17,6 @@ import getClaims from './helpers/claims.js';
 import getContext from './helpers/oidc_context.js';
 import { SessionNotFound, OIDCProviderError } from './helpers/errors.js';
 import * as models from './models/index.js';
-import DPoPNonces from './helpers/dpop_nonces.js';
 import ServerChallenge from './helpers/challenge.js';
 import als from './helpers/als.js';
 
@@ -101,14 +100,22 @@ export class Provider extends Koa {
       configuration.features.dPoP.enabled
       && configuration.features.dPoP.nonceSecret !== undefined
     ) {
-      this.#int.DPoPNonces = new DPoPNonces(configuration.features.dPoP.nonceSecret);
+      try {
+        this.#int.DPoPNonces = new ServerChallenge(configuration.features.dPoP.nonceSecret, 'DPoP');
+      } catch (cause) {
+        throw new TypeError('features.dPoP.nonceSecret secret must be a 32-byte Buffer instance', { cause });
+      }
     }
 
     if (configuration.features.attestClientAuth.enabled) {
-      this.#int.AttestChallenges = new ServerChallenge(
-        configuration.features.attestClientAuth.challengeSecret,
-        'OAuth-Client-Attestation-PoP',
-      );
+      try {
+        this.#int.AttestChallenges = new ServerChallenge(
+          configuration.features.attestClientAuth.challengeSecret,
+          'OAuth-Client-Attestation-PoP',
+        );
+      } catch (cause) {
+        throw new TypeError('features.attestClientAuth.challengeSecret secret must be a 32-byte Buffer instance', { cause });
+      }
     }
 
     this.#int.responseModes = new Map();

Original file line number	Diff line number	Diff line change
`@@ -8,7 +8,7 @@ export default [`
`8`	`8`
`9`	`9`	`ctx.body = {};`
`10`	`10`
`11`		`- const nextNonce = DPoPNonces?.nextNonce();`
	`11`	`+ const nextNonce = DPoPNonces?.nextChallenge();`
`12`	`12`	`if (nextNonce) {`
`13`	`13`	`ctx.set('dpop-nonce', nextNonce);`
`14`	`14`	`}`