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

Add Rack::Protection::ReferrerPolicy #1291

Merged
merged 1 commit into from Mar 13, 2020

Conversation

@stefansundin
Copy link
Contributor

@stefansundin stefansundin commented May 7, 2017

Hi everyone. I decided to implement the Referrer-Policy header for rack-protection. It's a really simple header with just a string value, more information:

I considered making it enabled by default, since it has low risk of breaking the web, but I want your opinion first.

Worth noting is that the default value I picked, "strict-origin-when-cross-origin", does not work in Chrome at the moment. I picked it as it will be the most sensible default in the future, especially if this is enabled by default. See this bug: https://bugs.chromium.org/p/chromium/issues/detail?id=627968

@Ice-Storm
Copy link
Contributor

@Ice-Storm Ice-Storm commented Jul 11, 2017

many browers not support this header. but it's future ~

@olleolleolle
Copy link
Member

@olleolleolle olleolleolle commented Aug 19, 2017

@stefansundin Thanks for creating this.

The Chrome issue you linked to has since been marked fixed.

Implement new referrer policies

This CL implements the policies 'same-origin' and 'strict-origin', and
repurposes existing logic that was previously only available behind a flag for
'strict-origin-when-cross-origin'. (I've left it as a TODO for a follow-up
to rename this policy to match the spec.)

Existing web platform tests cover the new policies and should now pass.

Intent to Implement and Ship:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/TgtPUowSWuU/Y-Sn2oRsCAAJ

Does that fact change any of your earlier assessment of the mergeability of this change?

@stefansundin
Copy link
Contributor Author

@stefansundin stefansundin commented Aug 19, 2017

That note in the issue is confusing to say the least. Anyway, when Chrome 61 ships, it will be easy to test if the default value works as expected.

@zzak
Copy link
Member

@zzak zzak commented Aug 20, 2017

Let's wait for chrome 61 to test this then. My reading of this seems to be that this name is only temporary? And will likely change to match the spec? Not sure which spec they're... referring to here tho :)

@stefansundin
Copy link
Contributor Author

@stefansundin stefansundin commented Aug 23, 2017

This post suggests that the policy name hasn't changed: https://blog.chromium.org/2017/08/chrome-61-beta-javascript-modules.html

I think the note was talking about renaming something internally. But we can still wait.

@namusyaka namusyaka added this to the Beyond milestone Feb 19, 2018
@stefansundin
Copy link
Contributor Author

@stefansundin stefansundin commented Feb 18, 2019

@zzak I think this can merged now. :)

@jkowens jkowens merged commit fade5fe into sinatra:master Mar 13, 2020
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jkowens
Copy link
Member

@jkowens jkowens commented Mar 13, 2020

Thanks @stefansundin, I think at some point we may want to circle back and enable this protection by default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.