feat: incorporate feedback and fix tests

Sphereon-Opensource · Jul 31, 2024 · c7c6af4 · c7c6af4
1 parent b696dba
commit c7c6af4
Show file tree

Hide file tree

Showing 10 changed files with 55 additions and 57 deletions.
diff --git a/packages/client/lib/AccessTokenClient.ts b/packages/client/lib/AccessTokenClient.ts
@@ -26,7 +26,7 @@ import { ObjectUtils } from '@sphereon/ssi-types';
 
 import { MetadataClientV1_0_13 } from './MetadataClientV1_0_13';
 import { createJwtBearerClientAssertion } from './functions';
-import { dPoPShouldRetryRequestWithNonce } from './functions/dpopUtil';
+import { shouldRetryTokenRequestWithDPoPNonce } from './functions/dpopUtil';
 import { LOG } from './types';
 
 export class AccessTokenClient {
@@ -99,7 +99,7 @@ export class AccessTokenClient {
     let response = await this.sendAuthCode(requestTokenURL, accessTokenRequest, dPoP ? { headers: { dpop: dPoP } } : undefined);
 
     let nextDPoPNonce = createDPoPOpts?.jwtPayloadProps.nonce;
-    const retryWithNonce = dPoPShouldRetryRequestWithNonce(response);
+    const retryWithNonce = shouldRetryTokenRequestWithDPoPNonce(response);
     if (retryWithNonce.ok && createDPoPOpts) {
       createDPoPOpts.jwtPayloadProps.nonce = retryWithNonce.dpopNonce;
 
@@ -110,7 +110,7 @@ export class AccessTokenClient {
       nextDPoPNonce = successDPoPNonce ?? retryWithNonce.dpopNonce;
     }
 
-    if (response.successBody && createDPoPOpts && createDPoPOpts && response.successBody.token_type !== 'DPoP') {
+    if (response.successBody && createDPoPOpts && response.successBody.token_type !== 'DPoP') {
       throw new Error('Invalid token type returned. Expected DPoP. Received: ' + response.successBody.token_type);
     }
 

diff --git a/packages/client/lib/AccessTokenClientV1_0_11.ts b/packages/client/lib/AccessTokenClientV1_0_11.ts
@@ -29,7 +29,7 @@ import Debug from 'debug';
 
 import { MetadataClientV1_0_13 } from './MetadataClientV1_0_13';
 import { createJwtBearerClientAssertion } from './functions';
-import { dPoPShouldRetryRequestWithNonce } from './functions/dpopUtil';
+import { shouldRetryTokenRequestWithDPoPNonce } from './functions/dpopUtil';
 
 const debug = Debug('sphereon:oid4vci:token');
 
@@ -103,7 +103,7 @@ export class AccessTokenClientV1_0_11 {
     let response = await this.sendAuthCode(requestTokenURL, accessTokenRequest, dPoP ? { headers: { dpop: dPoP } } : undefined);
 
     let nextDPoPNonce = createDPoPOpts?.jwtPayloadProps.nonce;
-    const retryWithNonce = dPoPShouldRetryRequestWithNonce(response);
+    const retryWithNonce = shouldRetryTokenRequestWithDPoPNonce(response);
     if (retryWithNonce.ok && createDPoPOpts) {
       createDPoPOpts.jwtPayloadProps.nonce = retryWithNonce.dpopNonce;
 
@@ -114,9 +114,10 @@ export class AccessTokenClientV1_0_11 {
       nextDPoPNonce = successDPoPNonce ?? retryWithNonce.dpopNonce;
     }
 
-    if (response.successBody && createDPoPOpts && createDPoPOpts && response.successBody.token_type !== 'DPoP') {
+    if (response.successBody && createDPoPOpts && response.successBody.token_type !== 'DPoP') {
       throw new Error('Invalid token type returned. Expected DPoP. Received: ' + response.successBody.token_type);
     }
+
     return {
       ...response,
       params: { ...(nextDPoPNonce && { dpop: { dpopNonce: nextDPoPNonce } }) },

diff --git a/packages/client/lib/CredentialRequestClient.ts b/packages/client/lib/CredentialRequestClient.ts
@@ -23,7 +23,7 @@ import Debug from 'debug';
 import { CredentialRequestClientBuilderV1_0_11 } from './CredentialRequestClientBuilderV1_0_11';
 import { CredentialRequestClientBuilderV1_0_13 } from './CredentialRequestClientBuilderV1_0_13';
 import { ProofOfPossessionBuilder } from './ProofOfPossessionBuilder';
-import { dPoPShouldRetryResourceRequestWithNonce } from './functions/dpopUtil';
+import { shouldRetryResourceRequestWithDPoPNonce } from './functions/dpopUtil';
 
 const debug = Debug('sphereon:oid4vci:credential');
 
@@ -135,7 +135,7 @@ export class CredentialRequestClient {
     };
 
     let nextDPoPNonce = createDPoPOpts?.jwtPayloadProps.nonce;
-    const retryWithNonce = dPoPShouldRetryResourceRequestWithNonce(response);
+    const retryWithNonce = shouldRetryResourceRequestWithDPoPNonce(response);
     if (retryWithNonce.ok && createDPoPOpts) {
       createDPoPOpts.jwtPayloadProps.nonce = retryWithNonce.dpopNonce;
       dPoP = await createDPoP(getCreateDPoPOptions(createDPoPOpts, credentialEndpoint, { accessToken: requestToken }));

diff --git a/packages/client/lib/CredentialRequestClientV1_0_11.ts b/packages/client/lib/CredentialRequestClientV1_0_11.ts
@@ -22,7 +22,7 @@ import Debug from 'debug';
 import { buildProof } from './CredentialRequestClient';
 import { CredentialRequestClientBuilderV1_0_11 } from './CredentialRequestClientBuilderV1_0_11';
 import { ProofOfPossessionBuilder } from './ProofOfPossessionBuilder';
-import { dPoPShouldRetryResourceRequestWithNonce } from './functions/dpopUtil';
+import { shouldRetryResourceRequestWithDPoPNonce } from './functions/dpopUtil';
 
 const debug = Debug('sphereon:oid4vci:credential');
 
@@ -99,7 +99,7 @@ export class CredentialRequestClientV1_0_11 {
     };
 
     let nextDPoPNonce = createDPoPOpts?.jwtPayloadProps.nonce;
-    const retryWithNonce = dPoPShouldRetryResourceRequestWithNonce(response);
+    const retryWithNonce = shouldRetryResourceRequestWithDPoPNonce(response);
     if (retryWithNonce.ok && createDPoPOpts) {
       createDPoPOpts.jwtPayloadProps.nonce = retryWithNonce.dpopNonce;
       dPoP = await createDPoP(getCreateDPoPOptions(createDPoPOpts, credentialEndpoint, { accessToken: requestToken }));

diff --git a/packages/client/lib/__tests__/AccessTokenClient.spec.ts b/packages/client/lib/__tests__/AccessTokenClient.spec.ts
@@ -1,11 +1,4 @@
-import {
-  AccessTokenRequest,
-  AccessTokenResponse,
-  GrantTypes,
-  OpenIDResponse,
-  PRE_AUTH_CODE_LITERAL,
-  WellKnownEndpoints,
-} from '@sphereon/oid4vci-common';
+import { AccessTokenRequest, AccessTokenResponse, GrantTypes, PRE_AUTH_CODE_LITERAL, WellKnownEndpoints } from '@sphereon/oid4vci-common';
 // eslint-disable-next-line @typescript-eslint/ban-ts-comment
 // @ts-ignore
 import nock from 'nock';
@@ -50,7 +43,7 @@ describe('AccessTokenClient should', () => {
       };
       nock(MOCK_URL).post(/.*/).reply(200, JSON.stringify(body));
 
-      const accessTokenResponse: OpenIDResponse<AccessTokenResponse> = await accessTokenClient.acquireAccessTokenUsingRequest({
+      const accessTokenResponse = await accessTokenClient.acquireAccessTokenUsingRequest({
         accessTokenRequest,
         pinMetadata: {
           isPinRequired: true,
@@ -88,7 +81,7 @@ describe('AccessTokenClient should', () => {
       };
       nock(MOCK_URL).post(/.*/).reply(200, JSON.stringify(body));
 
-      const accessTokenResponse: OpenIDResponse<AccessTokenResponse> = await accessTokenClient.acquireAccessTokenUsingRequest({
+      const accessTokenResponse = await accessTokenClient.acquireAccessTokenUsingRequest({
         accessTokenRequest,
         asOpts: { as: MOCK_URL },
       });
@@ -227,7 +220,7 @@ describe('AccessTokenClient should', () => {
       .post(/.*/)
       .reply(200, {});
 
-    const response: OpenIDResponse<AccessTokenResponse> = await accessTokenClient.acquireAccessToken({
+    const response = await accessTokenClient.acquireAccessToken({
       credentialOffer: INITIATION_TEST,
       pin: '1234',
     });

diff --git a/packages/client/lib/functions/dpopUtil.ts b/packages/client/lib/functions/dpopUtil.ts
@@ -1,33 +1,35 @@
-import { dpopResourceAuthenticateError, dpopTokenRequestNonceError } from '@sphereon/oid4vc-common';
+import { dpopTokenRequestNonceError } from '@sphereon/oid4vc-common';
 import { OpenIDResponse } from 'oid4vci-common';
 
-export function dPoPShouldRetryRequestWithNonce(response: OpenIDResponse<unknown, unknown>) {
-  if (response.errorBody && response.errorBody.error === dpopTokenRequestNonceError) {
-    const dPoPNonce = response.errorBody.headers.get('DPoP-Nonce');
-    if (!dPoPNonce) {
-      throw new Error('The DPoP nonce was not returned');
-    }
+export type RetryRequestWithDPoPNonce = { ok: true; dpopNonce: string } | { ok: false };
 
-    return { ok: true, dpopNonce: dPoPNonce } as const;
+export function shouldRetryTokenRequestWithDPoPNonce(response: OpenIDResponse<unknown, unknown>): RetryRequestWithDPoPNonce {
+  if (!response.errorBody || response.errorBody.error !== dpopTokenRequestNonceError) {
+    return { ok: false };
   }
 
-  return { ok: false } as const;
+  const dPoPNonce = response.errorBody.headers.get('DPoP-Nonce');
+  if (!dPoPNonce) {
+    throw new Error('Missing required DPoP-Nonce header.');
+  }
+
+  return { ok: true, dpopNonce: dPoPNonce };
 }
 
-export function dPoPShouldRetryResourceRequestWithNonce(response: OpenIDResponse<unknown, unknown>) {
-  if (response.errorBody && response.origResponse.status === 401) {
-    const wwwAuthenticateHeader = response.errorBody.headers?.get('WWW-Authenticate');
-    if (!wwwAuthenticateHeader?.includes(dpopResourceAuthenticateError)) {
-      return { ok: false } as const;
-    }
+export function shouldRetryResourceRequestWithDPoPNonce(response: OpenIDResponse<unknown, unknown>): RetryRequestWithDPoPNonce {
+  if (!response.errorBody || response.origResponse.status !== 401) {
+    return { ok: false };
+  }
 
-    const dPoPNonce = response.errorBody.headers.get('DPoP-Nonce');
-    if (!dPoPNonce) {
-      throw new Error('The DPoP nonce was not returned');
-    }
+  const wwwAuthenticateHeader = response.errorBody.headers?.get('WWW-Authenticate');
+  if (!wwwAuthenticateHeader?.includes(dpopTokenRequestNonceError)) {
+    return { ok: false };
+  }
 
-    return { ok: true, dpopNonce: dPoPNonce } as const;
+  const dPoPNonce = response.errorBody.headers.get('DPoP-Nonce');
+  if (!dPoPNonce) {
+    throw new Error('Missing required DPoP-Nonce header.');
   }
 
-  return { ok: false } as const;
+  return { ok: true, dpopNonce: dPoPNonce };
 }
diff --git a/packages/common/lib/dpop/DPoP.ts b/packages/common/lib/dpop/DPoP.ts
@@ -18,7 +18,6 @@ import {
 } from './../jwt';
 
 export const dpopTokenRequestNonceError = 'use_dpop_nonce';
-export const dpopResourceAuthenticateError = 'DPoP error="use_dpop_nonce", error_description="Resource server requires nonce in DPoP proof"';
 
 export interface DPoPJwtIssuerWithContext extends JwtIssuerJwk {
   type: 'dpop';
@@ -170,10 +169,10 @@ export async function verifyDPoP(
   // Validate iat claim
   const { nowSkewedPast, nowSkewedFuture } = getNowSkewed(options.now);
   if (
-    // iat claim is to far in the future
-    nowSkewedPast - (options.maxIatAgeInSeconds ?? 300) > dPoPPayload.iat ||
+    // iat claim is too far in the future
+    nowSkewedPast - (options.maxIatAgeInSeconds ?? 60) > dPoPPayload.iat ||
     // iat claim is too old
-    nowSkewedFuture + (options.maxIatAgeInSeconds ?? 300) < dPoPPayload.iat
+    nowSkewedFuture + (options.maxIatAgeInSeconds ?? 60) < dPoPPayload.iat
   ) {
     // 5 minute window
     throw new Error('invalid_dpop_proof. Invalid iat claim');

diff --git a/packages/common/lib/index.ts b/packages/common/lib/index.ts
@@ -1,7 +1,7 @@
 import { Loggers } from '@sphereon/ssi-types';
 
 export const VCI_LOGGERS = Loggers.DEFAULT;
-export const VCI_LOG_COMMON = VCI_LOGGERS.get('sphereon:common');
+export const VCI_LOG_COMMON = VCI_LOGGERS.get('sphereon:oid4vci:common');
 
 export * from './jwt';
 export * from './dpop';

diff --git a/packages/common/lib/jwt/jwtUtils.ts b/packages/common/lib/jwt/jwtUtils.ts
@@ -23,7 +23,7 @@ export function parseJWT<Header = JwtHeader, Payload = JwtPayload>(jwt: string)
  *
  * See https://datatracker.ietf.org/doc/html/rfc7519#section-4.1.5
  */
-const DEFAULT_SKEW_TIME = 300;
+const DEFAULT_SKEW_TIME = 60;
 
 export function getNowSkewed(now?: number, skewTime?: number) {
   const _now = now ? now : epochTime();

diff --git a/packages/issuer-rest/lib/IssuerTokenEndpoint.ts b/packages/issuer-rest/lib/IssuerTokenEndpoint.ts
@@ -21,12 +21,13 @@ export const handleTokenRequest = <T extends object>({
   cNonceExpiresIn, // expiration in seconds
   issuer,
   interval,
-  dPoPVerifyJwtCallback,
-  requireDPoP,
+  dpop,
 }: Required<Pick<ITokenEndpointOpts, 'accessTokenIssuer' | 'cNonceExpiresIn' | 'interval' | 'accessTokenSignerCallback' | 'tokenExpiresIn'>> & {
   issuer: VcIssuer<T>
-  dPoPVerifyJwtCallback?: DPoPVerifyJwtCallback
-  requireDPoP?: boolean
+  dpop?: {
+    requireDPoP?: boolean
+    dPoPVerifyJwtCallback: DPoPVerifyJwtCallback
+  }
   // The full URL of the access token endpoint
   accessTokenEndpoint?: string
 }) => {
@@ -52,18 +53,20 @@ export const handleTokenRequest = <T extends object>({
     }
 
     let dPoPJwk: JWK | undefined
-    if (requireDPoP && !request.headers.dpop) {
+    if (dpop?.requireDPoP && !request.headers.dpop) {
       return sendErrorResponse(response, 400, {
         error: TokenErrorResponse.invalid_request,
-        error_description: 'DPoP is required for requesting access tokens',
+        error_description: 'DPoP is required for requesting access tokens.',
       })
     }
 
     if (request.headers.dpop) {
-      if (!dPoPVerifyJwtCallback) {
+      if (!dpop) {
+        console.error('Received unsupported DPoP header. The issuer is not configured to work with DPoP. Provide DPoP options for it to work.')
+
         return sendErrorResponse(response, 400, {
           error: TokenErrorResponse.invalid_request,
-          error_description: 'DPOP is not supported',
+          error_description: 'Received unsupported DPoP header.',
         })
       }
 
@@ -72,7 +75,7 @@ export const handleTokenRequest = <T extends object>({
         dPoPJwk = await verifyDPoP(
           { method: request.method, headers: request.headers, fullUrl },
           {
-            jwtVerifyCallback: dPoPVerifyJwtCallback,
+            jwtVerifyCallback: dpop.dPoPVerifyJwtCallback,
             expectAccessToken: false,
             maxIatAgeInSeconds: undefined,
           },