diff --git a/backend/__tests__/integration/oidcSso.test.js b/backend/__tests__/integration/oidcSso.test.js index a1b6e0b7..19d3299f 100644 --- a/backend/__tests__/integration/oidcSso.test.js +++ b/backend/__tests__/integration/oidcSso.test.js @@ -222,6 +222,21 @@ describe('OIDC SSO (#798)', () => { expect(cfg.clientSecret).toBe(idp.clientSecret); }); + it('refuses local password login for OIDC-owned accounts', async () => { + // Give the JIT admin a KNOWN password hash directly in the DB — the + // auth_provider check must reject the login even with valid credentials + // (otherwise a password reset would mint an IdP-bypassing local login). + await db('admin_users').where({ id: agentCookies.jitAdminId }).update({ + password_hash: await bcrypt.hash('KnownPass123', 4), + }); + const row = await db('admin_users').where({ id: agentCookies.jitAdminId }).first(); + + const res = await request(app) + .post('/api/auth/admin/login') + .send({ username: row.email, password: 'KnownPass123' }); + expect(res.status).toBe(401); + }); + it('returns 404 from /sso/login when SSO is disabled', async () => { await oidcService.saveOidcSettings({ oidc_enabled: false }); await request(app).get('/api/auth/admin/sso/login').expect(404); diff --git a/backend/__tests__/routes/authSession.symmetry.test.js b/backend/__tests__/routes/authSession.symmetry.test.js index 50ba848f..7b78e61e 100644 --- a/backend/__tests__/routes/authSession.symmetry.test.js +++ b/backend/__tests__/routes/authSession.symmetry.test.js @@ -32,9 +32,17 @@ jest.mock('../../src/database/db', () => { if (table === 'admin_users') { let rowFilter = () => true; return { + // The session route joins roles for the adminUser payload (#798); + // fake rows carry no role fields, so the join is a pass-through. + leftJoin() { + return this; + }, where(criteria) { rowFilter = (row) => { - return Object.entries(criteria).every(([k, v]) => { + return Object.entries(criteria).every(([rawKey, v]) => { + // Joined queries prefix columns ('admin_users.id') — the fake + // rows use bare names. + const k = rawKey.replace(/^admin_users\./, ''); if (k === 'is_active') return Boolean(row.is_active) === Boolean(v); return row[k] === v; }); @@ -50,7 +58,12 @@ jest.mock('../../src/database/db', () => { if (!row) return undefined; if (!this._cols) return row; const out = {}; - for (const c of this._cols) out[c] = row[c]; + for (const c of this._cols) { + // Support 'table.col' and 'table.col as alias' shapes. + const [source, alias] = c.split(/\s+as\s+/i); + const bare = source.includes('.') ? source.split('.').pop() : source; + out[alias || bare] = row[bare]; + } return out; }, }; diff --git a/backend/src/routes/auth.js b/backend/src/routes/auth.js index 32299f81..31e9c9c6 100644 --- a/backend/src/routes/auth.js +++ b/backend/src/routes/auth.js @@ -143,8 +143,11 @@ router.post('/admin/login', [ ) .first(); - // Use generic error to prevent user enumeration - if (!admin || !await bcrypt.compare(password, admin.password_hash)) { + // Use generic error to prevent user enumeration. OIDC-owned accounts + // (#798) never authenticate locally — their random hash is unusable by + // design, and the explicit check keeps that true even if a hash ever + // gets set through some other path. + if (!admin || admin.auth_provider === 'oidc' || !await bcrypt.compare(password, admin.password_hash)) { await trackFailedAttempt(username, ipAddress, userAgent); return res.status(401).json({ error: getGenericAuthError() }); } @@ -654,12 +657,22 @@ router.get('/session', async (req, res) => { // or the gallery event was archived/deleted. Mirror those checks // here so the session endpoint is always at least as strict as // what the protected endpoints will enforce next. + // Full user payload for admin sessions — the SSO callback establishes + // the session via redirect (no JSON response the SPA could store), so + // session restoration must be able to hydrate the user object (#798). + let adminUser = null; + if (decoded.type === 'admin') { let admin = null; try { admin = await db('admin_users') - .where({ id: decoded.id, is_active: formatBoolean(true) }) - .select('id', 'username', 'email', 'password_changed_at') + .leftJoin('roles', 'roles.id', 'admin_users.role_id') + .where({ 'admin_users.id': decoded.id, 'admin_users.is_active': formatBoolean(true) }) + .select( + 'admin_users.id', 'admin_users.username', 'admin_users.email', + 'admin_users.password_changed_at', 'admin_users.must_change_password', + 'roles.name as role_name', 'roles.display_name as role_display_name' + ) .first(); } catch (lookupErr) { // admin_users table not present (test fixture, fresh DB) — fall @@ -698,6 +711,19 @@ router.get('/session', async (req, res) => { // Helper lookup failed (test stub may not export it) — fall through // and trust the token. Real deployments always have the middleware. } + + if (admin) { + adminUser = { + id: admin.id, + username: admin.username, + email: admin.email, + mustChangePassword: admin.must_change_password || false, + role: admin.role_name ? { + name: admin.role_name, + displayName: admin.role_display_name + } : null + }; + } } else if (decoded.type === 'gallery') { try { const event = await db('events') @@ -729,7 +755,11 @@ router.get('/session', async (req, res) => { expiresIn: Math.floor(remainingTime), user: decoded.username || decoded.eventSlug, eventSlug: decoded.eventSlug, - adminUsername: decoded.username + adminUsername: decoded.username, + // Full admin payload (or null) — lets the SPA hydrate its user + // state after a redirect-established session (SSO, #798) where no + // login JSON response ever reached it. + adminUser }); } catch (err) { res.json({ @@ -894,7 +924,11 @@ router.get('/admin/sso/login', async (req, res) => { return res.status(404).json({ error: 'SSO is not enabled' }); } logger.error('OIDC login initiation failed', { error: error.message }); - return res.redirect('/admin/login?sso_error=config'); + // Absolute like the callback's redirects: in split-origin deployments a + // relative path would resolve on the API origin and 404. + const { getFrontendBaseUrl } = require('../utils/frontendUrl'); + const frontendBase = (await getFrontendBaseUrl().catch(() => '')) || ''; + return res.redirect(`${frontendBase}/admin/login?sso_error=config`); } }); diff --git a/backend/src/services/userManagementService.js b/backend/src/services/userManagementService.js index fc51a34f..dc95dee9 100644 --- a/backend/src/services/userManagementService.js +++ b/backend/src/services/userManagementService.js @@ -459,6 +459,13 @@ async function resetAdminPassword(id, resetById) { throw new NotFoundError('Admin user', id); } + // OIDC-owned accounts (#798) have no usable local password by design — + // minting one here would hand out a login that bypasses the IdP's MFA + // and access policies. + if (user.auth_provider === 'oidc') { + throw new ValidationError('This account is managed by your identity provider (SSO) — reset the password there.'); + } + const newPassword = generateReadablePassword(); const passwordHash = await bcrypt.hash(newPassword, getBcryptRounds()); diff --git a/docker-compose.yml b/docker-compose.yml index 786c6652..978cd902 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -60,6 +60,9 @@ services: - SMTP_PASS=${SMTP_PASS} - EMAIL_FROM=${EMAIL_FROM:-noreply@picpeak.local} - FRONTEND_URL=${FRONTEND_URL:-http://localhost:3000} + # Public API origin for split-origin deployments (#798 SSO redirect_uri). + # Empty = same origin as FRONTEND_URL (the standard proxied setup). + - API_URL=${API_URL:-} - ADMIN_URL=${ADMIN_URL:-http://localhost:3001} - TZ=${TZ:-UTC} - STORAGE_PATH=/app/storage diff --git a/frontend/src/contexts/AdminAuthContext.tsx b/frontend/src/contexts/AdminAuthContext.tsx index 4f71bc87..6b943397 100644 --- a/frontend/src/contexts/AdminAuthContext.tsx +++ b/frontend/src/contexts/AdminAuthContext.tsx @@ -50,12 +50,19 @@ export const AdminAuthProvider: React.FC = ({ children } } } - const response = await api.get<{ valid: boolean; type: string; adminUsername?: string; user?: string }>( + const response = await api.get<{ valid: boolean; type: string; adminUsername?: string; user?: string; adminUser?: AdminUser | null }>( '/auth/session' ); if (response.data?.valid && response.data.type === 'admin') { setIsAuthenticated(true); + // Redirect-established sessions (SSO, #798) never went through + // login(), so sessionStorage has no user — hydrate from the + // session payload. Server truth also refreshes stale local copies. + if (response.data.adminUser) { + setUser(response.data.adminUser); + sessionStorage.setItem('admin_user', JSON.stringify(response.data.adminUser)); + } } else { sessionStorage.removeItem('admin_user'); setIsAuthenticated(false);