Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
I have a website where some of the users are authenticated through SAML. However most users are not (and none of the admin users are).
In simplesamlphp_auth_user_logout() the current user's session is destroyed to avoid infinite loops.
However, this breaks the functionality of the Masquerade module (that offers admin the ability to login as another user).
Can we commit this patch that will make the code in simplesamlphp_auth_user_logout() inactive for non-SAML authenticated users?
Comment | File | Size | Author |
---|---|---|---|
logout_only_when_active.patch | 1.39 KB | bjaxelsen | |
Comments
Comment #2
dpagini CreditAttribution: dpagini as a volunteer commentedI am having a very similar problem with custom logout redirects. SimpleSAMLPHP allows you to set a logout redirect URL, but not really dynamically. Since `simplesamlphp_auth_user_logout()` hijacks the logout stack, my custom redirect code gets completely skipped. This PR would help me with some of that issue if it only affects the SAML users (employees) and not our end clients.
Just a question, the Drupal core `user_logout` function uses the `session_handler` service, which I sort of think SimpleSaml should do as well, and set the user to anonymous, since that code is being "skipped".
So it seems like the "session_destory()" line should be replaced with the following...
Thoughts?
Comment #3
dpagini CreditAttribution: dpagini as a volunteer commentedComment #4
dpagini CreditAttribution: dpagini as a volunteer commentedI've been using this patch and it is working successfully for me. I'm going to mark as RTBC. I'm going to open a new ticket for my suggestion in comment #2 and submit my own patch, since it is sort of a separate issue.
Comment #5
trwill CreditAttribution: trwill commentedDoes anything else need to be done for this patch?
The hook_user_logout function is very heavy handed and basically breaks any other hooks and/or event subscribers that are running on logout. This is exactly the fix needed for those.
Comment #6
joelpittetThis may need a reroll if #2900442: Use Drupal session manager in logout gets in first.
RTBC++
Comment #7
dakku CreditAttribution: dakku as a volunteer commentedThanks guys, I will get this committed later today.
Comment #9
dakku CreditAttribution: dakku as a volunteer commentedComment #11
partyka CreditAttribution: partyka commentedHi,
Thank you for fixing this -- but I've noticed it's still in the 'dev' branch and yet to be incorporated into a released version. Please consider this a request to make a new release with this fix in it.
Thanks!
--partyka
Comment #12
Kuldeep K CreditAttribution: Kuldeep K commentedDoes it also work for another custom "hook_user_logout"? I want to destroy some extra cookies when user getting logged out but saml logout won't allow me to do that from my custom logout.
Comment #13
etiennejacquot CreditAttribution: etiennejacquot commentedWas this ever fixed? I still get issues using 8.x-2.0-rc4 on Drupal 9.5.9 when trying to masquerade as a SAML authenticated user
Error: Call to undefined method Drupal\user\Entity\User::setAccount() in simplesamlphp_auth_user_logout() (line 68 of /code/web/modules/contrib/simplesamlphp_auth/simplesamlphp_auth.module)