Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

repo sync #70

Merged
merged 1 commit into from
Sep 28, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 0 additions & 10 deletions includes/head.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
14 changes: 14 additions & 0 deletions javascripts/get-csrf.js
Original file line number Diff line number Diff line change
@@ -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"]')
Expand Down
2 changes: 2 additions & 0 deletions javascripts/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -26,6 +27,7 @@ document.addEventListener('DOMContentLoaded', () => {
wrapCodeTerms()
print()
localization()
fillCsrf()
helpfulness()
experiment()
})
8 changes: 0 additions & 8 deletions middleware/context.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

Expand Down
2 changes: 1 addition & 1 deletion middleware/csrf-route.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
3 changes: 1 addition & 2 deletions middleware/csrf.js
Original file line number Diff line number Diff line change
@@ -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']
})
7 changes: 7 additions & 0 deletions tests/browser/browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
2 changes: 1 addition & 1 deletion tests/rendering/csrf-route.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
})
Expand Down
74 changes: 55 additions & 19 deletions tests/rendering/events.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -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)
)
})
Expand All @@ -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 = {
Expand All @@ -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)
)
})