fix(api): exclude disabled and invited users from PAM elevation approvers (#3174)

apps/api/src/services/pamApprovers.test.ts

@@ -14,6 +14,7 @@ vi.mock('../db/schema', () => ({
   rolePermissions: { roleId: 'role_id', permissionId: 'permission_id' },
   permissions: { id: 'id', resource: 'resource', action: 'action' },
   mobileDevices: { userId: 'user_id', status: 'status', notificationsEnabled: 'notifications_enabled' },
+  users: { id: 'users.id', status: 'users.status' },
 }));
 
 import { db } from '../db';
@@ -23,11 +24,29 @@ import { resolveElevationApprovers } from './pamApprovers';
  * The resolver issues these selects in order:
  *   1. granting roles:  select().from(rolePermissions).innerJoin(permissions).where()
  *   2. org partner:     select().from(organizations).where().limit()
- *   3. org members:     select().from(organizationUsers).where()
- *   4. partner members: select().from(partnerUsers).where()
+ *   3. org members:     select().from(organizationUsers).innerJoin(users).where()
+ *   4. partner members: select().from(partnerUsers).innerJoin(users).where()
  *   5. mobile devices:  select().from(mobileDevices).where()
  * (4 is skipped when the org has no partner; 5 is skipped when no candidates.)
+ *
+ * 3 and 4 gained their `users` innerJoin in #3174. `spies` exposes the
+ * arguments those two calls actually received so a test can assert on the real
+ * join target and predicate rather than on what the mock was told to return —
+ * the mock resolves its rows regardless of the WHERE, so a test that only
+ * checked returned ids could not fail if the status gate were deleted.
  */
+const spies = {
+  orgMembersInnerJoin: vi.fn(),
+  orgMembersWhere: vi.fn(),
+  partnerMembersInnerJoin: vi.fn(),
+  partnerMembersWhere: vi.fn(),
+};
+
+/** Serialize a drizzle condition so a test can look for a column/value in it. */
+function conditionText(cond: unknown): string {
+  return JSON.stringify(cond, (_k, v) => (typeof v === 'bigint' ? String(v) : v)) ?? '';
+}
+
 function queueSelects(opts: {
   grantingRoles: Array<{ roleId: string }>;
   org: Array<{ partnerId: string | null }>;
@@ -50,18 +69,18 @@ function queueSelects(opts: {
     }),
   } as any);
 
-  // 3. org members (where())
+  // 3. org members (innerJoin().where())
+  spies.orgMembersWhere.mockResolvedValue(opts.orgMembers);
+  spies.orgMembersInnerJoin.mockReturnValue({ where: spies.orgMembersWhere });
   vi.mocked(db.select).mockReturnValueOnce({
-    from: vi.fn().mockReturnValue({
-      where: vi.fn().mockResolvedValue(opts.orgMembers),
-    }),
+    from: vi.fn().mockReturnValue({ innerJoin: spies.orgMembersInnerJoin }),
   } as any);
 
-  // 4. partner members (where())
+  // 4. partner members (innerJoin().where())
+  spies.partnerMembersWhere.mockResolvedValue(opts.partnerMembers);
+  spies.partnerMembersInnerJoin.mockReturnValue({ where: spies.partnerMembersWhere });
   vi.mocked(db.select).mockReturnValueOnce({
-    from: vi.fn().mockReturnValue({
-      where: vi.fn().mockResolvedValue(opts.partnerMembers),
-    }),
+    from: vi.fn().mockReturnValue({ innerJoin: spies.partnerMembersInnerJoin }),
   } as any);
 
   // 5. mobile devices (where())
@@ -76,6 +95,7 @@ describe('resolveElevationApprovers', () => {
   beforeEach(() => {
     vi.clearAllMocks();
     vi.mocked(db.select).mockReset();
+    for (const s of Object.values(spies)) s.mockReset();
   });
 
   it('returns distinct userIds with an active mobile device (org + partner members)', async () => {
@@ -129,6 +149,52 @@ describe('resolveElevationApprovers', () => {
     expect(db.select).toHaveBeenCalledTimes(1);
   });
 
+  // #3174: memberships survive an account being disabled or left in 'invited',
+  // so both candidate queries must join `users` and require status='active'.
+  // These assert the join target and the predicate that reach drizzle, not the
+  // ids the mock was primed to return — the latter cannot fail if the gate goes.
+  it('gates direct org members on an active user account', async () => {
+    queueSelects({
+      grantingRoles: [{ roleId: 'role-exec' }],
+      org: [{ partnerId: 'partner-1' }],
+      orgMembers: [{ userId: 'u-org' }],
+      partnerMembers: [],
+      mobile: [{ userId: 'u-org' }],
+    });
+
+    await resolveElevationApprovers('org-1');
+
+    expect(spies.orgMembersInnerJoin).toHaveBeenCalledTimes(1);
+    expect(spies.orgMembersInnerJoin.mock.calls[0]?.[0]).toEqual({
+      id: 'users.id',
+      status: 'users.status',
+    });
+    const where = conditionText(spies.orgMembersWhere.mock.calls[0]?.[0]);
+    expect(where).toContain('users.status');
+    expect(where).toContain('active');
+  });
+
+  it('gates partner members on an active user account', async () => {
+    queueSelects({
+      grantingRoles: [{ roleId: 'role-exec' }],
+      org: [{ partnerId: 'partner-1' }],
+      orgMembers: [],
+      partnerMembers: [{ userId: 'u-all', orgAccess: 'all', orgIds: null }],
+      mobile: [{ userId: 'u-all' }],
+    });
+
+    await resolveElevationApprovers('org-1');
+
+    expect(spies.partnerMembersInnerJoin).toHaveBeenCalledTimes(1);
+    expect(spies.partnerMembersInnerJoin.mock.calls[0]?.[0]).toEqual({
+      id: 'users.id',
+      status: 'users.status',
+    });
+    const where = conditionText(spies.partnerMembersWhere.mock.calls[0]?.[0]);
+    expect(where).toContain('users.status');
+    expect(where).toContain('active');
+  });
+
   it('returns [] when eligible members have no active mobile device', async () => {
     queueSelects({
       grantingRoles: [{ roleId: 'role-exec' }],

apps/api/src/services/pamApprovers.ts

@@ -3,8 +3,9 @@
  *
  * Given an org, returns the distinct set of user ids who may approve a
  * uac_intercept elevation on their phone: a user is eligible iff
- *   1. their role in (or covering) the org grants DEVICES_EXECUTE, AND
- *   2. they have at least one active mobile device with notifications enabled
+ *   1. their account is active (users.status = 'active'), AND
+ *   2. their role in (or covering) the org grants DEVICES_EXECUTE, AND
+ *   3. they have at least one active mobile device with notifications enabled
  *      (mobile_devices.status = 'active' AND notifications_enabled = true).
  *
  * Org membership mirrors how permissions.ts resolves access:
@@ -26,6 +27,7 @@ import {
   rolePermissions,
   permissions,
   mobileDevices,
+  users,
 } from '../db/schema';
 import { PERMISSIONS } from './permissions';
 
@@ -64,19 +66,27 @@ export async function resolveElevationApprovers(orgId: string): Promise<string[]
 
     const candidateUserIds = new Set<string>();
 
-    // 1. Direct org members holding a devices:execute role.
+    // 1. Direct org members holding a devices:execute role. Joined against
+    // `users` and gated on status='active' (#3174) so a disabled or still-
+    // invited account is never counted as an eligible approver: memberships
+    // are retained when an account is disabled, so without this the approver
+    // set is inflated with people who can never respond, and any logic keyed
+    // on the approver count is skewed by those ghosts.
     const orgMembers = await db
       .select({ userId: organizationUsers.userId })
       .from(organizationUsers)
+      .innerJoin(users, eq(users.id, organizationUsers.userId))
       .where(
         and(
           eq(organizationUsers.orgId, orgId),
           inArray(organizationUsers.roleId, grantingRoleIds),
+          eq(users.status, 'active'),
         ),
       );
     for (const m of orgMembers) candidateUserIds.add(m.userId);
 
     // 2. Partner members of the org's partner whose org_access covers this org.
+    // Same `users` join + status='active' gate as above (#3174).
     if (org?.partnerId) {
       const partnerMembers = await db
         .select({
@@ -85,10 +95,12 @@ export async function resolveElevationApprovers(orgId: string): Promise<string[]
           orgIds: partnerUsers.orgIds,
         })
         .from(partnerUsers)
+        .innerJoin(users, eq(users.id, partnerUsers.userId))
         .where(
           and(
             eq(partnerUsers.partnerId, org.partnerId),
             inArray(partnerUsers.roleId, grantingRoleIds),
+            eq(users.status, 'active'),
           ),
         );
       for (const m of partnerMembers) {