From 71ce9c16e40424745f33a8a3753a47ba4cdada06 Mon Sep 17 00:00:00 2001 From: Heimdall Date: Sun, 8 Nov 2026 13:30:39 +0000 Subject: [PATCH] Consent review fixes: ask-only cookie, fixed-expiry grants links, wording carried to the receipt, one revocation per grant The ask sets its own cookie and no launch binding; Continue sets the binding and carries the exact text shown into it; the receipt stores that text and its hash. Grants-page form tokens keep the link's expiry. Revocations are unique per grant and revoke reports only what it wrote. access_denied's attempt denial code is pinned for Bifrost and Repixelizer. Co-Authored-By: Claude Sonnet 5.5 --- src/app.ts | 161 +++++++++++++++++++++++------- src/consent.ts | 27 ++++- src/store/in-memory.ts | 8 +- src/store/postgres.ts | 21 +++- src/store/schema.ts | 6 ++ src/store/types.ts | 9 +- tests/consent-drift.test.ts | 65 ++++++++++++ tests/joystick-consent.test.ts | 131 ++++++++++++++++++++---- tests/oauth-flow.ts | 4 +- tests/oauth-state-binding.test.ts | 32 ++++++ tests/store.test.ts | 20 +++- 11 files changed, 413 insertions(+), 71 deletions(-) create mode 100644 tests/consent-drift.test.ts diff --git a/src/app.ts b/src/app.ts index c4d8e93..c6c7c68 100644 --- a/src/app.ts +++ b/src/app.ts @@ -1,4 +1,5 @@ import { createHmac, randomUUID } from "node:crypto"; +import { deflateRawSync, inflateRawSync } from "node:zlib"; import cookie from "@fastify/cookie"; import cors from "@fastify/cors"; import Fastify, { type FastifyError, type FastifyInstance, type FastifyReply, type FastifyRequest } from "fastify"; @@ -26,7 +27,7 @@ import { import { issueAccessClaim, type IssueAccessClaimInput } from "./claims.js"; import { builtInAppProfiles, getAppProfile, isAllowedReturnOrigin, serializeAppProfile, admitsProviderForMode, callbackMint } from "./app-profiles.js"; import { renderBrowserHandoffPage } from "./browser-handoff.js"; -import { consentDigest, grantViews, GRANTS_PATH, renderConsentPage, renderGrantsPage, type ConsentAsk } from "./consent.js"; +import { consentText, digestOfText, grantViews, GRANTS_PATH, renderConsentPage, renderGrantsPage, type ConsentAsk } from "./consent.js"; import { type HeimdallConfig, loadConfig } from "./config.js"; import { createTokenCustody, type TokenCustody } from "./custody.js"; import { grantFact } from "./facts.js"; @@ -252,14 +253,74 @@ function oauthBindingCookieName(jti: string): string { function signOAuthBinding( config: HeimdallConfig, keys: ReturnType, - state: OAuthStatePayload + state: OAuthStatePayload, + shownText?: string ): string { return signJwt( - { typ: oauthBindingType, iss: config.issuer, jti: state.jti, iat: nowEpochSeconds(), exp: state.exp }, + { + typ: oauthBindingType, + iss: config.issuer, + jti: state.jti, + iat: nowEpochSeconds(), + exp: state.exp, + // The wording the person agreed to, carried from the ask to the receipt. + ...(shownText === undefined ? {} : { ct: deflateRawSync(shownText).toString("base64url") }), + }, keys ); } +const consentAskType = "heimdall_consent_ask"; + +function consentAskCookieName(jti: string): string { + return `__Host-heimdall-ask-${jti}`; +} + +/** + * The ask's own cookie. It proves a browser was shown the ask and carries the + * exact text shown; only Continue and No accept it. It is not the launch + * binding: that is set when Continue claims the launch, so that merely opening + * the launch URL never makes a browser the state's owner. + */ +function signConsentAsk(config: HeimdallConfig, keys: ReturnType, state: OAuthStatePayload, shownText: string): string { + return signJwt( + { typ: consentAskType, iss: config.issuer, jti: state.jti, iat: nowEpochSeconds(), exp: state.exp, ct: deflateRawSync(shownText).toString("base64url") }, + keys + ); +} + +/** The text a held cookie of the given type carries, or null when the cookie is absent, foreign, expired or carries none. */ +function cookieConsentText( + config: HeimdallConfig, + keys: ReturnType, + token: string | undefined, + type: string, + state: OAuthStatePayload +): string | null { + if (!token) return null; + const verification = verifyJwt(token, keys.publicKey); + if (!verification.valid) return null; + const { typ, iss, jti, exp, ct } = verification.payload; + if (typ !== type || iss !== config.issuer || jti !== state.jti || typeof exp !== "number" || exp <= nowEpochSeconds() || typeof ct !== "string") { + return null; + } + try { + return inflateRawSync(Buffer.from(ct, "base64url")).toString("utf8"); + } catch { + return null; + } +} + +function setConsentAskCookie(reply: FastifyReply, state: OAuthStatePayload, value: string, maxAgeSeconds: number): void { + reply.setCookie(consentAskCookieName(state.jti), value, { + path: "/", + secure: true, + httpOnly: true, + sameSite: "lax", + maxAge: Math.max(0, maxAgeSeconds), + }); +} + function oauthBindingHolds( config: HeimdallConfig, keys: ReturnType, @@ -827,18 +888,27 @@ const grantsPageTokenType = "heimdall_grants_page"; const grantsPageTokenTtlSeconds = 600; /** A short, purpose-bound proof of a signed-in account, so the grants page can be opened and its forms posted by a browser that holds no session. */ -function signGrantsPageToken(config: HeimdallConfig, keys: ReturnType, accountId: string): string { - const iat = nowEpochSeconds(); - return signJwt({ typ: grantsPageTokenType, iss: config.issuer, account_id: accountId, iat, exp: iat + grantsPageTokenTtlSeconds }, keys); +function signGrantsPageToken( + config: HeimdallConfig, + keys: ReturnType, + accountId: string, + exp: number = nowEpochSeconds() + grantsPageTokenTtlSeconds +): string { + return signJwt({ typ: grantsPageTokenType, iss: config.issuer, account_id: accountId, iat: nowEpochSeconds(), exp }, keys); } -function grantsPageTokenAccount(config: HeimdallConfig, keys: ReturnType, token: unknown): string | null { +/** The account and expiry a page token proves. A page's own forms are signed to this same expiry, so a link is never renewed by being used. */ +function grantsPageTokenAccount( + config: HeimdallConfig, + keys: ReturnType, + token: unknown +): { accountId: string; exp: number } | null { if (typeof token !== "string") return null; const verification = verifyJwt(token, keys.publicKey); if (!verification.valid) return null; const { typ, iss, account_id: accountId, exp } = verification.payload; return typ === grantsPageTokenType && iss === config.issuer && typeof accountId === "string" && typeof exp === "number" && exp > nowEpochSeconds() - ? accountId + ? { accountId, exp } : null; } @@ -1239,7 +1309,7 @@ export async function buildApp(options: BuildAppOptions = {}): Promise { + const beginProviderRedirect = async (request: FastifyRequest, reply: FastifyReply, state: OAuthStatePayload, rawState: string, shownText?: string) => { const clientId = config.providers[state.provider].clientId; if (!clientId) { return providerNotConfigured(reply, state); @@ -1256,7 +1326,7 @@ export async function buildApp(options: BuildAppOptions = {}): Promise => - (await resolveAccessAccount({ config, keys, store }, request.headers.authorization)) ?? - grantsPageTokenAccount(config, keys, pageToken); + /** The signed-in account, and the expiry the page's own forms get: a bearer starts a fresh window, a page link only passes on the one it has. */ + const grantsAccount = async (request: FastifyRequest, pageToken?: unknown): Promise<{ accountId: string; exp: number } | null> => { + const bearerAccount = await resolveAccessAccount({ config, keys, store }, request.headers.authorization); + if (bearerAccount) return { accountId: bearerAccount, exp: nowEpochSeconds() + grantsPageTokenTtlSeconds }; + return grantsPageTokenAccount(config, keys, pageToken); + }; const grantsReply = (reply: FastifyReply) => { reply.header("Cache-Control", "no-store"); @@ -2526,13 +2610,14 @@ export async function buildApp(options: BuildAppOptions = {}): Promise { - const accountId = await grantsAccount(request, request.query.t); - if (!accountId) return signInRequired(reply); + const caller = await grantsAccount(request, request.query.t); + if (!caller) return signInRequired(reply); + const { accountId } = caller; grantsReply(reply); const receipts = await store.listConsentReceipts(accountId); if (prefersHtml(request.headers.accept)) { reply.type("text/html; charset=utf-8"); - return reply.send(renderGrantsPage(grantViews(receipts), signGrantsPageToken(config, keys, accountId))); + return reply.send(renderGrantsPage(grantViews(receipts), signGrantsPageToken(config, keys, accountId, caller.exp))); } return { grants: grantViews(receipts), receipts }; } @@ -2559,8 +2644,9 @@ export async function buildApp(options: BuildAppOptions = {}): Promise { - const accountId = await grantsAccount(request, request.body?.t); - if (!accountId) return signInRequired(reply); + const caller = await grantsAccount(request, request.body?.t); + if (!caller) return signInRequired(reply); + const { accountId } = caller; grantsReply(reply); const receipts = await store.listConsentReceipts(accountId); const grant = grantViews(receipts).find((view) => view.id === request.params.grantId); @@ -2580,7 +2666,7 @@ export async function buildApp(options: BuildAppOptions = {}): Promise ({ kind: "revoke", updatedAt: at })); } for (const live of grantViews(receipts).filter((view) => view.provider === grant.provider && view.status === "active")) { - await store.createConsentReceipt({ + const { created } = await store.createConsentReceipt({ kind: "revocation", accountId, appSlug: live.appSlug, @@ -2588,19 +2674,22 @@ export async function buildApp(options: BuildAppOptions = {}): Promise `
  • ${escapeHtml(line)}
    • `).join("\n")}
    - ${form("continue", `Continue to ${provider}`)} - ${form("decline", "No, return to the app")} + ${form("continue", consentButtons(ask).go)} + ${form("decline", consentButtons(ask).no)}
    ` ); } export interface GrantView { + consentText: string; id: string; appSlug: AppSlug; provider: Provider; @@ -135,6 +155,7 @@ export function grantViews(receipts: readonly StoredConsentReceipt[]): GrantView scopesGranted: receipt.scopesGranted, grantedAt: receipt.createdAt, consentPageDigest: receipt.consentPageDigest, + consentText: receipt.consentText, status: revocation ? "revoked" : "active", ...(revocation ? { revokedAt: revocation.createdAt, revocationReceiptId: revocation.id } : {}), }); diff --git a/src/store/in-memory.ts b/src/store/in-memory.ts index c31b098..cfc869e 100644 --- a/src/store/in-memory.ts +++ b/src/store/in-memory.ts @@ -486,10 +486,14 @@ export class InMemoryStore implements HeimdallStore { this.auditEvents.set(event.id ?? randomUUID(), event); } - async createConsentReceipt(input: CreateConsentReceiptInput): Promise { + async createConsentReceipt(input: CreateConsentReceiptInput): Promise<{ receipt: StoredConsentReceipt; created: boolean }> { + if (input.kind === "revocation") { + const existing = this.consentReceipts.find((r) => r.kind === "revocation" && r.revokesReceiptId === input.revokesReceiptId); + if (existing) return { receipt: clone(existing), created: false }; + } const receipt = { id: randomUUID(), ...clone(input) } as StoredConsentReceipt; this.consentReceipts.push(clone(receipt)); - return clone(receipt); + return { receipt: clone(receipt), created: true }; } async listConsentReceipts(accountId: string): Promise { diff --git a/src/store/postgres.ts b/src/store/postgres.ts index 45240a5..3d34047 100644 --- a/src/store/postgres.ts +++ b/src/store/postgres.ts @@ -260,6 +260,7 @@ interface ConsentReceiptRow { scopes_asked: string | null; scopes_granted: string | null; consent_page_digest: string | null; + consent_text: string | null; revokes_receipt_id: string | null; } @@ -280,6 +281,7 @@ function mapConsentReceiptRow(row: ConsentReceiptRow): StoredConsentReceipt { scopesAsked: parseScopes(row.scopes_asked ?? ""), scopesGranted: parseScopes(row.scopes_granted ?? ""), consentPageDigest: row.consent_page_digest ?? "", + consentText: row.consent_text ?? "", }; } @@ -1232,15 +1234,18 @@ export class PostgresStore implements HeimdallStore { ); } - async createConsentReceipt(input: CreateConsentReceiptInput): Promise { + async createConsentReceipt(input: CreateConsentReceiptInput): Promise<{ receipt: StoredConsentReceipt; created: boolean }> { const grant = input.kind === "grant" ? input : null; + // No conflict target and the partial unique index decides: a second + // revocation of one grant inserts nothing and the first is read back. const result = await this.pool.query( ` INSERT INTO consent_receipts ( id, kind, account_id, app_slug, provider, created_at, - scopes_asked, scopes_granted, consent_page_digest, revokes_receipt_id + scopes_asked, scopes_granted, consent_page_digest, consent_text, revokes_receipt_id ) - VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11) + ON CONFLICT DO NOTHING RETURNING * `, [ @@ -1253,10 +1258,18 @@ export class PostgresStore implements HeimdallStore { grant ? grant.scopesAsked.join(" ") : null, grant ? grant.scopesGranted.join(" ") : null, grant ? grant.consentPageDigest : null, + grant ? grant.consentText : null, input.kind === "revocation" ? input.revokesReceiptId : null, ] ); - return mapConsentReceiptRow(expectRow(result.rows[0], "createConsentReceipt")); + if (result.rowCount) { + return { receipt: mapConsentReceiptRow(expectRow(result.rows[0], "createConsentReceipt")), created: true }; + } + const existing = await this.pool.query( + `SELECT * FROM consent_receipts WHERE revokes_receipt_id = $1`, + [input.kind === "revocation" ? input.revokesReceiptId : null] + ); + return { receipt: mapConsentReceiptRow(expectRow(existing.rows[0], "createConsentReceipt")), created: false }; } async listConsentReceipts(accountId: string): Promise { diff --git a/src/store/schema.ts b/src/store/schema.ts index 9c87dff..71a4a43 100644 --- a/src/store/schema.ts +++ b/src/store/schema.ts @@ -192,9 +192,15 @@ CREATE TABLE IF NOT EXISTS consent_receipts ( scopes_asked TEXT, scopes_granted TEXT, consent_page_digest TEXT, + consent_text TEXT, revokes_receipt_id TEXT ); +-- A grant is taken back once: the second revocation naming it conflicts. +CREATE UNIQUE INDEX IF NOT EXISTS consent_receipts_revokes_unique_idx + ON consent_receipts(revokes_receipt_id) + WHERE revokes_receipt_id IS NOT NULL; + CREATE INDEX IF NOT EXISTS consent_receipts_account_idx ON consent_receipts(account_id, created_at); diff --git a/src/store/types.ts b/src/store/types.ts index 4c4ce17..bb2e080 100644 --- a/src/store/types.ts +++ b/src/store/types.ts @@ -257,7 +257,9 @@ export type CreatePrivateCommandReceiptInput = StoredPrivateCommandReceipt; * edited, and a revocation is a second row naming it, so a grant is live * exactly when no revocation names it. `scopesAsked` is what Heimdall sent the * provider, `scopesGranted` what the provider returned, `consentPageDigest` - * the hash of the wording the person was shown. + * the hash of the wording the person was shown and `consentText` that wording + * itself. A grant is revoked at most once: a second revocation naming the same + * grant is not written, and the store answers with the first. */ export type StoredConsentReceipt = { id: string; @@ -266,7 +268,7 @@ export type StoredConsentReceipt = { provider: Provider; createdAt: string; } & ( - | { kind: "grant"; scopesAsked: string[]; scopesGranted: string[]; consentPageDigest: string } + | { kind: "grant"; scopesAsked: string[]; scopesGranted: string[]; consentPageDigest: string; consentText: string } | { kind: "revocation"; revokesReceiptId: string } ); @@ -350,7 +352,8 @@ export interface HeimdallStore { */ claimOAuthStateUse(input: ClaimOAuthStateUseInput): Promise; createAuditEvent(input: CreateAuditEventInput): Promise; - createConsentReceipt(input: CreateConsentReceiptInput): Promise; + /** `created` is false when a revocation for that grant already existed and is what comes back. */ + createConsentReceipt(input: CreateConsentReceiptInput): Promise<{ receipt: StoredConsentReceipt; created: boolean }>; /** Every receipt the account holds, oldest first. */ listConsentReceipts(accountId: string): Promise; } diff --git a/tests/consent-drift.test.ts b/tests/consent-drift.test.ts new file mode 100644 index 0000000..8671d4b --- /dev/null +++ b/tests/consent-drift.test.ts @@ -0,0 +1,65 @@ +import { afterEach, expect, it, vi } from "vitest"; +import { type FastifyInstance } from "fastify"; + +// The receipt records the wording the person was shown. If the wording changes +// between the ask and the callback (a deploy inside the state's lifetime), the +// receipt must still hold what they saw. + +const drift = vi.hoisted(() => ({ on: false })); +vi.mock("../src/consent.js", async (importOriginal) => { + const real = await importOriginal(); + return { ...real, consentText: (ask: Parameters[0]) => real.consentText(ask) + (drift.on ? "\nREWORDED AFTER THE ASK" : "") }; +}); + +import { buildApp } from "../src/app.js"; +import { InMemoryStore } from "../src/store/index.js"; +import { createTestConfig, setCookieOf, stateOf } from "./oauth-flow.js"; + +const apps: FastifyInstance[] = []; +afterEach(async () => { + drift.on = false; + while (apps.length) await apps.pop()?.close(); +}); + +it("a receipt holds the wording of the ask, not wording rebuilt at the callback", async () => { + const store = new InMemoryStore(); + const app = await buildApp({ + config: createTestConfig(), + store, + oauthRuntimes: { + joystick: { + async exchangeAuthorizationCode() { + return { accessToken: "joy-access-code-a", refreshToken: "r", tokenType: "Bearer", scope: ["identity:read"], expiresAt: "2099-01-01T00:00:00.000Z", raw: {} }; + }, + async resolveIdentity() { + return { provider: "joystick", providerUserId: "joy-a", username: "a", profile: {} }; + }, + async evaluateEntitlements() { + return { facts: [], snapshots: [] }; + }, + }, + }, + }); + apps.push(app); + const start = await app.inject({ + method: "POST", + url: "/v1/oauth/joystick/start", + headers: { "x-heimdall-app-secret": "streampixels-secret" }, + payload: { appSlug: "streampixels", mode: "connect", connection: { kind: "creator", targetId: "c" }, returnTo: "https://streampixels.gamecult.org/s", handoff: { kind: "browser_completion" } }, + }); + const state = stateOf(start.json()); + const ask = await app.inject({ method: "GET", url: `/v1/oauth/joystick/launch?state=${encodeURIComponent(state)}`, headers: { accept: "text/html" } }); + const askCookie = setCookieOf(ask).find((c) => c.startsWith("__Host-heimdall-ask-"))!.split(";")[0]!; + const go = await app.inject({ + method: "POST", + url: "/v1/oauth/joystick/consent", + headers: { "content-type": "application/x-www-form-urlencoded", cookie: askCookie }, + payload: new URLSearchParams({ state, decision: "continue" }).toString(), + }); + const binding = setCookieOf(go).find((c) => c.startsWith("__Host-heimdall-oauth-"))!.split(";")[0]!; + drift.on = true; // the wording changes after the person has agreed + const done = await app.inject({ method: "GET", url: `/v1/oauth/joystick/callback?state=${encodeURIComponent(state)}&code=code-a`, headers: { cookie: binding, accept: "application/json" } }); + expect(done.statusCode).toBe(201); + const [receipt] = await store.listConsentReceipts(done.json().account.id); + expect(receipt?.kind === "grant" && receipt.consentText).not.toContain("REWORDED"); +}); diff --git a/tests/joystick-consent.test.ts b/tests/joystick-consent.test.ts index f674f12..963f1e5 100644 --- a/tests/joystick-consent.test.ts +++ b/tests/joystick-consent.test.ts @@ -1,8 +1,9 @@ import { createHash } from "node:crypto"; import { afterEach, describe, expect, it, vi } from "vitest"; import { type FastifyInstance } from "fastify"; -import { buildApp } from "../src/app.js"; -import { consentDigest, consentLines, type ConsentAsk } from "../src/consent.js"; +import { buildApp, getHeimdallRuntimeContext } from "../src/app.js"; +import { consentDigest, consentText, digestOfText, type ConsentAsk } from "../src/consent.js"; +import { decodeJwt, signJwt } from "../src/signing.js"; import { type OAuthProviderRuntime } from "../src/oauth.js"; import { providerCatalog } from "../src/providers.js"; import { InMemoryStore } from "../src/store/index.js"; @@ -64,14 +65,17 @@ async function startConnect(app: FastifyInstance, handoff: Record `/v1/oauth/joystick/launch?state=${encodeURIComponent(state)}`; -const cookieOf = (response: { headers: Record }) => - setCookieOf(response).find((value) => value.startsWith("__Host-heimdall-oauth-"))!.split(";")[0]!; +const cookieNamed = (prefix: string) => (response: { headers: Record }) => + setCookieOf(response).find((value) => value.startsWith(prefix))?.split(";")[0]; +const askCookieOf = cookieNamed("__Host-heimdall-ask-"); +const bindingCookieOf = cookieNamed("__Host-heimdall-oauth-"); -/** The browser is shown the ask. */ +/** The browser is shown the ask. The cookie it keeps is the ask's, which binds nothing. */ async function seeAsk(app: FastifyInstance, state: string) { const response = await app.inject({ method: "GET", url: launchUrl(state), headers: { accept: "text/html" } }); expect(response.statusCode).toBe(200); - return { page: response, cookie: cookieOf(response) }; + expect(bindingCookieOf(response), "the ask must not set the launch binding").toBeUndefined(); + return { page: response, cookie: askCookieOf(response)! }; } function answer(app: FastifyInstance, state: string, decision: "continue" | "decline", cookie?: string, accept = "text/html") { @@ -94,10 +98,10 @@ function callback(app: FastifyInstance, state: string, cookie: string, query: st /** Ask, accept, and complete as Joystick user `user`; returns the callback's JSON. */ async function grantAs(app: FastifyInstance, user: string) { const state = await startConnect(app); - const { cookie } = await seeAsk(app, state); - const go = await answer(app, state, "continue", cookie); + const asked = await seeAsk(app, state); + const go = await answer(app, state, "continue", asked.cookie); expect(go.statusCode).toBe(302); - const done = await callback(app, state, cookie, `code=code-${user}`); + const done = await callback(app, state, bindingCookieOf(go)!, `code=code-${user}`); expect(done.statusCode).toBe(201); return done.json() as { accessToken: string; consentReceiptId: string; account: { id: string }; session: { accountId: string } }; } @@ -167,6 +171,44 @@ describe("the ask", () => { expect((await answer(app, state, "continue", cookie)).statusCode).toBe(302); }); + it("a stranger who only opens the launch URL is not bound to the state and cannot complete it", async () => { + const { app } = await newApp(); + const state = await startConnect(app); + const victim = await seeAsk(app, state); + await answer(app, state, "continue", victim.cookie); + // A fresh browser that holds the URL loads the ask and calls back with its own provider code. + const stranger = await seeAsk(app, state); + for (const cookie of [stranger.cookie, ""]) { + const done = await callback(app, state, cookie, "code=code-attacker"); + expect(done.statusCode).toBe(400); + expect(done.json()).toEqual({ error: "state_not_bound" }); + } + }); + + it("only Continue sets the launch binding, and the ask's cookie is no binding: it cannot complete a callback", async () => { + const { app } = await newApp(); + const state = await startConnect(app); + const asked = await seeAsk(app, state); + const done = await callback(app, state, asked.cookie, "code=code-a"); + expect(done.json()).toEqual({ error: "state_not_bound" }); + const go = await answer(app, state, "continue", asked.cookie); + expect(bindingCookieOf(go)).toBeDefined(); + expect(bindingCookieOf(go)!.length).toBeLessThan(3500); + }); + + it("a managed connection that never passed through the ask completes nothing and leaves no receipt", async () => { + const { app, store } = await newApp(); + const ctx = getHeimdallRuntimeContext(app); + const state = await startConnect(app); + const { jti, exp } = decodeJwt(state).payload as { jti: string; exp: number }; + // A binding that names this state but carries no wording: what any launch outside the ask would leave. + const bare = signJwt({ typ: "heimdall_oauth_binding", iss: ctx.config.issuer, jti, iat: exp - 600, exp }, ctx.keys); + const done = await callback(app, state, `__Host-heimdall-oauth-${jti}=${bare}`, "code=code-a"); + expect(done.statusCode).not.toBe(201); + expect(done.json()).not.toHaveProperty("consentReceiptId"); + expect(await store.listConsentReceipts((await store.findStoredLinkedIdentity("joystick", "joy-a"))?.accountId ?? "none")).toEqual([]); + }); + it("the consent route refuses a flow that has no ask", async () => { const { app } = await newApp(); const start = await app.inject({ @@ -189,8 +231,8 @@ describe("the ask", () => { describe("the wording digest", () => { const ask: ConsentAsk = { appSlug: "streampixels", provider: "joystick", scopes: ["identity:read"], returnOrigin: "https://streampixels.gamecult.org" }; - it("is the sha-256 of the lines shown, and moves with the app, provider, scopes and origin", () => { - expect(consentDigest(ask)).toBe(createHash("sha256").update(consentLines(ask).join("\n"), "utf8").digest("hex")); + it("is the sha-256 of the text shown, and moves with the app, provider, scopes and origin", () => { + expect(consentDigest(ask)).toBe(createHash("sha256").update(consentText(ask), "utf8").digest("hex")); const variants: ConsentAsk[] = [ { ...ask, appSlug: "bifrost" }, { ...ask, provider: "twitch" }, @@ -212,7 +254,7 @@ describe("refusal", () => { // Nothing launches or completes afterwards, and nothing was kept. expect((await answer(app, state, "continue", cookie, "application/json")).json()).toEqual({ error: "state_already_launched" }); - expect((await callback(app, state, cookie, "code=code-a")).json()).toEqual({ error: "state_already_used" }); + expect((await callback(app, state, cookie, "code=code-a")).json()).toEqual({ error: "state_not_bound" }); expect(await store.findStoredLinkedIdentity("joystick", "joy-a")).toBeNull(); const events = [...(store as unknown as { auditEvents: Map }).auditEvents.values()]; expect(events.filter((event) => event.eventType === "consent_refused").map((event) => event.eventPayloadJson)).toEqual([ @@ -259,8 +301,8 @@ describe("refusal", () => { await answer(app, byViewer, "decline", cookie); const byProvider = await startConnect(app, handoff); const second = await seeAsk(app, byProvider); - await answer(app, byProvider, "continue", second.cookie); - await callback(app, byProvider, second.cookie, "error=access_denied"); + const proceeded = await answer(app, byProvider, "continue", second.cookie); + await callback(app, byProvider, bindingCookieOf(proceeded)!, "error=access_denied"); expect(delivered.map((payload) => [payload.status, payload.error, payload.by])).toEqual([ ["error", "consent_refused", "viewer"], @@ -272,8 +314,9 @@ describe("refusal", () => { async function newFlow(app: FastifyInstance) { const state = await startConnect(app); const { cookie } = await seeAsk(app, state); - expect((await answer(app, state, "continue", cookie)).statusCode).toBe(302); - return { state, cookie }; + const go = await answer(app, state, "continue", cookie); + expect(go.statusCode).toBe(302); + return { state, cookie: bindingCookieOf(go)! }; } describe("the receipt", () => { @@ -295,6 +338,21 @@ describe("the receipt", () => { }); }); + it("the receipt holds the wording the person was shown, and its digest is that wording's hash", async () => { + const { app, store } = await newApp(); + const state = await startConnect(app); + const asked = await seeAsk(app, state); + const go = await answer(app, state, "continue", asked.cookie); + const done = await callback(app, state, bindingCookieOf(go)!, "code=code-a"); + const [receipt] = await store.listConsentReceipts(done.json().account.id); + if (receipt?.kind !== "grant") throw new Error("no grant"); + expect(receipt.consentPageDigest).toBe(digestOfText(receipt.consentText)); + const shown = asked.page.body.replace(/'/g, "'").replace(/"/g, '"').replace(/>/g, ">").replace(/</g, "<").replace(/&/g, "&"); + for (const line of receipt.consentText.split("\n")) expect(shown, line).toContain(line); + expect(receipt.consentText).toContain("Continue to Joystick"); + expect(receipt.consentText).toContain("No, return to the app"); + }); + it("a refusal leaves no receipt", async () => { const { app, store } = await newApp(); const state = await startConnect(app); @@ -308,8 +366,8 @@ describe("the receipt", () => { const { app } = await newApp(); const state = await startConnect(app); const { cookie } = await seeAsk(app, state); - await answer(app, state, "continue", cookie); - const page = await callback(app, state, cookie, "code=code-a", "text/html"); + const go = await answer(app, state, "continue", cookie); + const page = await callback(app, state, bindingCookieOf(go)!, "code=code-a", "text/html"); expect(page.statusCode).toBe(200); expect(page.body).toMatch(/receipt [0-9a-f-]{36}/); }); @@ -424,6 +482,43 @@ describe("my grants", () => { expect((await resolveCredential(app, a.account.id)).statusCode).toBe(410); }); + it("a page link is never renewed by being used: the forms on its page expire when the link does", async () => { + const { app } = await newApp(); + const a = await grantAs(app, "a"); + const link = await app.inject({ method: "POST", url: "/v1/me/grants/link", headers: bearer(a.accessToken) }); + const url = new URL(link.json().url as string); + const linkToken = url.searchParams.get("t")!; + const page = await app.inject({ method: "GET", url: `${url.pathname}${url.search}`, headers: { accept: "text/html" } }); + const formToken = /name="t" value="([^"]+)"/.exec(page.body)![1]!; + expect(decodeJwt(formToken).payload.exp).toBe(decodeJwt(linkToken).payload.exp); + // The same holds one hop on: the page a form post answers with. + const post = await app.inject({ + method: "POST", + url: "/v1/me/grants/nope/revoke", + headers: { ...form, accept: "text/html" }, + payload: new URLSearchParams({ t: formToken }).toString(), + }); + expect(post.statusCode).toBe(404); + // A bearer, by contrast, opens a fresh window. + const fresh = await app.inject({ method: "GET", url: "/v1/me/grants", headers: { ...bearer(a.accessToken), accept: "text/html" } }); + const freshToken = /name="t" value="([^"]+)"/.exec(fresh.body)![1]!; + expect(decodeJwt(freshToken).payload.exp).toBeGreaterThanOrEqual(decodeJwt(linkToken).payload.exp as number); + }); + + it("two revokes at once take the grant back once: one revocation row, one audit event, one reporter", async () => { + const { app, store } = await newApp(); + const a = await grantAs(app, "a"); + const url = `/v1/me/grants/${a.consentReceiptId}/revoke`; + const [one, two] = await Promise.all([ + app.inject({ method: "POST", url, headers: bearer(a.accessToken) }), + app.inject({ method: "POST", url, headers: bearer(a.accessToken) }), + ]); + expect([one.json().revoked.length, two.json().revoked.length].sort()).toEqual([0, 1]); + expect((await store.listConsentReceipts(a.account.id)).filter((r) => r.kind === "revocation")).toHaveLength(1); + const events = [...(store as unknown as { auditEvents: Map }).auditEvents.values()]; + expect(events.filter((event) => event.eventType === "consent_revoked")).toHaveLength(1); + }); + it("a page link proves only what it was minted for: an access token, a foreign token or none opens nothing", async () => { const { app } = await newApp(); const a = await grantAs(app, "a"); diff --git a/tests/oauth-flow.ts b/tests/oauth-flow.ts index b85b2ce..2616da1 100644 --- a/tests/oauth-flow.ts +++ b/tests/oauth-flow.ts @@ -120,8 +120,8 @@ async function followLaunch(app: FastifyInstance, state: string): Promise value.startsWith("__Host-heimdall-oauth-")); - expect(asked, "the ask sets the binding cookie").toBeDefined(); + const asked = setCookieOf(response).find((value) => value.startsWith("__Host-heimdall-ask-")); + expect(asked, "the ask sets its own cookie").toBeDefined(); response = await app.inject({ method: "POST", url: `/v1/oauth/${provider}/consent`, diff --git a/tests/oauth-state-binding.test.ts b/tests/oauth-state-binding.test.ts index 1bffc19..01abba0 100644 --- a/tests/oauth-state-binding.test.ts +++ b/tests/oauth-state-binding.test.ts @@ -312,6 +312,38 @@ describe("OAuth state binding", () => { expect(replay.json()).toEqual({ error: "state_already_used" }); }); + // The private command plane's completeAuth answers a denied attempt with + // attempt.denialCode as its error (private-command-plane.ts, completeAuth), so + // these two codes are what Bifrost, Repixelizer and Ghostlight poll for. + it.each([ + ["bifrost", "https://bifrost.gamecult.org/auth/heimdall/complete?attempt=a1", "access_denied", "consent_refused"], + ["repixelizer", "https://repixelizer.gamecult.org/app/", "access_denied", "consent_refused"], + ["repixelizer", "https://repixelizer.gamecult.org/app/", "server_error", "provider_error"], + ] as const)("a %s attempt whose provider answers %s is denied with %s", async (appSlug, returnTo, providerError, denialCode) => { + const app = await newApp(); + const ctx = getHeimdallRuntimeContext(app); + const attempt = await ctx.store.createAuthAttempt({ + handle: `attempt-${providerError}`, + appSlug, + provider: "discord", + mode: "sign_in", + returnTo, + createdAt: new Date().toISOString(), + expiresAt: new Date(Date.now() + 600_000).toISOString(), + }); + const caller = resolveAppCaller(ctx.config, appSlug, { "x-heimdall-app-secret": `${appSlug}-secret` }); + const started = await startOAuthFlow( + ctx, + "discord", + caller, + { appSlug, mode: "sign_in", returnTo, handoff: { kind: "browser_completion" } }, + { trustedBrowserAttemptId: attempt.handle } + ); + const flow = await launchFlow(app, started.body as unknown as { authorizationUrl: string }); + await callback(app, flow.state, flow.cookie, `error=${providerError}`); + expect(await ctx.store.findAuthAttempt(appSlug, attempt.handle)).toMatchObject({ status: "denied", denialCode }); + }); + describe("a state that is expired, foreign or tampered is refused at launch and at callback", () => { it.each([ ["expired beyond the skew", { exp: Math.floor(Date.now() / 1000) - 100 }, "state_expired"], diff --git a/tests/store.test.ts b/tests/store.test.ts index 684aa41..ddec729 100644 --- a/tests/store.test.ts +++ b/tests/store.test.ts @@ -198,6 +198,19 @@ describe("PostgresStore", () => { }); }); +describe("InMemoryStore.createConsentReceipt", () => { + it("writes one revocation per grant: the second names the first and is not created", async () => { + const store = new InMemoryStore(); + const base = { accountId: "a", appSlug: "streampixels" as const, provider: "joystick" as const, createdAt: "2026-11-08T00:00:00.000Z" }; + const { receipt: grant } = await store.createConsentReceipt({ ...base, kind: "grant", scopesAsked: [], scopesGranted: [], consentPageDigest: "d", consentText: "t" }); + const first = await store.createConsentReceipt({ ...base, kind: "revocation", revokesReceiptId: grant.id }); + const second = await store.createConsentReceipt({ ...base, createdAt: "2026-11-09T00:00:00.000Z", kind: "revocation", revokesReceiptId: grant.id }); + expect([first.created, second.created]).toEqual([true, false]); + expect(second.receipt).toEqual(first.receipt); + expect(await store.listConsentReceipts("a")).toHaveLength(2); + }); +}); + describe("InMemoryStore.claimOAuthStateUse", () => { const use = { jti: "state-1", @@ -273,7 +286,7 @@ describe("PostgresStore.checkSchema", () => { "consent_receipts", "audit_events", ]); - expect(REQUIRED_COLUMNS).toHaveLength(111); + expect(REQUIRED_COLUMNS).toHaveLength(112); expect(REQUIRED_COLUMNS).toEqual( expect.arrayContaining([ { table: "auth_completions", column: "code", type: "TEXT", nullable: false }, @@ -284,12 +297,13 @@ describe("PostgresStore.checkSchema", () => { { table: "auth_completions", column: "attempt_id", type: "TEXT", nullable: true }, ]) ); - expect(REQUIRED_KEYS).toHaveLength(15); + expect(REQUIRED_KEYS).toHaveLength(16); expect(REQUIRED_KEYS).toEqual( expect.arrayContaining([ { table: "linked_identities", columns: "provider,provider_user_id", predicate: "" }, { table: "private_command_receipts", columns: "app_slug,idempotency_key", predicate: "" }, { table: "oauth_state_uses", columns: "jti,phase", predicate: "" }, + { table: "consent_receipts", columns: "revokes_receipt_id", predicate: "revokes_receipt_id IS NOT NULL" }, { table: "entitlement_snapshots", columns: "account_id,provider,scope", predicate: "" }, { table: "auth_completions", columns: "app_slug,attempt_id", predicate: "attempt_id IS NOT NULL AND consumed_at IS NULL" }, ]) @@ -419,7 +433,7 @@ async function exerciseStore(store: PostgresStore): Promise> { expect(await s.claimOAuthStateUse(stateUse)).toBe(true); expect(await s.claimOAuthStateUse(stateUse)).toBe(false); expect(await s.claimOAuthStateUse({ ...stateUse, phase: "callback" })).toBe(true); - const grant = await s.createConsentReceipt({ kind: "grant", accountId: account.id, appSlug: "streampixels", provider: "joystick", createdAt: now, scopesAsked: ["a", "b"], scopesGranted: ["a"], consentPageDigest: "d" }); + const { receipt: grant } = await s.createConsentReceipt({ kind: "grant", accountId: account.id, appSlug: "streampixels", provider: "joystick", createdAt: now, scopesAsked: ["a", "b"], scopesGranted: ["a"], consentPageDigest: "d", consentText: "t" }); await s.createConsentReceipt({ kind: "revocation", accountId: account.id, appSlug: "streampixels", provider: "joystick", createdAt: later, revokesReceiptId: grant.id }); expect((await s.listConsentReceipts(account.id)).map((r) => r.kind)).toEqual(["grant", "revocation"]); await s.createAuditEvent({ eventType: "probe", eventPayloadJson: {}, createdAt: now });