fix(auth,orgs,uploads,users): address CodeRabbit review findings

- Add JSDoc to rateLimiter passthrough and Proxy handler
- Rollback user account if email verification init fails (auth signup)
- Return standard error envelope for 401 invalid credentials
- Rate-limit resend-verification endpoint
- Add JSDoc to organizations route registrar
- Make org creation atomic with rollback in crud service
- Sanitize domain-match orgs before returning from signup flow
- Complete cleanupUser helper JSDoc in org tests
- Scope upload reads/deletes to user's organization via CASL
- Narrow guest CASL access from UserAccount to UserStats
- Validate upsert filter keys on every row in users push()
- Add runValidators to updateById, findByIdAndUpdatePopulated, updateMany
- Update auth tests for new 401 error envelope format

Author: PierreBrisorgueil

lib/middlewares/policy.js

@@ -174,7 +174,7 @@ const USER_ROUTE_SUBJECTS = {
   '/api/users/avatar': 'UserAccount',
   '/api/users/data': 'UserAccount',
   '/api/users/data/mail': 'UserAccount',
-  '/api/users/stats': 'UserAccount',
+  '/api/users/stats': 'UserStats',
   '/api/users': 'UserSelf',
 };
 

lib/middlewares/rateLimiter.js

@@ -7,8 +7,22 @@ import rateLimit from 'express-rate-limit';
 
 import config from '../../config/index.js';
 
+/**
+ * @desc No-op middleware used when a rate-limit profile is not configured.
+ * @param {import('express').Request} req - Express request object
+ * @param {import('express').Response} res - Express response object
+ * @param {import('express').NextFunction} next - Express next function
+ * @returns {void}
+ */
 const passthrough = (req, res, next) => next();
 
