[comp] Production Deploy

apps/api/src/people/people-invite.service.spec.ts

@@ -356,6 +356,97 @@ describe('PeopleInviteService', () => {
       );
     });
 
+    // Regression (CS): promoting an EXISTING ACTIVE member must upgrade their
+    // role in place. Previously an active member's role was left unchanged, so
+    // adding admin to an existing employee never granted app access and the
+    // user hit "Access Denied" after accepting.
+    it('upgrades an existing active member in place when promoted (no invitation, no email)', async () => {
+      (mockDb.user.findFirst as jest.Mock).mockResolvedValue({
+        id: 'user_existing',
+        email: 'zub@example.com',
+      });
+      (mockDb.member.findFirst as jest.Mock).mockResolvedValue({
+        id: 'member_existing',
+        role: 'employee',
+        deactivated: false,
+        isActive: true,
+      });
+      (mockDb.member.update as jest.Mock).mockResolvedValue({
+        id: 'member_existing',
+      });
+
+      const results = await service.inviteMembers({
+        ...baseParams,
+        invites: [{ email: 'zub@example.com', roles: ['admin', 'employee'] }],
+      });
+
+      expect(results[0].success).toBe(true);
+      // Role is unioned (sorted, de-duped) onto the existing membership.
+      expect(mockDb.member.update).toHaveBeenCalledWith(
+        expect.objectContaining({
+          where: { id: 'member_existing' },
+          data: { role: 'admin,employee' },
+        }),
+      );
+      // An already-active member is upgraded directly — no re-invitation, no email.
+      expect(mockDb.invitation.create).not.toHaveBeenCalled();
+      expect(mockTriggerEmail).not.toHaveBeenCalled();
+    });
+
+    it('does not rewrite an active member who already holds the invited roles', async () => {
+      (mockDb.user.findFirst as jest.Mock).mockResolvedValue({
+        id: 'user_existing',
+        email: 'a@example.com',
+      });
+      (mockDb.member.findFirst as jest.Mock).mockResolvedValue({
+        id: 'member_existing',
+        role: 'admin,employee',
+        deactivated: false,
+        isActive: true,
+      });
+
+      const results = await service.inviteMembers({
+        ...baseParams,
+        invites: [{ email: 'a@example.com', roles: ['admin'] }],
+      });
+
+      expect(results[0].success).toBe(true);
+      expect(mockDb.member.update).not.toHaveBeenCalled();
+      expect(mockDb.invitation.create).not.toHaveBeenCalled();
+    });
+
+    it('unions roles for an active member re-added via the employee path', async () => {
+      (mockDb.organization.findUnique as jest.Mock).mockResolvedValue({
+        name: 'Test Org',
+      });
+      (mockDb.user.findFirst as jest.Mock).mockResolvedValue({
+        id: 'user_existing',
+        email: 'c@example.com',
+      });
+      (mockDb.member.findFirst as jest.Mock).mockResolvedValue({
+        id: 'member_existing',
+        role: 'contractor',
+        deactivated: false,
+        isActive: true,
+      });
+      (mockDb.member.update as jest.Mock).mockResolvedValue({
+        id: 'member_existing',
+      });
+
+      const results = await service.inviteMembers({
+        ...baseParams,
+        invites: [{ email: 'c@example.com', roles: ['employee'] }],
+      });
+
+      expect(results[0].success).toBe(true);
+      expect(mockDb.member.update).toHaveBeenCalledWith(
+        expect.objectContaining({
+          where: { id: 'member_existing' },
+          data: { role: 'contractor,employee' },
+        }),
+      );
+    });
+
     it('should handle multiple invites', async () => {
       (mockDb.organization.findUnique as jest.Mock).mockResolvedValue({
         name: 'Test Org',

apps/api/src/people/people-invite.service.ts

@@ -193,7 +193,17 @@ export class PeopleInviteService {
           data: { deactivated: false, isActive: true, role: roleString },
         });
       } else {
-        member = existingMember;
+        // Active member re-added: union the new roles into their existing roles
+        // so we never strip a role they already have, and so adding a role
+        // actually takes effect instead of silently no-op'ing.
+        const mergedRole = this.mergeRoleString(existingMember.role, roles);
+        member =
+          mergedRole === this.normalizeRoleString(existingMember.role)
+            ? existingMember
+            : await db.member.update({
+                where: { id: existingMember.id },
+                data: { role: mergedRole },
+              });
       }
     } else {
       member = await db.member.create({
@@ -276,14 +286,18 @@ export class PeopleInviteService {
           return;
         }
 
-        await this.sendInvitationEmailToExistingMember({
-          email,
-          roles,
-          organizationId,
-          inviterId: currentUserId,
-          sendPortalEmail,
-          sendAppEmail,
-        });
+        // Already an active member: an invitation/accept round-trip can't grant
+        // new roles to someone who is already in the org (and historically left
+        // their role unchanged, so promoting an employee to admin silently
+        // failed and the user hit "Access Denied"). Upgrade their role in place
+        // by unioning the new roles into their existing roles.
+        const mergedRole = this.mergeRoleString(existingMember.role, roles);
+        if (mergedRole !== this.normalizeRoleString(existingMember.role)) {
+          await db.member.update({
+            where: { id: existingMember.id },
+            data: { role: mergedRole },
+          });
+        }
         return;
       }
     }
@@ -319,51 +333,36 @@ export class PeopleInviteService {
     });
   }
 
