When using Masquerade with this module, Masquerading in a variety of ways, including masquarading as a user who has not authenticated and some other combinations, will cause logging out instead of masquerading.

CommentFileSizeAuthor
#6 check-session-for-masquarading-on-event-listener2975184-6.patch2.31 KBberdir
#2 check-session-for-masquarading-on-event-listener2975184-1.patch611 bytesjnicola

Comments

jnicola created an issue. See original summary.

jnicola’s picture

jnicola commented
StatusFileSize
new check-session-for-masquarading-on-event-listener2975184-1.patch611 bytes

Looking for $_SESSION['masquerading'] in SimplesamlSubscriber->checkAuthStatus may allow Masquarade to work effectively with this module.

WARNING: This has potential security implications and has not been tested at this time!

vood002’s picture

vood002 commented

I'm using 8.x-3.x-dev and was experiencing this issue. This patch solved the issue for me~

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

Note: You also need to patch hook_user_logout(): #2124117: Unable to masquerade when logging in via SAML

andypost’s picture

andypost
he/him
Primary language Russian
commented

masquerade no longer use session https://cgit.drupalcode.org/masquerade/commit/?id=3dfe12a

The suggestion is to check that masquerade service exists in container and call its method isMasquerading() https://cgit.drupalcode.org/masquerade/tree/src/Masquerade.php#n135

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs work » Needs review
StatusFileSize
new check-session-for-masquarading-on-event-listener2975184-6.patch2.31 KB

Well, it's still exactly the same session key behind the scenes, but yeah, we should use the API even though it is a bit more complicated, as we have to integrate with the service optionally.

fengtan’s picture

fengtan
Location Montreal, Canada
commented

Successfully tested #6 -- thanks. The code change also makes sense.

I have noticed there is a related issue (#2124117: Unable to masquerade when logging in via SAML) with a different patch, although I did not have to apply the patch there in order to fix the problem.

vijaycs85’s picture

vijaycs85
Location London, UK
commented
Title: Masquarading as user not authenticated causes logout » Masquerading as user not authenticated causes logout

I have tested the patch in #6 and confirm it is working as expected.

Though the solution looks good, I wonder it would be a good idea to provide a way for other modules to react rather than adding the code here. For example, we dispatch an event so that Masquerade does the check to let the user go.

hazn’s picture

hazn commented
Status: Needs review » Reviewed & tested by the community

Looks good to me :) Thanks!