feedback

Author: brendan-kellam

packages/web/src/ee/features/oauth/dpop.test.ts

@@ -1,4 +1,4 @@
-import { beforeEach, describe, expect, test, vi } from 'vitest';
+import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest';
 import crypto from 'crypto';
 import {
     __clearDpopReplayCacheForTests,
@@ -31,6 +31,10 @@ beforeEach(() => {
     __clearDpopReplayCacheForTests();
 });
 
+afterEach(() => {
+    vi.useRealTimers();
+});
+
 describe('verifyDpopProof', () => {
     test('accepts a valid resource request proof for a DPoP-bound access token', async () => {
         const keyPair = await generateKeyPair();
@@ -117,6 +121,79 @@ describe('verifyDpopProof', () => {
             error: 'invalid_dpop_proof',
         });
     });
+
+    test('does not record a proof id before access-token hash validation passes', async () => {
+        const keyPair = await generateKeyPair();
+        const request = new Request('http://internal.test/api/ee/mcp', { method: 'POST' });
+        const proof = await signDpopProof({
+            ...keyPair,
+            htm: 'POST',
+            htu: 'https://sourcebot.test/api/mcp',
+            accessToken: 'sboa_actual-token',
+            jti: 'ath-mismatch-not-recorded',
+        });
+
+        await expect(verifyDpopProof({
+            request,
+            proof,
+            expectedJkt: calculateDpopJkt(keyPair.publicJwk),
+            accessToken: 'sboa_other-token',
+            requireAccessTokenHash: true,
+        })).resolves.toMatchObject({
+            ok: false,
+            error: 'invalid_dpop_proof',
+        });
+
+        await expect(verifyDpopProof({
+            request,
+            proof,
+            expectedJkt: calculateDpopJkt(keyPair.publicJwk),
+            accessToken: 'sboa_actual-token',
+            requireAccessTokenHash: true,
+        })).resolves.toMatchObject({ ok: true });
+    });
+
+    test('expires replay cache entries based on proof iat plus the accepted window', async () => {
+        vi.useFakeTimers();
+        vi.setSystemTime(new Date('2026-01-01T00:00:00.000Z'));
+
+        const keyPair = await generateKeyPair();
+        const request = new Request('http://internal.test/api/ee/oauth/token', { method: 'POST' });
+        const proof = await signDpopProof({
+            ...keyPair,
+            htm: 'POST',
+            htu: 'https://sourcebot.test/api/ee/oauth/token',
+            iat: Math.floor(Date.now() / 1000) + 60,
+            jti: 'future-iat-replay',
+        });
+
+        await expect(verifyDpopProof({ request, proof })).resolves.toMatchObject({ ok: true });
+
+        vi.setSystemTime(new Date('2026-01-01T00:05:01.000Z'));
+
+        await expect(verifyDpopProof({ request, proof })).resolves.toMatchObject({
+            ok: false,
+            error: 'invalid_dpop_proof',
+            errorDescription: 'DPoP proof jti has already been used.',
+        });
+    });
+
+    test.each([
+        { header: null, payload: {} },
+        { header: [], payload: {} },
+        { header: 'header', payload: {} },
+        { header: {}, payload: null },
+        { header: {}, payload: [] },
+        { header: {}, payload: 'payload' },
+    ])('rejects non-object JWT JSON values %#', async ({ header, payload }) => {
+        const request = new Request('http://internal.test/api/ee/oauth/token', { method: 'POST' });
+        const proof = `${base64UrlJson(header)}.${base64UrlJson(payload)}.signature`;
+
+        await expect(verifyDpopProof({ request, proof })).resolves.toMatchObject({
+            ok: false,
+            error: 'invalid_dpop_proof',
+        });
+    });
 });
 
 async function generateKeyPair(): Promise<KeyPair> {
@@ -144,11 +221,13 @@ async function signDpopProof({
     htm,
     htu,
     accessToken,
+    iat = Math.floor(Date.now() / 1000),
     jti = crypto.randomUUID(),
 }: KeyPair & {
     htm: string;
     htu: string;
     accessToken?: string;
+    iat?: number;
     jti?: string;
 }): Promise<string> {
     const encodedHeader = base64UrlJson({
@@ -159,7 +238,7 @@ async function signDpopProof({
     const encodedPayload = base64UrlJson({
         htm,
         htu,
-        iat: Math.floor(Date.now() / 1000),
+        iat,
         jti,
         ...(accessToken ? { ath: getDpopAccessTokenHash(accessToken) } : {}),
     });

packages/web/src/ee/features/oauth/dpop.ts

@@ -99,15 +99,22 @@ export async function verifyDpopProof({
         return invalidDpopProof('DPoP proof must be a compact JWT.');
     }
 
-    let header: DpopHeader;
-    let payload: DpopPayload;
+    let parsedHeader: unknown;
+    let parsedPayload: unknown;
     try {
-        header = JSON.parse(base64UrlDecode(parts[0]).toString('utf8')) as DpopHeader;
-        payload = JSON.parse(base64UrlDecode(parts[1]).toString('utf8')) as DpopPayload;
+        parsedHeader = JSON.parse(base64UrlDecode(parts[0]).toString('utf8'));
+        parsedPayload = JSON.parse(base64UrlDecode(parts[1]).toString('utf8'));
     } catch {
         return invalidDpopProof('DPoP proof header or payload is not valid JSON.');
     }
 
+    if (!isPlainObject(parsedHeader) || !isPlainObject(parsedPayload)) {
+        return invalidDpopProof('DPoP proof header and payload must be JSON objects.');
+    }
+
+    const header = parsedHeader as DpopHeader;
+    const payload = parsedPayload as DpopPayload;
+
     if (header.typ?.toLowerCase() !== 'dpop+jwt') {
         return invalidDpopProof('DPoP proof typ must be dpop+jwt.');
     }
@@ -160,10 +167,7 @@ export async function verifyDpopProof({
     if (typeof payload.jti !== 'string' || payload.jti.length === 0) {
         return invalidDpopProof('DPoP proof jti is required.');
     }
-
-    if (!recordProofJti(jkt, payload.jti)) {
-        return invalidDpopProof('DPoP proof jti has already been used.');
-    }
+    const proofExpiresAt = (payload.iat + DPOP_PROOF_IAT_WINDOW_SECONDS) * 1000;
 
     if (accessToken || requireAccessTokenHash) {
         if (typeof payload.ath !== 'string' || !accessToken) {
@@ -175,6 +179,10 @@ export async function verifyDpopProof({
         }
     }
 
+    if (!recordProofJti(jkt, payload.jti, proofExpiresAt)) {
+        return invalidDpopProof('DPoP proof jti has already been used.');
+    }
+
     return { ok: true, jkt };
 }
 
@@ -190,6 +198,10 @@ function invalidDpopProof(errorDescription: string): VerifyDpopProofResult {
     };
 }
 
+function isPlainObject(value: unknown): value is Record<string, unknown> {
+    return typeof value === 'object' && value !== null && !Array.isArray(value);
+}
+
 function isSupportedPublicJwk(jwk: DpopJwk): jwk is Required<Pick<DpopJwk, 'kty' | 'crv' | 'x' | 'y'>> & DpopJwk {
     return (
         jwk.kty === 'EC' &&
@@ -238,7 +250,7 @@ function base64UrlDecode(value: string): Buffer {
     return Buffer.from(value, 'base64url');
 }
 
-function recordProofJti(jkt: string, jti: string): boolean {
+function recordProofJti(jkt: string, jti: string, proofExpiresAt: number): boolean {
     const now = Date.now();
     for (const [cacheKey, expiresAt] of seenProofJtis.entries()) {
         if (expiresAt <= now) {
@@ -251,6 +263,6 @@ function recordProofJti(jkt: string, jti: string): boolean {
         return false;
     }
 
-    seenProofJtis.set(cacheKey, now + DPOP_PROOF_IAT_WINDOW_SECONDS * 1000);
+    seenProofJtis.set(cacheKey, proofExpiresAt);
     return true;
 }