diff --git a/browserid/app.js b/browserid/app.js
index 62193ab387997950e3e757946f2debe417b7678e..968a262120bc9a79ab92f2a88c7be10135c74b56 100644
--- a/browserid/app.js
+++ b/browserid/app.js
@@ -7,14 +7,14 @@ try { fs.mkdirSync(VAR_DIR, 0755); } catch(e) { };
const
url = require('url'),
+ crypto = require('crypto'),
wsapi = require('./lib/wsapi.js'),
httputils = require('./lib/httputils.js'),
webfinger = require('./lib/webfinger.js'),
sessions = require('cookie-sessions'),
express = require('express'),
secrets = require('./lib/secrets.js'),
- db = require('./lib/db.js'),
- csrf = require('express-csrf');
+ db = require('./lib/db.js')
// looks unused, see run.js
// const STATIC_DIR = path.join(path.dirname(__dirname), "static");
@@ -42,8 +42,16 @@ function router(app) {
// simple redirects (internal for now)
app.get('/register_iframe', internal_redirector('/dialog/register_iframe.html'));
+ // 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
+ // by a third party
+ app.get('/csrf', function(req, res) {
+ res.write(req.session.csrf);
+ res.end();
+ });
+
app.get('/', function(req,res) {
- console.log("CSRF: " + req.session.csrf);
res.render('index.ejs', {title: 'A Better Way to Sign In', fullpage: true});
});
@@ -64,7 +72,7 @@ function router(app) {
});
app.get('/manage', function(req,res) {
- res.render('manage.ejs', {title: 'My Account', fullpage: false});
+ res.render('manage.ejs', {title: 'My Account', fullpage: false, csrf: req.session.csrf});
});
app.get('/tos', function(req, res) {
@@ -121,6 +129,13 @@ exports.setup = function(server) {
server.use(function(req, resp, next) {
if (typeof req.session == 'undefined')
req.session = {};
+
+ if (typeof req.session.csrf == 'undefined') {
+ // FIXME: using express-csrf's approach for generating randomness
+ // not awesome, but probably sufficient for now.
+ req.session.csrf = crypto.createHash('md5').update('' + new Date().getTime()).digest('hex');
+ }
+
next();
});
@@ -138,13 +153,18 @@ exports.setup = function(server) {
next();
});
- // setup CSRF protection
- //server.use(csrf.check());
-
- server.dynamicHelpers({
- csrf: csrf.token
- });
-
+ // check CSRF token
+ server.use(function(req, resp, next) {
+ // only on POSTs
+ if (req.method == "POST") {
+ if (req.body.csrf != req.session.csrf) {
+ // error, problem with CSRF
+ throw new Error("CSRF violation - " + req.body.csrf + '/' + req.session.csrf);
+ }
+ }
+
+ next();
+ });
// add the actual URL handlers other than static
router(server);
}
diff --git a/browserid/lib/wsapi.js b/browserid/lib/wsapi.js
index 57bccb035e72245d1b78649cf006a3017d9f4587..9e7cfd4b349629730df5e8cde32d911284e70bdd 100644
--- a/browserid/lib/wsapi.js
+++ b/browserid/lib/wsapi.js
@@ -33,7 +33,10 @@ function checkParams(params) {
}
function isAuthed(req) {
- return (req.session && typeof req.session.authenticatedUser === 'string');
+ var result= (req.session && typeof req.session.authenticatedUser === 'string');
+ if (!result && req.session)
+ console.log("AUTH" + req.session.authenticatedUser);
+ return result;
}
// turned this into a proper middleware
@@ -153,20 +156,19 @@ function setup(app) {
});
});
- // FIXME: need CSRF protection
- app.get('/wsapi/add_email', checkAuthed, checkParams(["email", "pubkey", "site"]), function (req, resp) {
+ app.post('/wsapi/add_email', checkAuthed, checkParams(["email", "pubkey", "site"]), function (req, resp) {
try {
// upon success, stage_user returns a secret (that'll get baked into a url
// and given to the user), on failure it throws
- var secret = db.stageEmail(req.session.authenticatedUser, req.query.email, req.query.pubkey);
+ var secret = db.stageEmail(req.session.authenticatedUser, req.body.email, req.body.pubkey);
// store the email being added in session data
- req.session.pendingAddition = req.query.email;
+ req.session.pendingAddition = req.body.email;
httputils.jsonResponse(resp, true);
// let's now kick out a verification email!
- email.sendVerificationEmail(req.query.email, req.query.site, secret);
+ email.sendVerificationEmail(req.body.email, req.body.site, secret);
} catch(e) {
// we should differentiate tween' 400 and 500 here.
httputils.badRequest(resp, e.toString());
@@ -220,7 +222,7 @@ function setup(app) {
});
app.post('/wsapi/sync_emails', checkAuthed, function(req,resp) {
- var emails = req.body;
+ var emails = req.body.emails;
db.getSyncResponse(req.session.authenticatedUser, emails, function(err, syncResponse) {
if (err) httputils.serverError(resp, err);
diff --git a/browserid/static/dialog/controllers/dialog_controller.js b/browserid/static/dialog/controllers/dialog_controller.js
index fe771e70f1ad53f078c7d436bf645e6b1ea0c81f..d9e49b1a6357805d4573aa3732428ccc76d8f0ab 100644
--- a/browserid/static/dialog/controllers/dialog_controller.js
+++ b/browserid/static/dialog/controllers/dialog_controller.js
@@ -7,13 +7,27 @@
$.Controller("Dialog", {}, {
init: function(el) {
+ // FIXME: there's a small chance the CSRF token doesn't come back in time.
+ this.csrf = null;
+ this._getCSRF();
+
this.element.html("views/body.ejs", {});
this.element.show();
// keep track of where we are and what we do on success and error
this.onsuccess = null;
this.onerror = null;
- var chan = setupChannel(this);
+ var chan = setupChannel(this);
+ },
+
+ _getCSRF: function(cb) {
+ // go get CSRF token
+ var self = this;
+ $.get('/csrf', {}, function(result) {
+ self.csrf = result;
+ if (cb)
+ cb();
+ });
},
setupEnterKey: function() {
@@ -97,9 +111,14 @@ $.Controller("Dialog", {}, {
);
$.ajax({
- url: '/wsapi/add_email?email=' + encodeURIComponent(email)
- + '&pubkey=' + encodeURIComponent(keypair.pub)
- + '&site=' + encodeURIComponent(this.remoteOrigin.replace(/^(http|https):\/\//, '')),
+ type: 'POST',
+ url: '/wsapi/add_email',
+ data: {
+ email: email,
+ pubkey: keypair.pub,
+ site: this.remoteOrigin.replace(/^(http|https):\/\//, ''),
+ csrf: this.csrf
+ },
success: function() {
// email successfully staged, now wait for email confirmation
self.doConfirmEmail(email, keypair);
@@ -116,8 +135,10 @@ $.Controller("Dialog", {}, {
"#notme click": function(event) {
clearEmails();
var self = this;
- $.post("/wsapi/logout", function() {
- self.doAuthenticate();
+ $.post("/wsapi/logout", {csrf: this.csrf}, function() {
+ self._getCSRF(function() {
+ self.doAuthenticate();
+ });
});
},
@@ -435,7 +456,8 @@ $.Controller("Dialog", {}, {
$.ajax({
url: '/wsapi/sync_emails',
type: "post",
- data: JSON.stringify(issued_identities),
+ data: {'emails': JSON.stringify(issued_identities),
+ 'csrf': this.csrf},
success: function(resp, textStatus, jqXHR) {
// first remove idenitites that the server doesn't know about
if (resp.unknown_emails) {
diff --git a/browserid/tests/forgotten-email-test.js b/browserid/tests/forgotten-email-test.js
index 469978e75dd25862236dae90ff16a3de6e8afb5c..41375976f33d7eaa8e745702440a86d2329a5cbd 100755
--- a/browserid/tests/forgotten-email-test.js
+++ b/browserid/tests/forgotten-email-test.js
@@ -13,7 +13,7 @@ start_stop.addStartupBatches(suite);
// ever time a new token is sent out, let's update the global
// var 'token'
var token = undefined;
-interceptor.onEmail = function(newtok) { token = newtok; }
+interceptor.onEmail = function(newtok) { token = newtok; };
// create a new account via the api with (first address)
suite.addBatch({
@@ -54,7 +54,7 @@ suite.addBatch({
// add a new email address to the account (second address)
suite.addBatch({
"add a new email address to our account": {
- topic: wsapi.get('/wsapi/add_email', {
+ topic: wsapi.post('/wsapi/add_email', {
email: 'second@fakeemail.com',
pubkey: 'fakepubkey',
site:'fakesite.com'
@@ -159,9 +159,17 @@ suite.addBatch({
assert.strictEqual(JSON.parse(r.body), true);
}
},
+ "sync emails": {
+ topic: wsapi.post('/wsapi/sync_emails', {'emails': '{}'}),
+ "should work" : function(r, err) {
+ var parsed_body = JSON.parse(r.body);
+ assert.equal(typeof parsed_body.unknown_emails, "object");
+ assert.equal(typeof parsed_body.key_refresh, "object");
+ }
+ },
"logout": {
topic: wsapi.post('/wsapi/logout', {}),
- "should work": function(r, err) {
+ "should work": function(r, err) {
assert.strictEqual(JSON.parse(r.body), "ok");
}
},
diff --git a/browserid/tests/lib/wsapi.js b/browserid/tests/lib/wsapi.js
index 2a15fc7b99c3ce44d0fa29735627109c06ca0303..f6ba8b174b764d43025c29580e51924b55d0dfe4 100644
--- a/browserid/tests/lib/wsapi.js
+++ b/browserid/tests/lib/wsapi.js
@@ -47,14 +47,32 @@ exports.get = function (path, getArgs) {
};
};
+// fetch the CSRF token for POSTs
+var fetchCSRF = function(headers, cb) {
+ return http.get({
+ host: '127.0.0.1',
+ port: '62700',
+ path: '/csrf',
+ headers: headers
+ }, function(res) {
+ var body = "";
+ res.on('data', function(chunk) { body += chunk; })
+ .on('end', function() {
+ cb(body);
+ });
+ }).on('error', function (e) {
+ cb();
+ });
+};
+
+
// FIXME: dedup code
// A macro for wsapi requests
+// add CSRF protection to this test
exports.post = function (path, postArgs) {
return function () {
var cb = this.callback;
- if (typeof postArgs === 'object')
- body = querystring.stringify(postArgs);
var headers = {
'content-type': 'application/x-www-form-urlencoded'
@@ -67,34 +85,41 @@ exports.post = function (path, postArgs) {
}
}
- var req = http.request({
- host: '127.0.0.1',
- port: '62700',
- path: path,
- headers: headers,
- method: "POST"
- }, function(res) {
- // see if there are any set-cookies that we should honor
- if (res.headers['set-cookie']) {
- res.headers['set-cookie'].forEach(function(cookie) {
- var m = /^([^;]+)(?:;.*)$/.exec(cookie);
- if (m) {
- var x = m[1].split('=');
- cookieJar[x[0]] = x[1];
- }
- });
+ fetchCSRF(headers, function(csrf) {
+ if (typeof postArgs === 'object') {
+ postArgs['csrf'] = csrf;
+ body = querystring.stringify(postArgs);
}
- var body = '';
- res.on('data', function(chunk) { body += chunk; })
- .on('end', function() {
- cb({code: res.statusCode, headers: res.headers, body: body});
- });
- }).on('error', function (e) {
- cb();
- });
- // send the POST
- req.write(body);
- req.end();
+ var req = http.request({
+ host: '127.0.0.1',
+ port: '62700',
+ path: path,
+ headers: headers,
+ method: "POST"
+ }, function(res) {
+ // see if there are any set-cookies that we should honor
+ if (res.headers['set-cookie']) {
+ res.headers['set-cookie'].forEach(function(cookie) {
+ var m = /^([^;]+)(?:;.*)$/.exec(cookie);
+ if (m) {
+ var x = m[1].split('=');
+ cookieJar[x[0]] = x[1];
+ }
+ });
+ }
+ var body = '';
+ res.on('data', function(chunk) { body += chunk; })
+ .on('end', function() {
+ cb({code: res.statusCode, headers: res.headers, body: body});
+ });
+ }).on('error', function (e) {
+ cb();
+ });
+
+ // send the POST
+ req.write(body);
+ req.end();
+ });
};
};
diff --git a/browserid/views/layout.ejs b/browserid/views/layout.ejs
index 1ae2936f35a5f283b92d5e0d15f5e396055efc97..e396a3811932294a9f62ccd70b13d27f6d7c06bc 100644
--- a/browserid/views/layout.ejs
+++ b/browserid/views/layout.ejs
@@ -24,7 +24,7 @@ function display_saved_ids()
$('#cancellink').click(function() {
if (confirm('Are you sure you want to cancel your account?')) {
- $.post("/wsapi/account_cancel", {}, function(result) {
+ $.post("/wsapi/account_cancel", {csrf: window.csrf}, function(result) {
window.localStorage.emails = null;
document.location="/";
});
@@ -58,7 +58,7 @@ function display_saved_ids()
deauth.click(function() {
var t = JSON.parse(window.localStorage.emails);
// remove email from server
- $.post("/wsapi/remove_email", {"email" : e}, function(response) {
+ $.post("/wsapi/remove_email", {"email" : e, "csrf": window.csrf}, function(response) {
// we delete from store only once we got response
delete t[e];
window.localStorage.emails = JSON.stringify(t);
diff --git a/browserid/views/manage.ejs b/browserid/views/manage.ejs
index f48ac42d97b917162a7bc7338a9795d4f5e8bcbf..8ca5046c78132eefc349b7b3878cd9c45a6362d2 100644
--- a/browserid/views/manage.ejs
+++ b/browserid/views/manage.ejs
@@ -1,3 +1,6 @@
+ <script>
+window.csrf = "<%= csrf %>";
+ </script>
<div class="why">
<p>
Manage your email addresses in BrowserID.