Add secure attribute to cookies if served over HTTPS

[ties-v]

By submitting a PR to this repository, you agree to the terms within the Auth0 Code of Conduct. Please see the contributing guidelines for how to create and submit a high-quality PR for this repo.

Description

This PR adds the secure flag to the cookies used by auth0-spa-js, when the web app is served over https. Otherwise, it doesn't change anything.

---

Old description
This PR adds samesite=strict to the cookies used by auth0-spa-js. This because browsers will reject cookies with samesite=none (which is the default) when served over http in the future.

References

See #459

Testing

• This change adds test coverage for new/changed/fixed functionality

Checklist

• I have added documentation for new/changed functionality in this PR or in auth0.com/docs
• All active GitHub checks for tests, formatting, and security are passing
• The correct base branch is being used, if not master

[stevehobbsdev]

Thanks for raising @ties-v - let me review and get back to you.

[natbusa]

please if this solution is correct, approve it, Currently, I have to manually delete the auth0.is.authenticated cookie every time I reload my page on vue.js

[stevehobbsdev]

@natbusa that's strange - why are you having to delete the cookie when your app starts?

[natbusa]

Hi @stevehobbsdev, I am using the vue.js spa demo provided by auth0 https://github.com/auth0-samples/auth0-vue-samples aa a primer for my spa design. After refreshing the page the auth0.is.authenticated cookie is left back. After reloading however oauth0 cookies fail same site check on chrome. I was hoping this PR could help.

[stevehobbsdev]

@natbusa Thanks. To be clear, nothing is failing here, it's a warning about the cookie possibly being removed in the future. As it stands at the moment your app will still work just fine.

Removing the cookie manually could cause unintended side effects. For example, you might find that your users will suddenly appear logged out when they refresh the page.

[stevehobbsdev]

@ties-v We've had a look at this and setting SameSite=strict will not work for us here. The cookie will not be able to be read on page load - this change affects not only the auth0.is.authenticated cookie but the transaction cookies as well. You should have found in your testing that when you try to complete a login flow, you should have got an error complaining about invalid state (because it can't read the cookie).

So we're proposing the following, if you'd like to update the PR:

• On HTTPS origins use SameSite=None and secure=true
• On non-HTTPS origins, don't set either the SameSite or the Secure attributes at all

For HTTP origins, this means that new browsers will default to SameSite=Lax and older browsers will default to SameSite=None, and this should all just work everywhere.

Let me know your thoughts. Unit tests will need to be updated as well when you can find an environment to run them in.

[ties-v]

@stevehobbsdev Thanks for the clear feedback. It seems that once again something is not as simple as it looks 😉.
Today I was able to get the test suites up and running on my ubuntu environment, so I am probably good to go.

The suggested strategy is a good one I think, so I'll implement it somewhere next week. Just to be sure: the proposed cookie flags are applicable to all cookies that the lib uses, right?

[stevehobbsdev]

Just marking as "request changes" for visibility.

Just to be sure: the proposed cookie flags are applicable to all cookies that the lib uses, right?

Yes, all our cookies go through the storage broker, so it makes sense to make these changes there.

[ties-v]

I've updated the code as discussed. But I was thinking if we could default (explicitly) to samesite=lax in all cases, as this is / becomes the default in browsers? Or is this too much a breaking change?

[stevehobbsdev]

@ties-v Thanks for the updates. Let's stick the plan and be explicit about setting SameSite=None for secure browser requests, and no value (browser default) for non-secure requests.

[ties-v]

Added sameSite=None. Had to change the logic a bit because typescript only allows sameSite='none' when secure=true in object constructor.

[stevehobbsdev]

Thanks @ties-v - let me review this week!