AccessControl: upgrade apikeys by adding service accounts (#42425)

Co-authored-by: Eric Leijonmarck <eric.leijonmarck@gmail.com>

Co-authored-by: Emil Tullstedt <emil.tullstedt@grafana.com>

* Change default options for cloned service account

* Run in background

* Add endpoint to upgrade api keys to service accounts

pkg/api/apikey.go

@@ -85,6 +85,7 @@ func (hs *HTTPServer) AddAPIKey(c *models.ReqContext) response.Response {
 			//Create a new service account for the new API key
 			serviceAccount, err := hs.SQLStore.CloneUserToServiceAccount(c.Req.Context(), c.SignedInUser)
 			if err != nil {
+				hs.log.Warn("Unable to clone user to service account", "err", err)
 				return response.Error(500, "Unable to clone user to service account", err)
 			}
 			cmd.ServiceAccountId = serviceAccount.Id
@@ -95,10 +96,12 @@ func (hs *HTTPServer) AddAPIKey(c *models.ReqContext) response.Response {
 			query := models.GetUserByIdQuery{Id: cmd.ServiceAccountId}
 			err = bus.DispatchCtx(c.Req.Context(), &query)
 			if err != nil {
-				return response.Error(500, "Unable to clone user to service account", err)
+				hs.log.Warn("Unable to link new API key to existing service account", "err", err, "query", query)
+				return response.Error(500, "Unable to link new API key to existing service account", err)
 			}
 			serviceAccountDetails := query.Result
 			if serviceAccountDetails.OrgId != c.OrgId || serviceAccountDetails.OrgId != cmd.OrgId {
+				hs.log.Warn("Target service is not in the same organisation as requesting user or api key", "err", err, "reqOrg", cmd.OrgId, "serviceAccId", serviceAccountDetails.OrgId, "userOrgId", c.OrgId)
 				return response.Error(403, "Target service is not in the same organisation as requesting user or api key", err)
 			}
 		}

pkg/services/serviceaccounts/api/api.go

@@ -19,17 +19,20 @@ type ServiceAccountsAPI struct {
 	service        serviceaccounts.Service
 	accesscontrol  accesscontrol.AccessControl
 	RouterRegister routing.RouteRegister
+	store          serviceaccounts.Store
 }
 
 func NewServiceAccountsAPI(
 	service serviceaccounts.Service,
 	accesscontrol accesscontrol.AccessControl,
 	routerRegister routing.RouteRegister,
+	store serviceaccounts.Store,
 ) *ServiceAccountsAPI {
 	return &ServiceAccountsAPI{
 		service:        service,
 		accesscontrol:  accesscontrol,
 		RouterRegister: routerRegister,
+		store:          store,
 	}
 }
 
@@ -42,6 +45,7 @@ func (api *ServiceAccountsAPI) RegisterAPIEndpoints(
 	auth := acmiddleware.Middleware(api.accesscontrol)
 	api.RouterRegister.Group("/api/serviceaccounts", func(serviceAccountsRoute routing.RouteRegister) {
 		serviceAccountsRoute.Delete("/:serviceAccountId", auth(middleware.ReqOrgAdmin, accesscontrol.EvalPermission(serviceaccounts.ActionDelete, serviceaccounts.ScopeID)), routing.Wrap(api.DeleteServiceAccount))
+		serviceAccountsRoute.Get("/upgrade", auth(middleware.ReqOrgAdmin, accesscontrol.EvalPermission(serviceaccounts.ActionCreate, serviceaccounts.ScopeID)), routing.Wrap(api.UpgradeServiceAccounts))
 		serviceAccountsRoute.Post("/", auth(middleware.ReqOrgAdmin, accesscontrol.EvalPermission(serviceaccounts.ActionCreate, serviceaccounts.ScopeID)), routing.Wrap(api.CreateServiceAccount))
 	})
 }
@@ -71,3 +75,11 @@ func (api *ServiceAccountsAPI) DeleteServiceAccount(ctx *models.ReqContext) resp
 	}
 	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")
+	} else {
+		return response.Error(500, "Internal server error", err)
+	}
+}

pkg/services/serviceaccounts/api/api_test.go

