fix(auth): resolve auth-link hosts from the brands table, drop NEXT_PUBLIC_WHITELABEL_DOMAINS (#2376)
* fix(auth): resolve auth-link hosts from the brands table, drop NEXT_PUBLIC_WHITELABEL_DOMAINS Password reset, invite, email change and signup links now resolve the request host against brands.domain server-side. The env var was a second copy of that registry compiled into the browser; every new brand needed the row, the env var, the GoTrue allowlist and a redeploy, and two partners shipped with the env var stale, so their reset mails went out canonical-branded to the canonical host. - New POST /api/auth/password-reset: the login page no longer calls GoTrue directly, so the browser carries no domain list. - lib/domains/trusted-app-origin.ts is async and registry-backed; it also trusts this deployment's own VERCEL_URL / VERCEL_BRANCH_URL so previews keep sending links to themselves. - Signup shares the same resolver instead of following the raw host. - Docs and .env.example describe the single registry; GoTrue keeps the redirect allowlist as backstop (hosted: *.accounted.se wildcard). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NUNB7qjua8EUaJmZgfFscx * fix(auth): await the async origin resolver in the billing routes merged from main PR #2370 added resolveRequestAppOrigin callers in billing/checkout and billing/portal after this branch made the resolver async. Await them and move their tests from the removed env var to the brands mock; update the login source-assert test to the server-routed reset. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NUNB7qjua8EUaJmZgfFscx * fix(auth): refuse auth links on a failed brand lookup, keep local dev hosts, correct GoTrue allowlist docs Skeptic and CI findings on #2376, one pass: - A failed brands lookup now throws BrandLookupFailedError (TRANSIENT_ERROR, 503, retryable) instead of falling back to the canonical origin: a canonical link is the wrong-brand mail this PR removes. Password reset and email change answer 503 themselves; withRouteContext routes map the code. - A local canonical (dev) trusts other local hosts and ports on the same scheme, so lane servers on 3001-3003 confirm signups on themselves. - GoTrue matches the full redirect_to including the query and `*` stops at `.` and `/`: docs and decision line now prescribe https://*.accounted.se/auth/callback** and https://*.accounted.se/invite/**. - The Turnstile contract test asserts the server-routed reset forwards the captcha token (it still asserted the removed browser call). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NUNB7qjua8EUaJmZgfFscx --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5.1
parent
7a30f623ba
commit
d29a5bda14
@@ -1,62 +1,129 @@
|
||||
import { afterEach, beforeEach, describe, expect, it } from 'vitest'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const resolveBrandResultByHostMock = vi.hoisted(() => vi.fn())
|
||||
vi.mock('@/lib/branding/resolve', () => ({
|
||||
resolveBrandResultByHost: (...args: unknown[]) => resolveBrandResultByHostMock(...args),
|
||||
}))
|
||||
|
||||
import {
|
||||
BrandLookupFailedError,
|
||||
buildPasswordResetRedirectTo,
|
||||
getCanonicalAppOrigin,
|
||||
requestHost,
|
||||
resolveRequestAppOrigin,
|
||||
resolveTrustedAppOrigin,
|
||||
} from '../trusted-app-origin'
|
||||
|
||||
const ORIGINAL_APP_URL = process.env.NEXT_PUBLIC_APP_URL
|
||||
const ORIGINAL_WHITELABEL_DOMAINS = process.env.NEXT_PUBLIC_WHITELABEL_DOMAINS
|
||||
const REGISTERED = new Set(['portal.brand.test', 'books.partner.test'])
|
||||
|
||||
const ORIGINAL_ENV = {
|
||||
NEXT_PUBLIC_APP_URL: process.env.NEXT_PUBLIC_APP_URL,
|
||||
VERCEL_URL: process.env.VERCEL_URL,
|
||||
VERCEL_BRANCH_URL: process.env.VERCEL_BRANCH_URL,
|
||||
}
|
||||
|
||||
function restoreEnv() {
|
||||
for (const [key, value] of Object.entries(ORIGINAL_ENV)) {
|
||||
if (value === undefined) delete process.env[key]
|
||||
else process.env[key] = value
|
||||
}
|
||||
}
|
||||
|
||||
describe('trusted application origins', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
process.env.NEXT_PUBLIC_APP_URL = 'https://app.accounted.test'
|
||||
delete process.env.NEXT_PUBLIC_WHITELABEL_DOMAINS
|
||||
delete process.env.VERCEL_URL
|
||||
delete process.env.VERCEL_BRANCH_URL
|
||||
resolveBrandResultByHostMock.mockImplementation(async (host: string) => ({
|
||||
brand: REGISTERED.has(host) ? { domain: host } : null,
|
||||
lookupFailed: false,
|
||||
}))
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
if (ORIGINAL_APP_URL === undefined) delete process.env.NEXT_PUBLIC_APP_URL
|
||||
else process.env.NEXT_PUBLIC_APP_URL = ORIGINAL_APP_URL
|
||||
afterEach(restoreEnv)
|
||||
|
||||
if (ORIGINAL_WHITELABEL_DOMAINS === undefined) {
|
||||
delete process.env.NEXT_PUBLIC_WHITELABEL_DOMAINS
|
||||
} else {
|
||||
process.env.NEXT_PUBLIC_WHITELABEL_DOMAINS = ORIGINAL_WHITELABEL_DOMAINS
|
||||
}
|
||||
})
|
||||
|
||||
it('uses an exact registered white-label host over HTTPS', () => {
|
||||
process.env.NEXT_PUBLIC_WHITELABEL_DOMAINS = 'portal.brand.test, books.partner.test'
|
||||
|
||||
expect(resolveTrustedAppOrigin('https://portal.brand.test')).toBe(
|
||||
it('uses an exact registered brand host over HTTPS', async () => {
|
||||
expect(await resolveTrustedAppOrigin('https://portal.brand.test')).toBe(
|
||||
'https://portal.brand.test',
|
||||
)
|
||||
expect(resolveTrustedAppOrigin('PORTAL.BRAND.TEST.')).toBe(
|
||||
expect(await resolveTrustedAppOrigin('PORTAL.BRAND.TEST.')).toBe(
|
||||
'https://portal.brand.test',
|
||||
)
|
||||
expect(resolveBrandResultByHostMock).toHaveBeenCalledWith('portal.brand.test')
|
||||
})
|
||||
|
||||
it('rejects spoofed, credential, wildcard, and non-default-port hosts', () => {
|
||||
process.env.NEXT_PUBLIC_WHITELABEL_DOMAINS = 'portal.brand.test,*.wildcard.test'
|
||||
|
||||
it('rejects spoofed, credential, suffix, and non-default-port hosts', async () => {
|
||||
for (const candidate of [
|
||||
'https://portal.brand.test.attacker.test',
|
||||
'https://portal.brand.test@attacker.test',
|
||||
'https://child.wildcard.test',
|
||||
'https://child.portal.brand.test',
|
||||
'https://portal.brand.test:444',
|
||||
]) {
|
||||
expect(resolveTrustedAppOrigin(candidate), candidate).toBe(
|
||||
expect(await resolveTrustedAppOrigin(candidate), candidate).toBe(
|
||||
'https://app.accounted.test',
|
||||
)
|
||||
}
|
||||
})
|
||||
|
||||
it('falls back to the canonical origin when the request host is not registered', () => {
|
||||
expect(resolveTrustedAppOrigin('https://unregistered.test')).toBe(
|
||||
it('falls back to the canonical origin when the host is not registered', async () => {
|
||||
expect(await resolveTrustedAppOrigin('https://unregistered.test')).toBe(
|
||||
'https://app.accounted.test',
|
||||
)
|
||||
expect(await resolveTrustedAppOrigin(null)).toBe('https://app.accounted.test')
|
||||
})
|
||||
|
||||
it('does not consult the registry for the canonical host itself', async () => {
|
||||
expect(await resolveTrustedAppOrigin('app.accounted.test')).toBe(
|
||||
'https://app.accounted.test',
|
||||
)
|
||||
expect(resolveBrandResultByHostMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('refuses with BrandLookupFailedError when the brands lookup fails', async () => {
|
||||
resolveBrandResultByHostMock.mockResolvedValue({ brand: null, lookupFailed: true })
|
||||
|
||||
await expect(resolveTrustedAppOrigin('https://portal.brand.test')).rejects.toBeInstanceOf(
|
||||
BrandLookupFailedError,
|
||||
)
|
||||
await expect(resolveTrustedAppOrigin('https://portal.brand.test')).rejects.toMatchObject({
|
||||
code: 'TRANSIENT_ERROR',
|
||||
status: 503,
|
||||
})
|
||||
// The canonical host never consults the registry, so it is unaffected.
|
||||
expect(await resolveTrustedAppOrigin('app.accounted.test')).toBe('https://app.accounted.test')
|
||||
// Redirect-only callers opt into the canonical fallback explicitly.
|
||||
expect(
|
||||
await resolveTrustedAppOrigin('https://portal.brand.test', { onLookupFailure: 'canonical' }),
|
||||
).toBe('https://app.accounted.test')
|
||||
})
|
||||
|
||||
it('lets a local canonical trust other local hosts and ports on the same scheme', async () => {
|
||||
process.env.NEXT_PUBLIC_APP_URL = 'http://localhost:3000'
|
||||
|
||||
expect(await resolveTrustedAppOrigin('localhost:3001')).toBe('http://localhost:3001')
|
||||
expect(await resolveTrustedAppOrigin('http://127.0.0.1:3000')).toBe('http://127.0.0.1:3000')
|
||||
expect(await resolveTrustedAppOrigin('lane.localhost:3002')).toBe('http://lane.localhost:3002')
|
||||
expect(resolveBrandResultByHostMock).not.toHaveBeenCalled()
|
||||
|
||||
// A hosted canonical grants nothing to local hosts.
|
||||
process.env.NEXT_PUBLIC_APP_URL = 'https://app.accounted.test'
|
||||
expect(await resolveTrustedAppOrigin('localhost:3001')).toBe('https://app.accounted.test')
|
||||
})
|
||||
|
||||
it("trusts this deployment's own Vercel hostnames, but no other *.vercel.app", async () => {
|
||||
process.env.VERCEL_URL = 'erp-base-abc123-team.vercel.app'
|
||||
process.env.VERCEL_BRANCH_URL = 'erp-base-git-feature-team.vercel.app'
|
||||
|
||||
expect(await resolveTrustedAppOrigin('erp-base-abc123-team.vercel.app')).toBe(
|
||||
'https://erp-base-abc123-team.vercel.app',
|
||||
)
|
||||
expect(await resolveTrustedAppOrigin('https://erp-base-git-feature-team.vercel.app')).toBe(
|
||||
'https://erp-base-git-feature-team.vercel.app',
|
||||
)
|
||||
expect(await resolveTrustedAppOrigin('https://someone-else.vercel.app')).toBe(
|
||||
'https://app.accounted.test',
|
||||
)
|
||||
expect(resolveTrustedAppOrigin(null)).toBe('https://app.accounted.test')
|
||||
})
|
||||
|
||||
it('normalises the canonical URL to its origin and has a local safe fallback', () => {
|
||||
@@ -67,46 +134,53 @@ describe('trusted application origins', () => {
|
||||
expect(getCanonicalAppOrigin()).toBe('http://localhost:3000')
|
||||
})
|
||||
|
||||
it('validates the request URL and ignores a spoofed forwarded host', () => {
|
||||
process.env.NEXT_PUBLIC_WHITELABEL_DOMAINS = 'portal.brand.test'
|
||||
it('reads the forwarded host first, then Host, then the request URL', () => {
|
||||
expect(
|
||||
requestHost(
|
||||
new Request('https://internal/api/x', {
|
||||
headers: { host: 'internal', 'x-forwarded-host': 'portal.brand.test' },
|
||||
}),
|
||||
),
|
||||
).toBe('portal.brand.test')
|
||||
expect(
|
||||
requestHost(new Request('https://internal/api/x', { headers: { host: 'books.partner.test' } })),
|
||||
).toBe('books.partner.test')
|
||||
expect(requestHost(new Request('https://portal.brand.test/api/x'))).toBe('portal.brand.test')
|
||||
})
|
||||
|
||||
const trusted = new Request('https://portal.brand.test/api/company/members/invite', {
|
||||
headers: { 'x-forwarded-host': 'attacker.test' },
|
||||
})
|
||||
const spoofed = new Request('https://attacker.test/api/company/members/invite', {
|
||||
it('resolves a request through the registry and ignores an unregistered forwarded host', async () => {
|
||||
const registered = new Request('https://internal/api/company/members/invite', {
|
||||
headers: { 'x-forwarded-host': 'portal.brand.test' },
|
||||
})
|
||||
const spoofed = new Request('https://portal.brand.test/api/company/members/invite', {
|
||||
headers: { 'x-forwarded-host': 'attacker.test' },
|
||||
})
|
||||
|
||||
expect(resolveRequestAppOrigin(trusted)).toBe('https://portal.brand.test')
|
||||
expect(resolveRequestAppOrigin(spoofed)).toBe('https://app.accounted.test')
|
||||
expect(await resolveRequestAppOrigin(registered)).toBe('https://portal.brand.test')
|
||||
expect(await resolveRequestAppOrigin(spoofed)).toBe('https://app.accounted.test')
|
||||
})
|
||||
})
|
||||
|
||||
describe('password reset callback', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
process.env.NEXT_PUBLIC_APP_URL = 'https://app.accounted.test'
|
||||
process.env.NEXT_PUBLIC_WHITELABEL_DOMAINS = 'portal.brand.test'
|
||||
resolveBrandResultByHostMock.mockImplementation(async (host: string) => ({
|
||||
brand: REGISTERED.has(host) ? { domain: host } : null,
|
||||
lookupFailed: false,
|
||||
}))
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
if (ORIGINAL_APP_URL === undefined) delete process.env.NEXT_PUBLIC_APP_URL
|
||||
else process.env.NEXT_PUBLIC_APP_URL = ORIGINAL_APP_URL
|
||||
afterEach(restoreEnv)
|
||||
|
||||
if (ORIGINAL_WHITELABEL_DOMAINS === undefined) {
|
||||
delete process.env.NEXT_PUBLIC_WHITELABEL_DOMAINS
|
||||
} else {
|
||||
process.env.NEXT_PUBLIC_WHITELABEL_DOMAINS = ORIGINAL_WHITELABEL_DOMAINS
|
||||
}
|
||||
})
|
||||
|
||||
it('keeps a registered brand callback on the brand domain', () => {
|
||||
expect(buildPasswordResetRedirectTo('https://portal.brand.test')).toBe(
|
||||
it('keeps a registered brand callback on the brand domain', async () => {
|
||||
expect(await buildPasswordResetRedirectTo('portal.brand.test')).toBe(
|
||||
'https://portal.brand.test/auth/callback?next=/reset-password',
|
||||
)
|
||||
})
|
||||
|
||||
it('uses the allowlisted canonical callback for an unknown browser origin', () => {
|
||||
expect(buildPasswordResetRedirectTo('https://attacker.test')).toBe(
|
||||
it('uses the canonical callback for an unknown host', async () => {
|
||||
expect(await buildPasswordResetRedirectTo('attacker.test')).toBe(
|
||||
'https://app.accounted.test/auth/callback?next=/reset-password',
|
||||
)
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user