Skip to content

Commit 3505835

Browse files
OctomergerOctomerger
authored
Merge pull request #70 from github/repo-sync
repo sync
2 parents cc7bd5d + c450d8d commit 3505835

File tree

9 files changed

+81
-41
lines changed

9 files changed

+81
-41
lines changed

‎includes/head.html‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,4 @@
2525
<link rel="stylesheet" href="/dist/index.css">
2626
<link rel="alternate icon" type="image/png" href="/assets/images/site/favicon.png">
2727
<link rel="icon" type="image/svg+xml" href="/assets/images/site/favicon.svg">
28-
29-
{% if page.relativePath contains "managing-your-work-on-github/disabling-project-boards-in-your-organization" %}
30-
<!--TODO CSRF remove the outer check -->
31-
{% if fastlyEnabled %}
32-
<!-- https://www.fastly.com/blog/caching-uncacheable-csrf-security -->
33-
<esi:include src="/csrf" />
34-
{% else %}
35-
<meta name="csrf-token" content="{{ csrfToken }}" />
36-
{% endif %}
37-
{% endif %}
3828
</head>

‎javascripts/get-csrf.js‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,17 @@
1+
export async function fillCsrf () {
2+
const response = await fetch('/csrf', {
3+
method: 'GET',
4+
headers: {
5+
'Content-Type': 'application/json'
6+
}
7+
})
8+
const json = response.ok ? await response.json() : {}
9+
const meta = document.createElement('meta')
10+
meta.setAttribute('name', 'csrf-token')
11+
meta.setAttribute('content', json.token)
12+
document.querySelector('head').append(meta)
13+
}
14+
115
export default function getCsrf () {
216
const csrfEl = document
317
.querySelector('meta[name="csrf-token"]')

‎javascripts/index.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import print from './print'
1313
import localization from './localization'
1414
import helpfulness from './helpfulness'
1515
import experiment from './experiment'
16+
import { fillCsrf } from './get-csrf'
1617

1718
document.addEventListener('DOMContentLoaded', () => {
1819
displayPlatformSpecificContent()
@@ -26,6 +27,7 @@ document.addEventListener('DOMContentLoaded', () => {
2627
wrapCodeTerms()
2728
print()
2829
localization()
30+
fillCsrf()
2931
helpfulness()
3032
experiment()
3133
})

‎middleware/context.js‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -55,14 +55,6 @@ module.exports = async function contextualize (req, res, next) {
5555
req.context.siteTree = siteTree
5656
req.context.pages = pages
5757

58-
// To securely accept data from end users,
59-
// we need to validate that they were actually on a docs page first.
60-
// We'll put this token in the <head> and call on it when we send user data
61-
// to the docs server, so we know the request came from someone on the page.
62-
req.context.csrfToken = req.csrfToken()
63-
req.context.fastlyEnabled = process.env.NODE_ENV === 'production' &&
64-
req.hostname === 'docs.github.com'
65-
6658
return next()
6759
}
6860

‎middleware/csrf-route.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ router.get('/', (req, res) => {
88
'surrogate-control': 'private, no-store',
99
'cache-control': 'private, no-store'
1010
})
11-
res.send(`<meta name="csrf-token" content="${req.context.csrfToken}" />`)
11+
res.json({ token: req.csrfToken() })
1212
})
1313

1414
module.exports = router

‎middleware/csrf.js‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
module.exports = require('csurf')({
22
cookie: require('../lib/cookie-settings'),
3-
ignoreMethods: ['GET', 'HEAD', 'OPTIONS', 'POST', 'PUT']
4-
// TODO CSRF edit this to include POST and PUT to require it
3+
ignoreMethods: ['GET', 'HEAD', 'OPTIONS']
54
})

‎tests/browser/browser.js‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,13 @@ describe('helpfulness', () => {
135135
})
136136
})
137137

