Skip to content

Fix login when a different user is already logged in - #125

Merged
dmitriim merged 2 commits into
catalyst:MOODLE_405_STABLEfrom
MartinR2295:fix/switch-user-session-regenerate
Sep 3, 2026
Merged

dmitriim merged 2 commits into
catalyst:MOODLE_405_STABLEfrom
MartinR2295:fix/switch-user-session-regenerate

Conversation

@MartinR2295

Copy link
Copy Markdown

Hi! 👋 Thanks for maintaining this plugin — we're using it for an SSO integration and it has saved
us a lot of work.

While building that integration we ran into the problem described in #102, dug into the root cause,
and this PR is our attempt at a fix. You asked there for a PR with automated tests, so here it is —
very happy to adjust the approach if you'd prefer a different one.

require_logout() closes the PHP session, so completing the login in the same
request made complete_user_login()'s session_regenerate_id() run against a
closed session. Log the old user out and redirect back to the login endpoint
so the key login runs on a clean session; the key is not consumed until after
that branch, so it is still valid on the second pass.

Fixes #102

  require_logout() closes the PHP session, so completing the login in the same
  request made complete_user_login()'s session_regenerate_id() run against a
  closed session. Log the old user out and redirect back to the login endpoint
  so the key login runs on a clean session; the key is not consumed until after
  that branch, so it is still valid on the second pass.

  Fixes catalyst#102
@dmitriim

dmitriim commented Aug 2, 2026

Copy link
Copy Markdown
Member

Thanks @MartinR2295 for working on the patch. It looks ok from the first glance. It has automated tests which is great.
Can you please add manual testing instructions so we can verify the patch. Thanks!

@MartinR2295

Copy link
Copy Markdown
Author

Sure 👍

  • A Moodle 5.0 dev site with developer debugging on and displayed (Site administration → Development → Debugging: DEVELOPER + Display debug messages = Yes). -> without it, the bug shows up only as a login that silently does not happen.
  • Plugin settings: Mapping field = Email address, User key life time = 60, IP restriction = No (tightened again in test 7).
  • Enabled web services + the REST protocol, create a token for the User key authentication web service, give that user the auth/userkey:generatekey capability and add it to the service's authorised users (README steps 5–9).
  • Two users: A (whoever is already logged in — admin is fine, but we tested it with a via SSO logged in user) and B (the SSO target).

Steps to reproduce:

  • In a browser, log in normally as A (or do it via login URL).
  • Mint a login URL for B and append a wantsurl, e.g. …/auth/userkey/login.php?key=&wantsurl=https://MOODLE/my/.
  • Open that URL in the same browser session (not incognito).

With this PR the login should be working without showing an error. In the prev. version, the login wasn't working, and when debugging is activated, an error message appears.

Happy to clarify anything if needed. 🙂

@GigaUmbo

Copy link
Copy Markdown

@dmitriim Please review and apply this patch (we need it) thanks

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes SSO login when another user is already authenticated by restarting login after logout.

Changes:

  • Redirects back to the key-login endpoint after logout.
  • Preserves the key and destination URL.
  • Adds mismatch-session regression tests.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
auth.php Implements clean-session login redirect flow.
tests/auth_plugin_test.php Tests logout, redirect, and subsequent login.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tests/auth_plugin_test.php Outdated

/**
* Test that if one user logged, he will be logged out before a new one is authorised.
* Test that if one user logged, he will be logged out and redirected back to the login endpoint.
@dmitriim

Copy link
Copy Markdown
Member

@MartinR2295
Thank you. This all looks good.
AI review looks minor and it's up to you to fix.
Lat thing before merging in - can you please bump version like 2025120300 -> 2025120301?

@MartinR2295

Copy link
Copy Markdown
Author

done 👍

@dmitriim
dmitriim merged commit 64f614b into catalyst:MOODLE_405_STABLE Sep 3, 2026
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow login if a different session is open

4 participants