silverstripe / silverstripe-realme

Other
9 stars 24 forks source link

Signing into RealMe loses session data in SilverStripe #165

Closed torleif closed 3 weeks ago

torleif commented 1 month ago

Module version(s) affected

5.3.2

Description

The default Session variable in SilverStripe 5 has the SameSite=Lax setting enabled.

SameSite Cookie Policy: The SameSite=Lax setting allows cookies to be sent with cross-site GET requests but not POST requests. Since the RealMe return uses a POSTrequest, the session cookie is not sent back, causing the session to reset.

How to reproduce

  1. Create a session variable in your SilverStripe instance.
  2. Sign in to RealMe
  3. Your session will be reset

Possible Solution

Change the default cookie settings, or enable a way to pass through additional data

Additional Context

No response

Validations

GuySartorelli commented 1 month ago

When you say "Your session will be reset" - what are the actual effects of that? What problems does this cause, specifically?

GuySartorelli commented 3 weeks ago

Closing as I cannot reproduce and additional information was not provided when requested.

torleif commented 3 weeks ago

Appologies - when i say my session was reset - all session varables set $_Session[...] etc get erased

NightJar commented 1 day ago

I'm not sure there's a way around this within the module. Because the session cookie is absent in the request (per SameSite setting) a new session cookie is created. This then overrides whatever was set before the AuthN at the IdP. Without a session existing, there is no way to persist state between requests, thus no one could log in.

Other than explicitly loosening the cookie attributes before redirection to RealMe (which seems counter-intuitive), I don't have enough experience to know if there's some standard way to deal with the problem.

NightJar commented 1 day ago

Ah, duh. That's what RelayState is for, of course.

It needs to be checked properly, of course (ensure it's a site URL before using it) https://github.com/silverstripe/silverstripe-saml/commit/798d5036604ecafada69fa1bfe65e516bfc1f683

@torleif are you able to inspect your SAML response to see if the request var is set? It should be falling back to it (and is checked for validSiteURL). https://github.com/silverstripe/silverstripe-realme/blob/214710edeb3d0aa18229970515bf2a77c9c05f75/src/RealMeService.php#L654-L657