diff --git a/packages/openauth/package.json b/packages/openauth/package.json index a5e3276b..4a8b7140 100644 --- a/packages/openauth/package.json +++ b/packages/openauth/package.json @@ -12,7 +12,7 @@ "@tsconfig/node22": "22.0.0", "@types/node": "22.10.1", "arctic": "2.2.2", - "hono": "4.6.9", + "hono": "4.10.5", "typescript": "5.6.3", "valibot": "1.0.0-beta.15" }, diff --git a/packages/openauth/src/client.ts b/packages/openauth/src/client.ts index c4e282a3..962a2ff3 100644 --- a/packages/openauth/src/client.ts +++ b/packages/openauth/src/client.ts @@ -286,7 +286,16 @@ export interface VerifyOptions { */ issuer?: string /** - * @internal + * The expected audience (aud) claim value. This should match the client ID + * that the token was issued for. If not provided, defaults to the client's + * configured clientID. + * + * @example + * ```ts + * { + * audience: "api" + * } + * ``` */ audience?: string /** @@ -700,6 +709,7 @@ export function createClient(input: ClientInput): Client { options?: VerifyOptions, ): Promise | VerifyError> { const jwks = await getJWKS() + const expectedAudience = options?.audience || input.clientID try { const result = await jwtVerify<{ mode: "access" @@ -707,6 +717,7 @@ export function createClient(input: ClientInput): Client { properties: v1.InferInput }>(token, jwks, { issuer, + audience: expectedAudience, }) const validated = await subjects[result.payload.type][ "~standard" @@ -732,6 +743,7 @@ export function createClient(input: ClientInput): Client { { refresh: refreshed.tokens!.refresh, issuer, + audience: expectedAudience, fetch: options?.fetch, }, ) diff --git a/packages/openauth/src/issuer.ts b/packages/openauth/src/issuer.ts index f4c1f277..b997e4b5 100644 --- a/packages/openauth/src/issuer.ts +++ b/packages/openauth/src/issuer.ts @@ -1114,26 +1114,48 @@ export function issuer< ) } - const result = await jwtVerify<{ - mode: "access" - type: keyof SubjectSchema - properties: v1.InferInput - }>(token, () => signingKey().then((item) => item.public), { - issuer: issuer(c), - }) + try { + const result = await jwtVerify<{ + mode: "access" + type: keyof SubjectSchema + properties: v1.InferInput + aud?: string + }>(token, () => signingKey().then((item) => item.public), { + issuer: issuer(c), + }) - const validated = await input.subjects[result.payload.type][ - "~standard" - ].validate(result.payload.properties) + // Validate that the token has an audience claim + if (!result.payload.aud) { + return c.json( + { + error: "invalid_token", + error_description: "Token missing audience claim", + }, + 401, + ) + } - if (!validated.issues && result.payload.mode === "access") { - return c.json(validated.value as SubjectSchema) - } + const validated = await input.subjects[result.payload.type][ + "~standard" + ].validate(result.payload.properties) - return c.json({ - error: "invalid_token", - error_description: "Invalid token", - }) + if (!validated.issues && result.payload.mode === "access") { + return c.json(validated.value as SubjectSchema) + } + + return c.json({ + error: "invalid_token", + error_description: "Invalid token", + }) + } catch (e) { + return c.json( + { + error: "invalid_token", + error_description: "Token verification failed", + }, + 401, + ) + } }) app.onError(async (err, c) => { diff --git a/packages/openauth/test/vuln-jwt-audience-validation.test.ts b/packages/openauth/test/vuln-jwt-audience-validation.test.ts new file mode 100644 index 00000000..5da839ec --- /dev/null +++ b/packages/openauth/test/vuln-jwt-audience-validation.test.ts @@ -0,0 +1,533 @@ +/** + * VULNERABILITY TEST: JWT Audience (aud) Claim Validation + * + * These tests FAIL when vulnerability exists (before commit 4676a2f) + * These tests PASS when vulnerability is fixed (after commit 4676a2f) + * + * Vulnerability: JWT tokens were created with audience claims but never + * validated during verification, violating RFC 7519 §4.1.3 requirements. + * + * Impact: Token mix-up attacks where tokens issued for one service could + * be used to authenticate with another service. + * + * Reference: + * - Commit: 4676a2f03420927c1a4c23de1cfb18e799aebe51 + * - RFC 7519 §4.1.3: https://tools.ietf.org/html/rfc7519#section-4.1.3 + */ + +import { expect, test, describe, beforeEach } from "bun:test" +import { object, string } from "valibot" +import { issuer } from "../src/issuer.js" +import { createClient } from "../src/client.js" +import { createSubjects } from "../src/subject.js" +import { MemoryStorage } from "../src/storage/memory.js" +import { Provider } from "../src/provider/provider.js" + +const subjects = createSubjects({ + user: object({ + userID: string(), + }), +}) + +let storage = MemoryStorage() + +describe("VULN: JWT Audience Validation (RFC 7519 §4.1.3)", () => { + let auth: ReturnType + let clientA: ReturnType + let clientB: ReturnType + + beforeEach(() => { + storage = MemoryStorage() + auth = issuer({ + storage, + subjects, + providers: { + dummy: { + type: "dummy", + init(route, ctx) { + route.get("/authorize", async (c) => { + return ctx.success(c, { + email: "foo@bar.com", + }) + }) + }, + } satisfies Provider<{ email: string }>, + }, + success: async (ctx, value) => { + if (value.provider === "dummy") { + return ctx.subject("user", { + userID: "123", + }) + } + throw new Error("Invalid provider: " + value.provider) + }, + }) + + // Client A - represents one service + clientA = createClient({ + issuer: "https://auth.example.com", + clientID: "client-a", + fetch: (a, b) => Promise.resolve(auth.request(a, b)), + }) + + // Client B - represents a different service + clientB = createClient({ + issuer: "https://auth.example.com", + clientID: "client-b", + fetch: (a, b) => Promise.resolve(auth.request(a, b)), + }) + }) + + async function generateTokenForClient(clientID: string) { + const client = createClient({ + issuer: "https://auth.example.com", + clientID, + fetch: (a, b) => Promise.resolve(auth.request(a, b)), + }) + + const { challenge, url } = await client.authorize( + "https://client.example.com/callback", + "code", + { pkce: true }, + ) + + let response = await auth.request(url) + response = await auth.request(response.headers.get("location")!, { + headers: { cookie: response.headers.get("set-cookie")! }, + }) + + const location = new URL(response.headers.get("location")!) + const code = location.searchParams.get("code")! + + const exchanged = await client.exchange( + code, + "https://client.example.com/callback", + challenge.verifier, + ) + + if (exchanged.err) throw exchanged.err + return exchanged.tokens.access + } + + describe("Token Mix-Up Attack Prevention", () => { + test("VULN: Token issued for client-a should NOT work for client-b", async () => { + // Generate token for client A + const tokenForClientA = await generateTokenForClient("client-a") + + // Try to use client A's token with client B + const result = await clientB.verify(subjects, tokenForClientA) + + // EXPECTED: Should fail with audience mismatch error + // ACTUAL (vulnerable): Token is accepted (no audience validation) + expect(result.err).toBeDefined() + + if (result.err) { + // Should be an authentication error + expect(result.err.message.toLowerCase()).toMatch( + /audience|invalid|verification/, + ) + } else { + // If it succeeded, that's a vulnerability! + console.error("❌ VULNERABILITY: Token mix-up attack succeeded!") + console.error(" Token for client-a was accepted by client-b") + throw new Error( + "SECURITY: Token should have been rejected due to audience mismatch", + ) + } + }) + + test("VULN: Token issued for client-b should NOT work for client-a", async () => { + // Generate token for client B + const tokenForClientB = await generateTokenForClient("client-b") + + // Try to use client B's token with client A + const result = await clientA.verify(subjects, tokenForClientB) + + // EXPECTED: Should fail with audience mismatch + // ACTUAL (vulnerable): Token is accepted + expect(result.err).toBeDefined() + + if (!result.err) { + console.error( + "❌ VULNERABILITY: Reverse token mix-up attack succeeded!", + ) + throw new Error("SECURITY: Token should have been rejected") + } + }) + + test("VULN: Token with audience=api should NOT work for client=web", async () => { + // This tests explicit audience values + const tokenForAPI = await generateTokenForClient("api") + + const webClient = createClient({ + issuer: "https://auth.example.com", + clientID: "web", + fetch: (a, b) => Promise.resolve(auth.request(a, b)), + }) + + const result = await webClient.verify(subjects, tokenForAPI) + + // Token for 'api' should not work for 'web' client + expect(result.err).toBeDefined() + }) + }) + + describe("Audience Claim Validation", () => { + test("VULN: /userinfo endpoint should validate audience claim exists", async () => { + const token = await generateTokenForClient("test-client") + + // Call /userinfo endpoint + const response = await auth.request("https://auth.example.com/userinfo", { + headers: { Authorization: `Bearer ${token}` }, + }) + + // Should succeed with valid token that has audience + expect(response.status).toBe(200) + + const userinfo = await response.json() + expect(userinfo.userID).toBe("123") + }) + + test("VULN: Token without audience claim should be rejected by /userinfo", async () => { + // This would require manually crafting a token without aud claim + // For now, we test that the endpoint validates the claim exists + + const token = await generateTokenForClient("test-client") + + // Verify the token has an audience claim when decoded + const parts = token.split(".") + expect(parts.length).toBe(3) + + const payload = JSON.parse(atob(parts[1])) + + // EXPECTED: Token should have 'aud' claim + // ACTUAL (before fix): May not validate aud exists + expect(payload.aud).toBeDefined() + expect(payload.aud).toBe("test-client") + }) + + test("VULN: Malformed token should be rejected with proper error", async () => { + const response = await auth.request("https://auth.example.com/userinfo", { + headers: { Authorization: "Bearer invalid.token.here" }, + }) + + expect(response.status).toBe(401) + + const body = await response.json() + expect(body.error).toBe("invalid_token") + expect(body.error_description).toBeDefined() + }) + }) + + describe("Correct Audience Validation", () => { + test("SECURE: Token issued for client-a should work for client-a", async () => { + const token = await generateTokenForClient("client-a") + + // Use token with the same client it was issued for + const result = await clientA.verify(subjects, token) + + // Should succeed - correct audience + if (result.err) throw result.err + + expect(result.subject).toBeDefined() + expect(result.subject.type).toBe("user") + expect(result.subject.properties.userID).toBe("123") + }) + + test("SECURE: Token with matching audience should verify successfully", async () => { + const token = await generateTokenForClient("my-service") + + const myServiceClient = createClient({ + issuer: "https://auth.example.com", + clientID: "my-service", + fetch: (a, b) => Promise.resolve(auth.request(a, b)), + }) + + const result = await myServiceClient.verify(subjects, token) + + // Should succeed - audience matches + if (result.err) throw result.err + expect(result.subject.properties.userID).toBe("123") + }) + + test("SECURE: Explicit audience parameter should be validated", async () => { + const token = await generateTokenForClient("specific-aud") + + const client = createClient({ + issuer: "https://auth.example.com", + clientID: "any-client", + fetch: (a, b) => Promise.resolve(auth.request(a, b)), + }) + + // Try to verify with explicit audience that matches + const resultMatch = await client.verify(subjects, token, { + audience: "specific-aud", + }) + + // Should succeed when audience matches + if (resultMatch.err) throw resultMatch.err + expect(resultMatch.subject).toBeDefined() + + // Try to verify with explicit audience that doesn't match + const resultMismatch = await client.verify(subjects, token, { + audience: "different-aud", + }) + + // Should fail when audience doesn't match + expect(resultMismatch.err).toBeDefined() + }) + }) + + describe("Backward Compatibility", () => { + test("SECURE: Default audience should be clientID when not specified", async () => { + const token = await generateTokenForClient("my-client") + + const client = createClient({ + issuer: "https://auth.example.com", + clientID: "my-client", + fetch: (a, b) => Promise.resolve(auth.request(a, b)), + }) + + // Verify without specifying audience - should default to clientID + const result = await client.verify(subjects, token) + + // Should succeed - defaults to clientID + if (result.err) throw result.err + expect(result.subject).toBeDefined() + }) + + test("SECURE: Tokens contain audience claim in payload", async () => { + const token = await generateTokenForClient("audience-test") + + // Decode token payload + const parts = token.split(".") + const payload = JSON.parse(atob(parts[1])) + + // Verify audience claim is present + expect(payload.aud).toBeDefined() + expect(payload.aud).toBe("audience-test") + + // Verify other required claims + expect(payload.iss).toBeDefined() + expect(payload.sub).toBeDefined() + expect(payload.exp).toBeDefined() + expect(payload.mode).toBe("access") + }) + }) + + describe("RFC 7519 Compliance", () => { + test("SECURE: Token without aud claim should be rejected", async () => { + // This tests the /userinfo endpoint validation + // In a real scenario, we'd need to craft a token without aud + // For now, we ensure our tokens always have aud + + const token = await generateTokenForClient("test") + const parts = token.split(".") + const payload = JSON.parse(atob(parts[1])) + + // RFC 7519 §4.1.3: "aud" claim is OPTIONAL but MUST be validated if present + // Our implementation makes it REQUIRED for security + expect(payload.aud).toBeDefined() + expect(typeof payload.aud).toBe("string") + expect(payload.aud.length).toBeGreaterThan(0) + }) + + test("SECURE: Multiple audiences should be handled correctly", async () => { + // RFC 7519 allows aud to be string or array of strings + // Our implementation uses single string for simplicity + + const token = await generateTokenForClient("single-aud") + const parts = token.split(".") + const payload = JSON.parse(atob(parts[1])) + + // We use single string audience + expect(typeof payload.aud).toBe("string") + expect(Array.isArray(payload.aud)).toBe(false) + }) + + test("SECURE: Audience validation happens before token acceptance", async () => { + const tokenA = await generateTokenForClient("client-a") + const tokenB = await generateTokenForClient("client-b") + + // Tokens are different + expect(tokenA).not.toBe(tokenB) + + // Each token only works with its own client + const resultA = await clientA.verify(subjects, tokenA) + const resultB = await clientB.verify(subjects, tokenB) + + if (resultA.err) throw resultA.err + if (resultB.err) throw resultB.err + + expect(resultA.subject).toBeDefined() + expect(resultB.subject).toBeDefined() + + // But cross-validation fails + const crossA = await clientA.verify(subjects, tokenB) + const crossB = await clientB.verify(subjects, tokenA) + + expect(crossA.err).toBeDefined() + expect(crossB.err).toBeDefined() + }) + }) + + describe("Attack Scenarios", () => { + test("VULN: Attacker steals token for service-a, tries to use at service-b", async () => { + // Simulate: User logs into service-a, attacker steals token + const stolenToken = await generateTokenForClient("service-a") + + // Attacker tries to use stolen token at service-b + const attackerClient = createClient({ + issuer: "https://auth.example.com", + clientID: "service-b", + fetch: (a, b) => Promise.resolve(auth.request(a, b)), + }) + + const result = await attackerClient.verify(subjects, stolenToken) + + // EXPECTED: Attack fails due to audience mismatch + // ACTUAL (vulnerable): Attack succeeds + expect(result.err).toBeDefined() + + if (!result.err) { + console.error("❌ CRITICAL: Token theft attack succeeded!") + console.error( + " Stolen token from service-a was accepted by service-b", + ) + throw new Error("SECURITY BREACH: Token mix-up attack succeeded") + } + }) + + test("VULN: Attacker with token for 'read' tries to access 'admin' service", async () => { + const readToken = await generateTokenForClient("read-service") + + const adminClient = createClient({ + issuer: "https://auth.example.com", + clientID: "admin-service", + fetch: (a, b) => Promise.resolve(auth.request(a, b)), + }) + + const result = await adminClient.verify(subjects, readToken) + + // Privilege escalation should fail + expect(result.err).toBeDefined() + }) + + test("VULN: Attacker modifies token audience claim (should fail signature)", async () => { + const token = await generateTokenForClient("original") + + // Try to modify the payload (this will break the signature) + const parts = token.split(".") + const payload = JSON.parse(atob(parts[1])) + + // Verify original audience + expect(payload.aud).toBe("original") + + // Even if attacker tries to modify aud in payload, + // signature verification will fail + payload.aud = "modified" + const modifiedPayload = btoa(JSON.stringify(payload)) + .replace(/\+/g, "-") + .replace(/\//g, "_") + .replace(/=/g, "") + + const modifiedToken = `${parts[0]}.${modifiedPayload}.${parts[2]}` + + const client = createClient({ + issuer: "https://auth.example.com", + clientID: "modified", + fetch: (a, b) => Promise.resolve(auth.request(a, b)), + }) + + const result = await client.verify(subjects, modifiedToken) + + // Should fail due to invalid signature + expect(result.err).toBeDefined() + }) + }) + + describe("Edge Cases", () => { + test("SECURE: Empty string audience should be rejected", async () => { + // This tests that audience must be a non-empty string + const token = await generateTokenForClient("valid") + const parts = token.split(".") + const payload = JSON.parse(atob(parts[1])) + + expect(payload.aud).not.toBe("") + expect(payload.aud.length).toBeGreaterThan(0) + }) + + test("SECURE: Null or undefined audience should be rejected", async () => { + const token = await generateTokenForClient("valid") + const parts = token.split(".") + const payload = JSON.parse(atob(parts[1])) + + expect(payload.aud).not.toBeNull() + expect(payload.aud).not.toBeUndefined() + }) + + test("SECURE: Case-sensitive audience matching", async () => { + const token = await generateTokenForClient("MyService") + + const lowerClient = createClient({ + issuer: "https://auth.example.com", + clientID: "myservice", // lowercase + fetch: (a, b) => Promise.resolve(auth.request(a, b)), + }) + + const upperClient = createClient({ + issuer: "https://auth.example.com", + clientID: "MyService", // correct case + fetch: (a, b) => Promise.resolve(auth.request(a, b)), + }) + + // Lowercase should fail (case-sensitive) + const lowerResult = await lowerClient.verify(subjects, token) + expect(lowerResult.err).toBeDefined() + + // Correct case should succeed + const upperResult = await upperClient.verify(subjects, token) + if (upperResult.err) throw upperResult.err + expect(upperResult.subject).toBeDefined() + }) + }) +}) + +describe("Test Suite Self-Check", () => { + test("INFO: These tests should currently FAIL (vulnerability exists)", () => { + console.log("\n⚠️ EXPECTED TEST RESULTS:") + console.log(" BEFORE commit 4676a2f (vulnerable):") + console.log(" ❌ Token mix-up attacks succeed") + console.log(" ❌ No audience validation") + console.log(" ❌ Tokens work across different services") + console.log("") + console.log(" AFTER commit 4676a2f (fixed):") + console.log(" ✅ Token mix-up attacks prevented") + console.log(" ✅ Audience validation enforced") + console.log(" ✅ Tokens restricted to intended service") + console.log("") + console.log("📋 VULNERABILITY:") + console.log(" JWT tokens were created with audience claims but never") + console.log(" validated during verification, violating RFC 7519 §4.1.3") + console.log("") + console.log("🎯 IMPACT:") + console.log(" Token mix-up attacks where tokens issued for one service") + console.log(" could be used to authenticate with another service") + console.log("") + console.log("🔧 FIX APPLIED IN COMMIT 4676a2f:") + console.log(" [ ] Add audience parameter to VerifyOptions") + console.log(" [ ] Validate audience in client.verify() using jose") + console.log(" [ ] Default to clientID if audience not provided") + console.log(" [ ] Validate audience in /userinfo endpoint") + console.log(" [ ] Add proper error handling") + console.log("") + console.log("📚 REFERENCES:") + console.log( + " • RFC 7519 §4.1.3: https://tools.ietf.org/html/rfc7519#section-4.1.3", + ) + console.log(" • Commit: 4676a2f03420927c1a4c23de1cfb18e799aebe51") + console.log("") + + expect(true).toBe(true) + }) +})