Skip to content
Snippets Groups Projects
Commit 1f7461e9 authored by Lloyd Hilaiel's avatar Lloyd Hilaiel
Browse files

ensure users must authenticate every week (issue #309). Also, move all magic...

ensure users must authenticate every week (issue #309).  Also, move all magic numbers out of implementation and into the configuration abstraction.
parent 925fae8e
No related branches found
No related tags found
No related merge requests found
...@@ -183,7 +183,7 @@ exports.setup = function(server) { ...@@ -183,7 +183,7 @@ exports.setup = function(server) {
httpOnly: true, httpOnly: true,
// IMPORTANT: we allow users to go 1 weeks on the same device // IMPORTANT: we allow users to go 1 weeks on the same device
// without entering their password again // without entering their password again
maxAge: (7 * 24 * 60 * 60 * 1000), maxAge: configuration.get('authentication_duration_ms'),
secure: overSSL secure: overSSL
} }
}); });
......
...@@ -75,11 +75,6 @@ function flush() { ...@@ -75,11 +75,6 @@ function flush() {
} }
} }
// when should a key created right now expire?
function getExpiryTime() {
return ((new Date()).getTime() + (14 * 24 * 60 * 60 * 1000));
}
// when unit_test is set in configuration, database should be // when unit_test is set in configuration, database should be
// ephemeral. which simply means we use a temp file and delete // ephemeral. which simply means we use a temp file and delete
// on close; // on close;
......
...@@ -47,7 +47,7 @@ bcrypt = require('bcrypt'), ...@@ -47,7 +47,7 @@ bcrypt = require('bcrypt'),
crypto = require('crypto'), crypto = require('crypto'),
logger = require('../../libs/logging.js').logger, logger = require('../../libs/logging.js').logger,
ca = require('./ca.js'), ca = require('./ca.js'),
BCRYPT_WORK_FACTOR = 12; configuration = require('../../libs/configuration.js');
function checkParams(params) { function checkParams(params) {
return function(req, resp, next) { return function(req, resp, next) {
...@@ -72,9 +72,37 @@ function checkParams(params) { ...@@ -72,9 +72,37 @@ function checkParams(params) {
}; };
} }
// log a user out, clearing everything from their session except the csrf token
function clearAuthenticatedUser(session) {
Object.keys(session).forEach(function(k) {
if (k !== 'csrf') delete session[k];
});
}
function setAuthenticatedUser(session, email) {
session.authenticatedUser = email;
session.authenticatedAt = new Date();
}
function isAuthed(req) { function isAuthed(req) {
var result= (req.session && typeof req.session.authenticatedUser === 'string'); var who;
return result; try {
if (req.session.authenticatedUser) {
if (!Date.parse(req.session.authenticatedAt) > 0) throw "bad timestamp";
if (new Date() - new Date(req.session.authenticatedAt) >
configuration.get('authentication_duration_ms'))
{
throw "expired";
}
who = req.session.authenticatedUser;
}
} catch(e) {
logger.debug("Session authentication has expired:", e);
clearAuthenticatedUser(req.session);
}
return who;
} }
// turned this into a proper middleware // turned this into a proper middleware
...@@ -87,13 +115,6 @@ function checkAuthed(req, resp, next) { ...@@ -87,13 +115,6 @@ function checkAuthed(req, resp, next) {
} }
function setup(app) { function setup(app) {
// log a user out, clearing everything from their session except the csrf token
function clearAuthenticatedUser(session) {
Object.keys(session).forEach(function(k) {
if (k !== 'csrf') delete session[k];
});
}
// return the CSRF token // return the CSRF token
// IMPORTANT: this should be safe because it's only readable by same-origin code // IMPORTANT: this should be safe because it's only readable by same-origin code
// but we must be careful that this is never a JSON structure that could be hijacked // but we must be careful that this is never a JSON structure that could be hijacked
...@@ -142,7 +163,7 @@ function setup(app) { ...@@ -142,7 +163,7 @@ function setup(app) {
} }
// bcrypt the password // bcrypt the password
bcrypt.gen_salt(BCRYPT_WORK_FACTOR, function (err, salt) { bcrypt.gen_salt(configuration.get('bcrypt_work_factor'), function (err, salt) {
if (err) { if (err) {
winston.error("error generating salt with bcrypt: " + err); winston.error("error generating salt with bcrypt: " + err);
return resp.json(false); return resp.json(false);
...@@ -222,7 +243,7 @@ function setup(app) { ...@@ -222,7 +243,7 @@ function setup(app) {
db.checkAuth(v.email, function(hash) { db.checkAuth(v.email, function(hash) {
if (hash === v.hash) { if (hash === v.hash) {
delete req.session.pendingVerification; delete req.session.pendingVerification;
req.session.authenticatedUser = v.email; setAuthenticatedUser(req.session, v.email);
resp.json('complete'); resp.json('complete');
} else { } else {
resp.json('pending'); resp.json('pending');
...@@ -247,7 +268,7 @@ function setup(app) { ...@@ -247,7 +268,7 @@ function setup(app) {
} }
if (success) { if (success) {
if (!req.session) req.session = {}; if (!req.session) req.session = {};
req.session.authenticatedUser = req.body.email; setAuthenticatedUser(req.session, req.body.email);
// if the work factor has changed, update the hash here // if the work factor has changed, update the hash here
...@@ -309,7 +330,7 @@ function setup(app) { ...@@ -309,7 +330,7 @@ function setup(app) {
// same account, we certify the key // same account, we certify the key
// we certify it for a day for now // we certify it for a day for now
var expiration = new Date(); var expiration = new Date();
expiration.setTime(new Date().valueOf() + (24*3600*1000)); expiration.setTime(new Date().valueOf() + configuration.get('certificate_validity_ms'));
var cert = ca.certify(req.body.email, pk, expiration); var cert = ca.certify(req.body.email, pk, expiration);
resp.writeHead(200, {'Content-Type': 'text/plain'}); resp.writeHead(200, {'Content-Type': 'text/plain'});
......
...@@ -70,7 +70,10 @@ g_configs.production = { ...@@ -70,7 +70,10 @@ g_configs.production = {
database: { database: {
driver: "mysql", driver: "mysql",
user: 'browserid' user: 'browserid'
} },
bcrypt_work_factor: 12,
authentication_duration_ms: (7 * 24 * 60 * 60 * 1000),
certificate_validity_ms: (24 * 60 * 60 * 1000)
}; };
// beta (diresworb.org) the only difference from production // beta (diresworb.org) the only difference from production
...@@ -91,7 +94,10 @@ g_configs.local = { ...@@ -91,7 +94,10 @@ g_configs.local = {
email_to_console: true, // don't send email, just dump verification URLs to console. email_to_console: true, // don't send email, just dump verification URLs to console.
use_minified_resources: false, use_minified_resources: false,
var_path: path.join(__dirname, "..", "var"), var_path: path.join(__dirname, "..", "var"),
database: { driver: "json" } database: { driver: "json" },
bcrypt_work_factor: g_configs.production.bcrypt_work_factor,
authentication_duration_ms: g_configs.production.authentication_duration_ms,
certificate_validity_ms: g_configs.production.certificate_validity_ms
}; };
// test environments are variations on local // test environments are variations on local
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment