Make basic auth and SSO mutually exclusive per host; fix silently-broken validation errors
- Auth tab is now a single choice (Off / Basic auth / SSO) instead of two independent toggles that could both be on at once, which made it ambiguous which gate actually protected a request. Enforced both in the UI and server-side (POST/PUT), accounting for partial PUT updates against the existing record. - Add per-user basic-auth management (change password, delete) so an admin no longer has to blow away and retype the whole user list to remove or rotate one account. - Fix: `Model.errors.ObjectValidateError(...)` is a constructor and was being called without `new` everywhere in this codebase. Without `new`, `this` inside it was the module's shared `errors` object (mutated in place) and the call evaluated to `undefined` — so every `throw Model.errors.ObjectValidateError(...)` actually threw `undefined`, which Express's `next(undefined)` treats as "no error" and silently falls through to the catch-all 404 handler. Every host/user/group/ permission/dns-provider validation error (bad hostname, bad IP, etc.) was showing a confusing "Page not found" instead of the real message. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
+57
-4
@@ -6,7 +6,7 @@ const {Host, Domain, User} = require('../models').models;
|
||||
const {LocalGroup} = require('../models/local_group');
|
||||
const {Permission} = require('../models/permission');
|
||||
const authz = require('../middleware/authz');
|
||||
const {normalizeHostFeatures} = require('../utils/host_features');
|
||||
const {normalizeHostFeatures, sanitizeBasicAuthObject} = require('../utils/host_features');
|
||||
const {collectHostFieldErrors} = require('../utils/hostname_validate');
|
||||
const {hashBasicAuthUsers} = require('../utils/basicauth');
|
||||
|
||||
@@ -16,7 +16,22 @@ const Model = Host;
|
||||
// ObjectValidateError (per-field keys) that the frontend surfaces inline.
|
||||
function validateHostFields(body){
|
||||
let errors = collectHostFieldErrors(body);
|
||||
if(errors.length) throw Model.errors.ObjectValidateError(errors);
|
||||
if(errors.length) throw new Model.errors.ObjectValidateError(errors);
|
||||
}
|
||||
|
||||
// Basic auth and SSO are mutually exclusive per host (having both enabled
|
||||
// invites confusion about which gate actually protected a request). `existing`
|
||||
// is the current record (undefined on create), so a partial PUT that only
|
||||
// touches one of the two fields is still checked against the other's current
|
||||
// value.
|
||||
function validateAuthExclusivity(body, existing){
|
||||
let basic = 'basicauth_enabled' in body ? body.basicauth_enabled : (existing ? existing.basicauth_enabled : false);
|
||||
let sso = 'sso_enabled' in body ? body.sso_enabled : (existing ? existing.sso_enabled : false);
|
||||
if(basic && sso){
|
||||
throw new Model.errors.ObjectValidateError([
|
||||
{key: 'sso_enabled', message: 'Basic auth and SSO cannot both be enabled for the same host — pick one.'},
|
||||
]);
|
||||
}
|
||||
}
|
||||
|
||||
// After normalizeHostFeatures has parsed basic-auth creds to {user: plaintext},
|
||||
@@ -73,6 +88,7 @@ router.post('/', authz.requireDomainRole('manager', authz.resolve.hostBody), asy
|
||||
req.body.created_by = authz.reqUsername(req);
|
||||
validateHostFields(req.body);
|
||||
normalizeHostFeatures(req.body);
|
||||
validateAuthExclusivity(req.body);
|
||||
hashHostSecrets(req.body);
|
||||
let item = await Model.create(req.body);
|
||||
|
||||
@@ -139,9 +155,10 @@ router.put('/:item', authz.requireDomainRole('manager', authz.resolve.hostParam)
|
||||
req.body.updated_by = authz.reqUsername(req);
|
||||
validateHostFields(req.body);
|
||||
normalizeHostFeatures(req.body);
|
||||
let existing = await Model.get(req.params.item);
|
||||
validateAuthExclusivity(req.body, existing);
|
||||
hashHostSecrets(req.body);
|
||||
let item = await Model.get(req.params.item);
|
||||
item = await item.update(req.body);
|
||||
let item = await existing.update(req.body);
|
||||
|
||||
return res.json({
|
||||
message: `"${req.params.item}" updated.`,
|
||||
@@ -170,6 +187,42 @@ router.delete('/:item', authz.requireDomainRole('manager', authz.resolve.hostPar
|
||||
}
|
||||
});
|
||||
|
||||
// Manage a single basic-auth user without replacing the whole list — the bulk
|
||||
// PUT /:item endpoint always replaces basicauth_users wholesale (an empty
|
||||
// textarea there means "leave existing users untouched", see
|
||||
// normalizeHostFeatures), which makes deleting or rotating one user's
|
||||
// password error-prone from that form. These two routes touch exactly one key.
|
||||
router.put('/:item/basicauth-user/:username', authz.requireDomainRole('manager', authz.resolve.hostParam), async function(req, res, next){
|
||||
try{
|
||||
let item = await Model.get(req.params.item);
|
||||
let sanitized = sanitizeBasicAuthObject({[req.params.username]: req.body.password});
|
||||
let username = Object.keys(sanitized)[0];
|
||||
if(!username){
|
||||
throw new Model.errors.ObjectValidateError([{key: 'password', message: 'Invalid username or empty password.'}]);
|
||||
}
|
||||
|
||||
let users = Object.assign({}, item.basicauth_users, hashBasicAuthUsers(sanitized));
|
||||
item = await item.update({basicauth_users: users, updated_by: authz.reqUsername(req)});
|
||||
|
||||
return res.json({message: `User "${username}" saved.`, ...item});
|
||||
}catch(error){
|
||||
next(error);
|
||||
}
|
||||
});
|
||||
|
||||
router.delete('/:item/basicauth-user/:username', authz.requireDomainRole('manager', authz.resolve.hostParam), async function(req, res, next){
|
||||
try{
|
||||
let item = await Model.get(req.params.item);
|
||||
let users = Object.assign({}, item.basicauth_users);
|
||||
delete users[req.params.username];
|
||||
item = await item.update({basicauth_users: users, updated_by: authz.reqUsername(req)});
|
||||
|
||||
return res.json({message: `User "${req.params.username}" removed.`, ...item});
|
||||
}catch(error){
|
||||
next(error);
|
||||
}
|
||||
});
|
||||
|
||||
router.put('/:item/renew', authz.requireDomainRole('manager', authz.resolve.hostParam), async function(req, res, next){
|
||||
try{
|
||||
let item = await Model.get(req.params.item);
|
||||
|
||||
Reference in New Issue
Block a user