Skip to content

Commit

Permalink
Merge pull request #315 from webkom/fix-race-condition
Browse files Browse the repository at this point in the history
Fix race condition and add test
  • Loading branch information
orhanhenrik authored Feb 18, 2019
2 parents bcbc692 + c59cc2d commit 6fb6ef3
Show file tree
Hide file tree
Showing 9 changed files with 93 additions and 8 deletions.
6 changes: 4 additions & 2 deletions .drone.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ pipeline:
- sudo su -c "mkdir -p /root/.config/yarn/"
- sudo su -c "chown -R circleci:circleci ."
- sudo su -c "chown -R circleci:circleci /root/"
- MONGO_URL=mongodb://mongodb:27017/vote-integration-test HEADLESS=true yarn protractor
- MONGO_URL=mongodb://mongodb:27017/vote-integration-test REDIS_URL=redis HEADLESS=true yarn protractor
lint:
image: node:11
when:
Expand All @@ -39,7 +39,7 @@ pipeline:
- pull_request
group: testing
commands:
- MONGO_URL=mongodb://mongodb:27017/vote-test yarn mocha
- MONGO_URL=mongodb://mongodb:27017/vote-test REDIS_URL=redis yarn mocha
build:
image: node:11
when:
Expand Down Expand Up @@ -68,3 +68,5 @@ pipeline:
services:
mongodb:
image: mongo:3.6
redis:
image: redis:latest
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ $ yarn start
- `MONGO_URL`
- Url to the database connection
- `default`: `mongodb://localhost:27017/vote`
- `REDIS_URL`
- Hostname of the redis server
- `default`: `localhost`
- `LOGO_SRC` _(optional)_
- Url to the main logo on all pages
- `default`: `/static/images/Abakule.jpg`
Expand Down
23 changes: 20 additions & 3 deletions app/models/alternative.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -38,19 +44,30 @@ 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');
const vote = new Vote({ hash: voteHash, alternative: this.id });

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);
3 changes: 3 additions & 0 deletions deployment/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,14 @@ version: '2'
services:
mongo:
image: mongo:3.6
redis:
image: redis:latest
vote:
image: abakus/vote:latest
environment:
# Reed more about these in the README-file in the base directory
MONGO_URL: 'mongodb://mongo:27017/vote'
REDIS_URL: 'redis'
COOKIE_SECRET: 'long-secret-here-is-important'
LOGO_SRC: 'https://raw.githubusercontent.com/webkom/lego/master/assets/abakus_webkom.png'
ports:
Expand Down
5 changes: 5 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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'

3 changes: 2 additions & 1 deletion env.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
};
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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",
Expand Down
21 changes: 21 additions & 0 deletions test/api/vote.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
33 changes: 32 additions & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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==
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down

0 comments on commit 6fb6ef3

Please sign in to comment.