The programmatic login process used in the main callback (urllogin_link_page()) may introduce some problems for cases of user switching (where user A is already logged in but then follows the login URL generated for user B). Though user switching is likely not a main use case for the module, it will certainly happen in practice, and should either be properly supported or disallowed entirely.

urllogin_link_page() has a conditional check to see if the current user is authenticated based on whether-or-not the current uid matches the uid specified in the login URL. However, it does not further differentiate to see if there is a current logged-in user with a different uid than the one specified, and instead it treats this "user switching" case the same way it treats a case where the current user is anonymous. The resulting logic then leverages some drupal methods (like user_login_finalize()) that I believe are really only meant for anonymous-to-authenticated transitions. Though things appear to work on the surface (the user specified in the login link gets logged in), the following issues arise:

  1. The session variables from user A are maintained for user B. This could lead to several unpredictable conditions (for setups using SSO or leveraging any form of user-specific state info inside a session).
  2. user_logout hooks are not invoked for user A.

To get around this the module could either disallow user switching or could ensure a proper logout sequence is triggered for the current user when a user switching case is detected.

CommentFileSizeAuthor
#2 urllogin-properly_support_user_switching-2613632-2.patch2.68 KBrjacobs

Comments

rjacobs created an issue. See original summary.

rjacobs’s picture

rjacobs commented
Status: Active » Needs review
StatusFileSize
new urllogin-properly_support_user_switching-2613632-2.patch2.68 KB

Here's a patch that offers one way to address this. It does not change the end-user behavior of the module in any way, but it forces a full Drupal logout when user switching is detected and then transparently redirects back to the same path so that the login URL can be re-run via an anonymous user request.

A nice side-product of the patch is that is also cleans up some of the general query-string handling in urllogin_link_page() (while also maintaining the query string across the added logout).