github · Octomerger · Sep 28, 2020 · Sep 28, 2020
diff --git a/includes/head.html b/includes/head.html
@@ -25,14 +25,4 @@
   <link rel="stylesheet" href="/dist/index.css">
   <link rel="alternate icon" type="image/png" href="/assets/images/site/favicon.png">
   <link rel="icon" type="image/svg+xml" href="/assets/images/site/favicon.svg">
-
-  {% if page.relativePath contains "managing-your-work-on-github/disabling-project-boards-in-your-organization" %}
-  <!--TODO CSRF remove the outer check -->
-  {% if fastlyEnabled %}
-    <!-- https://www.fastly.com/blog/caching-uncacheable-csrf-security -->
-    <esi:include src="/csrf" />
-  {% else %}
-    <meta name="csrf-token" content="{{ csrfToken }}" />
-  {% endif %}
-  {% endif %}
 </head>
diff --git a/javascripts/get-csrf.js b/javascripts/get-csrf.js
@@ -1,3 +1,17 @@
+export async function fillCsrf () {
+  const response = await fetch('/csrf', {
+    method: 'GET',
+    headers: {
+      'Content-Type': 'application/json'
+    }
+  })
+  const json = response.ok ? await response.json() : {}
+  const meta = document.createElement('meta')
+  meta.setAttribute('name', 'csrf-token')
+  meta.setAttribute('content', json.token)
+  document.querySelector('head').append(meta)
+}
+
 export default function getCsrf () {
   const csrfEl = document
     .querySelector('meta[name="csrf-token"]')

diff --git a/javascripts/index.js b/javascripts/index.js
@@ -13,6 +13,7 @@ import print from './print'
 import localization from './localization'
 import helpfulness from './helpfulness'
 import experiment from './experiment'
+import { fillCsrf } from './get-csrf'
 
 document.addEventListener('DOMContentLoaded', () => {
   displayPlatformSpecificContent()
@@ -26,6 +27,7 @@ document.addEventListener('DOMContentLoaded', () => {
   wrapCodeTerms()
   print()
   localization()
+  fillCsrf()
   helpfulness()
   experiment()
 })
diff --git a/middleware/context.js b/middleware/context.js
@@ -55,14 +55,6 @@ module.exports = async function contextualize (req, res, next) {
   req.context.siteTree = siteTree
   req.context.pages = pages
 
-  // To securely accept data from end users,
-  // we need to validate that they were actually on a docs page first.
-  // We'll put this token in the <head> and call on it when we send user data
-  // to the docs server, so we know the request came from someone on the page.
-  req.context.csrfToken = req.csrfToken()
-  req.context.fastlyEnabled = process.env.NODE_ENV === 'production' &&
-    req.hostname === 'docs.github.com'
-
   return next()
 }
 

diff --git a/middleware/csrf-route.js b/middleware/csrf-route.js
@@ -8,7 +8,7 @@ router.get('/', (req, res) => {
     'surrogate-control': 'private, no-store',
     'cache-control': 'private, no-store'
   })
-  res.send(`<meta name="csrf-token" content="${req.context.csrfToken}" />`)
+  res.json({ token: req.csrfToken() })
 })
 
 module.exports = router
diff --git a/middleware/csrf.js b/middleware/csrf.js
@@ -1,5 +1,4 @@
 module.exports = require('csurf')({
   cookie: require('../lib/cookie-settings'),
-  ignoreMethods: ['GET', 'HEAD', 'OPTIONS', 'POST', 'PUT']
-  // TODO CSRF edit this to include POST and PUT to require it
+  ignoreMethods: ['GET', 'HEAD', 'OPTIONS']
 })
diff --git a/tests/browser/browser.js b/tests/browser/browser.js
@@ -135,6 +135,13 @@ describe('helpfulness', () => {
   })
 })
 
