diff --git a/client/components/boards/boardsList.js b/client/components/boards/boardsList.js index 1d655fd11..db2ed2446 100644 --- a/client/components/boards/boardsList.js +++ b/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 diff --git a/models/boards.js b/models/boards.js index 1ff0b0fb4..569bb5e78 100644 --- a/models/boards.js +++ b/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 diff --git a/models/users.js b/models/users.js index da2fa0c0a..3298132a9 100644 --- a/models/users.js +++ b/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]; diff --git a/server/lib/tests/boards.security.tests.js b/server/lib/tests/boards.security.tests.js new file mode 100644 index 000000000..334a5d099 --- /dev/null +++ b/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); + }); + }); +}); diff --git a/server/lib/tests/index.js b/server/lib/tests/index.js index c46057bd6..4dcba3294 100644 --- a/server/lib/tests/index.js +++ b/server/lib/tests/index.js @@ -1,2 +1,3 @@ import './utils.tests'; import './users.security.tests'; +import './boards.security.tests'; diff --git a/server/lib/utils.js b/server/lib/utils.js index 2d19f6de3..b194bb246 100644 --- a/server/lib/utils.js +++ b/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); +};