fix(bff): let a key with allowedOrigins serve non-browser clients - #1861
fix(bff): let a key with allowedOrigins serve non-browser clients#1861Tonours wants to merge 5 commits into
Conversation
|
Coverage Impact This PR will not change total coverage. Modified Files with Diff Coverage (2)
🛟 Help
|
nbouliol
left a comment
There was a problem hiding this comment.
Core fix looks right, and the deviation from the ticket's suggested normalizeOrigin(...) !== null is the better call — that snippet would have let Origin: null and garbage through, against the ticket's own Expected. Three non-blocking notes below.

What
Layer 2 (per-key
allowedOrigins) now only rejects anOriginthe client actually sent. A request with noOriginheader — curl, a Node backend, CI — passes.One condition in
packages/agent-bff/src/cors/per-key-origin.ts: read the header once, skip the allow-list check when the client sent nothing.fixes PRD-1102
Why
A key created with
allowedOrigins: ["http://localhost:4567"]was usable from a browser only. Koa returns''for an absent header,normalizeOrigin('')returnsnull,originAllowedthen returnsfalse— so every server-side client got403 origin_not_allowed, with nothing in the error or the docs to explain it. The workaround was provisioning a second, origins-less key.The
Originis not the boundary for a server client; the API key is. The API key middleware runs before this guard (cli-core.ts) and is the only writer ofctx.state.apiKeyIdentity, so no unauthenticated request reaches the relaxed path.Scope and safety
Originstill gets 403. Unchanged, still tested.Origin: null(sandboxed iframe, cross-origin redirect) is a present origin and still gets 403. The ticket suggestednormalizeOrigin(...) !== nullas the skip condition; that would have let the opaque origin through, and a browser can produce it. Asking whether the client sent anything at all keeps every browser-producible origin gated.Originstill gets 403, as before.BFF_ALLOWED_ORIGINS,cors-middleware.ts) is untouched — byte-identical tomain. The two layers need opposite absent-Originsemantics, which is why the fix is at the caller and not inoriginAllowed.A blank
Origin:header counts as absent and passes. That is not a new decision so much as a fact about HTTP: Node strips leading and trailing whitespace from header values, soOrigin:andOrigin:both reach koa as the empty string, indistinguishable from no header at all. No browser sends either — an opaque origin serializes to the literal stringnull, which stays rejected.Documented, not just fixed
bffApiKeyin the OpenAPI document said only "a BFF API key". It now carries the rule, so a consumer reading the document alone learns that a key withallowedOriginsgates browsers and not server-side callers, and thatOrigin: nullis rejected. A test pins it against the README.How to test
yarn workspace @forestadmin/agent-bff testThe no-Origin case in
test/cors/per-key-origin.test.tswas flipped from 403 to 200 as the ticket asked, and two route cases were added:Origin: nullgets 403, an empty header gets 200. The blank-input contract ofhasOriginis pinned intest/cors/origin.test.tsrather than through the route, since HTTP cannot deliver whitespace to it.Definition of Done
General
Security