From 4256c0fb498aa08f161ce7850aa866fb4446523c Mon Sep 17 00:00:00 2001 From: eleith Date: Thu, 12 Nov 2026 14:33:59 -0800 Subject: [PATCH] feat(auth): generate a session secret when none is configured --- .changeset/allow-ephemeral-auth-secret.md | 5 + apps/web/config/hledger-reports.example.yaml | 2 +- apps/web/src/lib/server/auth/index.test.ts | 97 +++++++++++++++++++- apps/web/src/lib/server/auth/index.ts | 8 +- apps/web/src/lib/server/auth/secret.ts | 13 +++ docs/authentication.md | 9 +- packages/config/schema.json | 2 - packages/config/src/auth.test.ts | 53 +++++++++-- packages/config/src/index.ts | 10 +- packages/config/src/schema.ts | 9 +- 10 files changed, 174 insertions(+), 34 deletions(-) create mode 100644 .changeset/allow-ephemeral-auth-secret.md create mode 100644 apps/web/src/lib/server/auth/secret.ts diff --git a/.changeset/allow-ephemeral-auth-secret.md b/.changeset/allow-ephemeral-auth-secret.md new file mode 100644 index 0000000..cadd0a8 --- /dev/null +++ b/.changeset/allow-ephemeral-auth-secret.md @@ -0,0 +1,5 @@ +--- +'@hledger-reports/web': patch +--- + +Use a random session secret at startup when none is configured. Set `AUTH_SECRET` to stay signed in across restarts. diff --git a/apps/web/config/hledger-reports.example.yaml b/apps/web/config/hledger-reports.example.yaml index f626fa9..73ec6b7 100644 --- a/apps/web/config/hledger-reports.example.yaml +++ b/apps/web/config/hledger-reports.example.yaml @@ -14,7 +14,7 @@ accounts: url: app: '${APP_URL:-http://127.0.0.1:3000}' auth: - secret: '${AUTH_SECRET}' + # Optional: set AUTH_SECRET to keep sessions across restarts. credentials: username: household password: '${AUTH_PASSWORD}' diff --git a/apps/web/src/lib/server/auth/index.test.ts b/apps/web/src/lib/server/auth/index.test.ts index 60ce521..0d11c75 100644 --- a/apps/web/src/lib/server/auth/index.test.ts +++ b/apps/web/src/lib/server/auth/index.test.ts @@ -1,4 +1,6 @@ import { afterEach, beforeEach, expect, test, vi } from 'vitest'; +import type { Config } from '@hledger-reports/config'; +import type { BetterAuthOptions } from 'better-auth/minimal'; import { createAuth, getSession } from './index.js'; import { cookies, @@ -92,17 +94,102 @@ test('restart preserves the original seven-day expiry; tampering fails and reads expect(await getSession(auth, sessionHeaders(cookie))).toBeNull(); }); -test('configured secret wins over SDK environment keys; rotation invalidates existing sessions', async () => { +test.each([undefined, ''])( + 'omitted secret with AUTH_SECRET=%j lasts for one auth instance only', + async (environmentSecret) => { + vi.stubEnv('AUTH_SECRET', environmentSecret); + vi.stubEnv('BETTER_AUTH_SECRET', 'environment-secret-that-must-not-control-this-app'); + vi.stubEnv('BETTER_AUTH_SECRETS', '9:environment-key-that-must-not-control-this-app'); + const config = { ...credentialsConfig, auth: { credentials } }; + const auth = createAuth(config); + const cookie = cookies(await request(auth, '/sign-in/credentials', credentials)); + const initial = await getSession(auth, sessionHeaders(cookie)); + expect(initial).not.toBeNull(); + vi.setSystemTime(new Date('2026-10-30T12:00:00Z')); + expect(await getSession(auth, sessionHeaders(cookie))).toEqual(initial); + expect(await getSession(createAuth(config), sessionHeaders(cookie))).toBeNull(); + expect(await getSession(auth, sessionHeaders(cookie))).toEqual(initial); + } +); + +test('AUTH_SECRET preserves sessions across auth instances', async () => { + const config = { ...credentialsConfig, auth: { credentials } }; + const environmentSecret = ` ${secret} `; + vi.stubEnv('AUTH_SECRET', environmentSecret); vi.stubEnv('BETTER_AUTH_SECRET', 'environment-secret-that-must-not-control-this-app'); vi.stubEnv('BETTER_AUTH_SECRETS', '9:environment-key-that-must-not-control-this-app'); - const cookie = cookies( - await request(createAuth(credentialsConfig), '/sign-in/credentials', credentials) - ); + const auth = createAuth(config); + expect(auth.options.secret).toBe(environmentSecret); + const cookie = cookies(await request(auth, '/sign-in/credentials', credentials)); + expect(await getSession(auth, sessionHeaders(cookie))).not.toBeNull(); vi.stubEnv('BETTER_AUTH_SECRETS', '10:another-environment-key-that-must-not-control-this-app'); - expect(await getSession(createAuth(credentialsConfig), sessionHeaders(cookie))).not.toBeNull(); + expect(await getSession(createAuth(config), sessionHeaders(cookie))).toMatchObject({ + user: { name: credentials.username } + }); +}); + +test('configured secret wins over invalid AUTH_SECRET and SDK keys; rotation invalidates sessions', async () => { + vi.stubEnv('AUTH_SECRET', 'short'); + vi.stubEnv('BETTER_AUTH_SECRET', 'environment-secret-that-must-not-control-this-app'); + vi.stubEnv('BETTER_AUTH_SECRETS', '9:environment-key-that-must-not-control-this-app'); + const config = { ...credentialsConfig, auth: { secret: ` ${secret} `, credentials } }; + const auth = createAuth(config); + expect(auth.options.secret).toBe(config.auth.secret); + const cookie = cookies(await request(auth, '/sign-in/credentials', credentials)); + vi.stubEnv('BETTER_AUTH_SECRETS', '10:another-environment-key-that-must-not-control-this-app'); + expect(await getSession(createAuth(config), sessionHeaders(cookie))).not.toBeNull(); const rotated = createAuth({ ...credentialsConfig, auth: { secret: 'rotated-test-secret-q9V3m1R8s2H6d5K7', credentials } }); expect(await getSession(rotated, sessionHeaders(cookie))).toBeNull(); }); + +test('rejects invalid supplied keys without leaking them or falling back', () => { + const privateKey = 'private-key-do-not-log'; + const withoutSecret = { ...credentialsConfig, auth: { credentials } }; + const rejectsWithoutLeaking = (config: Config, rejected: string) => { + let failure: unknown; + try { + createAuth(config); + } catch (error) { + failure = error; + } + expect(failure).toBeInstanceOf(Error); + expect(failure).not.toHaveProperty('cause'); + const message = (failure as Error).message; + expect(message).toMatch(/^Authentication secret/); + if (rejected.length > 0) expect(message).not.toContain(rejected); + }; + for (const invalid of [privateKey, ' '.repeat(32)]) { + vi.stubEnv('AUTH_SECRET', invalid); + rejectsWithoutLeaking(withoutSecret, invalid); + } + vi.stubEnv('AUTH_SECRET', secret); + for (const invalid of ['', privateKey, ' '.repeat(32)]) { + rejectsWithoutLeaking( + { ...credentialsConfig, auth: { secret: invalid, credentials } }, + invalid + ); + } +}); + +test('application logging discards private SDK diagnostic arguments', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const error = vi.spyOn(console, 'error').mockImplementation(() => {}); + try { + const auth = createAuth(credentialsConfig); + await auth.$context; + const logger: NonNullable = auth.options.logger; + if (!logger || !logger.log) throw new Error('Expected the application auth logger.'); + warn.mockClear(); + error.mockClear(); + logger.log('warn', 'private SDK diagnostic', { secret: 'fictional-private-session-key' }); + logger.log('error', 'fictional-private-session-key', { diagnostic: '/private/fixture' }); + expect(warn).toHaveBeenCalledExactlyOnceWith('authentication.warning'); + expect(error).toHaveBeenCalledExactlyOnceWith('authentication.error'); + } finally { + warn.mockRestore(); + error.mockRestore(); + } +}); diff --git a/apps/web/src/lib/server/auth/index.ts b/apps/web/src/lib/server/auth/index.ts index f891446..3efb40a 100644 --- a/apps/web/src/lib/server/auth/index.ts +++ b/apps/web/src/lib/server/auth/index.ts @@ -1,6 +1,7 @@ import type { Config } from '@hledger-reports/config'; import { betterAuth } from 'better-auth/minimal'; import { authProviders } from './providers.js'; +import { resolveAuthSecret } from './secret.js'; export { signIn, signOut } from './actions.js'; export { callback } from './callback.js'; @@ -8,12 +9,13 @@ export { callback } from './callback.js'; const WEEK_SECONDS = 7 * 24 * 60 * 60; export function createAuth(config: Config) { + const secret = resolveAuthSecret(config.auth.secret); // Cookie scope ignores ports, so distinguish local instances by their public port. const publicPort = new URL(config.url.app).port || 'default'; return betterAuth({ - secret: config.auth.secret, - // Explicit key configuration also prevents BETTER_AUTH_SECRETS overriding YAML. - secrets: [{ version: 1, value: config.auth.secret }], + secret, + // The resolved key is authoritative, including over the SDK's environment keyring. + secrets: [{ version: 1, value: secret }], baseURL: config.url.app.replace(/\/+$/, ''), basePath: '/auth', telemetry: { enabled: false }, diff --git a/apps/web/src/lib/server/auth/secret.ts b/apps/web/src/lib/server/auth/secret.ts new file mode 100644 index 0000000..03d7556 --- /dev/null +++ b/apps/web/src/lib/server/auth/secret.ts @@ -0,0 +1,13 @@ +import { randomBytes } from 'node:crypto'; + +export function resolveAuthSecret( + configured?: string, + env: Readonly> = process.env +): string { + const secret = configured ?? (env.AUTH_SECRET || undefined); + if (secret === undefined) return randomBytes(32).toString('base64'); + if (secret.length < 32 || secret.trim().length === 0) { + throw new Error('Authentication secret must be at least 32 characters and not blank'); + } + return secret; +} diff --git a/docs/authentication.md b/docs/authentication.md index 554b15d..dd4af7c 100644 --- a/docs/authentication.md +++ b/docs/authentication.md @@ -9,7 +9,8 @@ Use the [configuration example](../apps/web/config/hledger-reports.example.yaml) Choose exactly one strategy: credentials or OIDC. - Set `url.app` to the exact browser origin. `localhost` and `127.0.0.1` are different origins. -- Supply a strong password and a stable random `AUTH_SECRET` of at least 32 characters through private environment or secret tooling. +- Use a strong password for credentials sign-in. +- The session key uses `auth.secret`, then a nonempty `AUTH_SECRET`, otherwise a random key generated at startup. Set a stable random secret of at least 32 characters to stay signed in across restarts. - Keep secrets out of commits, command arguments and logs. Restart services after configuration changes. For OIDC, set the issuer, client ID and client secret. Register @@ -29,9 +30,9 @@ development for direct local HTTP access. Do not expose the upstream publicly. ## Sessions Authentication cookies use an `hledger-` prefix scoped to the configured public -port, preventing accidental collisions between local apps and instances. Sign in -again after this update; old shared cookies are not reused or cleared. +port, preventing accidental collisions between local apps and instances. -Sessions survive restart with the same secret. Changing a password or provider +Without a supplied secret, restarting web signs users out. Sessions survive restart +with the same configured or environment secret. Changing a password or provider assignment does not immediately revoke existing sessions. Rotate the auth secret and restart web to sign everyone out. Local sign-out cannot revoke copied cookies. diff --git a/packages/config/schema.json b/packages/config/schema.json index c97e0bc..f5f0cb3 100644 --- a/packages/config/schema.json +++ b/packages/config/schema.json @@ -38,7 +38,6 @@ "required": ["credentials"], "properties": { "secret": { - "default": "${AUTH_SECRET}", "anyOf": [ { "minLength": 32, @@ -72,7 +71,6 @@ "required": ["oidc"], "properties": { "secret": { - "default": "${AUTH_SECRET}", "anyOf": [ { "minLength": 32, diff --git a/packages/config/src/auth.test.ts b/packages/config/src/auth.test.ts index 778891c..898bf08 100644 --- a/packages/config/src/auth.test.ts +++ b/packages/config/src/auth.test.ts @@ -26,22 +26,57 @@ it('requires exactly one complete auth strategy', () => { } }); -it('defaults an omitted secret to AUTH_SECRET and rejects missing or short secrets', () => { - const input = credentialsInput(); - expect( - validateConfig(input, { env: { AUTH_SECRET: testSecret, AUTH_PASSWORD: 'synthetic-password' } }) - .auth - ).toEqual(rawConfig().auth); - expect(input.auth).not.toHaveProperty('secret'); +it('keeps omitted secrets absent regardless of ambient AUTH_SECRET without changing input', () => { + const oidc = { + issuer: 'https://identity.example.test/tenant/', + client_id: 'reports', + client_secret: 'synthetic-client-secret' + }; + for (const input of [credentialsInput(), rawConfig({ auth: { oidc } })]) { + const original = structuredClone(input); + const env = { AUTH_PASSWORD: 'synthetic-password' }; + const expected = validateConfig(input, { env }); + expect(expected.auth).not.toHaveProperty('secret'); + for (const secret of ['', 'short', testSecret]) { + expect(validateConfig(input, { env: { ...env, AUTH_SECRET: secret } })).toEqual(expected); + } + expect(validateConfig(input, { env })).toEqual(expected); + expect(input).toEqual(original); + } +}); + +it('does not read environment values when no references are authored', () => { + const env = new Proxy( + {}, + { + getOwnPropertyDescriptor() { + throw new Error('Environment must not be read'); + } + } + ); + const auth = { credentials: { username: 'shared-user', password: 'synthetic-password' } }; + expect(validateConfig(rawConfig({ auth }), { env }).auth).toEqual(auth); +}); + +it('requires explicit secret environment references to exist', () => { + const input = rawConfig({ auth: { ...rawConfig().auth, secret: '${SESSION}' } }); expect(() => validateConfig(input, { env: {} })).toThrowError( expect.objectContaining({ code: 'CONFIG_MISSING_ENVIRONMENT', issues: [{ path: '/auth/secret', message: 'A required environment variable is missing.' }] }) ); - for (const secret of ['short', ' '.repeat(32)]) { +}); + +it('rejects explicitly supplied short or blank secrets before and after interpolation', () => { + for (const secret of ['', 'x'.repeat(31), ' '.repeat(32)]) { expect(() => - validateConfig(input, { env: { AUTH_SECRET: secret, AUTH_PASSWORD: 'password' } }) + validateConfig(rawConfig({ auth: { ...rawConfig().auth, secret } }), { env: {} }) + ).toThrow(ConfigError); + expect(() => + validateConfig(rawConfig({ auth: { ...rawConfig().auth, secret: '${SESSION}' } }), { + env: { SESSION: secret } + }) ).toThrow(ConfigError); } }); diff --git a/packages/config/src/index.ts b/packages/config/src/index.ts index e7d301c..2b4e6f1 100644 --- a/packages/config/src/index.ts +++ b/packages/config/src/index.ts @@ -5,7 +5,6 @@ import { ConfigError, invalidInput } from './errors.js'; import { checkSchema, readInput } from './input.js'; import { interpolateString } from './interpolate.js'; import { - authSecretDefault, configSchema, credentialsAuthSchema, oidcAuthSchema, @@ -55,10 +54,13 @@ function resolveAuth( raw: ConfigInput['auth'], env: Readonly> ): Config['auth'] { - const secret = interpolateString(raw.secret ?? authSecretDefault, env, '/auth/secret'); + const secretFields: { secret?: string } = {}; + if (raw.secret !== undefined) { + secretFields.secret = interpolateString(raw.secret, env, '/auth/secret'); + } if ('credentials' in raw) { const auth = { - secret, + ...secretFields, credentials: { username: interpolateString(raw.credentials.username, env, '/auth/credentials/username'), password: interpolateString(raw.credentials.password, env, '/auth/credentials/password') @@ -68,7 +70,7 @@ function resolveAuth( return auth; } const auth = { - secret, + ...secretFields, oidc: { issuer: interpolateString(raw.oidc.issuer, env, '/auth/oidc/issuer'), client_id: interpolateString(raw.oidc.client_id, env, '/auth/oidc/client_id'), diff --git a/packages/config/src/schema.ts b/packages/config/src/schema.ts index fa3fa63..e39424a 100644 --- a/packages/config/src/schema.ts +++ b/packages/config/src/schema.ts @@ -22,10 +22,7 @@ const issuerSchema = Type.String({ const environmentSchema = Type.String({ pattern: environmentTokenPattern }); const rawAppSchema = Type.Union([appSchema, environmentSchema]); const rawIssuerSchema = Type.Union([issuerSchema, environmentSchema]); -export const authSecretDefault = '${AUTH_SECRET}'; -const rawSecretSchema = Type.Union([secretSchema, environmentSchema], { - default: authSecretDefault -}); +const rawSecretSchema = Type.Union([secretSchema, environmentSchema]); const credentialsSchema = Type.Object( { username: identitySchema, password: passwordSchema }, @@ -40,11 +37,11 @@ const rawOidcSchema = Type.Object( { additionalProperties: false } ); export const credentialsAuthSchema = Type.Object( - { secret: secretSchema, credentials: credentialsSchema }, + { secret: Type.Optional(secretSchema), credentials: credentialsSchema }, { additionalProperties: false } ); export const oidcAuthSchema = Type.Object( - { secret: secretSchema, oidc: oidcSchema }, + { secret: Type.Optional(secretSchema), oidc: oidcSchema }, { additionalProperties: false } ); const rawAuthSchema = Type.Union([ -- 2.54.0