Skip to content

Commit 2552fc9

Browse files
john-westcott-ivzkayyali812
authored andcommitted
AAP-21301 Make serializer accept jinja like syntax (#757)
## Description <!-- Mandatory: Provide a clear, concise description of the changes and their purpose --> - What is being changed? Allowing the authenticator_map serializer to store `{% for_attr_value(something) %}` - Why is this change needed? This stages authenticator_maps to support a missing SAML option. - How does this change address the issue? This will allow the authenticator maps to have templates in them. ## Type of Change <!-- Mandatory: Check one or more boxes that apply --> - [ ] 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 - [X] 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 --> ### Steps to Test 1. Log into your instance and create an authenticator_map with different formats of {% in it. 2. 3. ### Expected Results <!-- Describe what should happen after following the steps --> ## Additional Context <!-- Optional but helpful information --> ### Required Actions <!-- Check if changes require work in other areas --> <!-- Remove section if no external actions needed --> - [ ] Requires documentation updates <!-- API docs, feature docs, deployment guides --> - [ ] Requires downstream repository changes <!-- Specify repos: django-ansible-base, eda-server, etc. --> - [ ] Requires infrastructure/deployment changes <!-- CI/CD, installer updates, new services --> - [ ] Requires coordination with other teams <!-- UI team, platform services, infrastructure --> - [ ] Blocked by PR/MR: #XXX <!-- Reference blocking PRs/MRs with brief context --> ### Screenshots/Logs <!-- Add if relevant to demonstrate the changes -->
1 parent c45a7a3 commit 2552fc9

File tree

6 files changed

+140
-6
lines changed

6 files changed

+140
-6
lines changed

ansible_base/authentication/migrations/0018_authenticatoruser_email.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,4 +15,19 @@ class Migration(migrations.Migration):
1515
name='email',
1616
field=models.EmailField(blank=True, default=None, help_text='The e-mail associated with this authenticator user.', max_length=254, null=True),
1717
),
18+
migrations.AlterField(
19+
model_name='authenticatormap',
20+
name='organization',
21+
field=models.CharField(blank=True, default=None, help_text='An organization name this rule works on. Will expand {% for_attr_value(user_orgs) %} syntax', max_length=512, null=True),
22+
),
23+
migrations.AlterField(
24+
model_name='authenticatormap',
25+
name='role',
26+
field=models.CharField(blank=True, default=None, help_text='The role this map will grant the authenticating user to the targeted object. Will expand {% for_attr_value(user_orgs) %} syntax', max_length=512, null=True),
27+
),
28+
migrations.AlterField(
29+
model_name='authenticatormap',
30+
name='team',
31+
field=models.CharField(blank=True, default=None, help_text='A team name this rule works on. Will expand {% for_attr_value(user_orgs) %} syntax.', max_length=512, null=True),
32+
),
1833
]

ansible_base/authentication/models/authenticator_map.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,22 +55,22 @@ class Meta:
5555
null=True,
5656
default=None,
5757
blank=True,
58-
help_text=_("The role this map will grant the authenticating user to the targeted object."),
58+
help_text=_("The role this map will grant the authenticating user to the targeted object. Will expand {% for_attr_value(user_orgs) %} syntax"),
5959
)
6060

