Inline CSRF token in HTML (github#17748)

* Inline CSRF token * Fix tests
shahrokh90 · Feb 9, 2021 · 1918d2e · 1918d2e
1 parent 6d20e43
commit 1918d2e
Show file tree

Hide file tree

Showing 8 changed files with 33 additions and 68 deletions.
diff --git a/includes/head.html b/includes/head.html
@@ -1,29 +1,30 @@
 <head>
+  {% comment %} For human readers {% endcomment %}
   <meta charset="utf-8" />
   <title>{% if error == '404' %}{% data ui.errors.oops %}{% elsif currentVersion == 'homepage' %}GitHub Documentation{% elsif page.fullTitle %}{{ page.fullTitle }}{% else %}GitHub Documentation{% endif %}</title>
-  <meta name="viewport" content="width=device-width, initial-scale=1">{% if page.hidden %}
-  <meta name="robots" content="noindex" />{% endif %}
-  <meta name="google-site-verification" content="OgdQc0GZfjDI52wDv1bkMT-SLpBUo_h5nn9mI9L22xQ" />
-  <meta name="google-site-verification" content="c1kuD-K2HIVF635lypcsWPoD4kilo5-jA_wBFyT4uMY" />
-
-  <!-- localized data needed by client-side JS  -->
-  <meta name="site.data.ui.search.placeholder" content="{% data ui.search.placeholder %}">
-  <!-- end localized data -->
+  <meta name="viewport" content="width=device-width, initial-scale=1">
+  <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">
+  <link rel="stylesheet" href="{{ builtAssets.main.css }}">
 
+  {% comment %} For Google and Bots {% endcomment %}
   {% if page.intro %}
     <meta name="description" content="{{ page.introPlainText }}">
   {% endif %}
-
-  <!-- hreflangs -->
+  {% if page.hidden %}
+    <meta name="robots" content="noindex" />
+  {% endif %}
   {% for languageVariant in page.languageVariants %}
     <link
       rel="alternate"
       hreflang="{{ languageVariant.hreflang }}"
       href="https://docs.github.com{{ languageVariant.href }}"
     />
   {% endfor %}
+  <meta name="google-site-verification" content="OgdQc0GZfjDI52wDv1bkMT-SLpBUo_h5nn9mI9L22xQ" />
+  <meta name="google-site-verification" content="c1kuD-K2HIVF635lypcsWPoD4kilo5-jA_wBFyT4uMY" />
 
-  <link rel="stylesheet" href="{{ builtAssets.main.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">
+  {% comment %} For our JS {% endcomment %}
+  <meta name="csrf-token" content="$CSRFTOKEN$">
+  <meta name="site.data.ui.search.placeholder" content="{% data ui.search.placeholder %}">
 </head>
diff --git a/javascripts/get-csrf.js b/javascripts/get-csrf.js
@@ -1,17 +1,3 @@
-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
@@ -12,7 +12,6 @@ import localization from './localization'
 import helpfulness from './helpfulness'
 import experiment from './experiment'
 import copyCode from './copy-code'
-import { fillCsrf } from './get-csrf'
 import initializeEvents from './events'
 import filterCards from './filter-cards'
 import allArticles from './all-articles'
@@ -35,9 +34,8 @@ document.addEventListener('DOMContentLoaded', async () => {
   allArticles()
   devToc()
   showMore()
-  await fillCsrf() // this must complete before any POST calls
-  initializeEvents() // requires fillCsrf to complete
-  experiment() // requires fillCsrf to complete
-  helpfulness() // requires fillCsrf to complete
   releaseNotes()
+  initializeEvents()
+  experiment()
+  helpfulness()
 })
diff --git a/middleware/csrf-route.js b/middleware/csrf-route.js
diff --git a/middleware/index.js b/middleware/index.js
@@ -83,7 +83,6 @@ module.exports = function (app) {
     maxAge: '7 days' // A bit longer since releases are more sparse
   }))
   app.use('/events', instrument('./events'))
-  app.use('/csrf', instrument('./csrf-route'))
   app.use('/search', instrument('./search'))
   app.use(instrument('./archived-enterprise-versions'))
   app.use(instrument('./robots'))

diff --git a/middleware/render-page.js b/middleware/render-page.js
@@ -21,6 +21,10 @@ const pageCache = new RedisAccessor({
 // a list of query params that *do* alter the rendered page, and therefore should be cached separately
 const cacheableQueries = ['learn']
 
+function addCsrf (req, text) {
+  return text.replace('$CSRFTOKEN$', req.csrfToken())
+}
+
 module.exports = async function renderPage (req, res, next) {
   const page = req.context.page
 
@@ -45,7 +49,7 @@ module.exports = async function renderPage (req, res, next) {
     if (cachedHtml) {
       console.log(`Serving from cached version of ${originalUrl}`)
       statsd.increment('page.sent_from_cache')
-      return res.send(cachedHtml)
+      return res.send(addCsrf(req, cachedHtml))
     }
   }
 
@@ -54,7 +58,12 @@ module.exports = async function renderPage (req, res, next) {
     if (process.env.NODE_ENV !== 'test' && req.context.redirectNotFound) {
       console.error(`\nTried to redirect to ${req.context.redirectNotFound}, but that page was not found.\n`)
     }
-    return res.status(404).send(await liquid.parseAndRender(layouts['error-404'], req.context))
+    return res.status(404).send(
+      addCsrf(
+        req,
+        await liquid.parseAndRender(layouts['error-404'], req.context)
+      )
+    )
   }
 
   if (req.method === 'HEAD') {
@@ -116,7 +125,7 @@ module.exports = async function renderPage (req, res, next) {
 
   // First, send the response so the user isn't waiting
   // NOTE: Do NOT `return` here as we still need to cache the response afterward!
-  res.send(output)
+  res.send(addCsrf(req, output))
 
   // Finally, save output to cache for the next time around
   if (isCacheable) {

diff --git a/tests/rendering/csrf-route.js b/tests/rendering/csrf-route.js
diff --git a/tests/rendering/events.js b/tests/rendering/events.js
@@ -1,5 +1,6 @@
 const request = require('supertest')
 const nock = require('nock')
+const cheerio = require('cheerio')
 const app = require('../../server')
 
 describe('POST /events', () => {
@@ -14,8 +15,9 @@ describe('POST /events', () => {
     process.env.HYDRO_SECRET = '$HYDRO_SECRET$'
     process.env.HYDRO_ENDPOINT = 'http://example.com/hydro'
     agent = request.agent(app)
-    const csrfRes = await agent.get('/csrf')
-    csrfToken = csrfRes.body.token
+    const csrfRes = await agent.get('/en')
+    const $ = cheerio.load(csrfRes.text || '', { xmlMode: true })
+    csrfToken = $('meta[name="csrf-token"]').attr('content')
     nock('http://example.com')
       .post('/hydro')
       .reply(200, {})