Skip to content
This repository has been archived by the owner on Oct 5, 2024. It is now read-only.

Use localstorage instead of cookies for password hash #296

Merged
merged 10 commits into from
Feb 11, 2018
4 changes: 1 addition & 3 deletions client/src/components/App/Setup/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import React, { Component, PropTypes } from 'react';
import { connect } from 'react-redux';
import { Redirect } from 'react-router-dom';
import SHA256 from 'crypto-js/sha256';
import Cookies from 'js-cookie';
import DocumentTitle from 'react-document-title';
import { register } from 'features/auth/actionCreators';
import Button from '../../Button';
Expand Down Expand Up @@ -63,8 +62,7 @@ class Setup extends Component {
const { username } = this.props;
const passwordHash = SHA256(privateCode + username).toString();
this.props.register(pin, passwordHash);
Cookies.set('passwordHash', passwordHash);
window.location.reload(); // Reload page after submitting to update cookies.
localStorage.setItem('passwordHash', passwordHash);
}
}

Expand Down
3 changes: 1 addition & 2 deletions client/src/components/App/VotingMenu/index.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import React from 'react';
import { connect } from 'react-redux';
import Cookies from 'js-cookie';
import { VOTING_NOT_STARTED, VOTING_IN_PROGRESS } from 'common/actionTypes/issues';
import { submitAnonymousVote, submitRegularVote } from 'features/voting/actionCreators';
import { getShuffledAlternatives } from 'features/alternative/selectors';
Expand Down Expand Up @@ -137,7 +136,7 @@ const mergeProps = (stateProps, dispatchProps, ownProps) => {
...stateProps,
handleVote: (issue, alternative) => {
dispatch(stateProps.issue.secret ?
submitAnonymousVote(issue, alternative, Cookies.get('passwordHash')) : submitRegularVote(issue, alternative));
submitAnonymousVote(issue, alternative, localStorage.getItem('passwordHash')) : submitRegularVote(issue, alternative));
},
};
};
Expand Down
14 changes: 14 additions & 0 deletions client/src/features/auth/sagas.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { call, takeLatest, put } from 'redux-saga/effects';
import { REQUEST_PASSWORD_HASH, SEND_PASSWORD_HASH } from 'common/actionTypes/auth';

function* requestPasswordHash() {
const passwordHash = yield call([localStorage, 'getItem'], 'passwordHash');
yield put({
type: SEND_PASSWORD_HASH,
passwordHash,
});
}

export default function* authSaga() {
yield takeLatest(REQUEST_PASSWORD_HASH, requestPasswordHash);
}
3 changes: 2 additions & 1 deletion client/src/features/sagas.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import createSagaMiddleware from 'redux-saga';
import { all } from 'redux-saga/effects';

import auth from './auth/sagas';
import userSettings from './userSettings/sagas';

function* rootSaga() {
yield all([userSettings()]);
yield all([auth(), userSettings()]);
}

