Fix SECURITY ISSUE 3: Unauthenticated (or any) user can update board sort.

Thanks to Siam Thanat Hack (STH) !

client/components/boards/boardsList.js

@@ -74,10 +74,9 @@ BlazeComponent.extendComponent({
       },
       stop(evt, ui) {
         // To attribute the new index number, we need to get the DOM element
-        // of the previous and the following card -- if any.
         const prevBoardDom = ui.item.prev('.js-board').get(0);
-        const nextBoardBom = ui.item.next('.js-board').get(0);
-        const sortIndex = Utils.calculateIndex(prevBoardDom, nextBoardBom, 1);
+        const nextBoardDom = ui.item.next('.js-board').get(0);
+        const sortIndex = Utils.calculateIndex(prevBoardDom, nextBoardDom, 1);
 
         const boardDomElement = ui.item.get(0);
         const board = Blaze.getData(boardDomElement);
@@ -89,7 +88,10 @@ BlazeComponent.extendComponent({
         // DOM in its initial state. The card move is then handled reactively by
         // Blaze with the below query.
         $boards.sortable('cancel');
-        board.move(sortIndex.base);
+        const currentUser = ReactiveCache.getCurrentUser();
+        if (currentUser && typeof currentUser.setBoardSortIndex === 'function') {
+          currentUser.setBoardSortIndex(board._id, sortIndex.base);
+        }
       },
     });
 
@@ -184,10 +186,13 @@ BlazeComponent.extendComponent({
       };
     }
 
-    const ret = ReactiveCache.getBoards(query, {
-      sort: { sort: 1 /* boards default sorting */ },
-    });
-    return ret;
+    const boards = ReactiveCache.getBoards(query, {});
+    const currentUser = ReactiveCache.getCurrentUser();
+    if (currentUser && typeof currentUser.sortBoardsForUser === 'function') {
+      return currentUser.sortBoardsForUser(boards);
+    }
+    // Fallback: deterministic title sort when no user mapping is available (e.g., public page)
+    return boards.slice().sort((a, b) => (a.title || '').localeCompare(b.title || ''));
   },
   boardLists(boardId) {
     /* Bug Board icons random dance https://github.com/wekan/wekan/issues/4214

models/boards.js

@@ -1711,9 +1711,10 @@ if (Meteor.isServer) {
   // All logged in users are allowed to reorder boards by dragging at All Boards page and Public Boards page.
   Boards.allow({
     update(userId, board, fieldNames) {
-      return _.contains(fieldNames, 'sort');
+      return canUpdateBoardSort(userId, board, fieldNames);
     },
-    fetch: [],
+    // Need members to verify membership in policy
+    fetch: ['members'],
   });
 
   // The number of users that have starred this board is managed by trusted code

models/users.js

@@ -809,17 +809,13 @@ Users.helpers({
     return ret;
   },
   boards() {
-    return Boards.userBoards(this._id, null, {}, { sort: { sort: 1 } });
+    // Fetch unsorted; sorting is per-user via profile.boardSortIndex
+    return Boards.userBoards(this._id, null, {}, {});
   },
 
   starredBoards() {
     const { starredBoards = [] } = this.profile || {};
-    return Boards.userBoards(
-      this._id,
-      false,
-      { _id: { $in: starredBoards } },
-      { sort: { sort: 1 } },
-    );
+    return Boards.userBoards(this._id, false, { _id: { $in: starredBoards } }, {});
   },
 
   hasStarred(boardId) {
@@ -834,12 +830,7 @@ Users.helpers({
 
   invitedBoards() {
     const { invitedBoards = [] } = this.profile || {};
-    return Boards.userBoards(
-      this._id,
-      false,
-      { _id: { $in: invitedBoards } },
-      { sort: { sort: 1 } },
-    );
+    return Boards.userBoards(this._id, false, { _id: { $in: invitedBoards } }, {});
   },
 
   isInvitedTo(boardId) {
@@ -858,6 +849,32 @@ Users.helpers({
     }
     return ret;
   },
+  /**
+   * Get per-user board sort index for a board, or null when not set
+   */
+  getBoardSortIndex(boardId) {
+    const mapping = (this.profile && this.profile.boardSortIndex) || {};
+    const v = mapping[boardId];
+    return typeof v === 'number' ? v : null;
+  },
+  /**
+   * Sort an array of boards by per-user mapping; fallback to title asc
+   */
+  sortBoardsForUser(boardsArr) {
+    const mapping = (this.profile && this.profile.boardSortIndex) || {};
+    const arr = (boardsArr || []).slice();
+    arr.sort((a, b) => {
+      const ia = typeof mapping[a._id] === 'number' ? mapping[a._id] : Number.POSITIVE_INFINITY;
+      const ib = typeof mapping[b._id] === 'number' ? mapping[b._id] : Number.POSITIVE_INFINITY;
+      if (ia !== ib) return ia - ib;
+      const ta = (a.title || '').toLowerCase();
+      const tb = (b.title || '').toLowerCase();
+      if (ta < tb) return -1;
+      if (ta > tb) return 1;
+      return 0;
+    });
+    return arr;
+  },
   hasSortBy() {
     // if use doesn't have dragHandle, then we can let user to choose sort list by different order
     return !this.hasShowDesktopDragHandles();
@@ -1306,6 +1323,19 @@ Users.mutations({
       },
     };
   },
+  /**
+   * Set per-user board sort index for a board
+   * Stored at profile.boardSortIndex[boardId] = sortIndex (Number)
+   */
+  setBoardSortIndex(boardId, sortIndex) {
+    const mapping = (this.profile && this.profile.boardSortIndex) || {};
+    mapping[boardId] = sortIndex;
+    return {
+      $set: {
+        'profile.boardSortIndex': mapping,
+      },
+    };
+  },
   toggleAutoWidth(boardId) {
     const { autoWidthBoards = {} } = this.profile || {};
     autoWidthBoards[boardId] = !autoWidthBoards[boardId];

server/lib/tests/boards.security.tests.js

@@ -0,0 +1,50 @@
+/* eslint-env mocha */
+import { expect } from 'chai';
+import { Random } from 'meteor/random';
+import '../utils';
+
+// Unit tests for canUpdateBoardSort policy
+
+describe('boards security', function() {
+  describe(canUpdateBoardSort.name, function() {
+    it('denies anonymous updates even if fieldNames include sort', function() {
+      const userId = null;
+      const board = {
+        hasMember: () => true,
+      };
+      const fieldNames = ['sort'];
+
+      expect(canUpdateBoardSort(userId, board, fieldNames)).to.equal(false);
+    });
+
+    it('denies updates by non-members', function() {
+      const userId = Random.id();
+      const board = {
+        hasMember: (id) => id === 'someone-else',
+      };
+      const fieldNames = ['sort'];
+
+      expect(canUpdateBoardSort(userId, board, fieldNames)).to.equal(false);
+    });
+
+    it('allows updates when user is a member and updating sort', function() {
+      const userId = Random.id();
+      const board = {
+        hasMember: (id) => id === userId,
+      };
+      const fieldNames = ['sort'];
+
+      expect(canUpdateBoardSort(userId, board, fieldNames)).to.equal(true);
+    });
+
+    it('denies updates when not updating sort', function() {
+      const userId = Random.id();
+      const board = {
+        hasMember: (id) => id === userId,
+      };
+      const fieldNames = ['title'];
+
+      expect(canUpdateBoardSort(userId, board, fieldNames)).to.equal(false);
+    });
+  });
+});

server/lib/tests/index.js

@@ -1,2 +1,3 @@
 import './utils.tests';
 import './users.security.tests';
+import './boards.security.tests';

server/lib/utils.js

@@ -24,3 +24,12 @@ allowIsBoardMemberByCard = function(userId, card) {
   const board = card.board();
   return board && board.hasMember(userId);
 };
+
+// Policy: can a user update a board's 'sort' field?
+// Requirements:
+//  - user must be authenticated
+//  - update must include 'sort' field
+//  - user must be a member of the board
+canUpdateBoardSort = function(userId, board, fieldNames) {
+  return !!userId && _.contains(fieldNames || [], 'sort') && allowIsBoardMember(userId, board);
+};