From 79340c087f97f83c72d82d48efb651bc5ad7676d Mon Sep 17 00:00:00 2001 From: Eric Leijonmarck Date: Tue, 8 Feb 2022 14:31:34 +0100 Subject: [PATCH] Add number of tokens to the service accounts view (#44919) * feat: add serviceaccountDTO * WIP * feat: listing number of tokens for a given service account * nit: removed fmt * Update pkg/services/serviceaccounts/database/database.go * Update public/app/features/serviceaccounts/ServiceAccountsListPage.tsx * fixes * align DTOProfile data to the frontend * reviewed myself fixes * fix: tests fix --- pkg/services/serviceaccounts/api/api.go | 11 +++-- pkg/services/serviceaccounts/api/api_test.go | 12 +++--- .../serviceaccounts/database/database.go | 41 ++++++++++++++----- .../serviceaccounts/manager/service.go | 3 +- pkg/services/serviceaccounts/models.go | 26 +++++++++++- .../serviceaccounts/serviceaccounts.go | 9 ++-- pkg/services/serviceaccounts/tests/common.go | 8 ++-- .../serviceaccounts/ServiceAccountProfile.tsx | 6 +-- .../ServiceAccountsListPage.tsx | 13 +++--- .../features/serviceaccounts/state/actions.ts | 9 ++-- .../serviceaccounts/state/selectors.ts | 2 +- public/app/types/serviceaccount.ts | 8 ++-- 12 files changed, 99 insertions(+), 49 deletions(-) diff --git a/pkg/services/serviceaccounts/api/api.go b/pkg/services/serviceaccounts/api/api.go index 86e7084c959..bde88d1bf39 100644 --- a/pkg/services/serviceaccounts/api/api.go +++ b/pkg/services/serviceaccounts/api/api.go @@ -33,6 +33,11 @@ type ServiceAccountsAPI struct { apiKeyStore APIKeyStore } +type serviceAccountIdDTO struct { + Id int64 `json:"id"` + Message string `json:"message"` +} + func NewServiceAccountsAPI( cfg *setting.Cfg, service serviceaccounts.Service, @@ -88,11 +93,11 @@ func (api *ServiceAccountsAPI) CreateServiceAccount(c *models.ReqContext) respon case err != nil: return response.Error(http.StatusInternalServerError, "Failed to create service account", err) } - result := models.UserIdDTO{ - Message: "Service account created", + sa := &serviceAccountIdDTO{ Id: user.Id, + Message: "Service account created", } - return response.JSON(http.StatusCreated, result) + return response.JSON(http.StatusCreated, sa) } func (api *ServiceAccountsAPI) DeleteServiceAccount(ctx *models.ReqContext) response.Response { diff --git a/pkg/services/serviceaccounts/api/api_test.go b/pkg/services/serviceaccounts/api/api_test.go index 2338c3dd159..a8ce037b38f 100644 --- a/pkg/services/serviceaccounts/api/api_test.go +++ b/pkg/services/serviceaccounts/api/api_test.go @@ -129,7 +129,7 @@ func TestServiceAccountsAPI_RetrieveServiceAccount(t *testing.T) { user *tests.TestUser expectedCode int acmock *accesscontrolmock.Mock - userID int + Id int } testCases := []testRetrieveSATestCase{ { @@ -157,9 +157,9 @@ func TestServiceAccountsAPI_RetrieveServiceAccount(t *testing.T) { expectedCode: http.StatusForbidden, }, { - desc: "should be not found when the user doesnt exist", - user: nil, - userID: 12, + desc: "should be not found when the user doesnt exist", + user: nil, + Id: 12, acmock: tests.SetupMockAccesscontrol( t, func(c context.Context, siu *models.SignedInUser) ([]*accesscontrol.Permission, error) { @@ -182,7 +182,7 @@ func TestServiceAccountsAPI_RetrieveServiceAccount(t *testing.T) { for _, tc := range testCases { t.Run(tc.desc, func(t *testing.T) { serviceAccountRequestScenario(t, http.MethodGet, serviceaccountIDPath, tc.user, func(httpmethod string, endpoint string, user *tests.TestUser) { - scopeID := tc.userID + scopeID := tc.Id if tc.user != nil { createdUser := tests.SetupUserServiceAccount(t, store, *tc.user) scopeID = int(createdUser.Id) @@ -198,7 +198,7 @@ func TestServiceAccountsAPI_RetrieveServiceAccount(t *testing.T) { actualBody := map[string]interface{}{} err := json.Unmarshal(actual.Body.Bytes(), &actualBody) require.NoError(t, err) - require.Equal(t, scopeID, int(actualBody["userId"].(float64))) + require.Equal(t, scopeID, int(actualBody["id"].(float64))) require.Equal(t, tc.user.Login, actualBody["login"].(string)) } }) diff --git a/pkg/services/serviceaccounts/database/database.go b/pkg/services/serviceaccounts/database/database.go index 4f4b9c2ced8..4541e59e8a6 100644 --- a/pkg/services/serviceaccounts/database/database.go +++ b/pkg/services/serviceaccounts/database/database.go @@ -25,7 +25,7 @@ func NewServiceAccountsStore(store *sqlstore.SQLStore) *ServiceAccountsStoreImpl } } -func (s *ServiceAccountsStoreImpl) CreateServiceAccount(ctx context.Context, sa *serviceaccounts.CreateServiceaccountForm) (user *models.User, err error) { +func (s *ServiceAccountsStoreImpl) CreateServiceAccount(ctx context.Context, sa *serviceaccounts.CreateServiceaccountForm) (saDTO *serviceaccounts.ServiceAccountDTO, err error) { // create a new service account - "user" with empty permissions generatedLogin := "Service-Account-" + uuid.New().String() cmd := models.CreateUserCommand{ @@ -38,7 +38,13 @@ func (s *ServiceAccountsStoreImpl) CreateServiceAccount(ctx context.Context, sa if err != nil { return nil, fmt.Errorf("failed to create user: %v", err) } - return newuser, nil + return &serviceaccounts.ServiceAccountDTO{ + Id: newuser.Id, + Name: newuser.Name, + Login: newuser.Login, + OrgId: newuser.OrgId, + Tokens: 0, + }, nil } func (s *ServiceAccountsStoreImpl) DeleteServiceAccount(ctx context.Context, orgID, serviceaccountID int64) error { @@ -132,33 +138,48 @@ func (s *ServiceAccountsStoreImpl) ListTokens(ctx context.Context, orgID int64, return result, err } -func (s *ServiceAccountsStoreImpl) ListServiceAccounts(ctx context.Context, orgID, serviceAccountID int64) ([]*models.OrgUserDTO, error) { +func (s *ServiceAccountsStoreImpl) ListServiceAccounts(ctx context.Context, orgID, serviceAccountID int64) ([]*serviceaccounts.ServiceAccountDTO, error) { query := models.GetOrgUsersQuery{OrgId: orgID, IsServiceAccount: true} if serviceAccountID > 0 { query.UserID = serviceAccountID } - err := s.sqlStore.GetOrgUsers(ctx, &query) if err != nil { return nil, err } - - return query.Result, err + saDTOs := make([]*serviceaccounts.ServiceAccountDTO, len(query.Result)) + for i, user := range query.Result { + saDTOs[i] = &serviceaccounts.ServiceAccountDTO{ + Id: user.UserId, + OrgId: user.OrgId, + Name: user.Name, + Login: user.Login, + } + tokens, err := s.ListTokens(ctx, user.OrgId, user.UserId) + if err != nil { + return nil, err + } + saDTOs[i].Tokens = int64(len(tokens)) + } + return saDTOs, err } // RetrieveServiceAccountByID returns a service account by its ID -func (s *ServiceAccountsStoreImpl) RetrieveServiceAccount(ctx context.Context, orgID, serviceAccountID int64) (*models.OrgUserDTO, error) { +func (s *ServiceAccountsStoreImpl) RetrieveServiceAccount(ctx context.Context, orgID, serviceAccountID int64) (*serviceaccounts.ServiceAccountProfileDTO, error) { query := models.GetOrgUsersQuery{UserID: serviceAccountID, OrgId: orgID, IsServiceAccount: true} err := s.sqlStore.GetOrgUsers(ctx, &query) if err != nil { return nil, err } - if len(query.Result) != 1 { return nil, serviceaccounts.ErrServiceAccountNotFound } - - return query.Result[0], err + saProfile := &serviceaccounts.ServiceAccountProfileDTO{ + Id: query.Result[0].UserId, + Name: query.Result[0].Name, + Login: query.Result[0].Login, + } + return saProfile, err } func contains(s []int64, e int64) bool { diff --git a/pkg/services/serviceaccounts/manager/service.go b/pkg/services/serviceaccounts/manager/service.go index 04cc111cb03..02a8b8f0154 100644 --- a/pkg/services/serviceaccounts/manager/service.go +++ b/pkg/services/serviceaccounts/manager/service.go @@ -5,7 +5,6 @@ import ( "github.com/grafana/grafana/pkg/api/routing" "github.com/grafana/grafana/pkg/infra/log" - "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/serviceaccounts" @@ -50,7 +49,7 @@ func ProvideServiceAccountsService( return s, nil } -func (sa *ServiceAccountsService) CreateServiceAccount(ctx context.Context, saForm *serviceaccounts.CreateServiceaccountForm) (*models.User, error) { +func (sa *ServiceAccountsService) CreateServiceAccount(ctx context.Context, saForm *serviceaccounts.CreateServiceaccountForm) (*serviceaccounts.ServiceAccountDTO, error) { if !sa.features.IsEnabled(featuremgmt.FlagServiceAccounts) { sa.log.Debug(ServiceAccountFeatureToggleNotFound) return nil, nil diff --git a/pkg/services/serviceaccounts/models.go b/pkg/services/serviceaccounts/models.go index e3b8cfd48c7..550b55be00b 100644 --- a/pkg/services/serviceaccounts/models.go +++ b/pkg/services/serviceaccounts/models.go @@ -1,6 +1,10 @@ package serviceaccounts -import "github.com/grafana/grafana/pkg/services/accesscontrol" +import ( + "time" + + "github.com/grafana/grafana/pkg/services/accesscontrol" +) var ( ScopeAll = "serviceaccounts:*" @@ -22,3 +26,23 @@ type CreateServiceaccountForm struct { OrgID int64 `json:"-"` Name string `json:"name" binding:"Required"` } + +type ServiceAccountDTO struct { + Id int64 `json:"id"` + Name string `json:"name"` + Login string `json:"login"` + OrgId int64 `json:"orgId"` + Tokens int64 `json:"tokens"` +} + +type ServiceAccountProfileDTO struct { + Id int64 `json:"id"` + Name string `json:"name"` + Login string `json:"login"` + OrgId int64 `json:"orgId"` + IsDisabled bool `json:"isDisabled"` + UpdatedAt time.Time `json:"updatedAt"` + CreatedAt time.Time `json:"createdAt"` + AvatarUrl string `json:"avatarUrl"` + AccessControl map[string]bool `json:"accessControl,omitempty"` +} diff --git a/pkg/services/serviceaccounts/serviceaccounts.go b/pkg/services/serviceaccounts/serviceaccounts.go index 28d738d8b06..80b2d258e1f 100644 --- a/pkg/services/serviceaccounts/serviceaccounts.go +++ b/pkg/services/serviceaccounts/serviceaccounts.go @@ -6,15 +6,16 @@ import ( "github.com/grafana/grafana/pkg/models" ) +// this should reflect the api type Service interface { - CreateServiceAccount(ctx context.Context, saForm *CreateServiceaccountForm) (*models.User, error) + CreateServiceAccount(ctx context.Context, saForm *CreateServiceaccountForm) (*ServiceAccountDTO, error) DeleteServiceAccount(ctx context.Context, orgID, serviceAccountID int64) error } type Store interface { - CreateServiceAccount(ctx context.Context, saForm *CreateServiceaccountForm) (*models.User, error) - ListServiceAccounts(ctx context.Context, orgID, serviceAccountID int64) ([]*models.OrgUserDTO, error) - RetrieveServiceAccount(ctx context.Context, orgID, serviceAccountID int64) (*models.OrgUserDTO, error) + CreateServiceAccount(ctx context.Context, saForm *CreateServiceaccountForm) (*ServiceAccountDTO, error) + ListServiceAccounts(ctx context.Context, orgID, serviceAccountID int64) ([]*ServiceAccountDTO, error) + RetrieveServiceAccount(ctx context.Context, orgID, serviceAccountID int64) (*ServiceAccountProfileDTO, error) DeleteServiceAccount(ctx context.Context, orgID, serviceAccountID int64) error UpgradeServiceAccounts(ctx context.Context) error ConvertToServiceAccounts(ctx context.Context, keys []int64) error diff --git a/pkg/services/serviceaccounts/tests/common.go b/pkg/services/serviceaccounts/tests/common.go index 664efc53dae..ed5473ea56f 100644 --- a/pkg/services/serviceaccounts/tests/common.go +++ b/pkg/services/serviceaccounts/tests/common.go @@ -29,7 +29,7 @@ func SetupUserServiceAccount(t *testing.T, sqlStore *sqlstore.SQLStore, testUser // create mock for serviceaccountservice type ServiceAccountMock struct{} -func (s *ServiceAccountMock) CreateServiceAccount(ctx context.Context, saForm *serviceaccounts.CreateServiceaccountForm) (*models.User, error) { +func (s *ServiceAccountMock) CreateServiceAccount(ctx context.Context, saForm *serviceaccounts.CreateServiceaccountForm) (*serviceaccounts.ServiceAccountDTO, error) { return nil, nil } @@ -69,7 +69,7 @@ type ServiceAccountsStoreMock struct { Calls Calls } -func (s *ServiceAccountsStoreMock) CreateServiceAccount(ctx context.Context, cmd *serviceaccounts.CreateServiceaccountForm) (*models.User, error) { +func (s *ServiceAccountsStoreMock) CreateServiceAccount(ctx context.Context, cmd *serviceaccounts.CreateServiceaccountForm) (*serviceaccounts.ServiceAccountDTO, error) { // now we can test that the mock has these calls when we call the function s.Calls.CreateServiceAccount = append(s.Calls.CreateServiceAccount, []interface{}{ctx, cmd}) return nil, nil @@ -95,12 +95,12 @@ func (s *ServiceAccountsStoreMock) ListTokens(ctx context.Context, orgID int64, s.Calls.ListTokens = append(s.Calls.ListTokens, []interface{}{ctx, orgID, serviceAccount}) return nil, nil } -func (s *ServiceAccountsStoreMock) ListServiceAccounts(ctx context.Context, orgID int64, serviceAccountID int64) ([]*models.OrgUserDTO, error) { +func (s *ServiceAccountsStoreMock) ListServiceAccounts(ctx context.Context, orgID int64, serviceAccountID int64) ([]*serviceaccounts.ServiceAccountDTO, error) { s.Calls.ListServiceAccounts = append(s.Calls.ListServiceAccounts, []interface{}{ctx, orgID}) return nil, nil } -func (s *ServiceAccountsStoreMock) RetrieveServiceAccount(ctx context.Context, orgID, serviceAccountID int64) (*models.OrgUserDTO, error) { +func (s *ServiceAccountsStoreMock) RetrieveServiceAccount(ctx context.Context, orgID, serviceAccountID int64) (*serviceaccounts.ServiceAccountProfileDTO, error) { s.Calls.RetrieveServiceAccount = append(s.Calls.RetrieveServiceAccount, []interface{}{ctx, orgID, serviceAccountID}) return nil, nil } diff --git a/public/app/features/serviceaccounts/ServiceAccountProfile.tsx b/public/app/features/serviceaccounts/ServiceAccountProfile.tsx index 95787e11f73..a900635f678 100644 --- a/public/app/features/serviceaccounts/ServiceAccountProfile.tsx +++ b/public/app/features/serviceaccounts/ServiceAccountProfile.tsx @@ -40,11 +40,11 @@ export function ServiceAccountProfile({ } }; - const handleServiceAccountDelete = () => onServiceAccountDelete(serviceaccount.userId); + const handleServiceAccountDelete = () => onServiceAccountDelete(serviceaccount.id); - const handleServiceAccountDisable = () => onServiceAccountDisable(serviceaccount.userId); + const handleServiceAccountDisable = () => onServiceAccountDisable(serviceaccount.id); - const handleServiceAccountEnable = () => onServiceAccountEnable(serviceaccount.userId); + const handleServiceAccountEnable = () => onServiceAccountEnable(serviceaccount.id); const onServiceAccountNameChange = (newValue: string) => { onServiceAccountUpdate({ diff --git a/public/app/features/serviceaccounts/ServiceAccountsListPage.tsx b/public/app/features/serviceaccounts/ServiceAccountsListPage.tsx index 9d5d00b49cb..1e07d6ceceb 100644 --- a/public/app/features/serviceaccounts/ServiceAccountsListPage.tsx +++ b/public/app/features/serviceaccounts/ServiceAccountsListPage.tsx @@ -1,6 +1,6 @@ import React, { memo, useEffect } from 'react'; import { connect, ConnectedProps } from 'react-redux'; -import { LinkButton, useStyles2 } from '@grafana/ui'; +import { Icon, LinkButton, useStyles2 } from '@grafana/ui'; import { css, cx } from '@emotion/css'; import Page from 'app/core/components/Page/Page'; @@ -67,7 +67,7 @@ const ServiceAccountsListPage: React.FC = ({ loadServiceAccounts, navMode {serviceAccounts.map((serviceaccount: ServiceAccountDTO) => ( - + ))} @@ -88,11 +88,11 @@ const getServiceAccountsAriaLabel = (name: string) => { }; const ServiceAccountListItem = memo(({ serviceaccount }: ServiceAccountListItemProps) => { - const editUrl = `org/serviceaccounts/${serviceaccount.userId}`; + const editUrl = `org/serviceaccounts/${serviceaccount.id}`; const styles = useStyles2(getStyles); return ( - + - 0 + + + + {serviceaccount.tokens} diff --git a/public/app/features/serviceaccounts/state/actions.ts b/public/app/features/serviceaccounts/state/actions.ts index 06818300aad..04ebe917b41 100644 --- a/public/app/features/serviceaccounts/state/actions.ts +++ b/public/app/features/serviceaccounts/state/actions.ts @@ -1,14 +1,13 @@ import { ThunkResult } from '../../../types'; import { getBackendSrv } from '@grafana/runtime'; -import { ServiceAccountDTO } from 'app/types'; import { serviceAccountLoaded, serviceAccountsLoaded, serviceAccountTokensLoaded } from './reducers'; const BASE_URL = `/api/serviceaccounts`; -export function loadServiceAccount(id: number): ThunkResult { +export function loadServiceAccount(saID: number): ThunkResult { return async (dispatch) => { try { - const response = await getBackendSrv().get(`${BASE_URL}/${id}`); + const response = await getBackendSrv().get(`${BASE_URL}/${saID}`); dispatch(serviceAccountLoaded(response)); } catch (error) { console.error(error); @@ -45,10 +44,10 @@ export function loadServiceAccounts(): ThunkResult { }; } -export function updateServiceAccount(serviceAccount: ServiceAccountDTO): ThunkResult { +export function updateServiceAccount(saID: number): ThunkResult { return async (dispatch) => { // TODO: implement on backend - await getBackendSrv().patch(`${BASE_URL}/${serviceAccount.userId}`, {}); + await getBackendSrv().patch(`${BASE_URL}/${saID}`, {}); dispatch(loadServiceAccounts()); }; } diff --git a/public/app/features/serviceaccounts/state/selectors.ts b/public/app/features/serviceaccounts/state/selectors.ts index 2efdbc5bad8..25781a219fd 100644 --- a/public/app/features/serviceaccounts/state/selectors.ts +++ b/public/app/features/serviceaccounts/state/selectors.ts @@ -4,7 +4,7 @@ export const getServiceAccounts = (state: ServiceAccountsState) => { const regex = new RegExp(state.searchQuery, 'i'); return state.serviceAccounts.filter((serviceaccount) => { - return regex.test(serviceaccount.login) || regex.test(serviceaccount.email) || regex.test(serviceaccount.name); + return regex.test(serviceaccount.name) || regex.test(serviceaccount.login); }); }; diff --git a/public/app/types/serviceaccount.ts b/public/app/types/serviceaccount.ts index ef7e9a51538..dfb54931e88 100644 --- a/public/app/types/serviceaccount.ts +++ b/public/app/types/serviceaccount.ts @@ -25,15 +25,13 @@ export interface ServiceAccount { } export interface ServiceAccountDTO extends WithAccessControlMetadata { + id: number; orgId: number; - userId: number; - email: string; + tokens: number; name: string; - avatarUrl?: string; login: string; + avatarUrl?: string; role: string; - lastSeenAt: string; - lastSeenAtAge: string; } export interface ServiceAccountProfileState {