From c01c750436eb8959122eb8681e55e4f6c7e5c6d2 Mon Sep 17 00:00:00 2001 From: Aravind Date: Thu, 17 Sep 2026 19:43:57 +0530 Subject: [PATCH] fix(auth): handle a missing AUTH_SECRET instead of dying mid sign-in MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `storeTokens()` and `createSessionToken()` ran outside any catch. Both read AUTH_SECRET — one derives the AES key that encrypts the platform bundle, the other signs the identity cookie — and in production both refuse to fall back to the development key. So an unset AUTH_SECRET threw after the credentials had already been accepted upstream, and the browser got a bare 500 on a sign-in that was entirely valid. The status was the smaller half. The upstream session minted moments earlier by `authApi.login` was ORPHANED: a live refresh token, issued to somebody who did not end up logged in, left to expire on its own. The platform-admin branch a few lines above already revokes for precisely this reason — declining because the server is broken is no different from declining because the account is wrong — so this now revokes too, best-effort, on the same terms. It fails closed. No cookie is set on this path, so a half-configured server cannot hand out a session it will be unable to verify on the next request. ConfigError moves to src/shared/errors/configError.ts because its throwers now span two runtimes: apiClient and tokenStore are server-only, while sessionToken is reached from src/proxy.ts, which Next compiles for Edge. Declaring it in apiClient would have dragged the whole platform client, `server-only` guard and all, into the proxy bundle to name one class. The new module imports nothing. Verified by exercising both functions directly: with AUTH_SECRET unset under NODE_ENV=production, sealTokens and createSessionToken each raise ConfigError rather than a bare Error; with it set, both succeed. Co-Authored-By: Claude Opus 5 (1M context) --- src/app/api/auth/login/route.ts | 71 ++++++++++++++++++---- src/features/auth/services/sessionToken.ts | 3 +- src/features/auth/services/tokenStore.ts | 3 +- src/services/api/apiClient.ts | 26 ++------ src/shared/errors/configError.ts | 29 +++++++++ src/shared/services/bff.ts | 3 +- 6 files changed, 99 insertions(+), 36 deletions(-) create mode 100644 src/shared/errors/configError.ts diff --git a/src/app/api/auth/login/route.ts b/src/app/api/auth/login/route.ts index 347c25b..4ea0f43 100644 --- a/src/app/api/auth/login/route.ts +++ b/src/app/api/auth/login/route.ts @@ -1,7 +1,8 @@ import {NextResponse} from 'next/server'; import type {NextRequest} from 'next/server'; import {authApi} from '@/services/api/authApi'; -import {ConfigError, UpstreamError} from '@/services/api/apiClient'; +import {UpstreamError} from '@/services/api/apiClient'; +import {ConfigError} from '@/shared/errors/configError'; import { LOGIN_ERROR_PARAM, type LoginErrorCode, @@ -215,20 +216,64 @@ export async function POST(req: NextRequest) { ); } - await storeTokens(bundle); - + /** + * Minting the local session, which is where AUTH_SECRET is first read. + * + * Both steps below need it — storeTokens ENCRYPTS the platform bundle with a + * key derived from it, createSessionToken SIGNS the identity cookie with it — + * and in production both refuse to fall back to the development key. They ran + * outside any catch, so an unset AUTH_SECRET surfaced as a bare 500 from a + * sign-in whose credentials were perfectly good, with nothing in the response + * to say which of the two required variables was missing. + * + * Worse than the status: the upstream session minted moments ago by + * `authApi.login` was ORPHANED. A live refresh token, issued to somebody who + * did not get logged in, left to expire on its own. The platform-admin branch + * above already revokes for exactly this reason; declining because the server + * is broken is no different from declining because the account is wrong. + * + * Fails closed: no cookie is set, so a half-configured server cannot hand out + * a session it is unable to verify on the next request. + */ const user = toAuthUser(bundle.user); const maxAge = rememberMe ? REMEMBERED_MAX_AGE_SECONDS : SESSION_MAX_AGE_SECONDS; - const sessionCookie = createSessionToken( - { - sub: user.id, - email: user.email, - name: user.name, - role: user.role, - organisation: user.organisation, - }, - maxAge, - ); + + let sessionCookie: string; + try { + await storeTokens(bundle); + sessionCookie = createSessionToken( + { + sub: user.id, + email: user.email, + name: user.name, + role: user.role, + organisation: user.organisation, + }, + maxAge, + ); + } catch (err) { + if (!(err instanceof ConfigError)) throw err; + console.error('[loyaly] configuration error:', err.message); + + try { + await authApi.logout(bundle.access_token); + } catch { + /* best-effort, exactly as in the platform-admin branch above */ + } + + const code: LoginErrorCode = 'misconfigured'; + if (isForm) { + return NextResponse.redirect( + new URL(`/login?${LOGIN_ERROR_PARAM}=${code}`, req.url), + 303, + ); + } + return failJson( + code, + 'Sign-in is unavailable right now. Please contact support.', + 500, + ); + } const session: AuthSession = {user, expiresAt: bundle.expires_at}; diff --git a/src/features/auth/services/sessionToken.ts b/src/features/auth/services/sessionToken.ts index 96bd5e1..24d5bb8 100644 --- a/src/features/auth/services/sessionToken.ts +++ b/src/features/auth/services/sessionToken.ts @@ -1,4 +1,5 @@ import {createHmac, timingSafeEqual} from 'node:crypto'; +import {ConfigError} from '@/shared/errors/configError'; /** * The session cookie format, and the only place that knows how to mint or @@ -37,7 +38,7 @@ function secret(): string { const fromEnv = process.env.AUTH_SECRET; if (fromEnv) return fromEnv; if (process.env.NODE_ENV === 'production') { - throw new Error( + throw new ConfigError( 'AUTH_SECRET is required in production — refusing to sign sessions with the development key.', ); } diff --git a/src/features/auth/services/tokenStore.ts b/src/features/auth/services/tokenStore.ts index dec411b..fd8952f 100644 --- a/src/features/auth/services/tokenStore.ts +++ b/src/features/auth/services/tokenStore.ts @@ -1,5 +1,6 @@ import 'server-only'; import {createCipheriv, createDecipheriv, createHash, randomBytes} from 'node:crypto'; +import {ConfigError} from '@/shared/errors/configError'; /** * Where the platform's access and refresh tokens live. @@ -24,7 +25,7 @@ const DEV_SECRET = 'loyaly-dev-secret-not-for-production'; function key(): Buffer { const fromEnv = process.env.AUTH_SECRET; if (!fromEnv && process.env.NODE_ENV === 'production') { - throw new Error( + throw new ConfigError( 'AUTH_SECRET is required in production — refusing to encrypt platform tokens with the development key.', ); } diff --git a/src/services/api/apiClient.ts b/src/services/api/apiClient.ts index ee935da..bc6eaeb 100644 --- a/src/services/api/apiClient.ts +++ b/src/services/api/apiClient.ts @@ -19,6 +19,7 @@ import 'server-only'; * is one API; this file is just the web console's way in. */ +import {ConfigError} from '@/shared/errors/configError'; import type {ApiRole} from './types'; /** @@ -85,27 +86,12 @@ const KNOWN_WRONG_HOSTS: Record = { }; /** - * A deployment is misconfigured. NOT a network failure, and the distinction is - * the whole point of the class existing. - * - * Both used to arrive at a caller as `UpstreamError(0, 'network')`, because - * `buildUrl` — which resolves the variable — was called INSIDE the try block - * that turns a failed fetch into that error. So "nobody set LOYALY_API_BASE" - * and "the platform is down" were indistinguishable at every call site, and - * the login route reported both as 502 platform_unreachable. That sent people - * to check DNS, egress and the platform's health for a fault whose fix is one - * line in an env file. - * - * Callers translate this to a 500: a server that cannot be configured is not a - * bad gateway, and no retry or status page will help. + * `buildUrl` — which resolves this variable — is called OUTSIDE the try block + * that turns a failed fetch into `UpstreamError(0, 'network')`. It used to be + * inside it, which made "nobody set LOYALY_API_BASE" indistinguishable from + * "the platform is down" at every call site, and had the login route report + * both as 502 platform_unreachable. */ -export class ConfigError extends Error { - constructor(message: string) { - super(message); - this.name = 'ConfigError'; - } -} - function configError(detail: string): ConfigError { return new ConfigError( `LOYALY_API_BASE is invalid: ${detail}. ` + diff --git a/src/shared/errors/configError.ts b/src/shared/errors/configError.ts new file mode 100644 index 0000000..8503ed8 --- /dev/null +++ b/src/shared/errors/configError.ts @@ -0,0 +1,29 @@ +/** + * A deployment is misconfigured — a required environment variable is missing, + * or set to something this app refuses to accept. + * + * ── Why it lives in its own module ─────────────────────────────────────── + * Its throwers span two runtimes. `apiClient` and `tokenStore` are Node-only + * (`server-only`), while `sessionToken` is reached from `src/proxy.ts`, which + * Next compiles for the Edge runtime. Declaring this in apiClient meant the + * proxy bundle would have to pull in the whole platform client — and its + * `server-only` guard — to name one error class. This file imports nothing, so + * both runtimes can share the type without sharing anything else. + * + * ── Why it is not just Error ───────────────────────────────────────────── + * Callers have to tell a misconfigured server apart from a failing one. Those + * read identically to a browser and have nothing in common as remedies: one is + * fixed by an operator in under a minute, the other by waiting. Collapsing them + * is what had a missing LOYALY_API_BASE reported as "Could not reach Loyaly, + * check your connection". + * + * Every caller treats this as a 500 whose detail is LOGGED, never returned: + * the message names environment variables, which is operator information. + * Grep production logs for `[loyaly] configuration error:`. + */ +export class ConfigError extends Error { + constructor(message: string) { + super(message); + this.name = 'ConfigError'; + } +} diff --git a/src/shared/services/bff.ts b/src/shared/services/bff.ts index cc147c8..6884c63 100644 --- a/src/shared/services/bff.ts +++ b/src/shared/services/bff.ts @@ -1,6 +1,7 @@ import 'server-only'; import type {NextRequest} from 'next/server'; -import {ConfigError, UpstreamError} from '@/services/api/apiClient'; +import {UpstreamError} from '@/services/api/apiClient'; +import {ConfigError} from '@/shared/errors/configError'; import {NoSessionError, withUpstream} from '@/features/auth/services/upstreamSession'; import {ok, parseQuery, type Query} from '@/shared/services/apiRoute'; import type {ApiErrorCode} from '@/shared/types/api';