Archive/screentinker
1
0
Fork 0
mirror of https://github.com/screentinker/screentinker.git synced 2026-06-15 02:33:15 -06:00
Code Issues Actions 2 Packages Projects Releases Wiki Activity

refactor(roles): normalize the platform-role model (#14)

Browse source 
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>
This commit is contained in:
ScreenTinker 2026-06-05 09:58:46 -05:00
parent 0fec335e75
commit 797eab7c8d
8 changed files with 68 additions and 22 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
frontend/js/i18n/en.js
View file

@ -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',

12
frontend/js/views/admin.js
View file

@ -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('')}

7
frontend/js/views/settings.js
View file

@ -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">

11
server/db/database.js
View file

@ -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 */ }

12
server/lib/permissions.js
View file

@ -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,
};

11
server/lib/tenancy.js
View file

@ -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.

19
server/middleware/auth.js
View file

@ -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 };

15
server/routes/auth.js
View file

@ -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 });
});