+/**
+ * @desc Proxy that lazily creates rate-limit middleware for each named profile.
+ * Returns the configured limiter or a passthrough when the profile is missing.
+ * @param {Object} target - Internal cache of created limiters
+ * @param {string|symbol} name - The rate-limit profile name to look up
+ * @returns {Function} Express middleware (rate limiter or passthrough)
+ */
 const limiters = new Proxy({}, {
   get(target, name) {
     if (name in target) return target[name];

modules/auth/controllers/auth.controller.js

@@ -64,22 +64,27 @@ const signup = async (req, res) => {
     const safeBody = { ...req.body, roles: ['user'] };
     const user = await UserService.create(safeBody);
 
-    // Handle email verification
-    if (isMailerConfigured()) {
-      // Generate verification token and persist it
-      const verificationToken = crypto.randomBytes(20).toString('hex');
-      const brutUser = await UserService.getBrut({ id: user.id });
-      await UserService.update(brutUser, {
-        emailVerificationToken: verificationToken,
-        emailVerificationExpires: Date.now() + 24 * 3600000, // 24 hours
-      }, 'recover');
-      // Send verification email (best-effort, do not block signup)
-      sendVerificationEmail(user, verificationToken).catch(() => {});
-    } else {
-      // No mailer configured — auto-verify so dev/test are not blocked
-      const brutUser = await UserService.getBrut({ id: user.id });
-      await UserService.update(brutUser, { emailVerified: true }, 'recover');
-      user.emailVerified = true;
+    // Handle email verification — rollback user on failure to avoid orphaned accounts
+    try {
+      if (isMailerConfigured()) {
+        // Generate verification token and persist it
+        const verificationToken = crypto.randomBytes(20).toString('hex');
+        const brutUser = await UserService.getBrut({ id: user.id });
+        await UserService.update(brutUser, {
+          emailVerificationToken: verificationToken,
+          emailVerificationExpires: Date.now() + 24 * 3600000, // 24 hours
+        }, 'recover');
+        // Send verification email (best-effort, do not block signup)
+        sendVerificationEmail(user, verificationToken).catch(() => {});
+      } else {
+        // No mailer configured — auto-verify so dev/test are not blocked
+        const brutUser = await UserService.getBrut({ id: user.id });
+        await UserService.update(brutUser, { emailVerified: true }, 'recover');
+        user.emailVerified = true;
+      }
+    } catch (verifyErr) {
+      try { await UserService.remove(user); } catch (_cleanupErr) { /* best-effort */ }
+      throw verifyErr;
     }
 
     // Handle organization provisioning based on config
@@ -144,7 +149,7 @@ const signinAuthenticate = (req, res, next) => {
       return responses.error(res, 500, 'Internal Server Error', errors.getMessage(err))(err);
     }
     if (!user) {
-      return res.status(401).send('Unauthorized');
+      return responses.error(res, 401, 'Unauthorized', info?.message || 'Unauthorized')();
     }
     req.user = user;
     return next();

modules/auth/routes/auth.routes.js

@@ -36,7 +36,11 @@ export default (app) => {
 
   // Email verification
   app.route('/api/auth/verify-email/:token').post(authLimiter, auth.verifyEmail);
-  app.route('/api/auth/resend-verification').post(passport.authenticate('jwt', { session: false }), auth.resendVerification);
+  app.route('/api/auth/resend-verification').post(
+    authLimiter,
+    passport.authenticate('jwt', { session: false }),
+    auth.resendVerification,
+  );
 
   // Jwt reset token
   app.route('/api/auth/token').get(passport.authenticate('jwt', { session: false }), auth.token);

modules/auth/tests/auth.integration.tests.js

@@ -211,7 +211,7 @@ describe('Auth integration tests:', () => {
             password: 'W@os.jsI$Aw3$0m3',
           })
           .expect(401);
-        expect(result.error.text).toBe('Unauthorized');
+        expect(result.body.message).toBe('Unauthorized');
       } catch (err) {
         console.log(err);
         expect(err).toBeFalsy();
@@ -249,7 +249,7 @@ describe('Auth integration tests:', () => {
             password: 'WrongPassword!123',
           })
           .expect(401);
-        expect(result.error.text).toBe('Unauthorized');
+        expect(result.body.message).toBe('Unauthorized');
       } catch (err) {
         console.log(err);
         expect(err).toBeFalsy();

modules/organizations/routes/organizations.routes.js

@@ -9,7 +9,9 @@ import organizations from '../controllers/organizations.controller.js';
 import organizationsSchema from '../models/organizations.schema.js';
 
 /**
- * Routes
+ * @desc Register organization CRUD and membership routes on the Express application.
+ * @param {Object} app - Express application instance
+ * @returns {void}
  */
 export default (app) => {
   // Organization CRUD

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

@@ -102,15 +102,22 @@ const create = async (body, user) => {
     throw err;
   }
 
-  // Create owner membership for the creator
-  await MembershipRepository.create({
-    userId: user.id || user._id,
-    organizationId: result._id,
-    role: 'owner',
-  });
+  // Create owner membership and set current org — rollback on failure
+  let membership;
+  try {
+    membership = await MembershipRepository.create({
+      userId: user.id || user._id,
+      organizationId: result._id,
+      role: 'owner',
+    });
 
-  // Set as user's current organization
-  await UserService.updateById(user.id || user._id, { currentOrganization: result._id });
+    await UserService.updateById(user.id || user._id, { currentOrganization: result._id });
+  } catch (err) {
+    // Rollback partially created artifacts
+    if (membership) await MembershipRepository.deleteMany({ _id: membership._id }).catch(() => {});
+    await OrganizationsRepository.remove(result).catch(() => {});
+    throw err;
+  }
 
   return result;
 };

modules/organizations/services/organizations.service.js

@@ -10,6 +10,18 @@ import MembershipService from './organizations.membership.service.js';
 import UserService from '../../users/services/users.service.js';
 import { slugify, generateOrganizationSlug } from '../helpers/organizations.slug.js';
 
+/**
+ * @desc Strip sensitive fields from an organization document before returning to public flows.
+ * @param {Object} org - Organization document (Mongoose or plain).
+ * @returns {Object} Safe projection without createdBy or plan.
+ */
+const sanitizeOrg = (org) => {
+  const obj = org.toJSON ? org.toJSON() : { ...org };
+  delete obj.createdBy;
+  delete obj.plan;
+  return obj;
+};
+
 /**
  * Extract the domain part from an email address.
  * @param {string} email - A valid email address.
@@ -158,8 +170,8 @@ const handleSignupOrganization = async (user) => {
       const existingOrgs = await OrganizationsRepository.list({ domain });
       if (existingOrgs.length > 0) {
         // Create a pending join request — admin must approve
-        const organization = existingOrgs[0];
-        await MembershipService.createJoinRequest(user.id || user._id, organization._id);
+        const organization = sanitizeOrg(existingOrgs[0]);
+        await MembershipService.createJoinRequest(user.id || user._id, existingOrgs[0]._id);
         const ability = await policy.defineAbilityFor(user, null);
         return {
           organization,
@@ -202,7 +214,7 @@ const handleSignupOrganization = async (user) => {
   if (orgConfig.domainMatching && !isPublic) {
     const existingOrgs = await OrganizationsRepository.list({ domain });
     if (existingOrgs.length > 0) {
-      suggestedOrganization = existingOrgs[0];
+      suggestedOrganization = sanitizeOrg(existingOrgs[0]);
     }
   }
 

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

@@ -25,6 +25,7 @@ describe('Organizations integration tests:', () => {
   /**
    * @description Clean up a user and their associated organizations/memberships.
    * @param {Object} user - The user object to clean up.
+   * @returns {Promise<void>}
    */
   const cleanupUser = async (user) => {
     if (!user) return;

modules/uploads/policies/uploads.policy.js

@@ -16,8 +16,15 @@ export function uploadAbilities(user, membership, { can }) {
     can('manage', 'all');
     return;
   }
-  can('read', 'Upload');
-  can('delete', 'Upload', { 'metadata.user': String(user._id) });
+  if (membership && membership.organizationId) {
+    const orgId = String(membership.organizationId._id || membership.organizationId);
+    can('read', 'Upload', { 'metadata.organizationId': orgId });
+    can('delete', 'Upload', { 'metadata.organizationId': orgId, 'metadata.user': String(user._id) });
+  } else {
+    // Users without a membership can only access their own uploads
+    can('read', 'Upload', { 'metadata.user': String(user._id) });
+    can('delete', 'Upload', { 'metadata.user': String(user._id) });
+  }
 }
 
 /**