Service Accounts: Link final components in service accounts detail page (#45929)

* ServiceAccounts: Delete/Disable service account from details page

* ServiceAccounts: capitalize viewable messages from UI

* ServiceAccounts: Link new update endpoint to details page

* ServiceAccounts: reimplement service account retrieve to include is_disabled and only target service accounts

* Cleanup styles

* Fix modal show

* ServiceAccounts: simplify handler functions

* Apply suggestions from code review

Co-authored-by: Alex Khomenko <Clarity-89@users.noreply.github.com>

Co-authored-by: Clarity-89 <homes89@ukr.net>
Co-authored-by: Alex Khomenko <Clarity-89@users.noreply.github.com>
This commit is contained in:
J Guerreiro 2022-03-01 08:21:55 +00:00 committed by GitHub
parent b491d6b4dc
commit bc5237e840
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 160 additions and 126 deletions

View File

@ -109,12 +109,12 @@ func (api *ServiceAccountsAPI) DeleteServiceAccount(ctx *models.ReqContext) resp
if err != nil {
return response.Error(http.StatusInternalServerError, "Service account deletion error", err)
}
return response.Success("service account deleted")
return response.Success("Service account deleted")
}
func (api *ServiceAccountsAPI) UpgradeServiceAccounts(ctx *models.ReqContext) response.Response {
if err := api.store.UpgradeServiceAccounts(ctx.Req.Context()); err == nil {
return response.Success("service accounts upgraded")
return response.Success("Service accounts upgraded")
} else {
return response.Error(http.StatusInternalServerError, "Internal server error", err)
}
@ -123,10 +123,10 @@ func (api *ServiceAccountsAPI) UpgradeServiceAccounts(ctx *models.ReqContext) re
func (api *ServiceAccountsAPI) ConvertToServiceAccount(ctx *models.ReqContext) response.Response {
keyId, err := strconv.ParseInt(web.Params(ctx.Req)[":keyId"], 10, 64)
if err != nil {
return response.Error(http.StatusBadRequest, "keyId is invalid", err)
return response.Error(http.StatusBadRequest, "Key ID is invalid", err)
}
if err := api.store.ConvertToServiceAccounts(ctx.Req.Context(), []int64{keyId}); err == nil {
return response.Success("service accounts converted")
return response.Success("Service accounts converted")
} else {
return response.Error(500, "Internal server error", err)
}
@ -174,7 +174,7 @@ func (api *ServiceAccountsAPI) getAccessControlMetadata(c *models.ReqContext, sa
func (api *ServiceAccountsAPI) RetrieveServiceAccount(ctx *models.ReqContext) response.Response {
scopeID, err := strconv.ParseInt(web.Params(ctx.Req)[":serviceAccountId"], 10, 64)
if err != nil {
return response.Error(http.StatusBadRequest, "serviceAccountId is invalid", err)
return response.Error(http.StatusBadRequest, "Service Account ID is invalid", err)
}
serviceAccount, err := api.store.RetrieveServiceAccount(ctx.Req.Context(), ctx.OrgId, scopeID)
@ -197,12 +197,12 @@ func (api *ServiceAccountsAPI) RetrieveServiceAccount(ctx *models.ReqContext) re
func (api *ServiceAccountsAPI) updateServiceAccount(c *models.ReqContext) response.Response {
scopeID, err := strconv.ParseInt(web.Params(c.Req)[":serviceAccountId"], 10, 64)
if err != nil {
return response.Error(http.StatusBadRequest, "serviceAccountId is invalid", err)
return response.Error(http.StatusBadRequest, "Service Account ID is invalid", err)
}
cmd := &serviceaccounts.UpdateServiceAccountForm{}
if err := web.Bind(c.Req, &cmd); err != nil {
return response.Error(http.StatusBadRequest, "bad request data", err)
return response.Error(http.StatusBadRequest, "Bad request data", err)
}
if cmd.Role != nil && !cmd.Role.IsValid() {

View File

@ -39,7 +39,7 @@ const sevenDaysAhead = 7 * 24 * time.Hour
func (api *ServiceAccountsAPI) ListTokens(ctx *models.ReqContext) response.Response {
saID, err := strconv.ParseInt(web.Params(ctx.Req)[":serviceAccountId"], 10, 64)
if err != nil {
return response.Error(http.StatusBadRequest, "serviceAccountId is invalid", err)
return response.Error(http.StatusBadRequest, "Service Account ID is invalid", err)
}
if saTokens, err := api.store.ListTokens(ctx.Req.Context(), ctx.OrgId, saID); err == nil {
@ -78,7 +78,7 @@ func (api *ServiceAccountsAPI) ListTokens(ctx *models.ReqContext) response.Respo
func (api *ServiceAccountsAPI) CreateToken(c *models.ReqContext) response.Response {
saID, err := strconv.ParseInt(web.Params(c.Req)[":serviceAccountId"], 10, 64)
if err != nil {
return response.Error(http.StatusBadRequest, "serviceAccountId is invalid", err)
return response.Error(http.StatusBadRequest, "Service Account ID is invalid", err)
}
// confirm service account exists
@ -93,7 +93,7 @@ func (api *ServiceAccountsAPI) CreateToken(c *models.ReqContext) response.Respon
cmd := models.AddApiKeyCommand{}
if err := web.Bind(c.Req, &cmd); err != nil {
return response.Error(http.StatusBadRequest, "bad request data", err)
return response.Error(http.StatusBadRequest, "Bad request data", err)
}
// Force affected service account to be the one referenced in the URL

View File

@ -4,6 +4,7 @@ package database
import (
"context"
"fmt"
"strings"
"time"
"github.com/google/uuid"
@ -169,14 +170,53 @@ func (s *ServiceAccountsStoreImpl) ListServiceAccounts(ctx context.Context, orgI
// RetrieveServiceAccountByID returns a service account by its ID
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)
serviceAccount := &serviceaccounts.ServiceAccountProfileDTO{}
err := s.sqlStore.WithDbSession(ctx, func(dbSession *sqlstore.DBSession) error {
sess := dbSession.Table("org_user")
sess.Join("INNER", s.sqlStore.Dialect.Quote("user"),
fmt.Sprintf("org_user.user_id=%s.id", s.sqlStore.Dialect.Quote("user")))
whereConditions := make([]string, 0, 3)
whereParams := make([]interface{}, 0)
whereConditions = append(whereConditions, "org_user.org_id = ?")
whereParams = append(whereParams, orgID)
whereConditions = append(whereConditions, "org_user.user_id = ?")
whereParams = append(whereParams, serviceAccountID)
whereConditions = append(whereConditions,
fmt.Sprintf("%s.is_service_account = %s",
s.sqlStore.Dialect.Quote("user"),
s.sqlStore.Dialect.BooleanStr(true)))
sess.Where(strings.Join(whereConditions, " AND "), whereParams...)
sess.Cols(
"org_user.user_id",
"org_user.org_id",
"org_user.role",
"user.email",
"user.name",
"user.login",
"user.created",
"user.updated",
"user.is_disabled",
)
if ok, err := sess.Get(serviceAccount); err != nil {
return err
} else if !ok {
return serviceaccounts.ErrServiceAccountNotFound
}
return nil
})
if err != nil {
return nil, err
}
if len(query.Result) != 1 {
return nil, serviceaccounts.ErrServiceAccountNotFound
}
// Get Teams of service account. Can be optimized by combining with the query above
// in refactor
@ -190,36 +230,24 @@ func (s *ServiceAccountsStoreImpl) RetrieveServiceAccount(ctx context.Context, o
teams[i] = getTeamQuery.Result[i].Name
}
saProfile := &serviceaccounts.ServiceAccountProfileDTO{
Id: query.Result[0].UserId,
Name: query.Result[0].Name,
Login: query.Result[0].Login,
OrgId: query.Result[0].OrgId,
UpdatedAt: query.Result[0].Updated,
CreatedAt: query.Result[0].Created,
Role: query.Result[0].Role,
Teams: teams,
}
return saProfile, nil
serviceAccount.Teams = teams
return serviceAccount, nil
}
func (s *ServiceAccountsStoreImpl) UpdateServiceAccount(ctx context.Context,
orgID, serviceAccountID int64,
saForm *serviceaccounts.UpdateServiceAccountForm) (*serviceaccounts.ServiceAccountDTO, error) {
updatedUser := &models.OrgUserDTO{}
saForm *serviceaccounts.UpdateServiceAccountForm) (*serviceaccounts.ServiceAccountProfileDTO, error) {
updatedUser := &serviceaccounts.ServiceAccountProfileDTO{}
err := s.sqlStore.WithTransactionalDbSession(ctx, func(sess *sqlstore.DBSession) error {
query := models.GetOrgUsersQuery{UserID: serviceAccountID, OrgId: orgID, IsServiceAccount: true}
if err := s.sqlStore.GetOrgUsers(ctx, &query); err != nil {
var err error
updatedUser, err = s.RetrieveServiceAccount(ctx, orgID, serviceAccountID)
if err != nil {
return err
}
if len(query.Result) != 1 {
return serviceaccounts.ErrServiceAccountNotFound
}
updatedUser = query.Result[0]
if saForm.Name == nil && saForm.Role == nil {
if saForm.Name == nil && saForm.Role == nil && saForm.IsDisabled == nil {
return nil
}
@ -236,29 +264,31 @@ func (s *ServiceAccountsStoreImpl) UpdateServiceAccount(ctx context.Context,
updatedUser.Role = string(*saForm.Role)
}
if saForm.Name != nil {
if saForm.Name != nil || saForm.IsDisabled != nil {
user := models.User{
Name: *saForm.Name,
Updated: updateTime,
}
if saForm.IsDisabled != nil {
user.IsDisabled = *saForm.IsDisabled
updatedUser.IsDisabled = *saForm.IsDisabled
sess.UseBool("is_disabled")
}
if saForm.Name != nil {
user.Name = *saForm.Name
updatedUser.Name = *saForm.Name
}
if _, err := sess.ID(serviceAccountID).Update(&user); err != nil {
return err
}
updatedUser.Name = *saForm.Name
}
return nil
})
return &serviceaccounts.ServiceAccountDTO{
Id: updatedUser.UserId,
Name: updatedUser.Name,
Login: updatedUser.Login,
Role: updatedUser.Role,
OrgId: updatedUser.OrgId,
}, err
return updatedUser, err
}
func contains(s []int64, e int64) bool {

View File

@ -24,8 +24,9 @@ type ServiceAccount struct {
}
type UpdateServiceAccountForm struct {
Name *string `json:"name"`
Role *models.RoleType `json:"role"`
Name *string `json:"name"`
Role *models.RoleType `json:"role"`
IsDisabled *bool `json:"isDisabled"`
}
type CreateServiceAccountForm struct {
@ -45,15 +46,15 @@ type ServiceAccountDTO struct {
}
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"`
Role string `json:"role"`
Teams []string `json:"teams"`
AccessControl map[string]bool `json:"accessControl,omitempty"`
Id int64 `json:"id" xorm:"user_id"`
Name string `json:"name" xorm:"name"`
Login string `json:"login" xorm:"login"`
OrgId int64 `json:"orgId" xorm:"org_id"`
IsDisabled bool `json:"isDisabled" xorm:"is_disabled"`
Created time.Time `json:"createdAt" xorm:"created"`
Updated time.Time `json:"updatedAt" xorm:"updated"`
AvatarUrl string `json:"avatarUrl" xorm:"-"`
Role string `json:"role" xorm:"role"`
Teams []string `json:"teams" xorm:"-"`
AccessControl map[string]bool `json:"accessControl,omitempty" xorm:"-"`
}

View File

@ -15,7 +15,7 @@ type Service interface {
type Store interface {
CreateServiceAccount(ctx context.Context, saForm *CreateServiceAccountForm) (*ServiceAccountDTO, error)
ListServiceAccounts(ctx context.Context, orgID, serviceAccountID int64) ([]*ServiceAccountDTO, error)
UpdateServiceAccount(ctx context.Context, orgID, serviceAccountID int64, saForm *UpdateServiceAccountForm) (*ServiceAccountDTO, error)
UpdateServiceAccount(ctx context.Context, orgID, serviceAccountID int64, saForm *UpdateServiceAccountForm) (*ServiceAccountProfileDTO, error)
RetrieveServiceAccount(ctx context.Context, orgID, serviceAccountID int64) (*ServiceAccountProfileDTO, error)
DeleteServiceAccount(ctx context.Context, orgID, serviceAccountID int64) error
UpgradeServiceAccounts(ctx context.Context) error

View File

@ -121,7 +121,7 @@ func (s *ServiceAccountsStoreMock) RetrieveServiceAccount(ctx context.Context, o
func (s *ServiceAccountsStoreMock) UpdateServiceAccount(ctx context.Context,
orgID, serviceAccountID int64,
saForm *serviceaccounts.UpdateServiceAccountForm) (*serviceaccounts.ServiceAccountDTO, error) {
saForm *serviceaccounts.UpdateServiceAccountForm) (*serviceaccounts.ServiceAccountProfileDTO, error) {
s.Calls.UpdateServiceAccount = append(s.Calls.UpdateServiceAccount, []interface{}{ctx, orgID, serviceAccountID, saForm})
return nil, nil

View File

@ -12,9 +12,10 @@ import {
createServiceAccountToken,
fetchACOptions,
updateServiceAccount,
deleteServiceAccount,
} from './state/actions';
import { ServiceAccountTokensTable } from './ServiceAccountTokensTable';
import { getTimeZone, NavModel, OrgRole } from '@grafana/data';
import { getTimeZone, NavModel } from '@grafana/data';
import { Button, VerticalGroup } from '@grafana/ui';
import { CreateTokenModal } from './CreateTokenModal';
import { contextSrv } from 'app/core/core';
@ -44,6 +45,8 @@ const mapDispatchToProps = {
loadServiceAccountTokens,
createServiceAccountToken,
deleteServiceAccountToken,
deleteServiceAccount,
updateServiceAccount,
fetchACOptions,
};
@ -63,6 +66,8 @@ const ServiceAccountPageUnconnected = ({
loadServiceAccountTokens,
createServiceAccountToken,
deleteServiceAccountToken,
deleteServiceAccount,
updateServiceAccount,
fetchACOptions,
}: Props) => {
const [isModalOpen, setIsModalOpen] = useState(false);
@ -90,12 +95,6 @@ const ServiceAccountPageUnconnected = ({
setNewToken('');
};
const onRoleChange = (role: OrgRole, serviceAccount: ServiceAccountDTO) => {
const updatedServiceAccount = { ...serviceAccount, role: role };
updateServiceAccount(updatedServiceAccount);
};
return (
<Page navModel={navModel}>
<Page.Contents isLoading={isLoading}>
@ -104,21 +103,10 @@ const ServiceAccountPageUnconnected = ({
<ServiceAccountProfile
serviceAccount={serviceAccount}
timeZone={timezone}
onServiceAccountDelete={() => {
console.log(`not implemented`);
}}
onServiceAccountUpdate={() => {
console.log(`not implemented`);
}}
onServiceAccountDisable={() => {
console.log(`not implemented`);
}}
onServiceAccountEnable={() => {
console.log(`not implemented`);
}}
onRoleChange={onRoleChange}
roleOptions={roleOptions}
builtInRoles={builtInRoles}
updateServiceAccount={updateServiceAccount}
deleteServiceAccount={deleteServiceAccount}
/>
</>
)}

View File

@ -1,35 +1,27 @@
import React, { PureComponent, useRef, useState } from 'react';
import { Role, ServiceAccountDTO } from 'app/types';
import { css, cx } from '@emotion/css';
import { config } from 'app/core/config';
import { dateTimeFormat, GrafanaTheme, OrgRole, TimeZone } from '@grafana/data';
import { Button, ConfirmButton, ConfirmModal, Input, LegacyInputStatus, stylesFactory } from '@grafana/ui';
import { dateTimeFormat, GrafanaTheme2, OrgRole, TimeZone } from '@grafana/data';
import { Button, ConfirmButton, ConfirmModal, Input, LegacyInputStatus, useStyles2 } from '@grafana/ui';
import { ServiceAccountRoleRow } from './ServiceAccountRoleRow';
interface Props {
serviceAccount: ServiceAccountDTO;
timeZone: TimeZone;
onServiceAccountUpdate: (serviceAccount: ServiceAccountDTO) => void;
onServiceAccountDelete: (serviceAccountId: number) => void;
onServiceAccountDisable: (serviceAccountId: number) => void;
onServiceAccountEnable: (serviceAccountId: number) => void;
onRoleChange: (role: OrgRole, serviceAccount: ServiceAccountDTO) => void;
roleOptions: Role[];
builtInRoles: Record<string, Role[]>;
deleteServiceAccount: (serviceAccountId: number) => void;
updateServiceAccount: (serviceAccount: ServiceAccountDTO) => void;
}
export function ServiceAccountProfile({
serviceAccount,
timeZone,
onServiceAccountUpdate,
onServiceAccountDelete,
onServiceAccountDisable,
onServiceAccountEnable,
onRoleChange,
roleOptions,
builtInRoles,
deleteServiceAccount,
updateServiceAccount,
}: Props) {
const [showDeleteModal, setShowDeleteModal] = useState(false);
const [showDisableModal, setShowDisableModal] = useState(false);
@ -50,20 +42,27 @@ export function ServiceAccountProfile({
}
};
const handleServiceAccountDelete = () => onServiceAccountDelete(serviceAccount.id);
const handleServiceAccountDisable = () => onServiceAccountDisable(serviceAccount.id);
const handleServiceAccountEnable = () => onServiceAccountEnable(serviceAccount.id);
const onServiceAccountNameChange = (newValue: string) => {
onServiceAccountUpdate({
...serviceAccount,
name: newValue,
});
const handleServiceAccountDelete = () => {
deleteServiceAccount(serviceAccount.id);
};
const handleServiceAccountDisable = () => {
updateServiceAccount({ ...serviceAccount, isDisabled: true });
setShowDisableModal(false);
};
const styles = getStyles(config.theme);
const handleServiceAccountEnable = () => {
updateServiceAccount({ ...serviceAccount, isDisabled: false });
};
const handleServiceAccountRoleChange = (role: OrgRole) => {
updateServiceAccount({ ...serviceAccount, role: role });
};
const onServiceAccountNameChange = (newValue: string) => {
updateServiceAccount({ ...serviceAccount, name: newValue });
};
const styles = useStyles2(getStyles);
return (
<>
@ -84,7 +83,7 @@ export function ServiceAccountProfile({
<ServiceAccountRoleRow
label="Roles"
serviceAccount={serviceAccount}
onRoleChange={onRoleChange}
onRoleChange={handleServiceAccountRoleChange}
builtInRoles={builtInRoles}
roleOptions={roleOptions}
/>
@ -98,26 +97,35 @@ export function ServiceAccountProfile({
</div>
<div className={styles.buttonRow}>
<>
<Button variant="destructive" onClick={showDeleteServiceAccountModal(true)} ref={deleteServiceAccountRef}>
<Button
type={'button'}
variant="destructive"
onClick={showDeleteServiceAccountModal(true)}
ref={deleteServiceAccountRef}
>
Delete service account
</Button>
<ConfirmModal
isOpen={showDeleteModal}
title="Delete serviceaccount"
title="Delete service account"
body="Are you sure you want to delete this service account?"
confirmText="Delete service account"
onConfirm={handleServiceAccountDelete}
onDismiss={showDeleteServiceAccountModal(false)}
/>
</>
{serviceAccount.isDisabled && (
<Button variant="secondary" onClick={handleServiceAccountEnable}>
{serviceAccount.isDisabled ? (
<Button type={'button'} variant="secondary" onClick={handleServiceAccountEnable}>
Enable service account
</Button>
)}
{!serviceAccount.isDisabled && (
) : (
<>
<Button variant="secondary" onClick={showDisableServiceAccountModal(true)} ref={disableServiceAccountRef}>
<Button
type={'button'}
variant="secondary"
onClick={showDisableServiceAccountModal(true)}
ref={disableServiceAccountRef}
>
Disable service account
</Button>
<ConfirmModal
@ -136,16 +144,16 @@ export function ServiceAccountProfile({
);
}
const getStyles = stylesFactory((theme: GrafanaTheme) => {
const getStyles = (theme: GrafanaTheme2) => {
return {
buttonRow: css`
margin-top: 0.8rem;
margin-top: ${theme.spacing(1.5)};
> * {
margin-right: 16px;
margin-right: ${theme.spacing(2)};
}
`,
};
});
};
interface ServiceAccountProfileRowProps {
label: string;

View File

@ -8,7 +8,7 @@ import { UserRolePicker } from 'app/core/components/RolePicker/UserRolePicker';
interface Props {
label: string;
serviceAccount: ServiceAccountDTO;
onRoleChange: (role: OrgRole, serviceAccount: ServiceAccountDTO) => void;
onRoleChange: (role: OrgRole) => void;
roleOptions: Role[];
builtInRoles: Record<string, Role[]>;
}
@ -37,7 +37,7 @@ export class ServiceAccountRoleRow extends PureComponent<Props> {
userId={serviceAccount.id}
orgId={serviceAccount.orgId}
builtInRole={serviceAccount.role}
onBuiltinRoleChange={(newRole) => onRoleChange(newRole, serviceAccount)}
onBuiltinRoleChange={onRoleChange}
roleOptions={roleOptions}
builtInRoles={builtInRoles}
disabled={rolePickerDisabled}
@ -47,7 +47,7 @@ export class ServiceAccountRoleRow extends PureComponent<Props> {
aria-label="Role"
value={serviceAccount.role}
disabled={!canUpdateRole}
onChange={(newRole) => onRoleChange(newRole, serviceAccount)}
onChange={onRoleChange}
/>
)}
</td>

View File

@ -1,5 +1,5 @@
import { ApiKey, ServiceAccountDTO, ThunkResult } from '../../../types';
import { getBackendSrv } from '@grafana/runtime';
import { getBackendSrv, locationService } from '@grafana/runtime';
import {
acOptionsLoaded,
builtInRolesLoaded,
@ -90,8 +90,8 @@ export function loadServiceAccounts(): ThunkResult<void> {
export function updateServiceAccount(serviceAccount: ServiceAccountDTO): ThunkResult<void> {
return async (dispatch) => {
await getBackendSrv().patch(`/api/org/users/${serviceAccount.id}`, { role: serviceAccount.role });
dispatch(loadServiceAccounts());
const response = await getBackendSrv().patch(`${BASE_URL}/${serviceAccount.id}`, { ...serviceAccount });
dispatch(serviceAccountLoaded(response));
};
}
@ -101,3 +101,10 @@ export function removeServiceAccount(serviceAccountId: number): ThunkResult<void
dispatch(loadServiceAccounts());
};
}
export function deleteServiceAccount(serviceAccountId: number): ThunkResult<void> {
return async (dispatch) => {
await getBackendSrv().delete(`${BASE_URL}/${serviceAccountId}`);
locationService.push('/org/serviceaccounts');
};
}