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

Issue #206 Add options for Cookie SameSite attribute #207

Merged
merged 10 commits into from
Dec 17, 2020

Conversation

tsujiguchitky
Copy link
Contributor

@tsujiguchitky tsujiguchitky commented Jan 23, 2020

Analysis

Chrome treats cookies that have no declared SameSite value as SameSite=Lax cookies.
As a result, OpenAM has some troubles (See #206).

Solution

Add the following settings.

  • Default value of SameSite attribute
    • If this value is the initial value, the SameSite attribute is not set.
  • SameSite setting list
    • Enable to specify each cookie in {Cookie Name}={SameSite Value} format.
    • This value works to override the default value above
    • By default, amlbcookie and authenticationStep are set to None

Also, add a properties file (SameSiteIncompatibleClient.properties) that excludes user agents, because there are incompatible browsers with SameSite.

Compatibility

This fix adds a parameter to CookieUtils.addCookieToResponse(). If using this method in a customized authentication module etc., rebuild it.

Testing

  • SameSite setting patterns
    • Set for all cookies
    • Not set for all cookies
    • amlbcookie only (server side cookie)
    • authId only (client side cookies)
    • amlbcookie & authenticationStep (server side cookies)
    • authId & iPlanetDirectoryPro (client side cookies)
  • Chrome & SameSite=None
    • SAML2 Authentication
    • XUI OFF & CDSSO & Session upgrade

Regression testing

  • Authentication succeeds if the SameSite attribute is not set
    • Chrome
    • Firefox
    • Edge
    • IE
  • Authentication succeeds if SameSite=None
    • Chrome
    • Firefox
    • Edge
    • IE

@tsujiguchitky tsujiguchitky self-assigned this Jan 23, 2020
@ogis-osada ogis-osada self-requested a review December 17, 2020 05:01
@ogis-osada ogis-osada merged commit ab98407 into master Dec 17, 2020
@ogis-osada ogis-osada deleted the issues/206-cookie-samesite branch December 17, 2020 05:02
@ogis-osada
Copy link
Contributor

close #206

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.

2 participants