export const sagaMiddleware = createSagaMiddleware();
Expand Down
23 changes: 21 additions & 2 deletions client/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,29 @@ const persistConfig = {

const socket = IO.connect();

const socketIoMiddleware = createSocketIoMiddleware(socket, 'server/');
const socketIoConfig = [
{
// Special listener used for initial connection auth
prefix: 'auth/',
eventName: 'auth',
},
{
// Admin listener
prefix: 'admin/',
eventName: 'admin',
},
{
// Default listener
prefix: 'server/',
eventName: 'action',
},
];

const socketIoMiddlewares = socketIoConfig.map(({ prefix, eventName }) => (
createSocketIoMiddleware(socket, prefix, { eventName })
));

let middleware = [socketIoMiddleware, sagaMiddleware];
let middleware = [...socketIoMiddlewares, sagaMiddleware];
if (process.env.NODE_ENV !== 'production') {
middleware = [...middleware, logger];
}
Expand Down
2 changes: 2 additions & 0 deletions common/actionTypes/auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,6 @@ module.exports = {
ADMIN_CREATE_GENFORS: 'server/ADMIN_CREATE_GENFORS',
ADMIN_LOGIN: 'server/ADMIN_LOGIN',
ADMIN_SIGNED_IN: 'ADMIN_SIGNED_IN',
REQUEST_PASSWORD_HASH: 'REQUEST_PASSWORD_HASH',
SEND_PASSWORD_HASH: 'auth/SEND_PASSWORD_HASH',
};
2 changes: 0 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@
"html-webpack-plugin": "^2.28.0",
"jest": "^21.0.2",
"jest-fetch-mock": "^1.2.1",
"js-cookie": "^2.1.4",
"mock-socket": "^7.0.0",
"moment": "^2.18.1",
"node-fetch": "^1.6.3",
Expand Down Expand Up @@ -86,7 +85,6 @@
"sillyname": "^0.1.0",
"socket.io": "^1.7.2",
"socket.io-client": "^1.7.2",
"socket.io-cookie": "^0.0.1",
"socketio": "^1.0.0",
"style-loader": "^0.13.1",
"url-loader": "^0.5.7",
Expand Down
15 changes: 15 additions & 0 deletions server/channels/__tests__/__snapshots__/vote.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,21 @@ Array [
]
`;

exports[`submitAnonymousVote emits error with wrong passwordHash 1`] = `
Array [
Array [
"action",
Object {
"data": Object {
"id": 0,
"message": "Din personlige kode er feil",
},
"type": "ERROR",
},
],
]
`;

exports[`submitAnonymousVote emits vote 1`] = `
Array [
Array [
Expand Down
8 changes: 4 additions & 4 deletions server/channels/__tests__/auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,9 @@ describe('register', () => {

it('emits registered when user is already registered with correct personal code', async () => {
getAnonymousUser.mockImplementation(
async (passwordHash, onlinewebId, genfors) => generateAnonymousUser({
async (passwordHash, onlinewebId, meetingId) => generateAnonymousUser({
passwordHash,
genfors,
meetingId,
},
));
const socket = generateSocket({ completedRegistration: true }, { passwordHash: 'correct' });
Expand Down Expand Up @@ -118,7 +118,7 @@ describe('listener', () => {
const socket = generateSocket({ completedRegistration: false });
await listener(socket);

await socket.createAction(generateData());
await socket.mockEmit('action', generateData());

expect(socket.emit).toBeCalled();
expect(socket.broadcast.emit).toBeCalled();
Expand All @@ -128,7 +128,7 @@ describe('listener', () => {
const socket = generateSocket({ completedRegistration: false });
await listener(socket);

await socket.createAction(generateData({ type: 'INVALID_ACTION' }));
await socket.mockEmit('action', generateData({ type: 'INVALID_ACTION' }));

expect(socket.emit).not.toBeCalled();
expect(socket.broadcast.emit).not.toBeCalled();
Expand Down
6 changes: 3 additions & 3 deletions server/channels/__tests__/authAdmin.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ describe('listener', () => {
const socket = generateSocket();
await listener(socket);

await socket.createAction(generateData({ type: ADMIN_CREATE_GENFORS }));
await socket.mockEmit('action', generateData({ type: ADMIN_CREATE_GENFORS }));

expect(socket.emit).toBeCalled();
});
Expand All @@ -70,7 +70,7 @@ describe('listener', () => {
const socket = generateSocket();
await listener(socket);

await socket.createAction(generateData({ type: ADMIN_LOGIN }));
await socket.mockEmit('action', generateData({ type: ADMIN_LOGIN }));

expect(socket.emit).toBeCalled();
});
Expand All @@ -79,7 +79,7 @@ describe('listener', () => {
const socket = generateSocket();
await listener(socket);

await socket.createAction(generateData({ type: 'INVALID_ACTION' }));
await socket.mockEmit('action', generateData({ type: 'INVALID_ACTION' }));

expect(socket.emit).not.toBeCalled();
expect(socket.broadcast.emit).not.toBeCalled();
Expand Down
7 changes: 5 additions & 2 deletions server/channels/__tests__/connection.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ jest.mock('../../models/meeting.accessors');
jest.mock('../../models/issue.accessors');
jest.mock('../../models/vote.accessors');
jest.mock('../../models/user.accessors');
jest.mock('../../utils/socketAction');
const { execSync } = require('child_process');

execSync.mockImplementation(() => Buffer.from('fake_git_hash'));
Expand All @@ -11,16 +12,17 @@ const { getActiveGenfors } = require('../../models/meeting.accessors');
const { getAnonymousUser, getUsers } = require('../../models/user.accessors');
const { getVotes, getUserVote, getAnonymousUserVote } = require('../../models/vote.accessors');
const { getActiveQuestion, getIssueWithAlternatives, getConcludedIssues, getIssueById } = require('../../models/issue.accessors');
const { waitForAction } = require('../../utils/socketAction');
const { generateSocket, generateGenfors, generateAnonymousUser, generateIssue, generateVote, generateUser } = require('../../utils/generateTestData');
const permissionLevels = require('../../../common/auth/permissions');

describe('connection', () => {
beforeEach(() => {
getActiveGenfors.mockImplementation(async () => generateGenfors());
getAnonymousUser.mockImplementation(
async (passwordHash, onlinewebId, genfors) => generateAnonymousUser({
async (passwordHash, onlinewebId, meetingId) => generateAnonymousUser({
passwordHash,
genfors,
meetingId,
},
));
getActiveQuestion.mockImplementation(async () => generateIssue());
Expand All @@ -45,6 +47,7 @@ describe('connection', () => {
);
getUsers.mockImplementation(async () => [generateUser()]);
getIssueById.mockImplementation(async id => generateIssue({ id }));
waitForAction.mockImplementation(async () => ({ passwordHash: 'fake_password_hash' }));
});

it('emits correct actions when signed in and active genfors', async () => {
Expand Down
13 changes: 12 additions & 1 deletion server/channels/__tests__/vote.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const { createUserVote, createAnonymousVote, getUserVote } = require('../../mode
const { getIssueById, getIssueWithAlternatives } = require('../../models/issue.accessors');
const { getActiveGenfors, getGenfors } = require('../../models/meeting.accessors');
const { getAnonymousUser } = require('../../models/user.accessors');
const { generateIssue, generateVote, generateSocket } = require('../../utils/generateTestData');
const { generateIssue, generateVote, generateSocket, generateAnonymousUser } = require('../../utils/generateTestData');
const { CAN_VOTE, IS_LOGGED_IN } = require('../../../common/auth/permissions');
const { VOTING_NOT_STARTED, VOTING_FINISHED } = require('../../../common/actionTypes/issues');

Expand Down Expand Up @@ -162,6 +162,9 @@ describe('submitAnonymousVote', () => {
createAnonymousVote.mockImplementation((userId, issueId, alternativeId) =>
generateVote({ userId, issueId, alternativeId }),
);
getAnonymousUser.mockImplementation(async (passwordHash, onlinewebId, meetingId) =>
generateAnonymousUser({ passwordHash, meetingId }),
);
});
it('emits error when not registered', async () => {
const socket = generateSocket({ completedRegistration: false });
Expand All @@ -170,6 +173,14 @@ describe('submitAnonymousVote', () => {
expect(socket.emit.mock.calls).toMatchSnapshot();
});

it('emits error with wrong passwordHash', async () => {
getAnonymousUser.mockImplementation(async () => null);
const socket = generateSocket();
await submitAnonymousVote(socket, generateData());

expect(socket.emit.mock.calls).toMatchSnapshot();
});

it('broadcasts nothing when not registered', async () => {
const socket = generateSocket({ completedRegistration: false });
await submitAnonymousVote(socket, generateData());
Expand Down
10 changes: 7 additions & 3 deletions server/channels/connection.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const { emit, emitError } = require('../utils');
const { waitForAction } = require('../utils/socketAction');
const logger = require('../logging');

const getActiveGenfors = require('../models/meeting.accessors').getActiveGenfors;
Expand All @@ -18,6 +19,8 @@ const { userIsAdmin } = require('../../common/auth/permissions');
const {
AUTH_SIGNED_IN,
AUTH_REGISTERED,
REQUEST_PASSWORD_HASH,
SEND_PASSWORD_HASH,
} = require('../../common/actionTypes/auth');
const {
RECEIVE_VOTE: SEND_VOTE,
Expand All @@ -44,7 +47,7 @@ const emitUserData = async (socket) => {

let validPasswordHash = false;
try {
const { passwordHash } = socket.request.headers.cookie;
const { passwordHash } = await waitForAction(socket, 'auth', REQUEST_PASSWORD_HASH, SEND_PASSWORD_HASH);
validPasswordHash = await validatePasswordHash(user, passwordHash);
} catch (err) {
logger.error('Failed to validate passwordHash', { userId: user.id, err: err.message });
Expand Down Expand Up @@ -79,8 +82,9 @@ const emitActiveQuestion = async (socket, meeting) => {
// Emit voted state if user has voted.
let voter;
if (issue.secret) {
voter = await getAnonymousUser(socket.request.headers.cookie.passwordHash,
user.onlinewebId, meeting);
// TODO: Consider refactoring so that password hash is only retrieved once
const { passwordHash } = await waitForAction(socket, 'auth', REQUEST_PASSWORD_HASH, SEND_PASSWORD_HASH);
voter = await getAnonymousUser(passwordHash, user.onlinewebId, meeting);
} else {
voter = user;
}
Expand Down
2 changes: 0 additions & 2 deletions server/channels/index.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
const cookieParser = require('cookie-parser');
const logger = require('../logging');
const socketio = require('socket.io');
const cookieParserIO = require('socket.io-cookie');
const session = require('express-session');
const SequelizeStore = require('connect-session-sequelize')(session.Store);
const passportSocketIo = require('passport.socketio');
Expand Down Expand Up @@ -39,7 +38,6 @@ const applyMiddlewares = (io) => {
success: authorizeSuccess,
fail: authorizeFailure,
}));
io.use(cookieParserIO);
};

const listen = (server) => {
Expand Down
9 changes: 6 additions & 3 deletions server/channels/vote.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const logger = require('../logging');
const { addVote, generatePublicVote } = require('../managers/vote');
const { getActiveGenfors } = require('../models/meeting.accessors');
const { getAnonymousUser } = require('../models/user.accessors');
const { isRegistered } = require('../managers/user');
const { isRegistered, validatePasswordHash } = require('../managers/user');

const {
RECEIVE_VOTE: SEND_VOTE,
Expand All @@ -15,8 +15,7 @@ const {

const checkRegistered = async (socket) => {
const user = await socket.request.user();
const { passwordHash } = socket.request.headers.cookie;
const registered = await isRegistered(user, passwordHash);
const registered = await isRegistered(user);
if (!registered) {
emitError(socket, new Error('Du er ikke registert'));
return false;
Expand Down Expand Up @@ -53,6 +52,10 @@ const submitAnonymousVote = async (socket, data) => {
return;
}
const user = await socket.request.user();
if (!await validatePasswordHash(user, data.passwordHash)) {
emitError(socket, new Error('Din personlige kode er feil'));
return;
}
const genfors = await getActiveGenfors();
const anonymousUser = await getAnonymousUser(data.passwordHash,
user.onlinewebId, genfors);
Expand Down
4 changes: 2 additions & 2 deletions server/managers/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ async function validatePasswordHash(user, passwordHash) {
return existingUser != null;
}

async function isRegistered(user, passwordHash) {
return user.completedRegistration && await validatePasswordHash(user, passwordHash) === true;
function isRegistered(user) {
return user.completedRegistration;
}


Expand Down
24 changes: 24 additions & 0 deletions server/utils/__tests__/socketAction.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
jest.mock('../../utils');
const { emit } = require('../../utils');
const { waitForAction } = require('../socketAction');
const { EventEmitter } = require('events');

function emulateSocketResponse(socket, eventName, payload) {
socket.emit(eventName, payload);
}

describe('waitForAction', () => {
const REQUEST_ACTION = 'REQUEST_ACTION';
const SEND_ACTION = 'SEND_ACTION';
it('calls emit and returns payload', async () => {
const socket = new EventEmitter();
const eventName = 'test';
const payload = { type: SEND_ACTION, data: 123 };

const actionPromise = waitForAction(socket, eventName, REQUEST_ACTION, SEND_ACTION);
emulateSocketResponse(socket, eventName, payload);

await expect(actionPromise).resolves.toEqual(payload);
expect(emit).toBeCalledWith(socket, REQUEST_ACTION);
});
});
Loading