diff --git a/docs/service-contract.md b/docs/service-contract.md index 47fa7fa..001b34b 100644 --- a/docs/service-contract.md +++ b/docs/service-contract.md @@ -495,14 +495,18 @@ Important behavior: response without a new refresh token revokes the credential instead of keeping the old token - `410 {"error":"managed_credential_revoked","provider":...}`: the provider - rejected the grant (`invalid_grant`), or the identity holds no access token. + rejected the grant (HTTP 400 whose OAuth `error` is exactly `invalid_grant`), + or the identity holds no access token. The stored tokens are cleared and an audit event `managed_credential_revoked` is written; the owner must reconnect. `404 managed_credential_not_found` means no linked identity exists - `502 {"error":"managed_credential_refresh_failed","provider":...}`: the refresh failed for any reason that is not a grant verdict (invalid client, - provider 5xx, timeout, malformed body). Stored tokens are untouched and a - later request may succeed + a 401 or 5xx whatever its body says, timeout, a 2xx without a string + `access_token` or carrying an `error`, an empty or non-JSON body). Stored + tokens are untouched and a later request may succeed. An answer without + `expires_in` stores the expiry as unknown, and an unknown expiry is never + due, so it does not trigger another refresh - app-local binding, diagnostics, subscription setup, and runtime behavior stay app-owned diff --git a/src/managed-credential.ts b/src/managed-credential.ts index 4da4b5a..6a06727 100644 --- a/src/managed-credential.ts +++ b/src/managed-credential.ts @@ -113,7 +113,10 @@ export async function refreshManagedCredential( refreshTokenEncrypted = locked.refreshTokenEncrypted!; } - const tokenExpiresAt = refreshed.expiresAt ?? locked.tokenExpiresAt; + // An answer without expires_in leaves the expiry unknown. The old expiry is + // already due, so keeping it would refresh again on every resolve; an + // unknown expiry is never due (dueForRefresh). + const tokenExpiresAt = refreshed.expiresAt; return { kind: "rotate", accessTokenEncrypted: ctx.tokenCustody.encrypt(refreshed.accessToken), diff --git a/src/oauth.ts b/src/oauth.ts index c46976e..9593b54 100644 --- a/src/oauth.ts +++ b/src/oauth.ts @@ -89,7 +89,7 @@ function readOAuthErrorCode(text: string): OAuthErrorCode | undefined { */ export class ProviderResponseError extends Error { readonly status: number; - readonly code: "provider_status" | "provider_body_not_json"; + readonly code: "provider_status" | "provider_body_not_json" | "provider_body_not_token_set"; /** * The RFC 6749 section 5.2 error code of a refusal, when the provider sent * one from that closed set. The only value taken from a provider's body. @@ -109,7 +109,14 @@ export class ProviderResponseError extends Error { async function readProviderJson(response: Response, failure: string): Promise { const text = await response.text(); if (!response.ok) { - throw new ProviderResponseError(response.status, "provider_status", failure, readOAuthErrorCode(text)); + // RFC 6749 section 5.2 sends a grant verdict as 400 only. The same JSON + // behind a 401, a 5xx or a gateway page is not one. + throw new ProviderResponseError( + response.status, + "provider_status", + failure, + response.status === 400 ? readOAuthErrorCode(text) : undefined + ); } if (!text) { return {} as T; @@ -173,13 +180,28 @@ export async function requestProviderTokens( init.signal = signal; } const label = grant.grant_type === "refresh_token" ? "refresh" : "exchange"; - const tokenResponse = await fetchJson<{ + const failure = `${descriptor.displayName} token ${label} failed`; + const response = await fetch(descriptor.tokenEndpoint, init); + const answer = await readProviderJson(response, failure); + // A 2xx is a token set only when it is an object holding a string access + // token and no error. + if ( + typeof answer !== "object" || + answer === null || + Array.isArray(answer) || + typeof (answer as { access_token?: unknown }).access_token !== "string" || + !(answer as { access_token: string }).access_token || + (answer as { error?: unknown }).error !== undefined + ) { + throw new ProviderResponseError(response.status, "provider_body_not_token_set", failure); + } + const tokenResponse = answer as { access_token: string; refresh_token?: string; token_type?: string; expires_in?: number; scope?: string | string[]; - }>(descriptor.tokenEndpoint, init, `${descriptor.displayName} token ${label} failed`); + }; const tokenSet: OAuthTokenSet = { accessToken: tokenResponse.access_token, diff --git a/tests/managed-credential.test.ts b/tests/managed-credential.test.ts index f9a9c88..7758534 100644 --- a/tests/managed-credential.test.ts +++ b/tests/managed-credential.test.ts @@ -719,6 +719,152 @@ describe("refreshManagedCredential without the lock", () => { }); }); +describe("refresh answer classification", () => { + const config = testConfig(); + const custody = createTokenCustody(config); + const now = Date.parse("2026-10-19T12:00:00.000Z"); + const dueAt = new Date(now).toISOString(); + + async function seedRow(store: InMemoryStore, provider: Provider) { + const created = new Date(now).toISOString(); + const account = await store.createAccount({ createdAt: created, lastSeenAt: created }); + return store.upsertLinkedIdentity({ + accountId: account.id, + provider, + providerUserId: `u-${randomUUID()}`, + scopes: ["s"], + profileJson: {}, + accessTokenEncrypted: custody.encrypt("access-0"), + refreshTokenEncrypted: custody.encrypt("refresh-0"), + tokenExpiresAt: dueAt, + createdAt: created, + updatedAt: created, + }); + } + + const ctxFor = (store: HeimdallStore, extra: { timeoutMs?: number } = {}) => ({ + config, + store, + tokenCustody: custody, + oauthRuntimes: createOAuthRuntimeRegistry(), + clock: () => now, + ...extra, + }); + + function answer(body: string, status: number) { + const calls: string[] = []; + globalThis.fetch = (async (_input: string | URL, init?: RequestInit) => { + calls.push(String((init!.body as URLSearchParams).get("refresh_token"))); + return new Response(body, { status }); + }) as typeof fetch; + return calls; + } + + afterEach(() => { + globalThis.fetch = originalFetch; + }); + + const grantVerdict = JSON.stringify({ error: "invalid_grant" }); + const keepsTokens: Array<[string, string, number]> = [ + ["an error code that only contains invalid_grant", JSON.stringify({ error: "invalid_grant_extra" }), 400], + ["an error code that is contained in a longer one", JSON.stringify({ error: "not_invalid_grant" }), 400], + ["a different error code", JSON.stringify({ error: "invalid_request" }), 400], + ["a 401 carrying invalid_grant", grantVerdict, 401], + ["a 500 carrying invalid_grant", grantVerdict, 500], + ["a 503 carrying invalid_grant", grantVerdict, 503], + ["a 2xx carrying invalid_grant", grantVerdict, 200], + ["a 2xx carrying an error beside an access token", JSON.stringify({ access_token: "a", error: "server_error" }), 200], + ["a 2xx with an empty object", "{}", 200], + ["a 2xx with an empty body", "", 200], + ["a 2xx with JSON null", "null", 200], + ["a 2xx with a JSON array", "[]", 200], + ["a 2xx with a non-string access token", JSON.stringify({ access_token: 7, refresh_token: "r" }), 200], + ["a 2xx with an empty access token", JSON.stringify({ access_token: "", refresh_token: "r" }), 200], + ["a 2xx that is not JSON", "", 200], + ["a 5xx page", "upstream exploded", 502], + ]; + + describe.each(["twitch", "joystick"] as const)("%s", (provider) => { + it.each(keepsTokens)("keeps the tokens and fails typed for %s", async (_name, body, status) => { + const store = new InMemoryStore(); + const row = await seedRow(store, provider); + const calls = answer(body, status); + await expect(refreshManagedCredential(ctxFor(store), row)).rejects.toBeInstanceOf(ManagedCredentialRefreshFailed); + expect(calls).toEqual(["refresh-0"]); + const after = (await store.findStoredLinkedIdentity(provider, row.providerUserId))!; + expect(after.accessTokenEncrypted).toBe(row.accessTokenEncrypted); + expect(after.refreshTokenEncrypted).toBe(row.refreshTokenEncrypted); + expect(after.tokenExpiresAt).toBe(dueAt); + }); + + it("keeps the tokens and fails typed when the provider never answers", async () => { + const store = new InMemoryStore(); + const row = await seedRow(store, provider); + globalThis.fetch = ((_input: string | URL, init?: RequestInit) => + new Promise((_resolve, reject) => { + init!.signal?.addEventListener("abort", () => reject(new DOMException("aborted", "AbortError"))); + })) as typeof fetch; + await expect(refreshManagedCredential(ctxFor(store, { timeoutMs: 30 }), row)).rejects.toBeInstanceOf(ManagedCredentialRefreshFailed); + const after = (await store.findStoredLinkedIdentity(provider, row.providerUserId))!; + expect(after.refreshTokenEncrypted).toBe(row.refreshTokenEncrypted); + }); + + it("revokes and clears the tokens on a 400 whose error is exactly invalid_grant", async () => { + const store = new InMemoryStore(); + const row = await seedRow(store, provider); + answer(grantVerdict, 400); + const outcome = await refreshManagedCredential(ctxFor(store), row); + expect(outcome.kind).toBe("revoked"); + const after = (await store.findStoredLinkedIdentity(provider, row.providerUserId))!; + expect(after.accessTokenEncrypted).toBeUndefined(); + expect(after.refreshTokenEncrypted).toBeUndefined(); + }); + }); + + it("a rotating provider's well-formed answer without a refresh token still revokes (rotation_missing)", async () => { + const store = new InMemoryStore(); + const row = await seedRow(store, "joystick"); + answer(JSON.stringify({ access_token: "a", expires_in: 3600 }), 200); + expect((await refreshManagedCredential(ctxFor(store), row)).kind).toBe("revoked"); + expect((await store.findStoredLinkedIdentity("joystick", row.providerUserId))!.refreshTokenEncrypted).toBeUndefined(); + }); + + it("an answer without expires_in stores the expiry as unknown, and the next resolve does not refresh again", async () => { + const store = new InMemoryStore(); + const row = await seedRow(store, "twitch"); + const calls = answer(JSON.stringify({ access_token: "access-1", refresh_token: "refresh-1" }), 200); + const first = await refreshManagedCredential(ctxFor(store), row); + expect(first.kind).toBe("current"); + expect(first.identity.tokenExpiresAt).toBeUndefined(); + const stored = (await store.findStoredLinkedIdentity("twitch", row.providerUserId))!; + expect(stored.tokenExpiresAt).toBeUndefined(); + expect(custody.decrypt(stored.accessTokenEncrypted!)).toBe("access-1"); + + await refreshManagedCredential(ctxFor(store), stored); + expect(calls).toEqual(["refresh-0"]); + }); + + it("a non-rotating provider keeps the locked row's refresh token, not the snapshot's", async () => { + const store = new InMemoryStore(); + const snapshot = await seedRow(store, "twitch"); + // Another caller rotated the pair after the snapshot; its expiry is still due. + await store.updateLinkedIdentityCredential(snapshot.id, async () => ({ + kind: "rotate", + accessTokenEncrypted: custody.encrypt("access-1"), + refreshTokenEncrypted: custody.encrypt("refresh-1"), + tokenExpiresAt: dueAt, + scopes: ["s"], + updatedAt: dueAt, + })); + const calls = answer(JSON.stringify({ access_token: "access-2", expires_in: 3600 }), 200); + await refreshManagedCredential(ctxFor(store), snapshot); + expect(calls).toEqual(["refresh-1"]); + const after = (await store.findStoredLinkedIdentity("twitch", snapshot.providerUserId))!; + expect(custody.decrypt(after.refreshTokenEncrypted!)).toBe("refresh-1"); + expect(custody.decrypt(after.accessTokenEncrypted!)).toBe("access-2"); + }); +}); + describe("live Postgres availability", () => { it.skipIf(!!adminUrl)("is present whenever the job requires it", () => { // Local workstation runs skip the live suites; the verification job sets diff --git a/tests/provider-token-request.test.ts b/tests/provider-token-request.test.ts index 875598e..501a6e3 100644 --- a/tests/provider-token-request.test.ts +++ b/tests/provider-token-request.test.ts @@ -134,10 +134,26 @@ describe("requestProviderTokens follows the provider descriptor", () => { expect((await requestProviderTokens("twitch", configFor(), { grant_type: "refresh_token", refresh_token: "r" })).scope).toEqual(["a", "b"]); }); - it("treats an empty 2xx body as an empty answer, not a malformed one", async () => { - capture(() => new Response("", { status: 200 })); - const tokens = await requestProviderTokens("twitch", configFor(), { grant_type: "refresh_token", refresh_token: "r" }); - expect(tokens.accessToken).toBeUndefined(); + it.each([ + ["an empty body", ""], + ["an empty object", "{}"], + ["JSON null", "null"], + ["a JSON array", "[]"], + ["an error beside an access token", JSON.stringify({ access_token: "a", error: "invalid_grant" })], + ["an empty access token", JSON.stringify({ access_token: "" })], + ["a numeric access token", JSON.stringify({ access_token: 5 })], + ])("refuses a 2xx with %s as not a token set, carrying nothing of the body", async (_name, body) => { + capture(() => new Response(body, { status: 200 })); + const error = await requestProviderTokens("twitch", configFor(), { grant_type: "refresh_token", refresh_token: "r" }).then( + () => { + throw new Error("expected a refusal"); + }, + (e: unknown) => e as Error & { code?: string; status?: number; oauthError?: unknown } + ); + expect(error.name).toBe("ProviderResponseError"); + expect(error.code).toBe("provider_body_not_token_set"); + expect(error.status).toBe(200); + expect(error.oauthError).toBeUndefined(); }); it("sets no refreshToken key when the provider sent none", async () => { @@ -182,6 +198,19 @@ describe("oauthError is a closed set", () => { expect(`${error.message} ${error.stack} ${inspect(error, { depth: 5, showHidden: true })} ${JSON.stringify(error)}`).not.toContain("CANARY"); }); + it.each([401, 403, 429, 500, 502, 503])("takes no oauthError from a %i, whatever its body says", async (status) => { + const error = await failure(JSON.stringify({ error: "invalid_grant" }), status); + expect(error.oauthError).toBeUndefined(); + }); + + it.each(["invalid_grant_extra", "not_invalid_grant", "INVALID_GRANT", " invalid_grant", "invalid_grant "])( + "takes the code by exact match only: %j is no code", + async (code) => { + const error = await failure(JSON.stringify({ error: code })); + expect(error.oauthError).toBeUndefined(); + } + ); + it.each([ ["an error outside the RFC set", JSON.stringify({ error: "CANARY_unknown" })], ["a non-string error", JSON.stringify({ error: { code: "invalid_grant" } })],