From 797eab7c8d5eecc0ad6fd7a53b07551b90f5df65 Mon Sep 17 00:00:00 2001
From: ScreenTinker <hello@screentinker.com>
Date: Fri, 5 Jun 2026 09:58:46 -0500
Subject: [PATCH] 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) <noreply@anthropic.com>
---
 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() {
               <td style="padding:8px;font-size:11px;color:var(--text-muted)">${u.last_login ? new Date(u.last_login * 1000).toLocaleString() : t('common.never')}</td>
               <td style="padding:8px">
                 <select class="input" style="max-width:120px;width:100%;background:var(--bg-input);font-size:12px;padding:4px" data-role-user="${u.id}">
-                  <option value="user" ${u.role === 'user' ? 'selected' : ''}>${t('admin.role.user')}</option>
-                  <option value="admin" ${u.role === 'admin' ? 'selected' : ''}>${t('admin.role.admin')}</option>
-                  <option value="superadmin" ${u.role === 'superadmin' ? 'selected' : ''}>${t('admin.role.superadmin')}</option>
+                  ${PLATFORM_ROLE_OPTIONS.map(r => `<option value="${r}" ${u.role === r ? 'selected' : ''}>${t('admin.role.' + r)}</option>`).join('')}
                 </select>
               </td>
               <td style="padding:8px">
@@ -77,7 +81,7 @@ async function loadUsers() {
               </td>
               <td style="padding:8px;white-space:nowrap">
                 ${u.auth_provider === 'local' && u.id !== currentUser.id ? `<button class="btn btn-secondary btn-sm" data-reset-pw-user="${u.id}" data-user-email="${u.email}" style="margin-right:4px">${t('admin.reset_password')}</button>` : ''}
-                ${u.role !== 'superadmin' ? `<button class="btn btn-danger btn-sm" data-delete-user="${u.id}">${t('admin.remove')}</button>` : `<span style="color:var(--text-muted);font-size:11px">${t('admin.owner')}</span>`}
+                ${!isPlatformAdmin(u) ? `<button class="btn btn-danger btn-sm" data-delete-user="${u.id}">${t('admin.remove')}</button>` : `<span style="color:var(--text-muted);font-size:11px">${t('admin.owner')}</span>`}
               </td>
             </tr>
           `).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 = `
     <div class="page-header">
@@ -430,7 +433,7 @@ async function loadUsers() {
                 <span style="background:var(--bg-primary);padding:2px 8px;border-radius:10px;font-size:11px">${u.auth_provider}</span>
               </td>
               <td style="padding:10px 12px">
-                <span style="color:${u.role === 'admin' ? 'var(--accent)' : 'var(--text-secondary)'}">${u.role}</span>
+                <span style="color:${isPlatformAdmin(u) ? 'var(--accent)' : 'var(--text-secondary)'}">${u.role}</span>
               </td>
               <td style="padding:10px 12px">
                 <select class="input plan-select" data-user-id="${u.id}" style="padding:4px 8px;font-size:12px;width:auto">
diff --git a/server/db/database.js b/server/db/database.js
index 4247e09..7104238 100644
--- a/server/db/database.js
+++ b/server/db/database.js
@@ -162,6 +162,17 @@ const migrations = [
   // only genuinely-new signups (NULL) become eligible going forward.
   "ALTER TABLE users ADD COLUMN activation_nudge_sent_at INTEGER",
   "UPDATE users SET activation_nudge_sent_at = 1 WHERE activation_nudge_sent_at IS NULL",
+  // Issue #14: normalize the platform-role model. The legacy /api/auth/users
+  // dropdown could write 'superadmin' and 'admin' strings that not every code
+  // path recognized (some checks matched only 'platform_admin', so a superadmin
+  // could list orgs but not act-as into them). Collapse to the current model:
+  //   superadmin -> platform_admin  (equivalent everywhere; fixes act-as)
+  //   admin      -> user            (legacy middle tier; elevated power now
+  //                                  comes from org/workspace membership)
+  // Strictly idempotent: mutates ONLY exact legacy strings, no-ops on rows
+  // 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'",
 ];
 for (const sql of migrations) {
   try { db.exec(sql); } catch (e) { /* already exists */ }
diff --git a/server/lib/permissions.js b/server/lib/permissions.js
index 36931e4..0db24b6 100644
--- a/server/lib/permissions.js
+++ b/server/lib/permissions.js
@@ -86,12 +86,11 @@ function requireOrgOwner(req, res, next) {
   next();
 }
 
-function requirePlatformAdmin(req, res, next) {
-  if (!req.user || req.user.role !== 'platform_admin') {
-    return res.status(403).json({ error: 'Platform admin required' });
-  }
-  next();
-}
+// #14: the dead/stricter requirePlatformAdmin that used to live here (bare
+// `=== 'platform_admin'`, excluding legacy superadmin) was removed. The single
+// platform-admin guard is requirePlatformAdmin in server/middleware/auth.js,
+// which is the alias every route already imports and which accepts the full
+// PLATFORM_ROLES set via isPlatformRole().
 
 // Decoupled "can admin this workspace" predicate. Unlike canAdmin(req) above,
 // this takes an explicit (user, workspace) pair instead of reading from req,
@@ -134,5 +133,4 @@ module.exports = {
   requireWorkspaceAdmin,
   requireOrgAdmin,
   requireOrgOwner,
-  requirePlatformAdmin,
 };
diff --git a/server/lib/tenancy.js b/server/lib/tenancy.js
index 49f2ee7..4b3d0ee 100644
--- a/server/lib/tenancy.js
+++ b/server/lib/tenancy.js
@@ -8,7 +8,8 @@
 //   req.organizationId   string | null   parent org of req.workspace
 //   req.workspaceRole    string | null   'workspace_admin' | 'workspace_editor' | 'workspace_viewer'
 //   req.orgRole          string | null   'org_owner' | 'org_admin'
-//   req.isPlatformAdmin  boolean         shortcut for req.user.role === 'platform_admin'
+//   req.isPlatformAdmin  boolean         true when req.user.role is a platform-owner role
+//                                        (isPlatformRole: platform_admin / legacy superadmin)
 //   req.actingAs         boolean         true when the user reached this workspace via
 //                                        org-level or platform-level access rather than
 //                                        a direct workspace_members row
@@ -26,6 +27,7 @@
 'use strict';
 
 const { db } = require('../db/database');
+const { isPlatformRole } = require('../middleware/auth');
 
 function membershipOf(userId, workspaceId) {
   return db.prepare(
@@ -58,7 +60,10 @@ function firstAccessibleWorkspace(userId) {
 // or platform admin). Returns the access context: { workspaceRole, actingAs }
 // or null if no access.
 function accessContext(userId, role, workspace) {
-  const isPlatformAdmin = role === 'platform_admin';
+  // #14: route through isPlatformRole so a legacy 'superadmin' is treated as a
+  // platform owner here too (previously this bare === check excluded it, so a
+  // superadmin couldn't act-as into orgs they didn't directly belong to).
+  const isPlatformAdmin = isPlatformRole(role);
   const wsMembership = membershipOf(userId, workspace.id);
   if (wsMembership) {
     return { workspaceRole: wsMembership.role, actingAs: false };
@@ -79,7 +84,7 @@ function resolveTenancy(req, res, next) {
     return next();
   }
 
-  const isPlatformAdmin = req.user.role === 'platform_admin';
+  const isPlatformAdmin = isPlatformRole(req.user.role);
   req.isPlatformAdmin = isPlatformAdmin;
 
   // Build the ordered candidate list of workspace_ids to try.
diff --git a/server/middleware/auth.js b/server/middleware/auth.js
index b270324..fbd2c62 100644
--- a/server/middleware/auth.js
+++ b/server/middleware/auth.js
@@ -26,7 +26,7 @@ function recoveryUser(decoded) {
     id: decoded.id,
     email: decoded.email || 'admin@localhost',
     name: 'Recovery Admin',
-    role: decoded.role || 'admin',
+    role: decoded.role || 'platform_admin',
     auth_provider: 'recovery',
     avatar_url: null,
     plan_id: 'enterprise'
@@ -82,10 +82,25 @@ function optionalAuth(req, res, next) {
 // either spelling so existing callers keep working without per-route edits.
 // New code should prefer requirePlatformAdmin / requireOrgAdmin / workspace
 // role guards from server/lib/permissions.js.
+//
+// Issue #14 (role normalization): the data migration in db/database.js collapses
+// any legacy 'superadmin' -> 'platform_admin' and 'admin' -> 'user'. 'superadmin'
+// is kept in PLATFORM_ROLES purely as back-compat belt-and-suspenders (recovery
+// tokens, stray strings) - no row should carry it post-migration. Owner-level
+// power lives here in PLATFORM_ROLES; anything not in this set is denied.
 
 const PLATFORM_ROLES = ['superadmin', 'platform_admin'];
 const ELEVATED_ROLES = ['admin', 'superadmin', 'platform_admin'];
 
+// isPlatformRole: single predicate for "is this string a platform-owner role".
+// Use this instead of a bare `role === 'platform_admin'` so a stray 'superadmin'
+// is never silently treated as lower-privileged (the act-as bug fixed in #14).
+// NOTE: this is the OWNER tier only - it deliberately does NOT include
+// 'platform_operator' (issue #13), which is cross-org staff, not an owner.
+function isPlatformRole(role) {
+  return PLATFORM_ROLES.includes(role);
+}
+
 function requireAdmin(req, res, next) {
   if (!req.user || !ELEVATED_ROLES.includes(req.user.role)) {
     return res.status(403).json({ error: 'Admin access required' });
@@ -103,4 +118,4 @@ function requireSuperAdmin(req, res, next) {
 // Preferred alias for new code.
 const requirePlatformAdmin = requireSuperAdmin;
 
-module.exports = { generateToken, verifyToken, requireAuth, optionalAuth, requireAdmin, requireSuperAdmin, requirePlatformAdmin, PLATFORM_ROLES, ELEVATED_ROLES };
+module.exports = { generateToken, verifyToken, requireAuth, optionalAuth, requireAdmin, requireSuperAdmin, requirePlatformAdmin, isPlatformRole, PLATFORM_ROLES, ELEVATED_ROLES };
diff --git a/server/routes/auth.js b/server/routes/auth.js
index 382834d..b9e5ce3 100644
--- a/server/routes/auth.js
+++ b/server/routes/auth.js
@@ -5,7 +5,7 @@ const https = require('https');
 const { v4: uuidv4 } = require('uuid');
 const { OAuth2Client } = require('google-auth-library');
 const { db } = require('../db/database');
-const { generateToken, requireAuth, requireAdmin, requireSuperAdmin, PLATFORM_ROLES } = require('../middleware/auth');
+const { generateToken, requireAuth, requireAdmin, requireSuperAdmin, isPlatformRole, PLATFORM_ROLES } = require('../middleware/auth');
 const { resolveTenancy } = require('../lib/tenancy');
 const { logActivity, getClientIp } = require('../services/activity');
 const { sendSignupEmails } = require('../services/signupEmails');
@@ -456,11 +456,18 @@ router.delete('/users/:id', requireAuth, requireSuperAdmin, (req, res) => {
   res.json({ success: true });
 });
 
-// Update user role (superadmin only)
+// Update user platform role (platform admin only).
+// #14: this manages users.role (the PLATFORM-level role) only - workspace and
+// 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'];
 router.put('/users/:id/role', requireAuth, requireSuperAdmin, (req, res) => {
   const { role } = req.body;
-  if (!['user', 'admin', 'superadmin'].includes(role)) return res.status(400).json({ error: 'Invalid role' });
-  if (req.params.id === req.user.id && role !== 'superadmin') return res.status(400).json({ error: 'Cannot demote yourself' });
+  if (!ASSIGNABLE_PLATFORM_ROLES.includes(role)) return res.status(400).json({ error: 'Invalid role' });
+  // Self-demotion guard: a platform admin can't strip their own platform role
+  // (would lock themselves out of platform admin actions).
+  if (req.params.id === req.user.id && !isPlatformRole(role)) return res.status(400).json({ error: 'Cannot demote yourself' });
   db.prepare('UPDATE users SET role = ? WHERE id = ?').run(role, req.params.id);
   res.json({ success: true });
 });