diff --git a/server/public/admin-people.html b/server/public/admin-people.html index 3b69015d13..e86144eed0 100644 --- a/server/public/admin-people.html +++ b/server/public/admin-people.html @@ -643,11 +643,33 @@

Sign-in cre alert('Confirmation did not match. Cancelled.'); return; } + const promoteUrl = `/api/admin/users/${encodeURIComponent(hostId)}/credentials/${encodeURIComponent(credId)}/promote`; try { - const res = await AdminSidebar.fetch(`/api/admin/users/${encodeURIComponent(hostId)}/credentials/${encodeURIComponent(credId)}/promote`, { - method: 'POST', - }); - const data = await res.json(); + let res = await AdminSidebar.fetch(promoteUrl, { method: 'POST' }); + let data = await res.json(); + // Server refused because the consolidation would DELETE memberships + // the incoming credential already holds. Escalate the confirmation + // and retry, same shape as the link-existing flow above. + if (res.status === 409 && data.consolidate_confirmation_required) { + const orgCount = (data.superseded_organization_ids || []).length; + const wgCount = (data.superseded_working_group_ids || []).length; + const lost = [ + orgCount ? `${orgCount} organization membership(s)` : null, + wgCount ? `${wgCount} working-group membership(s)` : null, + ].filter(Boolean).join(' and '); + const ack = prompt(`${credLabel} already belongs to the same organizations or working groups as the current primary.\n\nPromoting DELETES the current primary's ${lost}. Their role, seat, WorkOS membership id, and join provenance cannot be recovered — unlinking later will not bring them back. Certification progress, credentials, badges, and committee interest are also consolidated and deduplicated.\n\nType CONSOLIDATE (in caps) to confirm.`); + if (ack === null) return; + if (ack.trim() !== 'CONSOLIDATE') { + alert('Confirmation phrase did not match. Cancelled.'); + return; + } + res = await AdminSidebar.fetch(promoteUrl, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ consolidate: true }), + }); + data = await res.json(); + } if (!res.ok) { alert(data.message || data.error || `Promote failed (${res.status})`); return; diff --git a/server/src/db/membership-consolidation-db.ts b/server/src/db/membership-consolidation-db.ts new file mode 100644 index 0000000000..464d0ca205 --- /dev/null +++ b/server/src/db/membership-consolidation-db.ts @@ -0,0 +1,71 @@ +/** + * Consolidation checks for the credential-binding admin paths (#6827). + * + * `mergeUsers` moves a membership onto the target credential by rewriting + * `workos_user_id`. Where the target already holds a row for the same + * organization or working group, the unique constraint turns that move into a + * delete and the source row's role, seat type, upstream membership id, and + * join provenance are gone — an unlink afterwards cannot restore them. + * Endpoints that trigger a consolidation use this to require stated operator + * intent first. + * + * Scope is the membership tables. `mergeUsers` also deduplicates + * `learner_progress`, `user_credentials`, `committee_interest`, and + * `user_badges`; those are records rather than authority, and refusing a + * promote over an overlapping badge or completed module would block nearly + * every legitimate consolidation. Callers say so in the confirmation text + * instead of enumerating them. + */ + +import { query } from './client.js'; + +export interface SupersededMembershipOverlap { + /** Organizations where the source's `organization_memberships` row is deleted. */ + organizationIds: string[]; + /** Working groups where the source's `working_group_memberships` row is deleted. */ + workingGroupIds: string[]; +} + +/** + * Memberships that consolidating `sourceUserId` onto `targetUserId` would + * DELETE because the target already holds a row for the same partner key. + * + * Only the overlap is reported. Non-overlapping memberships move forward + * intact, so including those would refuse every ordinary consolidation. + */ +export async function findSupersededMemberships( + sourceUserId: string, + targetUserId: string, +): Promise { + const [organizations, workingGroups] = await Promise.all([ + query<{ workos_organization_id: string }>( + `SELECT om.workos_organization_id + FROM organization_memberships om + WHERE om.workos_user_id = $1 + AND EXISTS ( + SELECT 1 FROM organization_memberships target + WHERE target.workos_user_id = $2 + AND target.workos_organization_id = om.workos_organization_id + ) + ORDER BY om.workos_organization_id`, + [sourceUserId, targetUserId], + ), + query<{ working_group_id: string }>( + `SELECT wgm.working_group_id + FROM working_group_memberships wgm + WHERE wgm.workos_user_id = $1 + AND EXISTS ( + SELECT 1 FROM working_group_memberships target + WHERE target.workos_user_id = $2 + AND target.working_group_id = wgm.working_group_id + ) + ORDER BY wgm.working_group_id`, + [sourceUserId, targetUserId], + ), + ]); + + return { + organizationIds: organizations.rows.map((row) => row.workos_organization_id), + workingGroupIds: workingGroups.rows.map((row) => row.working_group_id), + }; +} diff --git a/server/src/routes/admin/users.ts b/server/src/routes/admin/users.ts index 4655f7b024..c0fb72974d 100644 --- a/server/src/routes/admin/users.ts +++ b/server/src/routes/admin/users.ts @@ -17,6 +17,7 @@ import { SlackDatabase } from '../../db/slack-db.js'; import { WorkingGroupDatabase } from '../../db/working-group-db.js'; import { getPool } from '../../db/client.js'; import { bumpAuthorizationEpochs } from '../../db/authorization-epoch-db.js'; +import { findSupersededMemberships } from '../../db/membership-consolidation-db.js'; import { backfillOrganizationMemberships, backfillUsers, backfillOrganizationDomains } from '../workos-webhooks.js'; import { sendSlackInviteEmail, hasSlackInviteBeenSent } from '../../notifications/email.js'; import { getWorkos } from '../../auth/workos-client.js'; @@ -1395,6 +1396,27 @@ export function createAdminUsersRouter(): Router { }); } + // Foot-gun gate: promote runs the same consolidation as link-credential. + // Memberships whose partner key the incoming credential already holds are + // DELETED by it — the outgoing row's role, seat, upstream membership id, + // and join provenance are not recoverable, and an unlink cannot put them + // back. Require stated intent, matching the `consolidate: true` gate on + // the bind path above. + if (req.body?.consolidate !== true) { + const superseded = await findSupersededMemberships(currentPrimaryId, newPrimaryId); + const supersededCount = + superseded.organizationIds.length + superseded.workingGroupIds.length; + if (supersededCount > 0) { + return res.status(409).json({ + error: 'Promoting would delete memberships', + message: `The outgoing primary holds ${supersededCount} membership(s) whose organization or working group the incoming credential already belongs to. Promoting deletes those rows — their role, seat, WorkOS membership id, and join provenance cannot be recovered. Certification progress, credentials, badges, and committee interest are consolidated in the same operation and deduplicated on conflict; they are not enumerated here. Re-submit with \`"consolidate": true\` to confirm this is intended.`, + consolidate_confirmation_required: true, + superseded_organization_ids: superseded.organizationIds, + superseded_working_group_ids: superseded.workingGroupIds, + }); + } + } + // Run mergeUsers with ensurePrimaryFlag so the data move, the secondary // rebind, AND the new primary's is_primary=TRUE flip all happen in one // transaction. This closes the window where the identity has zero diff --git a/server/tests/integration/admin-promote-credential.test.ts b/server/tests/integration/admin-promote-credential.test.ts index c03167bf66..36a63a96ce 100644 --- a/server/tests/integration/admin-promote-credential.test.ts +++ b/server/tests/integration/admin-promote-credential.test.ts @@ -193,6 +193,67 @@ describe('admin promote credential to primary', () => { expect(memberships.rows.every(r => r.workos_user_id === TARGET_USER_ID)).toBe(true); }); + /** + * Both credentials hold a membership in the same organization. The + * consolidation deletes the outgoing primary's row, so the endpoint must + * refuse without stated intent. + */ + async function setupOverlappingPair() { + await setupBoundPair(); + // Give the target its own membership in the host's org, so promoting + // would delete the host's row for that org rather than move it. + await pool.query( + `INSERT INTO organization_memberships (workos_user_id, workos_organization_id, email, role, created_at, updated_at) + VALUES ($1, $2, 'target@test.example', 'member', NOW(), NOW())`, + [TARGET_USER_ID, HOST_ORG_ID] + ); + } + + it('409s rather than silently deleting a membership the target already holds', async () => { + await setupOverlappingPair(); + + const response = await request(app) + .post(`/api/admin/users/${HOST_USER_ID}/credentials/${TARGET_USER_ID}/promote`) + .expect(409); + + expect(response.body).toMatchObject({ + consolidate_confirmation_required: true, + superseded_organization_ids: [HOST_ORG_ID], + superseded_working_group_ids: [], + }); + + // Nothing moved: the host is still primary and still holds its row. + const bindings = await pool.query<{ is_primary: boolean }>( + `SELECT is_primary FROM identity_workos_users WHERE workos_user_id = $1`, + [HOST_USER_ID] + ); + expect(bindings.rows[0].is_primary).toBe(true); + }); + + it('promotes once the caller confirms the consolidation', async () => { + await setupOverlappingPair(); + + await request(app) + .post(`/api/admin/users/${HOST_USER_ID}/credentials/${TARGET_USER_ID}/promote`) + .send({ consolidate: true }) + .expect(200); + + const bindings = await pool.query<{ workos_user_id: string; is_primary: boolean }>( + `SELECT workos_user_id, is_primary FROM identity_workos_users + WHERE workos_user_id IN ($1, $2)`, + [HOST_USER_ID, TARGET_USER_ID] + ); + expect(bindings.rows.find(r => r.workos_user_id === TARGET_USER_ID)?.is_primary).toBe(true); + }); + + it('promotes without confirmation when the memberships do not overlap', async () => { + await setupBoundPair(); + + await request(app) + .post(`/api/admin/users/${HOST_USER_ID}/credentials/${TARGET_USER_ID}/promote`) + .expect(200); + }); + it('writes a promote_credential_to_primary audit row', async () => { await setupBoundPair(); await request(app) diff --git a/server/tests/integration/membership-consolidation.test.ts b/server/tests/integration/membership-consolidation.test.ts new file mode 100644 index 0000000000..3a64661925 --- /dev/null +++ b/server/tests/integration/membership-consolidation.test.ts @@ -0,0 +1,189 @@ +/** + * Consolidation overlap detection (#6827). + * + * The promote endpoint runs the same consolidation as link-credential, which + * deletes the outgoing credential's membership wherever the incoming one + * already belongs to the organization. Only that overlap warrants refusing + * the operation — non-overlapping memberships move forward intact. + */ + +import { describe, it, expect, beforeAll, afterAll, beforeEach } from 'vitest'; +import type { Pool } from 'pg'; +import { initializeDatabase, closeDatabase } from '../../src/db/client.js'; +import { runMigrations } from '../../src/db/migrate.js'; +import { findSupersededMemberships } from '../../src/db/membership-consolidation-db.js'; + +const TEST_USER_PREFIX = 'user_consolidation_test_'; +const TEST_ORG_PREFIX = 'org_consolidation_test_'; +const TEST_WG_PREFIX = 'wg-consolidation-test-'; + +describe('findSupersededMemberships', () => { + let pool: Pool; + + beforeAll(async () => { + pool = initializeDatabase({ + connectionString: + process.env.DATABASE_URL || 'postgresql://adcp:localdev@localhost:5432/adcp_test', + }); + await runMigrations(); + }, 60000); + + afterAll(async () => { + await cleanup(); + await closeDatabase(); + }); + + beforeEach(cleanup); + + async function cleanup() { + await pool.query(`DELETE FROM organization_memberships WHERE workos_user_id LIKE $1`, [ + `${TEST_USER_PREFIX}%`, + ]); + await pool.query(`DELETE FROM working_group_memberships WHERE workos_user_id LIKE $1`, [ + `${TEST_USER_PREFIX}%`, + ]); + await pool.query(`DELETE FROM working_groups WHERE slug LIKE $1`, [`${TEST_WG_PREFIX}%`]); + await pool.query(`DELETE FROM users WHERE workos_user_id LIKE $1`, [`${TEST_USER_PREFIX}%`]); + } + + async function insertUser(suffix: string): Promise { + const userId = `${TEST_USER_PREFIX}${suffix}`; + await pool.query( + `INSERT INTO users (workos_user_id, email, first_name, last_name, email_verified, + workos_created_at, workos_updated_at, created_at, updated_at) + VALUES ($1, $2, 'Test', 'User', true, NOW(), NOW(), NOW(), NOW())`, + [userId, `${suffix}@consolidation.test`] + ); + return userId; + } + + async function insertMembership(userId: string, orgSuffix: string): Promise { + const orgId = `${TEST_ORG_PREFIX}${orgSuffix}`; + await pool.query( + `INSERT INTO organization_memberships ( + workos_user_id, workos_organization_id, workos_membership_id, email, role, seat_type + ) VALUES ($1, $2, $3, $4, 'member', 'community_only')`, + [userId, orgId, `om_${userId}_${orgSuffix}`, `${userId}@consolidation.test`] + ); + return orgId; + } + + async function insertWorkingGroup(suffix: string): Promise { + const result = await pool.query<{ id: string }>( + `INSERT INTO working_groups (name, slug) VALUES ($1, $2) RETURNING id`, + [`Consolidation Test ${suffix}`, `${TEST_WG_PREFIX}${suffix}`] + ); + return result.rows[0].id; + } + + async function insertWorkingGroupMembership(userId: string, workingGroupId: string) { + await pool.query( + `INSERT INTO working_group_memberships (working_group_id, workos_user_id, user_email) + VALUES ($1, $2, $3)`, + [workingGroupId, userId, `${userId}@consolidation.test`] + ); + } + + it('reports the organizations both credentials belong to', async () => { + const targetId = await insertUser('overlap_target'); + const sourceId = await insertUser('overlap_source'); + await insertMembership(targetId, 'shared'); + await insertMembership(sourceId, 'shared'); + + expect(await findSupersededMemberships(sourceId, targetId)).toEqual({ + organizationIds: [`${TEST_ORG_PREFIX}shared`], + workingGroupIds: [], + }); + }); + + it('ignores memberships that would move forward intact', async () => { + const targetId = await insertUser('solo_target'); + const sourceId = await insertUser('solo_source'); + await insertMembership(targetId, 'target_only'); + await insertMembership(sourceId, 'source_only'); + + expect(await findSupersededMemberships(sourceId, targetId)).toEqual({ + organizationIds: [], + workingGroupIds: [], + }); + }); + + it('reports only the overlap when the source holds both kinds', async () => { + const targetId = await insertUser('mixed_target'); + const sourceId = await insertUser('mixed_source'); + await insertMembership(targetId, 'mixed_shared'); + await insertMembership(sourceId, 'mixed_shared'); + await insertMembership(sourceId, 'mixed_solo'); + + expect(await findSupersededMemberships(sourceId, targetId)).toEqual({ + organizationIds: [`${TEST_ORG_PREFIX}mixed_shared`], + workingGroupIds: [], + }); + }); + + it('is directional — the reverse check is independent', async () => { + const targetId = await insertUser('directional_target'); + const sourceId = await insertUser('directional_source'); + await insertMembership(sourceId, 'directional_solo'); + + expect(await findSupersededMemberships(sourceId, targetId)).toEqual({ + organizationIds: [], + workingGroupIds: [], + }); + expect(await findSupersededMemberships(targetId, sourceId)).toEqual({ + organizationIds: [], + workingGroupIds: [], + }); + }); + + it('reports working groups both credentials belong to', async () => { + const targetId = await insertUser('wg_target'); + const sourceId = await insertUser('wg_source'); + const sharedGroupId = await insertWorkingGroup('shared'); + await insertWorkingGroupMembership(targetId, sharedGroupId); + await insertWorkingGroupMembership(sourceId, sharedGroupId); + + expect(await findSupersededMemberships(sourceId, targetId)).toEqual({ + organizationIds: [], + workingGroupIds: [sharedGroupId], + }); + }); + + it('ignores a working group only the source belongs to', async () => { + const targetId = await insertUser('wg_solo_target'); + const sourceId = await insertUser('wg_solo_source'); + const groupId = await insertWorkingGroup('solo'); + await insertWorkingGroupMembership(sourceId, groupId); + + expect(await findSupersededMemberships(sourceId, targetId)).toEqual({ + organizationIds: [], + workingGroupIds: [], + }); + }); + + it('reports an organization and a working group overlap together', async () => { + const targetId = await insertUser('both_target'); + const sourceId = await insertUser('both_source'); + await insertMembership(targetId, 'both_shared'); + await insertMembership(sourceId, 'both_shared'); + const sharedGroupId = await insertWorkingGroup('both'); + await insertWorkingGroupMembership(targetId, sharedGroupId); + await insertWorkingGroupMembership(sourceId, sharedGroupId); + + expect(await findSupersededMemberships(sourceId, targetId)).toEqual({ + organizationIds: [`${TEST_ORG_PREFIX}both_shared`], + workingGroupIds: [sharedGroupId], + }); + }); + + it('returns nothing when the source holds no memberships', async () => { + const targetId = await insertUser('empty_target'); + const sourceId = await insertUser('empty_source'); + await insertMembership(targetId, 'empty_target_only'); + + expect(await findSupersededMemberships(sourceId, targetId)).toEqual({ + organizationIds: [], + workingGroupIds: [], + }); + }); +});