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

replace some hard coded clauses of the Content-Security-Policy with config #4255

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

twrichards
Copy link
Contributor

@twrichards twrichards commented Mar 25, 2024

@AndyKilmory noticed that there were a few things hardcoded into our CSP which we decided ought to be coming from config.

For example youtube.com (introduced in #4127) is specific to a guardian add-on (https://github.com/guardian/pinboard) which is added via the scriptsToLoad config, so this PR extends the ScriptToLoad case class to represent additionalFrameSourcesForCSP and additionalImageSourcesForCSP (to replace another hard coded pinboard thing, which makes avatars work).

In the process I've removed https://accounts.google.com from the frame-src portion (added in bcdcb2e) since I can't see how it would be used (although @andrew-nowak might have an idea).


  • updated TEST config uploaded
  • tested in TEST ✅ 🎉
  • updated PROD config uploaded

Copy link

github-actions bot commented Mar 25, 2024

@twrichards twrichards force-pushed the improve-CSP-configurability branch 2 times, most recently from 94b144a to e72a36b Compare March 25, 2024 13:27
@twrichards twrichards marked this pull request as ready for review March 25, 2024 13:34
@twrichards twrichards requested a review from a team as a code owner March 25, 2024 13:34
@andrew-nowak
Copy link
Member

andrew-nowak commented Mar 26, 2024

In the process I've removed https://accounts.google.com from the frame-src portion (added in bcdcb2e) since I can't see how it would be used (although @andrew-nowak might have an idea).

this is necessary at least for the guardian for panda-session to create the iframe to relog users in after their panda cookie expires, as the reauth will include a redirect through accounts.google.com in that iframe

you can test by running in your browser console document.cookie = 'panda-forceExpiry=1;domain=.test.dev-gutools.co.uk' then waiting for the next auto-refresh of the search page

@twrichards
Copy link
Contributor Author

re-tested on TEST after some tweaks, ready for re-review 🙏

NOTE:
I've moved the https://accounts.google.com into the security.frameSources config property in kahuna.conf in response to...

In the process I've removed accounts.google.com from the frame-src portion (added in bcdcb2e) since I can't see how it would be used (although @andrew-nowak might have an idea).

this is necessary at least for the guardian for panda-session to create the iframe to relog users in after their panda cookie expires, as the reauth will include a redirect through accounts.google.com in that iframe

…alues from config to remove guardian specific stuff
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants