nginx/csp: allow frontend to include itself in an iframe - #2155
nginx/csp: allow frontend to include itself in an iframe#2155alxndrsn wants to merge 10 commits into
Conversation
|
|
||
| // when | ||
| await page.evaluate(() => { | ||
| /* global document */ |
There was a problem hiding this comment.
eslint complains if this is scoped to every evaluate block
| 'disposition': 'enforce', | ||
| 'blocked-uri': 'inline', | ||
| 'line-number': 5, | ||
| 'line-number': 4, |
There was a problem hiding this comment.
unclear why this changes when the CSP changes 馃し
matthew-white
left a comment
matthew-white
left a comment
There was a problem hiding this comment.
I think frame-ancestors needs to change in a different part of the CSP. The new tests look great though! I think those are still useful.
| "default-src 'report-sample' 'none'; connect-src 'self' https:; font-src 'self' data:; form-action 'self'; frame-ancestors 'self'; frame-src 'self' https://getodk.github.io/central/; img-src blob: data: https:; manifest-src 'self'; media-src blob:; object-src 'none'; script-src 'report-sample' 'self' 'wasm-unsafe-eval'; style-src 'self' 'unsafe-inline'; worker-src 'report-sample' blob: data:; report-uri /csp-report"; | ||
|
|
||
| default | ||
| "default-src 'report-sample' 'none'; connect-src 'self' ${SENTRY_DSN_FRONTEND_ROOT} https://translate.google.com https://translate.googleapis.com; font-src 'self'; form-action 'self'; frame-ancestors 'none'; frame-src 'self' https://getodk.github.io/central/; img-src data: https:; manifest-src 'self'; media-src 'none'; object-src 'none'; script-src 'report-sample' 'self'; style-src 'report-sample' 'self'; style-src-attr 'unsafe-inline'; worker-src 'report-sample' blob:; report-uri /csp-report"; |
There was a problem hiding this comment.
/blank.html is already allowed to be shown in an iframe:
central/files/nginx/odk.conf.template
Line 220 in 040ad86
It's more so that we need to show Backend JSON in an iframe. That's because the iframe form submits to a Backend URL. So I think it's this line that needs to change:
central/files/nginx/odk.conf.template
Line 113 in 040ad86
There was a problem hiding this comment.
The Backend JSON is only shown in an error case (e.g., for a wrong password). In the non-error case, Backend returns the .csv/.zip response: the download begins.
| }); | ||
|
|
||
| test.describe('bug: https://github.com/getodk/central/issues/2080', () => { | ||
| test('allows central-frontend to show itself in a frame', async ({ page }) => { |
There was a problem hiding this comment.
| test('allows central-frontend to show itself in a frame', async ({ page }) => { | |
| test('allows central-backend to show itself in a frame', async ({ page }) => { |
| // when | ||
| await page.evaluate(() => { | ||
| const frame = document.createElement('iframe'); | ||
| frame.src = '/'; |
There was a problem hiding this comment.
I think it should be a Backend URL instead, e.g.:
| frame.src = '/'; | |
| frame.src = '/v1/projects'; |
|
I've adapted the tests here and expanded them to cover a wider range of CSPs & scenarios. PR at #2159. Once that's merged, this one can be updated to be clearer on what it's changing and why. |
2 participants
Closes #2080
What has been done to verify that this works as intended?
Why is this the best possible solution? Were any other approaches considered?
frame-ancestorto more specific routes is impractical/impossibleHow does this change impact users? Describe intentional behavior changes from code updates. What are the regression risks?
Should just fix a bug - low risk of creating new ones.
Does this change require updates to documentation? If so, please file an issue here and include the link below.
No.