diff --git a/docs/architecture.md b/docs/architecture.md index f816a14..12fbd4f 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -447,13 +447,12 @@ builder was not used. process, so N Heimdall processes can present one token N times after a transient failure. Needs a failed-attempt marker on the row, read under the lock. Must land before or with Idunn onboarding. -- `joystick:follow_up:connect-start-needs-app-auth`: connect starts are - unauthenticated, so anyone can name `{creator, targetId}`; the callback - re-check proves a binding exists, not that the caller owns it. It also owns - the whitespace-only `targetId` that passes both gates and the callback's - fail-open `''` fallback for a vanished profile (finding - `joystick:finding:cut-h1.s4.whitespace-targetid-passes-custody-gate`). Must - land before the public Joystick bot or a second creator. +- `joystick:follow_up:connect-start-needs-app-auth` (Heimdall side done): a + connect start that names a connection target needs the app's own secret for + every managed provider, and a blank target is refused at start and at the + callback mint. What StreamPixels must do on its side is in + `notes/streampixels-connect-redeem.md`; its redeem must refuse a creator + connect whose target it did not start. - `joystick:follow_up:heimdall-idunn-onboarding` (gamecult-ops): live Heimdall is a hand-copied release under `heimdall.service`, not an Idunn-admitted generation; the recipe's Joystick lines are declared but unproven by a diff --git a/notes/streampixels-connect-redeem.md b/notes/streampixels-connect-redeem.md new file mode 100644 index 0000000..83f57a7 --- /dev/null +++ b/notes/streampixels-connect-redeem.md @@ -0,0 +1,35 @@ +# For Kiko: what StreamPixels must do for a creator connect + +Heimdall's side is done (this PR). A connect start that carries a `connection` +target is accepted only with StreamPixels' own `x-heimdall-app-secret`, so only +the StreamPixels backend can mint a state naming `{kind: "creator", targetId}`. +Heimdall cannot see StreamPixels' session; the secret proves the caller is the +backend, and the backend must have proved the creator-manage session itself +before it starts the connect. + +What StreamPixels has to change (not done here; this repo does not touch it): + +1. **Start the connect only after the creator-manage check.** The route that + calls `POST /v1/oauth/:provider/start` with `mode: "connect"` must have + verified, for the signed-in StreamPixels user, that they may manage creator + `targetId`. Never take `targetId` from the browser unchecked. +2. **Remember what you started.** Store, server-side and bound to the attempt + (the `attemptId` or browser session), the `targetId` StreamPixels asked + Heimdall to bind. +3. **Redeem with that stored target, never one read back from the request or + the completion.** `POST /v1/apps/streampixels/auth/redeem` takes `targetId`; + Heimdall answers `403 completion_target_mismatch` (code left unconsumed) when + it differs from the signed state's. That only protects you if the value you + send is the one you stored at step 2. A redeem with no stored target for this + attempt must be refused by StreamPixels before it calls Heimdall. +4. **Refuse a creator connect you did not start.** If a connect completion + arrives for a `targetId` with no matching started attempt, do not rebind that + creator's connector, whatever Heimdall's completion says. +5. **A connect completion with no creator target is not a creator connect and + not a sign-in.** Joystick is managed-connection only: a non-creator Joystick + completion is an error. (Twitch/YouTube `connect` without a `connection` is + Heimdall's viewer-session path; treat it as that, never as a connector rebind.) +6. Blank targets (`" "`, zero-width characters) are refused by Heimdall at + start with `400 invalid_connection_target`; send real ids only. + +Tracked as `joystick:follow_up:connect-start-needs-app-auth`. diff --git a/src/app-profiles.ts b/src/app-profiles.ts index c10f704..c25dbb2 100644 --- a/src/app-profiles.ts +++ b/src/app-profiles.ts @@ -263,6 +263,17 @@ export function isAllowedReturnOrigin(profile: AppProfile, returnTo: string): bo } } +/** + * A connection target names something: at least one letter or number that is + * not a default-ignorable code point (the Hangul and Khmer fillers are letters + * that render as nothing). An allowlist, so marks, private-use, unassigned and + * surrogate code points and symbols cannot stand in for a name. The one + * definition the start gate and the callback mint both use. + */ +export function isUsableConnectionTarget(targetId: string): boolean { + return /(?![\p{Default_Ignorable_Code_Point}])[\p{L}\p{N}]/u.test(targetId); +} + /** * What a completed OAuth callback may mint, decided at the callback from the * signed state's mode and the provider's role in the profile (the start gate @@ -289,7 +300,7 @@ export function callbackMint( return mode === "connect" && profile.managedConnectionProviders.includes(provider) && connection?.kind === "creator" && - connection.targetId !== "" + isUsableConnectionTarget(connection.targetId) ? "custody_only" : "refused"; } diff --git a/src/app.ts b/src/app.ts index 2051499..e2ba003 100644 --- a/src/app.ts +++ b/src/app.ts @@ -25,7 +25,7 @@ import { type Provider, } from "./contracts.js"; import { issueAccessClaim, type IssueAccessClaimInput } from "./claims.js"; -import { builtInAppProfiles, getAppProfile, isAllowedReturnOrigin, serializeAppProfile, admitsProviderForMode, callbackMint } from "./app-profiles.js"; +import { builtInAppProfiles, getAppProfile, isAllowedReturnOrigin, serializeAppProfile, admitsProviderForMode, callbackMint, isUsableConnectionTarget } from "./app-profiles.js"; import { renderBrowserHandoffPage } from "./browser-handoff.js"; import { consentText, digestOfText, grantViews, GRANTS_PATH, renderConsentPage, renderGrantsPage, type ConsentAsk } from "./consent.js"; import { type HeimdallConfig, loadConfig } from "./config.js"; @@ -702,16 +702,6 @@ export async function startOAuthFlow( }; } - if (callbackMint(profile, provider, input.mode, input.connection) === "refused") { - return { - statusCode: 400, - body: { - error: "creator_connection_required", - detail: `${provider} on ${profile.slug} is a managed creator connection: connect needs connection.kind creator and a targetId.`, - }, - }; - } - const descriptor = providerCatalog[provider]; // A provider that never mints an account attaches an identity only through // a bound link: the user's own access token, since the app's word does not @@ -811,6 +801,24 @@ export async function startOAuthFlow( return { statusCode: 401, body: { error: "app_auth_required" } }; } + // After the app-secret gate, so a caller without the secret learns nothing about what a target must look like. + if (input.connection !== undefined && !isUsableConnectionTarget(input.connection.targetId)) { + return { + statusCode: 400, + body: { error: "invalid_connection_target", detail: "connection.targetId must name a target." }, + }; + } + + if (callbackMint(profile, provider, input.mode, input.connection) === "refused") { + return { + statusCode: 400, + body: { + error: "creator_connection_required", + detail: `${provider} on ${profile.slug} is a managed creator connection: connect needs connection.kind creator and a targetId.`, + }, + }; + } + const providerConfig = config.providers[provider]; if (!providerConfig.clientId) { return { diff --git a/tests/app.test.ts b/tests/app.test.ts index 67d4183..20a0191 100644 --- a/tests/app.test.ts +++ b/tests/app.test.ts @@ -1440,6 +1440,105 @@ describe("Heimdall service", () => { expect(urls).toEqual([]); }); + // Targets made only of whitespace, format characters, controls and the invisible fillers that render as nothing. + const BLANK_TARGETS = [ + " ", "\t", "\n ", "\u200B", "\u00A0\uFEFF", + "\u2800", "\u3164", "\u115F", "\u1160", "\uFFA0", "\u034F", "\u0000", "\u0007", " \u2800\u3164 ", + // Marks, private-use, unassigned and lone-surrogate code points, and the symbols that read as nothing: none is a letter or a number. + "\u0300", "\uE000", "\u0378", "\uD800", "\u{16FE4}", "\u{1D159}", "\u{F0000}", "\u0300\uE000 ", + ]; + // A target is usable when it holds a letter or a number that is not an invisible filler. + const NAMING_TARGETS = ["creator-1", " creator-1 ", "Framgång", "42", "\u2800a\u3164", "\u3164\u0300x"]; + + describe("a connect start that names a connection target is the app backend's", () => { + // The target is app-backend authority: the StreamPixels backend proves the creator-manage session and states the + // creator it started the connect for. Anyone else naming {creator, X} would have X's connector rebound to their + // own OAuth account at StreamPixels' redeem. + const returnTo = "https://streampixels.gamecult.org/auth/connect"; + const managed = ["twitch", "youtube", "joystick"] as const; + const startFor = (app: FastifyInstance, provider: string, connection: unknown, headers: Record = {}) => + app.inject({ + method: "POST", + url: `/v1/oauth/${provider}/start`, + headers, + payload: { appSlug: "streampixels", mode: "connect", returnTo, connection }, + }); + const own = { "x-heimdall-app-secret": "streampixels-secret" }; + + it.each(managed)("%s: a creator target is refused without the app's own secret and minted for it", async (provider) => { + const config = createTestConfig(); + const app = await buildApp({ config }); + apps.push(app); + const creator = { kind: "creator", targetId: "victim-creator" }; + + for (const headers of [{}, { "x-heimdall-app-secret": "repixelizer-secret" }, { "x-heimdall-app-secret": "wrong" }]) { + const refused = await startFor(app, provider, creator, headers); + expect([refused.statusCode, refused.json()]).toEqual([401, { error: "app_auth_required" }]); + expect(refused.body).not.toContain("authorizationUrl"); + } + const started = await startFor(app, provider, creator, own); + expect(started.statusCode).toBe(201); + const verified = verifyJwt(stateOf(started.json()), getPublicKey(app)); + expect(verified.valid && verified.payload).toEqual(expect.objectContaining({ connection: expect.objectContaining(creator) })); + + // A direct caller (the command plane hands startOAuthFlow its own) is held to the slug too. + const input = { appSlug: "streampixels" as const, mode: "connect" as const, returnTo, connection: { kind: "creator" as const, targetId: "victim-creator" } }; + const foreign = resolveAppCaller(config, "repixelizer", { "x-heimdall-app-secret": "repixelizer-secret" }); + expect((await startOAuthFlow(getHeimdallRuntimeContext(app), provider, foreign, input)).statusCode).toBe(401); + expect((await startOAuthFlow(getHeimdallRuntimeContext(app), provider, null, input)).statusCode).toBe(401); + }); + + it.each(["twitch", "youtube"] as const)("%s: any connection kind needs the app's secret, not only creator", async (provider) => { + const app = await buildApp({ config: createTestConfig() }); + apps.push(app); + const account = { kind: "account", targetId: "victim-account" }; + + const refused = await startFor(app, provider, account); + expect([refused.statusCode, refused.json()]).toEqual([401, { error: "app_auth_required" }]); + expect((await startFor(app, provider, account, own)).statusCode).toBe(201); + }); + + it.each(managed)("%s: a blank target is refused even for the app, whatever the blank is made of", async (provider) => { + const app = await buildApp({ config: createTestConfig() }); + apps.push(app); + + for (const kind of ["creator", "account"]) { + for (const targetId of BLANK_TARGETS) { + const refused = await startFor(app, provider, { kind, targetId }, own); + expect([refused.statusCode, refused.json().error], `${kind} ${JSON.stringify(targetId)}`).toEqual([400, "invalid_connection_target"]); + expect(refused.body).not.toContain("authorizationUrl"); + } + } + for (const targetId of NAMING_TARGETS) { + expect((await startFor(app, provider, { kind: "creator", targetId }, own)).statusCode, JSON.stringify(targetId)).toBe(201); + } + }); + + it.each(managed)("%s: a blank target without the app's secret gets 401, not the 400 that would tell a stranger what the gate checks", async (provider) => { + const app = await buildApp({ config: createTestConfig() }); + apps.push(app); + + for (const headers of [{}, { "x-heimdall-app-secret": "wrong" }]) { + for (const targetId of [" ", "\u2800"]) { + const refused = await startFor(app, provider, { kind: "creator", targetId }, headers); + expect([refused.statusCode, refused.json()], JSON.stringify(targetId)).toEqual([401, { error: "app_auth_required" }]); + } + } + }); + + it("the callback mint holds a blank creator target to the same bar", () => { + const streampixels = builtInAppProfiles.streampixels!; + for (const targetId of BLANK_TARGETS) { + expect(callbackMint(streampixels, "joystick", "connect", { kind: "creator", targetId }), JSON.stringify(targetId)).toBe("refused"); + } + // A visible character among the blanks names a target. + expect(callbackMint(streampixels, "joystick", "connect", { kind: "creator", targetId: "\u2800a\u3164" })).toBe("custody_only"); + for (const targetId of NAMING_TARGETS) { + expect(callbackMint(streampixels, "joystick", "connect", { kind: "creator", targetId }), JSON.stringify(targetId)).toBe("custody_only"); + } + }); + }); + describe("joystick connect", () => { const creator = { kind: "creator", targetId: "creator-1" }; async function connect(app: FastifyInstance, extra: Record = { connection: creator }) {