fix(security): read the password-complexity key the settings UI writes (stable) (#844)
* 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.
This commit is contained in:
@@ -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');
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -123,20 +123,32 @@ async function getPasswordComplexitySettings() {
|
|||||||
|
|
||||||
// Use retry wrapper to handle connection failures
|
// Use retry wrapper to handle connection failures
|
||||||
const settings = await withRetry(async () => {
|
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')
|
return await db('app_settings')
|
||||||
.where('setting_key', 'security_password_complexity_level')
|
.where('setting_key', 'security_password_complexity')
|
||||||
.first();
|
.first();
|
||||||
});
|
});
|
||||||
|
|
||||||
if (!settings || !settings.setting_value) {
|
if (!settings || !settings.setting_value) {
|
||||||
return 'moderate'; // Default
|
return 'moderate'; // Default
|
||||||
}
|
}
|
||||||
|
|
||||||
const value = typeof settings.setting_value === 'string'
|
// Parse with fallback, mirroring getAppSetting: on SQLite the TEXT
|
||||||
? JSON.parse(settings.setting_value)
|
// column returns the JSON-stringified value ('"very_strong"'), but on
|
||||||
: settings.setting_value;
|
// Postgres the json column comes back already decoded ('very_strong')
|
||||||
|
// — a bare JSON.parse would throw there and the outer catch would
|
||||||
return value;
|
// 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) {
|
} catch (error) {
|
||||||
logger.error('Failed to get password complexity settings:', error);
|
logger.error('Failed to get password complexity settings:', error);
|
||||||
return 'moderate'; // Default on error - ensures app continues working
|
return 'moderate'; // Default on error - ensures app continues working
|
||||||
|
|||||||
Reference in New Issue
Block a user