From 0a1a075f3153e71d9a858576f1c68d2925230d9c Mon Sep 17 00:00:00 2001 From: Lauri Ojansivu Date: Sun, 2 Nov 2025 11:12:41 +0200 Subject: [PATCH] =?UTF-8?q?Fix=20SECURITY=20ISSUE=204:=20Members=20can=20f?= =?UTF-8?q?orge=20others=E2=80=99=20votes=20(Low).=20Bonus:=20Similar=20fi?= =?UTF-8?q?xes=20to=20planning=20poker=20too=20done=20by=20xet7.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Thanks to Siam Thanat Hack (STH) and xet7 ! --- SECURITY.md | 16 ++ client/components/cards/cardDetails.js | 65 +++-- models/cards.js | 290 ++++++++++++++++++++++- server/lib/tests/cards.methods.tests.js | 118 +++++++++ server/lib/tests/cards.security.tests.js | 56 +++++ server/lib/tests/index.js | 2 + 6 files changed, 505 insertions(+), 42 deletions(-) create mode 100644 server/lib/tests/cards.methods.tests.js create mode 100644 server/lib/tests/cards.security.tests.js diff --git a/SECURITY.md b/SECURITY.md index c13cd8818..f93b34ac8 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -192,6 +192,22 @@ Meteor.startup(() => { - Attempts to update forbidden fields from the client are denied. - Admin operations like managing org/team membership or toggling flags must use server methods that check permissions. +## Voting: integrity and authorization + +- Client updates to card `vote` fields are blocked to prevent forged votes and inconsistent policy enforcement. +- Voting is performed via a server method that enforces: + - Authentication and board membership, or an explicit per-card flag allowing non-members to vote. + - Only the caller's own userId is added/removed from `vote.positive`/`vote.negative`. +- This prevents members from fabricating other users' votes and ensures non-members cannot vote unless explicitly allowed. + +## Planning Poker: integrity and authorization + +- Client updates to card `poker` fields are blocked. All poker actions go through server methods that enforce: + - Authentication and board membership for configuration and results. + - For casting a poker vote, either board membership or an explicit per-card flag allowing non-members to participate. + - Only the caller's own userId is added/removed from the selected estimation bucket (e.g., one, two, five, etc.). +- Methods cover setting/unsetting poker question/end, casting votes, replaying, and setting final estimation. + ## Brute force login protection - https://github.com/wekan/wekan/commit/23e5e1e3bd081699ce39ce5887db7e612616014d diff --git a/client/components/cards/cardDetails.js b/client/components/cards/cardDetails.js index bbbd49a73..43ee28473 100644 --- a/client/components/cards/cardDetails.js +++ b/client/components/cards/cardDetails.js @@ -430,56 +430,57 @@ BlazeComponent.extendComponent({ ) { newState = forIt; } - this.data().setVote(Meteor.userId(), newState); + // Use secure server method; direct client updates to vote are blocked + Meteor.call('cards.vote', this.data()._id, newState); }, 'click .js-poker'(e) { let newState = null; if ($(e.target).hasClass('js-poker-vote-one')) { newState = 'one'; - this.data().setPoker(Meteor.userId(), newState); + Meteor.call('cards.pokerVote', this.data()._id, newState); } if ($(e.target).hasClass('js-poker-vote-two')) { newState = 'two'; - this.data().setPoker(Meteor.userId(), newState); + Meteor.call('cards.pokerVote', this.data()._id, newState); } if ($(e.target).hasClass('js-poker-vote-three')) { newState = 'three'; - this.data().setPoker(Meteor.userId(), newState); + Meteor.call('cards.pokerVote', this.data()._id, newState); } if ($(e.target).hasClass('js-poker-vote-five')) { newState = 'five'; - this.data().setPoker(Meteor.userId(), newState); + Meteor.call('cards.pokerVote', this.data()._id, newState); } if ($(e.target).hasClass('js-poker-vote-eight')) { newState = 'eight'; - this.data().setPoker(Meteor.userId(), newState); + Meteor.call('cards.pokerVote', this.data()._id, newState); } if ($(e.target).hasClass('js-poker-vote-thirteen')) { newState = 'thirteen'; - this.data().setPoker(Meteor.userId(), newState); + Meteor.call('cards.pokerVote', this.data()._id, newState); } if ($(e.target).hasClass('js-poker-vote-twenty')) { newState = 'twenty'; - this.data().setPoker(Meteor.userId(), newState); + Meteor.call('cards.pokerVote', this.data()._id, newState); } if ($(e.target).hasClass('js-poker-vote-forty')) { newState = 'forty'; - this.data().setPoker(Meteor.userId(), newState); + Meteor.call('cards.pokerVote', this.data()._id, newState); } if ($(e.target).hasClass('js-poker-vote-one-hundred')) { newState = 'oneHundred'; - this.data().setPoker(Meteor.userId(), newState); + Meteor.call('cards.pokerVote', this.data()._id, newState); } if ($(e.target).hasClass('js-poker-vote-unsure')) { newState = 'unsure'; - this.data().setPoker(Meteor.userId(), newState); + Meteor.call('cards.pokerVote', this.data()._id, newState); } }, 'click .js-poker-finish'(e) { if ($(e.target).hasClass('js-poker-finish')) { e.preventDefault(); - const now = formatDateTime(new Date()); - this.data().setPokerEnd(now); + const now = new Date(); + Meteor.call('cards.setPokerEnd', this.data()._id, now); } }, @@ -487,9 +488,9 @@ BlazeComponent.extendComponent({ if ($(e.target).hasClass('js-poker-replay')) { e.preventDefault(); this.currentCard = this.currentData(); - this.currentCard.replayPoker(); - this.data().unsetPokerEnd(); - this.data().unsetPokerEstimation(); + Meteor.call('cards.replayPoker', this.currentCard._id); + Meteor.call('cards.unsetPokerEnd', this.currentCard._id); + Meteor.call('cards.unsetPokerEstimation', this.currentCard._id); } }, 'click .js-poker-estimation'(event) { @@ -500,9 +501,9 @@ BlazeComponent.extendComponent({ this.find('#pokerEstimation').value = ''; if (ruleTitle) { - this.data().setPokerEstimation(parseInt(ruleTitle, 10)); + Meteor.call('cards.setPokerEstimation', this.data()._id, parseInt(ruleTitle, 10)); } else { - this.data().setPokerEstimation(''); + Meteor.call('cards.unsetPokerEstimation', this.data()._id); } } }, @@ -1105,20 +1106,15 @@ BlazeComponent.extendComponent({ 'is-checked', ); const endString = this.currentCard.getVoteEnd(); - - this.currentCard.setVoteQuestion( - voteQuestion, - publicVote, - allowNonBoardMembers, - ); + Meteor.call('cards.setVoteQuestion', this.currentCard._id, voteQuestion, publicVote, allowNonBoardMembers); if (endString) { - this.currentCard.setVoteEnd(endString); + Meteor.call('cards.setVoteEnd', this.currentCard._id, endString); } Popup.back(); }, 'click .js-remove-vote': Popup.afterConfirm('deleteVote', () => { event.preventDefault(); - this.currentCard.unsetVote(); + Meteor.call('cards.unsetVote', this.currentCard._id); Popup.back(); }), 'click a.js-toggle-vote-public'(event) { @@ -1317,10 +1313,10 @@ BlazeComponent.extendComponent({ ]; } _storeDate(newDate) { - this.card.setVoteEnd(newDate); + Meteor.call('cards.setVoteEnd', this.card._id, newDate); } _deleteDate() { - this.card.unsetVoteEnd(); + Meteor.call('cards.unsetVoteEnd', this.card._id); } }.register('editVoteEndDatePopup')); @@ -1342,17 +1338,14 @@ BlazeComponent.extendComponent({ ); const endString = this.currentCard.getPokerEnd(); - this.currentCard.setPokerQuestion( - pokerQuestion, - allowNonBoardMembers, - ); + Meteor.call('cards.setPokerQuestion', this.currentCard._id, pokerQuestion, allowNonBoardMembers); if (endString) { - this.currentCard.setPokerEnd(endString); + Meteor.call('cards.setPokerEnd', this.currentCard._id, new Date(endString)); } Popup.back(); }, 'click .js-remove-poker': Popup.afterConfirm('deletePoker', (event) => { - this.currentCard.unsetPoker(); + Meteor.call('cards.unsetPoker', this.currentCard._id); Popup.back(); }), 'click a.js-toggle-poker-allow-non-members'(event) { @@ -1573,10 +1566,10 @@ BlazeComponent.extendComponent({ ]; } _storeDate(newDate) { - this.card.setPokerEnd(newDate); + Meteor.call('cards.setPokerEnd', this.card._id, newDate); } _deleteDate() { - this.card.unsetPokerEnd(); + Meteor.call('cards.unsetPokerEnd', this.card._id); } }.register('editPokerEndDatePopup')); diff --git a/models/cards.js b/models/cards.js index 1959e5de3..546efdfe6 100644 --- a/models/cards.js +++ b/models/cards.js @@ -515,18 +515,29 @@ Cards.attachSchema( }), ); +// Centralized update policy for Cards +// Security: deny any direct client updates to 'vote' fields; require membership otherwise +canUpdateCard = function(userId, doc, fields) { + if (!userId) return false; + const fieldNames = fields || []; + // Block direct updates to voting fields; voting must go through Meteor method 'cards.vote' + if (_.some(fieldNames, f => typeof f === 'string' && (f === 'vote' || f.indexOf('vote.') === 0))) { + return false; + } + // Block direct updates to poker fields; poker must go through Meteor methods + if (_.some(fieldNames, f => typeof f === 'string' && (f === 'poker' || f.indexOf('poker.') === 0))) { + return false; + } + return allowIsBoardMember(userId, ReactiveCache.getBoard(doc.boardId)); +}; + Cards.allow({ insert(userId, doc) { return allowIsBoardMember(userId, ReactiveCache.getBoard(doc.boardId)); }, update(userId, doc, fields) { - // Allow board members or logged in users if only vote get's changed - return ( - allowIsBoardMember(userId, ReactiveCache.getBoard(doc.boardId)) || - (_.isEqual(fields, ['vote', 'modifiedAt', 'dateLastActivity']) && - !!userId) - ); + return canUpdateCard(userId, doc, fields); }, remove(userId, doc) { return allowIsBoardMember(userId, ReactiveCache.getBoard(doc.boardId)); @@ -3105,6 +3116,273 @@ const addCronJob = _.debounce( if (Meteor.isServer) { Meteor.methods({ + // Secure poker voting: only the caller's userId is modified + 'cards.pokerVote'(cardId, state) { + check(cardId, String); + if (state !== undefined && state !== null) check(state, String); + if (!this.userId) throw new Meteor.Error('not-authorized'); + + const card = ReactiveCache.getCard(cardId) || Cards.findOne(cardId); + if (!card) throw new Meteor.Error('not-found'); + const board = ReactiveCache.getBoard(card.boardId) || Boards.findOne(card.boardId); + if (!board) throw new Meteor.Error('not-found'); + + const isMember = allowIsBoardMember(this.userId, board); + const allowNBM = !!(card.poker && card.poker.allowNonBoardMembers); + if (!(isMember || allowNBM /* && board.permission === 'public' */)) { + throw new Meteor.Error('not-authorized'); + } + + let mod = card.setPoker(this.userId, state); + if (!mod || typeof mod !== 'object') mod = {}; + mod.$set = Object.assign({}, mod.$set, { modifiedAt: new Date(), dateLastActivity: new Date() }); + return Cards.update({ _id: cardId }, mod); + }, + + // Configure planning poker on a card (members only) + 'cards.setPokerQuestion'(cardId, question, allowNonBoardMembers) { + check(cardId, String); + check(question, Boolean); + check(allowNonBoardMembers, Boolean); + if (!this.userId) throw new Meteor.Error('not-authorized'); + + const card = ReactiveCache.getCard(cardId) || Cards.findOne(cardId); + if (!card) throw new Meteor.Error('not-found'); + const board = ReactiveCache.getBoard(card.boardId) || Boards.findOne(card.boardId); + if (!allowIsBoardMember(this.userId, board)) throw new Meteor.Error('not-authorized'); + + const modifier = { + $set: { + poker: { + question, + allowNonBoardMembers, + one: [], two: [], three: [], five: [], eight: [], thirteen: [], twenty: [], forty: [], oneHundred: [], unsure: [], + }, + modifiedAt: new Date(), + dateLastActivity: new Date(), + }, + }; + return Cards.update({ _id: cardId }, modifier); + }, + + 'cards.setPokerEnd'(cardId, end) { + check(cardId, String); + check(end, Date); + if (!this.userId) throw new Meteor.Error('not-authorized'); + + const card = ReactiveCache.getCard(cardId) || Cards.findOne(cardId); + if (!card) throw new Meteor.Error('not-found'); + const board = ReactiveCache.getBoard(card.boardId) || Boards.findOne(card.boardId); + if (!allowIsBoardMember(this.userId, board)) throw new Meteor.Error('not-authorized'); + + const modifier = { + $set: { 'poker.end': end, modifiedAt: new Date(), dateLastActivity: new Date() }, + }; + return Cards.update({ _id: cardId }, modifier); + }, + + 'cards.unsetPokerEnd'(cardId) { + check(cardId, String); + if (!this.userId) throw new Meteor.Error('not-authorized'); + + const card = ReactiveCache.getCard(cardId) || Cards.findOne(cardId); + if (!card) throw new Meteor.Error('not-found'); + const board = ReactiveCache.getBoard(card.boardId) || Boards.findOne(card.boardId); + if (!allowIsBoardMember(this.userId, board)) throw new Meteor.Error('not-authorized'); + + const modifier = { + $unset: { 'poker.end': '' }, + $set: { modifiedAt: new Date(), dateLastActivity: new Date() }, + }; + return Cards.update({ _id: cardId }, modifier); + }, + + 'cards.unsetPoker'(cardId) { + check(cardId, String); + if (!this.userId) throw new Meteor.Error('not-authorized'); + + const card = ReactiveCache.getCard(cardId) || Cards.findOne(cardId); + if (!card) throw new Meteor.Error('not-found'); + const board = ReactiveCache.getBoard(card.boardId) || Boards.findOne(card.boardId); + if (!allowIsBoardMember(this.userId, board)) throw new Meteor.Error('not-authorized'); + + const modifier = { + $unset: { poker: '' }, + $set: { modifiedAt: new Date(), dateLastActivity: new Date() }, + }; + return Cards.update({ _id: cardId }, modifier); + }, + + 'cards.setPokerEstimation'(cardId, estimation) { + check(cardId, String); + check(estimation, Number); + if (!this.userId) throw new Meteor.Error('not-authorized'); + + const card = ReactiveCache.getCard(cardId) || Cards.findOne(cardId); + if (!card) throw new Meteor.Error('not-found'); + const board = ReactiveCache.getBoard(card.boardId) || Boards.findOne(card.boardId); + if (!allowIsBoardMember(this.userId, board)) throw new Meteor.Error('not-authorized'); + + const modifier = { + $set: { 'poker.estimation': estimation, modifiedAt: new Date(), dateLastActivity: new Date() }, + }; + return Cards.update({ _id: cardId }, modifier); + }, + + 'cards.unsetPokerEstimation'(cardId) { + check(cardId, String); + if (!this.userId) throw new Meteor.Error('not-authorized'); + + const card = ReactiveCache.getCard(cardId) || Cards.findOne(cardId); + if (!card) throw new Meteor.Error('not-found'); + const board = ReactiveCache.getBoard(card.boardId) || Boards.findOne(card.boardId); + if (!allowIsBoardMember(this.userId, board)) throw new Meteor.Error('not-authorized'); + + const modifier = { + $unset: { 'poker.estimation': '' }, + $set: { modifiedAt: new Date(), dateLastActivity: new Date() }, + }; + return Cards.update({ _id: cardId }, modifier); + }, + + 'cards.replayPoker'(cardId) { + check(cardId, String); + if (!this.userId) throw new Meteor.Error('not-authorized'); + + const card = ReactiveCache.getCard(cardId) || Cards.findOne(cardId); + if (!card) throw new Meteor.Error('not-found'); + const board = ReactiveCache.getBoard(card.boardId) || Boards.findOne(card.boardId); + if (!allowIsBoardMember(this.userId, board)) throw new Meteor.Error('not-authorized'); + + // Reset all poker votes arrays + const modifier = { + $set: { + 'poker.one': [], 'poker.two': [], 'poker.three': [], 'poker.five': [], 'poker.eight': [], 'poker.thirteen': [], 'poker.twenty': [], 'poker.forty': [], 'poker.oneHundred': [], 'poker.unsure': [], + modifiedAt: new Date(), + dateLastActivity: new Date(), + }, + $unset: { 'poker.end': '' }, + }; + return Cards.update({ _id: cardId }, modifier); + }, + // Configure voting on a card (members only) + 'cards.setVoteQuestion'(cardId, question, publicVote, allowNonBoardMembers) { + check(cardId, String); + check(question, String); + check(publicVote, Boolean); + check(allowNonBoardMembers, Boolean); + if (!this.userId) throw new Meteor.Error('not-authorized'); + + const card = ReactiveCache.getCard(cardId) || Cards.findOne(cardId); + if (!card) throw new Meteor.Error('not-found'); + const board = ReactiveCache.getBoard(card.boardId) || Boards.findOne(card.boardId); + if (!allowIsBoardMember(this.userId, board)) throw new Meteor.Error('not-authorized'); + + const modifier = { + $set: { + vote: { + question, + public: publicVote, + allowNonBoardMembers, + positive: [], + negative: [], + }, + modifiedAt: new Date(), + dateLastActivity: new Date(), + }, + }; + return Cards.update({ _id: cardId }, modifier); + }, + + 'cards.setVoteEnd'(cardId, end) { + check(cardId, String); + check(end, Date); + if (!this.userId) throw new Meteor.Error('not-authorized'); + + const card = ReactiveCache.getCard(cardId) || Cards.findOne(cardId); + if (!card) throw new Meteor.Error('not-found'); + const board = ReactiveCache.getBoard(card.boardId) || Boards.findOne(card.boardId); + if (!allowIsBoardMember(this.userId, board)) throw new Meteor.Error('not-authorized'); + + const modifier = { + $set: { 'vote.end': end, modifiedAt: new Date(), dateLastActivity: new Date() }, + }; + return Cards.update({ _id: cardId }, modifier); + }, + + 'cards.unsetVoteEnd'(cardId) { + check(cardId, String); + if (!this.userId) throw new Meteor.Error('not-authorized'); + + const card = ReactiveCache.getCard(cardId) || Cards.findOne(cardId); + if (!card) throw new Meteor.Error('not-found'); + const board = ReactiveCache.getBoard(card.boardId) || Boards.findOne(card.boardId); + if (!allowIsBoardMember(this.userId, board)) throw new Meteor.Error('not-authorized'); + + const modifier = { + $unset: { 'vote.end': '' }, + $set: { modifiedAt: new Date(), dateLastActivity: new Date() }, + }; + return Cards.update({ _id: cardId }, modifier); + }, + + 'cards.unsetVote'(cardId) { + check(cardId, String); + if (!this.userId) throw new Meteor.Error('not-authorized'); + + const card = ReactiveCache.getCard(cardId) || Cards.findOne(cardId); + if (!card) throw new Meteor.Error('not-found'); + const board = ReactiveCache.getBoard(card.boardId) || Boards.findOne(card.boardId); + if (!allowIsBoardMember(this.userId, board)) throw new Meteor.Error('not-authorized'); + + const modifier = { + $unset: { vote: '' }, + $set: { modifiedAt: new Date(), dateLastActivity: new Date() }, + }; + return Cards.update({ _id: cardId }, modifier); + }, + // Secure voting: only the caller can set/unset their vote; non-members can vote only when allowed + 'cards.vote'(cardId, forIt) { + check(cardId, String); + // forIt may be true (upvote), false (downvote), or null/undefined (clear) + if (forIt !== undefined && forIt !== null) check(forIt, Boolean); + if (!this.userId) throw new Meteor.Error('not-authorized'); + + const card = ReactiveCache.getCard(cardId) || Cards.findOne(cardId); + if (!card) throw new Meteor.Error('not-found'); + const board = ReactiveCache.getBoard(card.boardId) || Boards.findOne(card.boardId); + if (!board) throw new Meteor.Error('not-found'); + + const isMember = allowIsBoardMember(this.userId, board); + const allowNBM = !!(card.vote && card.vote.allowNonBoardMembers); + if (!(isMember || allowNBM /* && board.permission === 'public' */)) { + throw new Meteor.Error('not-authorized'); + } + + // Only modify the caller's own userId in vote arrays + let modifier; + if (forIt === true) { + modifier = { + $pull: { 'vote.negative': this.userId }, + $addToSet: { 'vote.positive': this.userId }, + $set: { modifiedAt: new Date(), dateLastActivity: new Date() }, + }; + } else if (forIt === false) { + modifier = { + $pull: { 'vote.positive': this.userId }, + $addToSet: { 'vote.negative': this.userId }, + $set: { modifiedAt: new Date(), dateLastActivity: new Date() }, + }; + } else { + // Clear vote + modifier = { + $pull: { 'vote.positive': this.userId, 'vote.negative': this.userId }, + $set: { modifiedAt: new Date(), dateLastActivity: new Date() }, + }; + } + + return Cards.update({ _id: cardId }, modifier); + }, /** copies a card *
  • this method is needed on the server because attachments can only be copied on the server (access to file system) * @param card id to copy diff --git a/server/lib/tests/cards.methods.tests.js b/server/lib/tests/cards.methods.tests.js new file mode 100644 index 000000000..b81487a85 --- /dev/null +++ b/server/lib/tests/cards.methods.tests.js @@ -0,0 +1,118 @@ +/* eslint-env mocha */ +import { expect } from 'chai'; +import sinon from 'sinon'; +import { Meteor } from 'meteor/meteor'; +import '/models/cards'; + +// Helpers to access method handlers +const voteHandler = () => Meteor.server.method_handlers['cards.vote']; +const pokerVoteHandler = () => Meteor.server.method_handlers['cards.pokerVote']; + +// Preserve originals to restore after stubbing +const origGetCard = ReactiveCache.getCard; +const origGetBoard = ReactiveCache.getBoard; + +describe('cards methods security', function() { + let updateStub; + + beforeEach(function() { + // Stub collection update to capture modifiers + updateStub = sinon.stub(Cards, 'update').returns(1); + }); + + afterEach(function() { + if (updateStub) updateStub.restore(); + ReactiveCache.getCard = origGetCard; + ReactiveCache.getBoard = origGetBoard; + }); + + describe('cards.vote', function() { + it('denies non-member when allowNonBoardMembers=false', function() { + const cardId = 'card1'; + const callerId = 'user-nonmember'; + const board = { hasMember: id => id === 'someone-else' }; + const card = { _id: cardId, boardId: 'board1', vote: { allowNonBoardMembers: false } }; + + ReactiveCache.getCard = () => card; + ReactiveCache.getBoard = () => board; + + const callMethod = () => voteHandler().call({ userId: callerId }, cardId, true); + expect(callMethod).to.throw(); + expect(updateStub.called).to.equal(false); + }); + + it('allows non-member only for own userId when allowNonBoardMembers=true', function() { + const cardId = 'card2'; + const callerId = 'user-guest'; + const board = { hasMember: id => id === 'someone-else' }; + const card = { _id: cardId, boardId: 'board2', vote: { allowNonBoardMembers: true } }; + + ReactiveCache.getCard = () => card; + ReactiveCache.getBoard = () => board; + + voteHandler().call({ userId: callerId }, cardId, true); + + expect(updateStub.calledOnce).to.equal(true); + const [, modifier] = updateStub.getCall(0).args; + expect(modifier.$addToSet['vote.positive']).to.equal(callerId); + expect(modifier.$pull['vote.negative']).to.equal(callerId); + expect(modifier.$set.modifiedAt).to.be.instanceOf(Date); + expect(modifier.$set.dateLastActivity).to.be.instanceOf(Date); + }); + + it('ensures member votes only affect caller userId', function() { + const cardId = 'card3'; + const callerId = 'member1'; + const otherId = 'member2'; + const board = { hasMember: id => (id === callerId || id === otherId) }; + const card = { _id: cardId, boardId: 'board3', vote: { allowNonBoardMembers: false } }; + + ReactiveCache.getCard = () => card; + ReactiveCache.getBoard = () => board; + + voteHandler().call({ userId: callerId }, cardId, true); + + expect(updateStub.calledOnce).to.equal(true); + const [, modifier] = updateStub.getCall(0).args; + // Only callerId present in modifier + expect(modifier.$addToSet['vote.positive']).to.equal(callerId); + expect(modifier.$pull['vote.negative']).to.equal(callerId); + }); + }); + + describe('cards.pokerVote', function() { + it('denies non-member when allowNonBoardMembers=false', function() { + const cardId = 'card4'; + const callerId = 'nm'; + const board = { hasMember: id => id === 'someone-else' }; + const card = { _id: cardId, boardId: 'board4', poker: { allowNonBoardMembers: false } }; + + ReactiveCache.getCard = () => card; + ReactiveCache.getBoard = () => board; + + const callMethod = () => pokerVoteHandler().call({ userId: callerId }, cardId, 'five'); + expect(callMethod).to.throw(); + expect(updateStub.called).to.equal(false); + }); + + it('allows non-member only for own userId when allowNonBoardMembers=true', function() { + const cardId = 'card5'; + const callerId = 'guest'; + const board = { hasMember: id => id === 'someone-else' }; + const card = { _id: cardId, boardId: 'board5', poker: { allowNonBoardMembers: true } }; + + ReactiveCache.getCard = () => card; + ReactiveCache.getBoard = () => board; + + pokerVoteHandler().call({ userId: callerId }, cardId, 'eight'); + + expect(updateStub.calledOnce).to.equal(true); + const [, modifier] = updateStub.getCall(0).args; + expect(modifier.$addToSet['poker.eight']).to.equal(callerId); + // Ensure removal from other buckets includes callerId + expect(modifier.$pull['poker.one']).to.equal(callerId); + expect(modifier.$set.modifiedAt).to.be.instanceOf(Date); + expect(modifier.$set.dateLastActivity).to.be.instanceOf(Date); + }); + }); +}); diff --git a/server/lib/tests/cards.security.tests.js b/server/lib/tests/cards.security.tests.js new file mode 100644 index 000000000..ae2a640df --- /dev/null +++ b/server/lib/tests/cards.security.tests.js @@ -0,0 +1,56 @@ +/* eslint-env mocha */ +import { expect } from 'chai'; +import '../utils'; +import '/models/cards'; + +// Unit tests for canUpdateCard policy (deny direct vote updates) +describe('cards security', function() { + describe(canUpdateCard.name, function() { + const userId = 'user1'; + const board = { + hasMember: (id) => id === userId, + }; + const doc = { boardId: 'board1' }; + + // Patch ReactiveCache.getBoard for this unit test scope if not defined + const origGetBoard = ReactiveCache && ReactiveCache.getBoard; + before(function() { + if (typeof ReactiveCache === 'object') { + ReactiveCache.getBoard = () => board; + } + }); + after(function() { + if (typeof ReactiveCache === 'object') { + ReactiveCache.getBoard = origGetBoard; + } + }); + + it('denies anonymous users', function() { + expect(canUpdateCard(null, doc, ['title'])).to.equal(false); + }); + + it('denies direct vote updates', function() { + expect(canUpdateCard(userId, doc, ['vote'])).to.equal(false); + expect(canUpdateCard(userId, doc, ['vote', 'modifiedAt', 'dateLastActivity'])).to.equal(false); + expect(canUpdateCard(userId, doc, ['vote.positive'])).to.equal(false); + expect(canUpdateCard(userId, doc, ['vote.negative'])).to.equal(false); + }); + + it('denies direct poker updates', function() { + expect(canUpdateCard(userId, doc, ['poker'])).to.equal(false); + expect(canUpdateCard(userId, doc, ['poker.one'])).to.equal(false); + expect(canUpdateCard(userId, doc, ['poker.allowNonBoardMembers'])).to.equal(false); + expect(canUpdateCard(userId, doc, ['poker.end'])).to.equal(false); + }); + + it('allows member updates when not touching vote', function() { + expect(canUpdateCard(userId, doc, ['title'])).to.equal(true); + expect(canUpdateCard(userId, doc, ['description', 'modifiedAt'])).to.equal(true); + }); + + it('denies non-members even when not touching vote', function() { + const nonMemberId = 'user2'; + expect(canUpdateCard(nonMemberId, doc, ['title'])).to.equal(false); + }); + }); +}); diff --git a/server/lib/tests/index.js b/server/lib/tests/index.js index 4dcba3294..7b8ef48ed 100644 --- a/server/lib/tests/index.js +++ b/server/lib/tests/index.js @@ -1,3 +1,5 @@ import './utils.tests'; import './users.security.tests'; import './boards.security.tests'; +import './cards.security.tests'; +import './cards.methods.tests';