-  private async sendInvitationEmailToExistingMember(params: {
-    email: string;
-    roles: string[];
-    organizationId: string;
-    inviterId: string;
-    sendPortalEmail?: boolean;
-    sendAppEmail?: boolean;
-  }): Promise<void> {
-    const {
-      email,
-      roles,
-      organizationId,
-      inviterId,
-      sendPortalEmail,
-      sendAppEmail,
-    } = params;
-
-    const organization = await db.organization.findUnique({
-      where: { id: organizationId },
-      select: { name: true },
-    });
-
-    if (!organization) {
-      throw new BadRequestException('Organization not found.');
-    }
-
-    const invitation = await db.invitation.create({
-      data: {
-        email: email.toLowerCase(),
-        organizationId,
-        role: roles.length === 1 ? roles[0] : roles.join(','),
-        status: 'pending',
-        expiresAt: new Date(Date.now() + 7 * 24 * 60 * 60 * 1000),
-        inviterId,
-      },
-    });
+  /** Sort + de-dupe a comma-separated role string into a canonical form. */
+  private normalizeRoleString(role: string | null | undefined): string {
+    return [
+      ...new Set(
+        (role ?? '')
+          .split(',')
+          .map((r) => r.trim())
+          .filter(Boolean),
+      ),
+    ]
+      .sort()
+      .join(',');
+  }
 
-    await this.sendInviteEmails({
-      email: email.toLowerCase(),
-      organizationName: organization.name,
-      sendPortalEmail,
-      sendAppEmail,
-      portalLink: this.buildPortalUrl(organizationId),
-      appLink: this.buildInviteLink(invitation.id),
-    });
+  /** Union new roles into an existing comma-separated role string. */
+  private mergeRoleString(
+    existingRole: string | null | undefined,
+    addedRoles: string[],
+  ): string {
+    return [
+      ...new Set([
+        ...(existingRole ?? '')
+          .split(',')
+          .map((r) => r.trim())
+          .filter(Boolean),
+        ...addedRoles.map((r) => r.trim()).filter(Boolean),
+      ]),
+    ]
+      .sort()
+      .join(',');
   }
 
   async resendPortalInvite(params: {

apps/app/src/actions/organization/accept-invitation.ts

@@ -1,6 +1,7 @@
 'use server';
 
 import { createTrainingVideoEntries } from '@/lib/db/employee';
+import { mergeRoleStrings, normalizeRoleString } from '@/lib/permissions';
 import { auth } from '@/utils/auth';
 import { db } from '@db/server';
 import { revalidatePath, revalidateTag } from 'next/cache';
@@ -74,8 +75,14 @@ export const completeInvitation = authActionClientWithoutOrg
         });
 
         if (existingMembership) {
-          // Reactivate member before setting active org, since better-auth
-          // validates membership status when setting the active organization.
+          // Ensure the member ends up with at least the invited roles.
+          // Reactivate first since better-auth validates membership status when
+          // setting the active organization.
+          // - Deactivated members are reactivated with the invited roles.
+          // - Active members have the invited roles UNIONed into their existing
+          //   roles. Previously this branch left an active member's role
+          //   untouched, so promoting e.g. an employee to admin via an invite
+          //   never granted app access and the user hit "Access Denied".
           if (existingMembership.deactivated) {
             await db.member.update({
               where: { id: existingMembership.id },
@@ -84,6 +91,17 @@ export const completeInvitation = authActionClientWithoutOrg
                 role: invitation.role,
               },
             });
+          } else {
+            const mergedRole = mergeRoleStrings(
+              existingMembership.role,
+              invitation.role,
+            );
+            if (mergedRole !== normalizeRoleString(existingMembership.role)) {
+              await db.member.update({
+                where: { id: existingMembership.id },
+                data: { role: mergedRole },
+              });
+            }
           }
 
           if (ctx.session.activeOrganizationId !== invitation.organizationId) {

apps/app/src/lib/permissions.test.ts

@@ -6,6 +6,8 @@ import {
   canAccessRoute,
   getDefaultRoute,
   hasPermission,
+  mergeRoleStrings,
+  normalizeRoleString,
   type UserPermissions,
 } from './permissions';
 
@@ -48,6 +50,47 @@ describe('canAccessApp', () => {
   });
 });
 
