theta42/sso-manager-node
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(multi-site): coordinated master promotion + a dead-on-arrival authz bug

Browse Source 
Two real bugs, both only surfaced by the live two-container e2e test
(docker-compose.multisite-e2e.yml), not by inspection:

1. POST /site-promote's god_admin check read req.user.groups -- a field
   nothing in the codebase ever populates (Auth.checkToken returns
   User.get(), which has no .groups; every other admin gate resolves
   membership live via permission.byGroup()/Group.list(user.dn), which
   also handles nested-group membership). The check silently evaluated to
   an empty array on every request, so site-promote returned 403 for
   every user, including a real god_admin -- unusable since it shipped in
   v2.0.0. Fixed to use permission.byGroup(), the same pattern used
   elsewhere in this file and in api_site.js.

2. The read-only write-gate middleware (api_directory_admin.js) is
   registered before router.post('/site-promote', ...) later in the same
   file, so on a spoke it 403'd every promotion attempt before the
   handler ever ran -- the one mutating request a spoke must be able to
   make to itself. Exempted /site-promote from the gate.

Added coordinated demotion (MULTI_SITE_SPEC.md §3.2 -- promotion as ONE
action, never a two-step gap with two masters): site-promote now calls
the previous master's new POST /api/site/demote (Bearer the join key it
already holds, handing over a freshly-minted key for the demoted node's
own future use) before flipping itself to master. Best-effort: an
unreachable old master never blocks a god_admin's local promotion (the
WAN-outage scenario is the entire reason this control exists), it's
just reported in the response for manual reconciliation.

