Skip to content

Commit 59aaf99

Browse files
authored
[AAP-47897] Use configured groups attribute in SAML authenticator (#797)
SAML authenticator will use group attribute specified in the IDP configuration rather than only expecting to find "Group" hard-coded attribute
1 parent b6068b3 commit 59aaf99

File tree

2 files changed

+79
-3
lines changed
  • ansible_base/authentication/authenticator_plugins
  • test_app/tests/authentication/authenticator_plugins

2 files changed

+79
-3
lines changed

ansible_base/authentication/authenticator_plugins/saml.py

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -294,9 +294,21 @@ def extra_data(self, user, backend, response, *args, **kwargs):
294294
if perm in attrs:
295295
kwargs["social"].extra_data[perm] = attrs[perm]
296296

297-
# Move group spec up a level if present
298-
if "Group" in attrs:
299-
response["Group"] = attrs["Group"]
297+
# Get configured group attribute, if present
298+
configuration = getattr(self.database_instance, 'configuration', {})
299+
idp_groups_attribute_name = self.configuration_class.settings_to_enabled_idps_fields.get('IDP_GROUPS', None)
300+
configured_groups_attribute = configuration.get('ENABLED_IDPS', {}).get(idp_string, {}).get(idp_groups_attribute_name, None)
301+
302+
if configured_groups_attribute in attrs:
303+
logger.debug(f"Setting {self.groups_claim} from attribute: {configured_groups_attribute}")
304+
response[self.groups_claim] = attrs[configured_groups_attribute]
305+
# Else try getting the "Group" attribute, if present
306+
elif self.groups_claim in attrs:
307+
logger.debug(f"Setting {self.groups_claim} from attribute: {self.groups_claim}")
308+
response[self.groups_claim] = attrs[self.groups_claim]
309+
else:
310+
logger.debug("Unable to get any group claims from the SAML response")
311+
300312
data = super().extra_data(user, backend, response, *args, **kwargs)
301313

302314
# Ideally we would always have a DB instance

test_app/tests/authentication/authenticator_plugins/test_saml.py

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -369,12 +369,30 @@ def __init__(self):
369369
'attr_last_name': 'last_name',
370370
'attr_first_name': 'first_name',
371371
'attr_user_permanent_id': 'name_id',
372+
'attr_groups': 'member',
372373
},
373374
{
374375
'last_name': ['Admin'],
375376
'username': ['gateway_admin'],
376377
'first_name': ['Gateway'],
377378
'name_id': 'gateway_admin',
379+
'member': ['group-1', 'group-2'],
380+
},
381+
),
382+
(
383+
{
384+
'attr_username': 'username',
385+
'attr_last_name': 'last_name',
386+
'attr_first_name': 'first_name',
387+
'attr_user_permanent_id': 'name_id',
388+
'attr_groups': 'nonexistent_group_attr', # Configure a group attribute that won't be in response
389+
},
390+
{
391+
'last_name': ['Admin'],
392+
'username': ['gateway_admin'],
393+
'first_name': ['Gateway'],
394+
'name_id': 'gateway_admin',
395+
# No group data should be present - will hit the "Unable to get any group claims" branch
378396
},
379397
),
380398
],
@@ -403,6 +421,7 @@ def test_extra_data_default_attrs(idp_fields, expected_results):
403421
'first_name': ['Gateway'],
404422
'Role': ['default-roles-gateway realm', 'manage-account', 'uma_authorization', 'view-profile', 'offline_access', 'manage-account-links'],
405423
'name_id': 'gateway_admin',
424+
'member': ['group-1', 'group-2'],
406425
},
407426
}
408427
au = AuthenticatorUser()
@@ -411,6 +430,51 @@ def test_extra_data_default_attrs(idp_fields, expected_results):
411430
assert results == expected_results
412431

413432

433+
def test_extra_data_no_group_claims_logging(caplog):
434+
"""Test that the 'Unable to get any group claims' logging is triggered when no group attributes are found."""
435+
import logging
436+
437+
from ansible_base.authentication.authenticator_plugins.saml import idp_string
438+
from ansible_base.authentication.models import AuthenticatorUser
439+
440+
ap = AuthenticatorPlugin()
441+
database_instance = SimpleNamespace()
442+
enabled_idps = {
443+
'ENABLED_IDPS': {
444+
idp_string: {
445+
'attr_username': 'username',
446+
'attr_user_permanent_id': 'name_id',
447+
'attr_groups': 'missing_group_attr', # This attribute won't be in the response
448+
},
449+
}
450+
}
451+
database_instance.configuration = enabled_idps
452+
ap.database_instance = database_instance
453+
454+
response = {
455+
'idp_name': 'IdP',
456+
'attributes': {
457+
'username': ['gateway_admin'],
458+
'name_id': 'gateway_admin',
459+
# Note: No 'missing_group_attr' and no default 'Group' attribute
460+
},
461+
}
462+
463+
au = AuthenticatorUser()
464+
465+
# Set logging level to DEBUG to capture the debug message
466+
with caplog.at_level(logging.DEBUG, logger='ansible_base.authentication.authenticator_plugins.saml'):
467+
with mock.patch('social_core.backends.saml.SAMLAuth.extra_data', return_value={}):
468+
results = ap.extra_data(None, 'IdP:gateway_admin', response, **{'social': au})
469+
470+
# Verify the log message was captured
471+
assert "Unable to get any group claims from the SAML response" in caplog.text
472+
473+
# Verify no group data in results
474+
assert 'missing_group_attr' not in results
475+
assert 'Group' not in results
476+
477+
414478
def test_saml_create_via_api_without_callback_url(admin_api_client, saml_configuration):
415479
del saml_configuration['CALLBACK_URL']
416480

0 commit comments

Comments
 (0)