From 64488f6b908528c1d9f2178ecec056551aef8f1a Mon Sep 17 00:00:00 2001 From: Serge Zaitsev Date: Tue, 2 Aug 2022 16:55:19 +0200 Subject: [PATCH] Chore: split APIKey store (#52781) * move apikey store into a separate service * add apikey service to wire graph * fix linter * switch api to use apikey service * fix provideservice in tests * add apikey service test double * try different sql syntax * rolling back the dialect * trigger drone * trigger drone --- pkg/api/apikey.go | 6 +- pkg/api/common_test.go | 2 +- pkg/api/http_server.go | 5 +- pkg/api/index.go | 2 +- pkg/middleware/middleware_basic_auth_test.go | 2 +- pkg/middleware/middleware_test.go | 14 +- pkg/middleware/rate_limit_test.go | 2 +- pkg/middleware/recovery_test.go | 2 +- pkg/middleware/testing.go | 2 + pkg/server/wire.go | 2 + pkg/services/apikey/apikey.go | 18 ++ pkg/services/apikey/apikeyimpl/apikey.go | 43 +++ pkg/services/apikey/apikeyimpl/store.go | 188 ++++++++++++ pkg/services/apikey/apikeyimpl/store_test.go | 267 ++++++++++++++++++ pkg/services/apikey/apikeytest/fake.go | 42 +++ pkg/services/apikey/model.go | 3 + .../contexthandler/auth_proxy_test.go | 2 +- pkg/services/contexthandler/contexthandler.go | 12 +- pkg/services/sqlstore/store.go | 8 - 19 files changed, 594 insertions(+), 28 deletions(-) create mode 100644 pkg/services/apikey/apikey.go create mode 100644 pkg/services/apikey/apikeyimpl/apikey.go create mode 100644 pkg/services/apikey/apikeyimpl/store.go create mode 100644 pkg/services/apikey/apikeyimpl/store_test.go create mode 100644 pkg/services/apikey/apikeytest/fake.go create mode 100644 pkg/services/apikey/model.go diff --git a/pkg/api/apikey.go b/pkg/api/apikey.go index 675fc837f57..bffc312dc23 100644 --- a/pkg/api/apikey.go +++ b/pkg/api/apikey.go @@ -28,7 +28,7 @@ import ( func (hs *HTTPServer) GetAPIKeys(c *models.ReqContext) response.Response { query := models.GetApiKeysQuery{OrgId: c.OrgId, User: c.SignedInUser, IncludeExpired: c.QueryBool("includeExpired")} - if err := hs.SQLStore.GetAPIKeys(c.Req.Context(), &query); err != nil { + if err := hs.apiKeyService.GetAPIKeys(c.Req.Context(), &query); err != nil { return response.Error(500, "Failed to list api keys", err) } @@ -76,7 +76,7 @@ func (hs *HTTPServer) DeleteAPIKey(c *models.ReqContext) response.Response { } cmd := &models.DeleteApiKeyCommand{Id: id, OrgId: c.OrgId} - err = hs.SQLStore.DeleteApiKey(c.Req.Context(), cmd) + err = hs.apiKeyService.DeleteApiKey(c.Req.Context(), cmd) if err != nil { var status int if errors.Is(err, models.ErrApiKeyNotFound) { @@ -132,7 +132,7 @@ func (hs *HTTPServer) AddAPIKey(c *models.ReqContext) response.Response { } cmd.Key = newKeyInfo.HashedKey - if err := hs.SQLStore.AddAPIKey(c.Req.Context(), &cmd); err != nil { + if err := hs.apiKeyService.AddAPIKey(c.Req.Context(), &cmd); err != nil { if errors.Is(err, models.ErrInvalidApiKeyExpiration) { return response.Error(400, err.Error(), nil) } diff --git a/pkg/api/common_test.go b/pkg/api/common_test.go index c9f30dd7a66..fc8c6765c92 100644 --- a/pkg/api/common_test.go +++ b/pkg/api/common_test.go @@ -196,7 +196,7 @@ func getContextHandler(t *testing.T, cfg *setting.Cfg) *contexthandler.ContextHa authProxy := authproxy.ProvideAuthProxy(cfg, remoteCacheSvc, loginservice.LoginServiceMock{}, sqlStore) loginService := &logintest.LoginServiceFake{} authenticator := &logintest.AuthenticatorFake{} - ctxHdlr := contexthandler.ProvideService(cfg, userAuthTokenSvc, authJWTSvc, remoteCacheSvc, renderSvc, sqlStore, tracer, authProxy, loginService, authenticator) + ctxHdlr := contexthandler.ProvideService(cfg, userAuthTokenSvc, authJWTSvc, remoteCacheSvc, renderSvc, sqlStore, tracer, authProxy, loginService, nil, authenticator) return ctxHdlr } diff --git a/pkg/api/http_server.go b/pkg/api/http_server.go index 04ae711c5c9..86c09c037e4 100644 --- a/pkg/api/http_server.go +++ b/pkg/api/http_server.go @@ -37,6 +37,7 @@ import ( "github.com/grafana/grafana/pkg/plugins/plugincontext" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/alerting" + "github.com/grafana/grafana/pkg/services/apikey" "github.com/grafana/grafana/pkg/services/cleanup" "github.com/grafana/grafana/pkg/services/comments" "github.com/grafana/grafana/pkg/services/contexthandler" @@ -171,6 +172,7 @@ type HTTPServer struct { PublicDashboardsApi *publicdashboardsApi.Api starService star.Service playlistService playlist.Service + apiKeyService apikey.Service CoremodelRegistry *registry.Generic CoremodelStaticRegistry *registry.Static kvStore kvstore.KVStore @@ -210,7 +212,7 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi teamsPermissionsService accesscontrol.TeamPermissionsService, folderPermissionsService accesscontrol.FolderPermissionsService, dashboardPermissionsService accesscontrol.DashboardPermissionsService, dashboardVersionService dashver.Service, starService star.Service, csrfService csrf.Service, coremodelRegistry *registry.Generic, coremodelStaticRegistry *registry.Static, - playlistService playlist.Service, kvStore kvstore.KVStore, secretsMigrator secrets.Migrator, remoteSecretsCheck secretsKV.UseRemoteSecretsPluginCheck, + playlistService playlist.Service, apiKeyService apikey.Service, kvStore kvstore.KVStore, secretsMigrator secrets.Migrator, remoteSecretsCheck secretsKV.UseRemoteSecretsPluginCheck, publicDashboardsApi *publicdashboardsApi.Api, userService user.Service) (*HTTPServer, error) { web.Env = cfg.Env m := web.New() @@ -293,6 +295,7 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi dashboardVersionService: dashboardVersionService, starService: starService, playlistService: playlistService, + apiKeyService: apiKeyService, CoremodelRegistry: coremodelRegistry, CoremodelStaticRegistry: coremodelStaticRegistry, kvStore: kvStore, diff --git a/pkg/api/index.go b/pkg/api/index.go index 7d44ab2b667..db8c1845789 100644 --- a/pkg/api/index.go +++ b/pkg/api/index.go @@ -305,7 +305,7 @@ func (hs *HTTPServer) getNavTree(c *models.ReqContext, hasEditPerm bool, prefs * } hideApiKeys, _, _ := hs.kvStore.Get(c.Req.Context(), c.OrgId, "serviceaccounts", "hideApiKeys") - apiKeys := hs.SQLStore.GetAllAPIKeys(c.Req.Context(), c.OrgId) + apiKeys := hs.apiKeyService.GetAllAPIKeys(c.Req.Context(), c.OrgId) apiKeysHidden := hideApiKeys == "1" && len(apiKeys) == 0 if hasAccess(ac.ReqOrgAdmin, apiKeyAccessEvaluator) && !apiKeysHidden { configNodes = append(configNodes, &dtos.NavLink{ diff --git a/pkg/middleware/middleware_basic_auth_test.go b/pkg/middleware/middleware_basic_auth_test.go index bcefdcdfa88..559f6bf64af 100644 --- a/pkg/middleware/middleware_basic_auth_test.go +++ b/pkg/middleware/middleware_basic_auth_test.go @@ -28,7 +28,7 @@ func TestMiddlewareBasicAuth(t *testing.T) { keyhash, err := util.EncodePassword("v5nAwpMafFP6znaS4urhdWDLS5511M42", "asd") require.NoError(t, err) - sc.mockSQLStore.ExpectedAPIKey = &models.ApiKey{OrgId: orgID, Role: models.ROLE_EDITOR, Key: keyhash} + sc.apiKeyService.ExpectedAPIKey = &models.ApiKey{OrgId: orgID, Role: models.ROLE_EDITOR, Key: keyhash} authHeader := util.GetBasicAuthHeader("api_key", "eyJrIjoidjVuQXdwTWFmRlA2em5hUzR1cmhkV0RMUzU1MTFNNDIiLCJuIjoiYXNkIiwiaWQiOjF9") sc.fakeReq("GET", "/").withAuthorizationHeader(authHeader).exec() diff --git a/pkg/middleware/middleware_test.go b/pkg/middleware/middleware_test.go index eb5c46a4714..caffc796fe2 100644 --- a/pkg/middleware/middleware_test.go +++ b/pkg/middleware/middleware_test.go @@ -20,6 +20,7 @@ import ( "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/login" "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/apikey/apikeytest" "github.com/grafana/grafana/pkg/services/auth" "github.com/grafana/grafana/pkg/services/contexthandler" "github.com/grafana/grafana/pkg/services/contexthandler/authproxy" @@ -150,7 +151,7 @@ func TestMiddlewareContext(t *testing.T) { keyhash, err := util.EncodePassword("v5nAwpMafFP6znaS4urhdWDLS5511M42", "asd") require.NoError(t, err) - sc.mockSQLStore.ExpectedAPIKey = &models.ApiKey{OrgId: orgID, Role: models.ROLE_EDITOR, Key: keyhash} + sc.apiKeyService.ExpectedAPIKey = &models.ApiKey{OrgId: orgID, Role: models.ROLE_EDITOR, Key: keyhash} sc.fakeReq("GET", "/").withValidApiKey().exec() @@ -163,7 +164,7 @@ func TestMiddlewareContext(t *testing.T) { middlewareScenario(t, "Valid API key, but does not match DB hash", func(t *testing.T, sc *scenarioContext) { const keyhash = "Something_not_matching" - sc.mockSQLStore.ExpectedAPIKey = &models.ApiKey{OrgId: 12, Role: models.ROLE_EDITOR, Key: keyhash} + sc.apiKeyService.ExpectedAPIKey = &models.ApiKey{OrgId: 12, Role: models.ROLE_EDITOR, Key: keyhash} sc.fakeReq("GET", "/").withValidApiKey().exec() @@ -178,7 +179,7 @@ func TestMiddlewareContext(t *testing.T) { require.NoError(t, err) expires := sc.contextHandler.GetTime().Add(-1 * time.Second).Unix() - sc.mockSQLStore.ExpectedAPIKey = &models.ApiKey{OrgId: 12, Role: models.ROLE_EDITOR, Key: keyhash, Expires: &expires} + sc.apiKeyService.ExpectedAPIKey = &models.ApiKey{OrgId: 12, Role: models.ROLE_EDITOR, Key: keyhash, Expires: &expires} sc.fakeReq("GET", "/").withValidApiKey().exec() @@ -627,7 +628,8 @@ func middlewareScenario(t *testing.T, desc string, fn scenarioFunc, cbs ...func( sc.mockSQLStore = mockstore.NewSQLStoreMock() sc.loginService = &loginservice.LoginServiceMock{} - ctxHdlr := getContextHandler(t, cfg, sc.mockSQLStore, sc.loginService) + sc.apiKeyService = &apikeytest.Service{} + ctxHdlr := getContextHandler(t, cfg, sc.mockSQLStore, sc.loginService, sc.apiKeyService) sc.sqlStore = ctxHdlr.SQLStore sc.contextHandler = ctxHdlr sc.m.Use(ctxHdlr.Middleware) @@ -657,7 +659,7 @@ func middlewareScenario(t *testing.T, desc string, fn scenarioFunc, cbs ...func( }) } -func getContextHandler(t *testing.T, cfg *setting.Cfg, mockSQLStore *mockstore.SQLStoreMock, loginService *loginservice.LoginServiceMock) *contexthandler.ContextHandler { +func getContextHandler(t *testing.T, cfg *setting.Cfg, mockSQLStore *mockstore.SQLStoreMock, loginService *loginservice.LoginServiceMock, apiKeyService *apikeytest.Service) *contexthandler.ContextHandler { t.Helper() if cfg == nil { @@ -674,7 +676,7 @@ func getContextHandler(t *testing.T, cfg *setting.Cfg, mockSQLStore *mockstore.S tracer := tracing.InitializeTracerForTest() authProxy := authproxy.ProvideAuthProxy(cfg, remoteCacheSvc, loginService, mockSQLStore) authenticator := &logintest.AuthenticatorFake{ExpectedUser: &user.User{}} - return contexthandler.ProvideService(cfg, userAuthTokenSvc, authJWTSvc, remoteCacheSvc, renderSvc, mockSQLStore, tracer, authProxy, loginService, authenticator) + return contexthandler.ProvideService(cfg, userAuthTokenSvc, authJWTSvc, remoteCacheSvc, renderSvc, mockSQLStore, tracer, authProxy, loginService, apiKeyService, authenticator) } type fakeRenderService struct { diff --git a/pkg/middleware/rate_limit_test.go b/pkg/middleware/rate_limit_test.go index 88fb7e021c5..b0c182a99d8 100644 --- a/pkg/middleware/rate_limit_test.go +++ b/pkg/middleware/rate_limit_test.go @@ -33,7 +33,7 @@ func rateLimiterScenario(t *testing.T, desc string, rps int, burst int, fn rateL m := web.New() m.UseMiddleware(web.Renderer("../../public/views", "[[", "]]")) - m.Use(getContextHandler(t, cfg, nil, nil).Middleware) + m.Use(getContextHandler(t, cfg, nil, nil, nil).Middleware) m.Get("/foo", RateLimit(rps, burst, func() time.Time { return currentTime }), defaultHandler) fn(func() *httptest.ResponseRecorder { diff --git a/pkg/middleware/recovery_test.go b/pkg/middleware/recovery_test.go index 251558056dc..85bbef0246b 100644 --- a/pkg/middleware/recovery_test.go +++ b/pkg/middleware/recovery_test.go @@ -67,7 +67,7 @@ func recoveryScenario(t *testing.T, desc string, url string, fn scenarioFunc) { sc.userAuthTokenService = auth.NewFakeUserAuthTokenService() sc.remoteCacheService = remotecache.NewFakeStore(t) - contextHandler := getContextHandler(t, nil, nil, nil) + contextHandler := getContextHandler(t, nil, nil, nil, nil) sc.m.Use(contextHandler.Middleware) // mock out gc goroutine sc.m.Use(OrgRedirect(cfg, sc.mockSQLStore)) diff --git a/pkg/middleware/testing.go b/pkg/middleware/testing.go index aa677b53770..e35346a66dd 100644 --- a/pkg/middleware/testing.go +++ b/pkg/middleware/testing.go @@ -8,6 +8,7 @@ import ( "github.com/grafana/grafana/pkg/infra/remotecache" "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/apikey/apikeytest" "github.com/grafana/grafana/pkg/services/auth" "github.com/grafana/grafana/pkg/services/contexthandler" "github.com/grafana/grafana/pkg/services/login/loginservice" @@ -39,6 +40,7 @@ type scenarioContext struct { mockSQLStore *mockstore.SQLStoreMock contextHandler *contexthandler.ContextHandler loginService *loginservice.LoginServiceMock + apiKeyService *apikeytest.Service req *http.Request } diff --git a/pkg/server/wire.go b/pkg/server/wire.go index 592e95c152a..467f8d2b8b7 100644 --- a/pkg/server/wire.go +++ b/pkg/server/wire.go @@ -41,6 +41,7 @@ import ( "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol/ossaccesscontrol" "github.com/grafana/grafana/pkg/services/alerting" + "github.com/grafana/grafana/pkg/services/apikey/apikeyimpl" "github.com/grafana/grafana/pkg/services/auth/jwt" "github.com/grafana/grafana/pkg/services/cleanup" "github.com/grafana/grafana/pkg/services/comments" @@ -297,6 +298,7 @@ var wireBasicSet = wire.NewSet( wire.Bind(new(accesscontrol.DashboardPermissionsService), new(*ossaccesscontrol.DashboardPermissionsService)), starimpl.ProvideService, playlistimpl.ProvideService, + apikeyimpl.ProvideService, dashverimpl.ProvideService, publicdashboardsService.ProvideService, wire.Bind(new(publicdashboards.Service), new(*publicdashboardsService.PublicDashboardServiceImpl)), diff --git a/pkg/services/apikey/apikey.go b/pkg/services/apikey/apikey.go new file mode 100644 index 00000000000..ba6f58b50e0 --- /dev/null +++ b/pkg/services/apikey/apikey.go @@ -0,0 +1,18 @@ +package apikey + +import ( + "context" + + "github.com/grafana/grafana/pkg/models" +) + +type Service interface { + GetAPIKeys(ctx context.Context, query *models.GetApiKeysQuery) error + GetAllAPIKeys(ctx context.Context, orgID int64) []*models.ApiKey + DeleteApiKey(ctx context.Context, cmd *models.DeleteApiKeyCommand) error + AddAPIKey(ctx context.Context, cmd *models.AddApiKeyCommand) error + GetApiKeyById(ctx context.Context, query *models.GetApiKeyByIdQuery) error + GetApiKeyByName(ctx context.Context, query *models.GetApiKeyByNameQuery) error + GetAPIKeyByHash(ctx context.Context, hash string) (*models.ApiKey, error) + UpdateAPIKeyLastUsedDate(ctx context.Context, tokenID int64) error +} diff --git a/pkg/services/apikey/apikeyimpl/apikey.go b/pkg/services/apikey/apikeyimpl/apikey.go new file mode 100644 index 00000000000..ec0bd935500 --- /dev/null +++ b/pkg/services/apikey/apikeyimpl/apikey.go @@ -0,0 +1,43 @@ +package apikeyimpl + +import ( + "context" + + "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/apikey" + "github.com/grafana/grafana/pkg/services/sqlstore/db" + "github.com/grafana/grafana/pkg/setting" +) + +type Service struct { + store store +} + +func ProvideService(db db.DB, cfg *setting.Cfg) apikey.Service { + return &Service{store: &sqlStore{db: db, cfg: cfg}} +} + +func (s *Service) GetAPIKeys(ctx context.Context, query *models.GetApiKeysQuery) error { + return s.store.GetAPIKeys(ctx, query) +} +func (s *Service) GetAllAPIKeys(ctx context.Context, orgID int64) []*models.ApiKey { + return s.store.GetAllAPIKeys(ctx, orgID) +} +func (s *Service) GetApiKeyById(ctx context.Context, query *models.GetApiKeyByIdQuery) error { + return s.store.GetApiKeyById(ctx, query) +} +func (s *Service) GetApiKeyByName(ctx context.Context, query *models.GetApiKeyByNameQuery) error { + return s.store.GetApiKeyByName(ctx, query) +} +func (s *Service) GetAPIKeyByHash(ctx context.Context, hash string) (*models.ApiKey, error) { + return s.store.GetAPIKeyByHash(ctx, hash) +} +func (s *Service) DeleteApiKey(ctx context.Context, cmd *models.DeleteApiKeyCommand) error { + return s.store.DeleteApiKey(ctx, cmd) +} +func (s *Service) AddAPIKey(ctx context.Context, cmd *models.AddApiKeyCommand) error { + return s.store.AddAPIKey(ctx, cmd) +} +func (s *Service) UpdateAPIKeyLastUsedDate(ctx context.Context, tokenID int64) error { + return s.store.UpdateAPIKeyLastUsedDate(ctx, tokenID) +} diff --git a/pkg/services/apikey/apikeyimpl/store.go b/pkg/services/apikey/apikeyimpl/store.go new file mode 100644 index 00000000000..8b427771988 --- /dev/null +++ b/pkg/services/apikey/apikeyimpl/store.go @@ -0,0 +1,188 @@ +package apikeyimpl + +import ( + "context" + "fmt" + "time" + + "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/sqlstore" + "github.com/grafana/grafana/pkg/services/sqlstore/db" + "github.com/grafana/grafana/pkg/setting" + "xorm.io/xorm" +) + +type store interface { + GetAPIKeys(ctx context.Context, query *models.GetApiKeysQuery) error + GetAllAPIKeys(ctx context.Context, orgID int64) []*models.ApiKey + DeleteApiKey(ctx context.Context, cmd *models.DeleteApiKeyCommand) error + AddAPIKey(ctx context.Context, cmd *models.AddApiKeyCommand) error + GetApiKeyById(ctx context.Context, query *models.GetApiKeyByIdQuery) error + GetApiKeyByName(ctx context.Context, query *models.GetApiKeyByNameQuery) error + GetAPIKeyByHash(ctx context.Context, hash string) (*models.ApiKey, error) + UpdateAPIKeyLastUsedDate(ctx context.Context, tokenID int64) error +} + +type sqlStore struct { + db db.DB + cfg *setting.Cfg +} + +// timeNow makes it possible to test usage of time +var timeNow = time.Now + +func (ss *sqlStore) GetAPIKeys(ctx context.Context, query *models.GetApiKeysQuery) error { + return ss.db.WithDbSession(ctx, func(dbSession *sqlstore.DBSession) error { + var sess *xorm.Session + + if query.IncludeExpired { + sess = dbSession.Limit(100, 0). + Where("org_id=?", query.OrgId). + Asc("name") + } else { + sess = dbSession.Limit(100, 0). + Where("org_id=? and ( expires IS NULL or expires >= ?)", query.OrgId, timeNow().Unix()). + Asc("name") + } + + sess = sess.Where("service_account_id IS NULL") + + if !accesscontrol.IsDisabled(ss.cfg) { + 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) + }) +} + +func (ss *sqlStore) GetAllAPIKeys(ctx context.Context, orgID int64) []*models.ApiKey { + result := make([]*models.ApiKey, 0) + err := ss.db.WithDbSession(ctx, func(dbSession *sqlstore.DBSession) error { + sess := dbSession.Where("service_account_id IS NULL").Asc("name") + if orgID != -1 { + sess = sess.Where("org_id=?", orgID) + } + return sess.Find(&result) + }) + if err != nil { + _ = err + // TODO: return error + } + return result +} + +func (ss *sqlStore) DeleteApiKey(ctx context.Context, cmd *models.DeleteApiKeyCommand) error { + return ss.db.WithDbSession(ctx, func(sess *sqlstore.DBSession) error { + rawSQL := "DELETE FROM api_key WHERE id=? and org_id=? and service_account_id IS NULL" + result, err := sess.Exec(rawSQL, cmd.Id, cmd.OrgId) + if err != nil { + return err + } + n, err := result.RowsAffected() + if err != nil { + return err + } else if n == 0 { + return models.ErrApiKeyNotFound + } + return nil + }) +} + +func (ss *sqlStore) AddAPIKey(ctx context.Context, cmd *models.AddApiKeyCommand) error { + return ss.db.WithTransactionalDbSession(ctx, func(sess *sqlstore.DBSession) error { + key := models.ApiKey{OrgId: cmd.OrgId, Name: cmd.Name} + exists, _ := sess.Get(&key) + if exists { + return models.ErrDuplicateApiKey + } + + updated := timeNow() + var expires *int64 = nil + if cmd.SecondsToLive > 0 { + v := updated.Add(time.Second * time.Duration(cmd.SecondsToLive)).Unix() + expires = &v + } else if cmd.SecondsToLive < 0 { + return models.ErrInvalidApiKeyExpiration + } + + t := models.ApiKey{ + OrgId: cmd.OrgId, + Name: cmd.Name, + Role: cmd.Role, + Key: cmd.Key, + Created: updated, + Updated: updated, + Expires: expires, + ServiceAccountId: nil, + } + + if _, err := sess.Insert(&t); err != nil { + return err + } + cmd.Result = &t + return nil + }) +} + +func (ss *sqlStore) GetApiKeyById(ctx context.Context, query *models.GetApiKeyByIdQuery) error { + return ss.db.WithDbSession(ctx, func(sess *sqlstore.DBSession) error { + var apikey models.ApiKey + has, err := sess.ID(query.ApiKeyId).Get(&apikey) + + if err != nil { + return err + } else if !has { + return models.ErrInvalidApiKey + } + + query.Result = &apikey + return nil + }) +} + +func (ss *sqlStore) GetApiKeyByName(ctx context.Context, query *models.GetApiKeyByNameQuery) error { + return ss.db.WithDbSession(ctx, func(sess *sqlstore.DBSession) error { + var apikey models.ApiKey + has, err := sess.Where("org_id=? AND name=?", query.OrgId, query.KeyName).Get(&apikey) + + if err != nil { + return err + } else if !has { + return models.ErrInvalidApiKey + } + + query.Result = &apikey + return nil + }) +} + +func (ss *sqlStore) GetAPIKeyByHash(ctx context.Context, hash string) (*models.ApiKey, error) { + var apikey models.ApiKey + err := ss.db.WithDbSession(ctx, func(sess *sqlstore.DBSession) error { + has, err := sess.Table("api_key").Where(fmt.Sprintf("%s = ?", ss.db.GetDialect().Quote("key")), hash).Get(&apikey) + if err != nil { + return err + } else if !has { + return models.ErrInvalidApiKey + } + return nil + }) + return &apikey, err +} + +func (ss *sqlStore) UpdateAPIKeyLastUsedDate(ctx context.Context, tokenID int64) error { + now := timeNow() + return ss.db.WithDbSession(ctx, func(sess *sqlstore.DBSession) error { + if _, err := sess.Table("api_key").ID(tokenID).Cols("last_used_at").Update(&models.ApiKey{LastUsedAt: &now}); err != nil { + return err + } + + return nil + }) +} diff --git a/pkg/services/apikey/apikeyimpl/store_test.go b/pkg/services/apikey/apikeyimpl/store_test.go new file mode 100644 index 00000000000..c861338b6cd --- /dev/null +++ b/pkg/services/apikey/apikeyimpl/store_test.go @@ -0,0 +1,267 @@ +package apikeyimpl + +import ( + "context" + "fmt" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/sqlstore" +) + +func mockTimeNow() { + var timeSeed int64 + timeNow = func() time.Time { + loc := time.FixedZone("MockZoneUTC-5", -5*60*60) + fakeNow := time.Unix(timeSeed, 0).In(loc) + timeSeed++ + return fakeNow + } +} + +func resetTimeNow() { + timeNow = time.Now +} + +func TestIntegrationApiKeyDataAccess(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test") + } + mockTimeNow() + defer resetTimeNow() + + t.Run("Testing API Key data access", func(t *testing.T) { + db := sqlstore.InitTestDB(t) + ss := &sqlStore{db: db, cfg: db.Cfg} + + t.Run("Given saved api key", func(t *testing.T) { + cmd := models.AddApiKeyCommand{OrgId: 1, Name: "hello", Key: "asd"} + err := ss.AddAPIKey(context.Background(), &cmd) + assert.Nil(t, err) + + t.Run("Should be able to get key by name", func(t *testing.T) { + query := models.GetApiKeyByNameQuery{KeyName: "hello", OrgId: 1} + err = ss.GetApiKeyByName(context.Background(), &query) + + assert.Nil(t, err) + assert.NotNil(t, query.Result) + }) + + t.Run("Should be able to get key by hash", func(t *testing.T) { + key, err := ss.GetAPIKeyByHash(context.Background(), cmd.Key) + + assert.Nil(t, err) + assert.NotNil(t, key) + }) + }) + + t.Run("Add non expiring key", func(t *testing.T) { + cmd := models.AddApiKeyCommand{OrgId: 1, Name: "non-expiring", Key: "asd1", SecondsToLive: 0} + err := ss.AddAPIKey(context.Background(), &cmd) + assert.Nil(t, err) + + query := models.GetApiKeyByNameQuery{KeyName: "non-expiring", OrgId: 1} + err = ss.GetApiKeyByName(context.Background(), &query) + assert.Nil(t, err) + + assert.Nil(t, query.Result.Expires) + }) + + t.Run("Add an expiring key", func(t *testing.T) { + // expires in one hour + cmd := models.AddApiKeyCommand{OrgId: 1, Name: "expiring-in-an-hour", Key: "asd2", SecondsToLive: 3600} + err := ss.AddAPIKey(context.Background(), &cmd) + assert.Nil(t, err) + + query := models.GetApiKeyByNameQuery{KeyName: "expiring-in-an-hour", OrgId: 1} + err = ss.GetApiKeyByName(context.Background(), &query) + assert.Nil(t, err) + + assert.True(t, *query.Result.Expires >= timeNow().Unix()) + + // timeNow() has been called twice since creation; once by AddAPIKey and once by GetApiKeyByName + // therefore two seconds should be subtracted by next value returned by timeNow() + // that equals the number by which timeSeed has been advanced + then := timeNow().Add(-2 * time.Second) + expected := then.Add(1 * time.Hour).UTC().Unix() + assert.Equal(t, *query.Result.Expires, expected) + }) + + t.Run("Last Used At datetime update", func(t *testing.T) { + // expires in one hour + cmd := models.AddApiKeyCommand{OrgId: 1, Name: "last-update-at", Key: "asd3", SecondsToLive: 3600} + err := ss.AddAPIKey(context.Background(), &cmd) + require.NoError(t, err) + + assert.Nil(t, cmd.Result.LastUsedAt) + + err = ss.UpdateAPIKeyLastUsedDate(context.Background(), cmd.Result.Id) + require.NoError(t, err) + + query := models.GetApiKeyByNameQuery{KeyName: "last-update-at", OrgId: 1} + err = ss.GetApiKeyByName(context.Background(), &query) + assert.Nil(t, err) + + assert.NotNil(t, query.Result.LastUsedAt) + }) + + t.Run("Add a key with negative lifespan", func(t *testing.T) { + // expires in one day + cmd := models.AddApiKeyCommand{OrgId: 1, Name: "key-with-negative-lifespan", Key: "asd3", SecondsToLive: -3600} + err := ss.AddAPIKey(context.Background(), &cmd) + assert.EqualError(t, err, models.ErrInvalidApiKeyExpiration.Error()) + + query := models.GetApiKeyByNameQuery{KeyName: "key-with-negative-lifespan", OrgId: 1} + err = ss.GetApiKeyByName(context.Background(), &query) + assert.EqualError(t, err, "invalid API key") + }) + + t.Run("Add keys", func(t *testing.T) { + // never expires + cmd := models.AddApiKeyCommand{OrgId: 1, Name: "key1", Key: "key1", SecondsToLive: 0} + err := ss.AddAPIKey(context.Background(), &cmd) + assert.Nil(t, err) + + // expires in 1s + cmd = models.AddApiKeyCommand{OrgId: 1, Name: "key2", Key: "key2", SecondsToLive: 1} + err = ss.AddAPIKey(context.Background(), &cmd) + assert.Nil(t, err) + + // expires in one hour + cmd = models.AddApiKeyCommand{OrgId: 1, Name: "key3", Key: "key3", SecondsToLive: 3600} + err = ss.AddAPIKey(context.Background(), &cmd) + assert.Nil(t, err) + + // advance mocked getTime by 1s + timeNow() + + testUser := &models.SignedInUser{ + OrgId: 1, + Permissions: map[int64]map[string][]string{ + 1: {accesscontrol.ActionAPIKeyRead: []string{accesscontrol.ScopeAPIKeysAll}}, + }, + } + query := models.GetApiKeysQuery{OrgId: 1, IncludeExpired: false, User: testUser} + err = ss.GetAPIKeys(context.Background(), &query) + assert.Nil(t, err) + + for _, k := range query.Result { + if k.Name == "key2" { + t.Fatalf("key2 should not be there") + } + } + + query = models.GetApiKeysQuery{OrgId: 1, IncludeExpired: true, User: testUser} + err = ss.GetAPIKeys(context.Background(), &query) + assert.Nil(t, err) + + found := false + for _, k := range query.Result { + if k.Name == "key2" { + found = true + } + } + assert.True(t, found) + }) + }) +} + +func TestIntegrationApiKeyErrors(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test") + } + mockTimeNow() + defer resetTimeNow() + + t.Run("Testing API Key errors", func(t *testing.T) { + db := sqlstore.InitTestDB(t) + ss := &sqlStore{db: db, cfg: db.Cfg} + + t.Run("Delete non-existing key should return error", func(t *testing.T) { + cmd := models.DeleteApiKeyCommand{Id: 1} + err := ss.DeleteApiKey(context.Background(), &cmd) + + assert.EqualError(t, err, models.ErrApiKeyNotFound.Error()) + }) + + t.Run("Testing API Duplicate Key Errors", func(t *testing.T) { + t.Run("Given saved api key", func(t *testing.T) { + cmd := models.AddApiKeyCommand{OrgId: 0, Name: "duplicate", Key: "asd"} + err := ss.AddAPIKey(context.Background(), &cmd) + assert.Nil(t, err) + + t.Run("Add API Key with existing Org ID and Name", func(t *testing.T) { + cmd := models.AddApiKeyCommand{OrgId: 0, Name: "duplicate", Key: "asd"} + err = ss.AddAPIKey(context.Background(), &cmd) + assert.EqualError(t, err, models.ErrDuplicateApiKey.Error()) + }) + }) + }) + }) +} + +type getApiKeysTestCase struct { + desc string + user *models.SignedInUser + expectedNumKeys int +} + +func TestIntegrationSQLStore_GetAPIKeys(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test") + } + 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) { + db := sqlstore.InitTestDB(t, sqlstore.InitTestDBOpt{}) + store := &sqlStore{db: db, cfg: db.Cfg} + 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 store, 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/pkg/services/apikey/apikeytest/fake.go b/pkg/services/apikey/apikeytest/fake.go new file mode 100644 index 00000000000..9b9c7cef88e --- /dev/null +++ b/pkg/services/apikey/apikeytest/fake.go @@ -0,0 +1,42 @@ +package apikeytest + +import ( + "context" + + "github.com/grafana/grafana/pkg/models" +) + +type Service struct { + ExpectedError error + ExpectedAPIKeys []*models.ApiKey + ExpectedAPIKey *models.ApiKey +} + +func (s *Service) GetAPIKeys(ctx context.Context, query *models.GetApiKeysQuery) error { + query.Result = s.ExpectedAPIKeys + return s.ExpectedError +} +func (s *Service) GetAllAPIKeys(ctx context.Context, orgID int64) []*models.ApiKey { + return s.ExpectedAPIKeys +} +func (s *Service) GetApiKeyById(ctx context.Context, query *models.GetApiKeyByIdQuery) error { + query.Result = s.ExpectedAPIKey + return s.ExpectedError +} +func (s *Service) GetApiKeyByName(ctx context.Context, query *models.GetApiKeyByNameQuery) error { + query.Result = s.ExpectedAPIKey + return s.ExpectedError +} +func (s *Service) GetAPIKeyByHash(ctx context.Context, hash string) (*models.ApiKey, error) { + return s.ExpectedAPIKey, s.ExpectedError +} +func (s *Service) DeleteApiKey(ctx context.Context, cmd *models.DeleteApiKeyCommand) error { + return s.ExpectedError +} +func (s *Service) AddAPIKey(ctx context.Context, cmd *models.AddApiKeyCommand) error { + cmd.Result = s.ExpectedAPIKey + return s.ExpectedError +} +func (s *Service) UpdateAPIKeyLastUsedDate(ctx context.Context, tokenID int64) error { + return s.ExpectedError +} diff --git a/pkg/services/apikey/model.go b/pkg/services/apikey/model.go new file mode 100644 index 00000000000..72c1a026489 --- /dev/null +++ b/pkg/services/apikey/model.go @@ -0,0 +1,3 @@ +package apikey + +// TODO: define all apikey models here diff --git a/pkg/services/contexthandler/auth_proxy_test.go b/pkg/services/contexthandler/auth_proxy_test.go index 2e02aa81812..117150e13eb 100644 --- a/pkg/services/contexthandler/auth_proxy_test.go +++ b/pkg/services/contexthandler/auth_proxy_test.go @@ -86,7 +86,7 @@ func getContextHandler(t *testing.T) *ContextHandler { authProxy := authproxy.ProvideAuthProxy(cfg, remoteCacheSvc, loginService, &FakeGetSignUserStore{}) authenticator := &fakeAuthenticator{} - return ProvideService(cfg, userAuthTokenSvc, authJWTSvc, remoteCacheSvc, renderSvc, sqlStore, tracer, authProxy, loginService, authenticator) + return ProvideService(cfg, userAuthTokenSvc, authJWTSvc, remoteCacheSvc, renderSvc, sqlStore, tracer, authProxy, loginService, nil, authenticator) } type FakeGetSignUserStore struct { diff --git a/pkg/services/contexthandler/contexthandler.go b/pkg/services/contexthandler/contexthandler.go index c8bd9470da2..2aaa4881fe6 100644 --- a/pkg/services/contexthandler/contexthandler.go +++ b/pkg/services/contexthandler/contexthandler.go @@ -19,6 +19,7 @@ import ( loginpkg "github.com/grafana/grafana/pkg/login" "github.com/grafana/grafana/pkg/middleware/cookies" "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/apikey" "github.com/grafana/grafana/pkg/services/contexthandler/authproxy" "github.com/grafana/grafana/pkg/services/contexthandler/ctxkey" "github.com/grafana/grafana/pkg/services/login" @@ -40,7 +41,8 @@ const ServiceName = "ContextHandler" func ProvideService(cfg *setting.Cfg, tokenService models.UserTokenService, jwtService models.JWTService, remoteCache *remotecache.RemoteCache, renderService rendering.Service, sqlStore sqlstore.Store, - tracer tracing.Tracer, authProxy *authproxy.AuthProxy, loginService login.Service, authenticator loginpkg.Authenticator) *ContextHandler { + tracer tracing.Tracer, authProxy *authproxy.AuthProxy, loginService login.Service, + apiKeyService apikey.Service, authenticator loginpkg.Authenticator) *ContextHandler { return &ContextHandler{ Cfg: cfg, AuthTokenService: tokenService, @@ -52,6 +54,7 @@ func ProvideService(cfg *setting.Cfg, tokenService models.UserTokenService, jwtS authProxy: authProxy, authenticator: authenticator, loginService: loginService, + apiKeyService: apiKeyService, } } @@ -67,6 +70,7 @@ type ContextHandler struct { authProxy *authproxy.AuthProxy authenticator loginpkg.Authenticator loginService login.Service + apiKeyService apikey.Service // GetTime returns the current time. // Stubbable by tests. GetTime func() time.Time @@ -197,7 +201,7 @@ func (h *ContextHandler) getPrefixedAPIKey(ctx context.Context, keyString string return nil, err } - return h.SQLStore.GetAPIKeyByHash(ctx, hash) + return h.apiKeyService.GetAPIKeyByHash(ctx, hash) } func (h *ContextHandler) getAPIKey(ctx context.Context, keyString string) (*models.ApiKey, error) { @@ -208,7 +212,7 @@ func (h *ContextHandler) getAPIKey(ctx context.Context, keyString string) (*mode // fetch key keyQuery := models.GetApiKeyByNameQuery{KeyName: decoded.Name, OrgId: decoded.OrgId} - if err := h.SQLStore.GetApiKeyByName(ctx, &keyQuery); err != nil { + if err := h.apiKeyService.GetApiKeyByName(ctx, &keyQuery); err != nil { return nil, err } @@ -274,7 +278,7 @@ func (h *ContextHandler) initContextWithAPIKey(reqContext *models.ReqContext) bo } // update api_key last used date - if err := h.SQLStore.UpdateAPIKeyLastUsedDate(reqContext.Req.Context(), apikey.Id); err != nil { + if err := h.apiKeyService.UpdateAPIKeyLastUsedDate(reqContext.Req.Context(), apikey.Id); err != nil { reqContext.JsonApiErr(http.StatusInternalServerError, InvalidAPIKey, errKey) return true } diff --git a/pkg/services/sqlstore/store.go b/pkg/services/sqlstore/store.go index bcba1b75781..1521c372cec 100644 --- a/pkg/services/sqlstore/store.go +++ b/pkg/services/sqlstore/store.go @@ -120,14 +120,6 @@ type Store interface { SetAlertNotificationStateToCompleteCommand(ctx context.Context, cmd *models.SetAlertNotificationStateToCompleteCommand) error SetAlertNotificationStateToPendingCommand(ctx context.Context, cmd *models.SetAlertNotificationStateToPendingCommand) error GetOrCreateAlertNotificationState(ctx context.Context, cmd *models.GetOrCreateNotificationStateQuery) error - GetAPIKeys(ctx context.Context, query *models.GetApiKeysQuery) error - GetAllAPIKeys(ctx context.Context, orgID int64) []*models.ApiKey - DeleteApiKey(ctx context.Context, cmd *models.DeleteApiKeyCommand) error - AddAPIKey(ctx context.Context, cmd *models.AddApiKeyCommand) error - GetApiKeyById(ctx context.Context, query *models.GetApiKeyByIdQuery) error - GetApiKeyByName(ctx context.Context, query *models.GetApiKeyByNameQuery) error - GetAPIKeyByHash(ctx context.Context, hash string) (*models.ApiKey, error) - UpdateAPIKeyLastUsedDate(ctx context.Context, tokenID int64) error UpdateTempUserStatus(ctx context.Context, cmd *models.UpdateTempUserStatusCommand) error CreateTempUser(ctx context.Context, cmd *models.CreateTempUserCommand) error UpdateTempUserWithEmailSent(ctx context.Context, cmd *models.UpdateTempUserWithEmailSentCommand) error