From 797eab7c8d5eecc0ad6fd7a53b07551b90f5df65 Mon Sep 17 00:00:00 2001 From: ScreenTinker Date: Fri, 5 Jun 2026 09:58:46 -0500 Subject: [PATCH 1/6] refactor(roles): normalize the platform-role model (#14) The legacy /api/auth/users dropdown could write 'superadmin' and 'admin' role strings that not every code path recognized. Some checks matched only 'platform_admin' (tenancy accessContext/resolveTenancy), so a 'superadmin' user could list orgs but not act-as into them. Normalize to the current two-tier platform model (users.role holds the PLATFORM role only; org/workspace roles live in the membership tables): - Migration (idempotent, exact-string): superadmin -> platform_admin, admin -> user. No-ops on rows already in the current model. - Add isPlatformRole() helper in middleware/auth.js; route the two superadmin-excluding checks in tenancy.js through it so a stray 'superadmin' is never treated as lower-privileged (fixes act-as). - Remove the dead/stricter requirePlatformAdmin in permissions.js (bare === 'platform_admin'); the single guard is the one in middleware/auth.js. - Recovery-token default role admin -> platform_admin so emergency recovery keeps full access once 'admin' no longer implies elevation. - PUT /api/auth/users/:id/role whitelist -> ['user','platform_admin']; self-demote guard retargeted via isPlatformRole. - Frontend: platform user-management dropdown now offers User / Platform admin only; owner-delete guard and settings highlight use isPlatformAdmin. EN i18n: add admin.role.platform_admin. Behaviour is identical under HOSTED_INSTANCE set or unset; the migration only touches exact legacy strings. Co-Authored-By: Claude Opus 4.8 (1M context) --- frontend/js/i18n/en.js | 3 +++ frontend/js/views/admin.js | 12 ++++++++---- frontend/js/views/settings.js | 7 +++++-- server/db/database.js | 11 +++++++++++ server/lib/permissions.js | 12 +++++------- server/lib/tenancy.js | 11 ++++++++--- server/middleware/auth.js | 19 +++++++++++++++++-- server/routes/auth.js | 15 +++++++++++---- 8 files changed, 68 insertions(+), 22 deletions(-) diff --git a/frontend/js/i18n/en.js b/frontend/js/i18n/en.js index ea00789..ad0aa90 100644 --- a/frontend/js/i18n/en.js +++ b/frontend/js/i18n/en.js @@ -799,6 +799,9 @@ export default { 'admin.col.monthly': 'Monthly', 'admin.col.yearly': 'Yearly', 'admin.role.user': 'User', + 'admin.role.platform_admin': 'Platform admin', + // Legacy labels kept for back-compat with any not-yet-normalized data; the + // role dropdown no longer offers these (#14 normalization). 'admin.role.admin': 'Admin', 'admin.role.superadmin': 'Superadmin', 'admin.remove': 'Remove', diff --git a/frontend/js/views/admin.js b/frontend/js/views/admin.js index 307eac2..18fcf29 100644 --- a/frontend/js/views/admin.js +++ b/frontend/js/views/admin.js @@ -6,6 +6,12 @@ import { t } from '../i18n.js'; const headers = () => ({ Authorization: `Bearer ${localStorage.getItem('token')}`, 'Content-Type': 'application/json' }); const API = (url, opts = {}) => fetch('/api' + url, { headers: headers(), ...opts }).then(r => r.json()); +// #14: the platform user-management dropdown manages users.role (the +// PLATFORM-level role) only - workspace/org roles are managed in the members +// views. Options are the current model; the legacy 'admin'/'superadmin' strings +// were normalized away. (#13 adds 'platform_operator' to this list.) +const PLATFORM_ROLE_OPTIONS = ['user', 'platform_admin']; + export async function render(container) { const user = JSON.parse(localStorage.getItem('user') || '{}'); if (!isPlatformAdmin(user)) { @@ -65,9 +71,7 @@ async function loadUsers() { ${u.last_login ? new Date(u.last_login * 1000).toLocaleString() : t('common.never')} @@ -77,7 +81,7 @@ async function loadUsers() { ${u.auth_provider === 'local' && u.id !== currentUser.id ? `` : ''} - ${u.role !== 'superadmin' ? `` : `${t('admin.owner')}`} + ${!isPlatformAdmin(u) ? `` : `${t('admin.owner')}`} `).join('')} diff --git a/frontend/js/views/settings.js b/frontend/js/views/settings.js index 84e370f..28693c2 100644 --- a/frontend/js/views/settings.js +++ b/frontend/js/views/settings.js @@ -12,7 +12,10 @@ export async function render(container) { try { user = await api.getMe(); localStorage.setItem('user', JSON.stringify(user)); } catch { user = JSON.parse(localStorage.getItem('user') || '{}'); } const isSuperAdmin = isPlatformAdmin(user); - const isAdmin = user.role === 'admin' || isSuperAdmin; + // #14: the legacy 'admin' platform role was normalized away; platform-level + // admin is now just isPlatformAdmin. (Elevated capability otherwise comes from + // org/workspace membership, gated in the members views, not users.role.) + const isAdmin = isSuperAdmin; container.innerHTML = `
@@ -430,7 +433,7 @@ async function loadUsers() { ${u.auth_provider} - ${u.role} + ${u.role} +
+
+ + +
+
+ +
+ + +
+
+
+ + +
+ + + +
+ + +
+ + `; + document.body.appendChild(overlay); + + const emailInput = overlay.querySelector('#addUserEmail'); + const nameInput = overlay.querySelector('#addUserName'); + const pwInput = overlay.querySelector('#addUserPassword'); + const genBtn = overlay.querySelector('#addUserGenerate'); + const roleSelect = overlay.querySelector('#addUserRole'); + const mustChange = overlay.querySelector('#addUserMustChange'); + const errorEl = overlay.querySelector('#addUserError'); + const submitBtn = overlay.querySelector('#addUserSubmit'); + emailInput.focus(); + + function close() { overlay.remove(); document.removeEventListener('keydown', onKey); } + function onKey(e) { if (e.key === 'Escape') close(); } + document.addEventListener('keydown', onKey); + overlay.addEventListener('click', (e) => { if (e.target === overlay) close(); }); + overlay.querySelectorAll('[data-add-close]').forEach(b => b.addEventListener('click', close)); + genBtn.addEventListener('click', () => { pwInput.value = generatePassword(); pwInput.type = 'text'; }); + + async function submit() { + errorEl.style.display = 'none'; + const email = emailInput.value.trim().toLowerCase(); + const name = nameInput.value.trim(); + const password = pwInput.value; + const role = roleSelect.value; + if (!email || !EMAIL_RE.test(email)) { showError(t('members.error.invalid_email')); emailInput.focus(); return; } + if (!password || password.length < 8) { showError(t('members.error.password_min_8')); pwInput.focus(); return; } + + submitBtn.disabled = true; + submitBtn.textContent = t('members.modal.creating'); + try { + const result = await api.adminCreateUser({ + email, name, password, role, + workspaceId: workspace.id, + mustChangePassword: mustChange.checked, + }); + close(); + if (typeof onSuccess === 'function') { + try { onSuccess(result); } + catch (e) { console.error('add-user modal onSuccess threw:', e); } + } + } catch (err) { + submitBtn.disabled = false; + submitBtn.textContent = t('members.modal.create'); + const msg = (typeof mapError === 'function') + ? mapError(err) + : (err?.message || t('members.error.mutation_generic', { error: '' })); + showError(msg); + } + } + + function showError(msg) { errorEl.textContent = msg; errorEl.style.display = 'block'; } + + submitBtn.addEventListener('click', submit); +} + +function esc(s) { + return String(s ?? '').replace(/[&<>"']/g, c => ({ '&':'&','<':'<','>':'>','"':'"',"'":''' }[c])); +} diff --git a/frontend/js/i18n/en.js b/frontend/js/i18n/en.js index db04ae4..0d2b5ca 100644 --- a/frontend/js/i18n/en.js +++ b/frontend/js/i18n/en.js @@ -1159,9 +1159,21 @@ export default { 'members.modal.send': 'Send invite', 'members.modal.sending': 'Sending...', + // Modal — Add User form (#10, admin-provisioned account) + 'members.modal.add_user_title': 'Add user to {workspace}', + 'members.modal.name_label': 'Name', + 'members.modal.name_placeholder': 'Full name (optional)', + 'members.modal.password_label': 'Password', + 'members.modal.password_placeholder': 'Set a password', + 'members.modal.generate': 'Generate', + 'members.modal.must_change_label': 'Require a password change on first login', + 'members.modal.create': 'Create user', + 'members.modal.creating': 'Creating...', + // Buttons — page header + per-row action affordances (titles double as // ARIA labels for the icon-only buttons). 'members.button.invite': 'Invite member', + 'members.button.add_user': 'Add user', 'members.button.remove': 'Remove member', 'members.button.cancel_invite': 'Cancel invite', @@ -1178,6 +1190,8 @@ export default { 'members.error.invalid_email': 'Please enter a valid email address.', 'members.error.org_owner_remove': 'Cannot remove the organization owner.', 'members.error.email_send_failed': 'Email send failed. Try again.', + 'members.error.user_exists': 'A user with that email already exists.', + 'members.error.password_min_8': 'Password must be at least 8 characters.', 'members.error.mutation_generic': 'Action failed: {error}', // Success toasts fired post-mutation. @@ -1185,6 +1199,23 @@ export default { 'members.success.invite_cancelled': 'Invite cancelled', 'members.success.role_changed': 'Role updated', 'members.success.member_removed': '{name} removed', + 'members.success.user_created': 'User {email} created', + + // Forced first-login password change (#10). Shown when an admin-provisioned + // user (must_change_password) is routed to #/change-password. + 'forcepw.title': 'Set a new password', + 'forcepw.subtitle': 'Your account was set up by an administrator. Choose your own password to continue.', + 'forcepw.current': 'Current password', + 'forcepw.new': 'New password', + 'forcepw.confirm': 'Confirm new password', + 'forcepw.hint': 'At least 8 characters.', + 'forcepw.submit': 'Set password', + 'forcepw.submitting': 'Saving...', + 'forcepw.success': 'Password updated', + 'forcepw.error_required': 'Enter your current and new password.', + 'forcepw.error_min8': 'Password must be at least 8 characters.', + 'forcepw.error_mismatch': "Passwords don't match.", + 'forcepw.error_generic': 'Could not update password. Try again.', // Accept-invite flow (Slice 2C). Toasts that fire post-accept on the // dashboard. Error variants share one helper in app.js's mapAcceptError(). diff --git a/frontend/js/views/force-password-change.js b/frontend/js/views/force-password-change.js new file mode 100644 index 0000000..c15307d --- /dev/null +++ b/frontend/js/views/force-password-change.js @@ -0,0 +1,81 @@ +// #10: forced first-login password change. When an admin provisions a user +// with must_change_password=1, route() in app.js redirects them here and blocks +// every other view until they set a new password. Reuses the same PUT /api/auth/me +// path as the Settings change-password form; on success the server clears +// must_change_password, we refresh the cached user, and return to the app. +import { api } from '../api.js'; +import { t } from '../i18n.js'; +import { showToast } from '../components/toast.js'; + +export async function render(container) { + container.innerHTML = ` +
+
+
+

${t('forcepw.title')}

+

${t('forcepw.subtitle')}

+
+
+
+ + +
+
+ + +
+
+ + +
+

${t('forcepw.hint')}

+ + +
+
+
+ `; + + const current = container.querySelector('#fpwCurrent'); + const next = container.querySelector('#fpwNew'); + const confirm = container.querySelector('#fpwConfirm'); + const submit = container.querySelector('#fpwSubmit'); + const errorEl = container.querySelector('#fpwError'); + current.focus(); + + const showError = (msg) => { errorEl.textContent = msg; errorEl.style.display = 'block'; }; + + async function doChange() { + errorEl.style.display = 'none'; + const cur = current.value; + const nw = next.value; + const cf = confirm.value; + if (!cur || !nw) { showError(t('forcepw.error_required')); return; } + if (nw.length < 8) { showError(t('forcepw.error_min8')); return; } + if (nw !== cf) { showError(t('forcepw.error_mismatch')); return; } + + submit.disabled = true; + submit.textContent = t('forcepw.submitting'); + try { + await api.updateMe({ password: nw, current_password: cur }); + // Refresh the cached user so the (now-cleared) must_change_password flag + // is reflected, then return to the app. + try { + const fresh = await api.getMe(); + localStorage.setItem('user', JSON.stringify(fresh)); + } catch { /* fall through; reload re-fetches */ } + showToast(t('forcepw.success'), 'success'); + window.location.hash = '#/'; + window.location.reload(); + } catch (err) { + submit.disabled = false; + submit.textContent = t('forcepw.submit'); + showError(err?.message || t('forcepw.error_generic')); + } + } + + submit.addEventListener('click', doChange); + [current, next, confirm].forEach(el => el.addEventListener('keydown', (e) => { if (e.key === 'Enter') doChange(); })); +} + +export function cleanup() {} diff --git a/frontend/js/views/workspace-members.js b/frontend/js/views/workspace-members.js index 2cca907..9422ee0 100644 --- a/frontend/js/views/workspace-members.js +++ b/frontend/js/views/workspace-members.js @@ -12,6 +12,7 @@ import { api } from '../api.js'; import { t } from '../i18n.js'; import { showToast } from '../components/toast.js'; import { openInviteMemberModal } from '../components/workspace-members-invite-modal.js'; +import { openAddUserModal } from '../components/workspace-members-add-user-modal.js'; export async function render(container, workspaceId) { container.innerHTML = ` @@ -62,10 +63,15 @@ export async function render(container, workspaceId) { } } - // Invite button - admin only, opens modal. + // Invite + Add User buttons - admin only. Invite is self-service (emails a + // link); Add User (#10) provisions an account directly with an admin-set + // password (for instances with no outbound email). They coexist. if (canAdmin) { headerActions.innerHTML = ` - +
+ + +
`; document.getElementById('inviteMemberBtn').addEventListener('click', () => { openInviteMemberModal({ id: workspaceId, name: workspaceName }, { @@ -76,6 +82,15 @@ export async function render(container, workspaceId) { mapError: mapMutationError, }); }); + document.getElementById('addUserBtn').addEventListener('click', () => { + openAddUserModal({ id: workspaceId, name: workspaceName }, { + onSuccess: (result) => { + showToast(t('members.success.user_created', { email: result.email }), 'success'); + render(container, workspaceId); + }, + mapError: mapMutationError, + }); + }); } const direct = members.filter(m => !m.via_org); @@ -264,6 +279,9 @@ export function mapMutationError(err) { if (/Cannot demote the last admin/i.test(msg)) return t('members.error.last_admin_demote'); if (/Cannot remove the last admin/i.test(msg)) return t('members.error.last_admin_remove'); if (/already a member/i.test(msg)) return t('members.error.already_member'); + // #10 Add User: duplicate email + weak password. + if (/user with that email already exists/i.test(msg)) return t('members.error.user_exists'); + if (/at least 8 characters/i.test(msg)) return t('members.error.password_min_8'); if (/Valid email required/i.test(msg)) return t('members.error.invalid_email'); if (/Cannot remove the organization owner/i.test(msg)) return t('members.error.org_owner_remove'); if (/Email send failed/i.test(msg)) return t('members.error.email_send_failed'); diff --git a/server/db/database.js b/server/db/database.js index 7104238..852c296 100644 --- a/server/db/database.js +++ b/server/db/database.js @@ -173,6 +173,10 @@ const migrations = [ // already in the current model ('user'/'platform_admin'/'platform_operator'). "UPDATE users SET role = 'platform_admin' WHERE role = 'superadmin'", "UPDATE users SET role = 'user' WHERE role = 'admin'", + // Issue #10: admin-provisioned users. When an admin creates a user with a + // known password, must_change_password=1 forces a password change on first + // login. Default 0 so all existing users are unaffected. + "ALTER TABLE users ADD COLUMN must_change_password INTEGER NOT NULL DEFAULT 0", ]; for (const sql of migrations) { try { db.exec(sql); } catch (e) { /* already exists */ } diff --git a/server/middleware/auth.js b/server/middleware/auth.js index 52660bb..04e3334 100644 --- a/server/middleware/auth.js +++ b/server/middleware/auth.js @@ -48,7 +48,7 @@ function requireAuth(req, res, next) { req.jwtWorkspaceId = null; return next(); } - const user = db.prepare('SELECT id, email, name, role, auth_provider, avatar_url, plan_id, email_alerts FROM users WHERE id = ?').get(decoded.id); + const user = db.prepare('SELECT id, email, name, role, auth_provider, avatar_url, plan_id, email_alerts, must_change_password FROM users WHERE id = ?').get(decoded.id); if (!user) return res.status(401).json({ error: 'User not found' }); req.user = user; // Tenancy middleware reads this on the resolver step. diff --git a/server/routes/admin.js b/server/routes/admin.js new file mode 100644 index 0000000..f20c45e --- /dev/null +++ b/server/routes/admin.js @@ -0,0 +1,102 @@ +const express = require('express'); +const router = express.Router(); +const bcrypt = require('bcryptjs'); +const { v4: uuidv4 } = require('uuid'); +const { db } = require('../db/database'); +const { canAdminWorkspace } = require('../lib/permissions'); +const { logActivity, getClientIp } = require('../services/activity'); + +// Admin-provisioned user creation (#10). Operates on a target workspace +// specified in the body, NOT the caller's active workspace - so this router is +// mounted with requireAuth only (no resolveTenancy), mirroring routes/workspaces.js. +// Permission is gated per-handler via canAdminWorkspace() against the TARGET +// workspace, which: +// - lets a platform_admin create users anywhere, +// - scopes an org_admin / org_owner to workspaces in orgs they administer, +// - and excludes platform_operator (isPlatformRole owner-only) - operators +// have no user/role-management power (#13). + +// Same email shape the invite-create endpoint validates against (workspaces.js). +const EMAIL_RE = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; +const WORKSPACE_ROLES = ['workspace_admin', 'workspace_editor', 'workspace_viewer']; +// Mirror the server-side minimum enforced by PUT /api/auth/me and register. +const MIN_PASSWORD_LENGTH = 8; + +// POST /api/admin/users - create a user with an admin-set password and assign +// them to a workspace + role. The result is indistinguishable from an +// invite-accepted user (a global users row + a workspace_members row). +router.post('/users', (req, res) => { + const email = String(req.body?.email || '').trim().toLowerCase(); + const name = String(req.body?.name || '').trim(); + const password = String(req.body?.password || ''); + // Accept workspaceId (preferred) or orgId as an alias for the target field. + const workspaceId = String(req.body?.workspaceId || req.body?.orgId || '').trim(); + const role = String(req.body?.role || '').trim(); + const mustChangePassword = !!req.body?.mustChangePassword; + + if (!email || !EMAIL_RE.test(email)) { + return res.status(400).json({ error: 'Valid email required' }); + } + if (!WORKSPACE_ROLES.includes(role)) { + return res.status(400).json({ error: 'Role must be workspace_admin, workspace_editor, or workspace_viewer' }); + } + if (password.length < MIN_PASSWORD_LENGTH) { + return res.status(400).json({ error: `Password must be at least ${MIN_PASSWORD_LENGTH} characters` }); + } + if (!workspaceId) { + return res.status(400).json({ error: 'workspaceId required' }); + } + + const ws = db.prepare('SELECT * FROM workspaces WHERE id = ?').get(workspaceId); + if (!ws) return res.status(404).json({ error: 'Workspace not found' }); + if (!canAdminWorkspace(db, req.user, ws)) { + return res.status(403).json({ error: 'Admin access required' }); + } + // Stamp the target workspace so the activityLogger middleware (and our + // explicit audit row) attribute to the right tenant. + req.workspaceId = ws.id; + + // Email uniqueness: clean 409, never overwrite an existing account. + const existing = db.prepare('SELECT id FROM users WHERE email = ?').get(email); + if (existing) { + return res.status(409).json({ error: 'A user with that email already exists' }); + } + + const id = uuidv4(); + const passwordHash = bcrypt.hashSync(password, 10); + + // HOSTED_INSTANCE: an admin-provisioned user is already set up with a + // password, so they must NOT receive the welcome email or enter the + // activation-nudge lifecycle. We never call sendSignupEmails here, and the + // nudge sweep already excludes them (they have a workspace_members row); we + // additionally stamp both *_sent_at sentinels so any future sweep treats them + // as already-handled. See services/signupEmails.js + services/activationNudge.js. + const txn = db.transaction(() => { + db.prepare(` + INSERT INTO users ( + id, email, name, password_hash, auth_provider, role, plan_id, + must_change_password, welcome_email_sent_at, activation_nudge_sent_at + ) VALUES (?, ?, ?, ?, 'local', 'user', 'free', ?, strftime('%s','now'), strftime('%s','now')) + `).run(id, email, name || email.split('@')[0], passwordHash, mustChangePassword ? 1 : 0); + + // Same membership footprint as an accepted invite: one workspace_members + // row, invited_by = the admin who created them. + db.prepare(` + INSERT INTO workspace_members (workspace_id, user_id, role, invited_by) + VALUES (?, ?, ?, ?) + `).run(ws.id, id, role, req.user.id); + }); + txn(); + + // Explicit audit row - who created whom, where, with what role. Never the + // plaintext password (and the generic activityLogger only summarizes name). + logActivity(req.user.id, 'admin_create_user', `target: ${email}, role: ${role}`, null, getClientIp(req), ws.id); + + // Response never includes password or hash. + const created = db.prepare( + 'SELECT id, email, name, role, auth_provider, plan_id, must_change_password, created_at FROM users WHERE id = ?' + ).get(id); + res.status(201).json({ ...created, workspace_id: ws.id, workspace_role: role }); +}); + +module.exports = router; diff --git a/server/routes/auth.js b/server/routes/auth.js index 196db92..ce7ed6b 100644 --- a/server/routes/auth.js +++ b/server/routes/auth.js @@ -429,10 +429,12 @@ router.put('/me', requireAuth, (req, res) => { } } const hash = bcrypt.hashSync(password, 10); - db.prepare('UPDATE users SET password_hash = ?, updated_at = strftime(\'%s\',\'now\') WHERE id = ?') + // #10: a successful password change clears must_change_password, releasing + // the first-login change-password gate. + db.prepare('UPDATE users SET password_hash = ?, must_change_password = 0, updated_at = strftime(\'%s\',\'now\') WHERE id = ?') .run(hash, req.user.id); } - const user = db.prepare('SELECT id, email, name, role, auth_provider, avatar_url, plan_id, email_alerts FROM users WHERE id = ?').get(req.user.id); + const user = db.prepare('SELECT id, email, name, role, auth_provider, avatar_url, plan_id, email_alerts, must_change_password FROM users WHERE id = ?').get(req.user.id); res.json(user); }); diff --git a/server/server.js b/server/server.js index fb3eeae..84bb4bc 100644 --- a/server/server.js +++ b/server/server.js @@ -365,6 +365,12 @@ app.use(activityLogger); // no resolveTenancy. Permission gated per-handler via canAdminWorkspace(). app.use('/api/workspaces', requireAuth, require('./routes/workspaces')); +// /api/admin: admin-provisioned user creation (#10). Like /api/workspaces it +// targets a workspace by body param (not the caller's active one), so +// requireAuth only - per-handler canAdminWorkspace() gates it. Mounted after +// activityLogger so creations are auto-logged. +app.use('/api/admin', requireAuth, require('./routes/admin')); + app.use('/api/devices', requireAuth, resolveTenancy, require('./routes/devices')); app.use('/api/content', requireAuth, resolveTenancy, require('./routes/content')); app.use('/api/folders', requireAuth, resolveTenancy, require('./routes/folders')); From 54549420e75fdd11b98e93ecc826968a362b2457 Mon Sep 17 00:00:00 2001 From: ScreenTinker Date: Fri, 5 Jun 2026 11:16:27 -0500 Subject: [PATCH 4/6] feat(signup): optional org-on-create for self-service signups (#12) MSP-style deployments want self-service signups created WITHOUT a personal org, so an admin/operator can assign them into an existing customer org afterward. - config.autoCreateOrgOnSignup (AUTO_CREATE_ORG_ON_SIGNUP env), default true - single-tenant and the hosted self-service flow are unchanged. - ensureDefaultOrgForUser gains { allowCreate }: an existing membership is always returned (idempotent); the MINT path is gated. allowCreate=false + no membership -> returns null (user created org-less). - register accepts a per-request createOrg flag overriding the deployment default; the first-ever user is always given an org (never headless). login / Google / Microsoft pass allowCreate from the global config, so an org-less user is not silently given an org on next sign-in. Edge case: a non-platform user with zero workspaces now lands on a "no workspaces yet" empty state (new no-workspace view) instead of being bounced into onboarding (whose pairing step needs a workspace). route() redirects them there, and refreshCurrentUser() redirects once /me reveals zero accessible_workspaces (covers the first-load race). The workspace switcher already rendered an empty placeholder and resource routes already return [] for a null workspace, so nothing crashes in between. Co-Authored-By: Claude Opus 4.8 (1M context) --- frontend/js/app.js | 46 +++++++++++++++++++++++++++++++ frontend/js/i18n/en.js | 5 ++++ frontend/js/views/no-workspace.js | 31 +++++++++++++++++++++ server/config.js | 6 ++++ server/routes/auth.js | 24 ++++++++++++---- 5 files changed, 106 insertions(+), 6 deletions(-) create mode 100644 frontend/js/views/no-workspace.js diff --git a/frontend/js/app.js b/frontend/js/app.js index dc31b78..12e2152 100644 --- a/frontend/js/app.js +++ b/frontend/js/app.js @@ -21,6 +21,7 @@ import * as designer from './views/designer.js'; import * as playlists from './views/playlists.js'; import * as workspaceMembers from './views/workspace-members.js'; import * as forcePasswordChange from './views/force-password-change.js'; +import * as noWorkspace from './views/no-workspace.js'; import { applyBranding } from './branding.js'; import { t } from './i18n.js'; import { isPlatformAdmin } from './utils.js'; @@ -211,6 +212,18 @@ function getCurrentUser() { } catch { return null; } } +// #12: true when a signed-in user provably has zero accessible workspaces and +// no platform-level reach. Requires accessible_workspaces to be present (only +// /me populates it) - undefined means "not loaded yet", so we DON'T trigger and +// fall through to the normal (workspace-empty-safe) views until /me resolves. +function hasNoAccessibleWorkspace(u) { + return !!u + && Array.isArray(u.accessible_workspaces) + && u.accessible_workspaces.length === 0 + && !u.current_workspace_id + && !isPlatformAdmin(u); +} + // Refresh the cached user from the server. The server reads plan_id fresh // from the DB on every request, but the frontend only wrote `user` into // localStorage at login — so plan/role changes made by an admin weren't @@ -227,6 +240,16 @@ async function refreshCurrentUser() { // the dropdown in sync if a workspace was added/removed in another tab. renderWorkspaceSwitcher(fresh); window.dispatchEvent(new CustomEvent('user-refreshed', { detail: fresh })); + // #12: /me is the first place accessible_workspaces is known. If it resolves + // to zero (org-less user), send them to the empty state now - on a fresh + // load route() may have already rendered the dashboard before /me returned. + // Guard against the login / change-password / already-there screens to avoid + // a redirect loop. + const hash = window.location.hash || '#/'; + if (hasNoAccessibleWorkspace(fresh) + && hash !== '#/no-workspace' && hash !== '#/login' && hash !== '#/change-password') { + window.location.hash = '#/no-workspace'; + } } catch {} } @@ -304,6 +327,29 @@ function route() { } } + // #12: a signed-in user with zero accessible workspaces (org-less self-signup + // on an AUTO_CREATE_ORG_ON_SIGNUP=false deployment) lands on a "no workspaces + // yet" empty state instead of being bounced into onboarding (whose pairing + // step needs a workspace). Only fires once /me has populated + // accessible_workspaces; until then the workspace-empty-safe dashboard shows. + if (isAuthenticated()) { + const u = getCurrentUser(); + if (hasNoAccessibleWorkspace(u) && hash !== '#/no-workspace') { + window.location.hash = '#/no-workspace'; + return; + } + if (hash === '#/no-workspace') { + if (!hasNoAccessibleWorkspace(u)) { window.location.hash = '#/'; return; } + sidebar.style.display = 'none'; + app.style.marginLeft = '0'; + const mb = document.getElementById('mobileMenuBtn'); + if (mb) mb.style.display = 'none'; + currentView = noWorkspace; + noWorkspace.render(app); + return; + } + } + // Onboarding for new users if (hash === '#/onboarding' && isAuthenticated()) { sidebar.style.display = 'none'; diff --git a/frontend/js/i18n/en.js b/frontend/js/i18n/en.js index 0d2b5ca..c1c1223 100644 --- a/frontend/js/i18n/en.js +++ b/frontend/js/i18n/en.js @@ -1217,6 +1217,11 @@ export default { 'forcepw.error_mismatch': "Passwords don't match.", 'forcepw.error_generic': 'Could not update password. Try again.', + // No-workspace empty state (#12): shown to an org-less signed-in user. + 'noworkspace.title': 'No workspaces yet', + 'noworkspace.body': "Your account isn't part of any workspace yet. Ask your administrator to add you to one, then sign in again.", + 'noworkspace.sign_out': 'Sign out', + // Accept-invite flow (Slice 2C). Toasts that fire post-accept on the // dashboard. Error variants share one helper in app.js's mapAcceptError(). 'accept.success': "You've joined {name}", diff --git a/frontend/js/views/no-workspace.js b/frontend/js/views/no-workspace.js new file mode 100644 index 0000000..29076fb --- /dev/null +++ b/frontend/js/views/no-workspace.js @@ -0,0 +1,31 @@ +// #12: empty state for a signed-in user who belongs to zero workspaces. Happens +// on deployments with AUTO_CREATE_ORG_ON_SIGNUP=false, where a self-service +// signup is created org-less and an admin/operator assigns them to a workspace +// afterward. Without this, such a user would be bounced into onboarding (whose +// device-pairing step needs a workspace) - a broken flow. Here they get a clear +// "ask your admin" message instead. +import { t } from '../i18n.js'; + +export function render(container) { + container.innerHTML = ` +
+
+ + + + +

${t('noworkspace.title')}

+

${t('noworkspace.body')}

+ +
+
+ `; + container.querySelector('#noWsSignOut').addEventListener('click', () => { + localStorage.removeItem('token'); + localStorage.removeItem('user'); + window.location.hash = '#/login'; + window.location.reload(); + }); +} + +export function cleanup() {} diff --git a/server/config.js b/server/config.js index 02b5d82..fa4d505 100644 --- a/server/config.js +++ b/server/config.js @@ -69,4 +69,10 @@ module.exports = { // Redirect / -> /app instead of serving the marketing landing page. // For self-hosted internal deployments that don't want the public homepage. disableHomepage: ['true', '1'].includes(String(process.env.DISABLE_HOMEPAGE || '').toLowerCase()), + // Issue #12: auto-create a personal org + Default workspace for self-service + // signups (public register + OAuth). Defaults TRUE so single-tenant and the + // hosted self-service flow are unaffected; set AUTO_CREATE_ORG_ON_SIGNUP=false + // on MSP-style deployments where an admin/operator assigns users to existing + // orgs after signup instead. + autoCreateOrgOnSignup: !['false', '0'].includes(String(process.env.AUTO_CREATE_ORG_ON_SIGNUP || '').toLowerCase()), }; diff --git a/server/routes/auth.js b/server/routes/auth.js index ce7ed6b..ff64cda 100644 --- a/server/routes/auth.js +++ b/server/routes/auth.js @@ -15,7 +15,11 @@ const config = require('../config'); // workspace_id to embed in the JWT. Idempotent: if the user already has // memberships (e.g. migrated from Phase 1), returns the first one without // creating anything. -function ensureDefaultOrgForUser(user) { +// #12: allowCreate gates the MINT path only. An existing membership is always +// returned (idempotent). When allowCreate is false and the user has no +// membership, returns null - the caller is created org-less and an admin / +// operator assigns them to a workspace afterward. +function ensureDefaultOrgForUser(user, { allowCreate = true } = {}) { const existing = db.prepare(` SELECT w.id FROM workspaces w JOIN workspace_members wm ON wm.workspace_id = w.id @@ -23,6 +27,7 @@ function ensureDefaultOrgForUser(user) { ORDER BY wm.joined_at ASC LIMIT 1 `).get(user.id); if (existing) return existing.id; + if (!allowCreate) return null; // No memberships -> mint a fresh org and Default workspace owned by user. const orgId = uuidv4(); @@ -85,7 +90,7 @@ router.post('/register', (req, res) => { if (!canRegister()) { return res.status(403).json({ error: 'Public registration is disabled. Contact your administrator.' }); } - const { email, password, name } = req.body; + const { email, password, name, createOrg } = req.body; if (!email || !password) return res.status(400).json({ error: 'Email and password required' }); if (password.length < 8) return res.status(400).json({ error: 'Password must be at least 8 characters' }); @@ -109,7 +114,14 @@ router.post('/register', (req, res) => { `).run(id, email.toLowerCase(), name || email.split('@')[0], passwordHash, role, plan, trialStarted, trialStarted ? 'pro' : null); const user = db.prepare('SELECT id, email, name, role, auth_provider, avatar_url, plan_id, stripe_customer_id, stripe_subscription_id, subscription_status, subscription_ends FROM users WHERE id = ?').get(id); - const workspaceId = ensureDefaultOrgForUser(user); + // #12: org-on-create. Per-request createOrg overrides the deployment default + // (config.autoCreateOrgOnSignup). The first user is always given an org so a + // fresh install is never left headless. When neither applies, the user is + // created org-less and lands on the "no workspaces yet" state until an admin + // assigns them. + const createOrgForUser = isFirstUser + || (createOrg !== undefined ? !!createOrg : config.autoCreateOrgOnSignup); + const workspaceId = ensureDefaultOrgForUser(user, { allowCreate: createOrgForUser }); const token = generateToken(user, workspaceId); res.status(201).json({ token, user, current_workspace_id: workspaceId }); @@ -135,7 +147,7 @@ router.post('/login', (req, res) => { } logSuccessfulLogin(user.id, email, getClientIp(req)); - const workspaceId = ensureDefaultOrgForUser(user); + const workspaceId = ensureDefaultOrgForUser(user, { allowCreate: config.autoCreateOrgOnSignup }); const token = generateToken(user, workspaceId); const { password_hash, ...safeUser } = user; res.json({ token, user: safeUser, current_workspace_id: workspaceId }); @@ -187,7 +199,7 @@ router.post('/google', async (req, res) => { user = db.prepare('SELECT * FROM users WHERE id = ?').get(user.id); } - const workspaceId = ensureDefaultOrgForUser(user); + const workspaceId = ensureDefaultOrgForUser(user, { allowCreate: config.autoCreateOrgOnSignup }); const token = generateToken(user, workspaceId); const { password_hash, ...safeUser } = user; res.json({ token, user: safeUser, current_workspace_id: workspaceId }); @@ -268,7 +280,7 @@ router.post('/microsoft', async (req, res) => { user = db.prepare('SELECT * FROM users WHERE id = ?').get(user.id); } - const workspaceId = ensureDefaultOrgForUser(user); + const workspaceId = ensureDefaultOrgForUser(user, { allowCreate: config.autoCreateOrgOnSignup }); const token = generateToken(user, workspaceId); const { password_hash, ...safeUser } = user; res.json({ token, user: safeUser, current_workspace_id: workspaceId }); From 7674f6dc9f0b27c412fc7a32e557410cf2f41031 Mon Sep 17 00:00:00 2001 From: ScreenTinker Date: Fri, 5 Jun 2026 11:23:06 -0500 Subject: [PATCH 5/6] test(admin): node:test coverage for Add User + role gating Adds server/test/admin-users.test.js and a `npm test` (node --test) script. No DB_PATH override: the suite mounts the real routers against an isolated in-memory better-sqlite3 instance injected into the require cache, seeded by the test itself. Node v20 built-ins only (node:test, node:assert, fetch). Covers: Add User success (response omits password/hash, hash stored not plaintext, membership written, hosted lifecycle sentinels stamped, audit row without the password), duplicate-email 409 (no overwrite), non-admin 403, platform_operator denied (403), org_admin scoped to their own org only, input validation, and the must_change_password lifecycle (set on create, surfaced on login, cleared on PUT /api/auth/me). Co-Authored-By: Claude Opus 4.8 (1M context) --- server/package.json | 3 +- server/test/admin-users.test.js | 226 ++++++++++++++++++++++++++++++++ 2 files changed, 228 insertions(+), 1 deletion(-) create mode 100644 server/test/admin-users.test.js diff --git a/server/package.json b/server/package.json index 9beba77..203d440 100644 --- a/server/package.json +++ b/server/package.json @@ -5,7 +5,8 @@ "main": "server.js", "scripts": { "start": "node --env-file-if-exists=.env server.js", - "dev": "node --watch --env-file-if-exists=.env server.js" + "dev": "node --watch --env-file-if-exists=.env server.js", + "test": "node --test" }, "dependencies": { "@azure/msal-node": "^5.2.1", diff --git a/server/test/admin-users.test.js b/server/test/admin-users.test.js new file mode 100644 index 0000000..44f0555 --- /dev/null +++ b/server/test/admin-users.test.js @@ -0,0 +1,226 @@ +'use strict'; + +// Tests for #10 (admin-provisioned user creation) and the must_change_password +// lifecycle, plus the #13 operator denial on this endpoint. +// +// No DB_PATH override (per project constraint): we mount the real routers +// against an isolated in-memory better-sqlite3 instance that we seed here, by +// injecting it into the require cache for ../db/database BEFORE any module that +// requires it is loaded. Node v20 built-ins only (node:test, node:assert, fetch). + +const test = require('node:test'); +const assert = require('node:assert/strict'); +const path = require('path'); +const Database = require('better-sqlite3'); + +process.env.JWT_SECRET = 'test-secret-admin-users'; + +// --- isolated in-memory DB + minimal schema (only what these paths touch) --- +const db = new Database(':memory:'); +db.pragma('foreign_keys = ON'); +db.exec(` + CREATE TABLE users ( + id TEXT PRIMARY KEY, + email TEXT UNIQUE NOT NULL, + name TEXT NOT NULL DEFAULT '', + password_hash TEXT, + auth_provider TEXT NOT NULL DEFAULT 'local', + provider_id TEXT, + avatar_url TEXT, + role TEXT NOT NULL DEFAULT 'user', + plan_id TEXT DEFAULT 'free', + email_alerts INTEGER DEFAULT 1, + must_change_password INTEGER NOT NULL DEFAULT 0, + welcome_email_sent_at INTEGER, + activation_nudge_sent_at INTEGER, + last_login INTEGER, + trial_started INTEGER, + trial_plan TEXT, + stripe_customer_id TEXT, + stripe_subscription_id TEXT, + subscription_status TEXT DEFAULT 'active', + subscription_ends INTEGER, + created_at INTEGER NOT NULL DEFAULT (strftime('%s','now')), + updated_at INTEGER NOT NULL DEFAULT (strftime('%s','now')) + ); + CREATE TABLE workspaces ( + id TEXT PRIMARY KEY, + organization_id TEXT NOT NULL, + name TEXT NOT NULL, + slug TEXT, + created_by TEXT, + created_at INTEGER NOT NULL DEFAULT (strftime('%s','now')), + updated_at INTEGER NOT NULL DEFAULT (strftime('%s','now')) + ); + CREATE TABLE organization_members ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + organization_id TEXT NOT NULL, + user_id TEXT NOT NULL, + role TEXT NOT NULL DEFAULT 'org_admin', + invited_by TEXT, + joined_at INTEGER NOT NULL DEFAULT (strftime('%s','now')), + UNIQUE(organization_id, user_id) + ); + CREATE TABLE workspace_members ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + workspace_id TEXT NOT NULL, + user_id TEXT NOT NULL, + role TEXT NOT NULL DEFAULT 'workspace_viewer', + invited_by TEXT, + joined_at INTEGER NOT NULL DEFAULT (strftime('%s','now')), + UNIQUE(workspace_id, user_id) + ); + CREATE TABLE activity_log ( + id INTEGER PRIMARY KEY AUTOINCREMENT, + user_id TEXT, + device_id TEXT, + action TEXT NOT NULL, + details TEXT, + ip_address TEXT, + workspace_id TEXT, + created_at INTEGER NOT NULL DEFAULT (strftime('%s','now')) + ); +`); + +// Inject the mock BEFORE requiring anything that pulls ../db/database. +const dbModulePath = require.resolve('../db/database'); +require.cache[dbModulePath] = { + id: dbModulePath, + filename: dbModulePath, + loaded: true, + exports: { db, pruneTelemetry() {}, pruneScreenshots() {} }, +}; + +const express = require('express'); +const bcrypt = require('bcryptjs'); +const { generateToken, requireAuth } = require('../middleware/auth'); +const { activityLogger } = require('../services/activity'); +const adminRouter = require('../routes/admin'); +const authRouter = require('../routes/auth'); + +// --- seed orgs/workspaces/users --- +db.prepare("INSERT INTO workspaces (id, organization_id, name) VALUES ('ws-a','org-a','Workspace A')").run(); +db.prepare("INSERT INTO workspaces (id, organization_id, name) VALUES ('ws-b','org-b','Workspace B')").run(); + +function seedUser({ id, email, role = 'user' }) { + db.prepare("INSERT INTO users (id, email, name, password_hash, auth_provider, role) VALUES (?, ?, ?, 'x', 'local', ?)") + .run(id, email, email.split('@')[0], role); + return { id, email, role }; +} +const adminUser = seedUser({ id: 'u-admin', email: 'admin@test.local', role: 'platform_admin' }); +const orgAdminA = seedUser({ id: 'u-orgadmin-a', email: 'orgadmin-a@test.local', role: 'user' }); +db.prepare("INSERT INTO organization_members (organization_id, user_id, role) VALUES ('org-a','u-orgadmin-a','org_admin')").run(); +const operator = seedUser({ id: 'u-operator', email: 'operator@test.local', role: 'platform_operator' }); +const regular = seedUser({ id: 'u-regular', email: 'regular@test.local', role: 'user' }); + +const tokens = { + admin: generateToken(adminUser, null), + orgAdminA: generateToken(orgAdminA, 'ws-a'), + operator: generateToken(operator, null), + regular: generateToken(regular, null), +}; + +// --- build + start the app --- +const app = express(); +app.use(express.json()); +app.use('/api/auth', authRouter); // matches prod: auth before activityLogger +app.use(activityLogger); +app.use('/api/admin', requireAuth, adminRouter); +const server = app.listen(0); +let base; +test.before(async () => { + await new Promise(r => server.listening ? r() : server.once('listening', r)); + base = `http://127.0.0.1:${server.address().port}`; +}); +test.after(() => { server.close(); db.close(); }); + +function post(pathname, token, body) { + return fetch(base + pathname, { + method: 'POST', + headers: { 'Content-Type': 'application/json', ...(token ? { Authorization: `Bearer ${token}` } : {}) }, + body: JSON.stringify(body), + }); +} +const newUserBody = (over = {}) => ({ + email: 'created@test.local', name: 'Created User', password: 'TempPass123', + workspaceId: 'ws-a', role: 'workspace_editor', mustChangePassword: true, ...over, +}); + +test('platform_admin can create a user (201); response omits password/hash; membership written', async () => { + const res = await post('/api/admin/users', tokens.admin, newUserBody()); + assert.equal(res.status, 201); + const body = await res.json(); + assert.equal(body.email, 'created@test.local'); + assert.equal(body.workspace_role, 'workspace_editor'); + assert.equal(body.must_change_password, 1); + assert.ok(!('password' in body), 'response must not include password'); + assert.ok(!('password_hash' in body), 'response must not include hash'); + + const row = db.prepare('SELECT * FROM users WHERE email = ?').get('created@test.local'); + assert.ok(row && row.password_hash && row.password_hash !== 'TempPass123', 'password is hashed, not plaintext'); + const mem = db.prepare("SELECT * FROM workspace_members WHERE workspace_id='ws-a' AND user_id=?").get(row.id); + assert.equal(mem.role, 'workspace_editor'); + assert.equal(mem.invited_by, 'u-admin'); + // HOSTED: excluded from welcome + activation-nudge lifecycle. + assert.ok(row.welcome_email_sent_at && row.activation_nudge_sent_at, 'lifecycle sentinels stamped'); + // Audit row written, never the password. + const audit = db.prepare("SELECT * FROM activity_log WHERE action='admin_create_user'").get(); + assert.ok(audit && /created@test\.local/.test(audit.details)); + assert.ok(!/TempPass123/.test(audit.details), 'audit must not contain the password'); +}); + +test('duplicate email returns 409 and does not overwrite', async () => { + const res = await post('/api/admin/users', tokens.admin, newUserBody({ password: 'Different999' })); + assert.equal(res.status, 409); + // original hash unchanged + const row = db.prepare('SELECT password_hash FROM users WHERE email = ?').get('created@test.local'); + assert.ok(bcrypt.compareSync('TempPass123', row.password_hash), 'existing password untouched'); +}); + +test('non-admin user is denied (403)', async () => { + const res = await post('/api/admin/users', tokens.regular, newUserBody({ email: 'x1@test.local' })); + assert.equal(res.status, 403); + assert.equal(db.prepare('SELECT COUNT(*) c FROM users WHERE email=?').get('x1@test.local').c, 0); +}); + +test('platform_operator is denied from Add User (403) - user mgmt is owner-only', async () => { + const res = await post('/api/admin/users', tokens.operator, newUserBody({ email: 'x2@test.local' })); + assert.equal(res.status, 403); + assert.equal(db.prepare('SELECT COUNT(*) c FROM users WHERE email=?').get('x2@test.local').c, 0); +}); + +test('org_admin can create in their own org but NOT another org', async () => { + const ok = await post('/api/admin/users', tokens.orgAdminA, newUserBody({ email: 'in-a@test.local', workspaceId: 'ws-a' })); + assert.equal(ok.status, 201); + + const denied = await post('/api/admin/users', tokens.orgAdminA, newUserBody({ email: 'in-b@test.local', workspaceId: 'ws-b' })); + assert.equal(denied.status, 403); + assert.equal(db.prepare('SELECT COUNT(*) c FROM users WHERE email=?').get('in-b@test.local').c, 0); +}); + +test('validation: bad email 400, bad role 400, short password 400, missing workspace 404', async () => { + assert.equal((await post('/api/admin/users', tokens.admin, newUserBody({ email: 'nope' }))).status, 400); + assert.equal((await post('/api/admin/users', tokens.admin, newUserBody({ email: 'r@test.local', role: 'org_admin' }))).status, 400); + assert.equal((await post('/api/admin/users', tokens.admin, newUserBody({ email: 'p@test.local', password: 'short' }))).status, 400); + assert.equal((await post('/api/admin/users', tokens.admin, newUserBody({ email: 'w@test.local', workspaceId: 'ws-missing' }))).status, 404); +}); + +test('must_change_password lifecycle: set on create, surfaced on login, cleared on /me password change', async () => { + // created@test.local was created with mustChangePassword:true in the first test. + const login = await post('/api/auth/login', null, { email: 'created@test.local', password: 'TempPass123' }); + assert.equal(login.status, 200); + const loginBody = await login.json(); + assert.equal(loginBody.user.must_change_password, 1, 'login response carries the flag (drives the redirect)'); + + // Change password via PUT /api/auth/me -> clears the flag. + const meRes = await fetch(base + '/api/auth/me', { + method: 'PUT', + headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${loginBody.token}` }, + body: JSON.stringify({ password: 'BrandNewPass1', current_password: 'TempPass123' }), + }); + assert.equal(meRes.status, 200); + const meBody = await meRes.json(); + assert.equal(meBody.must_change_password, 0, '/me response shows the flag cleared'); + const row = db.prepare('SELECT must_change_password FROM users WHERE email=?').get('created@test.local'); + assert.equal(row.must_change_password, 0, 'flag cleared in the DB'); +}); From 5502a3eaa83f7d3737d0a49790ee613031cbf4e6 Mon Sep 17 00:00:00 2001 From: ScreenTinker Date: Fri, 5 Jun 2026 12:44:39 -0500 Subject: [PATCH 6/6] fix(roles): make platform_operator assignable + add deny/assign regression tests The bug: #13 added 'platform_operator' to the frontend role dropdown (PLATFORM_ROLE_OPTIONS) but #14's PUT /api/auth/users/:id/role whitelist (ASSIGNABLE_PLATFORM_ROLES) only listed ['user','platform_admin'], so selecting "Platform operator" returned 400 "Invalid role" - the role was unassignable via the UI. Fix: add 'platform_operator' to ASSIGNABLE_PLATFORM_ROLES. One line; the self-demote guard is intentionally left untouched (a platform_admin still cannot self-assign the non-owner operator role and lock themselves out). Tests (node:test, isolated in-memory DB injection - no DB_PATH change): - admin-users.test.js: platform_admin can PUT role=platform_operator on a target user -> 200 and the row persists as platform_operator (regression guard for the whitelist gap). - operator-permissions.test.js (new): verify-then-test of the highest-blast -radius deny. Operator CAN update/delete a workspace-scoped content row (cross-org write works) but is denied (403) updating or deleting a shared (workspace_id IS NULL) row - proving the separate PLATFORM_ROLES gate in content.js's checkContentWrite still holds after canWrite was broadened to isPlatformStaff. Verified read-only (no leak): the other shared-asset write sites keep their PLATFORM_ROLES gate that excludes operator - kiosk.js:57, widgets.js:110, folders.js:31, layouts.js:59/117/133. cd server && npm test -> 12 pass / 0 fail. Co-Authored-By: Claude Opus 4.8 (1M context) --- server/routes/auth.js | 2 +- server/test/admin-users.test.js | 14 +++ server/test/operator-permissions.test.js | 129 +++++++++++++++++++++++ 3 files changed, 144 insertions(+), 1 deletion(-) create mode 100644 server/test/operator-permissions.test.js diff --git a/server/routes/auth.js b/server/routes/auth.js index ff64cda..dab360e 100644 --- a/server/routes/auth.js +++ b/server/routes/auth.js @@ -480,7 +480,7 @@ router.delete('/users/:id', requireAuth, requireSuperAdmin, (req, res) => { // org roles are managed in the members views. Whitelist is the current model: // 'user' and 'platform_admin' (the legacy 'admin'/'superadmin' strings are gone // after normalization and are no longer accepted here). -const ASSIGNABLE_PLATFORM_ROLES = ['user', 'platform_admin']; +const ASSIGNABLE_PLATFORM_ROLES = ['user', 'platform_operator', 'platform_admin']; router.put('/users/:id/role', requireAuth, requireSuperAdmin, (req, res) => { const { role } = req.body; if (!ASSIGNABLE_PLATFORM_ROLES.includes(role)) return res.status(400).json({ error: 'Invalid role' }); diff --git a/server/test/admin-users.test.js b/server/test/admin-users.test.js index 44f0555..a0c6240 100644 --- a/server/test/admin-users.test.js +++ b/server/test/admin-users.test.js @@ -112,6 +112,9 @@ const orgAdminA = seedUser({ id: 'u-orgadmin-a', email: 'orgadmin-a@test.local', db.prepare("INSERT INTO organization_members (organization_id, user_id, role) VALUES ('org-a','u-orgadmin-a','org_admin')").run(); const operator = seedUser({ id: 'u-operator', email: 'operator@test.local', role: 'platform_operator' }); const regular = seedUser({ id: 'u-regular', email: 'regular@test.local', role: 'user' }); +// Dedicated target for the role-assignment regression test (kept separate so it +// can't perturb the non-admin/operator tokens used by the deny tests above). +seedUser({ id: 'u-role-target', email: 'role-target@test.local', role: 'user' }); const tokens = { admin: generateToken(adminUser, null), @@ -224,3 +227,14 @@ test('must_change_password lifecycle: set on create, surfaced on login, cleared const row = db.prepare('SELECT must_change_password FROM users WHERE email=?').get('created@test.local'); assert.equal(row.must_change_password, 0, 'flag cleared in the DB'); }); + +test('platform_operator is assignable via PUT /users/:id/role (regression for #13/#14 whitelist gap)', async () => { + const res = await fetch(base + '/api/auth/users/u-role-target/role', { + method: 'PUT', + headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${tokens.admin}` }, + body: JSON.stringify({ role: 'platform_operator' }), + }); + assert.equal(res.status, 200); + const dbRole = db.prepare('SELECT role FROM users WHERE id = ?').get('u-role-target').role; + assert.equal(dbRole, 'platform_operator', 'role actually persisted as platform_operator'); +}); diff --git a/server/test/operator-permissions.test.js b/server/test/operator-permissions.test.js new file mode 100644 index 0000000..514253f --- /dev/null +++ b/server/test/operator-permissions.test.js @@ -0,0 +1,129 @@ +'use strict'; + +// #13 regression: platform_operator gets cross-org workspace read/write (via the +// canWrite broadening to isPlatformStaff) but must STILL be denied writing +// shared/global assets (workspace_id IS NULL), which carry a SEPARATE +// PLATFORM_ROLES gate on top of canWrite. This is the highest-blast-radius deny +// (operator editing platform-wide content), so we prove both halves: +// (a) operator CAN update/delete a workspace-scoped content row, and +// (b) operator CANNOT update/delete a shared (workspace_id IS NULL) row. +// +// Same isolated-in-memory-DB harness as admin-users.test.js: inject the DB into +// the require cache before any module that pulls ../db/database loads. Node v20 +// built-ins only. (node --test runs each file in its own process, so this +// injection does not collide with the other suite's.) + +const test = require('node:test'); +const assert = require('node:assert/strict'); +const Database = require('better-sqlite3'); +const { v4: uuidv4 } = require('uuid'); + +process.env.JWT_SECRET = 'test-secret-operator-perms'; + +const db = new Database(':memory:'); +db.pragma('foreign_keys = ON'); +db.exec(` + CREATE TABLE users ( + id TEXT PRIMARY KEY, email TEXT UNIQUE NOT NULL, name TEXT NOT NULL DEFAULT '', + password_hash TEXT, auth_provider TEXT NOT NULL DEFAULT 'local', avatar_url TEXT, + role TEXT NOT NULL DEFAULT 'user', plan_id TEXT DEFAULT 'free', email_alerts INTEGER DEFAULT 1, + must_change_password INTEGER NOT NULL DEFAULT 0, + created_at INTEGER NOT NULL DEFAULT (strftime('%s','now')) + ); + CREATE TABLE workspaces ( + id TEXT PRIMARY KEY, organization_id TEXT NOT NULL, name TEXT NOT NULL, slug TEXT, + created_at INTEGER NOT NULL DEFAULT (strftime('%s','now')) + ); + CREATE TABLE organization_members ( + id INTEGER PRIMARY KEY AUTOINCREMENT, organization_id TEXT NOT NULL, user_id TEXT NOT NULL, + role TEXT NOT NULL, UNIQUE(organization_id, user_id) + ); + CREATE TABLE workspace_members ( + id INTEGER PRIMARY KEY AUTOINCREMENT, workspace_id TEXT NOT NULL, user_id TEXT NOT NULL, + role TEXT NOT NULL, joined_at INTEGER NOT NULL DEFAULT (strftime('%s','now')), + UNIQUE(workspace_id, user_id) + ); + CREATE TABLE content ( + id TEXT PRIMARY KEY, filename TEXT NOT NULL, filepath TEXT NOT NULL, mime_type TEXT NOT NULL, + file_size INTEGER NOT NULL, duration_sec REAL, thumbnail_path TEXT, width INTEGER, height INTEGER, + remote_url TEXT, user_id TEXT, folder TEXT, folder_id TEXT, workspace_id TEXT, + created_at INTEGER NOT NULL DEFAULT (strftime('%s','now')) + ); + -- Empty, but the DELETE handler queries these for playlist cleanup. + CREATE TABLE devices (id TEXT PRIMARY KEY, playlist_id TEXT); + CREATE TABLE playlists (id TEXT PRIMARY KEY, workspace_id TEXT, published_snapshot TEXT); + CREATE TABLE playlist_items (id INTEGER PRIMARY KEY AUTOINCREMENT, playlist_id TEXT, content_id TEXT); +`); + +const dbModulePath = require.resolve('../db/database'); +require.cache[dbModulePath] = { + id: dbModulePath, filename: dbModulePath, loaded: true, + exports: { db, pruneTelemetry() {}, pruneScreenshots() {} }, +}; + +const express = require('express'); +const { generateToken, requireAuth } = require('../middleware/auth'); +const { resolveTenancy } = require('../lib/tenancy'); +const contentRouter = require('../routes/content'); + +// Seed: org + workspace, a platform_operator user, and two content rows. +db.prepare("INSERT INTO workspaces (id, organization_id, name) VALUES ('ws-a','org-a','Workspace A')").run(); +db.prepare("INSERT INTO users (id, email, role) VALUES ('u-op','op@test.local','platform_operator')").run(); +const operator = { id: 'u-op', email: 'op@test.local', role: 'platform_operator' }; +// JWT carries current_workspace_id so resolveTenancy lands the operator (acting-as) in ws-a. +const opToken = generateToken(operator, 'ws-a'); + +const wsContentId = uuidv4(); +const sharedContentId = uuidv4(); +function seedContent(id, workspaceId) { + db.prepare(`INSERT INTO content (id, filename, filepath, mime_type, file_size, workspace_id) + VALUES (?, 'orig.png', '/does/not/exist.png', 'image/png', 123, ?)`).run(id, workspaceId); +} +seedContent(wsContentId, 'ws-a'); // workspace-scoped +seedContent(sharedContentId, null); // shared / platform-global (workspace_id IS NULL) + +const app = express(); +app.use(express.json()); +app.use('/api/content', requireAuth, resolveTenancy, contentRouter); +const server = app.listen(0); +let base; +test.before(async () => { + await new Promise(r => server.listening ? r() : server.once('listening', r)); + base = `http://127.0.0.1:${server.address().port}`; +}); +test.after(() => { server.close(); db.close(); }); + +const op = (method, id, body) => fetch(`${base}/api/content/${id}`, { + method, + headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${opToken}` }, + ...(body ? { body: JSON.stringify(body) } : {}), +}); + +// (a) Proves the operator's cross-org write actually works (so (b) isn't just +// "operator can't write anything"). +test('operator CAN update a workspace-scoped content row', async () => { + const res = await op('PUT', wsContentId, { filename: 'renamed.png' }); + assert.equal(res.status, 200); + assert.equal(db.prepare('SELECT filename FROM content WHERE id=?').get(wsContentId).filename, 'renamed.png'); +}); + +// (b) The separate PLATFORM_ROLES gate on workspace_id IS NULL must deny operator. +test('operator CANNOT update a shared (workspace_id IS NULL) content row -> 403', async () => { + const res = await op('PUT', sharedContentId, { filename: 'hijacked.png' }); + assert.equal(res.status, 403); + assert.equal(db.prepare('SELECT filename FROM content WHERE id=?').get(sharedContentId).filename, 'orig.png', + 'shared row must be unchanged'); +}); + +test('operator CANNOT delete a shared (workspace_id IS NULL) content row -> 403', async () => { + const res = await op('DELETE', sharedContentId); + assert.equal(res.status, 403); + assert.ok(db.prepare('SELECT 1 FROM content WHERE id=?').get(sharedContentId), 'shared row must still exist'); +}); + +// Delete last so the workspace-scoped row survives the update assertion above. +test('operator CAN delete a workspace-scoped content row', async () => { + const res = await op('DELETE', wsContentId); + assert.equal(res.status, 200); + assert.ok(!db.prepare('SELECT 1 FROM content WHERE id=?').get(wsContentId), 'workspace row deleted'); +});