From 1f7461e95c818950eb66901aaf547a89ad3a0c58 Mon Sep 17 00:00:00 2001
From: Lloyd Hilaiel <lloyd@hilaiel.com>
Date: Thu, 29 Sep 2011 09:59:55 -0700
Subject: [PATCH] ensure users must authenticate every week (issue #309).
Also, move all magic numbers out of implementation and into the configuration
abstraction.
---
browserid/app.js | 2 +-
browserid/lib/db_json.js | 5 ----
browserid/lib/wsapi.js | 49 ++++++++++++++++++++++++++++------------
libs/configuration.js | 10 ++++++--
4 files changed, 44 insertions(+), 22 deletions(-)
diff --git a/browserid/app.js b/browserid/app.js
index 96e9dad9d..5ad05ab01 100644
--- a/browserid/app.js
+++ b/browserid/app.js
@@ -183,7 +183,7 @@ exports.setup = function(server) {
httpOnly: true,
// IMPORTANT: we allow users to go 1 weeks on the same device
// without entering their password again
- maxAge: (7 * 24 * 60 * 60 * 1000),
+ maxAge: configuration.get('authentication_duration_ms'),
secure: overSSL
}
});
diff --git a/browserid/lib/db_json.js b/browserid/lib/db_json.js
index 2a2c21859..42c6ecc49 100644
--- a/browserid/lib/db_json.js
+++ b/browserid/lib/db_json.js
@@ -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
// ephemeral. which simply means we use a temp file and delete
// on close;
diff --git a/browserid/lib/wsapi.js b/browserid/lib/wsapi.js
index 93c570f1b..602b30fa9 100644
--- a/browserid/lib/wsapi.js
+++ b/browserid/lib/wsapi.js
@@ -47,7 +47,7 @@ bcrypt = require('bcrypt'),
crypto = require('crypto'),
logger = require('../../libs/logging.js').logger,
ca = require('./ca.js'),
-BCRYPT_WORK_FACTOR = 12;
+configuration = require('../../libs/configuration.js');
function checkParams(params) {
return function(req, resp, next) {
@@ -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) {
- var result= (req.session && typeof req.session.authenticatedUser === 'string');
- return result;
+ var who;
+ 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
@@ -87,13 +115,6 @@ function checkAuthed(req, resp, next) {
}
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
// 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
@@ -142,7 +163,7 @@ function setup(app) {
}
// 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) {
winston.error("error generating salt with bcrypt: " + err);
return resp.json(false);
@@ -222,7 +243,7 @@ function setup(app) {
db.checkAuth(v.email, function(hash) {
if (hash === v.hash) {
delete req.session.pendingVerification;
- req.session.authenticatedUser = v.email;
+ setAuthenticatedUser(req.session, v.email);
resp.json('complete');
} else {
resp.json('pending');
@@ -247,7 +268,7 @@ function setup(app) {
}
if (success) {
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
@@ -309,7 +330,7 @@ function setup(app) {
// same account, we certify the key
// we certify it for a day for now
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);
resp.writeHead(200, {'Content-Type': 'text/plain'});
diff --git a/libs/configuration.js b/libs/configuration.js
index 1f6b99385..b8b34ff53 100644
--- a/libs/configuration.js
+++ b/libs/configuration.js
@@ -70,7 +70,10 @@ g_configs.production = {
database: {
driver: "mysql",
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
@@ -91,7 +94,10 @@ g_configs.local = {
email_to_console: true, // don't send email, just dump verification URLs to console.
use_minified_resources: false,
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
--
GitLab