From f3cd54bc096968d54ffecbdb8786c33b590ab376 Mon Sep 17 00:00:00 2001 From: Orhan Hirsch Date: Mon, 18 Feb 2019 22:51:52 +0100 Subject: [PATCH] Fix race condition and add test --- app/models/alternative.js | 23 ++++++++++++++++++++--- docker-compose.yml | 5 +++++ env.js | 3 ++- package.json | 4 +++- test/api/vote.test.js | 21 +++++++++++++++++++++ yarn.lock | 33 ++++++++++++++++++++++++++++++++- 6 files changed, 83 insertions(+), 6 deletions(-) diff --git a/app/models/alternative.js b/app/models/alternative.js index 20ca8140..ce2a9604 100644 --- a/app/models/alternative.js +++ b/app/models/alternative.js @@ -5,6 +5,12 @@ const mongoose = require('mongoose'); const Election = require('./election'); const Vote = require('./vote'); const errors = require('../errors'); +const env = require('../../env'); + +const redisClient = require('redis').createClient(6379, env.REDIS_URL); +const Redlock = require('redlock'); + +const redlock = new Redlock([redisClient], {}); const Schema = mongoose.Schema; @@ -38,11 +44,18 @@ alternativeSchema.methods.addVote = async function(user) { if (user.admin) throw new errors.AdminVotingError(); if (user.moderator) throw new errors.ModeratorVotingError(); + const lock = await redlock.lock('vote:' + user.username, 2000); const election = await Election.findById(this.election).exec(); - if (!election.active) throw new errors.InactiveElectionError(); + if (!election.active) { + await lock.unlock(); + throw new errors.InactiveElectionError(); + } const votedUsers = election.hasVotedUsers.toObject(); const hasVoted = _.find(votedUsers, { user: user._id }); - if (hasVoted) throw new errors.AlreadyVotedError(); + if (hasVoted) { + await lock.unlock(); + throw new errors.AlreadyVotedError(); + } // 24 character random string const voteHash = crypto.randomBytes(12).toString('hex'); @@ -50,7 +63,11 @@ alternativeSchema.methods.addVote = async function(user) { election.hasVotedUsers.push({ user: user._id }); await election.save(); - return vote.save(); + const savedVote = await vote.save(); + + await lock.unlock(); + + return savedVote; }; module.exports = mongoose.model('Alternative', alternativeSchema); diff --git a/docker-compose.yml b/docker-compose.yml index e12a9753..a1b4b556 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -5,3 +5,8 @@ services: image: mongo:3.6 ports: - '127.0.0.1:27017:27017' + redis: + image: redis:latest + ports: + - '127.0.0.1:6379:6379' + diff --git a/env.js b/env.js index 3dcfbfed..553c4c0a 100644 --- a/env.js +++ b/env.js @@ -10,5 +10,6 @@ module.exports = { HOST: process.env.HOST || 'localhost', // DSN url for reporting errors to sentry RAVEN_DSN: process.env.RAVEN_DSN, - MONGO_URL: process.env.MONGO_URL || 'mongodb://localhost:27017/vote' + MONGO_URL: process.env.MONGO_URL || 'mongodb://localhost:27017/vote', + REDIS_URL: process.env.REDIS_URL || 'localhost' }; diff --git a/package.json b/package.json index e42b058e..9d03a797 100644 --- a/package.json +++ b/package.json @@ -14,7 +14,7 @@ "lint:prettier": "prettier '**/*.js' --list-different --ignore-path .gitignore", "lint:yaml": "yarn yamllint .drone.yml docker-compose.yml usage.yml deployment/docker-compose.yml", "prettier": "prettier '**/*.js' --write", - "mocha": "NODE_ENV=test MONGO_URL=${MONGO_URL:-'mongodb://localhost:27017/vote-test'} nyc mocha test/**/*.test.js", + "mocha": "NODE_ENV=test MONGO_URL=${MONGO_URL:-'mongodb://localhost:27017/vote-test'} nyc mocha test/**/*.test.js --exit", "coverage": "nyc report --reporter=text-lcov | coveralls", "protractor": "webdriver-manager update --standalone false && NODE_ENV=protractor MONGO_URL=${MONGO_URL:-'mongodb://localhost:27017/vote-test'} protractor ./features/protractor-conf.js", "postinstall": "yarn build" @@ -62,6 +62,8 @@ "qr-scanner": "1.1.1", "qrcode": "1.3.3", "raven": "2.6.4", + "redis": "2.8.0", + "redlock": "3.1.2", "serve-favicon": "2.5.0", "socket.io": "2.2.0", "style-loader": "0.23.1", diff --git a/test/api/vote.test.js b/test/api/vote.test.js index 726ad1f2..1e66c946 100644 --- a/test/api/vote.test.js +++ b/test/api/vote.test.js @@ -136,6 +136,27 @@ describe('Vote API', () => { votes.length.should.equal(1); }); + it('should not be vulnerable to race conditions', async function() { + const create = () => + request(app) + .post('/api/vote') + .send(votePayload(this.activeAlternative.id)); + await Promise.all([ + create(), + create(), + create(), + create(), + create(), + create(), + create(), + create(), + create(), + create() + ]); + const votes = await Vote.find({ alternative: this.activeAlternative.id }); + votes.length.should.equal(1); + }); + it('should not be possible to vote without logging in', async function() { passportStub.logout(); const { body: error } = await request(app) diff --git a/yarn.lock b/yarn.lock index db0a9af1..2273418b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -750,7 +750,7 @@ bluebird@3.5.1: resolved "https://registry.yarnpkg.com/bluebird/-/bluebird-3.5.1.tgz#d9551f9de98f1fcda1e683d17ee91a0602ee2eb9" integrity sha512-MKiLiV+I1AA596t9w1sQJ8jkiSr5+ZKi0WKrYGUn6d1Fx+Ij4tIj+m2WMQSGczs5jZVxV339chE8iwk6F64wjA== -bluebird@3.5.3, bluebird@^3.5.3: +bluebird@3.5.3, bluebird@^3.3.3, bluebird@^3.5.3: version "3.5.3" resolved "https://registry.yarnpkg.com/bluebird/-/bluebird-3.5.3.tgz#7d01c6f9616c9a51ab0f8c549a79dfe6ec33efa7" integrity sha512-/qKPUQlaW1OyR51WeCPBvRnAlnZFUJkCSG5HzGnuIqhgyJtF+T94lFnn33eiazjRm2LAHVy2guNnaq48X9SJuw== @@ -1781,6 +1781,11 @@ domain-browser@^1.1.1: resolved "https://registry.yarnpkg.com/domain-browser/-/domain-browser-1.2.0.tgz#3d31f50191a6749dd1375a7f522e823d42e54eda" integrity sha512-jnjyiM6eRyZl2H+W8Q/zLMA481hzi0eszAaBUzIVnmYVDBbnLxVNnfu1HgEBvCbL+71FrxMl3E6lpKH7Ge3OXA== +double-ended-queue@^2.1.0-0: + version "2.1.0-0" + resolved "https://registry.yarnpkg.com/double-ended-queue/-/double-ended-queue-2.1.0-0.tgz#103d3527fd31528f40188130c841efdd78264e5c" + integrity sha1-ED01J/0xUo9AGIEwyEHv3XgmTlw= + duplexify@^3.4.2, duplexify@^3.6.0: version "3.6.1" resolved "https://registry.yarnpkg.com/duplexify/-/duplexify-3.6.1.tgz#b1a7a29c4abfd639585efaecce80d666b1e34125" @@ -5042,6 +5047,32 @@ readdirp@^2.0.0: micromatch "^3.1.10" readable-stream "^2.0.2" +redis-commands@^1.2.0: + version "1.4.0" + resolved "https://registry.yarnpkg.com/redis-commands/-/redis-commands-1.4.0.tgz#52f9cf99153efcce56a8f86af986bd04e988602f" + integrity sha512-cu8EF+MtkwI4DLIT0x9P8qNTLFhQD4jLfxLR0cCNkeGzs87FN6879JOJwNQR/1zD7aSYNbU0hgsV9zGY71Itvw== + +redis-parser@^2.6.0: + version "2.6.0" + resolved "https://registry.yarnpkg.com/redis-parser/-/redis-parser-2.6.0.tgz#52ed09dacac108f1a631c07e9b69941e7a19504b" + integrity sha1-Uu0J2srBCPGmMcB+m2mUHnoZUEs= + +redis@2.8.0: + version "2.8.0" + resolved "https://registry.yarnpkg.com/redis/-/redis-2.8.0.tgz#202288e3f58c49f6079d97af7a10e1303ae14b02" + integrity sha512-M1OkonEQwtRmZv4tEWF2VgpG0JWJ8Fv1PhlgT5+B+uNq2cA3Rt1Yt/ryoR+vQNOQcIEgdCdfH0jr3bDpihAw1A== + dependencies: + double-ended-queue "^2.1.0-0" + redis-commands "^1.2.0" + redis-parser "^2.6.0" + +redlock@3.1.2: + version "3.1.2" + resolved "https://registry.yarnpkg.com/redlock/-/redlock-3.1.2.tgz#27c82534da8548027aa8d61084a08078ae74ed36" + integrity sha512-CKXhOvLd4In5QOfbcF0GIcSsa+pL9JPZd+eKeMk/Sydxh93NUNtwQMTIKFqwPu3PjEgDStwYFJTurVzNA33pZw== + dependencies: + bluebird "^3.3.3" + regenerate@^1.2.1: version "1.4.0" resolved "https://registry.yarnpkg.com/regenerate/-/regenerate-1.4.0.tgz#4a856ec4b56e4077c557589cae85e7a4c8869a11"