app.ts: never trust a caller-chosen completion code or returnTo origin
createAuthCompletion no longer takes `code: handoff.attemptId` — the store mints it (previous commit); attemptId is passed through only as the new `attemptId` correlation field. The redeem route's updateAuthAttempt call used request.body.completionCode as if it were the attempt handle, which only ever worked by the coincidence that the private plane's own flow had code === handle; it now uses completion.attemptId, the real correlation. startOAuthFlow also refuses any returnTo whose origin is not in the target app's AppProfile.allowedReturnOrigins (400 return_to_not_allowed). Before this, returnTo was validated only as `format: uri`, and the browser handoff page postMessages the completion payload to new URL(returnTo).origin — an unlisted origin was an exfiltration primitive for whatever the browser flow hands back, completion code included. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
5a4da2ae30
commit
e5c9228be2
1 changed files with 27 additions and 6 deletions
33
src/app.ts
33
src/app.ts
|
|
@ -23,7 +23,7 @@ import {
|
|||
type Provider,
|
||||
} from "./contracts.js";
|
||||
import { issueAccessClaim, type IssueAccessClaimInput } from "./claims.js";
|
||||
import { builtInAppProfiles, getAppProfile, serializeAppProfile, supportsProvider } from "./app-profiles.js";
|
||||
import { builtInAppProfiles, getAppProfile, isAllowedReturnOrigin, serializeAppProfile, supportsProvider } from "./app-profiles.js";
|
||||
import { listAppProfiles, resolveAppProfile } from "./app-registry.js";
|
||||
import { renderBrowserHandoffPage } from "./browser-handoff.js";
|
||||
import { type HeimdallConfig, loadConfig } from "./config.js";
|
||||
|
|
@ -522,6 +522,13 @@ export async function startOAuthFlow(
|
|||
};
|
||||
}
|
||||
|
||||
if (!isAllowedReturnOrigin(profile, input.returnTo)) {
|
||||
return {
|
||||
statusCode: 400,
|
||||
body: { error: "return_to_not_allowed", detail: `returnTo must land on an origin ${profile.slug} owns.` },
|
||||
};
|
||||
}
|
||||
|
||||
let handoff: OAuthHandoff;
|
||||
try {
|
||||
handoff = normalizeOAuthHandoff(input.handoff);
|
||||
|
|
@ -1190,8 +1197,15 @@ export async function buildApp(options: BuildAppOptions = {}): Promise<FastifyIn
|
|||
}
|
||||
|
||||
const completionExpiresAt = new Date(Date.now() + config.completionTtlSeconds * 1000).toISOString();
|
||||
// The completion code is always minted here, never taken from the
|
||||
// caller. handoff.attemptId is a browser-carried correlation handle,
|
||||
// not a secret: before this cut it was assigned verbatim as the code
|
||||
// (createAuthCompletion({ code: handoff.attemptId })), so anyone who
|
||||
// started the flow could fix the redemption code in advance and
|
||||
// redeem a victim's signed tokens without ever touching the app
|
||||
// secret. It is stored on `attemptId` below purely for correlation.
|
||||
const completion = await store.createAuthCompletion({
|
||||
...(handoff.kind === "browser_completion" && handoff.attemptId ? { code: handoff.attemptId } : {}),
|
||||
...(handoff.kind === "browser_completion" && handoff.attemptId ? { attemptId: handoff.attemptId } : {}),
|
||||
appSlug: statePayload.app_slug,
|
||||
provider: request.params.provider,
|
||||
mode: statePayload.mode,
|
||||
|
|
@ -1356,10 +1370,17 @@ export async function buildApp(options: BuildAppOptions = {}): Promise<FastifyIn
|
|||
},
|
||||
createdAt: nowIso,
|
||||
});
|
||||
await store.updateAuthAttempt(request.params.appSlug, request.body.completionCode, {
|
||||
status: "consumed",
|
||||
at: nowIso,
|
||||
});
|
||||
// completion.attemptId is the browser-carried correlation handle, kept
|
||||
// separate from the redemption code itself (see createAuthCompletion
|
||||
// above); this used to key off the completion code, which only ever
|
||||
// matched an attempt by the coincidence of the pre-fix bug where the
|
||||
// code *was* the caller-chosen attemptId.
|
||||
if (completion.attemptId) {
|
||||
await store.updateAuthAttempt(request.params.appSlug, completion.attemptId, {
|
||||
status: "consumed",
|
||||
at: nowIso,
|
||||
});
|
||||
}
|
||||
|
||||
reply.code(201);
|
||||
return completion.payloadJson;
|
||||
|
|
|
|||
Loading…
Reference in a new issue