diff --git a/nodejs/package.json b/nodejs/package.json index 45ef98e..9337708 100644 --- a/nodejs/package.json +++ b/nodejs/package.json @@ -11,10 +11,10 @@ "scripts": { "start": "node ./bin/www", "dev": "npx nodemon --ignore public/ ./bin/www", - "test": "node --test test/unit/callback_queue.test.js test/unit/host_lookup.test.js test/unit/wildcard_matchany.test.js test/unit/roles.test.js test/unit/oidc.test.js test/unit/safe_redirect.test.js test/unit/host_features.test.js test/unit/dynamic_record.test.js test/unit/hostname_validate.test.js test/unit/unix_socket.test.js test/integration/dns_provider.test.js", - "test:unit": "node --test test/unit/callback_queue.test.js test/unit/host_lookup.test.js test/unit/wildcard_matchany.test.js test/unit/roles.test.js test/unit/oidc.test.js test/unit/safe_redirect.test.js test/unit/host_features.test.js test/unit/dynamic_record.test.js test/unit/hostname_validate.test.js test/unit/unix_socket.test.js", + "test": "node --test test/unit/callback_queue.test.js test/unit/host_lookup.test.js test/unit/wildcard_matchany.test.js test/unit/roles.test.js test/unit/oidc.test.js test/unit/safe_redirect.test.js test/unit/host_features.test.js test/unit/dynamic_record.test.js test/unit/hostname_validate.test.js test/unit/password_policy.test.js test/unit/unix_socket.test.js test/integration/dns_provider.test.js", + "test:unit": "node --test test/unit/callback_queue.test.js test/unit/host_lookup.test.js test/unit/wildcard_matchany.test.js test/unit/roles.test.js test/unit/oidc.test.js test/unit/safe_redirect.test.js test/unit/host_features.test.js test/unit/dynamic_record.test.js test/unit/hostname_validate.test.js test/unit/password_policy.test.js test/unit/unix_socket.test.js", "test:integration": "node --test test/integration/dns_provider.test.js", - "test:watch": "node --test --watch test/unit/callback_queue.test.js test/unit/host_lookup.test.js test/unit/wildcard_matchany.test.js test/unit/roles.test.js test/unit/oidc.test.js test/unit/safe_redirect.test.js test/unit/host_features.test.js test/unit/dynamic_record.test.js test/unit/hostname_validate.test.js test/unit/unix_socket.test.js test/integration/dns_provider.test.js" + "test:watch": "node --test --watch test/unit/callback_queue.test.js test/unit/host_lookup.test.js test/unit/wildcard_matchany.test.js test/unit/roles.test.js test/unit/oidc.test.js test/unit/safe_redirect.test.js test/unit/host_features.test.js test/unit/dynamic_record.test.js test/unit/hostname_validate.test.js test/unit/password_policy.test.js test/unit/unix_socket.test.js test/integration/dns_provider.test.js" }, "engines": { "node": ">=18.0.0" diff --git a/nodejs/public/lib/js/val.js b/nodejs/public/lib/js/val.js index 90c8742..0029957 100755 --- a/nodejs/public/lib/js/val.js +++ b/nodejs/public/lib/js/val.js @@ -176,10 +176,22 @@ } }, + // Mirrors utils/password_policy.js: >= 8 chars, and either 12+ chars + // or at least 3 of {lowercase, uppercase, number, symbol}. password: function( value ) { - var reg = /^(?=[^\d_].*?\d)\w(\w|[!@#$%]){1,48}/; - if ( reg.test( value ) === false ) { - return "Weak password, Try again"; + if ( typeof value !== 'string' || value.length < 8 ) { + return "Password must be at least 8 characters"; + } + if ( value.length >= 12 ) return; + + var classes = 0; + if ( /[a-z]/.test( value ) ) classes++; + if ( /[A-Z]/.test( value ) ) classes++; + if ( /[0-9]/.test( value ) ) classes++; + if ( /[^A-Za-z0-9]/.test( value ) ) classes++; + + if ( classes < 3 ) { + return "Use 3 of: lowercase, uppercase, number, symbol (or 12+ chars)"; } } } diff --git a/nodejs/routes/user.js b/nodejs/routes/user.js index 4be45a0..4195cfc 100755 --- a/nodejs/routes/user.js +++ b/nodejs/routes/user.js @@ -3,6 +3,14 @@ const router = require('express').Router(); const {User} = require('../models').models; const authz = require('../middleware/authz'); +const {passwordError} = require('../utils/password_policy'); + +// Reject a weak password before it reaches the model. Throws 422 with a +// per-field key the frontend surfaces inline. +function validatePassword(password){ + let message = passwordError(password); + if(message) throw User.errors.ObjectValidateError([{key: 'password', message}]); +} // User management is global-admin-only, except the self-service routes below // (GET /me, PUT /password, POST /key) which any authenticated user may call for @@ -21,8 +29,12 @@ router.get('/', authz.requireAdmin, async function(req, res, next){ router.post('/', authz.requireAdmin, async function(req, res, next){ try{ req.body.created_by = authz.reqUsername(req) + validatePassword(req.body.password); - return res.json(await User.add(req.body)); + // User.create (not the nonexistent User.add) — the drift here meant every + // API-created account threw, so the new credentials never existed to log + // in with (issue #48). + return res.json(await User.create(req.body)); }catch(error){ next(error); } @@ -62,6 +74,7 @@ router.get('/me', async function(req, res, next){ // Self-service: change your own password. router.put('/password', async function(req, res, next){ try{ + validatePassword(req.body.password); return res.json({results: await req.user.setPassword(req.body)}) }catch(error){ next(error); @@ -71,6 +84,7 @@ router.put('/password', async function(req, res, next){ // Admin: reset another user's password. router.put('/password/:username', authz.requireAdmin, async function(req, res, next){ try{ + validatePassword(req.body.password); let user = await User.get(req.params.username); return res.json({results: await user.setPassword(req.body)}); }catch(error){ diff --git a/nodejs/test/unit/password_policy.test.js b/nodejs/test/unit/password_policy.test.js new file mode 100644 index 0000000..c0d3ca3 --- /dev/null +++ b/nodejs/test/unit/password_policy.test.js @@ -0,0 +1,35 @@ +'use strict'; + +const {describe, test} = require('node:test'); +const assert = require('node:assert'); + +const {passwordError} = require('../../utils/password_policy'); + +/** + * The old rule (issue #48) rejected strong passwords and accepted weak ones. + * These pin the corrected behavior: length-forward, 3-of-4 character classes. + */ +describe('passwordError', () => { + test('accepts a strong mixed password (previously rejected)', () => { + assert.strictEqual(passwordError('@123Caplowercase'), null); + }); + test('accepts a 12+ char passphrase on length alone', () => { + assert.strictEqual(passwordError('correcthorsebattery'), null); + }); + test('rejects a weak two-class password (previously accepted)', () => { + assert.notStrictEqual(passwordError('lowercase1'), null); + }); + test('rejects too-short passwords', () => { + assert.notStrictEqual(passwordError('Ab3$xy'), null); // 6 chars + assert.notStrictEqual(passwordError(''), null); + assert.notStrictEqual(passwordError(undefined), null); + }); + test('accepts 8 chars with 3 classes', () => { + assert.strictEqual(passwordError('Abcd123!'), null); // upper, lower, num, sym + assert.strictEqual(passwordError('Abcdefg1'), null); // upper, lower, num + }); + test('rejects 8-11 chars with only 2 classes', () => { + assert.notStrictEqual(passwordError('abcdefg1'), null); // lower + num only + assert.notStrictEqual(passwordError('ABCDEFG1'), null); // upper + num only + }); +}); diff --git a/nodejs/utils/password_policy.js b/nodejs/utils/password_policy.js new file mode 100644 index 0000000..9a3bf8c --- /dev/null +++ b/nodejs/utils/password_policy.js @@ -0,0 +1,42 @@ +'use strict'; + +/** + * Local-account password policy. + * + * The previous rule was a single opaque regex that rejected strong passwords + * (e.g. "@123Caplowercase") while accepting weak ones (e.g. "lowercase1") — see + * issue #48. This replaces it with a clear, length-forward policy: + * + * - at least MIN characters, and + * - either PASSPHRASE+ characters (a long passphrase passes on length alone), + * or at least 3 of the 4 character classes (lowercase, uppercase, number, + * symbol). + * + * Pure and dependency-free so it can run server-side (routes/user.js) and be + * mirrored client-side (public/lib/js/val.js) and unit tested. + */ + +const MIN = 8; +const PASSPHRASE = 12; + +// Returns a human-readable error message if the password is unacceptable, else +// null when it passes. +function passwordError(value){ + if(typeof value !== 'string' || value.length < MIN){ + return `Password must be at least ${MIN} characters.`; + } + if(value.length >= PASSPHRASE) return null; + + let classes = 0; + if(/[a-z]/.test(value)) classes++; + if(/[A-Z]/.test(value)) classes++; + if(/[0-9]/.test(value)) classes++; + if(/[^A-Za-z0-9]/.test(value)) classes++; + + if(classes < 3){ + return 'Use at least 3 of: lowercase, uppercase, number, symbol — or make it 12+ characters.'; + } + return null; +} + +module.exports = {passwordError, MIN, PASSPHRASE}; diff --git a/nodejs/views/users.ejs b/nodejs/views/users.ejs index a27ba72..4945909 100755 --- a/nodejs/views/users.ejs +++ b/nodejs/views/users.ejs @@ -82,14 +82,14 @@
- +
-