Server-side SSO/local-auth enforcement + OIDC account-link hardening
Endpoint guards (defense-in-depth over hidden UI):
- POST /api/auth/change-password: 400 with SSO-aware message if
the caller's stored password is not a real bcrypt/argon2 hash.
Prior behaviour was to fail at passwords.verify() with an
ambiguous "current password is incorrect".
- POST /api/auth/setup-2fa: 400 with same SSO-aware message for
SSO-only accounts. Prior behaviour allowed TOTP setup on an
account where it could never actually trigger (user never logs
in locally).
OIDC account-link safety (src/routes/oidc.js):
- Auto-link to an existing local account now requires the IdP to
assert email_verified=true in the ID token (or userinfo). If
absent/false, the callback redirects with ?error=email_unverified.
Prevents an attacker at a misconfigured IdP from taking over a
local account by claiming an email they don't own.
- If an existing user already has oidc_sub set and the incoming
sub is different, refuse with ?error=sub_mismatch. Prior
behaviour silently did nothing, hiding a potential attack.
- Audit 'oidc_linked' written on first successful link.
Frontend:
- Added user-facing messages for the two new SSO error codes.
This commit is contained in:
parent
e161c221c4
commit
fc17032649
3 changed files with 42 additions and 2 deletions
|
|
@ -61,7 +61,15 @@ document.addEventListener('DOMContentLoaded', function() {
|
|||
.catch(function() { showAuthScreen(); clearSession(); });
|
||||
} else if (ssoError) {
|
||||
history.replaceState(null, '', window.location.pathname);
|
||||
var errorMsgs = { invalid_state: 'SSO session expired', expired: 'SSO session expired', no_email: 'Your identity provider did not return an email', disabled: 'Account disabled', sso_failed: 'SSO login failed' };
|
||||
var errorMsgs = {
|
||||
invalid_state: 'SSO session expired',
|
||||
expired: 'SSO session expired',
|
||||
no_email: 'Your identity provider did not return an email',
|
||||
disabled: 'Account disabled',
|
||||
sso_failed: 'SSO login failed',
|
||||
email_unverified: 'Your identity provider did not confirm your email is verified. Contact your administrator.',
|
||||
sub_mismatch: 'Your SSO identity does not match the linked account. Contact your administrator.'
|
||||
};
|
||||
showAuthScreen();
|
||||
setTimeout(function() { showToast(errorMsgs[ssoError] || 'SSO error', 'error'); }, 300);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -462,6 +462,12 @@ router.get('/2fa/backup-codes/count', authMiddleware, async (req, res) => {
|
|||
// 2FA
|
||||
router.post('/setup-2fa', authMiddleware, async (req, res) => {
|
||||
try {
|
||||
// SSO-only accounts never go through the local password flow, so TOTP
|
||||
// would sit dormant. Reject up front with a clear message.
|
||||
var row = await db.get('SELECT password FROM users WHERE id = ?', [req.user.id]);
|
||||
if (!row || !hasLocalPassword(row.password)) {
|
||||
return res.status(400).json({ error: 'This account uses SSO. Two-factor authentication is managed by your identity provider.' });
|
||||
}
|
||||
var secret = speakeasy.generateSecret({ name: 'PedScribe (' + req.user.email + ')', issuer: 'Pediatric AI Scribe' });
|
||||
await db.run('UPDATE users SET totp_secret = ? WHERE id = ?', [secret.base32, req.user.id]);
|
||||
var qrUrl = await QRCode.toDataURL(secret.otpauth_url);
|
||||
|
|
@ -563,6 +569,13 @@ router.post('/logout', async function(req, res) {
|
|||
res.json({ success: true });
|
||||
});
|
||||
|
||||
// Returns true if the user has a real password hash and can change it.
|
||||
// SSO-auto-created users have a random hex blob in `password` — changing
|
||||
// it is meaningless because they never authenticate locally.
|
||||
function hasLocalPassword(hash) {
|
||||
return !!(hash && (/^\$2[aby]\$/.test(hash) || hash.indexOf('$argon2') === 0));
|
||||
}
|
||||
|
||||
// Change password (requires current password)
|
||||
router.post('/change-password', authMiddleware, async (req, res) => {
|
||||
try {
|
||||
|
|
@ -571,6 +584,9 @@ router.post('/change-password', authMiddleware, async (req, res) => {
|
|||
if (newPassword.length < 8) return res.status(400).json({ error: 'New password must be 8+ characters' });
|
||||
|
||||
var user = await db.get('SELECT password FROM users WHERE id = ?', [req.user.id]);
|
||||
if (!hasLocalPassword(user.password)) {
|
||||
return res.status(400).json({ error: 'This account uses SSO. Password changes are managed by your identity provider.' });
|
||||
}
|
||||
var valid = await passwords.verify(currentPassword, user.password);
|
||||
if (!valid) return res.status(401).json({ error: 'Current password is incorrect' });
|
||||
|
||||
|
|
|
|||
|
|
@ -172,12 +172,14 @@ router.get('/oidc/callback', async function(req, res) {
|
|||
var claims = tokens.claims();
|
||||
var sub = claims.sub;
|
||||
var email = claims.email;
|
||||
var emailVerified = claims.email_verified;
|
||||
var name = claims.name || claims.preferred_username || email;
|
||||
|
||||
if (!email) {
|
||||
// Try userinfo endpoint
|
||||
var userinfo = await oidc.fetchUserInfo(config, tokens.access_token, sub);
|
||||
email = userinfo.email;
|
||||
if (typeof emailVerified === 'undefined') emailVerified = userinfo.email_verified;
|
||||
name = name || userinfo.name || userinfo.preferred_username || email;
|
||||
}
|
||||
|
||||
|
|
@ -191,9 +193,23 @@ router.get('/oidc/callback', async function(req, res) {
|
|||
var user = await db.get('SELECT * FROM users WHERE email = ?', [email]);
|
||||
|
||||
if (user) {
|
||||
// Existing user — link OIDC sub if not already linked
|
||||
// Existing user. Two safe linking paths:
|
||||
// 1. Already linked (oidc_sub matches) → log in
|
||||
// 2. Not yet linked → auto-link ONLY if the IdP asserts the email
|
||||
// is verified. Otherwise an attacker at the IdP could squat on
|
||||
// an email they don't own and take over the local account.
|
||||
if (user.oidc_sub && user.oidc_sub !== sub) {
|
||||
console.warn('[OIDC] sub mismatch on existing user id=' + user.id + ' — IdP returned different sub than recorded. Refusing to auto-relink.');
|
||||
return res.redirect(appUrl + '?error=sub_mismatch');
|
||||
}
|
||||
if (!user.oidc_sub) {
|
||||
if (emailVerified !== true) {
|
||||
console.warn('[OIDC] refusing to auto-link existing user ' + user.id + ' — IdP did not assert email_verified=true');
|
||||
return res.redirect(appUrl + '?error=email_unverified');
|
||||
}
|
||||
await db.run('UPDATE users SET oidc_sub = ?, email_verified = true WHERE id = ?', [sub, user.id]);
|
||||
await db.run('INSERT INTO audit_log (user_id, action, category, details, ip_address) VALUES (?, ?, ?, ?, ?)',
|
||||
[user.id, 'oidc_linked', 'auth', 'Linked SSO identity ' + sub + ' via ' + issuer, req.ip]).catch(function(){});
|
||||
}
|
||||
if (user.disabled) {
|
||||
return res.redirect(appUrl + '?error=disabled');
|
||||
|
|
|
|||
Loading…
Reference in a new issue