Skip to content
This repository has been archived by the owner on Apr 5, 2024. It is now read-only.

Headers #365

Merged
merged 3 commits into from Jan 13, 2020
Merged

Headers #365

merged 3 commits into from Jan 13, 2020

Conversation

XhmikosR
Copy link
Collaborator

  • Move referrer header in Hapi's security object
  • Switch to blankie for CSP

index.js Show resolved Hide resolved
@@ -38,12 +51,16 @@ const REFERRER_HEADER = 'no-referrer, strict-origin-when-cross-origin';
maxAge: 31536000,
preload: true
},
referrer: 'strict-origin-when-cross-origin',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before it was "no-referrer, strict-origin-when-origin", why the change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't find the exact thing we were using in the Hapi security options, so I went with the closest one... https://hapi.dev/api/?v=18.4.0#-routeoptionssecurity

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, after reading the docs, it seems that no-referrer is a fallback for when browsers don't support strict-origin-when-origin. Unfortunately, it doesn't seem hapi allows us to do this:

[1] "referrer" must be a boolean
[2] "referrer" must be one of [no-referrer, no-referrer-when-downgrade, unsafe-url, same-origin, origin, strict-origin, origin-when-cross-origin, strict-origin-when-cross-origin]

We could try asking upstream for this, not sure if they'll accept this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't find any modern browser that doesn't support strict-origin-when-cross-origin.

I believe the policy was written when the spec wasn't completely implemented by Chrome.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then if you think it's OK I guess we can move with merging this PR?

@XhmikosR XhmikosR marked this pull request as ready for review January 7, 2020 08:59
@XhmikosR XhmikosR force-pushed the headers branch 2 times, most recently from ba75ca1 to 303f1f6 Compare January 8, 2020 11:10
@XhmikosR
Copy link
Collaborator Author

@mozfreddyb I'd like to get this merged if you agree with the changes.

@XhmikosR XhmikosR mentioned this pull request Jan 12, 2020
Copy link
Contributor

@mozfreddyb mozfreddyb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's do this :)

@XhmikosR XhmikosR merged commit a064ec4 into master Jan 13, 2020
@XhmikosR XhmikosR deleted the headers branch January 13, 2020 11:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants