Updating csrf config to allow passing type of match (#145)

* Extra value types for xframe

* Add csrfFunction as well as xframeFunction

Also fixes a bug wherein / was always matching if part of allowlist or blocklist

* Rework shouldBypass function

* Rework shouldBypass on xframe too

* Addressing PR comments

* Unnecessary end function

* updating allowlist/blocklist config to specify match type

* changing includes with startsWith

* adding asserts for invalid config

Co-authored-by: Lincoln Race <lrace@paypal.com>

.jshintrc

@@ -42,8 +42,8 @@
 
     // ECMAScript 5
 
-    // Whether ES5 syntax should be allowed.
-    //"es5": true,
+    // Whether ES6 syntax should be allowed.
+    "esversion": 6,
     // Whether the "use strict"; pragma should be required.
     "strict": false,
     // Whether global "use strict"; should be allowed (also enables strict).

CHANGELOG.md

@@ -1,6 +1,8 @@
 ##### Unreleased
 
 * Changes `whitelist`/`blacklist` to `allowlist`/`blocklist` to follow [guidelines](https://chromium.googlesource.com/chromium/src/+/master/styleguide/inclusive_code.md#racially-neutral)
+* Updates `allowlist`, `blocklist` csrf config to allow specifying the type of match required
+
 
 
 ##### v1.6.1

README.md

@@ -60,8 +60,19 @@ __Please note that you must use [express-session](https://github.com/expressjs/s
 * `cookie.name` String - Required if cookie is an object and `angular` is not true. The CSRF cookie name to set.
 * `cookie.options` Object - Optional. A valid Express cookie options object.
 * `angular` Boolean - Optional. Shorthand setting to set `lusca` up to use the default settings for CSRF validation according to the [AngularJS docs]. Can be used with `cookie.options`.
-* `blocklist` Array or String - Optional. Allows defining a set of routes that will not have csrf protection.  All others will.
-* `allowlist` Array or String - Optional. Allows defining a set of routes that will have csrf protection.  All others will not.
+* `blocklist` Array or String - Optional. Allows defining a set of routes that will not have csrf protection.  All others will.  
+Example configuration:
+  ```
+  blocklist: [{path: '/details', type: 'exact'}, {path: '/summary', type: 'startWith'}]
+  //If match type is 'exact', route will get blocklisted only if it matches req.path exactly
+  //If match type is 'startsWith', Lusca will check if req.path starts with the specified path
+
+  For backwards compatiblity, following configuration is supported as well. It will be evaluated using the 'startsWith' match type.
+  blocklist: '/details';
+  blocklist: ['/details', '/summary'];
+  ```
+* `allowlist` Array or String - Optional. Allows defining a set of routes that will have csrf protection.  All others will not.  
+Configuration is similar to `blocklist` config
 
 Notes: The app can use either a `blocklist` or a `allowlist`, not both.  By default, all post routes are allowlisted.
 
@@ -142,4 +153,4 @@ Enables [X-Content-Type-Options](https://blogs.msdn.microsoft.com/ie/2008/09/02/
 
 * `value` String - Optional. The value for the header, e.g. `origin`, `same-origin`, `no-referrer`. Defaults to `` (empty string).
 
-Enables [Referrer-Policy](https://www.w3.org/TR/referrer-policy/#intro) header to control the Referer header.
+Enables [Referrer-Policy](https://www.w3.org/TR/referrer-policy/#intro) header to control the Referer header.

lib/csrf.js

@@ -2,8 +2,8 @@
 
 
 var token = require('./token'),
-    timeSafeCompare = require('tsscmp');
-
+    timeSafeCompare = require('tsscmp'),
+    assert = require('assert');
 
 /**
  * CSRF
@@ -14,7 +14,8 @@ var token = require('./token'),
  *    header {String} The name of the response header containing the CSRF token. Default "x-csrf-token".
  */
 module.exports = function (options) {
-    var impl, key, header, secret, cookie, allowlist, blocklist;
+    var impl, key, header, secret, cookie, allowlist, blocklist, allowlistStartsWith,
+    allowlistExact, blocklistStartsWith, blocklistExact;
 
     options = options || {};
 
@@ -29,21 +30,37 @@ module.exports = function (options) {
     allowlist = options.allowlist || options.whitelist;
 
     if (typeof allowlist === 'string') {
-        allowlist = [ allowlist ];
+        allowlist = [ { path: allowlist, type: 'startsWith' } ];
     } else if (!Array.isArray(allowlist)) {
         // Don't allow non string or array allowlist
         allowlist = null;
     }
 
+    if (allowlist) {
+        allowlistStartsWith = new Set();
+        allowlistExact = new Set();
+        populateSet(allowlist, allowlistStartsWith, allowlistExact);
+        //preemptively converting to array to use convinience methods in middleware
+        allowlistStartsWith = Array.from(allowlistStartsWith);
+    }
+
     blocklist = options.blocklist || options.blacklist;
 
     if (typeof blocklist === 'string') {
-        blocklist = [ blocklist ];
+        blocklist = [ { path: blocklist, type: 'startsWith' } ];
     } else if (!Array.isArray(blocklist)) {
         // Don't allow non string or array blocklist
         blocklist = null;
     }
 
+    if (blocklist) {
+        blocklistStartsWith = new Set();
+        blocklistExact = new Set();
+        populateSet(blocklist, blocklistStartsWith, blocklistExact);
+        //preemptively converting to array to use convinience methods in middleware
+        blocklistStartsWith = Array.from(blocklistStartsWith);
+    }
+
     key = options.key || '_csrf';
     impl = options.impl || token;
     header = options.header || 'x-csrf-token';
@@ -86,20 +103,21 @@ module.exports = function (options) {
         }
     }
 
-
     return function checkCsrf(req, res, next) {
 
         var shouldBypass = false;
 
         if (blocklist) {
-            blocklist.some(function (exclusion) {
+            shouldBypass = blocklistExact.has(req.path) ||
+                blocklistStartsWith.some(function (exclusion) {
                 shouldBypass = req.path.indexOf(exclusion) === 0;
                 return shouldBypass;
             });
         }
 
         if (allowlist) {
-            allowlist.some(function (inclusion) {
+            shouldBypass = !allowlistExact.has(req.path) ||
+                allowlistStartsWith.some(function (inclusion) {
                 shouldBypass = req.path.indexOf(inclusion) !== 0;
                 return shouldBypass;
             });
@@ -152,3 +170,25 @@ module.exports = function (options) {
         }
     };
 };
+
+function populateSet(list, startsWith, exact) {
+    const validTypesList = ['startsWith', 'exact'];
+
+    list.forEach(function (config) {
+        if (typeof config === 'string') {
+            startsWith.add(config);
+        } else {
+            const { type, path } = config;
+
+            assert(validTypesList.includes(type),
+            `Invalid csrf config. type '${type}' is not supported for path '${path}'. Please use one of ${validTypesList.join(', ')}`);
+            assert(path, 'Invalid csrf config. path is required');
+
+            if (type === 'startsWith') {
+                startsWith.add(path);
+            } else {
+                exact.add(path);
+            }
+        }
+    });
+}

lib/xframes.js

@@ -11,4 +11,4 @@ module.exports = function (value) {
         res.header('x-frame-options', value);
         next();
     };
-};
+};

package.json

@@ -37,7 +37,7 @@
     "express-session": "^1.7.5",
     "grunt": "^0.4.5",
     "grunt-cli": "^1.2.0",
-    "grunt-contrib-jshint": "^0.7.0",
+    "grunt-contrib-jshint": "^3.0.0",
     "grunt-mocha-test": "^0.7.0",
     "jshint": "*",
     "supertest": "^0.13.0"

test/csrf.js

@@ -45,7 +45,7 @@ describe('CSRF', function () {
     it('should not require token on post to blocklist', function (done) {
         var app = mock({
             csrf: {
-                blocklist: ['/blocklist1', '/blocklist2']
+                blocklist: ['/blocklist1', { path: '/blocklist2', type: 'startsWith' }, { path: '/', type: 'exact' }]
             }
         });
 
@@ -61,6 +61,10 @@ describe('CSRF', function () {
             res.send(200);
         });
 
+        app.post('/', function (req, res) {
+            res.send(200);
+        });
+
         request(app)
             .post('/blocklist1')
             .expect(200)
@@ -71,6 +75,38 @@ describe('CSRF', function () {
             .expect(200)
             .end(function (err, res) {});
 
+        request(app)
+            .post('/notblocklist')
+            .expect(403)
+            .end(function (err, res) {});
+
+        request(app)
+            .post('/')
+            .expect(200)
+            .end(function (err, res) {
+                done(err);
+            });
+    });
+    it('should not require token on post to blocklist (string config)', function (done) {
+        var app = mock({
+            csrf: {
+                blocklist: '/blocklist1'
+            }
+        });
+
+        app.post('/blocklist1', function (req, res) {
+            res.send(200);
+        });
+
+        app.post('/notblocklist', function (req, res) {
+            res.send(200);
+        });
+
+        request(app)
+            .post('/blocklist1')
+            .expect(200)
+            .end(function (err, res) {});
+
         request(app)
             .post('/notblocklist')
             .expect(403)
@@ -81,7 +117,7 @@ describe('CSRF', function () {
     it('should only require token on post to allowlist', function (done) {
         var app = mock({
             csrf: {
-                allowlist: ['/allowlist1', '/allowlist2']
+                allowlist: ['/allowlist1', { path: '/allowlist2', type: 'startsWith' }, { path: '/', type: 'exact' }]
             }
         });
 
@@ -97,23 +133,80 @@ describe('CSRF', function () {
             res.send(200);
         });
 
+        app.post('/', function (req, res) {
+            res.send(200);
+        });
+
         request(app)
             .post('/allowlist1')
             .expect(403)
             .end(function (err, res) {});
-        
+
         request(app)
             .post('/allowlist2')
             .expect(403)
             .end(function (err, res) {});
 
+        request(app)
+            .post('/notallowlist')
+            .expect(200)
+            .end(function (err, res) {
+                console.log(err);
+            });
+
+        request(app)
+            .post('/')
+            .expect(200)
+            .end(function (err, res) {
+                done(err);
+            });
+    });
+    it('should only require token on post to allowlist (string config)', function (done) {
+        var app = mock({
+            csrf: {
+                allowlist: '/allowlist1'
+            }
+        });
+
+        app.post('/allowlist1', function (req, res) {
+            res.send(200);
+        });
+
+        app.post('/notallowlist', function (req, res) {
+            res.send(200);
+        });
+
+        request(app)
+            .post('/allowlist1')
+            .expect(403)
+            .end(function (err, res) {});
+
         request(app)
             .post('/notallowlist')
             .expect(200)
             .end(function (err, res) {
                 done(err);
             });
     });
+
+    it('should throw error on invalid allowlist/blocklist config', function() {
+        assert.throws(function() {
+            mock({
+                csrf: {
+                    allowlist: [{ path: '/allowInvalid', type: 'regex' }]
+                }
+            });
+        }, /Invalid csrf config. type 'regex' is not supported.*/);
+
+        assert.throws(function() {
+            mock({
+                csrf: {
+                    allowlist: [{ path: '', type: 'startsWith' }]
+                }
+            });
+        }, /Invalid csrf config. path is required/);
+    });
+
     dd(sessionOptions, function () {
         it('GETs have a CSRF token (session type: {value})', function (ctx, done) {
             var mockConfig = (ctx.value === 'cookie') ? {
@@ -883,4 +976,4 @@ describe('CSRF', function () {
                 });
         });
     });
-});
+});