fix: lock the External Identities section instead of erroring at members without access

backend/backend/graphene/mutations/service_accounts.py

@@ -332,6 +332,16 @@ def mutate(cls, root, info, service_account_id, name, role_id, identity_ids=None
         service_account.name = name
         service_account.role = role
         if identity_ids is not None:
+            # Binding an identity lets it mint tokens for this account, so
+            # it needs ExternalIdentities access on top of SA update — the
+            # same pair the UI gates the control on.
+            if not user_has_permission(
+                user, "read", "ExternalIdentities", service_account.organisation
+            ):
+                raise GraphQLError(
+                    "You don't have permission to manage External Identities "
+                    "in this organisation"
+                )
             identities = Identity.objects.filter(
                 id__in=identity_ids,
                 organisation=service_account.organisation,

backend/backend/graphene/types.py

@@ -1011,6 +1011,13 @@ def resolve_network_policies(self, info):
         return list(chain(account_policies, global_policies))
 
     def resolve_identities(self, info):
+        # Return an empty list instead of raising — a field error here would fail
+        # the whole Service Account query for members who can legitimately
+        # view the account, just without ExternalIdentities access.
+        if not user_has_permission(
+            info.context.user, "read", "ExternalIdentities", self.organisation
+        ):
+            return []
         return self.identities.filter(deleted_at=None)
 
 

backend/tests/api/test_service_account_identities_access.py

@@ -0,0 +1,147 @@
+"""External Identity access checks on the Service Account detail path.
+
+A member who can reach a Service Account (org-level `ServiceAccounts.read`
+or team-based access) but has no `ExternalIdentities` permission — the
+default Developer role, or any custom role that leaves the resource empty
+— was still served the account's linked identities, and the console fired
+an ungated `identities` query at them, producing a permission error toast
+on page load.
+
+The resolver now withholds the rows silently (an exception here would
+fail the whole Service Account query), and the update mutation refuses to
+bind identities without ExternalIdentities access — the one point where
+the user genuinely has to be blocked.
+"""
+
+from unittest.mock import MagicMock, patch
+
+import pytest
+from graphql import GraphQLError
+
+
+_MUTATIONS = "backend.graphene.mutations.service_accounts"
+
+
+def _info(user):
+    info = MagicMock()
+    info.context.user = user
+    return info
+
+
+def _make_sa():
+    sa = MagicMock()
+    sa.organisation = MagicMock()
+    return sa
+
+
+@patch("backend.graphene.types.user_has_permission", return_value=False)
+def test_resolve_identities_returns_empty_without_permission(mock_perm):
+    """No ExternalIdentities.read → no identity rows, and no error, so
+    the rest of the Service Account query still resolves."""
+    from backend.graphene.types import ServiceAccountType
+
+    sa = _make_sa()
+    user = MagicMock()
+
+    result = ServiceAccountType.resolve_identities(sa, _info(user))
+
+    assert result == []
+    sa.identities.filter.assert_not_called()
+
+
+@patch("backend.graphene.types.user_has_permission", return_value=True)
+def test_resolve_identities_returns_rows_when_permitted(mock_perm):
+    from backend.graphene.types import ServiceAccountType
+
+    sa = _make_sa()
+    user = MagicMock()
+    expected_qs = MagicMock()
+    sa.identities.filter.return_value = expected_qs
+
+    result = ServiceAccountType.resolve_identities(sa, _info(user))
+
+    assert result is expected_qs
+    sa.identities.filter.assert_called_once_with(deleted_at=None)
+    # The gate must match the org-level `identities` query's gate.
+    args, _kwargs = mock_perm.call_args
+    assert args[0] is user
+    assert args[1] == "read"
+    assert args[2] == "ExternalIdentities"
+    assert args[3] is sa.organisation
+
+
+def _run_update(identity_ids, has_identity_permission):
+    from backend.graphene.mutations.service_accounts import (
+        UpdateServiceAccountMutation,
+    )
+
+    sa = _make_sa()
+    user = MagicMock()
+
+    with patch(f"{_MUTATIONS}.ServiceAccount") as mock_sa_cls, patch(
+        f"{_MUTATIONS}.Role"
+    ) as mock_role_cls, patch(f"{_MUTATIONS}._check_sa_permission"), patch(
+        f"{_MUTATIONS}.role_has_global_access", return_value=False
+    ), patch(
+        f"{_MUTATIONS}.user_has_permission", return_value=has_identity_permission
+    ) as mock_perm, patch(
+        f"{_MUTATIONS}.Identity"
+    ) as mock_identity_cls:
+        mock_sa_cls.objects.get.return_value = sa
+        mock_role_cls.objects.get.return_value = MagicMock(name="role")
+
+        try:
+            UpdateServiceAccountMutation.mutate(
+                None,
+                _info(user),
+                service_account_id="sa-1",
+                name="account",
+                role_id="role-1",
+                identity_ids=identity_ids,
+            )
+            raised = None
+        except GraphQLError as e:
+            raised = e
+
+        return sa, mock_perm, mock_identity_cls, raised
+
+
+def test_update_rejects_identity_binding_without_permission():
+    """The one place blocking is warranted — and nothing is persisted."""
+    sa, _perm, mock_identity_cls, raised = _run_update(
+        identity_ids=["idn-1"], has_identity_permission=False
+    )
+
+    assert raised is not None
+    assert "External Identities" in str(raised)
+    mock_identity_cls.objects.filter.assert_not_called()
+    sa.identities.set.assert_not_called()
+    sa.save.assert_not_called()
+
+
+def test_update_binds_identities_when_permitted():
+    sa, mock_perm, mock_identity_cls, raised = _run_update(
+        identity_ids=["idn-1"], has_identity_permission=True
+    )
+
+    assert raised is None
+    sa.identities.set.assert_called_once_with(
+        mock_identity_cls.objects.filter.return_value
+    )
+    sa.save.assert_called_once()
+    args, _kwargs = mock_perm.call_args
+    assert args[1] == "read"
+    assert args[2] == "ExternalIdentities"
+
+
+def test_update_without_identity_ids_needs_no_identity_permission():
+    """Renaming or re-roling an account must not start demanding
+    ExternalIdentities access."""
+    sa, mock_perm, _identity_cls, raised = _run_update(
+        identity_ids=None, has_identity_permission=False
+    )
+
+    assert raised is None
+    mock_perm.assert_not_called()
+    sa.identities.set.assert_not_called()
+    sa.save.assert_called_once()

frontend/app/[team]/access/service-accounts/[account]/_components/ServiceAccountIdentities.tsx

@@ -2,6 +2,7 @@
 
 import { ServiceAccountType } from '@/apollo/graphql'
 import { organisationContext } from '@/contexts/organisationContext'
+import { userHasPermission } from '@/utils/access/permissions'
 import { useContext, useMemo, useRef, useState } from 'react'
 import { Button } from '@/components/common/Button'
 import { EmptyState } from '@/components/common/EmptyState'
@@ -16,17 +17,37 @@ import { toast } from 'react-toastify'
 import { KeyManagementDialog } from '@/components/service-accounts/KeyManagementDialog'
 import UpdateServiceAccount from '@/graphql/mutations/service-accounts/updateServiceAccount.gql'
 import { TbLockShare } from 'react-icons/tb'
-import { FaSearch, FaTimesCircle, FaServer } from 'react-icons/fa'
+import { FaSearch, FaTimesCircle, FaServer, FaBan } from 'react-icons/fa'
 import clsx from 'clsx'
 import GenericDialog from '@/components/common/GenericDialog'
 import { MdSearchOff } from 'react-icons/md'
 import Link from 'next/link'
 
-export const ServiceAccountIdentities = ({ account }: { account: ServiceAccountType }) => {
+export const ServiceAccountIdentities = ({
+  account,
+  canManageAccount = false,
+}: {
+  account: ServiceAccountType
+  canManageAccount?: boolean
+}) => {
   const { activeOrganisation: organisation } = useContext(organisationContext)
+
+  // External Identities are an org-level resource, so read access comes from
+  // the org role rather than the SA's team-scoped role.
+  const userCanReadIdentities = organisation
+    ? userHasPermission(organisation.role!.permissions, 'ExternalIdentities', 'read')
+    : false
+
+  // Attaching identities mutates the Service Account, so managing them needs
+  // both: visibility of the identities, and update access on this account.
+  const userCanManageIdentities = userCanReadIdentities && canManageAccount
+
+  // Skip rather than let the resolver raise — an ungated query here throws a
+  // permission error toast at anyone who can reach this page without
+  // ExternalIdentities.read (e.g. a Developer with team-based SA access).
   const { data } = useQuery(GetOrganisationIdentities, {
     variables: { organisationId: organisation?.id },
-    skip: !organisation,
+    skip: !organisation || !userCanReadIdentities,
   })
 
   const dialogRef = useRef<{ openModal: () => void; closeModal: () => void }>(null)
@@ -57,29 +78,64 @@ export const ServiceAccountIdentities = ({ account }: { account: ServiceAccountT
   )
 
   const handleSave = async () => {
-    await updateAccount({
-      variables: {
-        serviceAccountId: account.id,
-        name: account.name,
-        roleId: account.role!.id,
-        identityIds: Array.from(selected),
-      },
-      refetchQueries: [
-        { query: GetServiceAccountDetail, variables: { orgId: organisation?.id, id: account.id } },
-      ],
-    })
+    try {
+      await updateAccount({
+        variables: {
+          serviceAccountId: account.id,
+          name: account.name,
+          roleId: account.role!.id,
+          identityIds: Array.from(selected),
+        },
+        refetchQueries: [
+          {
+            query: GetServiceAccountDetail,
+            variables: { orgId: organisation?.id, id: account.id },
+          },
+        ],
+      })
+    } catch {
+      // Surfaced by the global Apollo error link
+      return
+    }
     closeManageIdentitiesDialog()
     toast.success('Updated identities for this account')
   }
 
+  const sectionHeader = (
+    <>
+      <div className="text-base font-medium mb-2">External Identities</div>
+      <div className="text-neutral-500 text-sm mb-4">
+        Manage which external identities are trusted for this account
+      </div>
+    </>
+  )
+
+  // Lock the section rather than block the page — this is a read the user
+  // simply isn't entitled to, not an action that needs an error.
+  if (!userCanReadIdentities) {
+    return (
+      <div className="py-8">
+        {sectionHeader}
+        <EmptyState
+          title="Access restricted"
+          subtitle="You don't have the permissions required to view External Identities in this organisation."
+          graphic={
+            <div className="text-neutral-300 dark:text-neutral-700 text-7xl text-center">
+              <FaBan />
+            </div>
+          }
+        >
+          <></>
+        </EmptyState>
+      </div>
+    )
+  }
+
   if (!account.serverSideKeyManagementEnabled) {
     return (
       <div className="py-8">
         {/* Server-side key management is required state */}
-        <div className="text-base font-medium mb-2">External Identities</div>
-        <div className="text-neutral-500 text-sm mb-4">
-          Manage which external identities are trusted for this account
-        </div>
+        {sectionHeader}
         <EmptyState
           title="Enable server-side key management"
           subtitle="External identities require server-side key management to manage access tokens for Service Accounts."
@@ -89,7 +145,7 @@ export const ServiceAccountIdentities = ({ account }: { account: ServiceAccountT
             </div>
           }
         >
-          <KeyManagementDialog serviceAccount={account} />
+          {canManageAccount ? <KeyManagementDialog serviceAccount={account} /> : <></>}
         </EmptyState>
       </div>
     )
@@ -104,11 +160,13 @@ export const ServiceAccountIdentities = ({ account }: { account: ServiceAccountT
             <div className="text-neutral-500 text-sm">
               Manage which external identities are trusted for this account
             </div>
-            {(account as any).identities && (account as any).identities.length > 0 && (
-              <Button variant="primary" onClick={() => openManageIdentitiesDialog()}>
-                <TbLockShare /> Manage External Identities
-              </Button>
-            )}
+            {userCanManageIdentities &&
+              (account as any).identities &&
+              (account as any).identities.length > 0 && (
+                <Button variant="primary" onClick={() => openManageIdentitiesDialog()}>
+                  <TbLockShare /> Manage External Identities
+                </Button>
+              )}
           </div>
         </div>
         <div className="space-y-4">
@@ -140,9 +198,13 @@ export const ServiceAccountIdentities = ({ account }: { account: ServiceAccountT
                 </div>
               }
             >
-              <Button variant="primary" onClick={() => openManageIdentitiesDialog()}>
-                <TbLockShare /> Manage External Identities
-              </Button>
+              {userCanManageIdentities ? (
+                <Button variant="primary" onClick={() => openManageIdentitiesDialog()}>
+                  <TbLockShare /> Manage External Identities
+                </Button>
+              ) : (
+                <></>
+              )}
             </EmptyState>
           )}
         </div>

frontend/app/[team]/access/service-accounts/[account]/page.tsx

@@ -459,7 +459,10 @@ export default function ServiceAccount(props: {
           )}
         </div>
 
-        <ServiceAccountIdentities account={account} />
+        <ServiceAccountIdentities
+          account={account}
+          canManageAccount={effectiveCanUpdateSA && hasTeamAccess}
+        />
 
         {userCanViewNetworkAccess && (
           <div className="py-4">