Skip to content

Commit 5b22f8e

Browse files
authored
AAP-41636 Add configurable group claim and reading groups from backend (#708)
# [AAP-41636] Allow configurable name of "groups" claim for Azure AD and add support for reading groups from this claim **NOTE:** This PR depends on PR #682 . ## Description <!-- Mandatory: Provide a clear, concise description of the changes and their purpose --> - What is being changed? - The AzureAD plugin gets a new configuration option and the inherited function for reading groups is overriden so it does return the groups populated from the claim - Why is this change needed? - Without this change the authentication with Azure AD always defaults the name of groups claim to "Group" however that is not what Azure AD sends the groups in. In our tests we noticed it always been "groups". To avoid hard-coding and giving the users option to change the claim name, it needs to be configurable. - How does this change address the issue? - New configuration option shown in UI and user can use correct claim name ## Type of Change <!-- Mandatory: Check one or more boxes that apply --> - [x] Bug fix (non-breaking change which fixes an issue) - [x] 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 <!-- These items help ensure quality - they complement our automated CI checks --> - [x] I have performed a self-review of my code - [x] I have added relevant comments to complex code sections - [ ] I have updated documentation where needed - [x] I have considered the security impact of these changes - [x] I have considered performance implications - [x] I have thought about error handling and edge cases - [x] I have tested the changes in my local environment ## Testing Instructions <!-- Optional for test-only changes. Mandatory for all other changes --> <!-- Must be detailed enough for reviewers to reproduce --> ### Prerequisites <!-- List any specific setup required --> To test fully (with authentication) you will need to configure an application in Azure and you must use Microsoft Id for that (hotmail id works too). To test only the configuration, you don't need Azure at all. ### Steps to Test 1. Pull down the PR 2. Start AAP 3. Navigate to configuration of authentication methods and create one for Azure AD type. 4. (optional, when PR #682 is included as well) Perform login with Azure AD ### Expected Results <!-- Describe what should happen after following the steps --> Configuration of Azure AD shows new field - default is empty string which effectively means "Group". ![Screenshot_20250326_164609](https://github.com/user-attachments/assets/957c2aa1-9f5d-4348-8812-4183f87fbe12) ## Additional Context <!-- Optional but helpful information --> ### Required Actions - [x] Blocked by PR/MR: #682 <!-- Reference blocking PRs/MRs with brief context -->
1 parent 387c742 commit 5b22f8e

File tree

2 files changed

+45
-0
lines changed
  • ansible_base/authentication/authenticator_plugins
  • test_app/tests/authentication/authenticator_plugins

2 files changed

+45
-0
lines changed

ansible_base/authentication/authenticator_plugins/azuread.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,25 @@ class AzureADConfiguration(BaseAuthenticatorConfiguration):
3737
ui_field_label=_('OIDC Secret'),
3838
)
3939

40+
GROUPS_CLAIM = CharField(
41+
help_text=_("The JSON key used to extract the user's groups from the ID token or userinfo endpoint."),
42+
required=False,
43+
allow_null=False,
44+
default="Group",
45+
ui_field_label=_("Groups Claim"),
46+
)
47+
4048

4149
class AuthenticatorPlugin(SocialAuthMixin, SocialAuthValidateCallbackMixin, AzureADOAuth2, AbstractAuthenticatorPlugin):
4250
configuration_class = AzureADConfiguration
4351
type = "azuread"
4452
logger = logger
4553
category = "sso"
4654
configuration_encrypted_fields = ['SECRET']
55+
56+
@property
57+
def groups_claim(self):
58+
return self.setting('GROUPS_CLAIM')
59+
60+
def get_user_groups(self, extra_groups=[]):
61+
return extra_groups

test_app/tests/authentication/authenticator_plugins/test_azuread.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
import pytest
44
from django.test import override_settings
55

6+
from ansible_base.authentication.authenticator_plugins.utils import get_authenticator_plugin
67
from ansible_base.authentication.session import SessionAuthentication
8+
from ansible_base.authentication.social_auth import SocialAuthMixin
79
from ansible_base.lib.utils.response import get_fully_qualified_url, get_relative_url
810

911
authenticated_test_page = "authenticator-list"
@@ -79,3 +81,31 @@ def test_azuread_auth_failed(authenticate, unauthenticated_api_client, azuread_a
7981
url = get_relative_url(authenticated_test_page)
8082
response = client.get(url)
8183
assert response.status_code == 401
84+
85+
86+
def test_groups_setting_and_user_groups(keycloak_authenticator):
87+
custom_groups_claim = "some_groups"
88+
89+
class MockedDb:
90+
def __init__(self, group_claim):
91+
self.slug = "fake"
92+
self.configuration = {"GROUPS_CLAIM": group_claim}
93+
94+
class MockBackend(SocialAuthMixin):
95+
database_instance = MockedDb(custom_groups_claim)
96+
97+
def __init__(self):
98+
pass
99+
100+
backend = MockBackend()
101+
102+
ad = get_authenticator_plugin("ansible_base.authentication.authenticator_plugins.azuread")
103+
ad.database_instance = MockedDb(custom_groups_claim)
104+
105+
# assert that groups claim setting is there and AD has the expected groups claim
106+
assert ad.strategy.get_setting('GROUPS_CLAIM', backend) == custom_groups_claim
107+
assert ad.groups_claim == custom_groups_claim
108+
109+
# assert that AD returns expected user groups
110+
assert ad.get_user_groups() == []
111+
assert ad.get_user_groups(["a", "b"]) == ["a", "b"]

0 commit comments

Comments
 (0)