Fix user creation and password policy (#48)
Root cause of "can't log in with new credentials": routes/user.js POST called User.add, which doesn't exist on the redis User model (it has create) — so every API-created account threw and was never persisted. Switch to User.create and make the Add button a submit. Replace the broken password rule (rejected strong "@123Caplowercase", accepted weak "lowercase1") with a clear policy in utils/password_policy.js: >= 8 chars and either 12+ chars or 3-of-4 character classes. Enforced server-side on create and password changes, mirrored in public/lib/js/val.js, with unit tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
+3
-3
@@ -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"
|
||||
|
||||
@@ -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)";
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+15
-1
@@ -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){
|
||||
|
||||
@@ -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
|
||||
});
|
||||
});
|
||||
@@ -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};
|
||||
@@ -82,14 +82,14 @@
|
||||
</div>
|
||||
<div class="form-group">
|
||||
<label class="control-label">Password</label>
|
||||
<input type="password" class="form-control" name="password" placeholder="Atleast 5 char. long" validate="password:5"/>
|
||||
<input type="password" class="form-control" name="password" placeholder="8+ chars; mix upper/lower/number/symbol (or 12+)" validate="password"/>
|
||||
</div>
|
||||
<div class="form-group">
|
||||
<label class="control-label">Again</label>
|
||||
<input type="password" class="form-control" name="passwordMatch" placeholder="Retype password" validate="eq:password"/>
|
||||
</div>
|
||||
<hr />
|
||||
<button type="button" class="btn btn-info">
|
||||
<button type="submit" class="btn btn-info">
|
||||
Add
|
||||
</button>
|
||||
</form>
|
||||
|
||||
Reference in New Issue
Block a user