Skip to content

Conversation

@bhavenst
Copy link
Contributor

Description

The determine_username_from_uid_social pipeline was selecting the 'username' value from the pipeline kwargs/details, which is generally correct for most authenticators, however for SAML specifically the 'uid' is not the exact username, but the username prepended with 'IdP:' (hard coded for our SAML authenticators). The incorrect use of 'username' was causing the AuthenticatorUser not to be found. This was OK in the case that another user didn't exist with the same email, but if such a user does exist, the authentication process errors out with a "more than one user found" error, blocking the SAML user from logging in permanently unless the "conflicting" local user was removed.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Test update
  • Refactoring (no functional changes)
  • Development environment change
  • Configuration change

Self-Review Checklist

  • I have performed a self-review of my code
  • I have added relevant comments to complex code sections
  • I have updated documentation where needed
  • I have considered the security impact of these changes
  • I have considered performance implications
  • I have thought about error handling and edge cases
  • I have tested the changes in my local environment

Testing Instructions

Prerequisites

Steps to Test

  1. Start gateway with a SAML authenticator configured and log in with a SAML account
  2. Create a local user with the same email address as the SAML account
  3. Log in again with the SAML user. This would have failed before the change but will now pass.

Expected Results

see above

- Add parameterized test for determine_username_from_uid_social function
- Cover both SAML (uses uid) and non-SAML (uses details.username) branches
- Test multiple authenticator types: SAML, LDAP, OIDC, Keycloak
- Ensures comprehensive coverage of lines 140-143 in authentication.py

Co-authored-by: Claude <claude@anthropic.com>
@github-actions
Copy link

DVCS PR Check Results:

PR appears valid (JIRA key(s) found)

@bhavenst bhavenst enabled auto-merge (squash) October 22, 2025 02:03
@sonarqubecloud
Copy link

@bhavenst bhavenst merged commit 2ccbd39 into ansible:devel Oct 22, 2025
17 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.

2 participants