6161
team = models.CharField(
6262
max_length=512,
6363
null=True,
6464
default=None,
6565
blank=True,
66-
help_text=_('A team name this rule works on.'),
66+
help_text=_('A team name this rule works on. Will expand {% for_attr_value(user_orgs) %} syntax.'),
6767
)
6868
organization = models.CharField(
6969
max_length=512,
7070
null=True,
7171
default=None,
7272
blank=True,
73-
help_text=(_('An organization name this rule works on.')),
73+
help_text=(_('An organization name this rule works on. Will expand {% for_attr_value(user_orgs) %} syntax')),
7474
)
7575
triggers = models.JSONField(
7676
null=False,

ansible_base/authentication/serializers/authenticator_map.py

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
from rest_framework.serializers import ValidationError
77

88
from ansible_base.authentication.models import AuthenticatorMap
9+
from ansible_base.authentication.utils.authenticator_map import _EXPANSION_FIELDS, check_expansion_syntax, has_expansion
910
from ansible_base.authentication.utils.trigger_definition import TRIGGER_DEFINITION
1011
from ansible_base.lib.serializers.common import NamedCommonModelSerializer
1112
from ansible_base.lib.utils.auth import get_organization_model, get_team_model
@@ -40,6 +41,12 @@ def validate(self, data) -> dict:
4041
if role:
4142
errors.update(self.validate_role_data(map_type, role, org, team))
4243

44+
for field in _EXPANSION_FIELDS:
45+
if error_message := check_expansion_syntax(data.get(field, None)):
46+
# Its really not possible to have two errors on the same time.
47+
# Other errors indicate that things are missing so they would never get into here
48+
errors[field] = error_message
49+
4350
if errors:
4451
raise ValidationError(errors)
4552
return data
@@ -58,6 +65,10 @@ def validate_role_data(self, map_type, role, org, team):
5865

5966
from ansible_base.rbac.models import RoleDefinition
6067

68+
# If this role is dynamically expanded we can't check it now, only at run time.
69+
if has_expansion(role):
70+
return errors
71+
6172
try:
6273
rbac_role = RoleDefinition.objects.get(name=role)
6374
is_system_role = rbac_role.content_type is None
@@ -66,9 +77,8 @@ def validate_role_data(self, map_type, role, org, team):
6677
if is_system_role and map_type == 'role':
6778
return errors
6879

69-
if is_system_role:
70-
is_org_role, is_team_role = False, False
71-
else:
80+
is_org_role, is_team_role = False, False
81+
if not is_system_role:
7282
model_class = rbac_role.content_type.model_class()
7383
is_org_role = issubclass(model_class, get_organization_model())
7484
is_team_role = issubclass(model_class, get_team_model())
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
import re
2+
from typing import Any, Optional
3+
4+
from django.utils.translation import gettext_lazy as _
5+
6+
_EXPANSION_FIELDS = ['organization', 'role', 'team']
7+
8+
9+
def has_expansion(value: Optional[str]) -> bool:
10+
"""
11+
Checks the given value to see if it has the expansion syntax
12+
"""
13+
if not value:
14+
return False
15+
if re.search(r'{%.*%}', value):
16+
return True
17+
else:
18+
return False
19+
20+
21+
def check_expansion_syntax(value: Optional[str]) -> Optional[Any]:
22+
"""
23+
Check a given field to see if it contains the proper syntax for {% for_attr_value(user_orgs) %}
24+
25+
Raises a ValidationError if its incorrect
26+
"""
27+
28+
if has_expansion(value) and not re.search(r'{%\s*for_attr_value\(.+\)\s*%}', value):
29+
return _("Expansion only supports the format {% for_attr_value(attribute) %}")

test_app/tests/authentication/serializers/test_authenticator_map.py

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,3 +126,40 @@ def test_validate_role_organization_role(self, serializer, org_member_rd):
126126
)
127127
except ValidationError as e:
128128
pytest.fail(f"Validation should pass, but: {str(e)}")
129+
130+
131+
@pytest.mark.django_db
132+
class TestAuthenticatorMapEscapeSequence:
133+
@pytest.fixture(autouse=True)
134+
def init_serializer(self, serializer):
135+
serializer.validate_trigger_data = MagicMock(return_value={})
136+
serializer._is_rbac_installed = MagicMock(return_value=True)
137+
138+
@pytest.mark.parametrize(
139+
"role,organization,team",
140+
[
141+
(TEAM_MEMBER_ROLE_NAME, "asdf", "1234"),
142+
("Team {% for_attr_value(a) %}", "asdf", "1234"),
143+
(TEAM_MEMBER_ROLE_NAME, "Organization {% for_attr_value(member_of) %}", "1234"),
144+
(TEAM_MEMBER_ROLE_NAME, "asdf", "{% for_attr_value(member_of) %} Team"),
145+
],
146+
)
147+
def test_validate_expansion_fields(self, serializer, member_rd, role, organization, team):
148+
try:
149+
serializer.validate(dict(name="authentication_map_4", map_type="role", role=role, organization=organization, team=team))
150+
except ValidationError as e:
151+
pytest.fail(f"Validation should pass, but: {str(e)}")
152+
153+
@pytest.mark.parametrize(
154+
"role,organization,team,failure_type",
155+
[
156+
("Team {% ) %}", "asdf", "1234", "role"),
157+
(TEAM_MEMBER_ROLE_NAME, "Organization {% for_attr_value() %}", "1234", "organization"),
158+
(TEAM_MEMBER_ROLE_NAME, "asdf", "{% (member_of) %} Team", "team"),
159+
],
160+
)
161+
def test_validate_expansion_fields_negative(self, serializer, member_rd, role, organization, team, failure_type):
162+
with pytest.raises(ValidationError) as e:
163+
serializer.validate(dict(name="authentication_map_4", map_type="role", role=role, organization=organization, team=team))
164+
assert failure_type in str(e.value)
165+
assert 'Expansion only supports' in str(e.value)
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
import pytest
2+
3+
from ansible_base.authentication.utils.authenticator_map import check_expansion_syntax, has_expansion
4+
5+
# check_role_type is tested only though the serializer
6+
7+
8+
@pytest.mark.parametrize(
9+
"value,expected_result",
10+
[
11+
("a", False),
12+
("{% junkj}", False),
13+
("{%%}", True),
14+
("Pre-string {%%}", True),
15+
("Pre-string {%%} Post-STring", True),
16+
("{%%} Post-String", True),
17+
],
18+
)
19+
def test_has_expansion(value, expected_result):
20+
assert has_expansion(value) == expected_result
21+
22+
23+
@pytest.mark.parametrize(
24+
"value,should_work",
25+
[
26+
("a", True),
27+
("Pre-string {%%}", False),
28+
("Pre-string {% junk %} Post-STring", False),
29+
("{%%} Post-String", False),
30+
("Pre-string {% for_attr_value(testing) %}", True),
31+
("Pre-string {% for_attr_value(testing2) %} Post-STring", True),
32+
("{% for_attr_value(a) %} Post-String", True),
33+
("{% for_attr_value() %} Post-String", False),
34+
("{% for_attr_value(f) %}", True),
35+
("{% i for_attr_value(12) %}", False),
36+
],
37+
)
38+
def test_check_expansion_syntax(value, should_work):
39+
response = check_expansion_syntax(value)
40+
if should_work:
41+
assert response is None
42+
else:
43+
assert response is not None

0 commit comments

Comments
 (0)