Add cache-control & pragma headers to redirect responses #19
Conversation
index.js
Outdated
| }], | ||
| 'cache-control': [{ | ||
| key: 'Cache-Control', | ||
| value: 'no-cache, no-store, max-age=0, must-revalidate' |
There was a problem hiding this comment.
The MDN documentation page for Cache-Control discourages putting that many directives in the header. It seems that no-store, max-age=0 would achieve the same results.
Is there a reason (such as a browser-specific edge case) you are providing redundant directives?
There was a problem hiding this comment.
Good point. Reading through that I do agree that we can reduce this here to no-store, max-age=0. For context, I copied the headers here from other auth providers (Midway & Federate), which do provide the redundant headers.
There was a problem hiding this comment.
After investigating, browser support for the cache-control directives appears to be inconsistent so let's keep those redundant headers. My apologies for the delay in merging this and thanks for your contribution.
Issue # (if available): #18
Description of changes:
Add cache-control and pragma headers to 302 responses. This is my first time opening a PR for an OS project. I'd suggest updating the package version to 1.2 as well. Apologies, as I'm not sure if that's something I should do in the PR submission or if that is done at the time of merge. Let me know if any update is required.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.