Refactor to simplify csrf protection and expose enhancer API (#157)

#157
fusionjs · Nov 1, 2018 · 1ad13e4 · 1ad13e4
1 parent 16fdc5f
commit 1ad13e4
Show file tree

Hide file tree

Showing 12 changed files with 1,136 additions and 605 deletions.
diff --git a/README.md b/README.md
@@ -4,13 +4,8 @@
 
 Provides a modified `fetch` that is automatically secure against CSRF attacks for non-idempotent HTTP methods.
 
-This plugin handles csrf protection by adding a server side middleware that checks for a valid csrf token on 
-requests for non-idempotent HTTP methods (e.g. POST). It generates a csrf secret once per session based 
-on a combination of a timestamp and a server side stored secret and stores this using the provided session plugin 
-(usually via an encrypted cookie). It uses this csrf secret to generate and validate csrf tokens per request.
-
-NOTE: If you're making requests to CSRF protected endpoints from React, you should use [fusion-plugin-csrf-protection-react](https://github.com/fusionjs/fusion-plugin-csrf-protection-react) instead of this package.
-
+This enhancer handles csrf protection by adding a server side middleware that checks for a valid csrf token on 
+requests for non-idempotent HTTP methods (e.g. POST). 
 --- 
 
 ### Table of contents
@@ -23,10 +18,7 @@ NOTE: If you're making requests to CSRF protected endpoints from React, you shou
     * [`CsrfProtection`](#csrfprotection)
     * [`FetchToken`](#fetchtoken)
   * [Dependencies](#dependencies)
-    * [`CsrfExpireToken`](#csrfexpiretoken)
     * [`CsrfIgnoreRoutesToken`](#csrfignoreroutestoken)
-    * [`FetchForCsrfToken`](#fetchforcsrftoken)
-    * [`SessionToken`](#sessiontoken)
   * [Service API](#service-api)
 
 ---
@@ -62,28 +54,17 @@ const pluginUsingFetch = createPlugin({
 ```js
 // src/main.js
 import React from 'react';
-import {FetchToken, SessionToken} from 'fusion-tokens';
+import {FetchToken} from 'fusion-tokens';
 import App from 'fusion-react';
-import Session from 'fusion-plugin-jwt';
-import CsrfProtection, {
-  FetchForCsrfToken,
-  CsrfExpireToken,
+import CsrfProtectionEnhancer, {
   CsrfIgnoreRoutesToken,
 } from 'fusion-plugin-csrf-protection';
 import fetch from unfetch;
 
 export default () => {
   const app = new App(<div></div>);
-  app.register(SessionToken, Session);
-  app.register(FetchForCsrfToken, fetch);
-  app.register(FetchToken, CsrfProtection);
-  if (__BROWSER__) {
-    app.register(FetchForCsrfToken, fetch);
-    // see usage example above
-    app.register(someToken, pluginUsingFetch);
-  } 
-  // optional
-  app.register(CsrfExpireToken, 60 * 60 * 24); 
+  app.register(FetchToken, fetch);
+  app.enhance(FetchToken, CsrfProtectionEnhancer);
   // optional
   __NODE__ && app.register(CsrfIgnoreRoutesToken, []);
 }
@@ -112,24 +93,6 @@ For more, see [the fusion-tokens repo](https://github.com/fusionjs/fusion-tokens
 
 #### Dependencies
 
-##### `CsrfExpireToken`
-
-```js
-import {CsrfExpireToken} from 'fusion-plugin-csrf-protection';
-```
-
-The number of seconds for csrf tokens to remain valid. Optional.
-
-**Types**
-
-```js
-type CsrfExpire = number;
-```
-
-**Default value**
-
-The default expire is `86400` seconds, or 24 hours.
-
 ##### `CsrfIgnoreRoutesToken`
 
 ```js
@@ -148,27 +111,6 @@ type CsrfIgnoreRoutes = Array<string>;
 
 Empty array `[]`
 
-##### `FetchForCsrfToken`
-
-```js
-import {FetchForCsrfToken} from 'fusion-plugin-csrf-protection';
-```
-
-An implementation of `fetch` to be used by the `fusion-plugin-csrf-protection`. Usually this is simply a
-polyfill of fetch, or can even be a reference to `window.fetch`. It is useful to exist in the DI system 
-however for testing.
-
-For type information, see the [`FetchToken`](https://github.com/fusionjs/fusion-tokens#fetchtoken) docs. Required.
-
-##### `SessionToken`
-
-```js
-import {SessionToken} from 'fusion-tokens';
-```
-
-The canonical token for an implementation of a session. For type information, 
-see the [`SessionToken`](https://github.com/fusionjs/fusion-tokens#sessiontoken) docs. Required.
-
 #### Service API
 
 ```js

diff --git a/docs/migrations/00157.md b/docs/migrations/00157.md
@@ -0,0 +1,25 @@
+#### Update API to use enhancer pattern
+
+This change updates the API from a plugin to an enhancer, and greatly simplifies the configuration.
+The following tokens are removed:
+
+- `CsrfExpireToken`
+- `FetchForCsrfToken`
+- `SessionToken`
+
+Instead of registering this plugin on the `FetchToken`, you can now use this as an enhancer on the `FetchToken`.
+
+```diff
+import CsrfProtection from 'fusion-plugin-csrf-protection';
+-app.register(SessionToken, Session);	  
+-app.register(FetchForCsrfToken, fetch);	  
+-app.register(FetchToken, CsrfProtection);	
+-if (__BROWSER__) {	
+-  app.register(FetchForCsrfToken, fetch);	
+-} 	
+-app.register(CsrfExpireToken, 60 * 60 * 24); 
++app.register(FetchToken, fetch);
++app.enhance(FetchToken, CsrfProtection)
+```
+
+NOTE: There will be a codemod released for this
diff --git a/package.json b/package.json
@@ -38,7 +38,7 @@
     "eslint-plugin-react": "^7.11.1",
     "express": "^4.16.3",
     "flow-bin": "^0.83.0",
-    "fusion-core": "^1.5.0",
+    "fusion-core": "1.9.0-0",
     "fusion-test-utils": "^1.2.2",
     "fusion-tokens": "^1.0.4",
     "generic-session": "0.1.2",

diff --git a/src/__tests__/interface.js b/src/__tests__/interface.js
@@ -8,9 +8,10 @@
 
 import tape from 'tape-cup';
 
-import Plugin from '../index';
+import Plugin, {CsrfIgnoreRoutesToken} from '../index';
 
 tape('plugin api', t => {
   t.ok(Plugin);
+  t.ok(CsrfIgnoreRoutesToken);
   t.end();
 });
diff --git a/src/__tests__/test.browser.js b/src/__tests__/test.browser.js
@@ -14,13 +14,12 @@ import {FetchToken} from 'fusion-tokens';
 import type {Fetch} from 'fusion-tokens';
 
 import CsrfPlugin from '../index';
-import {CsrfExpireToken, FetchForCsrfToken} from '../shared';
 
 /* Test helpers */
 function getApp(fetchFn: Fetch) {
   const app = new App('element', el => el);
-  app.register(FetchForCsrfToken, fetchFn);
-  app.register(FetchToken, CsrfPlugin);
+  app.register(FetchToken, fetchFn);
+  app.enhance(FetchToken, CsrfPlugin);
   return app;
 }
 
@@ -48,22 +47,12 @@ test('exposes right methods', t => {
 
 test('includes routePrefix if exists', async t => {
   window.__ROUTE_PREFIX__ = '/something';
-  let called = 0;
-  const expectedUrls = ['/something/csrf-token', '/something/hello'];
   const fetch = (url, args) => {
-    called++;
-    t.equal(url, expectedUrls.shift());
+    t.equal(url, '/something/hello');
     return Promise.resolve(
       createMockFetch({
         url,
         args,
-        headers: {
-          get(key) {
-            if (key === 'x-csrf-token') {
-              return Math.round(Date.now() / 1000) + '-test';
-            }
-          },
-        },
       })
     );
   };
@@ -76,14 +65,9 @@ test('includes routePrefix if exists', async t => {
         t.equal(typeof fetch, 'function');
         // $FlowFixMe
         const {url, args} = await fetch('/hello', {method: 'POST'});
-        t.equals(called, 2, 'preflight works');
         t.equals(url, '/something/hello', 'ok url');
         t.equals(args.credentials, 'same-origin', 'ok credentials');
-        t.equals(
-          args.headers['x-csrf-token'].split('-')[1],
-          'test',
-          'ok token'
-        );
+        t.equals(args.headers['x-csrf-token'], 'x', 'sends token');
         delete window.__ROUTE_PREFIX__;
         t.end();
       },
@@ -92,32 +76,14 @@ test('includes routePrefix if exists', async t => {
   app.resolve();
 });
 
-test('supports getting initial token from dom element', async t => {
-  const el = document.createElement('div');
-  el.setAttribute('id', '__CSRF_TOKEN__');
-  el.setAttribute('type', 'application/json');
-  el.textContent = JSON.stringify(
-    `${Math.round(
-      Date.now() / 1000
-    )}-\u003C\u002Fscript\u003Etoken\u003Cscript\u003E`
-  );
-  document.body && document.body.appendChild(el);
-  let called = 0;
+test('sends token on POST', async t => {
   const expectedUrls = ['/hello'];
   const fetch = (url, args) => {
-    called++;
     t.equal(url, expectedUrls.shift());
     return Promise.resolve(
       createMockFetch({
         url,
         args,
-        headers: {
-          get(key) {
-            if (key === 'x-csrf-token') {
-              return Math.round(Date.now() / 1000) + '-lol';
-            }
-          },
-        },
       })
     );
   };
@@ -129,19 +95,9 @@ test('supports getting initial token from dom element', async t => {
       provides: async ({fetch}) => {
         // $FlowFixMe
         const {url, args} = await fetch('/hello', {method: 'POST'});
-        t.equals(
-          called,
-          1,
-          'does not preflight if deserializing token from html'
-        );
         t.equals(url, '/hello', 'ok url');
         t.equals(args.credentials, 'same-origin', 'ok credentials');
-        t.equals(
-          args.headers['x-csrf-token'].split('-')[1],
-          '</script>token<script>',
-          'unescapes the token correctly'
-        );
-        document.body && document.body.removeChild(el);
+        t.equal(args.headers['x-csrf-token'], 'x');
         t.end();
       },
     })
@@ -150,23 +106,13 @@ test('supports getting initial token from dom element', async t => {
 });
 
 test('defaults method to GET', async t => {
-  let called = 0;
   const expectedUrls = ['/hello'];
   const fetch = (url, args) => {
-    called++;
     t.equal(url, expectedUrls.shift());
-
     return Promise.resolve(
       createMockFetch({
         url,
         args,
-        headers: {
-          get(key) {
-            if (key === 'x-csrf-token') {
-              return Math.round(Date.now() / 1000) + '-test';
-            }
-          },
-        },
       })
     );
   };
@@ -178,101 +124,8 @@ test('defaults method to GET', async t => {
       provides: async ({fetch}) => {
         // $FlowFixMe
         const {url, args} = await fetch('/hello');
-        t.equals(called, 1, 'does not preflight for GET requests');
         t.equals(url, '/hello', 'ok url');
-        t.notok(
-          args.headers['x-csrf-token'],
-          'does not send token on GET requests'
-        );
-        t.end();
-      },
-    })
-  );
-  app.resolve();
-});
-
-test('fetch preflights if no token', t => {
-  let called = 0;
-  const fetch = (url, args) => {
-    called++;
-    return Promise.resolve(
-      createMockFetch({
-        url,
-        args,
-        headers: {
-          get(key) {
-            if (key === 'x-csrf-token') {
-              return Math.round(Date.now() / 1000) + '-test';
-            }
-          },
-        },
-      })
-    );
-  };
-
-  const app = getApp(fetch);
-  app.register(
-    createPlugin({
-      deps: {fetch: FetchToken},
-      provides: async ({fetch}) => {
-        // $FlowFixMe
-        const {url, args} = await fetch('/test', {method: 'POST'});
-        t.equals(called, 2, 'preflight works');
-        t.equals(url, '/test', 'ok url');
-        t.equals(args.credentials, 'same-origin', 'ok credentials');
-        t.equals(
-          args.headers['x-csrf-token'].split('-')[1],
-          'test',
-          'ok token'
-        );
-        await fetch('/foo', {method: 'POST'});
-        t.equals(called, 3, 'no preflight if token exists');
-        t.end();
-      },
-    })
-  );
-  app.resolve();
-});
-
-test('fetch preflights if token is expired', t => {
-  let called = 0;
-  const fetch = (url, args) => {
-    called++;
-    return Promise.resolve(
-      createMockFetch({
-        url,
-        args,
-        headers: {
-          get(key) {
-            if (key === 'x-csrf-token') {
-              return Math.round(Date.now() / 1000) + '-test';
-            }
-          },
-        },
-      })
-    );
-  };
-
-  const app = getApp(fetch);
-  app.register(CsrfExpireToken, 1);
-  app.register(
-    createPlugin({
-      deps: {fetch: FetchToken},
-      provides: async ({fetch}) => {
-        // $FlowFixMe
-        const {url, args} = await fetch('/test', {method: 'POST'});
-        t.equals(called, 2, 'preflight works');
-        t.equals(url, '/test', 'ok url');
-        t.equals(args.credentials, 'same-origin', 'ok credentials');
-        t.equals(
-          args.headers['x-csrf-token'].split('-')[1],
-          'test',
-          'ok token'
-        );
-
-        await new Promise(resolve => setTimeout(resolve, 2000));
-        await fetch('/foo', {method: 'POST'});
-        t.equals(called, 4, 'preflight if token expired');
+        t.notok(args.headers, 'does not send token on GET requests');
         t.end();
       },
     })