Skip to content

Commit 9b96d76

Browse files
fostersethTheRealHaoLiu
authored andcommitted
Prevent modifying shared resources when using platform ingress (ansible#15234)
* Prevent modifying shared resources Adds a class decorator to prevent modifying shared resources when gateway is being used. AWX_DIRECT_SHARED_RESOURCE_MANAGEMENT_ENABLED is the setting to enable/disable this feature. Works by overriding these view methods: - create - delete - perform_update create and delete are overridden to raise a PermissionDenied exception. perform_update is overridden to check if any shared fields are being modified, and raise a PermissionDenied exception if so. Additional changes: Prevent sso conf from registering external authentication related settings if AWX_DIRECT_SHARED_RESOURCE_MANAGEMENT_ENABLED is False Signed-off-by: Seth Foster <[email protected]> Co-authored-by: Hao Liu <[email protected]>
1 parent a10ee92 commit 9b96d76

File tree

5 files changed

+1581
-1411
lines changed

5 files changed

+1581
-1411
lines changed

awx/api/views/__init__.py

Lines changed: 88 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@
6262

6363
# django-ansible-base
6464
from ansible_base.rbac.models import RoleEvaluation, ObjectRole
65+
from ansible_base.resource_registry.shared_types import OrganizationType, TeamType, UserType
6566

6667
# AWX
6768
from awx.main.tasks.system import send_notifications, update_inventory_computed_fields
@@ -128,6 +129,7 @@
128129
from awx.api.pagination import UnifiedJobEventPagination
129130
from awx.main.utils import set_environ
130131

132+
131133
logger = logging.getLogger('awx.api.views')
132134

133135

@@ -710,16 +712,81 @@ def get(self, request):
710712
return Response(data)
711713

712714

715+
def immutablesharedfields(cls):
716+
'''
717+
Class decorator to prevent modifying shared resources when AWX_DIRECT_SHARED_RESOURCE_MANAGEMENT_ENABLED setting is set to False.
718+
719+
Works by overriding these view methods:
720+
- create
721+
- delete
722+
- perform_update
723+
create and delete are overridden to raise a PermissionDenied exception.
724+
perform_update is overridden to check if any shared fields are being modified,
725+
and raise a PermissionDenied exception if so.
726+
'''
727+
# create instead of perform_create because some of our views
728+
# override create instead of perform_create
729+
if hasattr(cls, 'create'):
730+
cls.original_create = cls.create
731+
732+
@functools.wraps(cls.create)
733+
def create_wrapper(*args, **kwargs):
734+
if settings.AWX_DIRECT_SHARED_RESOURCE_MANAGEMENT_ENABLED:
735+
return cls.original_create(*args, **kwargs)
736+
raise PermissionDenied({'detail': _('Creation of this resource is not allowed. Create this resource via the platform ingress.')})
737+
738+
cls.create = create_wrapper
739+
740+
if hasattr(cls, 'delete'):
741+
cls.original_delete = cls.delete
742+
743+
@functools.wraps(cls.delete)
744+
def delete_wrapper(*args, **kwargs):
745+
if settings.AWX_DIRECT_SHARED_RESOURCE_MANAGEMENT_ENABLED:
746+
return cls.original_delete(*args, **kwargs)
747+
raise PermissionDenied({'detail': _('Deletion of this resource is not allowed. Delete this resource via the platform ingress.')})
748+
749+
cls.delete = delete_wrapper
750+
751+
if hasattr(cls, 'perform_update'):
752+
cls.original_perform_update = cls.perform_update
753+
754+
@functools.wraps(cls.perform_update)
755+
def update_wrapper(*args, **kwargs):
756+
if not settings.AWX_DIRECT_SHARED_RESOURCE_MANAGEMENT_ENABLED:
757+
view, serializer = args
758+
instance = view.get_object()
759+
if instance:
760+
if isinstance(instance, models.Organization):
761+
shared_fields = OrganizationType._declared_fields.keys()
762+
elif isinstance(instance, models.User):
763+
shared_fields = UserType._declared_fields.keys()
764+
elif isinstance(instance, models.Team):
765+
shared_fields = TeamType._declared_fields.keys()
766+
attrs = serializer.validated_data
767+
for field in shared_fields:
768+
if field in attrs and getattr(instance, field) != attrs[field]:
769+
raise PermissionDenied({field: _(f"Cannot change shared field '{field}'. Alter this field via the platform ingress.")})
770+
return cls.original_perform_update(*args, **kwargs)
771+
772+
cls.perform_update = update_wrapper
773+
774+
return cls
775+
776+
777+
@immutablesharedfields
713778
class TeamList(ListCreateAPIView):
714779
model = models.Team
715780
serializer_class = serializers.TeamSerializer
716781

717782

783+
@immutablesharedfields
718784
class TeamDetail(RetrieveUpdateDestroyAPIView):
719785
model = models.Team
720786
serializer_class = serializers.TeamSerializer
721787

722788

789+
@immutablesharedfields
723790
class TeamUsersList(BaseUsersList):
724791
model = models.User
725792
serializer_class = serializers.UserSerializer
@@ -1101,6 +1168,7 @@ class ProjectCopy(CopyAPIView):
11011168
copy_return_serializer_class = serializers.ProjectSerializer
11021169

11031170

1171+
@immutablesharedfields
11041172
class UserList(ListCreateAPIView):
11051173
model = models.User
11061174
serializer_class = serializers.UserSerializer
@@ -1271,7 +1339,16 @@ def post(self, request, *args, **kwargs):
12711339
user = get_object_or_400(models.User, pk=self.kwargs['pk'])
12721340
role = get_object_or_400(models.Role, pk=sub_id)
12731341

1274-
credential_content_type = ContentType.objects.get_for_model(models.Credential)
1342+
content_types = ContentType.objects.get_for_models(models.Organization, models.Team, models.Credential) # dict of {model: content_type}
1343+
# Prevent user to be associated with team/org when AWX_DIRECT_SHARED_RESOURCE_MANAGEMENT_ENABLED is False
1344+
if not settings.AWX_DIRECT_SHARED_RESOURCE_MANAGEMENT_ENABLED:
1345+
for model in [models.Organization, models.Team]:
1346+
ct = content_types[model]
1347+
if role.content_type == ct and role.role_field in ['member_role', 'admin_role']:
1348+
data = dict(msg=_(f"Cannot directly modify user membership to {ct.model}. Direct shared resource management disabled"))
1349+
return Response(data, status=status.HTTP_403_FORBIDDEN)
1350+
1351+
credential_content_type = content_types[models.Credential]
12751352
if role.content_type == credential_content_type:
12761353
if 'disassociate' not in request.data and role.content_object.organization and user not in role.content_object.organization.member_role:
12771354
data = dict(msg=_("You cannot grant credential access to a user not in the credentials' organization"))
@@ -1343,6 +1420,7 @@ def get_queryset(self):
13431420
return qs.filter(Q(actor=parent) | Q(user__in=[parent]))
13441421

13451422

1423+
@immutablesharedfields
13461424
class UserDetail(RetrieveUpdateDestroyAPIView):
13471425
model = models.User
13481426
serializer_class = serializers.UserSerializer
@@ -4295,7 +4373,15 @@ def post(self, request, *args, **kwargs):
42954373
user = get_object_or_400(models.User, pk=sub_id)
42964374
role = self.get_parent_object()
42974375

4298-
credential_content_type = ContentType.objects.get_for_model(models.Credential)
4376+
content_types = ContentType.objects.get_for_models(models.Organization, models.Team, models.Credential) # dict of {model: content_type}
4377+
if not settings.AWX_DIRECT_SHARED_RESOURCE_MANAGEMENT_ENABLED:
4378+
for model in [models.Organization, models.Team]:
4379+
ct = content_types[model]
4380+
if role.content_type == ct and role.role_field in ['member_role', 'admin_role']:
4381+
data = dict(msg=_(f"Cannot directly modify user membership to {ct.model}. Direct shared resource management disabled"))
4382+
return Response(data, status=status.HTTP_403_FORBIDDEN)
4383+
4384+
credential_content_type = content_types[models.Credential]
42994385
if role.content_type == credential_content_type:
43004386
if 'disassociate' not in request.data and role.content_object.organization and user not in role.content_object.organization.member_role:
43014387
data = dict(msg=_("You cannot grant credential access to a user not in the credentials' organization"))

awx/api/views/organization.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,15 +53,18 @@
5353
CredentialSerializer,
5454
)
5555
from awx.api.views.mixin import RelatedJobsPreventDeleteMixin, OrganizationCountsMixin
56+
from awx.api.views import immutablesharedfields
5657

