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 426c25881..0a9606f8e 100644 --- a/deploy/helm/fuzefront/values-prod.yaml +++ b/deploy/helm/fuzefront/values-prod.yaml @@ -178,6 +178,22 @@ securityService: # # key: AUTHENTIK_MENDYS_GOOGLE_CLIENT_SECRET) here. Unresolved — see # # the PR's open questions. + # 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 image: diff --git a/deploy/helm/fuzefront/values.yaml b/deploy/helm/fuzefront/values.yaml index d2bef6d3c..054cc834a 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: [] 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.