fix(security): sanitize the user document in org-switch and /me responses

ERRORS.md

@@ -30,3 +30,4 @@ Use this file as a compact memory of recurring AI mistakes.
 - [2026-05-31] billing/stripe: reading `price.metadata.planId` in `customer.subscription.updated` webhook handler -> field is EMPTY in real Stripe webhook payloads (planId lives on the Product, not the Price); use a `priceId → plan` map built at boot from `config.stripe.prices` instead; see pierreb-devkit/Node#3742
 - [2026-06-04] repository: top-level `const Foo = mongoose.model('Foo')` in a repository file -> this is evaluated at import time; safe in an HTTP server (loadModels() runs first) but silently crashes standalone scripts (crons, migrations) with `MissingSchemaError` when import order differs; tests miss it because jest mocks intercept the module entirely; fix = lazy getter `const Foo = () => mongoose.model('Foo')` (call sites: `Foo().find(...)`) or dynamic import after `loadModels()` in the entrypoint; see pierreb-devkit/Node#3789
 - [2026-06-15] deps/audit: leaving `npm audit` advisories unaddressed on the assumption they need a major bump -> run `npm audit fix` (never `--force`) first; the runtime-tree DoS/ReDoS items (`qs`, `path-to-regexp`, `brace-expansion`) all fixed via in-range bumps, no residual. These are DoS-class but NOT attacker-reachable in this stack: Express route patterns are static (no user-controlled `path-to-regexp` input) and `qs`/`brace-expansion` only parse server-side query strings under fixed code paths — still bump them to keep the tree clean and avoid scanner noise.
+- [2026-07-16] security: `users.repository.js findByIdAndUpdatePopulated()` does `.populate()` with no `.select()`, so `organizations.controller.js switchOrganization` serialized the raw doc (password hash + OAuth tokens + reset/verification tokens) straight to the client; `users.account.controller.js me()` separately forwarded `providerData` (OAuth tokens) verbatim -> any endpoint returning a Mongoose user doc must go through `UserService.removeSensitive()` (whitelist, `modules/users/utils/sanitizeUser.js`) at the response boundary, never serialize `req.user`/a populated doc directly; a local-signup fixture's `providerData` defaults to `{}` so a naive falsy check won't catch this — seed a fake OAuth token in tests to prove the leak is actually closed; see pierreb-devkit/Node#3963

modules/organizations/controllers/organizations.controller.js

@@ -12,6 +12,7 @@ import serializeAbilities from '../../../lib/helpers/abilities.js';
 import OrganizationsService from '../services/organizations.crud.service.js';
 import MembershipService from '../services/organizations.membership.service.js';
 import AnalyticsService from '../../../lib/services/analytics.js';
+import UserService from '../../users/services/users.service.js';
 
 const tokenCookieOptions = {
   httpOnly: true,
@@ -257,7 +258,8 @@ const switchOrganization = async (req, res) => {
       expiresIn: config.jwt.expiresIn,
     });
 
-    // Build abilities for the new org context
+    // Build abilities for the new org context — uses the raw doc (needs
+    // user.roles/_id only), sanitization happens below, at the response boundary.
     const ability = await policy.defineAbilityFor(updatedUser, membership);
     const abilities = serializeAbilities(ability);
 
@@ -268,7 +270,10 @@ const switchOrganization = async (req, res) => {
         type: 'success',
         message: 'organization switched',
         data: {
-          user: updatedUser,
+          // updatedUser comes straight off an unselected findByIdAndUpdate().populate()
+          // (users.repository.js) and carries password/providerData/reset+verification
+          // tokens — sanitize before serializing (#3963).
+          user: UserService.removeSensitive(updatedUser),
           abilities,
           tokenExpiresIn: Date.now() + config.jwt.expiresIn * 1000,
         },

modules/organizations/tests/organizations.controller.unit.tests.js

@@ -4,12 +4,14 @@
 import { jest, describe, test, expect, beforeEach } from '@jest/globals';
 
 const mockCrudRemove = jest.fn();
+const mockCrudSwitchOrganization = jest.fn();
 const mockListByUser = jest.fn();
 const mockLeave = jest.fn();
 
 jest.unstable_mockModule('../services/organizations.crud.service.js', () => ({
   default: {
     remove: mockCrudRemove,
+    switchOrganization: mockCrudSwitchOrganization,
   },
 }));
 
@@ -26,6 +28,19 @@ jest.unstable_mockModule('../../../lib/services/analytics.js', () => ({
   },
 }));
 
