Trying to use both the legal module and simplesamlphp_auth causes an infinite login loop due to Legal destroying and recreating the users session in legal_user_login. The issue is that simplesamlphp_auth is configured to redirect all users to the SSO login before they can access the site and when logging in simplesaml uses hook_user_login to set a session value tied to simplesaml. When Legal invokes hook_user_logout simplesaml deletes the simplesaml session value so when the user is redirected to /legal_accept they no longer have an active simplesaml session and are redirected to the SSO login. This repeats forever.
Issue code:
legal/legal.module:456
>
// Log the user out and regenerate the Drupal session.
\Drupal::logger('user')
->notice('Session closed for %name.', array('%name' => $account->getAccountName()));
\Drupal::moduleHandler()->invokeAll('user_logout', array($account));
// Destroy the current session, and reset $user to the anonymous user.
\Drupal::service('session_manager')->destroy();
<Instead of logging users in and out during hook_user_login we should create an event subscriber that subscribes to kernel requests and if the user needs to accept a T&C then redirect them.
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | legal-event_based_tc_redirect-3005208-6.patch | 15.92 KB | radelson |
| #7 | interdiff_3_7.txt | 4.82 KB | radelson |
Comments
Comment #2
ctrladelComment #3
ctrladelHere's a first go at changing over to use a kernel::request subscriber instead of hooking into user_login. I recognize this represents a significant change in how the module currently works but using a redirect on requests feels a lot cleaner and more Symfony friendly than the current approach of manipulating Drupal's login/session management.
Quick summary of the patch:
Most of the logic remains the same for determining if T&C needs to be accepted or not. I made use of the existing "legal_login" session value to help determine if the redirect to "legal_accept" should be active. I removed the hash/token code since we are now working with a fully authenticated user session so Drupal's form handling should prevent most nefarious behavior.
Feedback is welcome.
Comment #4
ctrladelComment #6
mstrelan commentedA couple issues with the patch.
Comment #7
radelson commentedI made some modifications to ctrlADel's patch to make it work with password reset links
Let me know if I can improve it.
Comment #8
avpadernoBugs are fixed in the development snapshot.
Comment #9
mstrelan commentedThis seems to be a duplicate of #2897486: Don't log out users who do not accept the T&C. The patch in that issue seems to work better for me, specifically in the handling of the redirect after accepting the T&C's.