IAM: Protect external service accounts frontend list page (#77834)

* Add `isExternal` property to frontend model

* Remove enabled and token buttons for external SA

* Replace trash icon for lock icon for external SA

* Block the role picker for external SA

* Filter SA list using the external filter

* Add only external filter at backend

---------

Co-authored-by: Gabriel MABILLE <gamab@users.noreply.github.com>

pkg/services/serviceaccounts/api/api.go

@@ -12,6 +12,7 @@ import (
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
 	"github.com/grafana/grafana/pkg/services/auth/identity"
 	contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
+	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/grafana/grafana/pkg/services/org"
 	"github.com/grafana/grafana/pkg/services/serviceaccounts"
 	"github.com/grafana/grafana/pkg/setting"
@@ -27,6 +28,7 @@ type ServiceAccountsAPI struct {
 	RouterRegister       routing.RouteRegister
 	log                  log.Logger
 	permissionService    accesscontrol.ServiceAccountPermissionsService
+	isExternalSAEnabled  bool
 }
 
 func NewServiceAccountsAPI(
@@ -36,6 +38,7 @@ func NewServiceAccountsAPI(
 	accesscontrolService accesscontrol.Service,
 	routerRegister routing.RouteRegister,
 	permissionService accesscontrol.ServiceAccountPermissionsService,
+	features *featuremgmt.FeatureManager,
 ) *ServiceAccountsAPI {
 	return &ServiceAccountsAPI{
 		cfg:                  cfg,
@@ -45,6 +48,7 @@ func NewServiceAccountsAPI(
 		RouterRegister:       routerRegister,
 		log:                  log.New("serviceaccounts.api"),
 		permissionService:    permissionService,
+		isExternalSAEnabled:  features.IsEnabled(featuremgmt.FlagExternalServiceAccounts) || features.IsEnabled(featuremgmt.FlagExternalServiceAuth),
 	}
 }
 
@@ -265,10 +269,14 @@ func (api *ServiceAccountsAPI) SearchOrgServiceAccountsWithPaging(c *contextmode
 	// its okay that it fails, it is only filtering that might be weird, but to safe quard against any weird incoming query param
 	onlyWithExpiredTokens := c.QueryBool("expiredTokens")
 	onlyDisabled := c.QueryBool("disabled")
+	onlyExternal := c.QueryBool("external")
 	filter := serviceaccounts.FilterIncludeAll
 	if onlyWithExpiredTokens {
 		filter = serviceaccounts.FilterOnlyExpiredTokens
 	}
+	if api.isExternalSAEnabled && onlyExternal {
+		filter = serviceaccounts.FilterOnlyExternal
+	}
 	if onlyDisabled {
 		filter = serviceaccounts.FilterOnlyDisabled
 	}

pkg/services/serviceaccounts/models.go

@@ -85,7 +85,7 @@ type ServiceAccountDTO struct {
 	// example: false
 	IsDisabled bool `json:"isDisabled" xorm:"is_disabled"`
 	// example: false
-	IsManaged bool `json:"isManaged,omitempty" xorm:"-"`
+	IsExternal bool `json:"isExternal,omitempty" xorm:"-"`
 	// example: Viewer
 	Role string `json:"role" xorm:"role"`
 	// example: 0
@@ -157,7 +157,7 @@ type ServiceAccountProfileDTO struct {
 	// example: []
 	Teams []string `json:"teams" xorm:"-"`
 	// example: false
-	IsManaged bool `json:"isManaged,omitempty" xorm:"-"`
+	IsExternal bool `json:"isExternal,omitempty" xorm:"-"`
 
 	Tokens        int64           `json:"tokens,omitempty"`
 	AccessControl map[string]bool `json:"accessControl,omitempty" xorm:"-"`

pkg/services/serviceaccounts/proxy/service.go

@@ -41,7 +41,7 @@ func ProvideServiceAccountsProxy(
 		isProxyEnabled: features.IsEnabled(featuremgmt.FlagExternalServiceAccounts) || features.IsEnabled(featuremgmt.FlagExternalServiceAuth),
 	}
 
-	serviceaccountsAPI := api.NewServiceAccountsAPI(cfg, s, ac, accesscontrolService, routeRegister, permissionService)
+	serviceaccountsAPI := api.NewServiceAccountsAPI(cfg, s, ac, accesscontrolService, routeRegister, permissionService, features)
 	serviceaccountsAPI.RegisterAPIEndpoints()
 
 	return s, nil
@@ -138,7 +138,7 @@ func (s *ServiceAccountsProxy) RetrieveServiceAccount(ctx context.Context, orgID
 	}
 
 	if s.isProxyEnabled {
-		sa.IsManaged = isExternalServiceAccount(sa.Login)
+		sa.IsExternal = isExternalServiceAccount(sa.Login)
 	}
 
 	return sa, nil
@@ -175,7 +175,7 @@ func (s *ServiceAccountsProxy) SearchOrgServiceAccounts(ctx context.Context, que
 
 	if s.isProxyEnabled {
 		for i := range sa.ServiceAccounts {
-			sa.ServiceAccounts[i].IsManaged = isExternalServiceAccount(sa.ServiceAccounts[i].Login)
+			sa.ServiceAccounts[i].IsExternal = isExternalServiceAccount(sa.ServiceAccounts[i].Login)
 		}
 	}
 	return sa, nil

pkg/services/serviceaccounts/proxy/service_test.go

@@ -146,7 +146,7 @@ func TestProvideServiceAccount_crudServiceAccount(t *testing.T) {
 				serviceMock.ExpectedServiceAccountProfile = tc.expectedServiceAccount
 				sa, err := svc.RetrieveServiceAccount(context.Background(), testOrgId, testServiceAccountId)
 				assert.NoError(t, err, tc.description)
-				assert.Equal(t, tc.expectedIsExternal, sa.IsManaged, tc.description)
+				assert.Equal(t, tc.expectedIsExternal, sa.IsExternal, tc.description)
 			})
 		}
 	})
@@ -164,8 +164,8 @@ func TestProvideServiceAccount_crudServiceAccount(t *testing.T) {
 		res, err := svc.SearchOrgServiceAccounts(context.Background(), &serviceaccounts.SearchOrgServiceAccountsQuery{OrgID: 1})
 		require.Len(t, res.ServiceAccounts, 2)
 		require.NoError(t, err)
-		require.False(t, res.ServiceAccounts[0].IsManaged)
+		require.False(t, res.ServiceAccounts[0].IsExternal)
-		require.True(t, res.ServiceAccounts[1].IsManaged)
+		require.True(t, res.ServiceAccounts[1].IsExternal)
 	})
 
 	t.Run("should update service account", func(t *testing.T) {

public/api-enterprise-spec.json

@@ -6767,7 +6767,7 @@
           "type": "boolean",
           "example": false
         },
-        "isManaged": {
+        "isExternal": {
           "type": "boolean",
           "example": false
         },
@@ -6822,7 +6822,7 @@
           "type": "boolean",
           "example": false
         },
-        "isManaged": {
+        "isExternal": {
           "type": "boolean",
           "example": false
         },

public/api-merged.json

@@ -18652,7 +18652,7 @@
           "type": "boolean",
           "example": false
         },
-        "isManaged": {
+        "isExternal": {
           "type": "boolean",
           "example": false
         },
@@ -18707,7 +18707,7 @@
           "type": "boolean",
           "example": false
         },
-        "isManaged": {
+        "isExternal": {
           "type": "boolean",
           "example": false
         },

public/app/features/serviceaccounts/ServiceAccountsListPage.tsx

@@ -17,6 +17,7 @@ import {
 import EmptyListCTA from 'app/core/components/EmptyListCTA/EmptyListCTA';
 import { Page } from 'app/core/components/Page/Page';
 import PageLoader from 'app/core/components/PageLoader/PageLoader';
+import config from 'app/core/config';
 import { contextSrv } from 'app/core/core';
 import { StoreState, ServiceAccountDTO, AccessControlAction, ServiceAccountStateFilter } from 'app/types';
 
@@ -56,6 +57,16 @@ const mapDispatchToProps = {
 
 const connector = connect(mapStateToProps, mapDispatchToProps);
 
+const availableFilters = [
+  { label: 'All', value: ServiceAccountStateFilter.All },
+  { label: 'With expired tokens', value: ServiceAccountStateFilter.WithExpiredTokens },
+  { label: 'Disabled', value: ServiceAccountStateFilter.Disabled },
+];
+
+if (config.featureToggles.externalServiceAccounts || config.featureToggles.externalServiceAuth) {
+  availableFilters.push({ label: 'Managed', value: ServiceAccountStateFilter.External });
+}
+
 export const ServiceAccountsListPageUnconnected = ({
   page,
   changePage,
@@ -191,11 +202,7 @@ export const ServiceAccountsListPageUnconnected = ({
             />
           </InlineField>
           <RadioButtonGroup
-            options={[
+            options={availableFilters}
-              { label: 'All', value: ServiceAccountStateFilter.All },
-              { label: 'With expired tokens', value: ServiceAccountStateFilter.WithExpiredTokens },
-              { label: 'Disabled', value: ServiceAccountStateFilter.Disabled },
-            ]}
             onChange={onStateFilterChange}
             value={serviceAccountStateFilter}
             className={styles.filter}

public/app/features/serviceaccounts/components/ServiceAccountsListItem.tsx

@@ -91,7 +91,7 @@ const ServiceAccountListItem = memo(
             <OrgRolePicker
               aria-label="Role"
               value={serviceAccount.role}
-              disabled={!canUpdateRole || serviceAccount.isDisabled}
+              disabled={serviceAccount.isExternal || !canUpdateRole || serviceAccount.isDisabled}
               onChange={(newRole) => onRoleChange(newRole, serviceAccount)}
             />
           </td>
@@ -112,32 +112,48 @@ const ServiceAccountListItem = memo(
           </a>
         </td>
         <td>
-          <HorizontalGroup justify="flex-end">
+          {!serviceAccount.isExternal && (
-            {contextSrv.hasPermission(AccessControlAction.ServiceAccountsWrite) && !serviceAccount.tokens && (
+            <HorizontalGroup justify="flex-end">
-              <Button onClick={() => onAddTokenClick(serviceAccount)} disabled={serviceAccount.isDisabled}>
+              {contextSrv.hasPermission(AccessControlAction.ServiceAccountsWrite) && !serviceAccount.tokens && (
-                Add token
+                <Button
-              </Button>
+                  onClick={() => onAddTokenClick(serviceAccount)}
-            )}
+                  disabled={serviceAccount.isDisabled}
-            {contextSrv.hasPermissionInMetadata(AccessControlAction.ServiceAccountsWrite, serviceAccount) &&
+                  className={styles.actionButton}
-              (serviceAccount.isDisabled ? (
+                >
-                <Button variant="primary" onClick={() => onEnable(serviceAccount)}>
+                  Add token
-                  Enable
                 </Button>
-              ) : (
+              )}
-                <Button variant="secondary" onClick={() => onDisable(serviceAccount)}>
+              {contextSrv.hasPermissionInMetadata(AccessControlAction.ServiceAccountsWrite, serviceAccount) &&
-                  Disable
+                (serviceAccount.isDisabled ? (
-                </Button>
+                  <Button variant="primary" onClick={() => onEnable(serviceAccount)} className={styles.actionButton}>
-              ))}
+                    Enable
-            {contextSrv.hasPermissionInMetadata(AccessControlAction.ServiceAccountsDelete, serviceAccount) && (
+                  </Button>
+                ) : (
+                  <Button variant="secondary" onClick={() => onDisable(serviceAccount)} className={styles.actionButton}>
+                    Disable
+                  </Button>
+                ))}
+              {contextSrv.hasPermissionInMetadata(AccessControlAction.ServiceAccountsDelete, serviceAccount) && (
+                <IconButton
+                  className={styles.deleteButton}
+                  name="trash-alt"
+                  size="md"
+                  onClick={() => onRemoveButtonClick(serviceAccount)}
+                  tooltip={`Delete service account ${serviceAccount.name}`}
+                />
+              )}
+            </HorizontalGroup>
+          )}
+          {serviceAccount.isExternal && (
+            <HorizontalGroup justify="flex-end">
               <IconButton
-                className={styles.deleteButton}
+                disabled={true}
-                name="trash-alt"
+                name="lock"
                 size="md"
-                onClick={() => onRemoveButtonClick(serviceAccount)}
+                tooltip={`This is a managed service account and cannot be modified.`}
-                tooltip={`Delete service account ${serviceAccount.name}`}
               />
-            )}
+            </HorizontalGroup>
-          </HorizontalGroup>
+          )}
         </td>
       </tr>
     );
@@ -174,6 +190,9 @@ const getStyles = (theme: GrafanaTheme2) => {
         color: ${theme.colors.text.secondary};
       }
     `,
+    actionButton: css({
+      minWidth: 85,
+    }),
   };
 };
 

public/app/features/serviceaccounts/state/actions.ts

@@ -114,6 +114,8 @@ const getStateFilter = (value: ServiceAccountStateFilter) => {
       return '&expiredTokens=true';
     case ServiceAccountStateFilter.Disabled:
       return '&disabled=true';
+    case ServiceAccountStateFilter.External:
+      return '&external=true';
     default:
       return '';
   }

public/app/types/serviceaccount.ts

@@ -34,6 +34,7 @@ export interface ServiceAccountDTO extends WithAccessControlMetadata {
   avatarUrl?: string;
   createdAt: string;
   isDisabled: boolean;
+  isExternal?: boolean;
   teams: string[];
   role: OrgRole;
   roles?: Role[];
@@ -60,6 +61,7 @@ export interface ServiceAccountProfileState {
 export enum ServiceAccountStateFilter {
   All = 'All',
   WithExpiredTokens = 'WithExpiredTokens',
+  External = 'External',
   Disabled = 'Disabled',
 }
 

public/openapi3.json

@@ -9554,7 +9554,7 @@
             "example": false,
             "type": "boolean"
           },
-          "isManaged": {
+          "isExternal": {
             "example": false,
             "type": "boolean"
           },
@@ -9609,7 +9609,7 @@
             "example": false,
             "type": "boolean"
           },
-          "isManaged": {
+          "isExternal": {
             "example": false,
             "type": "boolean"
           },