Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: broken linked account feature #19565

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: broken linked account feature #19565

wants to merge 1 commit into from

Conversation

netroms
Copy link
Contributor

@netroms netroms commented Dec 26, 2024

Problem:

Users could not switch between linked account users.

Cause:

When the log out handler (DhisOidcLogoutSuccessHandler), was called and tried to set the user we want to switch to with the method setActiveLinkedAccounts(), nothing happened because the there was no transaction involved, because it called the UserStore directly, instead of going through the service layer.
So when it tried to persist the updated lastLogin field, nothing happened.

Solution:

Add the setActiveLinkedAccounts() method to the UserService and annotate it with @transactional and make the log out handler call the new method.

Automatic tests:

Automatic tests not available, because it needs a full setup with an external IDP and tests with E2E.

Manual test:

  1. Enable a OIDC provider. (See documentation for setting up with Google for example).
  2. Enable linked account. linked_accounts.enabled = on and linked_accounts.relogin_url = http://localhost:8080 in dhis.conf
  3. Create two users that has the same OIDC mapping.
  4. Log in as user A.
  5. Try to log out and switch to user B, with URL like this: http://localhost:8080/dhis-web-commons-security/logout.action?switch=userb&current=usera
  6. Observe next time you log in (with OIDC) you now get logged in as user B.

Signed-off-by: Morten Svanaes <msvanaes@dhis2.org>
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.

3 participants