From 34d6018ad98d69f21f3e8a8aaa8f785ebb280f0d Mon Sep 17 00:00:00 2001
From: Nimish <nimish@phase.dev>
Date: Sat, 14 Mar 2026 13:25:32 +0800
Subject: [PATCH] fix: enforce proper permission check for
 UpdateServiceAccountHandlersMutation

---
 .../graphene/mutations/service_accounts.py    |  6 +-
 backend/tests/api/mutations/__init__.py       |  0
 .../test_service_account_handlers.py          | 63 +++++++++++++++++++
 3 files changed, 67 insertions(+), 2 deletions(-)
 create mode 100644 backend/tests/api/mutations/__init__.py
 create mode 100644 backend/tests/api/mutations/test_service_account_handlers.py

diff --git a/backend/backend/graphene/mutations/service_accounts.py b/backend/backend/graphene/mutations/service_accounts.py
index cc91d8ba3..10680e303 100644
--- a/backend/backend/graphene/mutations/service_accounts.py
+++ b/backend/backend/graphene/mutations/service_accounts.py
@@ -213,9 +213,11 @@ def mutate(cls, root, info, organisation_id, handlers):
         user = info.context.user
         org = Organisation.objects.get(id=organisation_id)
 
-        if not user_is_org_member(user.userId, organisation_id):
+        if not user_has_permission(
+            user, "update", "ServiceAccounts", org
+        ):
             raise GraphQLError(
-                "You are not a member of this organisation and cannot perform this operation"
+                "You don't have the permissions required to update Service Accounts in this organisation"
             )
 
         ServiceAccountHandler.objects.filter(service_account__organisation=org).delete()
diff --git a/backend/tests/api/mutations/__init__.py b/backend/tests/api/mutations/__init__.py
new file mode 100644
index 000000000..e69de29bb
diff --git a/backend/tests/api/mutations/test_service_account_handlers.py b/backend/tests/api/mutations/test_service_account_handlers.py
new file mode 100644
index 000000000..ff0b86dc7
--- /dev/null
+++ b/backend/tests/api/mutations/test_service_account_handlers.py
@@ -0,0 +1,63 @@
+import pytest
+from unittest.mock import MagicMock, patch
+from graphql import GraphQLError
+
+
+class TestUpdateServiceAccountHandlersMutationPermission:
+    """Tests that UpdateServiceAccountHandlersMutation checks proper permissions."""
+
+    @patch("backend.graphene.mutations.service_accounts.ServiceAccountHandler")
+    @patch("backend.graphene.mutations.service_accounts.ServiceAccount")
+    @patch("backend.graphene.mutations.service_accounts.user_has_permission", return_value=False)
+    @patch("backend.graphene.mutations.service_accounts.Organisation")
+    def test_rejects_user_without_service_account_permission(
+        self, MockOrg, mock_perm, MockSA, MockHandler
+    ):
+        """A user without ServiceAccounts update permission should be rejected."""
+        from backend.graphene.mutations.service_accounts import (
+            UpdateServiceAccountHandlersMutation,
+        )
+
+        mock_org = MagicMock()
+        MockOrg.objects.get.return_value = mock_org
+
+        mock_info = MagicMock()
+        mock_info.context.user.userId = "user-123"
+
+        with pytest.raises(
+            GraphQLError,
+            match="You don't have the permissions required to update Service Accounts",
+        ):
+            UpdateServiceAccountHandlersMutation.mutate(
+                None, mock_info, "org-123", []
+            )
+
+        mock_perm.assert_called_once_with(
+            mock_info.context.user, "update", "ServiceAccounts", mock_org
+        )
+
+    @patch("backend.graphene.mutations.service_accounts.ServiceAccountHandler")
+    @patch("backend.graphene.mutations.service_accounts.ServiceAccount")
+    @patch("backend.graphene.mutations.service_accounts.user_has_permission", return_value=True)
+    @patch("backend.graphene.mutations.service_accounts.Organisation")
+    def test_allows_user_with_service_account_permission(
+        self, MockOrg, mock_perm, MockSA, MockHandler
+    ):
+        """A user with ServiceAccounts update permission should be allowed."""
+        from backend.graphene.mutations.service_accounts import (
+            UpdateServiceAccountHandlersMutation,
+        )
+
+        mock_org = MagicMock()
+        MockOrg.objects.get.return_value = mock_org
+
+        mock_info = MagicMock()
+
+        MockHandler.objects.filter.return_value = MockHandler.objects
+        MockHandler.objects.delete.return_value = None
+
+        result = UpdateServiceAccountHandlersMutation.mutate(
+            None, mock_info, "org-123", []
+        )
+
+        assert result.ok is True