Skip to content

Allow cookies to always have the Secure attribute if configured#59

Merged
lbalmaceda merged 3 commits intoauth0:masterfrom
jimmyjames:set-secure-cookie-attribute
Jul 2, 2020
Merged

Allow cookies to always have the Secure attribute if configured#59
lbalmaceda merged 3 commits intoauth0:masterfrom
jimmyjames:set-secure-cookie-attribute

Conversation

@jimmyjames
Copy link
Copy Markdown
Contributor

@jimmyjames jimmyjames commented Jul 1, 2020

Changes

Currently, cookies used for the state and nonce during the authentication flow only set the Secure attribute if SameSite=None (this happens if the response type includes id_token). This change allows configuration to always set the Secure attribute, regardless of the SameSite value.

References

Closes #58

Testing

  • This change adds test coverage
  • This change has been tested on the latest version of Java or why not

Checklist

@jimmyjames jimmyjames added this to the v1-Next milestone Jul 1, 2020
@jimmyjames jimmyjames marked this pull request as ready for review July 1, 2020 22:21
@jimmyjames jimmyjames requested review from a team and removed request for a team July 1, 2020 22:21
@jimmyjames jimmyjames added the medium Medium review label Jul 2, 2020
@jimmyjames jimmyjames requested a review from a team July 2, 2020 01:37
Comment thread src/main/java/com/auth0/AuthorizeUrl.java Outdated
public void shouldSetSecureCookieWhenConfigured() {
String url = new AuthorizeUrl(client, request, response, "https://redirect.to/me", "code")
.withState("asdfghjkl")
.withSecureCookie(true)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd add a test with this set to false, checking that when samesite=none this is still being set as secure even if we tell it not to with the setter. (the default behavior)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Pushed a commit to add this test case 👍

jimmyjames and others added 2 commits July 2, 2020 08:41
@jimmyjames jimmyjames requested a review from lbalmaceda July 2, 2020 14:00
@lbalmaceda lbalmaceda merged commit b73777d into auth0:master Jul 2, 2020
@jimmyjames jimmyjames modified the milestones: v1-Next, 1.3.0 Jul 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CH: Added medium Medium review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Using Secure Cookie flag when using 'Authorization Code Flow'

2 participants