From b543ad43f343083cdbb9765aef412d9de67fe717 Mon Sep 17 00:00:00 2001 From: garvitkaushik-123 Date: Sat, 29 Aug 2026 13:44:07 +0530 Subject: [PATCH 1/3] fix(admin): require stated intent before promote deletes memberships MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `POST /:userId/credentials/:credentialId/promote` runs `mergeUsers` to move the outgoing primary's app-state onto the incoming credential. Where both credentials belong to the same organization, the unique constraint on `organization_memberships` makes that a delete: the outgoing row's role, seat type, and upstream WorkOS membership id are gone, and unlinking afterwards cannot restore them. The sibling bind path, `POST /:userId/credentials`, already refuses to consolidate silently and requires `consolidate: true`. Promote had no such gate. It now returns 409 with the affected organization ids unless the caller confirms, and reports only the overlapping organizations — non-overlapping memberships move forward intact, so gating on those would refuse every ordinary promote. Refs #6827 Co-Authored-By: Claude Opus 5 --- server/src/db/membership-consolidation-db.ts | 38 ++++++ server/src/routes/admin/users.ts | 21 ++++ .../membership-consolidation.test.ts | 114 ++++++++++++++++++ 3 files changed, 173 insertions(+) create mode 100644 server/src/db/membership-consolidation-db.ts create mode 100644 server/tests/integration/membership-consolidation.test.ts diff --git a/server/src/db/membership-consolidation-db.ts b/server/src/db/membership-consolidation-db.ts new file mode 100644 index 0000000000..a54279dd6a --- /dev/null +++ b/server/src/db/membership-consolidation-db.ts @@ -0,0 +1,38 @@ +/** + * Consolidation checks for the credential-binding admin paths (#6827). + * + * `mergeUsers` moves an organization membership onto the target credential by + * rewriting `workos_user_id`. Where the target already belongs to the same + * organization the unique constraint forces the source row to be deleted + * instead, and its role, seat type, and upstream WorkOS membership id are + * gone — an unlink afterwards cannot restore them. Endpoints that trigger a + * consolidation use this to require stated operator intent first. + */ + +import { query } from './client.js'; + +/** + * Organizations where consolidating `sourceUserId` onto `targetUserId` would + * DELETE the source's membership because the target already holds one. + * + * Only the overlap is reported. Non-overlapping memberships move forward + * intact, so blocking on those would refuse every ordinary consolidation. + */ +export async function findSupersededMembershipOrganizations( + sourceUserId: string, + targetUserId: string, +): Promise { + const result = await 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], + ); + return result.rows.map((row) => row.workos_organization_id); +} diff --git a/server/src/routes/admin/users.ts b/server/src/routes/admin/users.ts index be86eaca19..148500f3c6 100644 --- a/server/src/routes/admin/users.ts +++ b/server/src/routes/admin/users.ts @@ -16,6 +16,7 @@ import { import { SlackDatabase } from '../../db/slack-db.js'; import { WorkingGroupDatabase } from '../../db/working-group-db.js'; import { getPool } from '../../db/client.js'; +import { findSupersededMembershipOrganizations } 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'; @@ -1365,6 +1366,26 @@ export function createAdminUsersRouter(): Router { }); } + // Foot-gun gate: promote runs the same consolidation as link-credential. + // Memberships in an organization the incoming credential already belongs + // to are DELETED by it — their role, seat, and WorkOS membership id 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 findSupersededMembershipOrganizations( + currentPrimaryId, + newPrimaryId + ); + if (superseded.length > 0) { + return res.status(409).json({ + error: 'Promoting would delete organization memberships', + message: `The outgoing primary holds memberships in ${superseded.length} organization(s) the incoming credential already belongs to. Promoting deletes those rows — their role, seat, and WorkOS membership id cannot be recovered. Re-submit with \`"consolidate": true\` to confirm this is intended.`, + consolidate_confirmation_required: true, + superseded_organization_ids: superseded, + }); + } + } + // 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/membership-consolidation.test.ts b/server/tests/integration/membership-consolidation.test.ts new file mode 100644 index 0000000000..5b0e6ada71 --- /dev/null +++ b/server/tests/integration/membership-consolidation.test.ts @@ -0,0 +1,114 @@ +/** + * 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 { findSupersededMembershipOrganizations } from '../../src/db/membership-consolidation-db.js'; + +const TEST_USER_PREFIX = 'user_consolidation_test_'; +const TEST_ORG_PREFIX = 'org_consolidation_test_'; + +describe('findSupersededMembershipOrganizations', () => { + 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 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; + } + + 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 findSupersededMembershipOrganizations(sourceId, targetId)).toEqual([ + `${TEST_ORG_PREFIX}shared`, + ]); + }); + + 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 findSupersededMembershipOrganizations(sourceId, targetId)).toEqual([]); + }); + + 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 findSupersededMembershipOrganizations(sourceId, targetId)).toEqual([ + `${TEST_ORG_PREFIX}mixed_shared`, + ]); + }); + + 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 findSupersededMembershipOrganizations(sourceId, targetId)).toEqual([]); + expect(await findSupersededMembershipOrganizations(targetId, sourceId)).toEqual([]); + }); + + 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 findSupersededMembershipOrganizations(sourceId, targetId)).toEqual([]); + }); +}); From 6c7ad5e86fa5853e89ea0bf45fb8d86e590160fb Mon Sep 17 00:00:00 2001 From: garvitkaushik-123 Date: Sat, 29 Aug 2026 13:58:20 +0530 Subject: [PATCH 2/3] fix(admin): report working-group overlap in the promote gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review on #7002: the gate enumerated only organization memberships, but the same mergeUsers call deduplicates working_group_memberships too, so a promote that overlapped in a working group but not an organization deleted the outgoing row with no 409 — and an operator reading the enumerated ids took them as the whole loss surface. Report both membership tables. The remaining conflict tables mergeUsers touches — learner_progress, user_credentials, committee_interest, user_badges — stay out of the blocking check: those are records rather than authority, and refusing a promote over an overlapping badge or completed module would block nearly every legitimate consolidation. The 409 message now names them so the enumerated ids are not read as exhaustive. Refs #6827 Co-Authored-By: Claude Opus 5 --- server/src/db/membership-consolidation-db.ts | 81 ++++++++++----- server/src/routes/admin/users.ts | 27 ++--- .../membership-consolidation.test.ts | 99 ++++++++++++++++--- 3 files changed, 158 insertions(+), 49 deletions(-) diff --git a/server/src/db/membership-consolidation-db.ts b/server/src/db/membership-consolidation-db.ts index a54279dd6a..464d0ca205 100644 --- a/server/src/db/membership-consolidation-db.ts +++ b/server/src/db/membership-consolidation-db.ts @@ -1,38 +1,71 @@ /** * Consolidation checks for the credential-binding admin paths (#6827). * - * `mergeUsers` moves an organization membership onto the target credential by - * rewriting `workos_user_id`. Where the target already belongs to the same - * organization the unique constraint forces the source row to be deleted - * instead, and its role, seat type, and upstream WorkOS membership id are - * gone — an unlink afterwards cannot restore them. Endpoints that trigger a - * consolidation use this to require stated operator intent first. + * `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[]; +} + /** - * Organizations where consolidating `sourceUserId` onto `targetUserId` would - * DELETE the source's membership because the target already holds one. + * 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 blocking on those would refuse every ordinary consolidation. + * intact, so including those would refuse every ordinary consolidation. */ -export async function findSupersededMembershipOrganizations( +export async function findSupersededMemberships( sourceUserId: string, targetUserId: string, -): Promise { - const result = await 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], - ); - return result.rows.map((row) => row.workos_organization_id); +): 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 148500f3c6..620992b862 100644 --- a/server/src/routes/admin/users.ts +++ b/server/src/routes/admin/users.ts @@ -16,7 +16,7 @@ import { import { SlackDatabase } from '../../db/slack-db.js'; import { WorkingGroupDatabase } from '../../db/working-group-db.js'; import { getPool } from '../../db/client.js'; -import { findSupersededMembershipOrganizations } from '../../db/membership-consolidation-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'; @@ -1367,21 +1367,22 @@ export function createAdminUsersRouter(): Router { } // Foot-gun gate: promote runs the same consolidation as link-credential. - // Memberships in an organization the incoming credential already belongs - // to are DELETED by it — their role, seat, and WorkOS membership id are - // not recoverable, and an unlink cannot put them back. Require stated - // intent, matching the `consolidate: true` gate on the bind path above. + // 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 findSupersededMembershipOrganizations( - currentPrimaryId, - newPrimaryId - ); - if (superseded.length > 0) { + 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 organization memberships', - message: `The outgoing primary holds memberships in ${superseded.length} organization(s) the incoming credential already belongs to. Promoting deletes those rows — their role, seat, and WorkOS membership id cannot be recovered. Re-submit with \`"consolidate": true\` to confirm this is intended.`, + 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, + superseded_organization_ids: superseded.organizationIds, + superseded_working_group_ids: superseded.workingGroupIds, }); } } diff --git a/server/tests/integration/membership-consolidation.test.ts b/server/tests/integration/membership-consolidation.test.ts index 5b0e6ada71..3a64661925 100644 --- a/server/tests/integration/membership-consolidation.test.ts +++ b/server/tests/integration/membership-consolidation.test.ts @@ -11,12 +11,13 @@ 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 { findSupersededMembershipOrganizations } from '../../src/db/membership-consolidation-db.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('findSupersededMembershipOrganizations', () => { +describe('findSupersededMemberships', () => { let pool: Pool; beforeAll(async () => { @@ -38,6 +39,10 @@ describe('findSupersededMembershipOrganizations', () => { 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}%`]); } @@ -63,15 +68,32 @@ describe('findSupersededMembershipOrganizations', () => { 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 findSupersededMembershipOrganizations(sourceId, targetId)).toEqual([ - `${TEST_ORG_PREFIX}shared`, - ]); + expect(await findSupersededMemberships(sourceId, targetId)).toEqual({ + organizationIds: [`${TEST_ORG_PREFIX}shared`], + workingGroupIds: [], + }); }); it('ignores memberships that would move forward intact', async () => { @@ -80,7 +102,10 @@ describe('findSupersededMembershipOrganizations', () => { await insertMembership(targetId, 'target_only'); await insertMembership(sourceId, 'source_only'); - expect(await findSupersededMembershipOrganizations(sourceId, targetId)).toEqual([]); + expect(await findSupersededMemberships(sourceId, targetId)).toEqual({ + organizationIds: [], + workingGroupIds: [], + }); }); it('reports only the overlap when the source holds both kinds', async () => { @@ -90,9 +115,10 @@ describe('findSupersededMembershipOrganizations', () => { await insertMembership(sourceId, 'mixed_shared'); await insertMembership(sourceId, 'mixed_solo'); - expect(await findSupersededMembershipOrganizations(sourceId, targetId)).toEqual([ - `${TEST_ORG_PREFIX}mixed_shared`, - ]); + expect(await findSupersededMemberships(sourceId, targetId)).toEqual({ + organizationIds: [`${TEST_ORG_PREFIX}mixed_shared`], + workingGroupIds: [], + }); }); it('is directional — the reverse check is independent', async () => { @@ -100,8 +126,54 @@ describe('findSupersededMembershipOrganizations', () => { const sourceId = await insertUser('directional_source'); await insertMembership(sourceId, 'directional_solo'); - expect(await findSupersededMembershipOrganizations(sourceId, targetId)).toEqual([]); - expect(await findSupersededMembershipOrganizations(targetId, sourceId)).toEqual([]); + 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 () => { @@ -109,6 +181,9 @@ describe('findSupersededMembershipOrganizations', () => { const sourceId = await insertUser('empty_source'); await insertMembership(targetId, 'empty_target_only'); - expect(await findSupersededMembershipOrganizations(sourceId, targetId)).toEqual([]); + expect(await findSupersededMemberships(sourceId, targetId)).toEqual({ + organizationIds: [], + workingGroupIds: [], + }); }); }); From c95c95976c88ac82e5b6eb802bb7added7822964 Mon Sep 17 00:00:00 2001 From: garvitkaushik-123 Date: Sat, 29 Aug 2026 14:25:10 +0530 Subject: [PATCH 3/3] fix(admin): let the promote UI confirm a consolidation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gate added in this branch returns 409 when promoting would delete a membership the incoming credential already holds, but `promoteCredential()` in admin-people.html posts no body and treats every non-2xx as a dead end. An admin hitting the overlap saw an alert and had no way to proceed, so the operation went from silently destructive to unreachable through the product. Mirror the escalation the link-existing flow in the same file already implements: on `consolidate_confirmation_required`, name what would be deleted, require the literal CONSOLIDATE, and retry with `{ consolidate: true }`. Cover the endpoint end to end as well — the 409 with its enumerated ids and an unchanged primary, the confirmed retry that promotes, and a non-overlapping promote that still needs no confirmation. Verified the 409 case fails against a neutered gate. Refs #6827 Co-Authored-By: Claude Opus 5 --- server/public/admin-people.html | 30 +++++++-- .../admin-promote-credential.test.ts | 61 +++++++++++++++++++ 2 files changed, 87 insertions(+), 4 deletions(-) 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/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)