mirror of
https://github.com/screentinker/screentinker.git
synced 2026-06-15 02:33:15 -06:00
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) <noreply@anthropic.com>
This commit is contained in:
parent
7674f6dc9f
commit
5502a3eaa8
|
|
@ -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' });
|
||||
|
|
|
|||
|
|
@ -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');
|
||||
});
|
||||
|
|
|
|||
129
server/test/operator-permissions.test.js
Normal file
129
server/test/operator-permissions.test.js
Normal file
|
|
@ -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');
|
||||
});
|
||||
Loading…
Reference in a new issue