Skip to content

fix: force the Secure cookie flag when SameSite=None applies - #861

Open
kishore7snehil wants to merge 3 commits into
mainfrom
fix/force-secure-cookie-samesite-none
Open

fix: force the Secure cookie flag when SameSite=None applies#861
kishore7snehil wants to merge 3 commits into
mainfrom
fix/force-secure-cookie-samesite-none

Conversation

@kishore7snehil

Copy link
Copy Markdown
Contributor

Changes

When responseMode is form_post, both CookieStore and SessionStore unconditionally set the cookie SameSite attribute to None, because a cross-origin POST callback requires it. However, the secure flag was still taken from cookieSecure (default false), so the SDK could emit a SameSite=None cookie without Secure. Browsers reject a SameSite=None cookie that does not also carry Secure (RFC 6265bis), so this combination results in the cookie being dropped on modern browsers.

This PR forces the Secure flag on whenever the resolved SameSite is None:

🔒 Security Fix:

  • CookieStore::getCookieOptions() now sets secure to true when the resolved samesite value is None, after the existing SameSite normalization, so both form_post and an explicit cookieSameSite of None are covered
  • SessionStore::start() applies the same rule to the native PHP session cookie params, forcing secure on when SameSite=None is in effect
  • cookieSecure still defaults to false and is otherwise unchanged. The $cookieSecure documentation now notes that Secure is forced on when SameSite=None applies and recommends enabling it in production

Applications using the default query response mode are unaffected, as their cookies keep the same attributes. For form_post applications, the session and transient cookies now always carry Secure and are therefore only sent over HTTPS. Such deployments should serve the callback over HTTPS.

References

N/A

Testing

  • Added SameSite=None forces the Secure flag on even when cookieSecure is false to the CookieStore unit tests, asserting that form_post yields samesite=None with secure=true

  • This change adds unit test coverage

  • This change has been tested on the latest version of the platform/language

Contributor Checklist

@kishore7snehil
kishore7snehil requested a review from a team as a code owner August 31, 2026 09:08
kishore7snehil added a commit that referenced this pull request Aug 31, 2026
…ersions (#862)

### Changes

CI installs its dev tools from open version ranges and there is no
committed `composer.lock`, so it recently pulled in a newer Rector
(2.6.x). That version enables `IfToNullCoalescingAssignRector`, which is
why the Rector job started failing on unrelated PRs.

- The rule rewrites `if (null === $x) { $x = ...; }` guards to `$x ??=
...`, which is equivalent.
- This affected four pre-existing spots: `src/Token.php`,
`src/Token/Parser.php`, `src/Utility/HttpResponsePaginator.php`, and
`src/Utility/HttpTelemetry.php`.
- In `src/Token/Parser.php` the `@codeCoverageIgnore` markers were kept
around the rewritten line, since it remains untestable with the current
JWT encoding test libraries.
- Behavior is unchanged.

### References

Unblocks the Rector CI job on open PRs (#852, #853, #861).

### Testing

- [ ] This change adds unit test coverage
- [x] This change has been tested on the latest version of the
platform/language

### Contributor Checklist

- [x] I have read the [Auth0 general contribution
guidelines](https://github.com/auth0/open-source-template/blob/master/GENERAL-CONTRIBUTING.md)
- [x] I have read the [Auth0 code of
conduct](https://github.com/auth0/open-source-template/blob/master/CODE-OF-CONDUCT.md)
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.

1 participant