diff --git a/app/entry.server.test.ts b/app/entry.server.test.ts new file mode 100644 index 00000000..5069e812 --- /dev/null +++ b/app/entry.server.test.ts @@ -0,0 +1,79 @@ +import { describe, expect, it, vi } from 'vitest' + +// boot.server probes the database when it is evaluated; handleRequest only needs it to have run. +vi.mock('~/shared/infra/boot.server', () => ({})) + +vi.mock('~/shared/infra/logger.server', () => ({ + default: { warn: vi.fn(), error: vi.fn(), info: vi.fn() }, +})) + +vi.mock('~/i18n/paraglide/runtime', () => ({ + baseLocale: 'fr', + isLocale: (value: unknown) => value === 'fr' || value === 'en', + overwriteGetLocale: vi.fn(), +})) + +vi.mock('~/shared/utils/locale.server', () => ({ + resolveDocumentLocale: vi.fn(), +})) + +// Rendering is React's part: report the shell ready at once, so handleRequest answers. +vi.mock('react-dom/server', () => ({ + renderToPipeableStream: (_element: unknown, options: { onShellReady?: () => void; onAllReady?: () => void }) => { + queueMicrotask(() => (options.onShellReady ?? options.onAllReady)?.()) + return { pipe: (body: { end: () => void }) => body.end(), abort: () => {} } + }, +})) + +const { default: handleRequest } = await import('./entry.server') +const { resolveDocumentLocale } = await import('~/shared/utils/locale.server') + +describe('handleRequest', () => { + // Regression: a subdomain no congregation owns was a 500 on every unknown path, because the + // redirect was thrown from here. It has to come back from resolveDocumentLocale and be the answer. + it('answers with the redirect resolveDocumentLocale hands back instead of rendering', async () => { + const redirect = new Response(null, { status: 302, headers: { Location: '/congregation-not-found' } }) + vi.mocked(resolveDocumentLocale).mockResolvedValue({ redirect }) + const responseHeaders = new Headers() + + const response = await handleRequest( + new Request('https://inconnu.unitae.app/nope'), + 404, + responseHeaders, + {} as never, + {} as never, + ) + + expect(response).toBe(redirect) + expect(responseHeaders.has('Set-Cookie')).toBe(false) + }) + + it('remembers a resolved locale in the locale cookie', async () => { + vi.mocked(resolveDocumentLocale).mockResolvedValue({ locale: 'en', persist: true }) + + const response = await handleRequest( + new Request('https://lyon.unitae.app/login'), + 200, + new Headers(), + {} as never, + {} as never, + ) + + expect(response.headers.get('Set-Cookie')).toContain('PARAGLIDE_LOCALE=en;') + }) + + // A failed lookup renders in a fallback locale. Writing that one down replaced the visitor's own. + it('leaves the locale cookie alone when the locale is a fallback', async () => { + vi.mocked(resolveDocumentLocale).mockResolvedValue({ locale: 'fr', persist: false }) + + const response = await handleRequest( + new Request('https://lyon.unitae.app/login'), + 200, + new Headers(), + {} as never, + {} as never, + ) + + expect(response.headers.get('Set-Cookie')).toBeNull() + }) +}) diff --git a/app/entry.server.tsx b/app/entry.server.tsx index 7b28e79c..77d6bd34 100644 --- a/app/entry.server.tsx +++ b/app/entry.server.tsx @@ -8,7 +8,7 @@ import { renderToPipeableStream } from 'react-dom/server' import type { EntryContext, RouterContextProvider } from 'react-router' import { ServerRouter } from 'react-router' -import { baseLocale, isLocale, type Locale, overwriteGetLocale } from '~/i18n/paraglide/runtime' +import { baseLocale, type Locale, overwriteGetLocale } from '~/i18n/paraglide/runtime' import logger from '~/shared/infra/logger.server' import { resolveDocumentLocale } from '~/shared/utils/locale.server' @@ -28,13 +28,18 @@ export default async function handleRequest( _loadContext: RouterContextProvider, ) { const resolved = await resolveDocumentLocale(request) - // A subdomain no congregation owns: send the visitor to the page that says so, as the loaders do. + // A subdomain no congregation owns. The sign-in loaders and the session guard send it to + // /congregation-not-found themselves; this covers the documents that get here without either + // (public shell pages, unmatched paths). if ('redirect' in resolved) return resolved.redirect - const locale = isLocale(resolved.locale) ? resolved.locale : baseLocale + const { locale } = resolved // Set the PARAGLIDE_LOCALE cookie via HTTP header so the client-side - // cookie strategy can read it during hydration (before any JS executes) - responseHeaders.append('Set-Cookie', `PARAGLIDE_LOCALE=${locale}; Path=/; Max-Age=34560000; SameSite=Lax`) + // cookie strategy can read it during hydration (before any JS executes). + // A fallback locale is not written: it would replace the one the visitor already has. + if (resolved.persist) { + responseHeaders.append('Set-Cookie', `PARAGLIDE_LOCALE=${locale}; Path=/; Max-Age=34560000; SameSite=Lax`) + } return localeStore.run(locale, () => { return isbot(request.headers.get('user-agent') || '') diff --git a/app/features/authentication/server/session.server.test.ts b/app/features/authentication/server/session.server.test.ts index bd89f308..46e2c8ca 100644 --- a/app/features/authentication/server/session.server.test.ts +++ b/app/features/authentication/server/session.server.test.ts @@ -244,9 +244,9 @@ describe('verifySession', () => { expect(response.headers.get('Location')).toBe('/login?redirectTo=%2Fterritories%2F1%3Fx%3D2') }) - // Regression: a session outliving its congregation used to throw a plain Error out of the - // guard, which every authenticated page answered with a 500. - it("redirects to /login and drops the session when the account's congregation no longer exists", async () => { + // The account's congregation row is missing: a deletion racing the request. The guard used to let + // resolveCongregation's plain Error through, a 500 instead of the sign-in page. + it('sends an account whose congregation is gone to /login, without a return URL or its session', async () => { setSession({ userId: '42', sessionEpoch: '0' }) vi.mocked(db.userAccount.findUnique).mockResolvedValue(fakeUser as never) vi.mocked(resolveCongregationFromRequest).mockResolvedValue(null as never) diff --git a/app/features/authentication/server/session.server.ts b/app/features/authentication/server/session.server.ts index bf017ad7..7db8f63d 100644 --- a/app/features/authentication/server/session.server.ts +++ b/app/features/authentication/server/session.server.ts @@ -139,7 +139,9 @@ export async function verifySession(request: Request) { const congregation = await findCongregation(user.congregationId) if (congregation == null) { - // The cookie outlived its congregation: drop it and send the visitor to sign in again. + // No congregation behind this account. The foreign key forbids that as a lasting state, so a + // deletion is racing this request: drop the cookie and send the visitor to sign in, without a + // return URL. logger.warn(`verifySession: congregation ${user.congregationId} of userId ${user.id} no longer exists`) return redirectToLogin(session) } diff --git a/app/shared/domain/congregation.server.test.ts b/app/shared/domain/congregation.server.test.ts index 6dae73f9..d63574c2 100644 --- a/app/shared/domain/congregation.server.test.ts +++ b/app/shared/domain/congregation.server.test.ts @@ -20,6 +20,13 @@ beforeEach(() => { vi.resetAllMocks() }) +// Answers the slug lookup the way the table would: the row for that slug, or nothing. A test that +// reads the wrong slug from the host then gets no congregation instead of whatever was mocked. +function mockSlugLookup(rows: Record) { + vi.mocked(db.congregation.findUnique).mockImplementation(((args: { where: { slug?: string } }) => + Promise.resolve(rows[args.where.slug ?? ''] ?? null)) as never) +} + const baseCongregation = { id: 1, name: 'Congrégation Test', @@ -180,11 +187,20 @@ describe('findCongregation', () => { await expect(findCongregation(999)).resolves.toBeNull() }) - it('returns the same congregation as resolveCongregation when it exists', async () => { - vi.mocked(db.congregation.findUnique).mockResolvedValue(baseCongregation as never) + it('returns the mapped congregation when it exists', async () => { + vi.mocked(db.congregation.findUnique).mockResolvedValue({ + ...baseCongregation, + locale: 'en', + timezone: 'Europe/London', + } as never) - const found = await findCongregation(1) - expect(found).toEqual(await resolveCongregation(1)) + await expect(findCongregation(1)).resolves.toMatchObject({ + id: 1, + slug: 'test', + displayName: 'Congrégation Test', + locale: 'en', + timezone: 'Europe/London', + }) }) }) @@ -242,26 +258,55 @@ describe('resolveCongregationFromRequest', () => { }) }) - // Regression: the base domain itself was read as the slug "unitae.app" and redirected to - // /congregation-not-found (it serves /setup in production). - it('looks up no slug for the base domain itself', async () => { + // The self-hosting guide writes UNITAE_BASE_URL with its scheme: a subdomain has to resolve either way. + it.each([ + 'unitae.app', + 'https://unitae.app', + 'http://unitae.app', + 'https://unitae.app/', + 'https://unitae.app:8443', + 'https://Unitae.App', + ])('resolves a subdomain when UNITAE_BASE_URL is %s', async baseUrl => { + process.env.UNITAE_MULTI_TENANT = 'true' + process.env.UNITAE_BASE_URL = baseUrl + mockSlugLookup({ 'tenant-alpha': { id: 1, slug: 'tenant-alpha' } }) + + const result = await resolveCongregationFromRequest(makeRequest('https://tenant-alpha.unitae.app/')) + expect(result).toEqual({ id: 1, slug: 'tenant-alpha' }) + }) + + // Set but left empty, the variable matched no host at all: every subdomain was served as the + // platform root, with nothing to say why. + it('reads an empty UNITAE_BASE_URL as unset', async () => { + process.env.UNITAE_MULTI_TENANT = 'true' + process.env.UNITAE_BASE_URL = '' + mockSlugLookup({ 'tenant-alpha': { id: 1, slug: 'tenant-alpha' } }) + + const result = await resolveCongregationFromRequest(makeRequest('https://tenant-alpha.unitae.app/')) + expect(result).toEqual({ id: 1, slug: 'tenant-alpha' }) + }) + + // Regression: the base domain itself was read as the slug "unitae.app", so every lookup on it + // threw the /congregation-not-found redirect. + it('resolves no congregation for the base domain itself', async () => { process.env.UNITAE_MULTI_TENANT = 'true' process.env.UNITAE_BASE_URL = 'https://unitae.app' + // Whatever slug is read from the host would find this row: none may be read. + vi.mocked(db.congregation.findUnique).mockResolvedValue({ id: 9, slug: 'any-slug' } as never) vi.mocked(db.congregation.findFirst).mockResolvedValue(null as never) - const result = await resolveCongregationFromRequest(makeRequest('https://unitae.app/setup')) + const result = await resolveCongregationFromRequest(makeRequest('https://unitae.app/')) expect(result).toBeNull() - expect(db.congregation.findUnique).not.toHaveBeenCalled() }) it('treats a host that only ends with the base domain as a custom domain, not a subdomain', async () => { process.env.UNITAE_MULTI_TENANT = 'true' process.env.UNITAE_BASE_URL = 'unitae.app' + vi.mocked(db.congregation.findUnique).mockResolvedValue({ id: 9, slug: 'any-slug' } as never) vi.mocked(db.congregation.findFirst).mockResolvedValue({ id: 3, slug: 'lyon' } as never) const result = await resolveCongregationFromRequest(makeRequest('https://notunitae.app/')) expect(result).toEqual({ id: 3, slug: 'lyon' }) - expect(db.congregation.findUnique).not.toHaveBeenCalled() }) it('retourne null pour le domaine racine sans slug ni domaine personnalisé', async () => { @@ -298,24 +343,28 @@ describe('getBrandingName — deterministic single-tenant pick', () => { }) describe('getBrandingName — multi-tenant host', () => { - it('looks up no slug for the base domain itself', async () => { + it('brands the base domain as the platform, not as a congregation', async () => { process.env.UNITAE_MULTI_TENANT = 'true' process.env.UNITAE_BASE_URL = 'unitae.app' + // Whatever slug is read from the host would find this row: none may be read. + vi.mocked(db.congregation.findUnique).mockResolvedValue({ name: 'Slug Match', displayName: null } as never) vi.mocked(db.congregation.findFirst).mockResolvedValue(null as never) - await expect(getBrandingName(new Request('https://unitae.app/setup'))).resolves.toBe('Unitae') - expect(db.congregation.findUnique).not.toHaveBeenCalled() + await expect(getBrandingName(new Request('https://unitae.app/'))).resolves.toBe('Unitae') }) - it("brands a subdomain with its congregation's name", async () => { + it.each([ + 'unitae.app', + 'https://unitae.app', + 'http://unitae.app', + 'https://unitae.app/', + 'https://unitae.app:8443', + 'https://Unitae.App', + ])("brands a subdomain with its congregation's name when UNITAE_BASE_URL is %s", async baseUrl => { process.env.UNITAE_MULTI_TENANT = 'true' - process.env.UNITAE_BASE_URL = 'unitae.app' - vi.mocked(db.congregation.findUnique).mockResolvedValue({ name: 'Lyon Centre', displayName: null } as never) + process.env.UNITAE_BASE_URL = baseUrl + mockSlugLookup({ lyon: { name: 'Lyon Centre', displayName: null } }) await expect(getBrandingName(new Request('https://lyon.unitae.app/login'))).resolves.toBe('Lyon Centre') - expect(db.congregation.findUnique).toHaveBeenCalledWith({ - where: { slug: 'lyon' }, - select: { name: true, displayName: true }, - }) }) }) diff --git a/app/shared/domain/congregation.server.ts b/app/shared/domain/congregation.server.ts index 6f0dbab0..0fdb5195 100644 --- a/app/shared/domain/congregation.server.ts +++ b/app/shared/domain/congregation.server.ts @@ -1,6 +1,7 @@ import { redirect } from 'react-router' import { unscopedDb } from '~/shared/infra/db.server' +import { getBaseDomain } from '~/shared/utils/env.server' const DEFAULT_PLATFORM_NAME = 'Unitae' const DEFAULT_EMAIL_FROM = process.env.UNITAE_EMAIL_FROM ?? 'Unitae ' @@ -41,8 +42,10 @@ export async function resolveCongregation(congregationId: number): Promise { const congregation = await unscopedDb.congregation.findUnique({ @@ -91,20 +94,20 @@ export function getPlatformName(): string { /** * The congregation slug in a `.` hostname. `null` for the base domain itself — it * is the platform, not a congregation — and for any other host, including one that merely ends with - * the same letters (`notunitae.app`). + * the same letters (`notunitae.app`) or has nothing before the dot. */ function slugFromHostname(hostname: string): string | null { - const appBaseUrl = (process.env.UNITAE_BASE_URL ?? 'unitae.app').replace('https://', '').replace('http://', '') - const suffix = `.${appBaseUrl}` - return hostname.endsWith(suffix) ? hostname.slice(0, -suffix.length) : null + const suffix = `.${getBaseDomain()}` + return hostname.endsWith(suffix) ? hostname.slice(0, -suffix.length) || null : null } /** * Resolves the congregation matching the subdomain or custom domain from the request. * - * - Returns `null` in single-tenant mode, or for a host that is neither a subdomain of the base domain - * nor a congregation's custom domain (the base domain itself among them). - * - Redirects to `/congregation-not-found` if a slug is present but doesn't match any congregation. + * - Returns `null` in single-tenant mode, and for any host that is neither `.` + * nor a congregation's custom domain. That includes the base domain itself. + * - Throws a redirect to `/congregation-not-found` for `.` when no congregation + * has that slug. The custom-domain lookup is not tried in that case. */ export async function resolveCongregationFromRequest(request: Request): Promise<{ id: number; slug: string } | null> { if (process.env.UNITAE_MULTI_TENANT !== 'true') return null diff --git a/app/shared/utils/env.server.test.ts b/app/shared/utils/env.server.test.ts index 5ad11da8..4a24a81e 100644 --- a/app/shared/utils/env.server.test.ts +++ b/app/shared/utils/env.server.test.ts @@ -4,7 +4,14 @@ vi.mock('~/shared/infra/logger.server', () => ({ default: { warn: vi.fn(), info: vi.fn(), error: vi.fn() }, })) -const ENV_KEYS = ['DB_URL', 'UNITAE_SESSION_SECRET', 'DB_RUNTIME_URL', 'UNITAE_CRON_SECRET'] as const +const ENV_KEYS = [ + 'DB_URL', + 'UNITAE_SESSION_SECRET', + 'DB_RUNTIME_URL', + 'UNITAE_CRON_SECRET', + 'UNITAE_BASE_URL', + 'UNITAE_MULTI_TENANT', +] as const describe('env.server', () => { const original: Record = {} @@ -166,4 +173,44 @@ describe('env.server', () => { expect(getSessionSecrets()).toEqual([]) }) }) + + describe('getBaseDomain', () => { + it.each([ + ['unitae.example.com', 'unitae.example.com'], + ['https://unitae.example.com', 'unitae.example.com'], + ['http://unitae.example.com', 'unitae.example.com'], + ['https://unitae.example.com/', 'unitae.example.com'], + ['https://unitae.example.com:8443', 'unitae.example.com'], + ['https://Unitae.Example.com', 'unitae.example.com'], + ])('reads the hostname out of %s', async (value, hostname) => { + const { getBaseDomain } = await import('./env.server') + process.env.UNITAE_BASE_URL = value + expect(getBaseDomain()).toBe(hostname) + }) + + it('falls back to unitae.app when the var is unset', async () => { + const { getBaseDomain } = await import('./env.server') + expect(getBaseDomain()).toBe('unitae.app') + }) + + it.each(['', 'not a host'])('falls back to unitae.app when the var is %j', async value => { + const { getBaseDomain } = await import('./env.server') + process.env.UNITAE_BASE_URL = value + expect(getBaseDomain()).toBe('unitae.app') + }) + }) + + // A value that names no host leaves subdomains resolved against the default domain. Boot reports + // it and carries on: an instance that only serves custom domains runs fine without one. + it.each([ + '', + 'not a host', + ])('validateEnv does not refuse a multi-tenant boot when UNITAE_BASE_URL is %j', async value => { + const { validateEnv } = await import('./env.server') + process.env.DB_URL = 'postgresql://localhost/db' + process.env.UNITAE_SESSION_SECRET = 'a-sufficiently-long-test-session-secret-value' + process.env.UNITAE_MULTI_TENANT = 'true' + process.env.UNITAE_BASE_URL = value + expect(() => validateEnv()).not.toThrow() + }) }) diff --git a/app/shared/utils/env.server.ts b/app/shared/utils/env.server.ts index 9868c8bb..0036120b 100644 --- a/app/shared/utils/env.server.ts +++ b/app/shared/utils/env.server.ts @@ -2,6 +2,7 @@ import logger from '~/shared/infra/logger.server' const MIN_SESSION_SECRET_LENGTH = 32 const PLACEHOLDER_SESSION_SECRETS = ['change-me-with-a-real-secret-key'] +const DEFAULT_BASE_DOMAIN = 'unitae.app' // Fail closed: treat anything that is not an explicit development/test environment (including an // unset or misspelled NODE_ENV) as production, mirroring the RLS guard (see rls-guard.server.ts). @@ -32,6 +33,33 @@ export function getSessionSecrets(): string[] { .filter(Boolean) } +function parseHostname(value: string): string | null { + try { + return new URL(value.includes('://') ? value : `https://${value}`).hostname || null + } catch { + return null + } +} + +// The hostname in UNITAE_BASE_URL, whether it is written with a scheme, a port or a trailing slash +// or without. Non-throwing for the same reason as getSessionSecrets: it runs on every multi-tenant +// request, so validateEnv() reports a value that names no host once, at boot. +export function getBaseDomain(): string { + return parseHostname(process.env.UNITAE_BASE_URL || DEFAULT_BASE_DOMAIN) ?? DEFAULT_BASE_DOMAIN +} + +// Reported, not refused: an instance that only serves custom domains runs fine without a base +// domain, so a boot failure here would break setups that work. +function warnOnUnusableBaseUrl() { + const baseUrl = process.env.UNITAE_BASE_URL + if (process.env.UNITAE_MULTI_TENANT !== 'true' || baseUrl === undefined || parseHostname(baseUrl) != null) return + + logger.warn( + `UNITAE_BASE_URL names no host: congregation subdomains are resolved against ${DEFAULT_BASE_DOMAIN}. ` + + 'Set it to your root domain, e.g. https://unitae.example.com.', + ) +} + // Enforces that every configured session secret (current + any rotated previous ones) is long // enough and is not the shipped placeholder. Fails closed in production; warns in development so // local setups keep working. Presence is checked first via requireEnv. @@ -77,4 +105,6 @@ export function validateEnv() { if (!process.env.UNITAE_CRON_SECRET) { logger.warn('UNITAE_CRON_SECRET is not set. Cron endpoints will reject all requests.') } + + warnOnUnusableBaseUrl() } diff --git a/app/shared/utils/locale.server.test.ts b/app/shared/utils/locale.server.test.ts index c3192f2f..c43d48b1 100644 --- a/app/shared/utils/locale.server.test.ts +++ b/app/shared/utils/locale.server.test.ts @@ -20,6 +20,11 @@ vi.mock('~/shared/infra/logger.server', () => ({ default: { warn: vi.fn(), error: vi.fn(), info: vi.fn() }, })) +vi.mock('~/i18n/paraglide/runtime', () => ({ + baseLocale: 'fr', + isLocale: (value: unknown) => value === 'fr' || value === 'en', +})) + const { resolveDocumentLocale, resolveLocaleFromRequest } = await import('./locale.server') const { getSession } = await import('~/features/authentication/server/session.server') const { unscopedDb: db } = await import('~/shared/infra/db.server') @@ -30,8 +35,8 @@ beforeEach(() => { delete process.env.UNITAE_MULTI_TENANT }) -function makeRequest(url = 'http://localhost:5173/board') { - return new Request(url) +function makeRequest(url = 'http://localhost:5173/board', headers: Record = {}) { + return new Request(url, { headers }) } function mockSession(userId: number | typeof NaN) { @@ -133,8 +138,8 @@ describe('resolveLocaleFromRequest', () => { }) }) -// Regression: a session whose congregation was deleted threw out of the locale lookup, which runs -// for every page, so that visitor got a 500 everywhere instead of the sign-in page. +// The same race as in verifySession: the account's congregation row is missing. resolveCongregation's +// Error used to escape the locale lookup, which runs for every document. describe("resolveLocaleFromRequest — the session's congregation no longer exists", () => { it("falls through to the request's congregation", async () => { mockSession(42) @@ -147,18 +152,25 @@ describe("resolveLocaleFromRequest — the session's congregation no longer exis }) }) -// resolveDocumentLocale runs in entry.server's handleRequest, after the loaders: a Response thrown -// there is not turned into the response, React Router re-renders, the lookup throws again and the -// visitor gets the last-resort 500. Every outcome has to come back as a value. +// resolveDocumentLocale must never throw: its JSDoc says what a throw costs in entry.server. describe('resolveDocumentLocale', () => { const notFoundRedirect = () => new Response(null, { status: 302, headers: { Location: '/congregation-not-found' } }) - it("returns the request's locale", async () => { + it('returns the resolved locale, to be remembered', async () => { mockSession(42) vi.mocked(db.userAccount.findUnique).mockResolvedValue({ congregationId: 1 } as never) vi.mocked(findCongregation).mockResolvedValue({ locale: 'en' } as never) - await expect(resolveDocumentLocale(makeRequest())).resolves.toEqual({ locale: 'en' }) + await expect(resolveDocumentLocale(makeRequest())).resolves.toEqual({ locale: 'en', persist: true }) + }) + + // Congregation.locale is a free-text column: what comes out of it is not always a locale the app ships. + it('renders in the base locale when the congregation holds a locale the app does not ship', async () => { + mockSession(42) + vi.mocked(db.userAccount.findUnique).mockResolvedValue({ congregationId: 1 } as never) + vi.mocked(findCongregation).mockResolvedValue({ locale: 'de' } as never) + + await expect(resolveDocumentLocale(makeRequest())).resolves.toEqual({ locale: 'fr', persist: true }) }) // Regression: any page on a subdomain no congregation owns, and any unknown path there, was a 500. @@ -180,14 +192,44 @@ describe('resolveDocumentLocale', () => { await expect( resolveDocumentLocale(makeRequest('https://inconnu.unitae.app/congregation-not-found')), - ).resolves.toEqual({ locale: 'fr' }) + ).resolves.toEqual({ locale: 'fr', persist: false }) }) - it('falls back to the default locale when the lookup fails', async () => { + it('falls back to the base locale when the lookup fails, without remembering it', async () => { mockSession(42) vi.mocked(db.userAccount.findUnique).mockRejectedValue(new Error('connection refused')) - await expect(resolveDocumentLocale(makeRequest())).resolves.toEqual({ locale: 'fr' }) + await expect(resolveDocumentLocale(makeRequest())).resolves.toEqual({ locale: 'fr', persist: false }) + }) + + // The fallback is nobody's locale. Written to the locale cookie, it switched an English + // congregation's member to French until their next full page load. + it("keeps the visitor's own locale when the lookup fails", async () => { + mockSession(42) + vi.mocked(db.userAccount.findUnique).mockRejectedValue(new Error('connection refused')) + const request = makeRequest(undefined, { Cookie: 'theme=dark; PARAGLIDE_LOCALE=en' }) + + await expect(resolveDocumentLocale(request)).resolves.toEqual({ locale: 'en', persist: false }) + }) + + it('ignores a locale cookie the app does not ship', async () => { + mockSession(42) + vi.mocked(db.userAccount.findUnique).mockRejectedValue(new Error('connection refused')) + const request = makeRequest(undefined, { Cookie: 'PARAGLIDE_LOCALE=de' }) + + await expect(resolveDocumentLocale(request)).resolves.toEqual({ locale: 'fr', persist: false }) + }) + + // Only a redirect may be answered as one. Any other Response thrown by the lookup would have + // been sent to the visitor as the page. + it('falls back instead of answering with a thrown Response that is not a redirect', async () => { + mockSession(NaN) + vi.mocked(resolveCongregationFromRequest).mockRejectedValue(new Response('forbidden', { status: 403 })) + + await expect(resolveDocumentLocale(makeRequest('https://inconnu.unitae.app/nope'))).resolves.toEqual({ + locale: 'fr', + persist: false, + }) }) }) diff --git a/app/shared/utils/locale.server.ts b/app/shared/utils/locale.server.ts index d9acc949..0e83ba24 100644 --- a/app/shared/utils/locale.server.ts +++ b/app/shared/utils/locale.server.ts @@ -1,9 +1,11 @@ import { getSession } from '~/features/authentication/server/session.server' +import { baseLocale, isLocale, type Locale } from '~/i18n/paraglide/runtime' import { findCongregation, resolveCongregationFromRequest } from '~/shared/domain/congregation.server' import { unscopedDb } from '~/shared/infra/db.server' import logger from '~/shared/infra/logger.server' const DEFAULT_LOCALE = 'fr' +const LOCALE_COOKIE = /(?:^|;\s*)PARAGLIDE_LOCALE=([^;]+)/ export async function resolveLocaleFromRequest(request: Request): Promise { // 1. Try session → user → congregation → locale (authenticated users) @@ -17,7 +19,7 @@ export async function resolveLocaleFromRequest(request: Request): Promise { try { - return { locale: await resolveLocaleFromRequest(request) } + const locale = await resolveLocaleFromRequest(request) + return { locale: isLocale(locale) ? locale : baseLocale, persist: true } } catch (error) { - if (error instanceof Response) { - return redirectsToItself(request, error) ? { locale: DEFAULT_LOCALE } : { redirect: error } + if (!isRedirect(error)) { + const { host, pathname } = new URL(request.url) + logger.error('resolveDocumentLocale: locale lookup failed, rendering in a fallback locale', { + // A Response has no enumerable fields: logged as it is, it would read `{}`. + error: error instanceof Response ? `Response ${error.status}` : error, + host, + path: pathname, + }) + } else if (!redirectsToItself(request, error)) { + return { redirect: error } } - logger.error('resolveDocumentLocale: locale lookup failed, rendering in the default locale', { error }) - return { locale: DEFAULT_LOCALE } + return { locale: localeFromCookie(request) ?? baseLocale, persist: false } } } -function redirectsToItself(request: Request, response: Response): boolean { - const location = response.headers.get('Location') - if (location == null) return false +function isRedirect(error: unknown): error is Response { + return error instanceof Response && error.status >= 300 && error.status < 400 && error.headers.has('Location') +} + +function redirectsToItself(request: Request, redirect: Response): boolean { const current = new URL(request.url) - return new URL(location, current).pathname === current.pathname + return new URL(redirect.headers.get('Location') ?? '', current).pathname === current.pathname +} + +function localeFromCookie(request: Request): Locale | null { + const value = LOCALE_COOKIE.exec(request.headers.get('Cookie') ?? '')?.[1] + return isLocale(value) ? value : null } diff --git a/app/shell/livez.test.ts b/app/shell/livez.test.ts index efe5e746..89c4e9b3 100644 --- a/app/shell/livez.test.ts +++ b/app/shell/livez.test.ts @@ -3,7 +3,7 @@ import { describe, expect, it } from 'vitest' import { loader } from './livez' describe('livez loader', () => { - it('answers 200 ok without a cacheable body', async () => { + it('answers 200 ok and forbids caching', async () => { const response = loader() expect(response.status).toBe(200) diff --git a/app/shell/livez.tsx b/app/shell/livez.tsx index 5e189a54..33df1d2a 100644 --- a/app/shell/livez.tsx +++ b/app/shell/livez.tsx @@ -1,7 +1,7 @@ // Liveness: answers as long as the server process can serve a request, and checks nothing else. // /health also needs PostgreSQL and Redis; as a liveness probe it would restart every pod at once -// when either one blips, which brings neither back. Use /health for readiness, this for liveness -// and startup. +// when either one stays unreachable for a few probe periods, which brings neither back. Use /health +// for readiness, this for liveness and startup. export function loader() { return new Response('ok', { status: 200, diff --git a/app/tests/e2e/health.spec.ts b/app/tests/e2e/health.spec.ts index 9737ecdd..eacc5eff 100644 --- a/app/tests/e2e/health.spec.ts +++ b/app/tests/e2e/health.spec.ts @@ -6,4 +6,10 @@ test.describe('Health check', () => { // We just verify the server is running and responds expect(response.status()).toBeLessThan(500) }) + + test('the liveness endpoint answers', async ({ request }) => { + const response = await request.get('/livez') + expect(response.status()).toBe(200) + expect(await response.text()).toBe('ok') + }) }) diff --git a/docs/self-hosting/health-monitoring.md b/docs/self-hosting/health-monitoring.md index 4e79a6d0..dc258491 100644 --- a/docs/self-hosting/health-monitoring.md +++ b/docs/self-hosting/health-monitoring.md @@ -20,8 +20,9 @@ Checks nothing but the web process itself: it answers as long as the server can - **200 OK** — `text/plain` body `ok`, `Cache-Control: no-store` Use it for liveness and startup probes, and `/health` for readiness. A liveness probe on `/health` -restarts every web pod at once when PostgreSQL or Redis is briefly unreachable, which brings neither -back; readiness only takes the pods out of the load balancer until the dependencies answer again. +restarts every web pod at once when PostgreSQL or Redis stays unreachable for a few probe periods, +which brings neither back; readiness only takes the pods out of the load balancer until the +dependencies answer again. Neither endpoint needs to be public: the orchestrator reaches them on the container port. @@ -39,13 +40,23 @@ services: web: # ... healthcheck: - test: ["CMD", "curl", "-sf", "http://localhost:8080/health"] + # The runtime image has no curl: Node's own fetch does the probe. + test: + [ + 'CMD', + 'node', + '-e', + "fetch('http://localhost:8080/health').then(r => process.exit(r.ok ? 0 : 1)).catch(() => process.exit(1))", + ] interval: 30s - timeout: 5s + timeout: 10s retries: 3 - start_period: 10s ``` +Compose reports this status and uses it to order `depends_on`; it does not restart an unhealthy +container. If your setup does (Docker Swarm, an autoheal sidecar), probe `/livez` instead, for the +reason given above. + ### Kubernetes probes ```yaml @@ -64,7 +75,6 @@ readinessProbe: httpGet: path: /health port: 8080 - initialDelaySeconds: 5 periodSeconds: 10 ```