From bd72ec91ea82240ab596cf91b918b5eb606d9172 Mon Sep 17 00:00:00 2001 From: Josema Camacho Date: Tue, 14 Jul 2026 13:52:12 +0200 Subject: [PATCH] fix(api): combine permissions across assigned roles (#11979) --- .../multi-role-permission-gates.fixed.md | 1 + api/src/backend/api/rbac/permissions.py | 11 +++-- api/src/backend/api/tests/test_rbac.py | 43 +++++++++++++++++++ api/src/backend/api/tests/test_views.py | 19 +++++--- 4 files changed, 61 insertions(+), 13 deletions(-) create mode 100644 api/changelog.d/multi-role-permission-gates.fixed.md diff --git a/api/changelog.d/multi-role-permission-gates.fixed.md b/api/changelog.d/multi-role-permission-gates.fixed.md new file mode 100644 index 0000000000..b9a6c1a8ee --- /dev/null +++ b/api/changelog.d/multi-role-permission-gates.fixed.md @@ -0,0 +1 @@ +RBAC permission gates now combine permissions from every role assigned to a user in the active tenant diff --git a/api/src/backend/api/rbac/permissions.py b/api/src/backend/api/rbac/permissions.py index ef0475fefb..3458346a5f 100644 --- a/api/src/backend/api/rbac/permissions.py +++ b/api/src/backend/api/rbac/permissions.py @@ -34,7 +34,7 @@ class HasPermissions(BasePermission): if not tenant_id: return False - user_roles = ( + user_roles = list( User.objects.using(MainRouter.admin_db) .get(id=request.user.id) .roles.using(MainRouter.admin_db) @@ -43,11 +43,10 @@ class HasPermissions(BasePermission): if not user_roles: return False - for perm in required_permissions: - if not getattr(user_roles[0], perm.value, False): - return False - - return True + return all( + any(getattr(role, permission.value, False) for role in user_roles) + for permission in required_permissions + ) def get_role(user: User, tenant_id: str) -> Role: diff --git a/api/src/backend/api/tests/test_rbac.py b/api/src/backend/api/tests/test_rbac.py index f2e2fc4bb5..ed177138b2 100644 --- a/api/src/backend/api/tests/test_rbac.py +++ b/api/src/backend/api/tests/test_rbac.py @@ -11,6 +11,7 @@ from api.models import ( User, UserRoleRelationship, ) +from api.rbac.permissions import HasPermissions, Permissions from api.v1.serializers import TokenSerializer from conftest import TEST_PASSWORD, TODAY from django.urls import reverse @@ -816,6 +817,48 @@ class TestRolePermissions: assert response.status_code == status.HTTP_403_FORBIDDEN +@pytest.mark.django_db +class TestHasPermissions: + def test_permissions_are_combined_across_roles( + self, create_test_user_rbac_no_roles + ): + user = create_test_user_rbac_no_roles + tenant = Membership.objects.get(user=user).tenant + manage_users_role = Role.objects.create( + name="manage_users_only", + tenant=tenant, + manage_users=True, + ) + UserRoleRelationship.objects.create( + user=user, + role=manage_users_role, + tenant=tenant, + ) + request = Mock(user=user, tenant_id=tenant.id) + view = Mock( + required_permissions=[ + Permissions.MANAGE_USERS, + Permissions.MANAGE_ACCOUNT, + ] + ) + permission = HasPermissions() + + assert not permission.has_permission(request, view) + + manage_account_role = Role.objects.create( + name="manage_account_only", + tenant=tenant, + manage_account=True, + ) + UserRoleRelationship.objects.create( + user=user, + role=manage_account_role, + tenant=tenant, + ) + + assert permission.has_permission(request, view) + + @pytest.mark.django_db class TestUserRoleLinkPermissions: def test_link_user_roles_with_manage_account_only_allowed( diff --git a/api/src/backend/api/tests/test_views.py b/api/src/backend/api/tests/test_views.py index be52393dfa..a1e73b46ec 100644 --- a/api/src/backend/api/tests/test_views.py +++ b/api/src/backend/api/tests/test_views.py @@ -9435,20 +9435,22 @@ class TestUserRoleRelationshipViewSet: assert added_role_ids.issubset(relationship_role_ids) def test_create_relationship_already_exists( - self, authenticated_client, roles_fixture, create_test_user + self, authenticated_client, roles_fixture, create_test_user_rbac_no_roles ): - # Only add Role One (which has manage_account=True) to ensure - # the second request has permission to add roles data = { "data": [ - {"type": "roles", "id": str(roles_fixture[0].id)}, + {"type": "roles", "id": str(role.id)} for role in roles_fixture[:2] ] } - authenticated_client.post( - reverse("user-roles-relationship", kwargs={"pk": create_test_user.id}), + setup_response = authenticated_client.post( + reverse( + "user-roles-relationship", + kwargs={"pk": create_test_user_rbac_no_roles.id}, + ), data=data, content_type="application/vnd.api+json", ) + assert setup_response.status_code == status.HTTP_204_NO_CONTENT data = { "data": [ @@ -9456,7 +9458,10 @@ class TestUserRoleRelationshipViewSet: ] } response = authenticated_client.post( - reverse("user-roles-relationship", kwargs={"pk": create_test_user.id}), + reverse( + "user-roles-relationship", + kwargs={"pk": create_test_user_rbac_no_roles.id}, + ), data=data, content_type="application/vnd.api+json", )