fix(security): allowlist mendys marketplace/live as social-broker return origins - #839
Merged
claude[bot] merged 3 commits intoAug 27, 2026
Merged
Conversation
…urn origins (#352) MendysRobotics' marketplace/live SPAs delegate sign-in to FuzeFront's server-brokered Google flow but never received the SPA back: the broker's success/error redirect always targets appBaseUrl() (the single ambient identity tenant's FRONTEND_URL, app.fuzefront.com in prod), regardless of which origin actually started the flow. Mechanism and why it's safe: - This repo already has a host-based allowlist mechanism (SECURITY_TENANTS / tenants.ts) that fail-closed-rejects any undeclared Host. It was NOT reused here: turning it on requires enumerating every host that legitimately reaches security-service in prod today, including in-cluster Service-DNS callers (provisioning-service, config-service, selection-list-service all hit fuzefront-security:3002/api/v1/security/* under the same tenantContext middleware) or those callers start getting hard-400'd the moment SECURITY_TENANTS is set. A pre-existing test fixture (tests/tenant-session-roundtrip.test.ts) already models a full "mendys" identity tenant on its own Authentik instance for a future white-label epic, but that instance (authentikMendys) is not deployed in prod (authentikMendys.enabled stays false, no AUTHENTIK_MENDYS_* secrets sealed) — wiring SECURITY_TENANTS now would depend on infra that doesn't exist yet. That is a separate, larger, currently-blocked effort; #352 only asks for the social broker's return origin, so this PR does not touch tenant resolution or identity backend selection at all. - Instead: a new, narrow, additive-only allowlist (providers/authentik/socialReturnOrigins.ts, env SECURITY_SOCIAL_RETURN_ORIGINS) resolved from the raw Host header at /social/:provider/start (same signal tenants.ts uses, for the same reason — X-Forwarded-Host is caller-supplied), carried through the broker's existing server-side state map to the callback, and consulted ONLY when building the final redirect Location. EXACT origin match — no wildcard, no subdomain, no startsWith/substring match (open-redirect / auth-code-leak surface). Unset/no-match = today's appBaseUrl()-only behaviour, byte for byte; every deployment without SECURITY_SOCIAL_RETURN_ORIGINS set is unaffected (verified: default helm render carries zero references to the new env var). - Wired into values-prod.yaml with exactly the two origins named in the issue: https://marketplace.mendysrobotics.com and https://live.mendysrobotics.com. `helm template`/`helm lint` verified. - A pending consent-denial or a mid-exchange failure also returns to the right origin (peekSocialReturnOrigin, a non-consuming read of the pending state) rather than only the happy path. Tests: new unit coverage for the allowlist itself (exact-match ACCEPT for both origins, and REJECT for an unlisted origin, a subdomain, and a prefix/suffix attack against an allowlisted host), provider-level threading tests, and route-level tests proving the Host header actually drives what gets passed to the provider and what the callback redirects to (including the rejection case at the HTTP layer). Also fixed one unrelated stale assertion in google-brokered-signin.test.ts (missing 4th `iss` arg) that was failing identically on unmodified master, in a file this PR already touches. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013tMciHkPE8To7V67CsgKKc
gate-identifier failed on this PR with 8 violations — 4 "'id' is minted as
a bare uuid" plus 4 "stale source exemption ... that line no longer mints
an id", all in AuthentikIdentityProvider.ts.
Neither is a new identifier defect. This PR inserts 23 lines above the
existing mint sites, and governance/identifier-allowlist.txt exempts them
by LINE NUMBER, so every exemption drifted off its target by exactly +23:
662 -> 685 email_verifications row
885 -> 908 mfa_recovery_codes row
976 -> 999 password_resets row
1082 -> 1105 email_verifications row
The gate is behaving correctly and is worth keeping strict: it refuses to
let a stale line number silently exempt whatever code drifts under it —
"a drifting line number exempts code nobody reviewed" — and reports both
halves (the now-unexempted mint site AND the exemption pointing at
nothing) rather than just going quiet.
Re-pointed each to its real site, which is what the gate's own message
prescribes. Every mapping was verified against the table the site inserts
into rather than assumed from the uniform +23 offset:
685 -> this.db('email_verifications').insert({ id: uuidv4(), ... })
908 -> this.db('mfa_recovery_codes').insert(... id: uuidv4() ...)
999 -> this.db('password_resets').insert({ id: uuidv4(), ... })
1105 -> this.db('email_verifications').insert({ id: uuidv4(), ... })
No exemption was added, removed, or widened; the same four sites are
exempt for the same four recorded reasons.
Verified: `python3 scripts/gate_identifier.py .` reproduced the 8
violations before the change and reports `gate-identifier: OK` after.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013tMciHkPE8To7V67CsgKKc
Conflict in deploy/helm/fuzefront/values-prod.yaml. Both sides ADD distinct, non-overlapping content under `securityService:` at the same point: ours (#352) — the live `socialReturnOrigins` allowlist for MendysRobotics' marketplace/live SPAs theirs (#835, now on master) — the commented-out, NOT-YET-ACTIVE second identity tenant block Neither replaces the other and neither loses behaviour if both are kept, so this is not a pick-one resolution: master's documentation block first, then this branch's live list. Verified by rendering rather than by eye, since a YAML conflict resolution that merely parses can still be structurally wrong: helm template ... -f values-prod.yaml -> OK, 61 resources SECURITY_SOCIAL_RETURN_ORIGINS = "https://marketplace.mendysrobotics.com,https://live.mendysrobotics.com" SECURITY_TENANTS -> still absent (single-tenant mode preserved) so #352's feature survives the merge and #835's tenant work stays inert, which is what its own comment says it must be until FuzeInfra#421 is live. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013tMciHkPE8To7V67CsgKKc
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #352. MendysRobotics'
marketplace/livemarketplace SPAs delegate sign-in entirely to FuzeFront's server-brokered Google flow (GET /api/v1/security/social/{provider}/start→ Google →/social/{google/}callback) and proxy/api/v1/security/*same-origin tofuzefront-security:3002. Today the broker's final redirect always targetsappBaseUrl()— the single ambient identity tenant'sFRONTEND_URL(app.fuzefront.comin prod) — regardless of which origin actually started the flow, so the mendys SPA never receives its?code=.Mechanism (and why it's this one, not the multi-tenant registry)
SECURITY_TENANTS(tenants.ts), which resolves an inbound request'sHostto an entire separate identity tenant (its own Authentik instance, OIDC client, admin token) and fail-closed rejects any undeclared host the moment it's set. I deliberately did not reuse it here: turning it on would require enumerating every host that legitimately reachessecurity-servicetoday, including in-cluster Service-DNS callers (provisioning-service,config-service,selection-list-serviceall callfuzefront-security:3002/api/v1/security/*, mounted behind the sametenantContextmiddleware) — or those callers start getting hard-400'd. There's already a test fixture (tests/tenant-session-roundtrip.test.ts) modelling a full "mendys" tenant on its own Authentik instance for a future white-label epic, but that instance (authentikMendys) is not deployed in prod (enabled: false, noAUTHENTIK_MENDYS_*secrets sealed) — wiringSECURITY_TENANTSnow would point at infra that doesn't exist. That's a separate, larger, currently-blocked effort; Social-broker return-origin allowlist: marketplace.mendysrobotics.com + live.mendysrobotics.com #352 only asks for the return origin, so this PR leaves tenant/identity-backend resolution completely untouched.backend/security/src/providers/authentik/socialReturnOrigins.ts, envSECURITY_SOCIAL_RETURN_ORIGINS— resolved from the rawHostheader at/social/:provider/start(same signaltenants.tsuses, for the same reason:X-Forwarded-Hostis caller-supplied), carried through the broker's existing server-side state map to the callback, and consulted only when building the final redirectLocation.startsWith/substring match. The destination receives a fresh single-use sign-in code, so an over-broad allowlist here is a direct open-redirect / auth-code-leak vector.appBaseUrl()-only behaviour, byte for byte. Verified viahelm templatethat a deployment withoutsecurityService.socialReturnOriginsset renders zero references to the new env var.peekSocialReturnOrigin, a non-consuming read of the pending state), not just the happy path.values-prod.yamlwith exactly the two origins named in the issue:https://marketplace.mendysrobotics.comandhttps://live.mendysrobotics.com.Test plan
socialReturnOrigins.test.ts): exact-match ACCEPT for both mendys origins; REJECT for an unlisted origin, a subdomain of an allowlisted host, and a prefix/suffix attack (marketplace.mendysrobotics.com.evil.com,notmarketplace.mendysrobotics.com); malformed config skipped safely.google-brokered-signin.test.ts(returnOrigin start→callback,peekSocialReturnOriginnon-consuming).security-routes.test.tsproving the Host header drives what's passed to the provider and what the callback redirects to, including the HTTP-layer rejection case and the optional-method fallback.helm template/helm lintagainstvalues.yaml+values-prod.yaml— new env renders only in prod, with the two mendys origins.backend/securityjest suite run locally (571 pre-existing passing tests unaffected; the handful of pre-existing failures —security-api/*.contract.test.tsMockIdentityProvider drift, an unrelatedauthentik-provider.test.tsnotification-mock flake, DB-dependent integration suites — reproduce identically on unmodifiedmasterviagit stash, so nothing here is a regression from this PR). Fixed one unrelated stale assertion ingoogle-brokered-signin.test.ts(missing 4thissarg) in passing, since this PR already touches that file.🤖 Generated with Claude Code
https://claude.ai/code/session_013tMciHkPE8To7V67CsgKKc
Generated by Claude Code