Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RHCLOUD-36017 Custom roles cannot be created or updated using existin… #1286

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open
15 changes: 14 additions & 1 deletion rbac/management/role/view.py
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ def create(self, request, *args, **kwargs):
}
"""
self.validate_role(request)
self.restrict_role_create_update_if_system_role_exists(request)
try:
with transaction.atomic():
return super().create(request=request, args=args, kwargs=kwargs)
Expand Down Expand Up @@ -463,7 +464,7 @@ def update(self, request, *args, **kwargs):
"""
validate_uuid(kwargs.get("uuid"), "role uuid validation")
self.validate_role(request)

self.restrict_role_create_update_if_system_role_exists(request)
try:
with transaction.atomic():
return super().update(request=request, args=args, kwargs=kwargs)
Expand Down Expand Up @@ -645,3 +646,15 @@ def delete_policies_if_no_role_attached(self, role):
for policy in policies:
if policy.roles.count() == 1:
policy.delete()

def restrict_role_create_update_if_system_role_exists(self, request):
"""If a system role has the same display_name when a role is being created or updated restrict the operation."""
req_method = request.method
req_name = request.data.get("name")
public_tenant = Tenant.objects.get(tenant_name="public")
base_role_queryset = Role.objects.filter(display_name=req_name, tenant__in=[request.tenant, public_tenant])
req_system_role_exists = base_role_queryset.filter(system=True).exists()
if req_system_role_exists and req_method == "PUT":
raise serializers.ValidationError({"role": f"Role '{req_name}' name cannot be updated with this value."})
elif req_system_role_exists and req_method == "POST":
raise serializers.ValidationError({"role": f"Role '{req_name}' already exists for a tenant."})
28 changes: 28 additions & 0 deletions tests/management/role/test_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -1463,6 +1463,7 @@ def test_update_role(self, mock_method):
current_relations = relation_api_tuples_for_v1_role(role_uuid, str(self.default_workspace.id))

response = client.put(url, test_data, format="json", **self.headers)

replication_event = replication_event_for_v1_role(response.data.get("uuid"), str(self.default_workspace.id))
replication_event["relations_to_remove"] = current_relations
actual_call_arg = mock_method.call_args[0][0]
Expand Down Expand Up @@ -1790,6 +1791,33 @@ def test_create_duplicate_role_fail(self):
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
self.assertEqual(response.data.get("errors")[0].get("detail"), f"Role '{name}' already exists for a tenant.")

def test_create_custom_role_with_same_name_as_system_role(self):
"""Test that trying to create a custom role with the same name as a system role is not possible"""
client = APIClient()
name = "system_display"
test_data = {"name": name, "access": []}

# Attempt to create custom role with the same name as system role
response = client.post(URL, test_data, format="json", **self.headers)
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
# Assert output message is correct
self.assertEqual(response.data.get("errors")[0].get("detail"), f"Role '{name}' already exists for a tenant.")

def test_update_custom_role_with_same_name_as_system_role(self):
"""Test that trying to update a custom role with the same name as a system role is not possible"""
name = "system_display"
test_data = {"name": name, "access": []}
url = reverse("v1_management:role-detail", kwargs={"uuid": self.sysRole.uuid})
client = APIClient()
response = client.put(url, test_data, format="json", **self.headers)
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)

# Assert output message is correct
self.assertEqual(
response.data.get("errors")[0].get("detail"),
f"Role '{name}' name cannot be updated with this value.",
)


class RoleViewNonAdminTests(IdentityRequest):
"""Test the role view for nonadmin user."""
Expand Down