diff --git a/src/services/auth-service.test.ts b/src/services/auth-service.test.ts index ff4ef650..75c8e27b 100644 --- a/src/services/auth-service.test.ts +++ b/src/services/auth-service.test.ts @@ -185,6 +185,114 @@ describe("AuthServiceImpl", () => { ); } }); + + it("coerces a numeric-string expires_in to a number", async () => { + const fetchMock = mock(() => + Promise.resolve( + new Response( + JSON.stringify({ + access_token: "new-access-token", + expires_in: "120", + }), + { status: 200 }, + ), + ), + ); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const result = await service.refreshAccessToken({ + tokenEndpoint: "https://auth.example.com/oauth/token", + clientId: "client-id", + clientSecret: "client-secret", + refreshToken: "refresh-token", + }); + + expect(result.expiresIn).toBe(120); + }); + + it("falls back to the default lifetime for non-positive or non-numeric expires_in", async () => { + for (const bad of [0, -5, "soon", null]) { + const fetchMock = mock(() => + Promise.resolve( + new Response( + JSON.stringify({ + access_token: "new-access-token", + expires_in: bad, + }), + { status: 200 }, + ), + ), + ); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const result = await service.refreshAccessToken({ + tokenEndpoint: "https://auth.example.com/oauth/token", + clientId: "client-id", + clientSecret: "client-secret", + refreshToken: "refresh-token", + }); + + expect(result.expiresIn).toBe(3600); + } + }); + + it("rejects responses whose access_token is not a non-empty string", async () => { + for (const bad of [123, "", null, { token: "x" }]) { + const fetchMock = mock(() => + Promise.resolve( + new Response(JSON.stringify({ access_token: bad }), { + status: 200, + }), + ), + ); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + await expect( + service.refreshAccessToken({ + tokenEndpoint: "https://auth.example.com/oauth/token", + clientId: "client-id", + clientSecret: "client-secret", + refreshToken: "refresh-token", + }), + ).rejects.toThrow("Token response missing required fields"); + } + }); + }); + + describe("classifyTerminalRefreshError", () => { + it("classifies invalid_grant regardless of description wording", () => { + const error = new TokenRefreshError( + 400, + JSON.stringify({ + error: "invalid_grant", + error_description: "the token is no longer recognized by the server", + }), + ); + expect(classifyTerminalRefreshError(error)).toBe("invalid_refresh_token"); + }); + + it("classifies invalid_client from the structured code", () => { + const error = new TokenRefreshError( + 401, + JSON.stringify({ error: "invalid_client" }), + ); + expect(classifyTerminalRefreshError(error)).toBe("invalid_client"); + }); + + it("returns undefined for non-terminal OAuth errors", () => { + const error = new TokenRefreshError( + 400, + JSON.stringify({ + error: "invalid_request", + error_description: "missing parameter", + }), + ); + expect(classifyTerminalRefreshError(error)).toBeUndefined(); + }); + + it("returns undefined for non-TokenRefreshError values", () => { + expect(classifyTerminalRefreshError(new Error("boom"))).toBeUndefined(); + }); }); describe("evaluateCallback", () => { diff --git a/src/services/auth-service.ts b/src/services/auth-service.ts index 666f2aac..25c344eb 100644 --- a/src/services/auth-service.ts +++ b/src/services/auth-service.ts @@ -6,6 +6,7 @@ import { generateCodeVerifier, generateState, } from "@githits/core-internal"; +import { z } from "zod"; /** * OAuth Authorization Server metadata from .well-known endpoint. @@ -449,13 +450,20 @@ export function classifyTerminalRefreshError( return "invalid_client"; } + // RFC 6749 ยง5.2: for the refresh_token grant, `invalid_grant` means the + // refresh token is expired, revoked, or otherwise no longer valid. Trust the + // structured code on its own; the substring checks below are a fallback for + // servers that report the same condition without a clean error code. + if (oauthError === "invalid_grant") { + return "invalid_refresh_token"; + } + if ( - oauthError === "invalid_grant" && - (text.includes("invalid refresh token") || - text.includes("refresh token already used") || - text.includes("already used") || - text.includes("session expired") || - text.includes("session not found")) + text.includes("invalid refresh token") || + text.includes("refresh token already used") || + text.includes("already used") || + text.includes("session expired") || + text.includes("session not found") ) { return "invalid_refresh_token"; } @@ -489,28 +497,66 @@ function stringField(value: unknown): string | undefined { return typeof value === "string" && value.trim() ? value : undefined; } +/** + * Default token lifetime (seconds) applied when the OAuth server omits + * `expires_in` or returns a value that is not a finite positive number. + */ +const DEFAULT_TOKEN_EXPIRES_IN_SECONDS = 3600; + +const nonEmptyString = z.string().min(1); + +/** + * Coerces `expires_in` to a usable lifetime. OAuth servers should send a JSON + * number per RFC 6749, but some emit numeric strings, and a missing, zero, + * negative, or non-numeric value would otherwise corrupt the computed expiry + * (e.g. `new Date(Date.now() + NaN)` throws). Any non-positive or non-finite + * value falls back to the default lifetime. + */ +const expiresInSchema = z + .union([z.number(), z.string()]) + .nullish() + .transform((value) => { + const numeric = typeof value === "string" ? Number(value) : value; + return typeof numeric === "number" && + Number.isFinite(numeric) && + numeric > 0 + ? numeric + : DEFAULT_TOKEN_EXPIRES_IN_SECONDS; + }); + +const TokenResponseSchema = z.object({ + access_token: nonEmptyString, + refresh_token: nonEmptyString, + expires_in: expiresInSchema, +}); + +const RefreshTokenResponseSchema = z.object({ + access_token: nonEmptyString, + refresh_token: nonEmptyString.optional(), + expires_in: expiresInSchema, +}); + function parseTokenResponse(data: unknown): TokenResponse { - const d = data as Record; - if (!d.access_token || !d.refresh_token) { + const parsed = TokenResponseSchema.safeParse(data); + if (!parsed.success) { throw new Error("Token response missing required fields"); } return { - accessToken: d.access_token as string, - refreshToken: d.refresh_token as string, - expiresIn: (d.expires_in as number) || 3600, + accessToken: parsed.data.access_token, + refreshToken: parsed.data.refresh_token, + expiresIn: parsed.data.expires_in, }; } function parseRefreshTokenResponse(data: unknown): RefreshTokenResponse { - const d = data as Record; - if (!d.access_token) { + const parsed = RefreshTokenResponseSchema.safeParse(data); + if (!parsed.success) { throw new Error("Token response missing required fields"); } return { - accessToken: d.access_token as string, - refreshToken: - typeof d.refresh_token === "string" ? d.refresh_token : undefined, - expiresIn: (d.expires_in as number) || 3600, + accessToken: parsed.data.access_token, + refreshToken: parsed.data.refresh_token, + expiresIn: parsed.data.expires_in, }; }