fix(users/organizations/auth): prevent 500 on signin when org deleted without cascade (#3709)

modules/auth/tests/auth.e2e.tests.js

@@ -237,6 +237,42 @@ describe('Auth E2E tests:', () => {
         expect(err).toBeFalsy();
       }
     });
+
+    test('should return 200 on signin even when autoSetCurrentOrganization would encounter a data-integrity anomaly', async () => {
+      config.organizations = { enabled: true, autoCreate: true, domainMatching: false };
+
+      // Signup user, then directly corrupt their currentOrganization to a non-existent ObjectId
+      // to simulate a pre-existing dangling ref in prod
+      try {
+        const signupRes = await agent
+          .post('/api/auth/signup')
+          .send({
+            firstName: 'Danglingref',
+            lastName: 'User',
+            email: 'e2e-dangling-ref-3709@test.com',
+            password: 'W@os.jsI$Aw3$0m3',
+            provider: 'local',
+          })
+          .expect(200);
+
+        user = signupRes.body.user;
+        // Directly write a bogus ObjectId as currentOrganization (simulates pre-existing corruption)
+        const mongoose = (await import('mongoose')).default;
+        const User = mongoose.model('User');
+        await User.updateOne({ _id: user.id }, { currentOrganization: new mongoose.Types.ObjectId() });
+
+        // Signin must NOT 500
+        const signinRes = await agent
+          .post('/api/auth/signin')
+          .send({ email: 'e2e-dangling-ref-3709@test.com', password: 'W@os.jsI$Aw3$0m3' })
+          .expect(200);
+
+        expect(signinRes.body.type).toBe('success');
+      } catch (err) {
+        console.log(err);
+        expect(err).toBeFalsy();
+      }
+    });
   });
 
   // Mongoose disconnect

modules/organizations/services/organizations.crud.service.js

@@ -253,13 +253,16 @@ const autoSetCurrentOrganization = async (user) => {
       organizationId: user.currentOrganization._id || user.currentOrganization,
       status: MEMBERSHIP_STATUSES.ACTIVE,
     });
-    if (stillActive) return user;
-    // Membership gone — clear stale reference and fall through to find another
+    // Guard: populated organizationId may be null when the org was hard-deleted
+    if (stillActive && stillActive.organizationId != null) return user;
+    // Membership gone or org deleted — clear stale reference and fall through to find another
     user.currentOrganization = null;
   }
   const memberships = await MembershipRepository.list({ userId: user._id || user.id, status: MEMBERSHIP_STATUSES.ACTIVE });