+describe('csrf meta', () => {
+  it('should have a csrf-token meta tag on the page', async () => {
+    await page.goto('http://localhost:4001/en/actions/getting-started-with-github-actions/about-github-actions')
+    await page.waitForSelector('meta[name="csrf-token"]')
+  })
+})
+
 async function getLocationObject (page) {
   const location = await page.evaluate(() => {
     return Promise.resolve(JSON.stringify(window.location, null, 2))

diff --git a/tests/rendering/csrf-route.js b/tests/rendering/csrf-route.js
@@ -9,7 +9,7 @@ describe('GET /csrf', () => {
   it('should render a non-cache include for CSRF token', async () => {
     const res = await request(app).get('/csrf')
     expect(res.status).toBe(200)
-    expect(res.text).toMatch(/^<meta name="csrf-token" content="(.*?)" \/>$/)
+    expect(res.body).toHaveProperty('token')
     expect(res.headers['surrogate-control']).toBe('private, no-store')
     expect(res.headers['cache-control']).toBe('private, no-store')
   })

diff --git a/tests/rendering/events.js b/tests/rendering/events.js
@@ -11,14 +11,23 @@ Airtable.mockImplementation(function () {
 })
 
 describe('POST /events', () => {
-  beforeEach(() => {
+  jest.setTimeout(60 * 1000)
+
+  let csrfToken = ''
+  let agent
+
+  beforeEach(async () => {
     process.env.AIRTABLE_API_KEY = '$AIRTABLE_API_KEY$'
     process.env.AIRTABLE_BASE_KEY = '$AIRTABLE_BASE_KEY$'
+    agent = request.agent(app)
+    const csrfRes = await agent.get('/csrf')
+    csrfToken = csrfRes.body.token
   })
 
   afterEach(() => {
     delete process.env.AIRTABLE_API_KEY
     delete process.env.AIRTABLE_BASE_KEY
+    csrfToken = ''
   })
 
   describe('HELPFULNESS', () => {
@@ -32,82 +41,93 @@ describe('POST /events', () => {
     }
 
     it('should accept a valid object', () =>
-      request(app)
+      agent
         .post('/events')
         .send(example)
         .set('Accept', 'application/json')
+        .set('csrf-token', csrfToken)
         .expect(201)
     )
 
     it('should reject extra properties', () =>
-      request(app)
+      agent
         .post('/events')
         .send({ ...example, toothpaste: false })
         .set('Accept', 'application/json')
+
+        .set('csrf-token', csrfToken)
         .expect(400)
     )
 
     it('should not accept if type is missing', () =>
-      request(app)
+      agent
         .post('/events')
         .send({ ...example, type: undefined })
         .set('Accept', 'application/json')
+        .set('csrf-token', csrfToken)
         .expect(400)
     )
 
     it('should not accept if url is missing', () =>
-      request(app)
+      agent
         .post('/events')
         .send({ ...example, url: undefined })
         .set('Accept', 'application/json')
+        .set('csrf-token', csrfToken)
         .expect(400)
     )
 
     it('should not accept if url is misformatted', () =>
-      request(app)
+      agent
         .post('/events')
         .send({ ...example, url: 'examplecom' })
         .set('Accept', 'application/json')
+        .set('csrf-token', csrfToken)
         .expect(400)
     )
 
     it('should not accept if vote is missing', () =>
-      request(app)
+      agent
         .post('/events')
         .send({ ...example, vote: undefined })
         .set('Accept', 'application/json')
+        .set('csrf-token', csrfToken)
         .expect(400)
     )
 
     it('should not accept if vote is not boolean', () =>
-      request(app)
+      agent
         .post('/events')
         .send({ ...example, vote: 'true' })
         .set('Accept', 'application/json')
+        .set('csrf-token', csrfToken)
         .expect(400)
     )
 
     it('should not accept if email is misformatted', () =>
-      request(app)
+      agent
         .post('/events')
         .send({ ...example, email: 'testexample.com' })
         .set('Accept', 'application/json')
+        .set('csrf-token', csrfToken)
         .expect(400)
     )
 
     it('should not accept if comment is not string', () =>
-      request(app)
+      agent
         .post('/events')
         .send({ ...example, comment: [] })
         .set('Accept', 'application/json')
+        .set('csrf-token', csrfToken)
         .expect(400)
     )
 
     it('should not accept if category is not an option', () =>
-      request(app)
+      agent
         .post('/events')
         .send({ ...example, category: 'Fabulous' })
         .set('Accept', 'application/json')
+        .set('csrf-token', csrfToken)
         .expect(400)
     )
   })
@@ -122,64 +142,79 @@ describe('POST /events', () => {
     }
 
     it('should accept a valid object', () =>
-      request(app)
+      agent
         .post('/events')
         .send(example)
         .set('Accept', 'application/json')
+        .set('csrf-token', csrfToken)
         .expect(201)
     )
 
     it('should reject extra fields', () =>
-      request(app)
+      agent
         .post('/events')
         .send({ ...example, toothpaste: false })
         .set('Accept', 'application/json')
+        .set('csrf-token', csrfToken)
         .expect(400)
     )
 
     it('should require a long unique user-id', () =>
-      request(app)
+      agent
         .post('/events')
         .send({ ...example, 'user-id': 'short' })
         .set('Accept', 'application/json')
+        .set('csrf-token', csrfToken)
         .expect(400)
     )
 
     it('should require a test', () =>
-      request(app)
+      agent
         .post('/events')
         .send({ ...example, test: undefined })
         .set('Accept', 'application/json')
+        .set('csrf-token', csrfToken)
         .expect(400)
     )
 
     it('should require a valid group', () =>
-      request(app)
+      agent
         .post('/events')
         .send({ ...example, group: 'revolution' })
         .set('Accept', 'application/json')
+        .set('csrf-token', csrfToken)
         .expect(400)
     )
 
     it('should default the success field', () =>
-      request(app)
+      agent
         .post('/events')
         .send({ ...example, success: undefined })
         .set('Accept', 'application/json')
+        .set('csrf-token', csrfToken)
         .expect(201)
     )
   })
 })
 
 describe('PUT /events/:id', () => {
-  beforeEach(() => {
+  jest.setTimeout(60 * 1000)
+
+  let csrfToken = ''
+  let agent
+
+  beforeEach(async () => {
     process.env.AIRTABLE_API_KEY = '$AIRTABLE_API_KEY$'
     process.env.AIRTABLE_BASE_KEY = '$AIRTABLE_BASE_KEY$'
+    agent = request.agent(app)
+    const csrfRes = await agent.get('/csrf')
+    csrfToken = csrfRes.body.token
   })
 
   afterEach(() => {
     delete process.env.AIRTABLE_API_KEY
     delete process.env.AIRTABLE_BASE_KEY
+    csrfToken = ''
   })
 
   const example = {
@@ -192,10 +227,11 @@ describe('PUT /events/:id', () => {
   }
 
   it('should update an existing HELPFULNESS event', () =>
-    request(app)
+    agent
       .put('/events/TESTID')
       .send(example)
       .set('Accept', 'application/json')
+      .set('csrf-token', csrfToken)
       .expect(200)
   )
 })