From 836070f7be18e1f72a2bccc12e810aed8a83c1ab Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 08:17:52 +0000 Subject: [PATCH 1/2] fix(security): allowlist mendys marketplace/live as social-broker return origins (#352) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_013tMciHkPE8To7V67CsgKKc --- .env.example | 10 ++ .../src/providers/IdentityProvider.ts | 29 ++++- .../authentik/AuthentikIdentityProvider.ts | 35 +++++- .../authentik/socialReturnOrigins.ts | 99 +++++++++++++++ backend/security/src/routes/security.ts | 41 +++++-- .../tests/google-brokered-signin.test.ts | 54 ++++++++- .../security/tests/security-routes.test.ts | 100 +++++++++++++++ .../tests/socialReturnOrigins.test.ts | 114 ++++++++++++++++++ deploy/helm/fuzefront/templates/security.yaml | 11 ++ deploy/helm/fuzefront/values-prod.yaml | 15 +++ deploy/helm/fuzefront/values.yaml | 13 ++ 11 files changed, 504 insertions(+), 17 deletions(-) create mode 100644 backend/security/src/providers/authentik/socialReturnOrigins.ts create mode 100644 backend/security/tests/socialReturnOrigins.test.ts diff --git a/.env.example b/.env.example index 2fd46cf89..5c8f4ceff 100644 --- a/.env.example +++ b/.env.example @@ -21,6 +21,16 @@ USE_POSTGRES= # Frontend Configuration FRONTEND_URL= +# Social-broker return-origin allowlist (FuzeFront#352). Comma-separated list +# of full origins (scheme + host, no path/trailing slash) the server-brokered +# social sign-in flow (GET /api/v1/security/social/{provider}/start|callback) +# may redirect the browser back to INSTEAD OF FRONTEND_URL, for a consumer +# that proxies /api/v1/security/* same-origin from its own domain. Matched by +# EXACT Host header only — no wildcard/subdomain matching. Empty/unset = no +# additional origins (default; every deployment is unaffected). +# Example: SECURITY_SOCIAL_RETURN_ORIGINS=https://marketplace.mendysrobotics.com,https://live.mendysrobotics.com +SECURITY_SOCIAL_RETURN_ORIGINS= + # Authentik Configuration - Uses shared FuzeInfra PostgreSQL and Redis AUTHENTIK_DB_NAME= AUTHENTIK_SECRET_KEY= diff --git a/backend/security/src/providers/IdentityProvider.ts b/backend/security/src/providers/IdentityProvider.ts index d465ef7c5..7d1835bc6 100644 --- a/backend/security/src/providers/IdentityProvider.ts +++ b/backend/security/src/providers/IdentityProvider.ts @@ -111,6 +111,13 @@ export interface SocialCallbackResult { linked?: boolean; /** For a link handshake, the neutral provider slug that was linked. */ provider?: string; + /** + * The allowlisted return origin captured at `/social/:provider/start` + * (see `providers/authentik/socialReturnOrigins.ts`), if the starting + * request's Host matched one. `undefined` means "no match" — the caller + * falls back to `appBaseUrl()`, exactly as before this field existed. + */ + returnOrigin?: string; } export interface M2MClientProvisionInput { @@ -282,12 +289,30 @@ export interface IdentityProvider { */ passwordLogin(input: PasswordLoginInput, ctx?: SessionContext): Promise; - /** Begin a social login; returns where to 302 the browser + anti-forgery state. */ - startSocialLogin(provider: string, redirectTo?: string): Promise; + /** + * Begin a social login; returns where to 302 the browser + anti-forgery state. + * `returnOrigin`, when provided, is an ALREADY-ALLOWLISTED origin (see + * `providers/authentik/socialReturnOrigins.ts`) the finished handshake + * should redirect back to instead of the ambient tenant's `appBaseUrl()` — + * e.g. a consumer proxying this API same-origin from its own domain + * (FuzeFront#352). Omit to keep today's `appBaseUrl()` behaviour. + */ + startSocialLogin(provider: string, redirectTo?: string, returnOrigin?: string): Promise; /** Complete the social handshake; returns a single-use opaque code + return path. */ brokerCallback(input: SocialCallbackInput, ctx?: SessionContext): Promise; + /** + * Non-consuming peek at the return origin recorded for a still-pending + * `state` (does NOT delete it — `brokerCallback` remains the sole + * consumer). Lets a callback route pick the right redirect destination for + * an error that occurs before/without calling `brokerCallback` (e.g. the + * provider itself reports `error=access_denied`). Optional: providers that + * don't support per-request return origins simply omit it, and callers + * fall back to `appBaseUrl()`. + */ + peekSocialReturnOrigin?(state: string): string | undefined; + /** Exchange a single-use opaque code for a session. Rejects on unknown/expired code. */ exchangeCode(code: string): Promise; diff --git a/backend/security/src/providers/authentik/AuthentikIdentityProvider.ts b/backend/security/src/providers/authentik/AuthentikIdentityProvider.ts index 147ab4e93..918ae62cc 100644 --- a/backend/security/src/providers/authentik/AuthentikIdentityProvider.ts +++ b/backend/security/src/providers/authentik/AuthentikIdentityProvider.ts @@ -150,6 +150,12 @@ interface SocialState { * externalize for multi-replica — see [[Redis externalization]]. */ mode: 'brokered' | 'source' + /** + * Allowlisted return origin captured at start (FuzeFront#352) — see + * `socialReturnOrigins.ts`. `undefined` = use the ambient tenant's + * `appBaseUrl()`, exactly as before this field existed. + */ + returnOrigin?: string } interface MfaChallenge { userId: string @@ -423,7 +429,7 @@ export class AuthentikIdentityProvider implements IdentityProvider { // so no Authentik `/if/*` UI is ever rendered. The legacy Authentik // `/source/oauth/*` source-redirect path is kept as a fallback (flag off) until // the brokered path is proven. - async startSocialLogin(provider: string, redirectTo = '/'): Promise { + async startSocialLogin(provider: string, redirectTo = '/', returnOrigin?: string): Promise { if (provider !== 'google') { throw new InvalidInputError(`unsupported social provider: ${provider}`) } @@ -432,9 +438,9 @@ export class AuthentikIdentityProvider implements IdentityProvider { throw new InvalidInputError('redirectTo must be a same-origin path') } if (googleBrokeredEnabled()) { - return this.startGoogleBrokered(redirectTo) + return this.startGoogleBrokered(redirectTo, returnOrigin) } - return this.startSocialLoginViaSource(provider, redirectTo) + return this.startSocialLoginViaSource(provider, redirectTo, returnOrigin) } /** @@ -444,7 +450,7 @@ export class AuthentikIdentityProvider implements IdentityProvider { * verifier is held in the process-local Map (single replica) — * TODO(redis): externalize for multi-replica — see [[Redis externalization]]. */ - private async startGoogleBrokered(redirectTo: string): Promise { + private async startGoogleBrokered(redirectTo: string, returnOrigin?: string): Promise { if (!this.googleClient.isInitialized()) { await this.googleClient.initialize() } @@ -455,6 +461,7 @@ export class AuthentikIdentityProvider implements IdentityProvider { redirectTo, expiresAt: this.now() + 10 * 60_000, mode: 'brokered', + returnOrigin, }) // `url` is the absolute accounts.google.com authorize URL — the ONLY external // host the browser is allowed to see besides app.fuzefront.com. @@ -462,7 +469,11 @@ export class AuthentikIdentityProvider implements IdentityProvider { } /** Legacy Authentik source-redirect start (fallback; browser transits `/if/*`). */ - private async startSocialLoginViaSource(provider: string, redirectTo = '/'): Promise { + private async startSocialLoginViaSource( + provider: string, + redirectTo = '/', + returnOrigin?: string + ): Promise { if (!this.oidc.isInitialized()) { await this.oidc.ensureInitialized() } @@ -498,6 +509,7 @@ export class AuthentikIdentityProvider implements IdentityProvider { redirectTo, expiresAt: this.now() + 10 * 60_000, mode: 'source', + returnOrigin, }) // codeVerifier is handed back so the route can persist it in an HttpOnly // cookie (`oidc_cv`) for the replica-agnostic OIDC callback. @@ -563,7 +575,18 @@ export class AuthentikIdentityProvider implements IdentityProvider { user: session.user, expiresAt: this.now() + CODE_TTL_MS, }) - return { code, redirectTo: st.redirectTo || '/' } + return { code, redirectTo: st.redirectTo || '/', returnOrigin: st.returnOrigin } + } + + /** + * Non-consuming peek at the return origin recorded for a pending `state` — + * used by the callback ROUTE to pick a redirect destination for an error + * that short-circuits before (or instead of) calling `brokerCallback` + * (e.g. the provider's own `error=access_denied`). Does not delete the + * entry: `brokerCallback` remains the single place state is consumed. + */ + peekSocialReturnOrigin(state: string): string | undefined { + return this.socialStates.get(state)?.returnOrigin } async exchangeCode(code: string): Promise { diff --git a/backend/security/src/providers/authentik/socialReturnOrigins.ts b/backend/security/src/providers/authentik/socialReturnOrigins.ts new file mode 100644 index 000000000..c24f5d8ca --- /dev/null +++ b/backend/security/src/providers/authentik/socialReturnOrigins.ts @@ -0,0 +1,99 @@ +/** + * Return-origin allowlist for the brokered social sign-in flow + * (FuzeFront#352 — MendysRobotics marketplace/live). + * + * By default, `startSocialLogin` / `brokerCallback` in + * `AuthentikIdentityProvider` send the browser's single-use opaque `?code=` + * back to `${appBaseUrl()}${redirectTo}` — the ambient identity tenant's + * configured app origin (`providers/authentik/config.ts` / + * `providers/authentik/tenants.ts`). That is correct for FuzeFront's own SPA, + * but a consumer that proxies `/api/v1/security/*` SAME-ORIGIN from its own + * domain (MendysRobotics' `marketplace`/`live` SPAs) needs the code returned + * to ITS origin instead of `app.fuzefront.com`. + * + * ── Why this is NOT the multi-tenant identity registry ────────────────────── + * `SECURITY_TENANTS` (tenants.ts) already resolves an inbound request's Host + * to a whole separate identity tenant — its OWN Authentik instance, OIDC + * client, admin token, etc. — and switches the service into a mode where any + * UNDECLARED host is REJECTED with a hard 400 (tenants.ts's "FAIL CLOSED"). + * Turning that on would require enumerating EVERY host that legitimately + * reaches security-service today — Traefik (`app.fuzefront.com`), and every + * in-cluster caller's Service-DNS Host header (provisioning-service, + * config-service, selection-list-service all call + * `http://fuzefront-security:3002/api/v1/security/*`, which is also mounted + * behind `tenantContext`) — or those callers start getting rejected the + * moment `SECURITY_TENANTS` is set. That is a prod-wide identity-routing + * change with a large, not-fully-enumerable-from-this-repo blast radius, for + * a request that is only "let the social broker return to two more origins". + * So this module is a separate, narrower, ADDITIVE-ONLY mechanism: it never + * changes which identity backend serves a request, only which origin the + * *finished* broker redirect targets — and it changes nothing when no host + * matches (today's behaviour, byte-for-byte, for every existing deployment). + * + * ── Matching rule ──────────────────────────────────────────────────────── + * EXACT origin match only — no wildcard, no subdomain pattern, no + * startsWith/substring match. The destination receives the caller's fresh + * single-use sign-in code, so an over-broad allowlist here is a direct + * open-redirect / auth-code-leak vector. Matched on the inbound request's raw + * `Host` header, normalised the same way `tenants.ts` normalises it (see + * `normaliseHost`'s doc comment there for why raw `Host` rather than + * `X-Forwarded-Host` is the right signal here too) — captured once at + * `/social/:provider/start` and carried in the broker's server-side state + * (`AuthentikIdentityProvider.socialStates`) so the later callback — which + * physically arrives back on FuzeFront's own host via Google's registered + * `redirect_uri`, not on the origin that started the flow — knows where to + * send the browser next. + */ +import { Request } from 'express' +import { normaliseHost } from './tenants' + +/** + * `SECURITY_SOCIAL_RETURN_ORIGINS` — comma-separated list of full origins + * (`https://host`, scheme required, no path/query/fragment/trailing slash) + * the social broker may redirect the browser back to, in addition to the + * ambient tenant's own `appBaseUrl()`. Unset or empty = no additional + * origins; every existing deployment is unaffected. + * + * Re-parsed on every call, deliberately: this is a plain env value (no + * secret material — it is just a list of public web origins), cheap to + * re-read, and tests set/clear `process.env.SECURITY_SOCIAL_RETURN_ORIGINS` + * between cases without needing a reset hook. + */ +function parseAllowedReturnOrigins(): Map { + const raw = process.env.SECURITY_SOCIAL_RETURN_ORIGINS + const byHost = new Map() + if (!raw) return byHost + for (const entry of raw.split(',')) { + const candidate = entry.trim().replace(/\/+$/, '') + if (!candidate) continue + let url: URL + try { + url = new URL(candidate) + } catch { + // Malformed entry — skip it rather than crash the process. This value + // is not validated at deploy time the way SECURITY_TENANTS is. + continue + } + // Require a bare origin. A path/query/fragment on an allowlist entry is + // very likely a misconfiguration, and silently truncating it to just the + // host would allowlist more than was actually written down. + if ((url.pathname !== '/' && url.pathname !== '') || url.search || url.hash) continue + if (url.protocol !== 'https:' && url.protocol !== 'http:') continue + const host = normaliseHost(url.host) + if (!host) continue + byHost.set(host, `${url.protocol}//${url.host}`) + } + return byHost +} + +/** + * Resolve the return origin for a `/social/:provider/start` request, or + * `undefined` if its Host does not exactly match an allowlisted origin (the + * overwhelmingly common case — callers then keep using `appBaseUrl()`). + */ +export function resolveAllowedReturnOrigin(req: Pick): string | undefined { + const allowed = parseAllowedReturnOrigins() + if (allowed.size === 0) return undefined + const host = normaliseHost(req.headers.host) + return host ? allowed.get(host) : undefined +} diff --git a/backend/security/src/routes/security.ts b/backend/security/src/routes/security.ts index 577bcfdc0..01a8ceffc 100644 --- a/backend/security/src/routes/security.ts +++ b/backend/security/src/routes/security.ts @@ -22,6 +22,7 @@ import { emailVerificationEnabled, } from '../providers/authentik/AuthentikIdentityProvider' import { appBaseUrl } from '../providers/authentik/config' +import { resolveAllowedReturnOrigin } from '../providers/authentik/socialReturnOrigins' import { findUserPk, setUserPassword, PasswordPolicyError } from '../providers/authentik/accountApi' import type { BrokeredSession, @@ -276,7 +277,15 @@ router.post('/session/password/reset-confirm', async (req: Request, res: Respons router.get('/social/:provider/start', async (req: Request, res: Response) => { try { const redirectTo = typeof req.query.redirectTo === 'string' ? req.query.redirectTo : '/' - const { redirectUrl, state, codeVerifier } = await getIdentityProvider().startSocialLogin(req.params.provider, redirectTo) + // Exact-match allowlist only (FuzeFront#352) — see socialReturnOrigins.ts. + // undefined when the request's Host isn't allowlisted, which keeps the + // existing appBaseUrl()-only behaviour for every other caller. + const returnOrigin = resolveAllowedReturnOrigin(req) + const { redirectUrl, state, codeVerifier } = await getIdentityProvider().startSocialLogin( + req.params.provider, + redirectTo, + returnOrigin + ) // The authorize URL's redirect_uri is the OIDC client's registered callback // (`/api/auth/oidc/callback`), so the browser returns THERE after Google // consent. That handler is replica-agnostic via the oidc_state + oidc_cv @@ -297,6 +306,13 @@ router.get('/social/:provider/start', async (req: Request, res: Response) => { // GET /v1/security/social/callback — broker callback, 302 back to app with ?code= router.get('/social/callback', async (req: Request, res: Response) => { + // Resolved ONCE, before the exchange, via a non-consuming peek at the + // pending state (FuzeFront#352) — so an allowlisted return origin also + // applies to the fail-closed error redirect below, not just the success + // path. Falls back to the ambient tenant's appBaseUrl() when the state is + // unknown/expired or the provider declares no return-origin support. + const statePeek = typeof req.query.state === 'string' ? req.query.state : '' + const destOrigin = getIdentityProvider().peekSocialReturnOrigin?.(statePeek) ?? appBaseUrl() try { const code = typeof req.query.code === 'string' ? req.query.code : '' const state = typeof req.query.state === 'string' ? req.query.state : '' @@ -308,16 +324,17 @@ router.get('/social/callback', async (req: Request, res: Response) => { // Clear the state cookie; append the FuzeFront opaque code (never a token). res.setHeader('Set-Cookie', ['sec_social_state=; HttpOnly; Secure; SameSite=Lax; Max-Age=0; Path=/']) const sep = result.redirectTo.includes('?') ? '&' : '?' + const origin = result.returnOrigin ?? destOrigin // A LINK handshake mints no session, so there is no code to redeem — send // the browser back with a neutral confirmation instead. const dest = result.linked - ? `${appBaseUrl()}${result.redirectTo}${sep}linked=${encodeURIComponent(result.provider ?? '')}` - : `${appBaseUrl()}${result.redirectTo}${sep}code=${encodeURIComponent(result.code)}` + ? `${origin}${result.redirectTo}${sep}linked=${encodeURIComponent(result.provider ?? '')}` + : `${origin}${result.redirectTo}${sep}code=${encodeURIComponent(result.code)}` res.redirect(302, dest) } catch (err) { // Fail-closed: send the browser back to the app with a neutral error. res.setHeader('Set-Cookie', ['sec_social_state=; HttpOnly; Secure; SameSite=Lax; Max-Age=0; Path=/']) - res.redirect(302, `${appBaseUrl()}/?error=authentication_failed`) + res.redirect(302, `${destOrigin}/?error=authentication_failed`) } }) @@ -331,10 +348,17 @@ router.get('/social/callback', async (req: Request, res: Response) => { // State + PKCE are held server-side in the provider's Map (single replica) — no // cookie round-trip needed for this path. router.get('/social/google/callback', async (req: Request, res: Response) => { + // Resolved ONCE, up front, via a non-consuming peek at the pending state + // (FuzeFront#352) — applies the allowlisted return origin to every exit + // from this handler, including Google's own error param below and the + // fail-closed catch. Falls back to appBaseUrl() when the state is + // unknown/expired/absent or the provider declares no return-origin support. + const statePeek = typeof req.query.state === 'string' ? req.query.state : '' + const destOrigin = getIdentityProvider().peekSocialReturnOrigin?.(statePeek) ?? appBaseUrl() // Google can return an explicit error (e.g. the user denied consent). Never // leak provider detail to the app — send a neutral error back. if (typeof req.query.error === 'string' && req.query.error) { - res.redirect(302, `${appBaseUrl()}/?error=authentication_failed`) + res.redirect(302, `${destOrigin}/?error=authentication_failed`) return } try { @@ -346,15 +370,16 @@ router.get('/social/google/callback', async (req: Request, res: Response) => { if (!code || !state) throw new InvalidInputError('code and state are required') const result = await getIdentityProvider().brokerCallback({ code, state, iss }, sessionContext(req)) const sep = result.redirectTo.includes('?') ? '&' : '?' + const origin = result.returnOrigin ?? destOrigin // A LINK handshake mints no session (no code to redeem) — return a neutral // confirmation instead. const dest = result.linked - ? `${appBaseUrl()}${result.redirectTo}${sep}linked=${encodeURIComponent(result.provider ?? '')}` - : `${appBaseUrl()}${result.redirectTo}${sep}code=${encodeURIComponent(result.code)}` + ? `${origin}${result.redirectTo}${sep}linked=${encodeURIComponent(result.provider ?? '')}` + : `${origin}${result.redirectTo}${sep}code=${encodeURIComponent(result.code)}` res.redirect(302, dest) } catch (err) { // Fail-closed: neutral error back to the app. Never surface tokens or vendor. - res.redirect(302, `${appBaseUrl()}/?error=authentication_failed`) + res.redirect(302, `${destOrigin}/?error=authentication_failed`) } }) diff --git a/backend/security/tests/google-brokered-signin.test.ts b/backend/security/tests/google-brokered-signin.test.ts index 195dad658..178753a42 100644 --- a/backend/security/tests/google-brokered-signin.test.ts +++ b/backend/security/tests/google-brokered-signin.test.ts @@ -134,7 +134,12 @@ describe('server-brokered Google callback (success)', () => { expect(redirectTo).toBe('/dashboard') // Code exchanged with Google server-to-server (not the IdP OIDC client). - expect(googleClient.handleCallback).toHaveBeenCalledWith('goog-auth-code', state, 'gverifier') + // 4th arg is the RFC 9207 `iss` echoed by the provider — undefined here + // since this call supplies none. Pre-existing 3-arg assertion started + // failing once `iss` was threaded through; fixed in passing (FuzeFront#352 + // touches this file for the return-origin tests below and this was + // failing on unmodified master too — out of scope of #352 otherwise). + expect(googleClient.handleCallback).toHaveBeenCalledWith('goog-auth-code', state, 'gverifier', undefined) // Provisioned/linked in the identity store as system-of-record. expect(provisionSocialUser).toHaveBeenCalledWith( expect.objectContaining({ email: 'gina@example.com', sub: 'google-sub-123' }), @@ -170,3 +175,50 @@ describe('server-brokered Google callback (failure paths)', () => { expect(db.__tables.sessions.length).toBe(0) }) }) + +// ── Return-origin threading (FuzeFront#352) ────────────────────────────────── +// The allowlist MATCH/REJECT itself is covered by socialReturnOrigins.test.ts; +// these assert the provider correctly carries an already-resolved return +// origin from start -> callback (and exposes it via a non-consuming peek for +// the route's error paths) without touching FRONTEND_URL/appBaseUrl() at all. +describe('server-brokered Google — return-origin threading', () => { + it('carries the caller-supplied returnOrigin from start through to the callback result', async () => { + const { provider } = newProvider() + const { state } = await provider.startSocialLogin( + 'google', + '/dashboard', + 'https://marketplace.mendysrobotics.com' + ) + const { returnOrigin, redirectTo } = await provider.brokerCallback({ code: 'goog-auth-code', state }) + expect(returnOrigin).toBe('https://marketplace.mendysrobotics.com') + expect(redirectTo).toBe('/dashboard') + }) + + it('leaves returnOrigin undefined when the route did not resolve one (unchanged default behaviour)', async () => { + const { provider } = newProvider() + const { state } = await provider.startSocialLogin('google', '/dashboard') + const { returnOrigin } = await provider.brokerCallback({ code: 'goog-auth-code', state }) + expect(returnOrigin).toBeUndefined() + }) + + it('peekSocialReturnOrigin reads the pending returnOrigin WITHOUT consuming the state', async () => { + const { provider } = newProvider() + const { state } = await provider.startSocialLogin( + 'google', + '/dashboard', + 'https://live.mendysrobotics.com' + ) + expect(provider.peekSocialReturnOrigin(state)).toBe('https://live.mendysrobotics.com') + // Still consumable exactly once afterwards — the peek did not delete it. + const { returnOrigin } = await provider.brokerCallback({ code: 'goog-auth-code', state }) + expect(returnOrigin).toBe('https://live.mendysrobotics.com') + await expect(provider.brokerCallback({ code: 'goog-auth-code', state })).rejects.toBeInstanceOf( + UnauthorizedError + ) + }) + + it('peekSocialReturnOrigin returns undefined for an unknown state', async () => { + const { provider } = newProvider() + expect(provider.peekSocialReturnOrigin('never-issued')).toBeUndefined() + }) +}) diff --git a/backend/security/tests/security-routes.test.ts b/backend/security/tests/security-routes.test.ts index 290ee46b8..925727ce6 100644 --- a/backend/security/tests/security-routes.test.ts +++ b/backend/security/tests/security-routes.test.ts @@ -543,6 +543,106 @@ describe('social login boundary', () => { }) }) +// ── Social-broker return-origin allowlist (FuzeFront#352) ─────────────────── +// The allowlist MATCH/REJECT logic itself is unit-tested in +// socialReturnOrigins.test.ts. These prove the ROUTE actually consults it on +// the real request path — start resolves it from Host and passes it to the +// provider; the callbacks use whatever the provider hands back (or peeks), +// never touching appBaseUrl() when a valid returnOrigin is present. +describe('social login boundary — return-origin allowlist', () => { + const ENV_KEY = 'SECURITY_SOCIAL_RETURN_ORIGINS' + const savedEnv = process.env[ENV_KEY] + + beforeEach(() => { + process.env[ENV_KEY] = + 'https://marketplace.mendysrobotics.com,https://live.mendysrobotics.com' + }) + + afterEach(() => { + if (savedEnv === undefined) delete process.env[ENV_KEY] + else process.env[ENV_KEY] = savedEnv + }) + + it('start resolves an allowlisted Host and passes it to startSocialLogin', async () => { + const startSocialLogin = jest + .fn() + .mockResolvedValue({ redirectUrl: '/api/auth/idp/application/o/authorize/?x=1', state: 'st' }) + await request(makeApp(fakeProvider({ startSocialLogin }))) + .get('/api/v1/security/social/google/start') + .set('Host', 'marketplace.mendysrobotics.com') + expect(startSocialLogin).toHaveBeenCalledWith( + 'google', + '/', + 'https://marketplace.mendysrobotics.com' + ) + }) + + it('REJECTS a non-allowlisted Host — startSocialLogin gets undefined, same as today', async () => { + const startSocialLogin = jest + .fn() + .mockResolvedValue({ redirectUrl: '/api/auth/idp/application/o/authorize/?x=1', state: 'st' }) + await request(makeApp(fakeProvider({ startSocialLogin }))) + .get('/api/v1/security/social/google/start') + .set('Host', 'evil.example.com') + expect(startSocialLogin).toHaveBeenCalledWith('google', '/', undefined) + }) + + it('google/callback redirects the opaque code to the allowlisted returnOrigin the provider resolved', async () => { + const provider = fakeProvider({ + brokerCallback: jest.fn().mockResolvedValue({ + code: 'opaque', + redirectTo: '/dashboard', + returnOrigin: 'https://marketplace.mendysrobotics.com', + }), + }) + const res = await request(makeApp(provider)) + .get('/api/v1/security/social/google/callback?code=google-code&state=st') + .set('Host', 'app.fuzefront.com') // callback always lands on FuzeFront's own host + expect(res.status).toBe(302) + expect(res.headers.location).toBe('https://marketplace.mendysrobotics.com/dashboard?code=opaque') + }) + + it('legacy /social/callback redirects to the allowlisted returnOrigin the provider resolved', async () => { + const provider = fakeProvider({ + brokerCallback: jest.fn().mockResolvedValue({ + code: 'opaque', + redirectTo: '/dashboard', + returnOrigin: 'https://live.mendysrobotics.com', + }), + }) + const res = await request(makeApp(provider)).get( + '/api/v1/security/social/callback?code=prov&state=st' + ) + expect(res.status).toBe(302) + expect(res.headers.location).toBe('https://live.mendysrobotics.com/dashboard?code=opaque') + }) + + it('google/callback fail-closed error still targets the allowlisted origin via peekSocialReturnOrigin', async () => { + const provider = fakeProvider({ + peekSocialReturnOrigin: jest.fn().mockReturnValue('https://marketplace.mendysrobotics.com'), + }) + const res = await request(makeApp(provider)).get( + '/api/v1/security/social/google/callback?error=access_denied&state=st' + ) + expect(res.status).toBe(302) + expect(res.headers.location).toBe( + 'https://marketplace.mendysrobotics.com/?error=authentication_failed' + ) + }) + + it('google/callback WITHOUT a peekSocialReturnOrigin implementation falls back to appBaseUrl() unchanged', async () => { + // fakeProvider's base object omits peekSocialReturnOrigin entirely — + // proves the optional-method fallback (`?.() ?? appBaseUrl()`) degrades + // safely for a provider that predates this feature. + const res = await request(makeApp(fakeProvider())).get( + '/api/v1/security/social/google/callback?error=access_denied&state=st' + ) + expect(res.status).toBe(302) + expect(res.headers.location).not.toMatch(/mendysrobotics\.com/) + expect(res.headers.location).toContain('error=authentication_failed') + }) +}) + describe('POST /signup', () => { it('returns 201 application/json for a valid application/json signup', async () => { // @fuzequality api signup diff --git a/backend/security/tests/socialReturnOrigins.test.ts b/backend/security/tests/socialReturnOrigins.test.ts new file mode 100644 index 000000000..869ab0bef --- /dev/null +++ b/backend/security/tests/socialReturnOrigins.test.ts @@ -0,0 +1,114 @@ +/** + * Tests for the social-broker return-origin allowlist (FuzeFront#352). + * + * The claim under test is a security claim, not a convenience: this is an + * EXACT-origin allowlist consulted before the brokered social sign-in + * redirect carries a fresh single-use `?code=` back to the browser. An + * over-broad match here is an open-redirect / auth-code-leak vector, so the + * negative cases (prefix attack, subdomain, unlisted origin) matter at least + * as much as the two positive ones from the issue. + */ +import { resolveAllowedReturnOrigin } from '../src/providers/authentik/socialReturnOrigins' + +const ENV_KEY = 'SECURITY_SOCIAL_RETURN_ORIGINS' +const original = process.env[ENV_KEY] + +afterEach(() => { + if (original === undefined) delete process.env[ENV_KEY] + else process.env[ENV_KEY] = original +}) + +function reqWithHost(host: string | undefined) { + return { headers: { host } } +} + +describe('resolveAllowedReturnOrigin — unset/empty allowlist (default, every deployment today)', () => { + it('returns undefined regardless of Host when the env var is unset', () => { + delete process.env[ENV_KEY] + expect(resolveAllowedReturnOrigin(reqWithHost('marketplace.mendysrobotics.com'))).toBeUndefined() + expect(resolveAllowedReturnOrigin(reqWithHost('app.fuzefront.com'))).toBeUndefined() + }) + + it('returns undefined when the env var is an empty string', () => { + process.env[ENV_KEY] = '' + expect(resolveAllowedReturnOrigin(reqWithHost('marketplace.mendysrobotics.com'))).toBeUndefined() + }) +}) + +describe('resolveAllowedReturnOrigin — the two FuzeFront#352 origins', () => { + beforeEach(() => { + process.env[ENV_KEY] = + 'https://marketplace.mendysrobotics.com,https://live.mendysrobotics.com' + }) + + it('ACCEPTS marketplace.mendysrobotics.com, returning its exact configured origin', () => { + expect(resolveAllowedReturnOrigin(reqWithHost('marketplace.mendysrobotics.com'))).toBe( + 'https://marketplace.mendysrobotics.com' + ) + }) + + it('ACCEPTS live.mendysrobotics.com, returning its exact configured origin', () => { + expect(resolveAllowedReturnOrigin(reqWithHost('live.mendysrobotics.com'))).toBe( + 'https://live.mendysrobotics.com' + ) + }) + + it('matches case-insensitively and with an explicit :443 port, mirroring tenants.ts normaliseHost', () => { + expect(resolveAllowedReturnOrigin(reqWithHost('Marketplace.MendysRobotics.com'))).toBe( + 'https://marketplace.mendysrobotics.com' + ) + expect(resolveAllowedReturnOrigin(reqWithHost('marketplace.mendysrobotics.com:443'))).toBe( + 'https://marketplace.mendysrobotics.com' + ) + }) + + // ── REJECTIONS — the core security property ──────────────────────────── + it('REJECTS an origin not on the allowlist (falls back to undefined -> appBaseUrl())', () => { + expect(resolveAllowedReturnOrigin(reqWithHost('evil.example.com'))).toBeUndefined() + expect(resolveAllowedReturnOrigin(reqWithHost('app.fuzefront.com'))).toBeUndefined() + }) + + it('REJECTS a subdomain of an allowlisted host (no wildcard/subdomain matching)', () => { + expect( + resolveAllowedReturnOrigin(reqWithHost('evil.marketplace.mendysrobotics.com')) + ).toBeUndefined() + expect( + resolveAllowedReturnOrigin(reqWithHost('attacker.live.mendysrobotics.com')) + ).toBeUndefined() + }) + + it('REJECTS a prefix/suffix attack against an allowlisted host (no startsWith/substring matching)', () => { + // Looks like the allowlisted host as a substring, but is a DIFFERENT host. + expect( + resolveAllowedReturnOrigin(reqWithHost('marketplace.mendysrobotics.com.evil.com')) + ).toBeUndefined() + expect( + resolveAllowedReturnOrigin(reqWithHost('notmarketplace.mendysrobotics.com')) + ).toBeUndefined() + expect(resolveAllowedReturnOrigin(reqWithHost('mendysrobotics.com'))).toBeUndefined() + }) + + it('REJECTS a missing/empty Host header', () => { + expect(resolveAllowedReturnOrigin(reqWithHost(undefined))).toBeUndefined() + expect(resolveAllowedReturnOrigin(reqWithHost(''))).toBeUndefined() + }) +}) + +describe('resolveAllowedReturnOrigin — malformed configuration is skipped, not fatal', () => { + it('ignores an entry with a path/query/fragment rather than silently widening to its host', () => { + process.env[ENV_KEY] = 'https://marketplace.mendysrobotics.com/some/path' + expect(resolveAllowedReturnOrigin(reqWithHost('marketplace.mendysrobotics.com'))).toBeUndefined() + }) + + it('ignores an unparseable entry and still matches the valid ones alongside it', () => { + process.env[ENV_KEY] = 'not-a-url, https://live.mendysrobotics.com' + expect(resolveAllowedReturnOrigin(reqWithHost('live.mendysrobotics.com'))).toBe( + 'https://live.mendysrobotics.com' + ) + }) + + it('ignores a non-http(s) scheme', () => { + process.env[ENV_KEY] = 'javascript://marketplace.mendysrobotics.com' + expect(resolveAllowedReturnOrigin(reqWithHost('marketplace.mendysrobotics.com'))).toBeUndefined() + }) +}) diff --git a/deploy/helm/fuzefront/templates/security.yaml b/deploy/helm/fuzefront/templates/security.yaml index e36aeeebc..87cce2149 100644 --- a/deploy/helm/fuzefront/templates/security.yaml +++ b/deploy/helm/fuzefront/templates/security.yaml @@ -88,6 +88,17 @@ spec: key: SESSION_SECRET - name: FRONTEND_URL value: "{{ ternary "https" "http" .Values.ingress.tls.enabled }}://{{ .Values.ingress.host }}" + {{- if .Values.securityService.socialReturnOrigins }} + # Social-broker return-origin allowlist (FuzeFront#352) — see + # providers/authentik/socialReturnOrigins.ts. Additive only: this + # never changes which identity backend/tenant serves a request + # (unlike SECURITY_TENANTS above), only which allowlisted origin + # the FINISHED brokered-social-login redirect may target instead + # of FRONTEND_URL. Absent/empty (the default) = unchanged + # behaviour for every deployment that hasn't set this. + - name: SECURITY_SOCIAL_RETURN_ORIGINS + value: {{ .Values.securityService.socialReturnOrigins | join "," | quote }} + {{- end }} - name: PERMIT_API_KEY valueFrom: secretKeyRef: diff --git a/deploy/helm/fuzefront/values-prod.yaml b/deploy/helm/fuzefront/values-prod.yaml index e16fb35a6..79b2a4f79 100644 --- a/deploy/helm/fuzefront/values-prod.yaml +++ b/deploy/helm/fuzefront/values-prod.yaml @@ -116,6 +116,21 @@ securityService: # the existing Traefik + intra-fuzefront paths to fuzefront-security still work. networkPolicy: enabled: true + # Social-broker return-origin allowlist (FuzeFront#352). MendysRobotics' + # marketplace/live SPAs delegate sign-in entirely to this brokered social + # flow and proxy /api/v1/security/* SAME-ORIGIN to fuzefront-security (see + # the networkPolicy comment above — same mendys-prod integration, FuzeInfra + # #339). Without this, the broker's success/error redirect always lands on + # FRONTEND_URL (app.fuzefront.com) instead of the origin that started the + # flow, so the mendys SPA never receives its `?code=`. EXACT origins only — + # see providers/authentik/socialReturnOrigins.ts; this does NOT add either + # host to `tenants` above (that would additionally require enumerating every + # in-cluster caller's Host header or SECURITY_TENANTS' fail-closed rejection + # breaks provisioning-service/config-service/selection-list-service too — + # out of scope for this ask, see the module doc comment). + socialReturnOrigins: + - "https://marketplace.mendysrobotics.com" + - "https://live.mendysrobotics.com" applicationsService: enabled: true # serves /api/apps (MF app registry) — needed for MF apps to load diff --git a/deploy/helm/fuzefront/values.yaml b/deploy/helm/fuzefront/values.yaml index 5df9a1a36..7e593faaa 100644 --- a/deploy/helm/fuzefront/values.yaml +++ b/deploy/helm/fuzefront/values.yaml @@ -262,6 +262,19 @@ securityService: googleBrokered: false googleRedirectUri: "" tenants: [] + # Social-broker return-origin allowlist (FuzeFront#352) — see + # providers/authentik/socialReturnOrigins.ts. List of full origins (scheme + + # host, no path/trailing slash) the brokered social sign-in flow may + # redirect the browser back to INSTEAD OF FRONTEND_URL, for a consumer + # proxying /api/v1/security/* same-origin from its own domain. Rendered as + # SECURITY_SOCIAL_RETURN_ORIGINS (comma-joined) below; matched by EXACT Host + # header only — no wildcard/subdomain matching. Empty here (the default) = + # env var is not set at all, so every existing deployment is unaffected. + # This is deliberately NOT `tenants` above: adding an entry here changes + # only where the finished broker redirect lands, never which identity + # backend serves the request (see the module doc comment for why the two + # must stay separate). + socialReturnOrigins: [] nodeSelector: {} affinity: {} tolerations: [] From 48bf5ba9f9ec7dcb21171e531142e4b77e17e5b1 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 08:34:58 +0000 Subject: [PATCH 2/2] fix(governance): re-point 4 identifier exemptions after line drift MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_013tMciHkPE8To7V67CsgKKc --- governance/identifier-allowlist.txt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/governance/identifier-allowlist.txt b/governance/identifier-allowlist.txt index 75ed3926b..109710bfe 100644 --- a/governance/identifier-allowlist.txt +++ b/governance/identifier-allowlist.txt @@ -54,10 +54,10 @@ src backend/src/services/portalProvisioning.ts:250 # portal_provisionin # One-shot credential artifacts. Same family as the token/nonce/verifier targets # NON_ENTITY_ID_RE already excludes: they are consumed once, expire, and are # never referenced across a service boundary. -src backend/security/src/providers/authentik/AuthentikIdentityProvider.ts:662 # email_verifications row -src backend/security/src/providers/authentik/AuthentikIdentityProvider.ts:1082 # email_verifications row -src backend/security/src/providers/authentik/AuthentikIdentityProvider.ts:885 # mfa_recovery_codes row -src backend/security/src/providers/authentik/AuthentikIdentityProvider.ts:976 # password_resets row +src backend/security/src/providers/authentik/AuthentikIdentityProvider.ts:685 # email_verifications row +src backend/security/src/providers/authentik/AuthentikIdentityProvider.ts:1105 # email_verifications row +src backend/security/src/providers/authentik/AuthentikIdentityProvider.ts:908 # mfa_recovery_codes row +src backend/security/src/providers/authentik/AuthentikIdentityProvider.ts:999 # password_resets row # In-memory only: a Map key for a pending tool confirmation, never persisted and # never leaving the process.