e2e test extended to promote the spoke, verify the old master was
actually demoted (isMaster:false, masterUrl pointing at the new master),
and verify writes now succeed on the new master and 403 on the old one.
Full chain verified passing: join -> live replication -> promotion ->
demotion -> write authority follows the promotion.
This commit is contained in:
William Mantly
2026-08-10 16:48:33 -04:00
parent d27763e556
commit 9c604f0258
3 changed files with 146 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
nodejs/routes/api_directory_admin.js
+59 -6
View File
@@ -2,6 +2,7 @@
const router = require('express').Router();
const permission = require('../utils/permission');
const { Resource, ResourceEdge, ResourceGroup } = require('../models/resource');
const { SiteJoinKey } = require('../models/site_join_key');
const { Group } = require('../models/group_ldap');
const { User } = require('../models/user_ldap');
const { cnFromDn } = require('../utils/user_groups');
@@ -266,9 +267,15 @@ router.get('/resources', async (req, res, next) => {
// changes. Fires on res.on('finish') (after the response is actually sent,
// status known) rather than before the handler runs, so a write that fails
// validation never triggers a pointless replication round-trip.
// /site-promote is deliberately exempt below: it's the ONE mutating request a
// spoke must be able to make to itself (that's the entire point -- a spoke
// promoting itself to master). Without this exemption the gate 403s the
// promotion request before it ever reaches the handler, since this
// middleware is registered ahead of router.post('/site-promote', ...) later
// in the file and Express matches router.use() against every path.
router.use((req, res, next) => {
const mutating = ['POST', 'PUT', 'DELETE', 'PATCH'].includes(req.method);
if (mutating) {
if (mutating && req.path !== '/site-promote') {
const cfg = siteConfig.get();
if (!cfg.isMaster) {
const hint = cfg.masterUrl ? ' Directory writes must go to the master at ' + cfg.masterUrl + '.' : '';
@@ -974,21 +981,67 @@ router.get('/site-status', async (req, res, next) => {
router.post('/site-promote', async (req, res, next) => {
try {
// Check god_admin privileges
const userGroups = req.user && req.user.groups ? req.user.groups : [];
const isGodAdmin = userGroups.includes('god_admin') || userGroups.includes(SUPER_ADMIN_GROUP);
// god_admin privilege check. This used to read req.user.groups, which
// nothing in the codebase ever populates -- User.get() (what
// Auth.checkToken returns as req.user) has no .groups field; every other
// admin gate in this app resolves membership live via
// permission.byGroup()/Group.list(user.dn), which also correctly
// resolves NESTED group membership (a user who is god_admin via a nested
// group, not just direct membership). The old check silently evaluated
// to an empty array for every request, making this endpoint
// unreachable for ANY user -- caught by the multi-site e2e promotion
// test (docker-compose.multisite-e2e.yml), not by inspection.
const isGodAdmin = await permission.byGroup(req.user, [SUPER_ADMIN_GROUP]).catch(() => false);
if (!isGodAdmin) {
return res.status(403).json({ status: 'error', message: 'Master promotion requires explicit god_admin authority' });
}
siteConfig.save({ isMaster: true, masterUrl: '' });
// MULTI_SITE_SPEC.md §3.2: promotion is ONE coordinated action, never a
// manual two-step "demote the old one first" — if we currently know a
// master (we were a spoke), hand it off before flipping ourselves. This
// is best-effort: an unreachable old master (the whole point of the
// WAN-outage promotion scenario §3 describes) must never block a
// god_admin's local promotion, it's just reported so the operator can
// reconcile it manually.
const beforeCfg = siteConfig.get();
let handoffNote = 'no previous master on file (already master, or fresh install)';
if (!beforeCfg.isMaster && beforeCfg.masterUrl && beforeCfg.masterJoinKey) {
try {
const { raw: freshKey } = await SiteJoinKey.issue({
label: 'promotion-handoff-' + new Date().toISOString().slice(0, 10),
createdBy: req.user ? req.user.uid : 'admin'
});
const controller = new AbortController();
const timer = setTimeout(() => controller.abort(), 15000);
let resp;
try {
resp = await fetch(beforeCfg.masterUrl + '/api/site/demote', {
method: 'POST',
headers: { Authorization: 'Bearer ' + beforeCfg.masterJoinKey, 'Content-Type': 'application/json' },
body: JSON.stringify({ newMasterUrl: (req.body && req.body.selfUrl) || '', newJoinKey: freshKey }),
signal: controller.signal
});
} finally { clearTimeout(timer); }
handoffNote = resp.ok ? 'previous master demoted' : ('previous master demote failed: HTTP ' + resp.status);
} catch (e) {
handoffNote = 'previous master unreachable (' + e.message + ') — promoted locally anyway; reconcile it manually once it\'s back';
}
}
console.log(`[MULTI-SITE] Node promoted to MASTER by user ${req.user ? req.user.uid : 'admin'}`);
siteConfig.save({ isMaster: true, masterUrl: '', masterJoinKey: undefined });
console.log(`[MULTI-SITE] Node promoted to MASTER by user ${req.user ? req.user.uid : 'admin'} (handoff: ${handoffNote})`);
// Fire-and-forget: let every known spoke know a new master exists so
// their next resync targets it. (They'll also learn this the hard way if
// their old-master resync calls start failing, but this speeds it up.)
meshReplicate.replicateToSpokes('master-promoted');
const cfg = siteConfig.get();
res.json({
status: 'ok',
message: 'Node successfully promoted to Master Site',
handoff: handoffNote,
config: {
isMaster: true,
masterUrl: '',
nodejs/routes/api_site.js
+34
View File
@@ -170,6 +170,40 @@ router.post('/resync', async (req, res, next) => {
} catch (e) { next(e); }
});
// ── Demote (called on the OLD master; Bearer site-join-key; no admin session)
// MULTI_SITE_SPEC.md §3.2: promoting a spoke must be a single coordinated
// action, never a two-step "hope nobody's master for a while" gap. The node
// being promoted calls this on whatever it currently believes is master,
// using the join-key credential it already holds from when it joined --
// authenticating "demote me" is exactly the same trust relationship as
// authenticating "let me pull an export," so no new credential type is
// needed for THIS direction. (The new master's future ability to push
// replication/resync to the newly-demoted node is a separate credential --
// newJoinKey below -- since that's the master->spoke direction, same as
// every other spoke registration.)
router.post('/demote', async (req, res, next) => {
try {
const auth = req.headers.authorization || '';
const rawKey = auth.startsWith('Bearer ') ? auth.slice(7).trim() : '';
const key = await SiteJoinKey.authenticate(rawKey);
if (!key) return res.status(401).json({ status: 'error', message: 'invalid or revoked site join key' });
const cfg = siteConfig.get();
if (!cfg.isMaster) {
return res.status(400).json({ status: 'error', message: 'this node is already a spoke' });
}
const { newMasterUrl, newJoinKey } = req.body || {};
if (!newMasterUrl || !newJoinKey) {
return res.status(400).json({ status: 'error', message: 'newMasterUrl and newJoinKey are required' });
}
const base = String(newMasterUrl).replace(/\/+$/, '');
siteConfig.save({ isMaster: false, masterUrl: base, masterJoinKey: newJoinKey });
logAudit('demoted', { demotedBy: key.keyPrefix, newMasterUrl: base });
res.json({ status: 'ok', message: 'Demoted to spoke of ' + base });
} catch (e) { next(e); }
});
// ── Everything below requires an admin session ──────────────────────────────
router.use(middleware.auth);
router.use(async (req, res, next) => {
test/multisite_join_e2e.js
+53 -3
View File
@@ -92,11 +92,24 @@ userPassword: ${hash}
await execFileAsync('ldapadd', ['-x', '-H', `ldap://${ldapHost}:389`, '-D', bindDn, '-w', LDAP_ADMIN_PASS], { input: ldif })
.catch((e) => { if (!/Already exists/.test(e.stderr || '')) throw e; });
for (const group of ['app_sso_admin']) {
// god_admin is needed for site-promote (SUPER_ADMIN_GROUP, utils/permission.js).
for (const group of ['app_sso_admin', 'god_admin']) {
const modLdif = `dn: cn=${group},ou=groups,${baseDn}\nchangetype: modify\nadd: member\nmember: cn=${ADMIN_UID},ou=people,${baseDn}\n`;
await execFileAsync('ldapmodify', ['-x', '-H', `ldap://${ldapHost}:389`, '-D', bindDn, '-w', LDAP_ADMIN_PASS], { input: modLdif })
.catch((e) => { if (!/[Tt]ype or value exists/.test(e.stderr || '')) throw e; });
try {
await execFileAsync('ldapmodify', ['-x', '-H', `ldap://${ldapHost}:389`, '-D', bindDn, '-w', LDAP_ADMIN_PASS], { input: modLdif });
console.log(` (added ${ADMIN_UID} to ${group} on ${ldapHost})`);
} catch (e) {
if (!/[Tt]ype or value exists/.test(e.stderr || '')) {
console.error(` FAILED adding ${ADMIN_UID} to ${group} on ${ldapHost}: ${e.stderr || e.message}`);
throw e;
}
console.log(` (${ADMIN_UID} already in ${group} on ${ldapHost})`);
}
}
const verify = await execFileAsync('ldapsearch', ['-x', '-H', `ldap://${ldapHost}:389`, '-D', bindDn, '-w', LDAP_ADMIN_PASS,
'-b', `cn=god_admin,ou=groups,${baseDn}`, 'member']);
console.log(` god_admin members on ${ldapHost}:\n${verify.stdout}`);
}
async function login(url) {
@@ -234,6 +247,43 @@ async function main() {
const { body: masterCfg } = await api(MASTER_URL, '/api/site/config', { token: masterToken });
if (masterCfg.config.isMaster !== true) fail('master flipped away from isMaster:true unexpectedly');
step('Promoting the spoke to master (coordinated handoff -- must demote the old master too)');
const promoteRes = await api(SPOKE_URL, '/api/directory-admin/site-promote', {
method: 'POST',
token: spokeToken,
body: { selfUrl: 'http://spoke:3001' }
});
if (promoteRes.status !== 200) fail(`promotion failed: ${promoteRes.status} ${JSON.stringify(promoteRes.body)}`);
if (promoteRes.body.handoff !== 'previous master demoted') {
fail(`expected the old master to be demoted as part of promotion, got handoff=${JSON.stringify(promoteRes.body.handoff)}`);
}
step('Verifying the newly-promoted node is master');
const { body: newMasterCfg } = await api(SPOKE_URL, '/api/site/config', { token: spokeToken });
if (newMasterCfg.config.isMaster !== true) fail(`newly-promoted node should be isMaster:true, got ${JSON.stringify(newMasterCfg.config)}`);
step('Verifying the old master was actually demoted to a spoke of the new master');
const { body: oldMasterCfg } = await api(MASTER_URL, '/api/site/config', { token: masterToken });
if (oldMasterCfg.config.isMaster !== false) fail(`old master should be isMaster:false after being demoted, got ${JSON.stringify(oldMasterCfg.config)}`);
if (oldMasterCfg.config.masterUrl !== 'http://spoke:3001') {
fail(`old master's masterUrl should now point at the new master, got ${JSON.stringify(oldMasterCfg.config.masterUrl)}`);
}
step('Verifying the (now-demoted) old master rejects writes, and the new master accepts them');
const oldMasterWrite = await api(MASTER_URL, '/api/directory-admin/resources', {
method: 'POST',
token: masterToken,
body: { name: 'Should Be Rejected Post-Demotion', slug: 'host_e2e_should_reject_2', kind: 'host' }
});
if (oldMasterWrite.status !== 403) fail(`expected 403 writing to the demoted old master, got ${oldMasterWrite.status} ${JSON.stringify(oldMasterWrite.body)}`);
const newMasterWrite = await api(SPOKE_URL, '/api/directory-admin/resources', {
method: 'POST',
token: spokeToken,
body: { name: 'E2E Post-Promotion Host', slug: 'host_e2e_postpromotion', kind: 'host', parentSlug: 'site_e2e' }
});
if (newMasterWrite.status !== 200) fail(`expected the newly-promoted master to accept writes, got ${newMasterWrite.status} ${JSON.stringify(newMasterWrite.body)}`);
if (failed) {
console.error('MULTISITE E2E: one or more checks failed (see above)');
process.exit(1);