Security Fix: IDOR CWE-639 that affected WeKan 7.80-7.93.

Thanks to apitech.fr and xet7 !

client/components/settings/peopleBody.js

@@ -1269,23 +1269,30 @@ Template.settingsUserPopup.events({
   },
   'click #deleteButton'(event) {
     event.preventDefault();
-    Users.remove(this.userId);
+
-    /*
+    // Use secure server method instead of direct client-side removal
-    // Delete user is enabled, but you should remove user from all boards
+    Meteor.call('removeUser', this.userId, (error, result) => {
-    // before deleting user, because there is possibility of leaving empty user avatars
+      if (error) {
-    // to boards. You can remove non-existing user ids manually from database,
+        if (process.env.DEBUG === 'true') {
-    // if that happens.
+          console.error('Error removing user:', error);
-    //. See:
+        }
-    // - wekan/client/components/settings/peopleBody.jade deleteButton
+        // Show error message to user
-    // - wekan/client/components/settings/peopleBody.js deleteButton
+        if (error.error === 'not-authorized') {
-    // - wekan/client/components/sidebar/sidebar.js Popup.afterConfirm('removeMember'
+          alert('You are not authorized to delete this user.');
-    //   that does now remove member from board, card members and assignees correctly,
+        } else if (error.error === 'user-not-found') {
-    //   but that should be used to remove user from all boards similarly
+          alert('User not found.');
-    // - wekan/models/users.js Delete is not enabled
+        } else if (error.error === 'not-authorized' && error.reason === 'Cannot delete the last administrator') {
-    //
+          alert('Cannot delete the last administrator.');
-    //
+        } else {
-    */
+          alert('Error deleting user: ' + error.reason);
+        }
+      } else {
+        if (process.env.DEBUG === 'true') {
+          console.log('User deleted successfully:', result);
+        }
         Popup.back();
+      }
+    });
   },
 });
 

client/components/users/userHeader.js

@@ -241,8 +241,21 @@ Template.editProfilePopup.events({
   },
   'click #deleteButton': Popup.afterConfirm('userDelete', function() {
     Popup.back();
-    Users.remove(Meteor.userId());
+
+    // Use secure server method for self-deletion
+    Meteor.call('removeUser', Meteor.userId(), (error, result) => {
+      if (error) {
+        if (process.env.DEBUG === 'true') {
+          console.error('Error removing user:', error);
+        }
+        alert('Error deleting account: ' + error.reason);
+      } else {
+        if (process.env.DEBUG === 'true') {
+          console.log('User deleted successfully:', result);
+        }
         AccountsTemplates.logout();
+      }
+    });
   }),
 });
 

models/users.js

@@ -571,27 +571,10 @@ Users.allow({
     return doc._id === userId;
   },
   remove(userId, doc) {
-    const adminsNumber = ReactiveCache.getUsers({
+    // Disable direct client-side user removal for security
-      isAdmin: true,
+    // All user removal should go through the secure server method 'removeUser'
-    }).length;
+    // This prevents IDOR vulnerabilities and ensures proper authorization checks
-    const isAdmin = ReactiveCache.getUser(
-      {
-        _id: userId,
-      },
-      {
-        fields: {
-          isAdmin: 1,
-        },
-      },
-    );
-
-    // Prevents remove of the only one administrator
-    if (adminsNumber === 1 && isAdmin && userId === doc._id) {
     return false;
-    }
-
-    // If it's the user or an admin
-    return userId === doc._id || isAdmin;
   },
   fetch: [],
 });
@@ -1314,6 +1297,50 @@ Users.mutations({
 });
 
 Meteor.methods({
+  // Secure user removal method with proper authorization checks
+  removeUser(targetUserId) {
+    check(targetUserId, String);
+
+    const currentUserId = Meteor.userId();
+    if (!currentUserId) {
+      throw new Meteor.Error('not-authorized', 'User must be logged in');
+    }
+
+    const currentUser = ReactiveCache.getUser(currentUserId);
+    if (!currentUser) {
+      throw new Meteor.Error('not-authorized', 'Current user not found');
+    }
+
+    const targetUser = ReactiveCache.getUser(targetUserId);
+    if (!targetUser) {
+      throw new Meteor.Error('user-not-found', 'Target user not found');
+    }
+
+    // Check if user is trying to delete themselves
+    if (currentUserId === targetUserId) {
+      // User can delete themselves
+      Users.remove(targetUserId);
+      return { success: true, message: 'User deleted successfully' };
+    }
+
+    // Check if current user is admin
+    if (!currentUser.isAdmin) {
+      throw new Meteor.Error('not-authorized', 'Only administrators can delete other users');
+    }
+
+    // Check if target user is the last admin
+    const adminsNumber = ReactiveCache.getUsers({
+      isAdmin: true,
+    }).length;
+
+    if (adminsNumber === 1 && targetUser.isAdmin) {
+      throw new Meteor.Error('not-authorized', 'Cannot delete the last administrator');
+    }
+
+    // Admin can delete non-admin users
+    Users.remove(targetUserId);
+    return { success: true, message: 'User deleted successfully' };
+  },
   setListSortBy(value) {
     check(value, String);
     ReactiveCache.getCurrentUser().setListSortBy(value);