From fc17032649d60ec5951199fb29bae119201af7b2 Mon Sep 17 00:00:00 2001 From: Daniel Date: Tue, 14 Apr 2026 04:35:00 +0200 Subject: [PATCH] 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. --- public/js/auth.js | 10 +++++++++- src/routes/auth.js | 16 ++++++++++++++++ src/routes/oidc.js | 18 +++++++++++++++++- 3 files changed, 42 insertions(+), 2 deletions(-) diff --git a/public/js/auth.js b/public/js/auth.js index b16805c..7d21d30 100644 --- a/public/js/auth.js +++ b/public/js/auth.js @@ -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); } diff --git a/src/routes/auth.js b/src/routes/auth.js index 6217029..1243911 100644 --- a/src/routes/auth.js +++ b/src/routes/auth.js @@ -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' }); diff --git a/src/routes/oidc.js b/src/routes/oidc.js index ae75126..3776983 100644 --- a/src/routes/oidc.js +++ b/src/routes/oidc.js @@ -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');