+// Mock the users service boundary with the REAL sanitizeUser.removeSensitive
+// (pure — lodash + config only, no mongoose) rather than a stub, so this test
+// exercises actual whitelist-based stripping (#3963) instead of merely
+// asserting a mock got called. Mocking the module at all is required because
+// users.service.js -> users.repository.js does `mongoose.model('User')` at
+// import time, which throws outside a bootstrapped app (see ERRORS.md 2026-06-04).
+const { removeSensitive } = await import('../../users/utils/sanitizeUser.js');
+jest.unstable_mockModule('../../users/services/users.service.js', () => ({
+  default: {
+    removeSensitive,
+  },
+}));
+
 const { default: organizationsController } = await import('../controllers/organizations.controller.js');
 
 /**
@@ -54,6 +69,7 @@ describe('Organizations controller unit tests:', () => {
     const res = {};
     res.status = jest.fn().mockReturnValue(res);
     res.json = jest.fn().mockReturnValue(res);
+    res.cookie = jest.fn().mockReturnValue(res);
     return res;
   }
 
@@ -120,6 +136,92 @@ describe('Organizations controller unit tests:', () => {
     });
   });
 
+  describe('switchOrganization', () => {
+    /**
+     * @desc Build a fake Mongoose-like updated-user document, the shape
+     * `UserService.findByIdAndUpdatePopulated` returns with no `.select()` —
+     * every sensitive field a real doc could carry, plus the fields the
+     * response legitimately needs.
+     * @returns {Object} fake mongoose document with a `toJSON` method
+     */
+    function fakeUpdatedUserDoc() {
+      const plain = {
+        _id: 'u1',
+        id: 'u1',
+        email: 'switcher@test.com',
+        roles: ['user'],
+        firstName: 'Switch',
+        lastName: 'User',
+        currentOrganization: 'org2',
+        password: '$2b$10$leakedHashShouldNeverReachClient',
+        providerData: { accessToken: 'leaked-access-token', refreshToken: 'leaked-refresh-token' },
+        additionalProvidersData: { google: { accessToken: 'leaked-additional-token' } },
+        resetPasswordToken: 'leaked-reset-token',
+        resetPasswordExpires: new Date(),
+        emailVerificationToken: 'leaked-verification-token',
+        emailVerificationExpires: new Date(),
+        failedLoginAttempts: 3,
+        lockUntil: null,
+      };
+      return { ...plain, toJSON: () => plain };
+    }
+
+    test('sanitizes the response user — strips password/providerData/reset tokens, keeps legit fields', async () => {
+      const updatedUser = fakeUpdatedUserDoc();
+      mockCrudSwitchOrganization.mockResolvedValue({
+        user: updatedUser,
+        membership: { role: 'owner', organizationId: 'org2' },
+      });
+
+      const req = mockReq({
+        params: { organizationId: 'org2' },
+        organization: { _id: 'org2', id: 'org2' },
+      });
+      const res = mockRes();
+
+      await organizationsController.switchOrganization(req, res);
+
+      expect(mockCrudSwitchOrganization).toHaveBeenCalledWith(req.user, 'org2');
+      expect(res.status).toHaveBeenCalledWith(200);
+      const [payload] = res.json.mock.calls[0];
+      const responseUser = payload.data.user;
+
+      // Sensitive fields — must be ABSENT
+      expect(responseUser.password).toBeUndefined();
+      expect(responseUser.providerData).toBeUndefined();
+      expect(responseUser.additionalProvidersData).toBeUndefined();
+      expect(responseUser.resetPasswordToken).toBeUndefined();
+      expect(responseUser.resetPasswordExpires).toBeUndefined();
+      expect(responseUser.emailVerificationToken).toBeUndefined();
+      expect(responseUser.emailVerificationExpires).toBeUndefined();
+      expect(responseUser.failedLoginAttempts).toBeUndefined();
+      expect(responseUser.lockUntil).toBeUndefined();
+
+      // Legit fields — must be PRESENT
+      expect(responseUser.id).toBe('u1');
+      expect(responseUser.email).toBe('switcher@test.com');
+      expect(responseUser.roles).toEqual(['user']);
+      expect(responseUser.currentOrganization).toBe('org2');
+      expect(responseUser.firstName).toBe('Switch');
+    });
+
+    test('returns 403 when the user is not a member of the target organization', async () => {
+      const err = new Error('User is not a member of this organization');
+      err.code = 'FORBIDDEN';
+      mockCrudSwitchOrganization.mockRejectedValue(err);
+
+      const req = mockReq({
+        params: { organizationId: 'org2' },
+        organization: { _id: 'org2', id: 'org2' },
+      });
+      const res = mockRes();
+
+      await organizationsController.switchOrganization(req, res);
+
+      expect(res.status).toHaveBeenCalledWith(403);
+    });
+  });
+
   describe('leave', () => {
     test('should call MembershipService.leave with user and org ids and return success', async () => {
       mockLeave.mockResolvedValue({ success: true });

modules/organizations/tests/organizations.integration.tests.js

@@ -206,6 +206,79 @@ describe('Organizations integration tests:', () => {
     });
   });
 
+  describe('POST /api/organizations/:organizationId/switch (#3963 — sanitized response)', () => {
+    let switchAgent;
+    let MembershipService;
+    let user;
+    let org2;
+
+    beforeAll(async () => {
+      config.organizations = { enabled: true, autoCreate: true, domainMatching: false };
+      switchAgent = request.agent((await bootstrap()).app);
+      MembershipService = (await import(path.resolve('./modules/organizations/services/organizations.membership.service.js'))).default;
+
+      const signupRes = await switchAgent
+        .post('/api/auth/signup')
+        .send({
+          firstName: 'Switch',
+          lastName: 'User',
+          email: 'switch-3963@test.com',
+          password: 'W@os.jsI$Aw3$0m3',
+          provider: 'local',
+        })
+        .expect(200);
+      user = signupRes.body.user;
+
+      // A second organization the user OWNS (owner role needed — `switch` maps to
+      // the CASL `create` action on Organization, which only the owner's `manage`
+      // wildcard grants), so the switch has somewhere valid to go.
+      org2 = await OrganizationsRepository.create({ name: 'Switch Target Org', slug: `switch-target-3963-${Date.now()}` });
+      await MembershipService.create({ userId: user._id || user.id, organizationId: org2._id, role: 'owner' });
+
+      // Simulate what a real production user document carries — a linked OAuth
+      // account and a stray reset token — so the assertions below prove the
+      // response strips them rather than merely "happening" not to have them.
+      await UserService.updateById(user.id, {
+        providerData: { accessToken: 'leaked-switch-access-token', refreshToken: 'leaked-switch-refresh-token' },
+        resetPasswordToken: 'leaked-switch-reset-token',
+        resetPasswordExpires: new Date(Date.now() + 3600000),
+      });
+    });
+
+    test('response user must not leak password/providerData/reset tokens, and must keep legit fields', async () => {
+      const result = await switchAgent.post(`/api/organizations/${org2._id}/switch`).expect(200);
+
+      expect(result.body.type).toBe('success');
+      expect(result.body.message).toBe('organization switched');
+      const responseUser = result.body.data.user;
+      expect(responseUser).toBeInstanceOf(Object);
+
+      // Sensitive fields — must be ABSENT
+      expect(responseUser.password).toBeUndefined();
+      expect(responseUser.providerData).toBeUndefined();
+      expect(responseUser.additionalProvidersData).toBeUndefined();
+      expect(responseUser.resetPasswordToken).toBeUndefined();
+      expect(responseUser.resetPasswordExpires).toBeUndefined();
+      expect(responseUser.emailVerificationToken).toBeUndefined();
+      expect(responseUser.emailVerificationExpires).toBeUndefined();
+      expect(responseUser.failedLoginAttempts).toBeUndefined();
+      expect(responseUser.lockUntil).toBeUndefined();
+      expect(JSON.stringify(result.body)).not.toContain('leaked-switch-access-token');
+      expect(JSON.stringify(result.body)).not.toContain('leaked-switch-refresh-token');
+      expect(JSON.stringify(result.body)).not.toContain('leaked-switch-reset-token');
+
+      // Legit fields — must be PRESENT and correct
+      expect(responseUser.id).toBe(String(user.id));
+      expect(responseUser.email).toBe(user.email);
+      expect(responseUser.roles).toBeInstanceOf(Array);
+      expect(String(responseUser.currentOrganization._id || responseUser.currentOrganization)).toBe(String(org2._id));
+    });
+
+    afterAll(async () => {
+      await cleanupUser(user);
+    });
+  });
+
   // Mongoose disconnect
   afterAll(async () => {
     config.organizations = { ...originalOrganizations };

modules/users/controllers/users.account.controller.js

@@ -59,6 +59,8 @@ const remove = async (req, res) => {
 const me = (req, res) => {
   // Sanitize the user - short term solution. Copied from core.controller.js
   // TODO create proper passport mock: See https://gist.github.com/mweibel/5219403
+  // providerData (OAuth access+refresh tokens, see auth/strategies/local/{google,apple}.js)
+  // is intentionally NOT whitelisted here — never forward it to the client (#3963).
   let user = null;
   if (req.user) {
     user = {
@@ -69,7 +71,6 @@ const me = (req, res) => {
       email: req.user.email,
       lastName: req.user.lastName,
       firstName: req.user.firstName,
-      providerData: req.user.providerData,
       // others
       complementary: req.user.complementary,
     };

modules/users/tests/user.account.integration.tests.js

@@ -198,6 +198,32 @@ describe('User integration tests:', () => {
       }
     });
 
+    // #3963: providerData holds OAuth access+refresh tokens (see
+    // auth/strategies/local/{google,apple}.js). A local-signup user's providerData
+    // is an empty object, which would trivially pass a naive "is it falsy" check —
+    // simulate a linked OAuth account (real tokens) to prove the leak is actually
+    // closed, not just absent because this fixture never had anything to leak.
+    test('should NOT include providerData (OAuth tokens) in /me response', async () => {
+      try {
+        await UserService.updateById(user.id, {
+          providerData: { accessToken: 'leaked-me-access-token', refreshToken: 'leaked-me-refresh-token' },
+        });
+
+        const result = await agent.get('/api/users/me').expect(200);
+        expect(result.body.data.providerData).toBeUndefined();
+        expect(JSON.stringify(result.body.data)).not.toContain('leaked-me-access-token');
+        expect(JSON.stringify(result.body.data)).not.toContain('leaked-me-refresh-token');
+
+        // legit fields still present
+        expect(result.body.data.email).toBe(user.email);
+        expect(result.body.data.id).toBe(String(user.id));
+        expect(result.body.data.roles).toBeInstanceOf(Array);
+      } catch (err) {
+        console.log(err);
+        expect(err).toBeFalsy();
+      }
+    });
+
     test('should include terms in user details after signing them', async () => {
       // Sign terms first
       try {