From 3292605e63d3efcf735f3997b82e9db53606fc47 Mon Sep 17 00:00:00 2001 From: Brandon Corbett Date: Sun, 28 Jun 2026 14:09:00 +0200 Subject: [PATCH] fix(config): apply LOGIN_METHODS and env-mapped config over seeded defaults bootstrapSystemConfig now re-applies env values over config rows that were never changed through the admin API (updatedBy IS NULL), so env vars like LOGIN_METHODS take effect instead of being permanently shadowed by the login-policy migration's seed. Admin edits now record updatedBy so they're preserved, and a migration re-applies env to existing un-edited rows. Closes #47 --- .changeset/login-methods-env-config.md | 11 +++++ resources/coverage-badge.svg | 8 ++-- src/config/bootstrapSystemConfig.ts | 22 +++++++++- src/controllers/systemConfig.ts | 4 ++ ...0260628120000-reseed-env-system-config.cjs | 36 ++++++++++++++++ .../unit/config/bootstrapSystemConfig.spec.ts | 42 +++++++++++++++++++ 6 files changed, 117 insertions(+), 6 deletions(-) create mode 100644 .changeset/login-methods-env-config.md create mode 100644 src/migrations/20260628120000-reseed-env-system-config.cjs diff --git a/.changeset/login-methods-env-config.md b/.changeset/login-methods-env-config.md new file mode 100644 index 0000000..efba088 --- /dev/null +++ b/.changeset/login-methods-env-config.md @@ -0,0 +1,11 @@ +--- +'seamless-auth-api': patch +--- + +Env-mapped system config (e.g. `LOGIN_METHODS`) now takes effect over +migration-seeded defaults. Previously the login-policy migration hard-seeded +`login_methods` and `bootstrapSystemConfig` only seeded missing rows, so the env +var was permanently ignored. Now bootstrap re-applies env values over config that +was never changed through the admin API (`updatedBy IS NULL`), admin edits record +`updatedBy` so they are preserved, and a migration re-applies env to existing +un-edited rows. diff --git a/resources/coverage-badge.svg b/resources/coverage-badge.svg index efd658f..ca9d3d6 100644 --- a/resources/coverage-badge.svg +++ b/resources/coverage-badge.svg @@ -1,5 +1,5 @@ - - coverage: 82.2% + + coverage: 82.3% @@ -17,7 +17,7 @@ coverage coverage - 82.2% - 82.2% + 82.3% + 82.3% diff --git a/src/config/bootstrapSystemConfig.ts b/src/config/bootstrapSystemConfig.ts index 3d23780..e410d43 100644 --- a/src/config/bootstrapSystemConfig.ts +++ b/src/config/bootstrapSystemConfig.ts @@ -15,13 +15,31 @@ export async function bootstrapSystemConfig() { for (const [key, envVar] of Object.entries(SYSTEM_CONFIG_ENV_MAP)) { const existing = await SystemConfig.findByPk(key); + const envValue = process.env[envVar]; if (existing) { - resolvedConfig[key] = existing.value; + // Re-apply the env value over config that hasn't been changed through the + // admin API (updatedBy IS NULL), so env-mapped config stays authoritative + // unless an admin has overridden it at runtime. Without this, a migration + // that seeds a default would permanently shadow the env var. + if (envValue && existing.updatedBy == null) { + const parsed = parseSystemConfigEnvValue( + key as keyof typeof SYSTEM_CONFIG_ENV_MAP, + envValue, + ); + + if (JSON.stringify(existing.value) !== JSON.stringify(parsed)) { + await existing.update({ value: parsed }); + } + + resolvedConfig[key] = parsed; + } else { + resolvedConfig[key] = existing.value; + } + continue; } - const envValue = process.env[envVar]; if (!envValue) { const defaultValue = SYSTEM_CONFIG_DEFAULTS[key as keyof typeof SYSTEM_CONFIG_DEFAULTS]; diff --git a/src/controllers/systemConfig.ts b/src/controllers/systemConfig.ts index 1dceead..3e70ee5 100644 --- a/src/controllers/systemConfig.ts +++ b/src/controllers/systemConfig.ts @@ -73,12 +73,16 @@ export async function updateSystemConfig(req: ServiceRequest, res: Response) { const existingMap = Object.fromEntries(existingRows.map((row) => [row.key, row.value])); + const updatedBy = typeof req.clientId === 'function' ? req.clientId() : (req.clientId ?? null); + await SystemConfig.sequelize!.transaction(async (tx) => { for (const [key, value] of Object.entries(updates)) { await SystemConfig.upsert( { key, value, + // Mark the row as admin-managed so bootstrap won't overwrite it from env. + updatedBy, }, { transaction: tx }, ); diff --git a/src/migrations/20260628120000-reseed-env-system-config.cjs b/src/migrations/20260628120000-reseed-env-system-config.cjs new file mode 100644 index 0000000..7e86847 --- /dev/null +++ b/src/migrations/20260628120000-reseed-env-system-config.cjs @@ -0,0 +1,36 @@ +'use strict'; + +// Re-apply env-mapped values over migration-seeded defaults so env vars like +// LOGIN_METHODS take effect on existing installs. Only touches rows never changed +// through the admin API (updatedBy IS NULL); bootstrapSystemConfig keeps them in +// sync on every boot going forward. +module.exports = { + async up(queryInterface) { + const loginMethods = process.env.LOGIN_METHODS; + if (loginMethods) { + const arr = loginMethods + .split(',') + .map((s) => s.trim()) + .filter(Boolean); + await queryInterface.sequelize.query( + `UPDATE system_config SET value = CAST(:val AS jsonb), "updatedAt" = NOW() + WHERE key = 'login_methods' AND "updatedBy" IS NULL`, + { replacements: { val: JSON.stringify(arr) } }, + ); + } + + const fallback = process.env.PASSKEY_LOGIN_FALLBACK_ENABLED; + if (fallback) { + const val = fallback.trim().toLowerCase() === 'true'; + await queryInterface.sequelize.query( + `UPDATE system_config SET value = CAST(:val AS jsonb), "updatedAt" = NOW() + WHERE key = 'passkey_login_fallback_enabled' AND "updatedBy" IS NULL`, + { replacements: { val: JSON.stringify(val) } }, + ); + } + }, + + async down() { + // No-op: re-seeding from env has no meaningful inverse. + }, +}; diff --git a/tests/unit/config/bootstrapSystemConfig.spec.ts b/tests/unit/config/bootstrapSystemConfig.spec.ts index b23b883..3f24161 100644 --- a/tests/unit/config/bootstrapSystemConfig.spec.ts +++ b/tests/unit/config/bootstrapSystemConfig.spec.ts @@ -84,6 +84,48 @@ describe('bootstrapSystemConfig', () => { expect(result).toBeDefined(); }); + it('re-applies env over an existing row that was not admin-edited', async () => { + const { SystemConfig } = await import('../../../src/models/systemConfig'); + const { parseSystemConfigEnvValue } = await import('../../../src/utils/parseEnvConfigs'); + const { SystemConfigSchema } = await import('../../../src/schemas/systemConfig.schema'); + + const update = vi.fn(); + (SystemConfig.findByPk as any).mockResolvedValue({ value: 'old', updatedBy: null, update }); + + process.env.APP_NAME = 'TestApp'; + process.env.RATE_LIMIT = '100'; + (parseSystemConfigEnvValue as any).mockReturnValue('parsed'); + (SystemConfigSchema.safeParse as any).mockReturnValue({ success: true, data: {} }); + + const { bootstrapSystemConfig } = await import('../../../src/config/bootstrapSystemConfig'); + await bootstrapSystemConfig(); + + expect(update).toHaveBeenCalledWith({ value: 'parsed' }); + }); + + it('preserves a row that was changed via the admin API (updatedBy set)', async () => { + const { SystemConfig } = await import('../../../src/models/systemConfig'); + const { parseSystemConfigEnvValue } = await import('../../../src/utils/parseEnvConfigs'); + const { SystemConfigSchema } = await import('../../../src/schemas/systemConfig.schema'); + + const update = vi.fn(); + (SystemConfig.findByPk as any).mockResolvedValue({ + value: 'admin-value', + updatedBy: 'admin-id', + update, + }); + + process.env.APP_NAME = 'TestApp'; + process.env.RATE_LIMIT = '100'; + (parseSystemConfigEnvValue as any).mockReturnValue('parsed'); + (SystemConfigSchema.safeParse as any).mockReturnValue({ success: true, data: {} }); + + const { bootstrapSystemConfig } = await import('../../../src/config/bootstrapSystemConfig'); + await bootstrapSystemConfig(); + + expect(update).not.toHaveBeenCalled(); + }); + it('throws when env missing', async () => { const { SystemConfig } = await import('../../../src/models/systemConfig');