diff --git a/.changeset/khaki-plants-grin.md b/.changeset/khaki-plants-grin.md new file mode 100644 index 00000000..8731af2e --- /dev/null +++ b/.changeset/khaki-plants-grin.md @@ -0,0 +1,9 @@ +--- +"@kagii/openauth": minor +--- + +Improve JWT audience validation in `client.verify()` while preserving existing behavior. + +`verify()` now validates token audience against the client `clientID` by default, with optional override via `options.audience` when needed. + +This release also adds audience validation regression tests and improves `/userinfo` handling for invalid tokens. diff --git a/bun.lockb b/bun.lockb index ec8160e6..3ec27ddf 100755 Binary files a/bun.lockb and b/bun.lockb differ diff --git a/packages/openauth/package.json b/packages/openauth/package.json index 446ee25f..a56bacdd 100644 --- a/packages/openauth/package.json +++ b/packages/openauth/package.json @@ -18,7 +18,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 e418bbcd..f2e03752 100644 --- a/packages/openauth/src/client.ts +++ b/packages/openauth/src/client.ts @@ -287,7 +287,14 @@ export interface VerifyOptions { */ issuer?: string /** - * @internal + * The expected audience (aud) claim value. + * + * @example + * ```ts + * { + * audience: "api" + * } + * ``` */ audience?: string /** @@ -701,6 +708,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" @@ -708,6 +716,7 @@ export function createClient(input: ClientInput): Client { properties: v1.InferInput }>(token, jwks, { issuer, + audience: expectedAudience, }) const validated = await subjects[result.payload.type][ "~standard" @@ -733,6 +742,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 aac601fe..ae5b3130 100644 --- a/packages/openauth/src/issuer.ts +++ b/packages/openauth/src/issuer.ts @@ -1140,26 +1140,47 @@ 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) + 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 { + return c.json( + { + error: "invalid_token", + error_description: "Token verification failed", + }, + 401, + ) + } }) app.onError(async (err, c) => { diff --git a/packages/openauth/test/client.test.ts b/packages/openauth/test/client.test.ts index 8c75b2df..16a06748 100644 --- a/packages/openauth/test/client.test.ts +++ b/packages/openauth/test/client.test.ts @@ -100,7 +100,9 @@ describe("verify", () => { test("success", async () => { const refreshSpy = spyOn(client, "refresh") - const verified = await client.verify(subjects, tokens.access) + const verified = await client.verify(subjects, tokens.access, { + audience: "123", + }) expect(verified).toStrictEqual({ aud: "123", subject: { @@ -113,10 +115,24 @@ describe("verify", () => { expect(refreshSpy).not.toBeCalled() }) + test("success without expected audience", async () => { + const verified = await client.verify(subjects, tokens.access) + expect(verified).toStrictEqual({ + aud: "123", + subject: { + type: "user", + properties: { + userID: "123", + }, + }, + }) + }) + test("success after refresh", async () => { const refreshSpy = spyOn(client, "refresh") setSystemTime(Date.now() + 1000 * 6000 + 1000) const verified = await client.verify(subjects, tokens.access, { + audience: "123", refresh: tokens.refresh, }) expect(verified).toStrictEqual({ @@ -138,7 +154,9 @@ describe("verify", () => { test("failure with expired access token", async () => { setSystemTime(Date.now() + 1000 * 6000 + 1000) - const verified = await client.verify(subjects, tokens.access) + const verified = await client.verify(subjects, tokens.access, { + audience: "123", + }) expect(verified).toStrictEqual({ err: expect.any(InvalidAccessTokenError), }) @@ -147,6 +165,7 @@ describe("verify", () => { test("failure with invalid refresh token", async () => { setSystemTime(Date.now() + 1000 * 6000 + 1000) const verified = await client.verify(subjects, tokens.access, { + audience: "123", refresh: "foo", }) expect(verified).toStrictEqual({ diff --git a/packages/openauth/test/issuer.test.ts b/packages/openauth/test/issuer.test.ts index 9ded63a1..cf9d67dc 100644 --- a/packages/openauth/test/issuer.test.ts +++ b/packages/openauth/test/issuer.test.ts @@ -147,7 +147,9 @@ describe("code flow", () => { refresh: expectNonEmptyString, expiresIn: 60, }) - const verified = await client.verify(subjects, tokens.access) + const verified = await client.verify(subjects, tokens.access, { + audience: "123", + }) if (verified.err) throw verified.err expect(verified.subject).toStrictEqual({ type: "user", @@ -251,7 +253,7 @@ describe("client credentials flow", () => { test("success", async () => { const client = createClient({ issuer: "https://auth.example.com", - clientID: "123", + clientID: "myuser", fetch: (a, b) => Promise.resolve(auth.request(a, b)), }) const response = await auth.request("https://auth.example.com/token", { @@ -272,7 +274,9 @@ describe("client credentials flow", () => { access_token: expectNonEmptyString, refresh_token: expectNonEmptyString, }) - const verified = await client.verify(subjects, tokens.access_token) + const verified = await client.verify(subjects, tokens.access_token, { + audience: "myuser", + }) expect(verified).toStrictEqual({ aud: "myuser", subject: { @@ -355,7 +359,9 @@ describe("refresh token", () => { expect(refreshed.access_token).not.toEqual(tokens.access) expect(refreshed.refresh_token).not.toEqual(tokens.refresh) - const verified = await client.verify(subjects, refreshed.access_token) + const verified = await client.verify(subjects, refreshed.access_token, { + audience: "123", + }) expect(verified).toStrictEqual({ aud: "123", subject: { @@ -382,7 +388,9 @@ describe("refresh token", () => { expect(refreshed.access_token).not.toEqual(tokens.access) expect(refreshed.refresh_token).not.toEqual(tokens.refresh) - const verified = await client.verify(subjects, refreshed.access_token) + const verified = await client.verify(subjects, refreshed.access_token, { + audience: "123", + }) expect(verified).toStrictEqual({ aud: "123", subject: { @@ -518,4 +526,16 @@ describe("user info", () => { expect(userinfo).toStrictEqual({ userID: "123" }) }) + + test("invalid token", async () => { + const response = await auth.request("https://auth.example.com/userinfo", { + headers: { Authorization: "Bearer invalid.token.here" }, + }) + + expect(response.status).toBe(401) + expect(await response.json()).toStrictEqual({ + error: "invalid_token", + error_description: "Token verification failed", + }) + }) }) diff --git a/packages/openauth/test/jwt-audience-validation.test.ts b/packages/openauth/test/jwt-audience-validation.test.ts new file mode 100644 index 00000000..3b25e3ca --- /dev/null +++ b/packages/openauth/test/jwt-audience-validation.test.ts @@ -0,0 +1,167 @@ +import { + afterEach, + beforeEach, + describe, + expect, + setSystemTime, + test, +} from "bun:test" +import { object, string } from "valibot" +import { createClient } from "../src/client.js" +import { InvalidAccessTokenError } from "../src/error.js" +import { issuer } from "../src/issuer.js" +import type { Provider } from "../src/provider/provider.js" +import { MemoryStorage } from "../src/storage/memory.js" +import { createSubjects } from "../src/subject.js" + +const subjects = createSubjects({ + user: object({ + userID: string(), + }), +}) + +describe("jwt audience validation", () => { + let auth: ReturnType + + beforeEach(() => { + setSystemTime(new Date("1/1/2024")) + auth = issuer({ + storage: MemoryStorage(), + subjects, + allow: async () => true, + ttl: { + access: 60, + refresh: 6000, + }, + 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) => { + return ctx.subject("user", { + userID: "123", + }) + }, + }) + }) + + afterEach(() => { + setSystemTime() + }) + + function createTestClient(clientID: string) { + return createClient({ + issuer: "https://auth.example.com", + clientID, + fetch: (a, b) => Promise.resolve(auth.request(a, b)), + }) + } + + async function issueTokens(clientID: string) { + const client = createTestClient(clientID) + const redirectURI = "https://client.example.com/callback" + const { challenge, url } = await client.authorize(redirectURI, "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!, + redirectURI, + challenge.verifier, + ) + + if (exchanged.err) throw exchanged.err + return exchanged.tokens + } + + test("rejects a token issued for another client", async () => { + const tokens = await issueTokens("client-a") + const client = createTestClient("client-b") + + expect( + await client.verify(subjects, tokens.access, { + audience: "client-b", + }), + ).toStrictEqual({ + err: expect.any(InvalidAccessTokenError), + }) + }) + + test("accepts an explicit matching audience override", async () => { + const tokens = await issueTokens("client-a") + const client = createTestClient("client-b") + + const verified = await client.verify(subjects, tokens.access, { + audience: "client-a", + }) + + if (verified.err) throw verified.err + expect(verified).toStrictEqual({ + aud: "client-a", + subject: { + type: "user", + properties: { + userID: "123", + }, + }, + }) + }) + + test("rejects an explicit mismatched audience override", async () => { + const tokens = await issueTokens("client-a") + const client = createTestClient("client-b") + + expect( + await client.verify(subjects, tokens.access, { + audience: "client-b", + }), + ).toStrictEqual({ + err: expect.any(InvalidAccessTokenError), + }) + }) + + test("preserves the explicit audience when refreshing an expired token", async () => { + const tokens = await issueTokens("client-a") + const client = createTestClient("client-b") + + setSystemTime(Date.now() + 1000 * 61) + + const verified = await client.verify(subjects, tokens.access, { + audience: "client-a", + refresh: tokens.refresh, + }) + + if (verified.err) throw verified.err + expect(verified).toStrictEqual({ + aud: "client-a", + tokens: { + access: expect.stringMatching(/.+/), + refresh: expect.stringMatching(/.+/), + expiresIn: 60, + }, + subject: { + type: "user", + properties: { + userID: "123", + }, + }, + }) + }) +}) diff --git a/packages/openauth/test/scrap.test.ts b/packages/openauth/test/scrap.test.ts index eb782914..57e4584a 100644 --- a/packages/openauth/test/scrap.test.ts +++ b/packages/openauth/test/scrap.test.ts @@ -65,15 +65,20 @@ test("code flow", async () => { if (exchanged.err) throw exchanged.err expect(exchanged.tokens.access).toBeTruthy() expect(exchanged.tokens.refresh).toBeTruthy() - const verified = await client.verify(subjects, exchanged.tokens.access) + const verified = await client.verify(subjects, exchanged.tokens.access, { + audience: "123", + }) if (verified.err) throw verified.err expect(verified.subject.type).toBe("user") if (verified.subject.type !== "user") throw new Error("Invalid subject") expect(verified.subject.properties.userID).toBe("123") await new Promise((resolve) => setTimeout(resolve, 2000)) - const failed = await client.verify(subjects, exchanged.tokens.access) + const failed = await client.verify(subjects, exchanged.tokens.access, { + audience: "123", + }) expect(failed.err).toBeInstanceOf(Error) const next = await client.verify(subjects, exchanged.tokens.access, { + audience: "123", refresh: exchanged.tokens.refresh, }) if (next.err) throw next.err @@ -81,5 +86,7 @@ test("code flow", async () => { expect(next.tokens?.refresh).toBeDefined() expect(next.tokens?.access).not.toEqual(exchanged.tokens.access) expect(next.tokens?.refresh).not.toEqual(exchanged.tokens.refresh) - await client.verify(subjects, next.tokens!.access!) + await client.verify(subjects, next.tokens!.access!, { + audience: "123", + }) })