From cb962fae88fa4a964d922441f2db0784f5182965 Mon Sep 17 00:00:00 2001 From: Mattsson <111893710+mattssonn@users.noreply.github.com> Date: Mon, 7 Sep 2026 16:29:29 +0200 Subject: [PATCH] fix(auth): resolve BankID confirmation and email-hook link hosts through the trusted-origin registry (#2380) * fix(auth): resolve BankID confirmation and email-hook link hosts through the trusted-origin registry The BankID confirmation mail built its /auth/callback link from the raw forwarded host and protocol; it is the one auth link GoTrue's redirect allowlist never sees, since the link is minted here and sent through Resend. The Send Email hook followed GoTrue's redirect_to verbatim: the webhook signature proves who sent the payload, not that every destination in it should be followed, and the GoTrue allowlist is a hand-configured glob. Both now resolve the destination through lib/domains/trusted-app-origin like every other auth link (canonical, this deployment's own Vercel hosts, or a registered brands.domain). Unknown, lookalike, credential-bearing, non-default-port and malformed destinations collapse to the canonical /auth/callback with no next path; a registered brand host over http is upgraded to https. Brand sender identity is taken from the RESOLVED host, so mail branding and link destination always agree. A brands-table read failure refuses instead of mailing a wrong-host link: the BankID helper returns step resolve_origin (signup rolls back, login re-send logs), the hook answers 500 so Supabase retries. Drops the proto parameter from the BankID helper; the resolver owns the scheme. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_0189cGB2YxptqVxBLJ2RkB5T * fix(auth): read the sender brand once, failure-aware, before minting or sending auth mail CodeRabbit: resolveTrustedAppOrigin could classify a brand host, then the separate resolveBrandByHost read for the sender could fail and return null, so a brand link went out with the platform sender; the BankID helper had already minted the magic link by then. Both sites now read the brand with resolveBrandResultByHost on the resolved host and refuse on a failed read for any non-canonical origin (BankID: step resolve_origin before generateLink; hook: 500 so Supabase retries). On the canonical origin a failed read is the platform sender either way, so mail still goes out. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_0189cGB2YxptqVxBLJ2RkB5T * fix(auth): treat a credential-bearing redirect_to as untrusted in the email hook Superagent P2: URL.origin drops userinfo, so a redirect_to with credentials on a served host passed the origin comparison and was cloned into the auth link with the credentials still in it. No flow of ours sends one; the hook now rejects any redirect_to carrying username or password outright and links to the canonical /auth/callback with no next path. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_0189cGB2YxptqVxBLJ2RkB5T --------- Co-authored-by: Claude Fable 5.1 --- DECISIONS.md | 1 + .../auth/email-hook/__tests__/route.test.ts | 131 ++++++++++++++- app/api/auth/email-hook/route.ts | 103 ++++++++++-- .../tic/__tests__/bankid-complete.test.ts | 1 - .../bankid-confirmation-mail.test.ts | 152 ++++++++++++++---- extensions/general/tic/index.ts | 2 - .../tic/lib/bankid-confirmation-mail.ts | 60 +++++-- 7 files changed, 376 insertions(+), 74 deletions(-) diff --git a/DECISIONS.md b/DECISIONS.md index 50c300c1..46b6af13 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1645,3 +1645,4 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [2026-09-07] Line breaks in line descriptions are collapsed by every single-line consumer (Peppol cbc:Name, accrual voucher text) and additionally in the SIE writer's quoted-text escaper: SIE is one record per line by spec, so the writer guards the format regardless of where the text came from. [2026-09-07] Enable Banking callbacks return to the initiating white-label host by recording the allowlisted request origin on the pending row and replaying the callback there, instead of extending the provider_otc handoff from PR #2305: the brand host already holds the session, so a /login?next= bounce on that host forwards straight back into the callback with cookies, needing one nullable column and no encrypted payload, no second table and no cron. The provider redirect URI stays canonical, so nothing changes in the Enable Banking console. Stripe, Gmail and cloud backup have the same shape but no partner-domain users yet; tracked as a follow-up issue rather than built speculatively. [2026-09-07] Stripe checkout and portal return URLs resolve through the existing resolveRequestAppOrigin allowlist (NEXT_PUBLIC_WHITELABEL_DOMAINS), not a DB brand lookup: it is the same trust boundary invites and email-change links already use, so one allowlist governs every host we redirect a browser to. Return paths stay fixed literals; no caller-supplied URL is accepted. Session-expiry and company-switch handling were left alone: the middleware already bounces to /login on the same host with the path preserved, and the webhook keys on company_id metadata. +[2026-09-07] BankID confirmation mail and the Send Email hook resolve their link host through lib/domains/trusted-app-origin instead of the raw forwarded host / GoTrue's redirect_to: the BankID mail is the one auth link GoTrue's redirect allowlist never sees (built here, sent via Resend), and the hook's signature proves the sender, not the destination, while the GoTrue allowlist is a hand-configured glob. Unknown, lookalike, credential-bearing, non-default-port and malformed destinations collapse to the canonical /auth/callback with no next path; a registered brand host over http is upgraded to https. Brand sender identity is resolved from the RESOLVED host so mail branding and link destination always agree. A brands-table read failure refuses (BankID: step resolve_origin, signup rolls back; hook: 500 so Supabase retries) rather than mailing a canonical link to a white-label user. Dropped from the audit's plan 7 as already in place after #2376: signup route, HTTPS enforcement, credential/port checks, recovery/invite/email-change coverage. diff --git a/app/api/auth/email-hook/__tests__/route.test.ts b/app/api/auth/email-hook/__tests__/route.test.ts index 072f0fc5..a944b80a 100644 --- a/app/api/auth/email-hook/__tests__/route.test.ts +++ b/app/api/auth/email-hook/__tests__/route.test.ts @@ -10,13 +10,20 @@ import type { Brand } from '@/lib/branding/resolve' vi.mock('@/lib/init', () => ({ ensureInitialized: vi.fn() })) -const resolveBrandByHostMock = vi.hoisted(() => vi.fn()) +const resolveBrandResultByHostMock = vi.hoisted(() => vi.fn()) vi.mock('@/lib/branding/resolve', () => ({ - resolveBrandByHost: resolveBrandByHostMock, + resolveBrandByHost: vi.fn(), + // The one registry read: the trusted-origin resolver classifies the + // redirect_to host through it, and the hook reads the sender brand from it. + resolveBrandResultByHost: (...args: unknown[]) => resolveBrandResultByHostMock(...args), // Imported by lib/email/brand-sender (not called on the hook path). resolveBrandForCompany: vi.fn(), })) +const CANONICAL = 'https://app.gnubok.se' +/** The one registered brand host in these tests; everything else is unknown. */ +const BRAND_HOST = 'app.siffra.se' + vi.mock('@/lib/branding/service', () => ({ getBranding: () => ({ appName: 'Accounted', appUrl: 'https://app.gnubok.se' }), })) @@ -87,15 +94,23 @@ function hookPayload(overrides?: { }) } +const ORIGINAL_APP_URL = process.env.NEXT_PUBLIC_APP_URL + beforeEach(() => { vi.clearAllMocks() process.env.SUPABASE_SEND_EMAIL_HOOK_SECRET = SECRET - resolveBrandByHostMock.mockResolvedValue(null) + process.env.NEXT_PUBLIC_APP_URL = CANONICAL + resolveBrandResultByHostMock.mockImplementation(async (host: string) => ({ + brand: host === BRAND_HOST ? makeBrand() : null, + lookupFailed: false, + })) sendEmailMock.mockResolvedValue({ success: true, messageId: 'msg-1' }) }) afterEach(() => { delete process.env.SUPABASE_SEND_EMAIL_HOOK_SECRET + if (ORIGINAL_APP_URL === undefined) delete process.env.NEXT_PUBLIC_APP_URL + else process.env.NEXT_PUBLIC_APP_URL = ORIGINAL_APP_URL }) describe('POST /api/auth/email-hook', () => { @@ -138,7 +153,6 @@ describe('POST /api/auth/email-hook', () => { }) it('brands the mail from the redirect_to host and rides the verified brand sender', async () => { - resolveBrandByHostMock.mockResolvedValue(makeBrand()) const res = await POST( signedRequest( hookPayload({ @@ -150,7 +164,7 @@ describe('POST /api/auth/email-hook', () => { ), ) expect(res.status).toBe(200) - expect(resolveBrandByHostMock).toHaveBeenCalledWith('app.siffra.se') + expect(resolveBrandResultByHostMock).toHaveBeenCalledWith('app.siffra.se') const options = sendEmailMock.mock.calls[0][0] expect(options.fromName).toBe('Siffra') @@ -163,7 +177,10 @@ describe('POST /api/auth/email-hook', () => { }) it('uses the via-fallback for a brand without a verified sender domain', async () => { - resolveBrandByHostMock.mockResolvedValue(makeBrand({ senderDomainStatus: 'pending' })) + resolveBrandResultByHostMock.mockResolvedValue({ + brand: makeBrand({ senderDomainStatus: 'pending' }), + lookupFailed: false, + }) await POST( signedRequest( hookPayload({ @@ -238,4 +255,106 @@ describe('POST /api/auth/email-hook', () => { const res = await POST(signedRequest(hookPayload())) expect(res.status).toBe(500) }) + + describe('redirect_to destinations (signature proves the sender, not the destination)', () => { + it.each([ + ['an unknown host', 'https://evil.example/auth/callback?next=/reset-password'], + ['a lookalike of a registered host', 'https://app.siffra.se.evil.example/auth/callback'], + ['a registered host on a non-default port', 'https://app.siffra.se:8443/auth/callback'], + ['a credential-bearing URL', 'https://app.siffra.se@evil.example/auth/callback'], + // URL.origin drops userinfo: the host alone would pass as trusted. + ['credentials on a registered host', 'https://evil.example@app.siffra.se/auth/callback?next=/x'], + ['credentials on the canonical host', 'https://user:pw@app.gnubok.se/auth/callback?next=/x'], + ['a malformed value', 'not a url'], + ])('links %s to the canonical callback without the requested path', async (_label, redirectTo) => { + const res = await POST( + signedRequest(hookPayload({ email_data: { redirect_to: redirectTo } })), + ) + expect(res.status).toBe(200) + + const options = sendEmailMock.mock.calls[0][0] + expect(options.text).toContain( + 'https://app.gnubok.se/auth/callback?token_hash=hash-1&type=recovery', + ) + expect(options.text).not.toContain('evil.example') + expect(options.text).not.toContain(':8443') + expect(options.text).not.toContain('next=') + // Canonical link means canonical sender: brand and destination agree. + expect(options.fromName).toBeUndefined() + expect(options.fromAddress).toBeUndefined() + }) + + it('upgrades http on a registered brand host to https and drops the requested path', async () => { + await POST( + signedRequest( + hookPayload({ + email_data: { redirect_to: 'http://app.siffra.se/auth/callback?next=/settings' }, + }), + ), + ) + const options = sendEmailMock.mock.calls[0][0] + expect(options.text).toContain( + 'https://app.siffra.se/auth/callback?token_hash=hash-1&type=recovery', + ) + expect(options.text).not.toContain('http://') + expect(options.text).not.toContain('next=') + expect(options.fromName).toBe('Siffra') + }) + + it('keeps the requested path on a registered brand host', async () => { + await POST( + signedRequest( + hookPayload({ + email_data: { redirect_to: 'https://app.siffra.se/auth/callback?next=%2Freset-password' }, + }), + ), + ) + const options = sendEmailMock.mock.calls[0][0] + expect(options.text).toContain( + 'https://app.siffra.se/auth/callback?next=%2Freset-password&token_hash=hash-1', + ) + expect(options.fromName).toBe('Siffra') + }) + + it('falls back to the canonical callback when redirect_to is missing', async () => { + await POST(signedRequest(hookPayload({ email_data: { redirect_to: undefined } }))) + const options = sendEmailMock.mock.calls[0][0] + expect(options.text).toContain('https://app.gnubok.se/auth/callback?token_hash=hash-1') + }) + + it('returns 500 without sending when the brand read fails after the origin resolved', async () => { + // First read (origin classification) succeeds, second (sender) fails: + // never platform-branded mail carrying a brand link. + resolveBrandResultByHostMock + .mockResolvedValueOnce({ brand: makeBrand(), lookupFailed: false }) + .mockResolvedValueOnce({ brand: null, lookupFailed: true }) + const res = await POST( + signedRequest( + hookPayload({ email_data: { redirect_to: 'https://app.siffra.se/auth/callback' } }), + ), + ) + expect(res.status).toBe(500) + expect(sendEmailMock).not.toHaveBeenCalled() + }) + + it('still sends canonical mail when the brand read fails on the canonical origin', async () => { + resolveBrandResultByHostMock.mockResolvedValue({ brand: null, lookupFailed: true }) + const res = await POST(signedRequest(hookPayload())) + expect(res.status).toBe(200) + const options = sendEmailMock.mock.calls[0][0] + expect(options.text).toContain('https://app.gnubok.se/auth/callback?next=%2Freset-password') + expect(options.fromName).toBeUndefined() + }) + + it('returns 500 without sending when the brand registry cannot be read', async () => { + resolveBrandResultByHostMock.mockResolvedValue({ brand: null, lookupFailed: true }) + const res = await POST( + signedRequest( + hookPayload({ email_data: { redirect_to: 'https://app.siffra.se/auth/callback' } }), + ), + ) + expect(res.status).toBe(500) + expect(sendEmailMock).not.toHaveBeenCalled() + }) + }) }) diff --git a/app/api/auth/email-hook/route.ts b/app/api/auth/email-hook/route.ts index 5ae22e44..4d449ee1 100644 --- a/app/api/auth/email-hook/route.ts +++ b/app/api/auth/email-hook/route.ts @@ -3,10 +3,15 @@ import { ensureInitialized } from '@/lib/init' import { createLogger } from '@/lib/logger' import { getEmailService } from '@/lib/email/service' import { getBranding } from '@/lib/branding/service' -import { resolveBrandByHost } from '@/lib/branding/resolve' +import { resolveBrandResultByHost } from '@/lib/branding/resolve' import { getSenderForBrand } from '@/lib/email/brand-sender' import { buildAuthEmail } from '@/lib/email/auth-templates' import { verifyStandardWebhookSignature } from '@/lib/email/standard-webhook' +import { + BrandLookupFailedError, + getCanonicalAppOrigin, + resolveTrustedAppOrigin, +} from '@/lib/domains/trusted-app-origin' // Loads the email extension so getEmailService() returns the Resend // implementation instead of the noop default. @@ -22,14 +27,21 @@ const log = createLogger('auth-email-hook') * sending auth mail itself and this endpoint sends every auth mail (signup * confirmation, recovery, magic link, invite, email change, reauthentication) * through the platform email service, branded per the requesting host: the - * brand is resolved from the redirect_to origin via resolveBrandByHost, so a + * brand is resolved from the trusted redirect_to origin, so a * reset requested on app.partner.se is sent in the partner's brand and links * back to app.partner.se. Unknown hosts get canonical platform mail. * * Unauthenticated by design (server-to-server): authenticity comes from the * Standard Webhooks signature, not a session, exactly like the Stripe and * Resend webhook routes. The raw body is verified byte-for-byte before - * parsing. This endpoint is availability-critical once the hook is enabled: + * parsing. The signature proves WHO sent the payload, not that every + * destination in it should be followed: redirect_to is GoTrue's already + * allowlisted referrer, but that allowlist is a glob configured by hand, so + * the origin is resolved here again through lib/domains/trusted-app-origin + * (canonical, this deployment's own Vercel hosts, or a registered brand + * domain). Anything else gets a canonical link, and the token never rides + * to a host this deployment does not serve. This endpoint is + * availability-critical once the hook is enabled: * any internal failure returns 500 so Supabase retries (up to 3 times within * a 5 second budget); success returns 200 {} fast. * @@ -71,8 +83,13 @@ interface SendEmailHookPayload { * /auth/callback consumes token_hash + type server-side and then honors the * `next` path. If redirect_to already points at /auth/callback (our client * flows do), its query (e.g. next=/reset-password) is preserved. + * + * `origin` is the already-trusted application origin; `redirectUrl` is the + * requested redirect_to only when it sits on that origin, else null (the + * link then lands on the origin's /auth/callback with no `next`). */ function buildActionUrl( + origin: string, redirectUrl: URL | null, tokenHash: string, actionType: string, @@ -82,7 +99,7 @@ function buildActionUrl( if (redirectUrl && redirectUrl.pathname === '/auth/callback') { url = new URL(redirectUrl.toString()) } else { - url = new URL('/auth/callback', redirectUrl ? redirectUrl.origin : getBranding().appUrl) + url = new URL('/auth/callback', origin) if (redirectUrl) { const next = redirectUrl.pathname + redirectUrl.search if (next && next !== '/') url.searchParams.set('next', next) @@ -93,6 +110,47 @@ function buildActionUrl( return url.toString() } +/** + * The requested redirect_to, kept only when its origin is one this + * deployment serves. The comparison is on the resolved origin, so an http + * link to a hosted domain, a lookalike host, a non-default port or a + * credential-bearing URL all collapse to the canonical /auth/callback. + * Throws BrandLookupFailedError when the brands table cannot be read. + */ +async function resolveRedirect( + requested: string | undefined, +): Promise<{ origin: string; redirectUrl: URL | null }> { + let requestedUrl: URL | null = null + if (requested) { + try { + requestedUrl = new URL(requested) + } catch { + requestedUrl = null + } + } + // URL.origin drops userinfo, so a credential-bearing redirect on a served + // host would pass the origin comparison and be cloned into the link with + // the credentials still in it. No flow of ours ever sends one: treat it as + // untrusted outright (canonical link, no next), never as a served host. + if (requestedUrl && (requestedUrl.username || requestedUrl.password)) { + log.warn('redirect_to carries credentials; linking to the canonical origin', { + host: requestedUrl.hostname, + }) + requestedUrl = null + } + const origin = await resolveTrustedAppOrigin(requestedUrl?.origin ?? null) + if (requestedUrl && requestedUrl.origin === origin) { + return { origin, redirectUrl: requestedUrl } + } + if (requestedUrl) { + // Hostname only: the URL may carry a query, never log the token side. + log.warn('redirect_to origin is not a served host; linking to the canonical origin', { + host: requestedUrl.hostname, + }) + } + return { origin, redirectUrl: null } +} + export async function POST(request: Request) { const secret = process.env.SUPABASE_SEND_EMAIL_HOOK_SECRET if (!secret) { @@ -126,16 +184,29 @@ export async function POST(request: Request) { return NextResponse.json({ error: 'Missing recipient' }, { status: 400 }) } - // Brand from the requesting host: redirect_to carries the tenant origin. - let redirectUrl: URL | null = null - if (emailData.redirect_to) { - try { - redirectUrl = new URL(emailData.redirect_to) - } catch { - redirectUrl = null - } + // Brand from the RESOLVED host: redirect_to carries the tenant origin, and + // the sender identity must match the host the link lands on. A lookup + // failure is a 500 so Supabase retries rather than sending a mail whose + // link would land on the wrong domain. + let resolved: Awaited> + try { + resolved = await resolveRedirect(emailData.redirect_to) + } catch (err) { + if (!(err instanceof BrandLookupFailedError)) throw err + log.error('brand lookup failed while resolving redirect_to', err, { host: err.host }) + return NextResponse.json({ error: 'Origin lookup failed' }, { status: 500 }) } - const brand = redirectUrl ? await resolveBrandByHost(redirectUrl.hostname) : null + const { origin, redirectUrl } = resolved + // A brand host whose row cannot be read right now (a second registry read + // can fail after the first succeeded) must not get platform-branded mail + // carrying a brand link: 500, Supabase retries. On the canonical origin a + // failed read is the platform sender either way, so it does not block. + const brandResult = await resolveBrandResultByHost(new URL(origin).hostname) + if (brandResult.lookupFailed && origin !== getCanonicalAppOrigin()) { + log.error('brand lookup failed for the resolved origin', undefined, { origin }) + return NextResponse.json({ error: 'Origin lookup failed' }, { status: 500 }) + } + const brand = brandResult.brand const sender = getSenderForBrand(brand) const appName = brand?.appName ?? getBranding().appName @@ -158,13 +229,13 @@ export async function POST(request: Request) { mails.push({ to: newEmail, actionType: 'email_change', - actionUrl: buildActionUrl(redirectUrl, emailData.token_hash, 'email_change'), + actionUrl: buildActionUrl(origin, redirectUrl, emailData.token_hash, 'email_change'), }) if (emailData.token_hash_new) { mails.push({ to: recipient, actionType: 'email_change_current', - actionUrl: buildActionUrl(redirectUrl, emailData.token_hash_new, 'email_change'), + actionUrl: buildActionUrl(origin, redirectUrl, emailData.token_hash_new, 'email_change'), }) } } else { @@ -174,7 +245,7 @@ export async function POST(request: Request) { mails.push({ to: recipient, actionType, - actionUrl: buildActionUrl(redirectUrl, emailData.token_hash, actionType), + actionUrl: buildActionUrl(origin, redirectUrl, emailData.token_hash, actionType), }) } diff --git a/extensions/general/tic/__tests__/bankid-complete.test.ts b/extensions/general/tic/__tests__/bankid-complete.test.ts index 31f8996f..aa5ec421 100644 --- a/extensions/general/tic/__tests__/bankid-complete.test.ts +++ b/extensions/general/tic/__tests__/bankid-complete.test.ts @@ -295,7 +295,6 @@ describe('POST /bankid/complete', () => { supabase: client, email: 'fresh@example.com', host: 'app.gnubok.se', - proto: 'https', }) expect(admin.deleteUser).not.toHaveBeenCalled() }) diff --git a/extensions/general/tic/__tests__/bankid-confirmation-mail.test.ts b/extensions/general/tic/__tests__/bankid-confirmation-mail.test.ts index b40dadc3..1e356f26 100644 --- a/extensions/general/tic/__tests__/bankid-confirmation-mail.test.ts +++ b/extensions/general/tic/__tests__/bankid-confirmation-mail.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, vi, beforeEach } from 'vitest' +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' import type { SupabaseClient } from '@supabase/supabase-js' const sendEmailMock = vi.hoisted(() => vi.fn()) @@ -6,9 +6,12 @@ vi.mock('@/lib/email/service', () => ({ getEmailService: () => ({ sendEmail: sendEmailMock, isConfigured: () => true }), })) -const resolveBrandByHostMock = vi.hoisted(() => vi.fn()) +const resolveBrandResultByHostMock = vi.hoisted(() => vi.fn()) vi.mock('@/lib/branding/resolve', () => ({ - resolveBrandByHost: resolveBrandByHostMock, + resolveBrandByHost: vi.fn(), + // The one registry read: the trusted-origin resolver classifies the + // request host through it, and the helper reads the sender brand from it. + resolveBrandResultByHost: (...args: unknown[]) => resolveBrandResultByHostMock(...args), // Imported by lib/email/brand-sender (not called on this path). resolveBrandForCompany: vi.fn(), })) @@ -22,6 +25,18 @@ import { sendBankIdSignupConfirmation, } from '../lib/bankid-confirmation-mail' +const CANONICAL = 'https://app.gnubok.se' +const BRAND_HOST = 'app.testbrand.example' +const TESTBRAND = { + appName: 'Testbrand', + domain: BRAND_HOST, + supportEmail: 'support@testbrand.example', + authEmailFrom: 'noreply@post.testbrand.example', + senderDomainStatus: 'verified', +} + +const ORIGINAL_APP_URL = process.env.NEXT_PUBLIC_APP_URL + function serviceClient(generateLinkResult: unknown) { const generateLink = vi.fn().mockResolvedValue(generateLinkResult) return { @@ -34,24 +49,25 @@ const LINK_OK = { data: { properties: { hashed_token: 'hashed-123' } }, error: n beforeEach(() => { vi.clearAllMocks() - resolveBrandByHostMock.mockResolvedValue(null) + process.env.NEXT_PUBLIC_APP_URL = CANONICAL + // Registry: only the test brand host is registered; everything else is + // unknown. Both resolvers read the same table. + resolveBrandResultByHostMock.mockImplementation(async (host: string) => ({ + brand: host === BRAND_HOST ? TESTBRAND : null, + lookupFailed: false, + })) sendEmailMock.mockResolvedValue({ success: true, messageId: 'm-1' }) }) +afterEach(() => { + if (ORIGINAL_APP_URL === undefined) delete process.env.NEXT_PUBLIC_APP_URL + else process.env.NEXT_PUBLIC_APP_URL = ORIGINAL_APP_URL +}) + describe('buildConfirmationUrl', () => { - it('lands on the originating host with the token_hash + magiclink verify pattern', () => { - expect(buildConfirmationUrl('app.siffra.se', 'https', 'tok')).toBe( - 'https://app.siffra.se/auth/callback?token_hash=tok&type=magiclink', - ) - }) - - it('defaults to https when the proxy did not forward a protocol', () => { - expect(buildConfirmationUrl('app.siffra.se', null, 'tok')).toMatch(/^https:\/\/app\.siffra\.se\//) - }) - - it('falls back to the canonical app URL without a host', () => { - expect(buildConfirmationUrl('', undefined, 'tok')).toBe( - 'https://app.gnubok.se/auth/callback?token_hash=tok&type=magiclink', + it('appends the token_hash + magiclink verify pattern to the resolved origin', () => { + expect(buildConfirmationUrl('https://app.testbrand.example', 'tok')).toBe( + 'https://app.testbrand.example/auth/callback?token_hash=tok&type=magiclink', ) }) }) @@ -64,7 +80,6 @@ describe('sendBankIdSignupConfirmation', () => { supabase, email: 'fresh@example.com', host: 'app.gnubok.se', - proto: 'https', }) expect(result).toEqual({ ok: true }) @@ -82,32 +97,103 @@ describe('sendBankIdSignupConfirmation', () => { expect(mail.fromAddress).toBeUndefined() }) - it('sends in the brand of the requesting host', async () => { - resolveBrandByHostMock.mockResolvedValue({ - appName: 'Siffra', - domain: 'app.siffra.se', - supportEmail: 'support@siffra.se', - authEmailFrom: 'noreply@post.siffra.se', - senderDomainStatus: 'verified', - }) + it('links to and sends in the brand of a registered requesting host', async () => { const { supabase } = serviceClient(LINK_OK) await sendBankIdSignupConfirmation({ supabase, email: 'fresh@example.com', - host: 'app.siffra.se', - proto: 'https', + host: BRAND_HOST, }) - expect(resolveBrandByHostMock).toHaveBeenCalledWith('app.siffra.se') + expect(resolveBrandResultByHostMock).toHaveBeenCalledWith(BRAND_HOST) const mail = sendEmailMock.mock.calls[0][0] - expect(mail.fromName).toBe('Siffra') - expect(mail.fromAddress).toBe('noreply@post.siffra.se') - expect(mail.replyTo).toBe('support@siffra.se') - expect(mail.text).toContain('https://app.siffra.se/auth/callback?token_hash=hashed-123') + expect(mail.fromName).toBe('Testbrand') + expect(mail.fromAddress).toBe('noreply@post.testbrand.example') + expect(mail.replyTo).toBe('support@testbrand.example') + expect(mail.text).toContain( + 'https://app.testbrand.example/auth/callback?token_hash=hashed-123', + ) expect(mail.html).not.toMatch(/accounted/i) }) + it.each([ + ['a spoofed unknown host', 'evil.example'], + ['a lookalike of a registered host', 'app.testbrand.example.evil.example'], + ['a registered host on a non-default port', 'app.testbrand.example:8443'], + ['a credential-bearing host', 'app.testbrand.example@evil.example'], + ['a missing host', ''], + ])('sends a canonical link for %s instead of following the header', async (_label, host) => { + const { supabase } = serviceClient(LINK_OK) + + const result = await sendBankIdSignupConfirmation({ + supabase, + email: 'fresh@example.com', + host, + }) + + expect(result).toEqual({ ok: true }) + const mail = sendEmailMock.mock.calls[0][0] + expect(mail.text).toContain( + 'https://app.gnubok.se/auth/callback?token_hash=hashed-123&type=magiclink', + ) + expect(mail.text).not.toContain('evil.example') + expect(mail.text).not.toContain(':8443') + // Canonical link means canonical sender: brand and destination agree. + expect(mail.fromName).toBeUndefined() + expect(mail.fromAddress).toBeUndefined() + }) + + it('refuses before minting a link when the brand registry cannot be read', async () => { + resolveBrandResultByHostMock.mockResolvedValue({ brand: null, lookupFailed: true }) + const { supabase, generateLink } = serviceClient(LINK_OK) + + const result = await sendBankIdSignupConfirmation({ + supabase, + email: 'fresh@example.com', + host: BRAND_HOST, + }) + + expect(result).toMatchObject({ ok: false, step: 'resolve_origin' }) + expect(generateLink).not.toHaveBeenCalled() + expect(sendEmailMock).not.toHaveBeenCalled() + }) + + it('refuses before minting a link when the brand read fails after the origin resolved', async () => { + // First read (origin classification) succeeds, second (sender) fails: + // never platform-branded mail carrying a brand link, and no token minted. + resolveBrandResultByHostMock + .mockResolvedValueOnce({ brand: TESTBRAND, lookupFailed: false }) + .mockResolvedValueOnce({ brand: null, lookupFailed: true }) + const { supabase, generateLink } = serviceClient(LINK_OK) + + const result = await sendBankIdSignupConfirmation({ + supabase, + email: 'fresh@example.com', + host: BRAND_HOST, + }) + + expect(result).toMatchObject({ ok: false, step: 'resolve_origin' }) + expect(generateLink).not.toHaveBeenCalled() + expect(sendEmailMock).not.toHaveBeenCalled() + }) + + it('still sends canonical mail when the brand read fails on the canonical origin', async () => { + resolveBrandResultByHostMock.mockResolvedValue({ brand: null, lookupFailed: true }) + const { supabase } = serviceClient(LINK_OK) + + const result = await sendBankIdSignupConfirmation({ + supabase, + email: 'fresh@example.com', + host: 'app.gnubok.se', + }) + + expect(result).toEqual({ ok: true }) + const mail = sendEmailMock.mock.calls[0][0] + expect(mail.text).toContain('https://app.gnubok.se/auth/callback?token_hash=hashed-123') + expect(mail.fromName).toBeUndefined() + }) + it('reports a generateLink failure without sending anything', async () => { const { supabase } = serviceClient({ data: null, error: { message: 'link boom', code: 'x' } }) diff --git a/extensions/general/tic/index.ts b/extensions/general/tic/index.ts index 23dfeed2..2330118c 100644 --- a/extensions/general/tic/index.ts +++ b/extensions/general/tic/index.ts @@ -105,7 +105,6 @@ async function refusePendingLogin( supabase, email: user.email, host: forwardedHost(request), - proto: request.headers.get('x-forwarded-proto'), }) if (!sent.ok) { log.warn('could not re-send bankid confirmation mail', { userId, step: sent.step }) @@ -1417,7 +1416,6 @@ export const ticExtension: Extension = { supabase, email: trimmedEmail!, host, - proto: request.headers.get('x-forwarded-proto'), }) if (!sent.ok) { diff --git a/extensions/general/tic/lib/bankid-confirmation-mail.ts b/extensions/general/tic/lib/bankid-confirmation-mail.ts index 5eb50b78..4aaa46d0 100644 --- a/extensions/general/tic/lib/bankid-confirmation-mail.ts +++ b/extensions/general/tic/lib/bankid-confirmation-mail.ts @@ -13,6 +13,13 @@ * pending identity tries to log in, which includes accounts created by the * old flow (confirmed by admin, address never proven). Verifying a magic link * confirms an unconfirmed address as a side effect, so both cases converge. + * + * This is the one auth mail GoTrue's redirect allowlist never sees: the link + * is built here and sent through the platform email service, so the host it + * points at is resolved through lib/domains/trusted-app-origin like every + * other auth link (canonical, this deployment's own Vercel hosts, or a + * registered brand domain). The raw Host header is an input, never the + * destination. */ import type { SupabaseClient } from '@supabase/supabase-js' @@ -20,7 +27,12 @@ import { getEmailService } from '@/lib/email/service' import { buildAuthEmail } from '@/lib/email/auth-templates' import { getSenderForBrand } from '@/lib/email/brand-sender' import { getBranding } from '@/lib/branding/service' -import { resolveBrandByHost } from '@/lib/branding/resolve' +import { resolveBrandResultByHost } from '@/lib/branding/resolve' +import { + BrandLookupFailedError, + getCanonicalAppOrigin, + resolveTrustedAppOrigin, +} from '@/lib/domains/trusted-app-origin' import { createLogger } from '@/lib/logger' const log = createLogger('tic/bankid-confirmation-mail') @@ -32,26 +44,18 @@ export interface SendBankIdConfirmationInput { email: string /** Forwarded host of the request, '' when unknown. */ host: string - /** Forwarded protocol of the request; defaults to https. */ - proto?: string | null } export type SendBankIdConfirmationResult = | { ok: true } - | { ok: false; step: 'generate_link' | 'send'; message?: string } + | { ok: false; step: 'resolve_origin' | 'generate_link' | 'send'; message?: string } /** - * Confirmation links must land on the ORIGINATING host (the brand mail - * resolves its brand from it), mirroring POST /api/auth/signup. With no host - * (direct invocation, tests) the canonical app URL is used. + * The verify link on an already-trusted application origin. Callers resolve + * the origin first (resolveTrustedAppOrigin), so this never sees a raw host. */ -export function buildConfirmationUrl( - host: string, - proto: string | null | undefined, - tokenHash: string, -): string { - const base = host ? `${proto || 'https'}://${host}` : getBranding().appUrl - const url = new URL('/auth/callback', base) +export function buildConfirmationUrl(origin: string, tokenHash: string): string { + const url = new URL('/auth/callback', origin) url.searchParams.set('token_hash', tokenHash) url.searchParams.set('type', 'magiclink') return url.toString() @@ -60,6 +64,31 @@ export function buildConfirmationUrl( export async function sendBankIdSignupConfirmation( input: SendBankIdConfirmationInput, ): Promise { + // Resolve the destination BEFORE minting a link: a token is only ever + // generated for a host this deployment is known to serve. An unknown host + // falls back to the canonical origin; an unreadable brands table refuses + // (a wrong-brand link whose session lands on a foreign domain is the + // failure this registry exists to prevent), and the caller rolls the + // signup back so the person can simply retry. + let origin: string + try { + origin = await resolveTrustedAppOrigin(input.host) + } catch (err) { + if (!(err instanceof BrandLookupFailedError)) throw err + return { ok: false, step: 'resolve_origin', message: err.message } + } + + // Sender identity from the RESOLVED host, read before the link is minted: + // a brand host whose brand row cannot be read right now must not get + // platform-branded mail carrying a brand link (a second registry read can + // fail after the first succeeded). On the canonical origin a failed read + // is the platform sender either way, so it does not block the mail. + const brandResult = await resolveBrandResultByHost(new URL(origin).hostname) + if (brandResult.lookupFailed && origin !== getCanonicalAppOrigin()) { + return { ok: false, step: 'resolve_origin', message: `brand lookup failed for ${origin}` } + } + const brand = brandResult.brand + const { data: link, error: linkError } = await input.supabase.auth.admin.generateLink({ type: 'magiclink', email: input.email, @@ -72,14 +101,13 @@ export async function sendBankIdSignupConfirmation( return { ok: false, step: 'generate_link', message: linkError?.message } } - const brand = input.host ? await resolveBrandByHost(input.host) : null const sender = getSenderForBrand(brand) const appName = brand?.appName ?? getBranding().appName const mail = buildAuthEmail({ actionType: 'bankid_signup', appName, - actionUrl: buildConfirmationUrl(input.host, input.proto, link.properties.hashed_token), + actionUrl: buildConfirmationUrl(origin, link.properties.hashed_token), }) const result = await getEmailService().sendEmail({