From a25c6827a71d4673cd9094580c1d70208168a35b Mon Sep 17 00:00:00 2001 From: ScreenTinker Date: Wed, 29 Jul 2026 19:36:10 -0500 Subject: [PATCH] Show every plan on the admin tab, with who is on each MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The admin plan table read /api/subscription/plans, which filters `active = 1` because that endpoint feeds the public pricing page. So the one screen meant to show the operator what plans exist could not show a hidden one — a comped or beta tier was invisible to us as well as to customers, with no way to see it existed or who was on it. Found immediately after creating exactly such a plan. GET /api/admin/plans (platform-admin only) returns every plan plus, per plan, the number of accounts, organisations and screens on it. Visible plans sort first so the list still reads like the pricing ladder, with hidden ones after and badged. The public endpoint is deliberately untouched: hiding a plan has to keep working, and the test pins BOTH directions because they pull against each other — the admin list must include an inactive plan, and the public list must never leak one. Counts are the point, not decoration: "how many people are on what plan" is the question you actually ask of this screen, and it was answerable only by hand in SQLite. Also carries a warning for accounts whose plan no longer resolves. Both users.plan_id and organizations.plan_id are FK-enforced to plans.id and there is no delete-plan route, so this should be unreachable — but migrations here do rebuild tables with foreign keys off (the tenant-cascade one rebuilt thirteen), and that is exactly how a row would be orphaned. Six lines for a state that would otherwise be silent. Strings added to en/de/es/fr/it/pt. Not hi: it has no admin translations at all, lookup falls back to English, and four Hindi strings among forty English ones would read worse than consistent English. --- frontend/js/api.js | 2 + frontend/js/i18n/de.js | 4 + frontend/js/i18n/en.js | 4 + frontend/js/i18n/es.js | 4 + frontend/js/i18n/fr.js | 4 + frontend/js/i18n/it.js | 4 + frontend/js/i18n/pt.js | 4 + frontend/js/views/admin.js | 20 ++- server/routes/admin.js | 28 ++++ server/test/admin-plans-visibility.test.js | 146 +++++++++++++++++++++ 10 files changed, 217 insertions(+), 3 deletions(-) create mode 100644 server/test/admin-plans-visibility.test.js diff --git a/frontend/js/api.js b/frontend/js/api.js index ae1e073..9ec7996 100644 --- a/frontend/js/api.js +++ b/frontend/js/api.js @@ -242,6 +242,8 @@ export const api = { adminCreateUser: (data) => request('/admin/users', { method: 'POST', body: JSON.stringify(data) }), adminCreateOrg: (name) => request('/admin/orgs', { method: 'POST', body: JSON.stringify({ name }) }), adminListOrgs: () => request('/admin/orgs'), + // Platform-admin view: EVERY plan incl. hidden ones, with subscriber counts. + adminListPlans: () => request('/admin/plans'), adminDeleteOrg: (id) => request(`/admin/orgs/${id}`, { method: 'DELETE' }), adminDeleteWorkspace: (id) => request(`/admin/workspaces/${id}`, { method: 'DELETE' }), aiGetSettings: () => request('/ai/settings'), diff --git a/frontend/js/i18n/de.js b/frontend/js/i18n/de.js index 35c3f19..692d7cd 100644 --- a/frontend/js/i18n/de.js +++ b/frontend/js/i18n/de.js @@ -865,6 +865,10 @@ export default { 'admin.access_denied_desc': 'Plattform-Admin-Zugriff erforderlich.', 'admin.all_users': 'Alle Benutzer', 'admin.plans': 'Abonnementpläne', + 'admin.col.accounts': 'Konten', + 'admin.col.screens': 'Bildschirme', + 'admin.plan_hidden': 'ausgeblendet', + 'admin.plan_orphaned': 'Konten mit einem nicht mehr vorhandenen Tarif', 'admin.system': 'System', 'admin.col.user': 'Benutzer', 'admin.col.auth': 'Auth', diff --git a/frontend/js/i18n/en.js b/frontend/js/i18n/en.js index cd4b087..4aaead6 100644 --- a/frontend/js/i18n/en.js +++ b/frontend/js/i18n/en.js @@ -1270,6 +1270,10 @@ export default { 'admin.access_denied_desc': 'Platform admin access required.', 'admin.all_users': 'All Users', 'admin.plans': 'Subscription Plans', + 'admin.col.accounts': 'Accounts', + 'admin.col.screens': 'Screens', + 'admin.plan_hidden': 'hidden', + 'admin.plan_orphaned': 'Accounts on a plan that no longer exists', 'admin.system': 'System', // #15: instance-level default branding 'admin.branding.title': 'Default branding', diff --git a/frontend/js/i18n/es.js b/frontend/js/i18n/es.js index c933749..b85d89e 100644 --- a/frontend/js/i18n/es.js +++ b/frontend/js/i18n/es.js @@ -895,6 +895,10 @@ export default { 'admin.access_denied_desc': 'Se requiere acceso de administrador de plataforma.', 'admin.all_users': 'Todos los usuarios', 'admin.plans': 'Planes de suscripción', + 'admin.col.accounts': 'Cuentas', + 'admin.col.screens': 'Pantallas', + 'admin.plan_hidden': 'oculto', + 'admin.plan_orphaned': 'Cuentas en un plan que ya no existe', 'admin.system': 'Sistema', 'admin.col.user': 'Usuario', 'admin.col.auth': 'Auth', diff --git a/frontend/js/i18n/fr.js b/frontend/js/i18n/fr.js index 963b30a..57ac05a 100644 --- a/frontend/js/i18n/fr.js +++ b/frontend/js/i18n/fr.js @@ -865,6 +865,10 @@ export default { 'admin.access_denied_desc': 'Accès administrateur plateforme requis.', 'admin.all_users': 'Tous les utilisateurs', 'admin.plans': 'Plans d\'abonnement', + 'admin.col.accounts': 'Comptes', + 'admin.col.screens': 'Écrans', + 'admin.plan_hidden': 'masqué', + 'admin.plan_orphaned': "Comptes sur un forfait qui n'existe plus", 'admin.system': 'Système', 'admin.col.user': 'Utilisateur', 'admin.col.auth': 'Auth', diff --git a/frontend/js/i18n/it.js b/frontend/js/i18n/it.js index 32894d1..ce6a98f 100644 --- a/frontend/js/i18n/it.js +++ b/frontend/js/i18n/it.js @@ -823,6 +823,10 @@ export default { 'admin.access_denied_desc': 'È richiesto l\'accesso come amministratore di piattaforma.', 'admin.all_users': 'Tutti gli Utenti', 'admin.plans': 'Piani di Abbonamento', + 'admin.col.accounts': 'Account', + 'admin.col.screens': 'Schermi', + 'admin.plan_hidden': 'nascosto', + 'admin.plan_orphaned': 'Account su un piano che non esiste più', 'admin.system': 'Sistema', 'admin.col.user': 'Utente', 'admin.col.auth': 'Autenticazione', diff --git a/frontend/js/i18n/pt.js b/frontend/js/i18n/pt.js index 9559f72..3d78ab5 100644 --- a/frontend/js/i18n/pt.js +++ b/frontend/js/i18n/pt.js @@ -865,6 +865,10 @@ export default { 'admin.access_denied_desc': 'Acesso de admin da plataforma necessário.', 'admin.all_users': 'Todos os usuários', 'admin.plans': 'Planos de assinatura', + 'admin.col.accounts': 'Contas', + 'admin.col.screens': 'Ecrãs', + 'admin.plan_hidden': 'oculto', + 'admin.plan_orphaned': 'Contas num plano que já não existe', 'admin.system': 'Sistema', 'admin.col.user': 'Usuário', 'admin.col.auth': 'Auth', diff --git a/frontend/js/views/admin.js b/frontend/js/views/admin.js index b05b8e3..8eee022 100644 --- a/frontend/js/views/admin.js +++ b/frontend/js/views/admin.js @@ -373,7 +373,10 @@ async function loadStatusDebug() { async function loadPlans() { const el = document.getElementById('plansTable'); try { - const plans = await fetch('/api/subscription/plans').then(r => r.json()); + // Admin endpoint, not /api/subscription/plans: that one filters `active = 1` because it feeds + // the pricing page, so a deliberately hidden plan (a comped or beta tier) was invisible to the + // operator too. Here we want every plan, plus who is actually on each one. + const { plans, orphaned } = await api.adminListPlans(); el.innerHTML = `
@@ -383,20 +386,31 @@ async function loadPlans() { + + ${plans.map(p => ` - - + + + + `).join('')}
${t('admin.col.storage')} ${t('admin.col.monthly')} ${t('admin.col.yearly')}${t('admin.col.accounts')}${t('admin.col.screens')}
${p.display_name}
${esc(p.display_name)} + ${esc(p.id)} + ${p.active ? '' : `${t('admin.plan_hidden')}`} + ${p.max_devices === -1 ? t('admin.unlimited') : p.max_devices} ${p.max_storage_mb === -1 ? t('admin.unlimited') : p.max_storage_mb >= 1024 ? (p.max_storage_mb/1024)+'GB' : p.max_storage_mb+'MB'} ${p.price_monthly > 0 ? '$'+p.price_monthly : t('admin.free')} ${p.price_yearly > 0 ? '$'+p.price_yearly : '-'}${p.user_count}${p.device_count}
+ ${(orphaned && orphaned.length) ? ` +

+ ${t('admin.plan_orphaned')}: ${orphaned.map(o => `${esc(o.plan_id)} (${o.user_count})`).join(', ')} +

` : ''} `; } catch (err) { el.innerHTML = `

${esc(err.message)}

`; } } diff --git a/server/routes/admin.js b/server/routes/admin.js index dabf9e6..ebf05a3 100644 --- a/server/routes/admin.js +++ b/server/routes/admin.js @@ -432,6 +432,34 @@ router.post('/trigger-update', requirePlatformAdmin, async (req, res) => { // distinct_accounts is the signal, not rejections. Values are counts only — the identifiers are // salted-hashed inside the telemetry module and never leave it, so this cannot become a roster // of a customer's email addresses. Platform-admin only, and in-memory (a restart clears it). +// Platform-admin plan overview. Deliberately NOT the public /api/subscription/plans list, which +// filters `active = 1` because that is what the pricing page renders — so an intentionally hidden +// plan (a comped or beta tier) was invisible to the operator as well as to customers, with no way +// to see it existed or who was on it. This returns EVERY plan plus how many accounts sit on each, +// so a hidden tier is manageable rather than folklore. +router.get('/plans', requirePlatformAdmin, (req, res) => { + const plans = db.prepare(` + SELECT p.*, + (SELECT COUNT(*) FROM users u WHERE u.plan_id = p.id) AS user_count, + (SELECT COUNT(*) FROM organizations o WHERE o.plan_id = p.id) AS org_count, + (SELECT COUNT(*) FROM devices d + JOIN workspaces w ON w.id = d.workspace_id + JOIN organizations o2 ON o2.id = w.organization_id + JOIN users u2 ON u2.id = o2.owner_user_id + WHERE u2.plan_id = p.id) AS device_count + FROM plans p + ORDER BY p.active DESC, p.sort_order ASC + `).all(); + // Accounts whose plan_id no longer resolves would otherwise be invisible in a per-plan view — + // they are the ones that actually need attention (a deleted plan leaves them with no entitlements). + const orphaned = db.prepare(` + SELECT u.plan_id, COUNT(*) AS user_count FROM users u + WHERE u.plan_id IS NOT NULL AND u.plan_id NOT IN (SELECT id FROM plans) + GROUP BY u.plan_id + `).all(); + res.json({ plans, orphaned }); +}); + router.get('/limiter-rejections', requirePlatformAdmin, (req, res) => { const rows = require('../lib/limiter-telemetry').snapshot(); res.json({ diff --git a/server/test/admin-plans-visibility.test.js b/server/test/admin-plans-visibility.test.js new file mode 100644 index 0000000..dabc2d4 --- /dev/null +++ b/server/test/admin-plans-visibility.test.js @@ -0,0 +1,146 @@ +'use strict'; + +// A plan can be hidden from customers by setting active = 0 — that is how a comped or beta tier is +// kept off the pricing page. But the only plan listing was /api/subscription/plans, which filters +// `active = 1` because it FEEDS that pricing page. So a hidden plan was invisible to the operator +// too: no way to see it existed, or who was on it, from the admin screen that is supposed to show +// exactly that. +// +// Two invariants, and they pull in opposite directions, which is why both are pinned here: +// - the PUBLIC list must never leak an inactive plan (that is the whole point of hiding it) +// - the ADMIN list must show every plan, with how many accounts are on each +// +// Isolated in-memory DB injected into the require cache, same approach as admin-users.test.js. + +const test = require('node:test'); +const assert = require('node:assert/strict'); +const path = require('path'); +const Database = require('better-sqlite3'); +const express = require('express'); + +process.env.JWT_SECRET = 'test-secret-admin-plans'; + +const db = new Database(':memory:'); +db.pragma('foreign_keys = ON'); +db.exec(` + CREATE TABLE plans ( + id TEXT PRIMARY KEY, + name TEXT NOT NULL, + display_name TEXT NOT NULL, + max_devices INTEGER NOT NULL DEFAULT 2, + max_storage_mb INTEGER NOT NULL DEFAULT 500, + remote_control INTEGER NOT NULL DEFAULT 0, + remote_url INTEGER NOT NULL DEFAULT 0, + priority_support INTEGER NOT NULL DEFAULT 0, + price_monthly REAL NOT NULL DEFAULT 0, + price_yearly REAL NOT NULL DEFAULT 0, + sort_order INTEGER NOT NULL DEFAULT 0, + active INTEGER NOT NULL DEFAULT 1 + ); + -- Columns here are the ones resolveSessionUser selects; a missing one makes requireAuth throw + -- and the request comes back 401, which reads like an auth bug rather than a schema gap. + 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, + email_alerts INTEGER DEFAULT 1, must_change_password INTEGER NOT NULL DEFAULT 0, + created_at INTEGER NOT NULL DEFAULT (strftime('%s','now')), + updated_at INTEGER NOT NULL DEFAULT (strftime('%s','now')) + ); + CREATE TABLE organizations ( + id TEXT PRIMARY KEY, name TEXT NOT NULL, owner_user_id TEXT NOT NULL, plan_id TEXT + ); + CREATE TABLE workspaces ( + id TEXT PRIMARY KEY, organization_id TEXT NOT NULL, name TEXT NOT NULL + ); + CREATE TABLE devices ( + id TEXT PRIMARY KEY, name TEXT, workspace_id TEXT + ); + + INSERT INTO plans (id,name,display_name,max_devices,price_monthly,sort_order,active) VALUES + ('free','free','Free',1,0,0,1), + ('pro','pro','Pro',15,99,3,1), + ('beta','beta','Beta Tester',8,0,9,0); + + INSERT INTO users (id,email,role,plan_id) VALUES + ('padmin','padmin@t.local','platform_admin','pro'), + ('u1','u1@t.local','user','beta'), + ('u2','u2@t.local','user','beta'), + ('u3','u3@t.local','user','pro'); + + INSERT INTO organizations (id,name,owner_user_id,plan_id) VALUES ('o1','Org One','u1','beta'); + INSERT INTO workspaces (id,organization_id,name) VALUES ('w1','o1','WS One'); + INSERT INTO devices (id,name,workspace_id) VALUES ('d1','Screen','w1'), ('d2','Screen 2','w1'); +`); + +const dbModulePath = require.resolve('../db/database'); +require.cache[dbModulePath] = { id: dbModulePath, filename: dbModulePath, loaded: true, exports: { db } }; + +const { requireAuth, generateToken } = require('../middleware/auth'); +const adminRouter = require('../routes/admin'); +const subscriptionRouter = require('../routes/subscription'); + +// Use the app's own token factory rather than hand-rolling claims — it owns the secret, the +// algorithm and the claim shape, and a test that guesses those tests the guess. +const row = (id) => db.prepare('SELECT id, email, role FROM users WHERE id = ?').get(id); +const adminToken = generateToken(row('padmin'), null); +const userToken = generateToken(row('u1'), null); + +const app = express(); +app.use(express.json()); +app.use('/api/admin', requireAuth, adminRouter); +app.use('/api/subscription', subscriptionRouter); +const server = app.listen(0); + +async function get(pathname, token) { + await new Promise(r => (server.listening ? r() : server.once('listening', r))); + const res = await fetch(`http://127.0.0.1:${server.address().port}${pathname}`, { + headers: token ? { Authorization: `Bearer ${token}` } : {}, + }); + let body = null; + try { body = await res.json(); } catch (_) {} + return { status: res.status, body }; +} + +test('THE POINT: the admin list includes a hidden plan', async () => { + const { status, body } = await get('/api/admin/plans', adminToken); + assert.equal(status, 200); + const ids = body.plans.map(p => p.id); + assert.ok(ids.includes('beta'), `hidden plan missing from the admin view: ${ids.join(',')}`); + assert.equal(body.plans.find(p => p.id === 'beta').active, 0); +}); + +test('and it reports how many accounts are on each plan', async () => { + const { body } = await get('/api/admin/plans', adminToken); + const by = Object.fromEntries(body.plans.map(p => [p.id, p])); + assert.equal(by.beta.user_count, 2); + assert.equal(by.pro.user_count, 2); // padmin + u3 + assert.equal(by.free.user_count, 0); + assert.equal(by.beta.org_count, 1); + assert.equal(by.beta.device_count, 2, 'screens owned by accounts on the plan'); +}); + +test('THE OTHER HALF: the public list must NOT leak a hidden plan', async () => { + // If this ever fails, hiding a comped tier stops working and it appears on the pricing page. + const { status, body } = await get('/api/subscription/plans'); + assert.equal(status, 200); + const ids = body.map(p => p.id); + assert.deepEqual(ids.sort(), ['free', 'pro'], `inactive plan leaked: ${ids.join(',')}`); +}); + +test('visible plans sort before hidden ones, so the list reads naturally', async () => { + const { body } = await get('/api/admin/plans', adminToken); + const firstHidden = body.plans.findIndex(p => !p.active); + const lastVisible = body.plans.reduce((acc, p, i) => (p.active ? i : acc), -1); + assert.ok(firstHidden > lastVisible, 'hidden plans should come after the visible ones'); +}); + +test('a non-platform-admin cannot read the plan overview', async () => { + assert.equal((await get('/api/admin/plans', userToken)).status, 403); +}); + +test('an unauthenticated caller cannot read it either', async () => { + assert.equal((await get('/api/admin/plans')).status, 401); +}); + +test.after(() => { server.close(); });