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

auth: use samesite=lax for oauth cookies #3158

Merged
merged 1 commit into from
Jul 11, 2023
Merged

Conversation

mastercactapus
Copy link
Member

Description:
Fixes a regression that prevents login when redirecting from an OIDC server by setting SameSite=Lax for OAuth cookies, and retaining SameSite=Strict for the session cookie.

Additional Info:

  1. SameSite=None: Cookies will be sent in all contexts. This was how cookies worked by default prior to the SameSite attribute implementation.

  2. SameSite=Lax: Cookies are not sent on normal cross-site subrequests (for example, to load images or frames into a third-party site), but are sent when a user is navigating to the origin site. This is the default value in modern browsers if the attribute is not specified.

  3. SameSite=Strict: Cookies will only be sent in a first-party context and not be sent along with requests initiated by third-party websites. This means that the cookies won't be sent if the user is not already on your site.

Copy link
Collaborator

@allending313 allending313 left a comment

Choose a reason for hiding this comment

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

lgtm!

@Forfold Forfold self-requested a review July 11, 2023 18:46
Copy link
Contributor

@Forfold Forfold left a comment

Choose a reason for hiding this comment

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

lgtm & functionally tested

@Forfold Forfold merged commit df8b7dc into master Jul 11, 2023
6 checks passed
@Forfold Forfold deleted the samesite-oauth-fix branch July 11, 2023 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants