Problem/Motivation

When "Log out different user upon re-authentication." is selected, the newly authenticated user is not logged in.

Steps to reproduce

1. Set up SAML authentication with "Log out different user upon re-authentication." checked
2. Log in via SAML
3. Trigger re-authentication, e.g. via redirecting to the relevant IDP URL
4. Log in as a different user

Expected result

You should be logged in as the new user

Actual result

Although the logs show a session being created, you are not logged in.
If the redirect URL is a restricted path, an error message appears:

To log in to this site, your browser must accept cookies from the domain 

  ID      Date           Type             Severity   Message
 ------- -------------- ---------------- ---------- ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
  19116   27/Jul 17:59   access denied    Warning    Path: /user/dashboard/phone?check_logged_in=1. Drupal\Core\Http\Exception\CacheableAccessDeniedHttpException: This route can only be accessed by authenticated users. in Drupal\Core\Routing
  19111   27/Jul 17:59   externalauth     Notice     External login of user <second user>
  19106   27/Jul 17:59   user             Notice     Session opened for <second user>.
  19101   27/Jul 17:59   externalauth     Debug      Authmap change (3f653d0e-048d-462e-bc90-20e640f82baa => 716abc0c-dc5b-4c9d-93da-b977a7207d6b) for user <second user> with uid 636 from provider samlauth
  19096   27/Jul 17:59   samlauth         Info       SAML login for name <second user> (as provided in a SAML attribute) matches existing Drupal account 636; linking account and logging in.
  19091   27/Jul 17:59   samlauth         Debug      No matching local users found for unique SAML ID 716abc0c-dc5b-4c9d-93da-b977a7207d6b.
  19086   27/Jul 17:59   user             Notice     Session closed for <first user>.

Proposed resolution

tbc

Comments

malcomio created an issue. See original summary.

malcomio’s picture

malcomio commented
Issue summary: View changes
alex_sansom’s picture

alex_sansom commented

This seems to be because when user_logout() gets called it destroys the session. If you only invalidate the session rather than destroy it, you're able to switch users OK.

  • roderik committed 02be295f on 8.x-3.x 
    Issue #3377532 by roderik, alex_sansom, malcomio: Fix "Log out different...
roderik’s picture

roderik
Primary language Dutch
Location Amsterdam,NL / Budapest,HU
commented
Status: Active » Fixed

Thanks for that last comment, it convinced me it must be the module and I'd have to take a look.

This meant that I couldn't delay learning more about session handling... because I couldn't just go along with the part of "not destroying the session", without knowing more:

  • Would that run any risk of data from the previous session into the next?
  • That would mean we can't use Core's helper method (user_logout()). How risky is that?
  • Am I sure about what you mean with "the session" anyway?

...so... several weeks later I emerged out of this rabbit hole with detailed knowledge about Drupal's Session code, two potential blog posts and at least three Core issues to work on. (I won't yet, because I really need to do other things now. They may appear as related issues here... sometime.)

As it turns out:

  • This issue has existed (the option hasn't worked) since Drupal 9.2, or #2238561: Use the default PHP session ID instead of generating a custom one
  • Before this, the option was... not very nice either (details withheld). Reading that older code proved to me that doing a logout + login was never really supported.
  • I guess you meant specifically "call Session::invalidate() instead of SessionManager::destroy()".
  • I agree now that this is the solution. Which unfortunately means that I can't call user_logout(), and we are not protected against any future changes to that function. No other workaround/preparation currently works.
  • Luckily, I'm now sure that the comment against calling Session::invalidate(), present in user_logout(), isn't actually true.

Further: I couldn't reproduce this on D10. As it turns out, this whole thing isn't an issue / the configuration option is unimportant if you set the 'samesite policy' for the session cookie to either "Strict" or "Lax" (because Lax also strips the cookie from the POST request), if the IdP is on a different domain (or hostname, depending on cookie domain settings, I guess).

Change is in the commit I just pushed. The comments/changes around PrivateTempStorage can be ignored; they are just adjustments to other changes in D9 core.

I added a /saml/reauth route for easier testing (though I'm not documenting it; it's harmless but I don't see the usefulness for anything except testing re-login of a different user).

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.