+describe('normalizeRoleString', () => {
+  it('sorts and de-dupes a role string', () => {
+    expect(normalizeRoleString('employee,admin,employee')).toBe('admin,employee');
+  });
+
+  it('trims whitespace and ignores empty segments', () => {
+    expect(normalizeRoleString(' admin , , employee ')).toBe('admin,employee');
+  });
+
+  it('returns empty string for null/undefined/empty', () => {
+    expect(normalizeRoleString(null)).toBe('');
+    expect(normalizeRoleString(undefined)).toBe('');
+    expect(normalizeRoleString('')).toBe('');
+  });
+});
+
+describe('mergeRoleStrings', () => {
+  it('unions invited roles into existing roles (employee promoted to admin)', () => {
+    expect(mergeRoleStrings('employee', 'admin')).toBe('admin,employee');
+  });
+
+  it('preserves existing roles not present in the invite (never strips a role)', () => {
+    // An owner re-invited as employee keeps owner.
+    expect(mergeRoleStrings('owner', 'employee')).toBe('employee,owner');
+  });
+
+  it('is a no-op (set-equal) when the member already holds the invited roles', () => {
+    const existing = 'admin,employee';
+    expect(mergeRoleStrings(existing, 'admin')).toBe(normalizeRoleString(existing));
+    expect(mergeRoleStrings(existing, 'employee,admin')).toBe(normalizeRoleString(existing));
+  });
+
+  it('handles multi-role invite strings and de-dupes', () => {
+    expect(mergeRoleStrings('employee', 'admin,employee')).toBe('admin,employee');
+  });
+
+  it('handles a missing existing role (acts like the invited roles)', () => {
+    expect(mergeRoleStrings(null, 'admin,employee')).toBe('admin,employee');
+  });
+});
+
 describe('canAccessRoute', () => {
   it('allows access to penetration-tests with pentest:read', () => {
     const permissions: UserPermissions = { pentest: ['read'] };

apps/app/src/lib/permissions.ts

@@ -21,6 +21,25 @@ export function parseRolesString(rolesStr: string | null | undefined): string[]
     .filter((r) => r.length > 0);
 }
 
+/** Sort + de-dupe a comma-separated role string into a canonical form. */
+export function normalizeRoleString(rolesStr: string | null | undefined): string {
+  return [...new Set(parseRolesString(rolesStr))].sort().join(',');
+}
+
+/**
+ * Union the invited roles into an existing comma-separated role string.
+ * Used when an existing member is (re-)invited: we add the new roles rather
+ * than replacing, so a member is never stripped of a role they already hold.
+ */
+export function mergeRoleStrings(
+  existingRoles: string | null | undefined,
+  invitedRoles: string | null | undefined,
+): string {
+  return [...new Set([...parseRolesString(existingRoles), ...parseRolesString(invitedRoles)])]
+    .sort()
+    .join(',');
+}
+
 /**
  * Check if a role name is a built-in role.
  */