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
2025-02-25 18:55:37 -06:00 · 2022-08-02 16:55:19 +02:00
parent 43955bdebd
commit 64488f6b90
19 changed files with 594 additions and 28 deletions
--- a/pkg/services/apikey/apikeyimpl/apikey.go
+++ 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)
+}
--- a/pkg/services/apikey/apikeyimpl/store.go
+++ 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
+	})
+}
--- a/pkg/services/apikey/apikeyimpl/store_test.go
+++ 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)
+	}
+}