5758
logger = logging.getLogger('awx.api.views.organization')
5859

5960

61+
@immutablesharedfields
6062
class OrganizationList(OrganizationCountsMixin, ListCreateAPIView):
6163
model = Organization
6264
serializer_class = OrganizationSerializer
6365

6466

67+
@immutablesharedfields
6568
class OrganizationDetail(RelatedJobsPreventDeleteMixin, RetrieveUpdateDestroyAPIView):
6669
model = Organization
6770
serializer_class = OrganizationSerializer
@@ -104,6 +107,7 @@ class OrganizationInventoriesList(SubListAPIView):
104107
relationship = 'inventories'
105108

106109

110+
@immutablesharedfields
107111
class OrganizationUsersList(BaseUsersList):
108112
model = User
109113
serializer_class = UserSerializer
@@ -112,6 +116,7 @@ class OrganizationUsersList(BaseUsersList):
112116
ordering = ('username',)
113117

114118

119+
@immutablesharedfields
115120
class OrganizationAdminsList(BaseUsersList):
116121
model = User
117122
serializer_class = UserSerializer
@@ -150,6 +155,7 @@ class OrganizationWorkflowJobTemplatesList(SubListCreateAPIView):
150155
parent_key = 'organization'
151156

152157

158+
@immutablesharedfields
153159
class OrganizationTeamsList(SubListCreateAttachDetachAPIView):
154160
model = Team
155161
serializer_class = TeamSerializer
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
import pytest
2+
3+
from awx.api.versioning import reverse
4+
from awx.main.models import Organization
5+
6+
7+
@pytest.mark.django_db
8+
class TestImmutableSharedFields:
9+
@pytest.fixture(autouse=True)
10+
def configure_settings(self, settings):
11+
settings.AWX_DIRECT_SHARED_RESOURCE_MANAGEMENT_ENABLED = False
12+
13+
def test_create_raises_permission_denied(self, admin_user, post):
14+
orgA = Organization.objects.create(name='orgA')
15+
resp = post(
16+
url=reverse('api:team_list'),
17+
data={'name': 'teamA', 'organization': orgA.id},
18+
user=admin_user,
19+
expect=403,
20+
)
21+
assert "Creation of this resource is not allowed" in resp.data['detail']
22+
23+
def test_perform_delete_raises_permission_denied(self, admin_user, delete):
24+
orgA = Organization.objects.create(name='orgA')
25+
team = orgA.teams.create(name='teamA')
26+
resp = delete(
27+
url=reverse('api:team_detail', kwargs={'pk': team.id}),
28+
user=admin_user,
29+
expect=403,
30+
)
31+
assert "Deletion of this resource is not allowed" in resp.data['detail']
32+
33+
def test_perform_update(self, admin_user, patch):
34+
orgA = Organization.objects.create(name='orgA')
35+
team = orgA.teams.create(name='teamA')
36+
# allow patching non-shared fields
37+
patch(
38+
url=reverse('api:team_detail', kwargs={'pk': team.id}),
39+
data={"description": "can change this field"},
40+
user=admin_user,
41+
expect=200,
42+
)
43+
orgB = Organization.objects.create(name='orgB')
44+
# prevent patching shared fields
45+
resp = patch(url=reverse('api:team_detail', kwargs={'pk': team.id}), data={"organization": orgB.id}, user=admin_user, expect=403)
46+
assert "Cannot change shared field" in resp.data['organization']
47+
48+
@pytest.mark.parametrize(
49+
'role',
50+
['admin_role', 'member_role'],
51+
)
52+
@pytest.mark.parametrize('resource', ['organization', 'team'])
53+
def test_prevent_assigning_member_to_organization_or_team(self, admin_user, post, resource, role):
54+
orgA = Organization.objects.create(name='orgA')
55+
if resource == 'organization':
56+
role = getattr(orgA, role)
57+
elif resource == 'team':
58+
teamA = orgA.teams.create(name='teamA')
59+
role = getattr(teamA, role)
60+
resp = post(
61+
url=reverse('api:user_roles_list', kwargs={'pk': admin_user.id}),
62+
data={'id': role.id},
63+
user=admin_user,
64+
expect=403,
65+
)
66+
assert f"Cannot directly modify user membership to {resource}." in resp.data['msg']

awx/settings/defaults.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -656,6 +656,10 @@
656656
# Automatically remove nodes that have missed their heartbeats after some time
657657
AWX_AUTO_DEPROVISION_INSTANCES = False
658658

659+
# If False, do not allow creation of resources that are shared with the platform ingress
660+
# e.g. organizations, teams, and users
661+
AWX_DIRECT_SHARED_RESOURCE_MANAGEMENT_ENABLED = True
662+
659663
# Enable Pendo on the UI, possible values are 'off', 'anonymous', and 'detailed'
660664
# Note: This setting may be overridden by database settings.
661665
PENDO_TRACKING_STATE = "off"

0 commit comments

Comments
 (0)