From 50f5ca1d5bbe6f03397b3dc66ea8c3bea538466e Mon Sep 17 00:00:00 2001 From: Paul Nothaft <53005142+the-luap@users.noreply.github.com> Date: Sun, 19 Jul 2026 20:04:35 +0200 Subject: [PATCH] fix(security): read the password-complexity key the settings UI writes (stable) (#844) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(security): read the password-complexity key the settings UI writes The settings UI saves the admin's complexity choice as security_password_complexity (useSettingsState.ts prefixes security_ to password_complexity), but getPasswordComplexitySettings() queried security_password_complexity_level — written by nothing — so the setting was silently ignored and password validation always used the 'moderate' default. Spotted in the filpgame fork (their main, 2026-07-14). * fix(security): accept the Postgres json-column shape of the complexity value (codex review of #843) On SQLite the TEXT column returns the JSON-stringified value ('"very_strong"'), but on Postgres (production default) setting_value is a json column and arrives already decoded ('very_strong') — the bare JSON.parse threw and the outer catch silently fell back to 'moderate' again. Parse with fallback, mirroring getAppSetting's documented pattern; test now covers both driver shapes + the empty-value default. --- .../passwordValidation.complexityKey.test.js | 61 +++++++++++++++++++ backend/src/utils/passwordValidation.js | 26 +++++--- 2 files changed, 80 insertions(+), 7 deletions(-) create mode 100644 backend/__tests__/utils/passwordValidation.complexityKey.test.js diff --git a/backend/__tests__/utils/passwordValidation.complexityKey.test.js b/backend/__tests__/utils/passwordValidation.complexityKey.test.js new file mode 100644 index 00000000..4841d88a --- /dev/null +++ b/backend/__tests__/utils/passwordValidation.complexityKey.test.js @@ -0,0 +1,61 @@ +/** + * Regression tests for the password-complexity setting read path. + * + * Bug 1 (key mismatch): the settings UI saves the admin's choice as + * `security_password_complexity` (useSettingsState.ts prefixes every + * security field with `security_`), but getPasswordComplexitySettings() + * queried `security_password_complexity_level` — a key nothing writes — + * so the configured level was silently ignored. + * + * Bug 2 (driver shape, codex review of #843): on SQLite the TEXT column + * returns the JSON-stringified value ('"very_strong"'), but on Postgres + * (production default) `setting_value` is a json column and comes back + * already decoded ('very_strong'). A bare JSON.parse throws on the + * decoded shape and the outer catch fell back to 'moderate' — the + * setting stayed unenforced on Postgres even with the right key. + */ + +const mockQueriedKeys = []; +let mockStoredValue; + +jest.mock('../../src/database/db', () => ({ + db: () => ({ + where(_col, key) { + mockQueriedKeys.push(key); + return this; + }, + first() { + return Promise.resolve( + mockQueriedKeys[mockQueriedKeys.length - 1] === 'security_password_complexity' + ? { setting_key: 'security_password_complexity', setting_value: mockStoredValue } + : undefined + ); + }, + }), + withRetry: (fn) => fn(), +})); + +const { getPasswordComplexitySettings } = require('../../src/utils/passwordValidation'); + +describe('getPasswordComplexitySettings', () => { + beforeEach(() => { mockQueriedKeys.length = 0; }); + + it('reads the key the settings UI actually writes (SQLite shape: JSON-stringified)', async () => { + mockStoredValue = JSON.stringify('very_strong'); // '"very_strong"' + const level = await getPasswordComplexitySettings(); + expect(mockQueriedKeys).toContain('security_password_complexity'); + expect(level).toBe('very_strong'); + }); + + it('accepts the Postgres json-column shape (already decoded, no quotes)', async () => { + mockStoredValue = 'very_strong'; // pg driver auto-parses the json column + const level = await getPasswordComplexitySettings(); + expect(level).toBe('very_strong'); + }); + + it('falls back to moderate on an empty value', async () => { + mockStoredValue = ''; + const level = await getPasswordComplexitySettings(); + expect(level).toBe('moderate'); + }); +}); diff --git a/backend/src/utils/passwordValidation.js b/backend/src/utils/passwordValidation.js index ca93bd28..49929ac1 100644 --- a/backend/src/utils/passwordValidation.js +++ b/backend/src/utils/passwordValidation.js @@ -123,20 +123,32 @@ async function getPasswordComplexitySettings() { // Use retry wrapper to handle connection failures const settings = await withRetry(async () => { + // Key must match what the settings UI writes: `security_` prefix + + // `password_complexity` (useSettingsState.ts saveSecurityMutation). + // The old `security_password_complexity_level` key is written by + // nothing, so the admin's choice was silently ignored. return await db('app_settings') - .where('setting_key', 'security_password_complexity_level') + .where('setting_key', 'security_password_complexity') .first(); }); if (!settings || !settings.setting_value) { return 'moderate'; // Default } - - const value = typeof settings.setting_value === 'string' - ? JSON.parse(settings.setting_value) - : settings.setting_value; - - return value; + + // Parse with fallback, mirroring getAppSetting: on SQLite the TEXT + // column returns the JSON-stringified value ('"very_strong"'), but on + // Postgres the json column comes back already decoded ('very_strong') + // — a bare JSON.parse would throw there and the outer catch would + // silently fall back to 'moderate' again. + let value = settings.setting_value; + if (typeof value === 'string') { + try { + value = JSON.parse(value); + } catch (_) { /* already-decoded plain string — keep as-is */ } + } + + return value || 'moderate'; } catch (error) { logger.error('Failed to get password complexity settings:', error); return 'moderate'; // Default on error - ensures app continues working