Permalink
Browse files

Added anti-CSRF protection.

  • Loading branch information...
1 parent 7d3b22d commit 324fbde60c1c390474191450aa88db54460de6ed @mde mde committed Nov 24, 2011
View
@@ -170,6 +170,22 @@ namespace('gen', function () {
jake.Task['gen:views'].invoke(name, {bare: true});
});
+ task('secret', [], function (name) {
+ var filename = process.cwd() + '/config/environment.js'
+ , conf = fs.readFileSync(filename).toString()
+ , confArr
+ , secret = utils.string.uuid(128);
+
+ // Remove any old secret
+ conf = conf.replace(/\nconfig.secret.+;\n/, '');
+
+ confArr = conf.split('module.exports = config;');
+ conf = confArr[0] + "config.secret = '" + secret + "';\n\n" +
+ 'module.exports = config;' + confArr[1];
+ fs.writeFileSync(filename, conf);
+ console.log('secret added to environment.js config.');
+ });
+
});
namespace('doc', function () {
View
@@ -105,18 +105,21 @@ else {
// `geddy app foo` or `geddy resource bar` etc. -- run generators
if (cmds.length) {
cmd = 'jake -t -f /' + __dirname + '/../Jakefile ';
- if (!cmds[1]) {
+ if (cmds[0] != 'secret' && !cmds[1]) {
throw new Error(cmds[0] + ' command requires another argument.');
}
switch (cmds[0]) {
case 'app':
- cmd += 'gen:app[' + cmds[1] + ']'
+ cmd += 'gen:app[' + cmds[1] + ']';
break;
case 'resource':
- cmd += 'gen:resource[' + cmds[1] + ']'
+ cmd += 'gen:resource[' + cmds[1] + ']';
break;
case 'controller':
- cmd += 'gen:bareController[' + cmds[1] + ']'
+ cmd += 'gen:bareController[' + cmds[1] + ']';
+ break;
+ case 'secret':
+ cmd += 'gen:secret';
break;
default:
Server.die(cmds[0] + ' is not a Geddy command.');
View
@@ -16,6 +16,7 @@
*
*/
var fs = require('fs')
+ , crypto = require('crypto')
, errors = require('./errors')
, response = require('./response')
, Templater = require('./template/adapters/ejs').Templater
@@ -117,7 +118,10 @@ controller.BaseController = function () {
// The template layout directory to look in when rendering templates
// Gets created programmatically based on controller name -- see renderTemplate
this.layout = null;
-
+ // Time accessed
+ this.accessTime = null;
+ // Anti-CSRF token for PUT/POST/DELETE
+ this.sameOriginToken = null;
};
/**
@@ -397,6 +401,9 @@ controller.BaseController.prototype = new (function () {
};
if (this.session) {
+ // Save to accessTime into session for verifying sameOriginToken
+ this.session.set('accessTime', this.accessTime);
+
this.session.close(callback);
}
else {
@@ -408,6 +415,46 @@ controller.BaseController.prototype = new (function () {
err = new errors.InternalServerError(
'Format not defined in response.formats.');
this.error(err);
+ }
+
+ , _generateSameOriginToken = function (t) {
+ var time = t || this.accessTime
+ , sha = crypto.createHash('sha1');
+ sha.update(geddy.config.secret);
+ sha.update(this.session.id);
+ sha.update(time.toString());
+ return sha.digest('hex');
+ }
+
+ , _protectFromForgery = function (complete) {
+ var methods = {
+ PUT: true
+ , POST: true
+ , DELETE: true
+ }
+ , lastAccess = this.session.get('accessTime')
+ , params = this.params
+ , token = params.same_origin_token || params.sameOriginToken
+ , forbidden = false;
+
+ if (methods[this.method]) {
+ if (!token) {
+ forbidden = true;
+ }
+ else {
+ if (_generateSameOriginToken.call(this, lastAccess) != token) {
+ forbidden = true;
+ }
+ }
+ }
+ if (forbidden) {
+ err = new errors.ForbiddenError(
+ 'Cross-site request not allowed.');
+ this.error(err);
+ }
+ else {
+ complete();
+ }
};
// Pseudo-private, non-API
@@ -425,6 +472,12 @@ controller.BaseController.prototype = new (function () {
self[action].apply(self, [self.request, self.response, self.params]);
}
};
+
+ // Generate the anti-CSRF token
+ if (geddy.config.secret && this.session) {
+ this.sameOriginToken = _generateSameOriginToken.call(this);
+ }
+
// Running filters asynchronously breaks handlers that depend on
// setting listeners on the request before the next tick -- only
// run them if necessary
@@ -472,6 +525,22 @@ controller.BaseController.prototype = new (function () {
_addFilter.apply(this, ['after', filter, options || {}]);
};
+ this.protectFromForgery = function (options) {
+ var opts = options || {};
+ if (!geddy.config.secret) {
+ geddy.log.warning('protectFromForgery requires an app-secret. ' +
+ 'Run `geddy secret` in your app.');
+ }
+ if (!this.session) {
+ geddy.log.warning('protectFromForgery requires sessions.');
+ }
+ if (typeof opts.async != 'undefined' && !opts.async) {
+ geddy.log.warning('protectFromForgery requires the async flag set to true.');
+ }
+ opts.async = true;
+ this.before(_protectFromForgery, opts);
+ };
+
/**
@name controller.BaseController#redirect
@public
@@ -515,6 +584,7 @@ controller.BaseController.prototype = new (function () {
@param {Object} err The error to use as the basis for the response.
*/
this.error = function (err) {
+ this.completed = true;
errors.respond(this.response, err);
};
View
@@ -68,7 +68,7 @@ cookies.Cookie = function (name, value, o) {
var opts = o || {};
this.name = name;
this.value = value;
- this.path = opts.path || null;
+ this.path = opts.path || '/';
this.expires = opts.expires || null;
this.domain = opts.domain || null;
this.httpOnly = opts.httpOnly || false;
View
@@ -23,6 +23,7 @@ var errors = new function () {
this.errorTypes = {
400: 'Bad Request',
401: 'Unauthorized',
+ 403: 'Forbidden',
404: 'Not Found',
406: 'Not Acceptable',
500: 'Internal Server Error'
View
@@ -238,6 +238,7 @@ geddy.mixin(geddy, new (function () {
this.server.addListener('request', function (req, resp) {
var params
, urlParams
+ , method
, ctor
, appCtor
, baseController
@@ -252,7 +253,8 @@ geddy.mixin(geddy, new (function () {
parseBody: false
, sessions: false
}
- , finish;
+ , finish
+ , accessTime = new Date();
finish = function (step) {
steps[step] = true;
@@ -261,30 +263,50 @@ geddy.mixin(geddy, new (function () {
return false;
}
}
-
+ controller.accessTime = accessTime.getTime();
controller._handleAction.call(controller, params.action);
};
//TODO: get better logs (including http status codes)
// by wrapping serverResponse.end()
req.addListener('end', function () {
self.log.access(req.connection.remoteAddress +
- " " + new Date() + " " + req.method + " " + req.url);
+ " " + accessTime + " " + req.method + " " + req.url);
});
self.requestTime = (new Date()).getTime();
if (router) {
- params = router.first(req);
+ urlParams = url.parse(req.url, true).query;
+
+ if (req.method.toUpperCase() == 'POST') {
+ // POSTs may be overridden by the _method param
+ if (urlParams._method) {
+ method = urlParams._method;
+ }
+ // Or x-http-method-override header
+ else if (req.headers['x-http-method-override']) {
+ method = req.headers['x-http-method-override'];
+ }
+ else {
+ method = req.method;
+ }
+ }
+ else {
+ method = req.method;
+ }
+ // Okay, let's be anal and force all the HTTP verbs to uppercase
+ method = method.toUpperCase();
+
+ params = router.first(req, method);
}
if (params) {
ctor = ctors[params.controller];
if (ctor) {
-
- // Parses form input, and merges it with params from
- // the URL and the query-string to produce a Grand Unified Params object
- urlParams = url.parse(req.url, true).query
+ // Merge params from the URL and the query-string to produce a
+ // Grand Unified Params object
geddy.mixin(params, urlParams);
+
// If it's a plain form-post, save the request-body, and parse it into
// params as well
if ((req.method == 'POST' || req.method == 'PUT') &&
@@ -320,6 +342,7 @@ geddy.mixin(geddy, new (function () {
if (typeof controller[params.action] == 'function') {
controller.request = req;
controller.response = resp;
+ controller.method = method;
controller.params = params;
controller.name = params.controller;
@@ -54,7 +54,9 @@ BaseRouter.prototype = new (function () {
// Iterate through your routes and return the controller/action
// corresponding to the matched route
- this.first = function (req) {
+ // 'method' is passed separately because it may be an overridden
+ // POST in crappier clients
+ this.first = function (req, method) {
var params = url.parse(req.url, true);
params.controller = null;
params.action = null;
@@ -27,7 +27,7 @@ var FunctionRouter = function (routes) {
FunctionRouter.prototype = new BaseRouter();
-FunctionRouter.prototype.first = function (req) {
+FunctionRouter.prototype.first = function (req, method) {
var routes = this.routes
, route
, handler
@@ -112,10 +112,9 @@ var Router = function () {
*
* if there's no match, this will return false
*/
- this.first = function (req) {
+ this.first = function (req, method) {
var route = false
- , path = req.url
- , method = req.method;
+ , path = req.url;
for (var i in this.routes) {
// if the method doesn't match: jog on
if (typeof(this.routes[i].method) != 'undefined' && this.routes[i].method != method) {
View
@@ -21,19 +21,6 @@ var session = new (function () {
this.store = null;
- this.generateSessionId = function () {
- var chars = '0123456789ABCDEFGHIJKLMNOPQRSTUVWXTZabcdefghiklmnopqrstuvwxyz';
- var len = KEY_LENGTH;
- var str = '';
- var mls = new Date().getTime();
- for (var i = 0; i < len; i++) {
- var rnum = (Math.random() * chars.length);
- rnum = Math.floor(rnum);
- str += chars.substring(rnum, rnum + 1);
- }
- return str;
- };
-
this.createStore = function (type, callback) {
var key = geddy.string.capitalize(type);
var constructor = require('./stores/' + type)[key];
@@ -48,23 +35,28 @@ session.Session = function (controller, callback) {
, keyName = geddy.config.sessions.key
, sid = cookies.get(keyName);
- if (!sid) {
- sid = session.generateSessionId()
- var dt = new Date();
- dt.setTime(dt.getTime() + (geddy.config.sessions.expiry * 1000));
- cookies.set(keyName, sid, {expires: dt.toGMTString()});
- }
-
- this.sid = sid;
+ this.id = null;
this.controller = controller;
this.data = null;
+ this.setId(sid);
+
setTimeout(function () {
self.init(callback);
}, 0);
};
session.Session.prototype = new function () {
+ this.setId = function (s) {
+ var sid = s || geddy.string.uuid(128)
+ , cookies = this.controller.cookies
+ , keyName = geddy.config.sessions.key
+ , dt = new Date();
+ dt.setTime(dt.getTime() + (geddy.config.sessions.expiry * 1000));
+ cookies.set(keyName, sid, {expires: dt.toGMTString()});
+ this.id = sid;
+ };
+
this.init = function(appCallback) {
var _this = this;
var localCallback = function (result) {
@@ -74,6 +66,10 @@ session.Session.prototype = new function () {
session.store.read(this, localCallback);
};
+ this.reset = function () {
+ this.setId();
+ };
+
this.get = function (key) {
return this.data[key];
};
@@ -44,7 +44,7 @@ Couchdb.prototype = new function () {
};
this.read = function (session, callback) {
- var sid = session.sid;
+ var sid = session.id;
_appCallback = callback;
this.request({url: '/' + geddy.config.sessions.dbName +
'/' + sid, method: 'GET'}, this.ensureRead);
@@ -92,7 +92,7 @@ Couchdb.prototype = new function () {
};
this.write = function (session, callback) {
- var sid = session.sid
+ var sid = session.id
, data = session.data;
this.request({url: '/' + geddy.config.sessions.dbName +
'/' + sid, method: 'PUT', data: data}, this.ensureUpdate);
@@ -28,7 +28,7 @@ Memory.prototype = new (function () {
};
this.read = function (session, callback) {
- var sid = session.sid;
+ var sid = session.id;
if (!_sessions[sid]) {
_sessions[sid] = {};
}
Oops, something went wrong.

2 comments on commit 324fbde

Contributor

kieran replied Nov 24, 2011

I love that you're continuing development on this :-)

FYI: I've since released the router as it's own package, with a few bug fixes & minor changes: https://github.com/kieran/barista - I'd recommend either using or forking the updated version.

Contributor

mde replied Dec 17, 2011

Dude, sorry I took so long to reply here. Sometimes feels like I'm juggling with only one arm. I'll definitely take a look at Barista, and either just use it as a dependency, or at least migrate in whatever improvements. I do like the idea of a turnkey-solution without a lot of external dependencies. This massive refactor was a long time coming. There are bunch of improvements to the HTTP service-layer that came out of work we did here at Yammer, and a massive stripping-down of what the ORM is intended to do. Besides all the cleanup, it's basically intended to provide MVC and other niceties where needed, while still allowing you to write Node apps in a more Node-like way (encouraging streaming, etc.).

Please sign in to comment.