diff --git a/pkg/api/api.go b/pkg/api/api.go index 99a8ed631e2..560cd323037 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -278,7 +278,7 @@ func (hs *HTTPServer) registerRoutes() { // auth api keys apiRoute.Group("/auth/keys", func(keysRoute routing.RouteRegister) { apikeyIDScope := ac.Scope("apikeys", "id", ac.Parameter(":id")) - keysRoute.Get("/", authorize(reqOrgAdmin, ac.EvalPermission(ac.ActionAPIKeyRead, ac.ScopeAPIKeysAll)), routing.Wrap(hs.GetAPIKeys)) + keysRoute.Get("/", authorize(reqOrgAdmin, ac.EvalPermission(ac.ActionAPIKeyRead)), routing.Wrap(hs.GetAPIKeys)) keysRoute.Post("/", authorize(reqOrgAdmin, ac.EvalPermission(ac.ActionAPIKeyCreate)), quota("api_key"), routing.Wrap(hs.AddAPIKey)) keysRoute.Delete("/:id", authorize(reqOrgAdmin, ac.EvalPermission(ac.ActionAPIKeyDelete, apikeyIDScope)), routing.Wrap(hs.DeleteAPIKey)) }) diff --git a/pkg/api/apikey.go b/pkg/api/apikey.go index 9d1aab24bf3..c1347eef115 100644 --- a/pkg/api/apikey.go +++ b/pkg/api/apikey.go @@ -15,20 +15,22 @@ import ( // GetAPIKeys returns a list of API keys func (hs *HTTPServer) GetAPIKeys(c *models.ReqContext) response.Response { - query := models.GetApiKeysQuery{OrgId: c.OrgId, IncludeExpired: c.QueryBool("includeExpired")} + query := models.GetApiKeysQuery{OrgId: c.OrgId, User: c.SignedInUser, IncludeExpired: c.QueryBool("includeExpired")} if err := hs.SQLStore.GetAPIKeys(c.Req.Context(), &query); err != nil { return response.Error(500, "Failed to list api keys", err) } - result := make([]*models.ApiKeyDTO, len(query.Result)) + ids := map[string]bool{} + result := make([]*dtos.ApiKeyDTO, len(query.Result)) for i, t := range query.Result { + ids[strconv.FormatInt(t.Id, 10)] = true var expiration *time.Time = nil if t.Expires != nil { v := time.Unix(*t.Expires, 0) expiration = &v } - result[i] = &models.ApiKeyDTO{ + result[i] = &dtos.ApiKeyDTO{ Id: t.Id, Name: t.Name, Role: t.Role, @@ -36,6 +38,13 @@ func (hs *HTTPServer) GetAPIKeys(c *models.ReqContext) response.Response { } } + metadata := hs.getMultiAccessControlMetadata(c, c.OrgId, "apikeys:id", ids) + if len(metadata) > 0 { + for _, key := range result { + key.AccessControl = metadata[strconv.FormatInt(key.Id, 10)] + } + } + return response.JSON(http.StatusOK, result) } diff --git a/pkg/api/docs/definitions/apikey.go b/pkg/api/docs/definitions/apikey.go index f8ff0cbc64c..3121a616680 100644 --- a/pkg/api/docs/definitions/apikey.go +++ b/pkg/api/docs/definitions/apikey.go @@ -70,7 +70,7 @@ type DeleteAPIkeyParams struct { type GetAPIkeyResponse struct { // The response message // in: body - Body []*models.ApiKeyDTO `json:"body"` + Body []*dtos.ApiKeyDTO `json:"body"` } // swagger:response postAPIkeyResponse diff --git a/pkg/api/dtos/apikey.go b/pkg/api/dtos/apikey.go index 6d42be65040..d96a5201679 100644 --- a/pkg/api/dtos/apikey.go +++ b/pkg/api/dtos/apikey.go @@ -1,7 +1,22 @@ package dtos +import ( + "time" + + "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/accesscontrol" +) + type NewApiKeyResult struct { ID int64 `json:"id"` Name string `json:"name"` Key string `json:"key"` } + +type ApiKeyDTO struct { + Id int64 `json:"id"` + Name string `json:"name"` + Role models.RoleType `json:"role"` + Expiration *time.Time `json:"expiration,omitempty"` + AccessControl accesscontrol.Metadata `json:"accessControl,omitempty"` +} diff --git a/pkg/models/apikey.go b/pkg/models/apikey.go index aabbc4e78b4..9085189b777 100644 --- a/pkg/models/apikey.go +++ b/pkg/models/apikey.go @@ -47,6 +47,7 @@ type DeleteApiKeyCommand struct { type GetApiKeysQuery struct { OrgId int64 IncludeExpired bool + User *SignedInUser Result []*ApiKey } @@ -60,13 +61,3 @@ type GetApiKeyByIdQuery struct { ApiKeyId int64 Result *ApiKey } - -// ------------------------ -// DTO & Projections - -type ApiKeyDTO struct { - Id int64 `json:"id"` - Name string `json:"name"` - Role RoleType `json:"role"` - Expiration *time.Time `json:"expiration,omitempty"` -} diff --git a/pkg/services/accesscontrol/filter.go b/pkg/services/accesscontrol/filter.go index 67eb5c49eaa..4980522d15f 100644 --- a/pkg/services/accesscontrol/filter.go +++ b/pkg/services/accesscontrol/filter.go @@ -9,6 +9,7 @@ import ( ) var sqlIDAcceptList = map[string]struct{}{ + "id": {}, "org_user.user_id": {}, "role.id": {}, "t.id": {}, diff --git a/pkg/services/sqlstore/apikey.go b/pkg/services/sqlstore/apikey.go index 876d41382d2..a06bc3e3c61 100644 --- a/pkg/services/sqlstore/apikey.go +++ b/pkg/services/sqlstore/apikey.go @@ -7,6 +7,8 @@ import ( "xorm.io/xorm" "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/featuremgmt" ) // GetAPIKeys queries the database based @@ -27,6 +29,14 @@ func (ss *SQLStore) GetAPIKeys(ctx context.Context, query *models.GetApiKeysQuer sess = sess.Where("service_account_id IS NULL") + if ss.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagAccesscontrol) { + filter, err := accesscontrol.Filter(query.User, "id", "apikeys:id:", accesscontrol.ActionAPIKeyRead) + if err != nil { + return err + } + sess.And(filter.Where, filter.Args...) + } + query.Result = make([]*models.ApiKey, 0) return sess.Find(&query.Result) }) diff --git a/pkg/services/sqlstore/apikey_test.go b/pkg/services/sqlstore/apikey_test.go index 06a17378f2c..2aa913fe01a 100644 --- a/pkg/services/sqlstore/apikey_test.go +++ b/pkg/services/sqlstore/apikey_test.go @@ -5,10 +5,14 @@ package sqlstore import ( "context" + "fmt" "testing" "time" + "github.com/stretchr/testify/require" + "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/stretchr/testify/assert" ) @@ -149,3 +153,60 @@ func TestApiKeyErrors(t *testing.T) { }) }) } + +type getApiKeysTestCase struct { + desc string + user *models.SignedInUser + expectedNumKeys int +} + +func TestSQLStore_GetAPIKeys(t *testing.T) { + tests := []getApiKeysTestCase{ + { + desc: "expect all keys for wildcard scope", + user: &models.SignedInUser{OrgId: 1, Permissions: map[int64]map[string][]string{ + 1: {"apikeys:read": {"apikeys:*"}}, + }}, + expectedNumKeys: 10, + }, + { + desc: "expect only api keys that user have scopes for", + user: &models.SignedInUser{OrgId: 1, Permissions: map[int64]map[string][]string{ + 1: {"apikeys:read": {"apikeys:id:1", "apikeys:id:3"}}, + }}, + expectedNumKeys: 2, + }, + { + desc: "expect no keys when user have no scopes", + user: &models.SignedInUser{OrgId: 1, Permissions: map[int64]map[string][]string{ + 1: {"apikeys:read": {}}, + }}, + expectedNumKeys: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + store := InitTestDB(t, InitTestDBOpt{FeatureFlags: []string{featuremgmt.FlagAccesscontrol}}) + seedApiKeys(t, store, 10) + + query := &models.GetApiKeysQuery{OrgId: 1, User: tt.user} + err := store.GetAPIKeys(context.Background(), query) + require.NoError(t, err) + assert.Len(t, query.Result, tt.expectedNumKeys) + }) + } +} + +func seedApiKeys(t *testing.T, store *SQLStore, num int) { + t.Helper() + + for i := 0; i < num; i++ { + err := store.AddAPIKey(context.Background(), &models.AddApiKeyCommand{ + Name: fmt.Sprintf("key:%d", i), + Key: fmt.Sprintf("key:%d", i), + OrgId: 1, + }) + require.NoError(t, err) + } +} diff --git a/public/app/features/api-keys/ApiKeysPage.test.tsx b/public/app/features/api-keys/ApiKeysPage.test.tsx index e4869316130..33e2142283d 100644 --- a/public/app/features/api-keys/ApiKeysPage.test.tsx +++ b/public/app/features/api-keys/ApiKeysPage.test.tsx @@ -13,6 +13,15 @@ import { ApiKeysPageUnconnected, Props } from './ApiKeysPage'; import { getMultipleMockKeys } from './__mocks__/apiKeysMock'; import { setSearchQuery } from './state/reducers'; +jest.mock('app/core/core', () => { + return { + contextSrv: { + hasPermission: () => true, + hasPermissionInMetadata: () => true, + }, + }; +}); + const setup = (propOverrides: Partial) => { const loadApiKeysMock = jest.fn(); const deleteApiKeyMock = jest.fn(); @@ -40,9 +49,7 @@ const setup = (propOverrides: Partial) => { includeExpired: false, includeExpiredDisabled: false, toggleIncludeExpired: toggleIncludeExpiredMock, - canRead: true, canCreate: true, - canDelete: true, }; Object.assign(props, propOverrides); diff --git a/public/app/features/api-keys/ApiKeysPage.tsx b/public/app/features/api-keys/ApiKeysPage.tsx index 4c95ac5db98..fb24c8ceef4 100644 --- a/public/app/features/api-keys/ApiKeysPage.tsx +++ b/public/app/features/api-keys/ApiKeysPage.tsx @@ -24,9 +24,7 @@ import { setSearchQuery } from './state/reducers'; import { getApiKeys, getApiKeysCount, getIncludeExpired, getIncludeExpiredDisabled } from './state/selectors'; function mapStateToProps(state: StoreState) { - const canRead = contextSrv.hasAccess(AccessControlAction.ActionAPIKeysRead, true); const canCreate = contextSrv.hasAccess(AccessControlAction.ActionAPIKeysCreate, true); - const canDelete = contextSrv.hasAccess(AccessControlAction.ActionAPIKeysDelete, true); return { navModel: getNavModel(state.navIndex, 'apikeys'), @@ -37,9 +35,7 @@ function mapStateToProps(state: StoreState) { timeZone: getTimeZone(state.user), includeExpired: getIncludeExpired(state.apiKeys), includeExpiredDisabled: getIncludeExpiredDisabled(state.apiKeys), - canRead: canRead, canCreate: canCreate, - canDelete: canDelete, }; } @@ -130,9 +126,7 @@ export class ApiKeysPageUnconnected extends PureComponent { timeZone, includeExpired, includeExpiredDisabled, - canRead, canCreate, - canDelete, } = this.props; if (!hasFetched) { @@ -181,13 +175,7 @@ export class ApiKeysPageUnconnected extends PureComponent { - + ) : null} diff --git a/public/app/features/api-keys/ApiKeysTable.tsx b/public/app/features/api-keys/ApiKeysTable.tsx index b04f1e82a62..15502aea0a6 100644 --- a/public/app/features/api-keys/ApiKeysTable.tsx +++ b/public/app/features/api-keys/ApiKeysTable.tsx @@ -3,6 +3,8 @@ import React, { FC } from 'react'; import { dateTimeFormat, GrafanaTheme2, TimeZone } from '@grafana/data'; import { DeleteButton, Icon, IconName, Tooltip, useTheme2 } from '@grafana/ui'; +import { contextSrv } from 'app/core/core'; +import { AccessControlAction } from 'app/types'; import { ApiKey } from '../../types'; @@ -10,11 +12,9 @@ interface Props { apiKeys: ApiKey[]; timeZone: TimeZone; onDelete: (apiKey: ApiKey) => void; - canRead: boolean; - canDelete: boolean; } -export const ApiKeysTable: FC = ({ apiKeys, timeZone, onDelete, canRead, canDelete }) => { +export const ApiKeysTable: FC = ({ apiKeys, timeZone, onDelete }) => { const theme = useTheme2(); const styles = getStyles(theme); @@ -28,7 +28,7 @@ export const ApiKeysTable: FC = ({ apiKeys, timeZone, onDelete, canRead, - {canRead && apiKeys.length > 0 ? ( + {apiKeys.length > 0 ? ( {apiKeys.map((key) => { const isExpired = Boolean(key.expiration && Date.now() > new Date(key.expiration).getTime()); @@ -51,7 +51,7 @@ export const ApiKeysTable: FC = ({ apiKeys, timeZone, onDelete, canRead, aria-label="Delete API key" size="sm" onConfirm={() => onDelete(key)} - disabled={!canDelete} + disabled={!contextSrv.hasPermissionInMetadata(AccessControlAction.ActionAPIKeysDelete, key)} /> diff --git a/public/app/features/api-keys/state/actions.ts b/public/app/features/api-keys/state/actions.ts index 467f45c6f77..1abb5355ab4 100644 --- a/public/app/features/api-keys/state/actions.ts +++ b/public/app/features/api-keys/state/actions.ts @@ -16,8 +16,8 @@ export function loadApiKeys(): ThunkResult { return async (dispatch) => { dispatch(isFetching()); const [keys, keysIncludingExpired] = await Promise.all([ - getBackendSrv().get('/api/auth/keys?includeExpired=false'), - getBackendSrv().get('/api/auth/keys?includeExpired=true'), + getBackendSrv().get('/api/auth/keys?includeExpired=false&accesscontrol=true'), + getBackendSrv().get('/api/auth/keys?includeExpired=true&accesscontrol=true'), ]); dispatch(apiKeysLoaded({ keys, keysIncludingExpired })); }; diff --git a/public/app/types/apiKeys.ts b/public/app/types/apiKeys.ts index 043fb3210d2..e024de1da1e 100644 --- a/public/app/types/apiKeys.ts +++ b/public/app/types/apiKeys.ts @@ -1,6 +1,8 @@ -import { OrgRole } from './acl'; +import { WithAccessControlMetadata } from '@grafana/data'; -export interface ApiKey { +import { OrgRole } from './acl'; + +export interface ApiKey extends WithAccessControlMetadata { id?: number; name: string; role: OrgRole;