@@ -18,6 +18,8 @@ import (
 	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/web"
 	"github.com/stretchr/testify/require"
+
+	"github.com/grafana/grafana/pkg/services/serviceaccounts/database"
 )
 
 var (
@@ -93,11 +95,8 @@ func serviceAccountDeletionScenario(t *testing.T, httpMethod string, endpoint st
 }
 
 func setupTestServer(t *testing.T, svc *tests.ServiceAccountMock, routerRegister routing.RouteRegister, acmock *accesscontrolmock.Mock) *web.Mux {
-	a := NewServiceAccountsAPI(
-		svc,
-		acmock,
-		routerRegister,
-	)
+	store := sqlstore.InitTestDB(t)
+	a := NewServiceAccountsAPI(svc, acmock, routerRegister, database.NewServiceAccountsStore(store))
 	a.RegisterAPIEndpoints(&setting.Cfg{FeatureToggles: map[string]bool{"service-accounts": true}})
 
 	m := web.New()

pkg/services/serviceaccounts/database/database.go

@@ -2,16 +2,18 @@ package database
 
 import (
 	"context"
+	"fmt"
 
 	"github.com/google/uuid"
+	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/services/serviceaccounts"
 	"github.com/grafana/grafana/pkg/services/sqlstore"
-	"github.com/pkg/errors"
 )
 
 type ServiceAccountsStoreImpl struct {
 	sqlStore *sqlstore.SQLStore
+	log      log.Logger
 }
 
 func NewServiceAccountsStore(store *sqlstore.SQLStore) *ServiceAccountsStoreImpl {
@@ -30,7 +32,7 @@ func (s *ServiceAccountsStoreImpl) CreateServiceAccount(ctx context.Context, sa
 	}
 	newuser, err := s.sqlStore.CreateUser(ctx, cmd)
 	if err != nil {
-		return nil, errors.Errorf("Failed to create user: %v", err)
+		return nil, fmt.Errorf("failed to create user: %v", err)
 	}
 	return newuser, nil
 }
@@ -58,3 +60,27 @@ func deleteServiceAccountInTransaction(sess *sqlstore.DBSession, orgID, serviceA
 	}
 	return nil
 }
+
+func (s *ServiceAccountsStoreImpl) UpgradeServiceAccounts(ctx context.Context) error {
+	basicKeys := s.sqlStore.GetNonServiceAccountAPIKeys(ctx)
+	if len(basicKeys) > 0 {
+		s.log.Info("Launching background thread to upgrade API keys to service accounts", "numberKeys", len(basicKeys))
+		go func() {
+			for _, key := range basicKeys {
+				sa, err := s.sqlStore.CreateServiceAccountForApikey(ctx, key.OrgId, key.Name, key.Role)
+				if err != nil {
+					s.log.Error("Failed to create service account for API key", "err", err, "keyId", key.Id)
+					continue
+				}
+
+				err = s.sqlStore.UpdateApikeyServiceAccount(ctx, key.Id, sa.Id)
+				if err != nil {
+					s.log.Error("Failed to attach new service account to API key", "err", err, "keyId", key.Id, "newServiceAccountId", sa.Id)
+					continue
+				}
+				s.log.Debug("Updated basic api key", "keyId", key.Id, "newServiceAccountId", sa.Id)
+			}
+		}()
+	}
+	return nil
+}

pkg/services/serviceaccounts/manager/service.go

@@ -38,8 +38,9 @@ func ProvideServiceAccountsService(
 	if err := ac.DeclareFixedRoles(role); err != nil {
 		return nil, err
 	}
-	serviceaccountsAPI := api.NewServiceAccountsAPI(s, ac, routeRegister)
+	serviceaccountsAPI := api.NewServiceAccountsAPI(s, ac, routeRegister, s.store)
 	serviceaccountsAPI.RegisterAPIEndpoints(cfg)
+
 	return s, nil
 }
 

pkg/services/serviceaccounts/serviceaccounts.go

@@ -13,4 +13,5 @@ type Service interface {
 type Store interface {
 	CreateServiceAccount(ctx context.Context, saForm *CreateServiceaccountForm) (*models.User, error)
 	DeleteServiceAccount(ctx context.Context, orgID, serviceAccountID int64) error
+	UpgradeServiceAccounts(ctx context.Context) error
 }

pkg/services/serviceaccounts/tests/common.go

@@ -52,8 +52,9 @@ func SetupMockAccesscontrol(t *testing.T, userpermissionsfunc func(c context.Con
 var _ serviceaccounts.Store = new(ServiceAccountsStoreMock)
 
 type Calls struct {
-	CreateServiceAccount []interface{}
-	DeleteServiceAccount []interface{}
+	CreateServiceAccount   []interface{}
+	DeleteServiceAccount   []interface{}
+	UpgradeServiceAccounts []interface{}
 }
 
 type ServiceAccountsStoreMock struct {
@@ -71,3 +72,8 @@ func (s *ServiceAccountsStoreMock) DeleteServiceAccount(ctx context.Context, org
 	s.Calls.DeleteServiceAccount = append(s.Calls.DeleteServiceAccount, []interface{}{ctx, orgID, serviceAccountID})
 	return nil
 }
+
+func (s *ServiceAccountsStoreMock) UpgradeServiceAccounts(ctx context.Context) error {
+	s.Calls.DeleteServiceAccount = append(s.Calls.UpgradeServiceAccounts, []interface{}{ctx})
+	return nil
+}

pkg/services/sqlstore/apikey.go

@@ -4,9 +4,10 @@ import (
 	"context"
 	"time"
 
+	"xorm.io/xorm"
+
 	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/models"
-	"xorm.io/xorm"
 )
 
 func (ss *SQLStore) addAPIKeysQueryAndCommandHandlers() {
@@ -38,6 +39,21 @@ func (ss *SQLStore) GetAPIKeys(ctx context.Context, query *models.GetApiKeysQuer
 	})
 }
 
+// GetAPIKeys queries the database based
+// on input on GetApiKeysQuery
+func (ss *SQLStore) GetNonServiceAccountAPIKeys(ctx context.Context) []*models.ApiKey {
+	result := make([]*models.ApiKey, 0)
+	err := ss.WithDbSession(ctx, func(dbSession *DBSession) error {
+		sess := dbSession. //CHECK how many API keys do our clients have?  Can we load them all?
+					Where("(expires IS NULL OR expires >= ?) AND service_account_id < 1 ", timeNow().Unix()).Asc("name")
+		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)
@@ -96,6 +112,30 @@ func (ss *SQLStore) AddAPIKey(ctx context.Context, cmd *models.AddApiKeyCommand)
 	})
 }
 
+// UpdateApikeyServiceAccount sets a service account for an existing API key
+func (ss *SQLStore) UpdateApikeyServiceAccount(ctx context.Context, apikeyId int64, saccountId int64) error {
+	return ss.WithTransactionalDbSession(ctx, func(sess *DBSession) error {
+		key := models.ApiKey{Id: apikeyId}
+		exists, err := sess.Get(&key)
+		if err != nil {
+			ss.log.Warn("API key not loaded", "err", err)
+			return err
+		}
+		if !exists {
+			ss.log.Warn("API key not found", "err", err)
+			return models.ErrApiKeyNotFound
+		}
+		key.ServiceAccountId = saccountId
+
+		if _, err := sess.ID(key.Id).Update(&key); err != nil {
+			ss.log.Warn("Could not update api key", "err", err)
+			return err
+		}
+
+		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

pkg/services/sqlstore/user.go

@@ -9,12 +9,12 @@ import (
 	"time"
 
 	"github.com/google/uuid"
+
 	"github.com/grafana/grafana/pkg/bus"
 	"github.com/grafana/grafana/pkg/events"
 	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/util"
-	"github.com/pkg/errors"
 )
 
 func (ss *SQLStore) addUserQueryAndCommandHandlers() {
@@ -197,7 +197,26 @@ func (ss *SQLStore) CloneUserToServiceAccount(ctx context.Context, siUser *model
 
 	newuser, err := ss.CreateUser(ctx, cmd)
 	if err != nil {
-		return nil, errors.Errorf("Failed to create user: %v", err)
+		ss.log.Warn("user not cloned", "err", err)
+		return nil, fmt.Errorf("failed to create user: %w", err)
+	}
+
+	return newuser, err
+}
+
+func (ss *SQLStore) CreateServiceAccountForApikey(ctx context.Context, orgId int64, keyname string, role models.RoleType) (*models.User, error) {
+	prefix := "Service-Account-Autogen-"
+	cmd := models.CreateUserCommand{
+		Login:            fmt.Sprintf("%v-%v-%v", prefix, orgId, keyname),
+		Name:             prefix + keyname,
+		OrgId:            orgId,
+		DefaultOrgRole:   string(role),
+		IsServiceAccount: true,
+	}
+
+	newuser, err := ss.CreateUser(ctx, cmd)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create user: %w", err)
 	}
 
 	return newuser, err