138+
describe('csrf meta', () => {
139+
it('should have a csrf-token meta tag on the page', async () => {
140+
await page.goto('http://localhost:4001/en/actions/getting-started-with-github-actions/about-github-actions')
141+
await page.waitForSelector('meta[name="csrf-token"]')
142+
})
143+
})
144+
138145
async function getLocationObject (page) {
139146
const location = await page.evaluate(() => {
140147
return Promise.resolve(JSON.stringify(window.location, null, 2))

‎tests/rendering/csrf-route.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ describe('GET /csrf', () => {
99
it('should render a non-cache include for CSRF token', async () => {
1010
const res = await request(app).get('/csrf')
1111
expect(res.status).toBe(200)
12-
expect(res.text).toMatch(/^<meta name="csrf-token" content="(.*?)" \/>$/)
12+
expect(res.body).toHaveProperty('token')
1313
expect(res.headers['surrogate-control']).toBe('private, no-store')
1414
expect(res.headers['cache-control']).toBe('private, no-store')
1515
})

‎tests/rendering/events.js‎

Lines changed: 55 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,23 @@ Airtable.mockImplementation(function () {
1111
})
1212

1313
describe('POST /events', () => {
14-
beforeEach(() => {
14+
jest.setTimeout(60 * 1000)
15+
16+
let csrfToken = ''
17+
let agent
18+
19+
beforeEach(async () => {
1520
process.env.AIRTABLE_API_KEY = '$AIRTABLE_API_KEY$'
1621
process.env.AIRTABLE_BASE_KEY = '$AIRTABLE_BASE_KEY$'
22+
agent = request.agent(app)
23+
const csrfRes = await agent.get('/csrf')
24+
csrfToken = csrfRes.body.token
1725
})
1826

1927
afterEach(() => {
2028
delete process.env.AIRTABLE_API_KEY
2129
delete process.env.AIRTABLE_BASE_KEY
30+
csrfToken = ''
2231
})
2332

2433
describe('HELPFULNESS', () => {
@@ -32,82 +41,93 @@ describe('POST /events', () => {
3241
}
3342

3443
it('should accept a valid object', () =>
35-
request(app)
44+
agent
3645
.post('/events')
3746
.send(example)
3847
.set('Accept', 'application/json')
48+
.set('csrf-token', csrfToken)
3949
.expect(201)
4050
)
4151

4252
it('should reject extra properties', () =>
43-
request(app)
53+
agent
4454
.post('/events')
4555
.send({ ...example, toothpaste: false })
4656
.set('Accept', 'application/json')
57+
58+
.set('csrf-token', csrfToken)
4759
.expect(400)
4860
)
4961

5062
it('should not accept if type is missing', () =>
51-
request(app)
63+
agent
5264
.post('/events')
5365
.send({ ...example, type: undefined })
5466
.set('Accept', 'application/json')
67+
.set('csrf-token', csrfToken)
5568
.expect(400)
5669
)
5770

5871
it('should not accept if url is missing', () =>
59-
request(app)
72+
agent
6073
.post('/events')
6174
.send({ ...example, url: undefined })
6275
.set('Accept', 'application/json')
76+
.set('csrf-token', csrfToken)
6377
.expect(400)
6478
)
6579

6680
it('should not accept if url is misformatted', () =>
67-
request(app)
81+
agent
6882
.post('/events')
6983
.send({ ...example, url: 'examplecom' })
7084
.set('Accept', 'application/json')
85+
.set('csrf-token', csrfToken)
7186
.expect(400)
7287
)
7388

7489
it('should not accept if vote is missing', () =>
75-
request(app)
90+
agent
7691
.post('/events')
7792
.send({ ...example, vote: undefined })
7893
.set('Accept', 'application/json')
94+
.set('csrf-token', csrfToken)
7995
.expect(400)
8096
)
8197

8298
it('should not accept if vote is not boolean', () =>
83-
request(app)
99+
agent
84100
.post('/events')
85101
.send({ ...example, vote: 'true' })
86102
.set('Accept', 'application/json')
103+
.set('csrf-token', csrfToken)
87104
.expect(400)
88105
)
89106

90107
it('should not accept if email is misformatted', () =>
91-
request(app)
108+
agent
92109
.post('/events')
93110
.send({ ...example, email: 'testexample.com' })
94111
.set('Accept', 'application/json')
112+
.set('csrf-token', csrfToken)
95113
.expect(400)
96114
)
97115

98116
it('should not accept if comment is not string', () =>
99-
request(app)
117+
agent
100118
.post('/events')
101119
.send({ ...example, comment: [] })
102120
.set('Accept', 'application/json')
121+
.set('csrf-token', csrfToken)
103122
.expect(400)
104123
)
105124

106125
it('should not accept if category is not an option', () =>
107-
request(app)
126+
agent
108127
.post('/events')
109128
.send({ ...example, category: 'Fabulous' })
110129
.set('Accept', 'application/json')
130+
.set('csrf-token', csrfToken)
111131
.expect(400)
112132
)
113133
})
@@ -122,64 +142,79 @@ describe('POST /events', () => {
122142
}
123143

124144
it('should accept a valid object', () =>
125-
request(app)
145+
agent
126146
.post('/events')
127147
.send(example)
128148
.set('Accept', 'application/json')
149+
.set('csrf-token', csrfToken)
129150
.expect(201)
130151
)
131152

132153
it('should reject extra fields', () =>
133-
request(app)
154+
agent
134155
.post('/events')
135156
.send({ ...example, toothpaste: false })
136157
.set('Accept', 'application/json')
158+
.set('csrf-token', csrfToken)
137159
.expect(400)
138160
)
139161

140162
it('should require a long unique user-id', () =>
141-
request(app)
163+
agent
142164
.post('/events')
143165
.send({ ...example, 'user-id': 'short' })
144166
.set('Accept', 'application/json')
167+
.set('csrf-token', csrfToken)
145168
.expect(400)
146169
)
147170

148171
it('should require a test', () =>
149-
request(app)
172+
agent
150173
.post('/events')
151174
.send({ ...example, test: undefined })
152175
.set('Accept', 'application/json')
176+
.set('csrf-token', csrfToken)
153177
.expect(400)
154178
)
155179

156180
it('should require a valid group', () =>
157-
request(app)
181+
agent
158182
.post('/events')
159183
.send({ ...example, group: 'revolution' })
160184
.set('Accept', 'application/json')
185+
.set('csrf-token', csrfToken)
161186
.expect(400)
162187
)
163188

164189
it('should default the success field', () =>
165-
request(app)
190+
agent
166191
.post('/events')
167192
.send({ ...example, success: undefined })
168193
.set('Accept', 'application/json')
194+
.set('csrf-token', csrfToken)
169195
.expect(201)
170196
)
171197
})
172198
})
173199

174200
describe('PUT /events/:id', () => {
175-
beforeEach(() => {
201+
jest.setTimeout(60 * 1000)
202+
203+
let csrfToken = ''
204+
let agent
205+
206+
beforeEach(async () => {
176207
process.env.AIRTABLE_API_KEY = '$AIRTABLE_API_KEY$'
177208
process.env.AIRTABLE_BASE_KEY = '$AIRTABLE_BASE_KEY$'
209+
agent = request.agent(app)
210+
const csrfRes = await agent.get('/csrf')
211+
csrfToken = csrfRes.body.token
178212
})
179213

180214
afterEach(() => {
181215
delete process.env.AIRTABLE_API_KEY
182216
delete process.env.AIRTABLE_BASE_KEY
217+
csrfToken = ''
183218
})
184219

185220
const example = {
@@ -192,10 +227,11 @@ describe('PUT /events/:id', () => {
192227
}
193228

194229
it('should update an existing HELPFULNESS event', () =>
195-
request(app)
230+
agent
196231
.put('/events/TESTID')
197232
.send(example)
198233
.set('Accept', 'application/json')
234+
.set('csrf-token', csrfToken)
199235
.expect(200)
200236
)
201237
})

0 commit comments

Comments
 (0)