diff --git a/pkg/services/serviceaccounts/api/api_test.go b/pkg/services/serviceaccounts/api/api_test.go index 8e136eabdd5..5dc1e2bdbe6 100644 --- a/pkg/services/serviceaccounts/api/api_test.go +++ b/pkg/services/serviceaccounts/api/api_test.go @@ -19,6 +19,7 @@ import ( acDatabase "github.com/grafana/grafana/pkg/services/accesscontrol/database" accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock" "github.com/grafana/grafana/pkg/services/accesscontrol/ossaccesscontrol" + "github.com/grafana/grafana/pkg/services/apikey/apikeyimpl" "github.com/grafana/grafana/pkg/services/contexthandler/ctxkey" "github.com/grafana/grafana/pkg/services/licensing" "github.com/grafana/grafana/pkg/services/serviceaccounts" @@ -38,8 +39,9 @@ var ( func TestServiceAccountsAPI_CreateServiceAccount(t *testing.T) { store := sqlstore.InitTestDB(t) + apiKeyService := apikeyimpl.ProvideService(store, store.Cfg) kvStore := kvstore.ProvideService(store) - saStore := database.ProvideServiceAccountsStore(store, kvStore) + saStore := database.ProvideServiceAccountsStore(store, apiKeyService, kvStore) svcmock := tests.ServiceAccountMock{} autoAssignOrg := store.Cfg.AutoAssignOrg @@ -204,7 +206,8 @@ func TestServiceAccountsAPI_CreateServiceAccount(t *testing.T) { func TestServiceAccountsAPI_DeleteServiceAccount(t *testing.T) { store := sqlstore.InitTestDB(t) kvStore := kvstore.ProvideService(store) - saStore := database.ProvideServiceAccountsStore(store, kvStore) + apiKeyService := apikeyimpl.ProvideService(store, store.Cfg) + saStore := database.ProvideServiceAccountsStore(store, apiKeyService, kvStore) svcmock := tests.ServiceAccountMock{} var requestResponse = func(server *web.Mux, httpMethod, requestpath string) *httptest.ResponseRecorder { @@ -304,8 +307,9 @@ func setupTestServer(t *testing.T, svc *tests.ServiceAccountMock, func TestServiceAccountsAPI_RetrieveServiceAccount(t *testing.T) { store := sqlstore.InitTestDB(t) + apiKeyService := apikeyimpl.ProvideService(store, store.Cfg) kvStore := kvstore.ProvideService(store) - saStore := database.ProvideServiceAccountsStore(store, kvStore) + saStore := database.ProvideServiceAccountsStore(store, apiKeyService, kvStore) svcmock := tests.ServiceAccountMock{} type testRetrieveSATestCase struct { desc string @@ -395,8 +399,9 @@ func newString(s string) *string { func TestServiceAccountsAPI_UpdateServiceAccount(t *testing.T) { store := sqlstore.InitTestDB(t) + apiKeyService := apikeyimpl.ProvideService(store, store.Cfg) kvStore := kvstore.ProvideService(store) - saStore := database.ProvideServiceAccountsStore(store, kvStore) + saStore := database.ProvideServiceAccountsStore(store, apiKeyService, kvStore) svcmock := tests.ServiceAccountMock{} type testUpdateSATestCase struct { desc string diff --git a/pkg/services/serviceaccounts/api/token_test.go b/pkg/services/serviceaccounts/api/token_test.go index 2c9f5c3107a..8973bcf85c5 100644 --- a/pkg/services/serviceaccounts/api/token_test.go +++ b/pkg/services/serviceaccounts/api/token_test.go @@ -18,6 +18,7 @@ import ( "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/accesscontrol" accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock" + "github.com/grafana/grafana/pkg/services/apikey/apikeyimpl" "github.com/grafana/grafana/pkg/services/serviceaccounts" "github.com/grafana/grafana/pkg/services/serviceaccounts/database" "github.com/grafana/grafana/pkg/services/serviceaccounts/tests" @@ -51,8 +52,9 @@ func createTokenforSA(t *testing.T, store serviceaccounts.Store, keyName string, func TestServiceAccountsAPI_CreateToken(t *testing.T) { store := sqlstore.InitTestDB(t) + apiKeyService := apikeyimpl.ProvideService(store, store.Cfg) kvStore := kvstore.ProvideService(store) - saStore := database.ProvideServiceAccountsStore(store, kvStore) + saStore := database.ProvideServiceAccountsStore(store, apiKeyService, kvStore) svcmock := tests.ServiceAccountMock{} sa := tests.SetupUserServiceAccount(t, store, tests.TestUser{Login: "sa", IsServiceAccount: true}) @@ -147,7 +149,7 @@ func TestServiceAccountsAPI_CreateToken(t *testing.T) { assert.Equal(t, tc.body["name"], actualBody["name"]) query := models.GetApiKeyByNameQuery{KeyName: tc.body["name"].(string), OrgId: sa.OrgID} - err = store.GetApiKeyByName(context.Background(), &query) + err = apiKeyService.GetApiKeyByName(context.Background(), &query) require.NoError(t, err) assert.Equal(t, sa.ID, *query.Result.ServiceAccountId) @@ -167,9 +169,10 @@ func TestServiceAccountsAPI_CreateToken(t *testing.T) { func TestServiceAccountsAPI_DeleteToken(t *testing.T) { store := sqlstore.InitTestDB(t) + apiKeyService := apikeyimpl.ProvideService(store, store.Cfg) kvStore := kvstore.ProvideService(store) svcMock := &tests.ServiceAccountMock{} - saStore := database.ProvideServiceAccountsStore(store, kvStore) + saStore := database.ProvideServiceAccountsStore(store, apiKeyService, kvStore) sa := tests.SetupUserServiceAccount(t, store, tests.TestUser{Login: "sa", IsServiceAccount: true}) type testCreateSAToken struct { @@ -243,7 +246,7 @@ func TestServiceAccountsAPI_DeleteToken(t *testing.T) { require.Equal(t, tc.expectedCode, actualCode, endpoint, actualBody) query := models.GetApiKeyByNameQuery{KeyName: tc.keyName, OrgId: sa.OrgID} - err := store.GetApiKeyByName(context.Background(), &query) + err := apiKeyService.GetApiKeyByName(context.Background(), &query) if actualCode == http.StatusOK { require.Error(t, err) } else { diff --git a/pkg/services/serviceaccounts/database/database.go b/pkg/services/serviceaccounts/database/database.go index a6c7fbb7042..d1d98f8a9b2 100644 --- a/pkg/services/serviceaccounts/database/database.go +++ b/pkg/services/serviceaccounts/database/database.go @@ -12,22 +12,25 @@ import ( "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/apikey" "github.com/grafana/grafana/pkg/services/serviceaccounts" "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/services/user" ) type ServiceAccountsStoreImpl struct { - sqlStore *sqlstore.SQLStore - kvStore kvstore.KVStore - log log.Logger + sqlStore *sqlstore.SQLStore + apiKeyService apikey.Service + kvStore kvstore.KVStore + log log.Logger } -func ProvideServiceAccountsStore(store *sqlstore.SQLStore, kvStore kvstore.KVStore) *ServiceAccountsStoreImpl { +func ProvideServiceAccountsStore(store *sqlstore.SQLStore, apiKeyService apikey.Service, kvStore kvstore.KVStore) *ServiceAccountsStoreImpl { return &ServiceAccountsStoreImpl{ - sqlStore: store, - kvStore: kvStore, - log: log.New("serviceaccounts.store"), + sqlStore: store, + apiKeyService: apiKeyService, + kvStore: kvStore, + log: log.New("serviceaccounts.store"), } } @@ -401,7 +404,7 @@ func (s *ServiceAccountsStoreImpl) HideApiKeysTab(ctx context.Context, orgId int } func (s *ServiceAccountsStoreImpl) MigrateApiKeysToServiceAccounts(ctx context.Context, orgId int64) error { - basicKeys := s.sqlStore.GetAllAPIKeys(ctx, orgId) + basicKeys := s.apiKeyService.GetAllAPIKeys(ctx, orgId) if len(basicKeys) > 0 { for _, key := range basicKeys { err := s.CreateServiceAccountFromApikey(ctx, key) @@ -419,7 +422,7 @@ func (s *ServiceAccountsStoreImpl) MigrateApiKeysToServiceAccounts(ctx context.C } func (s *ServiceAccountsStoreImpl) MigrateApiKey(ctx context.Context, orgId int64, keyId int64) error { - basicKeys := s.sqlStore.GetAllAPIKeys(ctx, orgId) + basicKeys := s.apiKeyService.GetAllAPIKeys(ctx, orgId) if len(basicKeys) == 0 { return fmt.Errorf("no API keys to convert found") } @@ -465,7 +468,7 @@ func (s *ServiceAccountsStoreImpl) CreateServiceAccountFromApikey(ctx context.Co // RevertApiKey converts service account token to old API key func (s *ServiceAccountsStoreImpl) RevertApiKey(ctx context.Context, saId int64, keyId int64) error { query := models.GetApiKeyByIdQuery{ApiKeyId: keyId} - if err := s.sqlStore.GetApiKeyById(ctx, &query); err != nil { + if err := s.apiKeyService.GetApiKeyById(ctx, &query); err != nil { return err } key := query.Result diff --git a/pkg/services/serviceaccounts/database/database_test.go b/pkg/services/serviceaccounts/database/database_test.go index 1815e71c97a..62a3159f71f 100644 --- a/pkg/services/serviceaccounts/database/database_test.go +++ b/pkg/services/serviceaccounts/database/database_test.go @@ -7,6 +7,7 @@ import ( "github.com/grafana/grafana/pkg/infra/kvstore" "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/apikey/apikeyimpl" "github.com/grafana/grafana/pkg/services/serviceaccounts" "github.com/grafana/grafana/pkg/services/serviceaccounts/tests" "github.com/grafana/grafana/pkg/services/sqlstore" @@ -105,8 +106,9 @@ func TestStore_DeleteServiceAccount(t *testing.T) { func setupTestDatabase(t *testing.T) (*sqlstore.SQLStore, *ServiceAccountsStoreImpl) { t.Helper() db := sqlstore.InitTestDB(t) + apiKeyService := apikeyimpl.ProvideService(db, db.Cfg) kvStore := kvstore.ProvideService(db) - return db, ProvideServiceAccountsStore(db, kvStore) + return db, ProvideServiceAccountsStore(db, apiKeyService, kvStore) } func TestStore_RetrieveServiceAccount(t *testing.T) { @@ -331,7 +333,7 @@ func TestStore_RevertApiKey(t *testing.T) { // Service account should be deleted require.Equal(t, int64(0), serviceAccounts.TotalCount) - apiKeys := store.sqlStore.GetAllAPIKeys(context.Background(), 1) + apiKeys := store.apiKeyService.GetAllAPIKeys(context.Background(), 1) require.Len(t, apiKeys, 1) apiKey := apiKeys[0] require.Equal(t, c.key.Name, apiKey.Name) diff --git a/pkg/services/serviceaccounts/tests/common.go b/pkg/services/serviceaccounts/tests/common.go index e638aa2041f..3a15cd1d4c3 100644 --- a/pkg/services/serviceaccounts/tests/common.go +++ b/pkg/services/serviceaccounts/tests/common.go @@ -7,6 +7,7 @@ import ( "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/accesscontrol" accesscontrolmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock" + "github.com/grafana/grafana/pkg/services/apikey/apikeyimpl" "github.com/grafana/grafana/pkg/services/serviceaccounts" "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/services/user" @@ -61,7 +62,9 @@ func SetupApiKey(t *testing.T, sqlStore *sqlstore.SQLStore, testKey TestApiKey) } else { addKeyCmd.Key = "secret" } - err := sqlStore.AddAPIKey(context.Background(), addKeyCmd) + + apiKeyService := apikeyimpl.ProvideService(sqlStore, sqlStore.Cfg) + err := apiKeyService.AddAPIKey(context.Background(), addKeyCmd) require.NoError(t, err) if testKey.IsExpired { diff --git a/pkg/services/sqlstore/apikey.go b/pkg/services/sqlstore/apikey.go deleted file mode 100644 index f9730cde6ae..00000000000 --- a/pkg/services/sqlstore/apikey.go +++ /dev/null @@ -1,177 +0,0 @@ -package sqlstore - -import ( - "context" - "fmt" - "time" - - "xorm.io/xorm" - - "github.com/grafana/grafana/pkg/models" - "github.com/grafana/grafana/pkg/services/accesscontrol" -) - -// GetAPIKeys queries the database based -// on input on GetApiKeysQuery -func (ss *SQLStore) GetAPIKeys(ctx context.Context, query *models.GetApiKeysQuery) error { - return ss.WithDbSession(ctx, func(dbSession *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) - }) -} - -// GetAllAPIKeys queries the database for valid non SA APIKeys across all orgs -func (ss *SQLStore) GetAllAPIKeys(ctx context.Context, orgID int64) []*models.ApiKey { - result := make([]*models.ApiKey, 0) - err := ss.WithDbSession(ctx, func(dbSession *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 { - ss.log.Warn("API key not loaded", "err", err) - } - return result -} - -func (ss *SQLStore) DeleteApiKey(ctx context.Context, cmd *models.DeleteApiKeyCommand) error { - return ss.WithDbSession(ctx, func(sess *DBSession) error { - return deleteAPIKey(sess, cmd.Id, cmd.OrgId) - }) -} - -func deleteAPIKey(sess *DBSession, id, orgID int64) error { - rawSQL := "DELETE FROM api_key WHERE id=? and org_id=? and service_account_id IS NULL" - result, err := sess.Exec(rawSQL, id, orgID) - if err != nil { - return err - } - n, err := result.RowsAffected() - if err != nil { - return err - } else if n == 0 { - return models.ErrApiKeyNotFound - } - return nil -} - -// AddAPIKey adds the API key to the database. -func (ss *SQLStore) AddAPIKey(ctx context.Context, cmd *models.AddApiKeyCommand) error { - return ss.WithTransactionalDbSession(ctx, func(sess *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.WithDbSession(ctx, func(sess *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.WithDbSession(ctx, func(sess *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.WithDbSession(ctx, func(sess *DBSession) error { - has, err := sess.Table("api_key").Where(fmt.Sprintf("%s = ?", dialect.Quote("key")), hash).Get(&apikey) - if err != nil { - return err - } else if !has { - return models.ErrInvalidApiKey - } - - return nil - }) - - return &apikey, err -} - -// UpdateAPIKeyLastUsedDate updates the last used date of the API key to current time. -func (ss *SQLStore) UpdateAPIKeyLastUsedDate(ctx context.Context, tokenID int64) error { - now := timeNow() - return ss.WithDbSession(ctx, func(sess *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/sqlstore/apikey_test.go b/pkg/services/sqlstore/apikey_test.go deleted file mode 100644 index 64d75d37707..00000000000 --- a/pkg/services/sqlstore/apikey_test.go +++ /dev/null @@ -1,249 +0,0 @@ -package sqlstore - -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" -) - -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) { - ss := InitTestDB(t) - - 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) { - ss := InitTestDB(t) - - 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) { - store := InitTestDB(t, InitTestDBOpt{}) - 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/pkg/services/sqlstore/stats_test.go b/pkg/services/sqlstore/stats_test.go index 36a161bdf7d..f9e62523959 100644 --- a/pkg/services/sqlstore/stats_test.go +++ b/pkg/services/sqlstore/stats_test.go @@ -28,7 +28,7 @@ func TestIntegrationStatsDataAccess(t *testing.T) { assert.Equal(t, int64(3), query.Result.Admins) assert.Equal(t, int64(0), query.Result.LibraryPanels) assert.Equal(t, int64(0), query.Result.LibraryVariables) - assert.Equal(t, int64(1), query.Result.APIKeys) + assert.Equal(t, int64(0), query.Result.APIKeys) }) t.Run("Get system user count stats should not results in error", func(t *testing.T) { @@ -127,9 +127,4 @@ func populateDB(t *testing.T, sqlStore *SQLStore) { // force renewal of user stats err = sqlStore.updateUserRoleCountsIfNecessary(context.Background(), true) require.NoError(t, err) - - // add 1st api key - addAPIKeyCmd := &models.AddApiKeyCommand{OrgId: org.Id, Name: "Test key 1", Key: "secret-key", Role: models.ROLE_VIEWER} - err = sqlStore.AddAPIKey(context.Background(), addAPIKeyCmd) - require.NoError(t, err) } diff --git a/pkg/services/sqlstore/transactions_test.go b/pkg/services/sqlstore/transactions_test.go index 6d1a48ce9e8..709cd9cd055 100644 --- a/pkg/services/sqlstore/transactions_test.go +++ b/pkg/services/sqlstore/transactions_test.go @@ -2,60 +2,11 @@ package sqlstore import ( "context" - "errors" "testing" "github.com/stretchr/testify/require" - - "github.com/grafana/grafana/pkg/models" ) -var ErrProvokedError = errors.New("testing error") - -func TestIntegrationTransaction(t *testing.T) { - if testing.Short() { - t.Skip("skipping integration test") - } - ss := InitTestDB(t) - - cmd := &models.AddApiKeyCommand{Key: "secret-key", Name: "key", OrgId: 1} - t.Run("can update key", func(t *testing.T) { - err := ss.AddAPIKey(context.Background(), cmd) - require.Nil(t, err) - - err = ss.WithTransactionalDbSession(context.Background(), func(sess *DBSession) error { - return deleteAPIKey(sess, cmd.Result.Id, 1) - }) - - require.Nil(t, err) - - query := &models.GetApiKeyByIdQuery{ApiKeyId: cmd.Result.Id} - err = ss.GetApiKeyById(context.Background(), query) - require.Equal(t, models.ErrInvalidApiKey, err) - }) - - t.Run("won't update if one handler fails", func(t *testing.T) { - err := ss.AddAPIKey(context.Background(), cmd) - require.Nil(t, err) - - err = ss.WithTransactionalDbSession(context.Background(), func(sess *DBSession) error { - err := deleteAPIKey(sess, cmd.Result.Id, 1) - if err != nil { - return err - } - - return ErrProvokedError - }) - - require.Equal(t, ErrProvokedError, err) - - query := &models.GetApiKeyByIdQuery{ApiKeyId: cmd.Result.Id} - err = ss.GetApiKeyById(context.Background(), query) - require.Nil(t, err) - require.Equal(t, cmd.Result.Id, query.Result.Id) - }) -} - func TestIntegrationReuseSessionWithTransaction(t *testing.T) { if testing.Short() { t.Skip("skipping integration test")