feat(auth): add safeguards to prevent self-role removal and enforce MANAGE_ACCOUNT role presence (#8729)

AdriiiPRodri · web-flow · commit 2200e6551992 · 2025-09-22T14:04:39.000+02:00
diff --git a/api/CHANGELOG.md b/api/CHANGELOG.md
@@ -9,6 +9,7 @@ All notable changes to the **Prowler API** are documented in this file.
 
 ### Changed
 - Now the MANAGE_ACCOUNT permission is required to modify or read user permissions instead of MANAGE_USERS [(#8281)](https://github.com/prowler-cloud/prowler/pull/8281)
+- Now at least one user with MANAGE_ACCOUNT permission is required in the tenant [(#8729)](https://github.com/prowler-cloud/prowler/pull/8729)
 
 ---
 
diff --git a/api/src/backend/api/specs/v1.yaml b/api/src/backend/api/specs/v1.yaml
@@ -6363,8 +6363,10 @@ paths:
           description: ''
     patch:
       operationId: roles_partial_update
-      description: Update certain fields of an existing role's information without
-        affecting other fields.
+      description: Update selected fields on an existing role. When changing the `users`
+        relationship of a role that grants MANAGE_ACCOUNT, the API blocks attempts
+        that would leave the tenant without any MANAGE_ACCOUNT assignees and prevents
+        callers from removing their own assignment to that role.
       summary: Partially update a role
       parameters:
       - in: path
@@ -6399,7 +6401,8 @@ paths:
           description: ''
     delete:
       operationId: roles_destroy
-      description: Remove a role from the system by their ID.
+      description: Delete the specified role. The API rejects deletion of the last
+        role in the tenant that grants MANAGE_ACCOUNT.
       summary: Delete a role
       parameters:
       - in: path
@@ -8439,7 +8442,8 @@ paths:
     patch:
       operationId: users_relationships_roles_partial_update
       description: Update the user-roles relationship information without affecting
-        other fields.
+        other fields. If the update would remove MANAGE_ACCOUNT from the last remaining
+        user in the tenant, the API rejects the request with a 400 response.
       summary: Partially update a user-roles relationship
       tags:
       - User
@@ -8463,6 +8467,10 @@ paths:
     delete:
       operationId: users_relationships_roles_destroy
       description: Remove the user-roles relationship from the system by their ID.
+        If removing MANAGE_ACCOUNT would take it away from the last remaining user
+        in the tenant, the API rejects the request with a 400 response. Users also
+        cannot delete their own role assignments; attempting to do so returns a 400
+        response.
       summary: Delete a user-roles relationship
       tags:
       - User
diff --git a/api/src/backend/api/tests/test_views.py b/api/src/backend/api/tests/test_views.py
@@ -51,6 +51,7 @@
     UserRoleRelationship,
 )
 from api.rls import Tenant
+from api.v1.serializers import TokenSerializer
 from api.v1.views import ComplianceOverviewViewSet, TenantFinishACSView
 
 
@@ -4720,6 +4721,36 @@ def test_role_clear_relationships(self, authenticated_client, roles_fixture):
         assert role.users.count() == 0
         assert role.provider_groups.count() == 0
 
+    def test_cannot_remove_own_assignment_via_role_update(
+        self, authenticated_client, roles_fixture
+    ):
+        role = roles_fixture[0]
+        # Ensure the authenticated user is assigned to this role
+        user = User.objects.get(email=TEST_USER)
+        if not UserRoleRelationship.objects.filter(user=user, role=role).exists():
+            UserRoleRelationship.objects.create(
+                user=user, role=role, tenant_id=role.tenant_id
+            )
+
+        # Attempt to update role users to exclude the current user
+        data = {
+            "data": {
+                "id": str(role.id),
+                "type": "roles",
+                "relationships": {"users": {"data": []}},
+            }
+        }
+        response = authenticated_client.patch(
+            reverse("role-detail", kwargs={"pk": role.id}),
+            data=json.dumps(data),
+            content_type="application/vnd.api+json",
+        )
+        assert response.status_code == status.HTTP_400_BAD_REQUEST
+        assert (
+            "cannot remove their own role"
+            in response.json()["errors"][0]["detail"].lower()
+        )
+
     def test_role_create_with_invalid_user_relationship(
         self, authenticated_client, provider_groups_fixture
     ):
@@ -4841,16 +4872,135 @@ def test_partial_update_relationship(
             roles_fixture[2].id,
         }
 
-    def test_destroy_relationship(
-        self, authenticated_client, roles_fixture, create_test_user
+    def test_destroy_relationship_other_user(
+        self, authenticated_client, roles_fixture, create_test_user, tenants_fixture
     ):
+        # Create another user in same tenant and assign a role
+        tenant = tenants_fixture[0]
+        other_user = User.objects.create_user(
+            name="other",
+            email="other_user@prowler.com",
+            password="TmpPass123@",
+        )
+        Membership.objects.create(user=other_user, tenant=tenant)
+        UserRoleRelationship.objects.create(
+            user=other_user, role=roles_fixture[0], tenant_id=tenant.id
+        )
+
+        # Delete roles for the other user (allowed)
         response = authenticated_client.delete(
-            reverse("user-roles-relationship", kwargs={"pk": create_test_user.id}),
+            reverse("user-roles-relationship", kwargs={"pk": other_user.id}),
         )
         assert response.status_code == status.HTTP_204_NO_CONTENT
-        relationships = UserRoleRelationship.objects.filter(role=roles_fixture[0].id)
+        relationships = UserRoleRelationship.objects.filter(user=other_user.id)
         assert relationships.count() == 0
 
+    def test_cannot_delete_own_roles(self, authenticated_client, create_test_user):
+        # Attempt to delete own roles should be forbidden
+        response = authenticated_client.delete(
+            reverse("user-roles-relationship", kwargs={"pk": create_test_user.id}),
+        )
+        assert response.status_code == status.HTTP_400_BAD_REQUEST
+
+    def test_prevent_removing_last_manage_account_on_patch(
+        self, authenticated_client, roles_fixture, create_test_user, tenants_fixture
+    ):
+        # roles_fixture[1] has manage_account=False
+        limited_role = roles_fixture[1]
+
+        # Ensure there is no other user with MANAGE_ACCOUNT in the tenant
+        tenant = tenants_fixture[0]
+        # Create a secondary user without MANAGE_ACCOUNT
+        user2 = User.objects.create_user(
+            name="limited_user",
+            email="limited_user@prowler.com",
+            password="TmpPass123@",
+        )
+        Membership.objects.create(user=user2, tenant=tenant)
+        UserRoleRelationship.objects.create(
+            user=user2, role=limited_role, tenant_id=tenant.id
+        )
+
+        # Attempt to switch the only MANAGE_ACCOUNT user to a role without it
+        data = {"data": [{"type": "roles", "id": str(limited_role.id)}]}
+        response = authenticated_client.patch(
+            reverse("user-roles-relationship", kwargs={"pk": create_test_user.id}),
+            data=data,
+            content_type="application/vnd.api+json",
+        )
+        assert response.status_code == status.HTTP_400_BAD_REQUEST
+        assert "MANAGE_ACCOUNT" in response.json()["errors"][0]["detail"]
+
+    def test_allow_role_change_when_other_user_has_manage_account_on_patch(
+        self, authenticated_client, roles_fixture, create_test_user, tenants_fixture
+    ):
+        # roles_fixture[1] has manage_account=False, roles_fixture[0] has manage_account=True
+        limited_role = roles_fixture[1]
+        ma_role = roles_fixture[0]
+
+        tenant = tenants_fixture[0]
+        # Create another user with MANAGE_ACCOUNT
+        user2 = User.objects.create_user(
+            name="ma_user",
+            email="ma_user@prowler.com",
+            password="TmpPass123@",
+        )
+        Membership.objects.create(user=user2, tenant=tenant)
+        UserRoleRelationship.objects.create(
+            user=user2, role=ma_role, tenant_id=tenant.id
+        )
+
+        # Now changing the first user's roles to a non-MA role should succeed
+        data = {"data": [{"type": "roles", "id": str(limited_role.id)}]}
+        response = authenticated_client.patch(
+            reverse("user-roles-relationship", kwargs={"pk": create_test_user.id}),
+            data=data,
+            content_type="application/vnd.api+json",
+        )
+        assert response.status_code == status.HTTP_204_NO_CONTENT
+
+    def test_role_destroy_only_manage_account_blocked(
+        self, authenticated_client, tenants_fixture
+    ):
+        # Use a tenant without default admin role (tenant3)
+        tenant = tenants_fixture[2]
+        user = User.objects.get(email=TEST_USER)
+        # Add membership for this tenant
+        Membership.objects.create(user=user, tenant=tenant)
+
+        # Create a single MANAGE_ACCOUNT role in this tenant
+        only_role = Role.objects.create(
+            name="only_ma",
+            tenant=tenant,
+            manage_users=True,
+            manage_account=True,
+            manage_billing=False,
+            manage_providers=False,
+            manage_integrations=False,
+            manage_scans=False,
+            unlimited_visibility=False,
+        )
+
+        # Switch token to this tenant
+        serializer = TokenSerializer(
+            data={
+                "type": "tokens",
+                "email": TEST_USER,
+                "password": TEST_PASSWORD,
+                "tenant_id": str(tenant.id),
+            }
+        )
+        serializer.is_valid(raise_exception=True)
+        access_token = serializer.validated_data["access"]
+        authenticated_client.defaults["HTTP_AUTHORIZATION"] = f"Bearer {access_token}"
+
+        # Attempt to delete the only MANAGE_ACCOUNT role
+        response = authenticated_client.delete(
+            reverse("role-detail", kwargs={"pk": only_role.id})
+        )
+        assert response.status_code == status.HTTP_400_BAD_REQUEST
+        assert Role.objects.filter(id=only_role.id).exists()
+
     def test_invalid_provider_group_id(self, authenticated_client, create_test_user):
         invalid_id = "non-existent-id"
         data = {"data": [{"type": "provider-groups", "id": invalid_id}]}
diff --git a/api/src/backend/api/v1/serializers.py b/api/src/backend/api/v1/serializers.py
@@ -422,6 +422,34 @@ def update(self, instance, validated_data):
         roles = Role.objects.filter(id__in=role_ids)
         tenant_id = self.context.get("tenant_id")
 
+        # Safeguard: A tenant must always have at least one user with MANAGE_ACCOUNT.
+        # If the target roles do NOT include MANAGE_ACCOUNT, and the current user is
+        # the only one in the tenant with MANAGE_ACCOUNT, block the update.
+        target_includes_manage_account = roles.filter(manage_account=True).exists()
+        if not target_includes_manage_account:
+            # Check if any other user has MANAGE_ACCOUNT
+            other_users_have_manage_account = (
+                UserRoleRelationship.objects.filter(
+                    tenant_id=tenant_id, role__manage_account=True
+                )
+                .exclude(user_id=instance.id)
+                .exists()
+            )
+
+            # Check if the current user has MANAGE_ACCOUNT
+            instance_has_manage_account = instance.roles.filter(
+                tenant_id=tenant_id, manage_account=True
+            ).exists()
+
+            # If the current user is the last holder of MANAGE_ACCOUNT, prevent removal
+            if instance_has_manage_account and not other_users_have_manage_account:
+                raise serializers.ValidationError(
+                    {
+                        "roles": "At least one user in the tenant must retain MANAGE_ACCOUNT. "
+                        "Assign MANAGE_ACCOUNT to another user before removing it here."
+                    }
+                )
+
         instance.roles.clear()
         new_relationships = [
             UserRoleRelationship(user=instance, role=r, tenant_id=tenant_id)
@@ -1741,6 +1769,37 @@ def update(self, instance, validated_data):
 
         if "users" in validated_data:
             users = validated_data.pop("users")
+            # Prevent a user from removing their own role assignment via Role update
+            request = self.context.get("request")
+            if request and getattr(request, "user", None):
+                request_user = request.user
+                is_currently_assigned = instance.users.filter(
+                    id=request_user.id
+                ).exists()
+                will_be_assigned = any(u.id == request_user.id for u in users)
+                if is_currently_assigned and not will_be_assigned:
+                    raise serializers.ValidationError(
+                        {"users": "Users cannot remove their own role."}
+                    )
+
+            # Safeguard MANAGE_ACCOUNT coverage when updating users of this role
+            if instance.manage_account:
+                # Existing MANAGE_ACCOUNT assignments on other roles within the tenant
+                other_ma_exists = (
+                    UserRoleRelationship.objects.filter(
+                        tenant_id=tenant_id, role__manage_account=True
+                    )
+                    .exclude(role_id=instance.id)
+                    .exists()
+                )
+
+                if not other_ma_exists and len(users) == 0:
+                    raise serializers.ValidationError(
+                        {
+                            "users": "At least one user in the tenant must retain MANAGE_ACCOUNT. "
+                            "Assign this MANAGE_ACCOUNT role to at least one user or ensure another user has it."
+                        }
+                    )
             instance.users.clear()
             through_model_instances = [
                 UserRoleRelationship(
diff --git a/api/src/backend/api/v1/views.py b/api/src/backend/api/v1/views.py
