From 659c55acf51e1748dcc272c17c2a923a7707d0a0 Mon Sep 17 00:00:00 2001
From: Ben Adida <ben@adida.net>
Date: Sun, 17 Jul 2011 13:36:40 -0700
Subject: [PATCH] started adding CSRF protection, but not enabled yet since
tests are not set up to do that properly
---
README.md | 1 +
browserid/app.js | 33 +++++++++---
browserid/lib/wsapi.js | 2 +-
.../dialog/controllers/dialog_controller.js | 2 +-
browserid/tests/forgotten-email-test.js | 6 +++
browserid/tests/lib/wsapi.js | 52 +++++++++++++++++++
6 files changed, 86 insertions(+), 10 deletions(-)
diff --git a/README.md b/README.md
index 1ac3fe761..c6372048f 100644
--- a/README.md
+++ b/README.md
@@ -15,6 +15,7 @@ All of the servers here are based on node.js, and some number of 3rd party node
* vows (>= 0.5.8)
* bcrypt (>= 0.2.3)
* ejs (>= 0.4.3)
+* express-csrf (>= 0.3.2)
## Getting started:
diff --git a/browserid/app.js b/browserid/app.js
index c685e4915..62193ab38 100644
--- a/browserid/app.js
+++ b/browserid/app.js
@@ -5,14 +5,16 @@ const fs = require('fs'),
var VAR_DIR = path.join(__dirname, "var");
try { fs.mkdirSync(VAR_DIR, 0755); } catch(e) { };
-const url = require('url'),
- 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');
+const
+ url = require('url'),
+ 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');
// looks unused, see run.js
// const STATIC_DIR = path.join(path.dirname(__dirname), "static");
@@ -41,6 +43,7 @@ function router(app) {
app.get('/register_iframe', internal_redirector('/dialog/register_iframe.html'));
app.get('/', function(req,res) {
+ console.log("CSRF: " + req.session.csrf);
res.render('index.ejs', {title: 'A Better Way to Sign In', fullpage: true});
});
@@ -114,6 +117,13 @@ exports.setup = function(server) {
server.use(express.bodyParser());
+ // we make sure that everyone has a session, otherwise we can't do CSRF properly
+ server.use(function(req, resp, next) {
+ if (typeof req.session == 'undefined')
+ req.session = {};
+ next();
+ });
+
// a tweak to get the content type of host-meta correct
server.use(function(req, resp, next) {
if (req.url === '/.well-known/host-meta') {
@@ -128,6 +138,13 @@ exports.setup = function(server) {
next();
});
+ // setup CSRF protection
+ //server.use(csrf.check());
+
+ server.dynamicHelpers({
+ csrf: csrf.token
+ });
+
// add the actual URL handlers other than static
router(server);
}
diff --git a/browserid/lib/wsapi.js b/browserid/lib/wsapi.js
index 53c2096c4..57bccb035 100644
--- a/browserid/lib/wsapi.js
+++ b/browserid/lib/wsapi.js
@@ -214,7 +214,7 @@ function setup(app) {
}
});
- app.get('/wsapi/logout', function(req,resp) {
+ app.post('/wsapi/logout', function(req,resp) {
req.session = {};
httputils.jsonResponse(resp, "ok");
});
diff --git a/browserid/static/dialog/controllers/dialog_controller.js b/browserid/static/dialog/controllers/dialog_controller.js
index 59b9e2b38..fe771e70f 100644
--- a/browserid/static/dialog/controllers/dialog_controller.js
+++ b/browserid/static/dialog/controllers/dialog_controller.js
@@ -116,7 +116,7 @@ $.Controller("Dialog", {}, {
"#notme click": function(event) {
clearEmails();
var self = this;
- $.get("/wsapi/logout", function() {
+ $.post("/wsapi/logout", function() {
self.doAuthenticate();
});
},
diff --git a/browserid/tests/forgotten-email-test.js b/browserid/tests/forgotten-email-test.js
index ecd259c8e..469978e75 100755
--- a/browserid/tests/forgotten-email-test.js
+++ b/browserid/tests/forgotten-email-test.js
@@ -159,6 +159,12 @@ suite.addBatch({
assert.strictEqual(JSON.parse(r.body), true);
}
},
+ "logout": {
+ topic: wsapi.post('/wsapi/logout', {}),
+ "should work": function(r, err) {
+ assert.strictEqual(JSON.parse(r.body), "ok");
+ }
+ },
"second email, first pass good": {
topic: wsapi.get('/wsapi/authenticate_user', { email: 'second@fakeemail.com', pass: 'firstfakepass' }),
"should work": function(r, err) {
diff --git a/browserid/tests/lib/wsapi.js b/browserid/tests/lib/wsapi.js
index f4ff2c051..2a15fc7b9 100644
--- a/browserid/tests/lib/wsapi.js
+++ b/browserid/tests/lib/wsapi.js
@@ -46,3 +46,55 @@ exports.get = function (path, getArgs) {
});
};
};
+
+// FIXME: dedup code
+
+// A macro for wsapi requests
+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'
+ };
+
+ if (Object.keys(cookieJar).length) {
+ headers['Cookie'] = "";
+ for (var k in cookieJar) {
+ headers['Cookie'] += k + "=" + cookieJar[k];
+ }
+ }
+
+ 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();
+ };
+};
--
GitLab