-  if (memberships.length > 0) {
-    const orgId = memberships[0].organizationId._id || memberships[0].organizationId;
+  // Filter out memberships whose org was deleted (Mongoose populate sets organizationId to null)
+  const liveMemberships = memberships.filter((m) => m.organizationId != null);
+  if (liveMemberships.length > 0) {
+    const orgId = liveMemberships[0].organizationId._id || liveMemberships[0].organizationId;
     await UserService.updateById(user._id || user.id, { currentOrganization: orgId });
     user.currentOrganization = orgId;
   } else {

modules/organizations/tests/organizations.crud.dangling-org.unit.tests.js

@@ -0,0 +1,115 @@
+/**
+ * Unit tests — autoSetCurrentOrganization must not crash when membership.organizationId
+ * is null after populate (deleted org, dangling ref). Issue #3709.
+ */
+import { jest, describe, test, expect } from '@jest/globals';
+
+const mockMembershipFindOne = jest.fn();
+const mockMembershipList = jest.fn();
+const mockUpdateById = jest.fn();
+const mockOrgRemove = jest.fn();
+
+jest.unstable_mockModule('../../../lib/services/logger.js', () => ({
+  default: { error: jest.fn(), warn: jest.fn(), info: jest.fn() },
+}));
+
+jest.unstable_mockModule('../repositories/organizations.repository.js', () => ({
+  default: {
+    create: jest.fn(),
+    findOne: jest.fn().mockResolvedValue(null),
+    remove: mockOrgRemove,
+    list: jest.fn().mockResolvedValue([]),
+    update: jest.fn(),
+    get: jest.fn(),
+    exists: jest.fn().mockResolvedValue(false),
+    updateById: jest.fn(),
+  },
+}));
+
+jest.unstable_mockModule('../repositories/organizations.membership.repository.js', () => ({
+  default: {
+    create: jest.fn(),
+    deleteMany: jest.fn(),
+    list: mockMembershipList,
+    findOne: mockMembershipFindOne,
+    update: jest.fn(),
+    remove: jest.fn(),
+    count: jest.fn(),
+  },
+}));
+
+jest.unstable_mockModule('../../users/services/users.service.js', () => ({
+  default: {
+    updateById: mockUpdateById,
+    findWithFilter: jest.fn().mockResolvedValue([]),
+    getBrut: jest.fn(),
+  },
+}));
+
+jest.unstable_mockModule('../../../lib/helpers/emailVerification.js', () => ({
+  assertEmailVerified: jest.fn(),
+}));
+
+jest.unstable_mockModule('../../../config/index.js', () => ({
+  default: { organizations: { enabled: true } },
+}));
+
+const { default: OrgCrudService } = await import('../services/organizations.crud.service.js');
+
+describe('autoSetCurrentOrganization — dangling org ref (#3709):', () => {
+  const user = { _id: 'uid1', id: 'uid1', currentOrganization: null };
+
+  beforeEach(() => {
+    jest.clearAllMocks();
+    mockUpdateById.mockResolvedValue({});
+  });
+
+  test('should not throw when membership.organizationId is null after populate', async () => {
+    // Simulate: user has an active membership but the org is deleted (populate yields null)
+    mockMembershipList.mockResolvedValue([
+      { _id: 'm1', organizationId: null, status: 'active' },  // null = deleted org
+    ]);
+
+    // Must not throw; must set currentOrganization to null (no live org)
+    const result = await OrgCrudService.autoSetCurrentOrganization({ ...user });
+    expect(result.currentOrganization).toBeNull();
+    expect(mockUpdateById).toHaveBeenCalledWith('uid1', { currentOrganization: null });
+  });
+
+  test('should skip null-org memberships and pick first live org when mixed', async () => {
+    // Two memberships: first has deleted org (null populate), second has live org
+    mockMembershipList.mockResolvedValue([
+      { _id: 'm1', organizationId: null, status: 'active' },          // deleted org
+      { _id: 'm2', organizationId: { _id: 'org2' }, status: 'active' }, // live org
+    ]);
+
+    const result = await OrgCrudService.autoSetCurrentOrganization({ ...user });
+    expect(result.currentOrganization).toBe('org2');
+    expect(mockUpdateById).toHaveBeenCalledWith('uid1', { currentOrganization: 'org2' });
+  });
+
+  test('should return user unchanged when currentOrganization is set and membership is live with a live org', async () => {
+    const userWithOrg = { _id: 'uid1', id: 'uid1', currentOrganization: { _id: 'org1' } };
+    mockMembershipFindOne.mockResolvedValue({ _id: 'm1', organizationId: { _id: 'org1' } });
+
+    const result = await OrgCrudService.autoSetCurrentOrganization(userWithOrg);
+    // Early return — no updateById called
+    expect(mockUpdateById).not.toHaveBeenCalled();
+    expect(result.currentOrganization).toEqual({ _id: 'org1' });
+  });
+
+  test('should fall through and clear currentOrganization when membership exists but org is null-populated', async () => {
+    // Membership still exists (findOne returns it) but org is deleted (organizationId = null)
+    const userWithOrg = { _id: 'uid1', id: 'uid1', currentOrganization: { _id: 'org1' } };
+    // findOne called in early branch — membership exists (stillActive truthy) but org deleted
+    mockMembershipFindOne.mockResolvedValue({ _id: 'm1', organizationId: null });
+    // list called in fallback branch — also returns the same dangling membership
+    mockMembershipList.mockResolvedValue([
+      { _id: 'm1', organizationId: null, status: 'active' },
+    ]);
+
+    const result = await OrgCrudService.autoSetCurrentOrganization(userWithOrg);
+    expect(result.currentOrganization).toBeNull();
+    expect(mockUpdateById).toHaveBeenCalledWith('uid1', { currentOrganization: null });
+  });
+});

modules/users/services/users.service.js

@@ -7,7 +7,8 @@ import config from '../../../config/index.js';
 import AuthService from '../../auth/services/auth.service.js';
 import UserRepository from '../repositories/users.repository.js';
 import MembershipService from '../../organizations/services/organizations.membership.service.js';
-import OrganizationsCrudService from '../../organizations/services/organizations.crud.service.js';
+import OrganizationsRepository from '../../organizations/repositories/organizations.repository.js';
+import MembershipRepository from '../../organizations/repositories/organizations.membership.repository.js';
 import { MEMBERSHIP_ROLES, MEMBERSHIP_STATUSES } from '../../organizations/lib/constants.js';
 import { removeSensitive } from '../utils/sanitizeUser.js';
 
@@ -119,12 +120,21 @@ const remove = async (user) => {
       // Check if this user is the only owner of the org
       const ownerCount = await MembershipService.count({ organizationId: orgId, role: MEMBERSHIP_ROLES.OWNER, status: MEMBERSHIP_STATUSES.ACTIVE });
       if (ownerCount <= 1) {
-        // Sole owner — delete the entire org and its memberships
-        // Clear currentOrganization for affected users
-        await UserRepository.updateMany({ currentOrganization: orgId }, { currentOrganization: null });
+        // Sole owner — delete org and cascade: repair co-member currentOrganization
+        // Step 1: Collect co-members whose currentOrganization points to this org
+        const affectedUsers = await UserRepository.findWithFilter({ currentOrganization: orgId }, '_id');
+        // Step 2: Delete all memberships for this org (including this user's and all co-members')
         await MembershipService.deleteMany({ organizationId: orgId });
-        await OrganizationsCrudService.removeById(orgId);
-        continue; // membership already deleted above
+        // Step 3: For each affected co-member, switch to their next available org or set null
+        await Promise.all(affectedUsers.map(async (u) => {
+          const remaining = await MembershipRepository.list({ userId: u._id, status: MEMBERSHIP_STATUSES.ACTIVE });
+          const liveMemberships = remaining.filter((m) => m.organizationId != null);
+          const nextOrg = liveMemberships.length > 0 ? (liveMemberships[0].organizationId._id || liveMemberships[0].organizationId) : null;
+          await UserRepository.updateById(u._id, { currentOrganization: nextOrg });
+        }));
+        // Step 4: Delete the org (bare remove — org-scoped tasks are intentionally not deleted here)
+        await OrganizationsRepository.remove({ _id: orgId });
+        continue; // memberships for this org already cleaned up
       }
     }
     // Delete this user's membership

modules/users/tests/users.remove.cascade.e2e.tests.js

@@ -0,0 +1,174 @@
+/**
+ * E2E test: user deletion cascades org cleanup so co-members can still sign in.
+ * Repro from issue #3709: User A (sole owner) deleted → Org X removed.
+ * User B (active member of Org X, currentOrganization=X) must be able to sign in
+ * without 500, with currentOrganization cleared to null.
+ */
+import request from 'supertest';
+import path from 'path';
+import { bootstrap } from '../../../lib/app.js';
+import mongooseService from '../../../lib/services/mongoose.js';
+import config from '../../../config/index.js';
+
+describe('users.service.remove cascade (#3709):', () => {
+  let UserService;
+  let OrganizationsRepository;
+  let MembershipRepository;
+  let agent;
+
+  const originalOrganizations = { ...config.organizations };
+
+  beforeAll(async () => {
+    try {
+      const init = await bootstrap();
+      UserService = (await import(path.resolve('./modules/users/services/users.service.js'))).default;
+      OrganizationsRepository = (await import(path.resolve('./modules/organizations/repositories/organizations.repository.js'))).default;
+      MembershipRepository = (await import(path.resolve('./modules/organizations/repositories/organizations.membership.repository.js'))).default;
+      agent = request.agent(init.app);
+    } catch (err) {
+      console.log(err);
+      expect(err).toBeFalsy();
+    }
+  });
+
+  afterAll(async () => {
+    config.organizations = { ...originalOrganizations };
+    try {
+      await mongooseService.disconnect();
+    } catch (err) {
+      console.log(err);
+      expect(err).toBeFalsy();
+    }
+  });
+
+  describe('User B can sign in after User A (sole owner) is deleted', () => {
+    let userA;
+    let userB;
+    let orgX;
+    let agentA;
+    let agentB;
+
+    beforeAll(async () => {
+      config.organizations = { enabled: true, autoCreate: true, domainMatching: false };
+      agentA = request.agent(agent.app);
+      agentB = request.agent(agent.app);
+    });
+
+    afterAll(async () => {
+      // Best-effort cleanup
+      try {
+        if (userB) await UserService.remove(userB);
+      } catch (_) { /* cleanup */ }
+      try {
+        if (orgX) {
+          await MembershipRepository.deleteMany({ organizationId: orgX._id });
+          await OrganizationsRepository.deleteMany({ _id: orgX._id });
+        }
+      } catch (_) { /* cleanup */ }
+    });
+
+    test('should not 500 on signin when currentOrganization points to a deleted org (issue #3709 repro)', async () => {
+      // Step 1: User A signs up — auto-creates Org X
+      try {
+        const resA = await agentA
+          .post('/api/auth/signup')
+          .send({
+            firstName: 'CascadeA',
+            lastName: 'User',
+            email: 'cascade-a-3709@test.com',
+            password: 'W@os.jsI$Aw3$0m3',
+            provider: 'local',
+          })
+          .expect(200);
+        userA = resA.body.user;
+        orgX = resA.body.organization;
+        expect(orgX).toBeDefined();
+        expect(orgX).not.toBeNull();
+      } catch (err) {
+        console.log(err);
+        expect(err).toBeFalsy();
+      }
+
+      // Step 2: User B signs up separately
+      try {
+        const resB = await agentB
+          .post('/api/auth/signup')
+          .send({
+            firstName: 'CascadeB',
+            lastName: 'User',
+            email: 'cascade-b-3709@test.com',
+            password: 'W@os.jsI$Aw3$0m3',
+            provider: 'local',
+          })
+          .expect(200);
+        userB = resB.body.user;
+      } catch (err) {
+        console.log(err);
+        expect(err).toBeFalsy();
+      }
+
+      // Step 3: Directly create an ACTIVE membership for User B on Org X + set currentOrganization
+      // (bypassing invite flow for test speed)
+      try {
+        const MembershipService = (await import(path.resolve('./modules/organizations/services/organizations.membership.service.js'))).default;
+        await MembershipService.create({
+          userId: userB._id || userB.id,
+          organizationId: orgX._id,
+          role: 'member',
+          status: 'active',
+        });
+        // Set User B's currentOrganization to Org X directly
+        await UserService.updateById(userB._id || userB.id, { currentOrganization: orgX._id });
+      } catch (err) {
+        console.log(err);
+        expect(err).toBeFalsy();
+      }
+
+      // Step 4: Delete User A (sole owner of Org X) — this should cascade-delete Org X + all memberships
+      try {
+        const brutUserA = await UserService.getBrut({ id: userA.id });
+        await UserService.remove(brutUserA);
+        userA = null; // Mark as cleaned
+      } catch (err) {
+        console.log(err);
+        expect(err).toBeFalsy();
+      }
+
+      // Step 5: Verify Org X no longer exists
+      try {
+        const org = await OrganizationsRepository.get(orgX._id);
+        expect(org).toBeNull();
+      } catch (err) {
+        console.log(err);
+        expect(err).toBeFalsy();
+      }
+
+      // Step 6: Verify User B's currentOrganization has been cleared (not left dangling)
+      try {
+        const brutUserB = await UserService.getBrut({ id: userB.id });
+        // currentOrganization must be null or undefined — not the deleted org ID
+        const currentOrgId = brutUserB.currentOrganization?._id || brutUserB.currentOrganization;
+        expect(currentOrgId == null || String(currentOrgId) !== String(orgX._id)).toBe(true);
+      } catch (err) {
+        console.log(err);
+        expect(err).toBeFalsy();
+      }
+
+      // Step 7: User B signs in — must NOT 500 (was: "Cannot read properties of null (reading '_id')")
+      try {
+        const signinRes = await agentB
+          .post('/api/auth/signin')
+          .send({ email: 'cascade-b-3709@test.com', password: 'W@os.jsI$Aw3$0m3' })
+          .expect(200); // Must be 200, not 500
+
+        expect(signinRes.body.type).toBe('success');
+        // currentOrganization must NOT be the deleted Org X (could be null or User B's own org)
+        const signedInOrgId = signinRes.body.user.currentOrganization?._id || signinRes.body.user.currentOrganization;
+        expect(signedInOrgId == null || String(signedInOrgId) !== String(orgX._id)).toBe(true);
+      } catch (err) {
+        console.log(err);
+        expect(err).toBeFalsy();
+      }
+    });
+  });
+});