API keys: Remove state hideAPIkeys and refactor interface to IsDisabled (#64018)

* remove state and refactor interface to IsDisabled

* update docs and span

* Update pkg/services/apikey/apikey.go

Co-authored-by: Alexander Zobnin <alexanderzobnin@gmail.com>

---------

Co-authored-by: Alexander Zobnin <alexanderzobnin@gmail.com>

docs/sources/administration/api-keys/index.md

@@ -69,14 +69,3 @@ You can choose to migrate a single API key or all API keys. Note that when you m
 1. Sign in to Grafana, hover your cursor over **Configuration** (the gear icon), and click **API Keys**.
 1. Find the API Key you want to migrate.
 1. Click **Migrate to service account**.
-
-### Revert service account token to API key
-
-**Note:** This is undesired operation and should be used only in emergency situations.
-
-It is possible to convert back service account token to API key. You can use the [Revert service account token to API key HTTP API]({{< relref "../../developers/http_api/create-api-tokens-for-org/#how-to-create-a-new-organization-and-an-api-token" >}}) for that.
-
-**The revert will perform the following actions:**
-
-1. Convert the given service account token back to API key
-1. Delete the service account associated with the given key. **Make sure there are no other tokens associated with the service account, otherwise they all will be deleted.**

pkg/services/apikey/apikey.go

@@ -7,11 +7,12 @@ import (
 type Service interface {
 	GetAPIKeys(ctx context.Context, query *GetApiKeysQuery) error
 	GetAllAPIKeys(ctx context.Context, orgID int64) ([]*APIKey, error)
-	CountAPIKeys(ctx context.Context, orgID int64) (int64, error)
 	DeleteApiKey(ctx context.Context, cmd *DeleteCommand) error
 	AddAPIKey(ctx context.Context, cmd *AddCommand) error
 	GetApiKeyById(ctx context.Context, query *GetByIDQuery) error
 	GetApiKeyByName(ctx context.Context, query *GetByNameQuery) error
 	GetAPIKeyByHash(ctx context.Context, hash string) (*APIKey, error)
 	UpdateAPIKeyLastUsedDate(ctx context.Context, tokenID int64) error
+	// IsDisabled returns true if the API key is not available for use.
+	IsDisabled(ctx context.Context, orgID int64) (bool, error)
 }

pkg/services/apikey/apikeyimpl/apikey.go

@@ -68,8 +68,15 @@ func (s *Service) AddAPIKey(ctx context.Context, cmd *apikey.AddCommand) error {
 func (s *Service) UpdateAPIKeyLastUsedDate(ctx context.Context, tokenID int64) error {
 	return s.store.UpdateAPIKeyLastUsedDate(ctx, tokenID)
 }
-func (s *Service) CountAPIKeys(ctx context.Context, orgID int64) (int64, error) {
+
-	return s.store.CountAPIKeys(ctx, orgID)
+// IsDisabled returns true if the apikey service is disabled for the given org.
+// This is the case if the org has no apikeys.
+func (s *Service) IsDisabled(ctx context.Context, orgID int64) (bool, error) {
+	apikeys, err := s.store.CountAPIKeys(ctx, orgID)
+	if err != nil {
+		return false, err
+	}
+	return apikeys == 0, nil
 }
 
 func readQuotaConfig(cfg *setting.Cfg) (*quota.Map, error) {

pkg/services/apikey/apikeytest/fake.go

@@ -8,7 +8,7 @@ import (
 
 type Service struct {
 	ExpectedError   error
-	ExpectedCount   int64
+	ExpectedBool    bool
 	ExpectedAPIKeys []*apikey.APIKey
 	ExpectedAPIKey  *apikey.APIKey
 }
@@ -20,9 +20,6 @@ func (s *Service) GetAPIKeys(ctx context.Context, query *apikey.GetApiKeysQuery)
 func (s *Service) GetAllAPIKeys(ctx context.Context, orgID int64) ([]*apikey.APIKey, error) {
 	return s.ExpectedAPIKeys, s.ExpectedError
 }
-func (s *Service) CountAPIKeys(ctx context.Context, orgID int64) (int64, error) {
-	return s.ExpectedCount, s.ExpectedError
-}
 func (s *Service) GetApiKeyById(ctx context.Context, query *apikey.GetByIDQuery) error {
 	query.Result = s.ExpectedAPIKey
 	return s.ExpectedError
@@ -44,3 +41,6 @@ func (s *Service) AddAPIKey(ctx context.Context, cmd *apikey.AddCommand) error {
 func (s *Service) UpdateAPIKeyLastUsedDate(ctx context.Context, tokenID int64) error {
 	return s.ExpectedError
 }
+func (s *Service) IsDisabled(ctx context.Context, orgID int64) (bool, error) {
+	return s.ExpectedBool, s.ExpectedError
+}

pkg/services/navtree/navtreeimpl/admin.go

@@ -79,15 +79,11 @@ func (s *ServiceImpl) getOrgAdminNode(c *contextmodel.ReqContext) (*navtree.NavL
 		})
 	}
 
-	hideApiKeys, _, _ := s.kvStore.Get(c.Req.Context(), c.OrgID, "serviceaccounts", "hideApiKeys")
+	disabled, err := s.apiKeyService.IsDisabled(c.Req.Context(), c.OrgID)
-	apiKeys, err := s.apiKeyService.CountAPIKeys(c.Req.Context(), c.OrgID)
 	if err != nil {
 		return nil, err
 	}
-
+	if hasAccess(ac.ReqOrgAdmin, ac.ApiKeyAccessEvaluator) && !disabled {
-	// Hide API keys if the global setting is set or if the org setting is set and there are no API keys
-	apiKeysHidden := hideApiKeys == "1" && apiKeys == 0
-	if hasAccess(ac.ReqOrgAdmin, ac.ApiKeyAccessEvaluator) && !apiKeysHidden {
 		configNodes = append(configNodes, &navtree.NavLink{
 			Text:     "API keys",
 			Id:       "apikeys",

pkg/services/serviceaccounts/api/api.go

@@ -40,7 +40,6 @@ type service interface {
 	SearchOrgServiceAccounts(ctx context.Context, query *serviceaccounts.SearchOrgServiceAccountsQuery) (*serviceaccounts.SearchOrgServiceAccountsResult, error)
 	ListTokens(ctx context.Context, query *serviceaccounts.GetSATokensQuery) ([]apikey.APIKey, error)
 	DeleteServiceAccount(ctx context.Context, orgID, serviceAccountID int64) error
-	HideApiKeysTab(ctx context.Context, orgID int64) error
 	MigrateApiKeysToServiceAccounts(ctx context.Context, orgID int64) error
 	MigrateApiKey(ctx context.Context, orgID int64, keyId int64) error
 	// Service account tokens
@@ -86,8 +85,6 @@ func (api *ServiceAccountsAPI) RegisterAPIEndpoints() {
 			accesscontrol.EvalPermission(serviceaccounts.ActionWrite, serviceaccounts.ScopeID)), routing.Wrap(api.CreateToken))
 		serviceAccountsRoute.Delete("/:serviceAccountId/tokens/:tokenId", auth(middleware.ReqOrgAdmin,
 			accesscontrol.EvalPermission(serviceaccounts.ActionWrite, serviceaccounts.ScopeID)), routing.Wrap(api.DeleteToken))
-		serviceAccountsRoute.Post("/hideApiKeys", auth(middleware.ReqOrgAdmin,
-			accesscontrol.EvalPermission(serviceaccounts.ActionCreate)), routing.Wrap(api.HideApiKeysTab))
 		serviceAccountsRoute.Post("/migrate", auth(middleware.ReqOrgAdmin,
 			accesscontrol.EvalPermission(serviceaccounts.ActionCreate)), routing.Wrap(api.MigrateApiKeysToServiceAccounts))
 		serviceAccountsRoute.Post("/migrate/:keyId", auth(middleware.ReqOrgAdmin,
@@ -357,14 +354,6 @@ func (api *ServiceAccountsAPI) SearchOrgServiceAccountsWithPaging(c *contextmode
 	return response.JSON(http.StatusOK, serviceAccountSearch)
 }
 
-// POST /api/serviceaccounts/hideapikeys
-func (api *ServiceAccountsAPI) HideApiKeysTab(ctx *contextmodel.ReqContext) response.Response {
-	if err := api.service.HideApiKeysTab(ctx.Req.Context(), ctx.OrgID); err != nil {
-		return response.Error(http.StatusInternalServerError, "Internal server error", err)
-	}
-	return response.Success("API keys hidden")
-}
-
 // POST /api/serviceaccounts/migrate
 func (api *ServiceAccountsAPI) MigrateApiKeysToServiceAccounts(ctx *contextmodel.ReqContext) response.Response {
 	if err := api.service.MigrateApiKeysToServiceAccounts(ctx.Req.Context(), ctx.OrgID); err != nil {

pkg/services/serviceaccounts/database/store.go

@@ -364,13 +364,6 @@ func (s *ServiceAccountsStoreImpl) SearchOrgServiceAccounts(ctx context.Context,
 	return searchResult, nil
 }
 
-func (s *ServiceAccountsStoreImpl) HideApiKeysTab(ctx context.Context, orgId int64) error {
-	if err := s.kvStore.Set(ctx, orgId, "serviceaccounts", "hideApiKeys", "1"); err != nil {
-		s.log.Error("Failed to hide API keys tab", err)
-	}
-	return nil
-}
-
 func (s *ServiceAccountsStoreImpl) MigrateApiKeysToServiceAccounts(ctx context.Context, orgId int64) error {
 	basicKeys, err := s.apiKeyService.GetAllAPIKeys(ctx, orgId)
 	if err != nil {

pkg/services/serviceaccounts/manager/service.go

@@ -226,13 +226,6 @@ func (sa *ServiceAccountsService) DeleteServiceAccountToken(ctx context.Context,
 	return sa.store.DeleteServiceAccountToken(ctx, orgID, serviceAccountID, tokenID)
 }
 
-func (sa *ServiceAccountsService) HideApiKeysTab(ctx context.Context, orgID int64) error {
-	if err := validOrgID(orgID); err != nil {
-		return err
-	}
-	return sa.store.HideApiKeysTab(ctx, orgID)
-}
-
 func (sa *ServiceAccountsService) MigrateApiKey(ctx context.Context, orgID, keyID int64) error {
 	if err := validOrgID(orgID); err != nil {
 		return err

pkg/services/serviceaccounts/manager/service_test.go

@@ -59,11 +59,6 @@ func (f *FakeServiceAccountStore) DeleteServiceAccount(ctx context.Context, orgI
 	return f.ExpectedError
 }
 
-// HideApiKeysTab is a fake hiding the api keys tab.
-func (f *FakeServiceAccountStore) HideApiKeysTab(ctx context.Context, orgID int64) error {
-	return f.ExpectedError
-}
-
 // MigrateApiKeysToServiceAccounts is a fake migrating api keys to service accounts.
 func (f *FakeServiceAccountStore) MigrateApiKeysToServiceAccounts(ctx context.Context, orgID int64) error {
 	return f.ExpectedError

pkg/services/serviceaccounts/manager/store.go

@@ -7,17 +7,6 @@ import (
 	"github.com/grafana/grafana/pkg/services/serviceaccounts"
 )
 
-/*
-Store is the database store for service accounts.
-
-migration from apikeys to service accounts:
-HideApiKeyTab is used to hide the api key tab in the UI.
-MigrateApiKeysToServiceAccounts migrates all API keys to service accounts.
-MigrateApiKey migrates a single API key to a service account.
-
-// only used for interal api calls
-RevertApiKey reverts a single service account to an API key.
-*/
 type store interface {
 	CreateServiceAccount(ctx context.Context, orgID int64, saForm *serviceaccounts.CreateServiceAccountForm) (*serviceaccounts.ServiceAccountDTO, error)
 	SearchOrgServiceAccounts(ctx context.Context, query *serviceaccounts.SearchOrgServiceAccountsQuery) (*serviceaccounts.SearchOrgServiceAccountsResult, error)
@@ -26,7 +15,6 @@ type store interface {
 	RetrieveServiceAccount(ctx context.Context, orgID, serviceAccountID int64) (*serviceaccounts.ServiceAccountProfileDTO, error)
 	RetrieveServiceAccountIdByName(ctx context.Context, orgID int64, name string) (int64, error)
 	DeleteServiceAccount(ctx context.Context, orgID, serviceAccountID int64) error
-	HideApiKeysTab(ctx context.Context, orgID int64) error
 	MigrateApiKeysToServiceAccounts(ctx context.Context, orgID int64) error
 	MigrateApiKey(ctx context.Context, orgID int64, keyId int64) error
 	ListTokens(ctx context.Context, query *serviceaccounts.GetSATokensQuery) ([]apikey.APIKey, error)

public/app/features/api-keys/APIKeysMigratedCard.tsx

@@ -1,52 +0,0 @@
-import { css } from '@emotion/css';
-import React, { useState } from 'react';
-
-import { GrafanaTheme2 } from '@grafana/data';
-import { Alert, ConfirmModal, useStyles2, Button } from '@grafana/ui';
-
-interface Props {
-  onHideApiKeys: () => void;
-  apikeys: number;
-}
-
-export const APIKeysMigratedCard = ({ onHideApiKeys, apikeys }: Props): JSX.Element => {
-  const [isModalOpen, setIsModalOpen] = useState(false);
-  const styles = useStyles2(getStyles);
-
-  return (
-    <Alert title="If you see any API keys please migrate them to Service account tokens." severity="info">
-      <div className={styles.text}>
-        Migrated API keys are safe and continue working as they used to. You can find them inside the respective service
-        account.
-      </div>
-      <div className={styles.actionRow}>
-        <Button className={styles.actionButton} onClick={() => setIsModalOpen(true)} disabled={apikeys !== 0}>
-          Hide API keys page
-        </Button>
-        <ConfirmModal
-          title={'Hide API Keys page'}
-          isOpen={isModalOpen}
-          body={'Did you want to hide the API keys page?'}
-          confirmText={'Yes, hide API keys page.'}
-          onConfirm={onHideApiKeys}
-          onDismiss={() => setIsModalOpen(false)}
-          confirmButtonVariant="primary"
-        />
-        <a href="org/serviceaccounts">View service accounts page</a>
-      </div>
-    </Alert>
-  );
-};
-
-export const getStyles = (theme: GrafanaTheme2) => ({
-  text: css`
-    margin-bottom: ${theme.spacing(2)};
-  `,
-  actionRow: css`
-    display: flex;
-    align-items: center;
-  `,
-  actionButton: css`
-    margin-right: ${theme.spacing(2)};
-  `,
-});

public/app/features/api-keys/ApiKeysController.tsx

@@ -1,21 +0,0 @@
-import { FC, useCallback, useState } from 'react';
-
-interface Api {
-  isAdding: boolean;
-  toggleIsAdding: () => void;
-}
-
-interface Props {
-  children: (props: Api) => JSX.Element;
-}
-
-export const ApiKeysController: FC<Props> = ({ children }) => {
-  // FIXME(eleijonmarck): could not remove state from this component
-  // as component cannot render properly without it
-  const [isAdding, setIsAdding] = useState<boolean>(false);
-  const toggleIsAdding = useCallback(() => {
-    setIsAdding(!isAdding);
-  }, [isAdding]);
-
-  return children({ isAdding, toggleIsAdding });
-};

public/app/features/api-keys/ApiKeysPage.test.tsx

@@ -29,7 +29,6 @@ const setup = (propOverrides: Partial<Props>) => {
   const migrateAllMock = jest.fn();
   const toggleIncludeExpiredMock = jest.fn();
   const setSearchQueryMock = mockToolkitActionCreator(setSearchQuery);
-  const hideApiKeysMock = jest.fn();
   const props: Props = {
     apiKeys: [] as ApiKey[],
     searchQuery: '',
@@ -39,7 +38,6 @@ const setup = (propOverrides: Partial<Props>) => {
     setSearchQuery: setSearchQueryMock,
     migrateApiKey: migrateApiKeyMock,
     migrateAll: migrateAllMock,
-    hideApiKeys: hideApiKeysMock,
     apiKeysCount: 0,
     timeZone: 'utc',
     includeExpired: false,

public/app/features/api-keys/ApiKeysPage.tsx

@@ -9,19 +9,10 @@ import { contextSrv } from 'app/core/core';
 import { getTimeZone } from 'app/features/profile/state/selectors';
 import { AccessControlAction, ApiKey, StoreState } from 'app/types';
 
-import { APIKeysMigratedCard } from './APIKeysMigratedCard';
 import { ApiKeysActionBar } from './ApiKeysActionBar';
-import { ApiKeysController } from './ApiKeysController';
 import { ApiKeysTable } from './ApiKeysTable';
 import { MigrateToServiceAccountsCard } from './MigrateToServiceAccountsCard';
-import {
+import { deleteApiKey, migrateApiKey, migrateAll, loadApiKeys, toggleIncludeExpired } from './state/actions';
-  deleteApiKey,
-  migrateApiKey,
-  migrateAll,
-  loadApiKeys,
-  toggleIncludeExpired,
-  hideApiKeys,
-} from './state/actions';
 import { setSearchQuery } from './state/reducers';
 import { getApiKeys, getApiKeysCount, getIncludeExpired, getIncludeExpiredDisabled } from './state/selectors';
 
@@ -51,7 +42,6 @@ const mapDispatchToProps = {
   migrateAll,
   setSearchQuery,
   toggleIncludeExpired,
-  hideApiKeys,
 };
 
 const connector = connect(mapStateToProps, mapDispatchToProps);
@@ -97,9 +87,9 @@ export class ApiKeysPageUnconnected extends PureComponent<Props, State> {
     this.props.toggleIncludeExpired();
   };
 
-  onHideApiKeys = async () => {
+  onMigrateApiKeys = async () => {
     try {
-      await this.props.hideApiKeys();
+      this.onMigrateAll();
       let serviceAccountsUrl = '/org/serviceaccounts';
       locationService.push(serviceAccountsUrl);
       window.location.reload();
@@ -128,42 +118,33 @@ export class ApiKeysPageUnconnected extends PureComponent<Props, State> {
       );
     }
 
+    const showTable = apiKeysCount > 0;
     return (
       <Page {...defaultPageProps}>
         <Page.Contents isLoading={false}>
-          <ApiKeysController>
+          <>
-            {({}) => {
+            <MigrateToServiceAccountsCard onMigrate={this.onMigrateApiKeys} apikeysCount={apiKeysCount} />
-              const showTable = apiKeysCount > 0;
+            {showTable ? (
-              return (
+              <ApiKeysActionBar
-                <>
+                searchQuery={searchQuery}
-                  {apiKeysCount !== 0 && <MigrateToServiceAccountsCard onMigrate={this.onMigrateAll} />}
+                disabled={!canCreate}
-                  {apiKeysCount === 0 && (
+                onSearchChange={this.onSearchQueryChange}
-                    <APIKeysMigratedCard onHideApiKeys={this.onHideApiKeys} apikeys={apiKeysCount} />
+              />
-                  )}
+            ) : null}
-                  {showTable ? (
+            {showTable ? (
-                    <ApiKeysActionBar
+              <VerticalGroup>
-                      searchQuery={searchQuery}
+                <InlineField disabled={includeExpiredDisabled} label="Include expired keys">
-                      disabled={!canCreate}
+                  <InlineSwitch id="showExpired" value={includeExpired} onChange={this.onIncludeExpiredChange} />
-                      onSearchChange={this.onSearchQueryChange}
+                </InlineField>
-                    />
+                <ApiKeysTable
-                  ) : null}
+                  apiKeys={apiKeys}
-                  {showTable ? (
+                  timeZone={timeZone}
-                    <VerticalGroup>
+                  onMigrate={this.onMigrateApiKey}
-                      <InlineField disabled={includeExpiredDisabled} label="Include expired keys">
+                  onDelete={this.onDeleteApiKey}
-                        <InlineSwitch id="showExpired" value={includeExpired} onChange={this.onIncludeExpiredChange} />
+                />
-                      </InlineField>
+              </VerticalGroup>
-                      <ApiKeysTable
+            ) : null}
-                        apiKeys={apiKeys}
+          </>
-                        timeZone={timeZone}
-                        onMigrate={this.onMigrateApiKey}
-                        onDelete={this.onDeleteApiKey}
-                      />
-                    </VerticalGroup>
-                  ) : null}
-                </>
-              );
-            }}
-          </ApiKeysController>
         </Page.Contents>
       </Page>
     );

public/app/features/api-keys/MigrateToServiceAccountsCard.tsx

@@ -6,10 +6,11 @@ import { Alert, Button, ConfirmModal, useStyles2 } from '@grafana/ui';
 
 interface Props {
   onMigrate: () => void;
+  apikeysCount: number;
   disabled?: boolean;
 }
 
-export const MigrateToServiceAccountsCard = ({ onMigrate, disabled }: Props): JSX.Element => {
+export const MigrateToServiceAccountsCard = ({ onMigrate, apikeysCount, disabled }: Props): JSX.Element => {
   const [isModalOpen, setIsModalOpen] = useState(false);
   const styles = useStyles2(getStyles);
 
@@ -23,30 +24,49 @@ export const MigrateToServiceAccountsCard = ({ onMigrate, disabled }: Props): JS
       Find out more about the migration here.
     </a>
   );
-  const migrationBoxDesc = <span>Are you sure you want to migrate all API keys to service accounts? {docsLink}</span>;
+  const migrationBoxDesc = (
+    <span>
+      Are you sure you want to migrate all API keys to service accounts? {docsLink}
+      <br>This hides the API keys tab.</br>
+    </span>
+  );
 
   return (
-    <Alert title="Switch from API keys to service accounts" severity="warning">
+    <>
-      <div className={styles.text}>
+      {apikeysCount > 0 && (
-        We will soon deprecate API keys. Each API key will be migrated into a service account with a token and will
+        <Alert title="Switch from API keys to service accounts" severity="warning">
-        continue to work as they were. We encourage you to migrate your API keys to service accounts now. {docsLink}
+          <div className={styles.text}>
-      </div>
+            We will soon deprecate API keys. Each API key will be migrated into a service account with a token and will
-      <div className={styles.actionRow}>
+            continue to work as they were. We encourage you to migrate your API keys to service accounts now. {docsLink}
-        <Button className={styles.actionButton} onClick={() => setIsModalOpen(true)}>
+          </div>
-          Migrate all service accounts
+          <div className={styles.actionRow}>
-        </Button>
+            <Button className={styles.actionButton} onClick={() => setIsModalOpen(true)}>
-        <ConfirmModal
+              Migrate all service accounts
-          title={'Migrate API keys to service accounts'}
+            </Button>
-          isOpen={isModalOpen}
+            <ConfirmModal
-          body={migrationBoxDesc}
+              title={'Migrate API keys to service accounts'}
-          confirmText={'Yes, migrate now'}
+              isOpen={isModalOpen}
-          onConfirm={onMigrate}
+              body={migrationBoxDesc}
-          onDismiss={() => setIsModalOpen(false)}
+              confirmText={'Yes, migrate now'}
-          confirmVariant="primary"
+              onConfirm={onMigrate}
-          confirmButtonVariant="primary"
+              onDismiss={() => setIsModalOpen(false)}
-        />
+              confirmVariant="primary"
-      </div>
+              confirmButtonVariant="primary"
-    </Alert>
+            />
+          </div>
+        </Alert>
+      )}
+      {apikeysCount === 0 && (
+        <>
+          <Alert title="No API keys found" severity="warning">
+            <div className={styles.text}>
+              No API keys were found. If you reload the browser, this tab will be not available. If any API keys are
+              created, this tab will appear again.
+            </div>
+          </Alert>
+        </>
+      )}
+    </>
   );
 };
 

public/app/features/api-keys/state/actions.ts

@@ -42,12 +42,6 @@ export function migrateAll(): ThunkResult<void> {
   };
 }
 
-export function hideApiKeys(): ThunkResult<void> {
-  return async (dispatch) => {
-    await getBackendSrv().post('/api/serviceaccounts/hideApiKeys');
-  };
-}
-
 export function toggleIncludeExpired(): ThunkResult<void> {
   return (dispatch) => {
     dispatch(includeExpiredToggled());