Skip to content

Commit 2ccbd39

Browse files
bhavenstBryan HavensteinClaude
authored
AAP-56394 Fix SAML authentication uid selection (ansible#871)
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. --------- Co-authored-by: Bryan Havenstein <[email protected]> Co-authored-by: Claude <[email protected]>
1 parent 011f49c commit 2ccbd39

File tree

2 files changed

+25
-1
lines changed

2 files changed

+25
-1
lines changed

ansible_base/authentication/utils/authentication.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,10 @@ def determine_username_from_uid_social(**kwargs) -> dict:
137137
if authenticator.setting('USERNAME_IS_FULL_EMAIL', False):
138138
selected_username = kwargs.get('details', {}).get('email', None)
139139
else:
140-
selected_username = kwargs.get('details', {}).get('username', None)
140+
if authenticator.type == 'SAML':
141+
selected_username = kwargs.get('uid', None)
142+
else:
143+
selected_username = kwargs.get('details', {}).get('username', None)
141144
if not selected_username:
142145
raise_auth_exception(
143146
_('Unable to get associated username from details, expected entry "username". Full user details: %(details)s')

test_app/tests/authentication/utils/test_authentication.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,27 @@ def test_determine_username_from_uid_social_happy_path(self, ldap_authenticator)
249249
)
250250
assert response == {'username': 'Bob'}
251251

252+
@pytest.mark.parametrize(
253+
"auth_fixture, expected_username, uid_param, details_username",
254+
[
255+
# SAML authenticator uses uid parameter (line 141)
256+
("saml_authenticator", "SamlUser", "SamlUser", "should_not_be_used"),
257+
# Non-SAML authenticators use details['username'] (line 143)
258+
("ldap_authenticator", "LdapUser", "should_not_be_used", "LdapUser"),
259+
("oidc_authenticator", "OidcUser", "should_not_be_used", "OidcUser"),
260+
("keycloak_authenticator", "KeycloakUser", "should_not_be_used", "KeycloakUser"),
261+
],
262+
)
263+
def test_determine_username_from_uid_social_authenticator_types(self, request, auth_fixture, expected_username, uid_param, details_username):
264+
"""Test username selection logic for different authenticator types (lines 140-143)"""
265+
authenticator = request.getfixturevalue(auth_fixture)
266+
response = authentication.determine_username_from_uid_social(
267+
details={'username': details_username},
268+
backend=get_authenticator_class(authenticator.type)(database_instance=authenticator),
269+
uid=uid_param,
270+
)
271+
assert response == {'username': expected_username}
272+
252273
def test_raise_auth_exception(self):
253274
try:
254275
authentication.raise_auth_exception('testing')

0 commit comments

Comments
 (0)