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 *