Accesscontrol: Add additional API keys to service account, move cloneserviceaccount to sqlstore (#41189)

* Add additional api key, move cloneserviceaccount

* Remove TODOs, for now

* Error messages

* Linter

* Security check

* Add comments

* Take service account id from correct variable

* Update user.go

pkg/api/api.go

@@ -258,6 +258,7 @@ func (hs *HTTPServer) registerRoutes() {
 		apiRoute.Group("/auth/keys", func(keysRoute routing.RouteRegister) {
 			keysRoute.Get("/", routing.Wrap(GetAPIKeys))
 			keysRoute.Post("/", quota("api_key"), bind(models.AddApiKeyCommand{}), routing.Wrap(hs.AddAPIKey))
+			keysRoute.Post("/additional", quota("api_key"), bind(models.AddApiKeyCommand{}), routing.Wrap(hs.AdditionalAPIKey))
 			keysRoute.Delete("/:id", routing.Wrap(DeleteAPIKey))
 		}, reqOrgAdmin)
 

pkg/api/apikey.go

@@ -73,14 +73,28 @@ func (hs *HTTPServer) AddAPIKey(c *models.ReqContext, cmd models.AddApiKeyComman
 	}
 	cmd.OrgId = c.OrgId
 	var err error
-	var serviceAccount *models.User = &models.User{Id: -1}
 	if hs.Cfg.FeatureToggles["service-accounts"] {
+		//Every new API key must have an associated service account
 		if cmd.CreateNewServiceAccount {
-			serviceAccount, err = hs.AccessControl.CloneUserToServiceAccount(c.Req.Context(), c.SignedInUser)
+			//Create a new service account for the new API key
+			serviceAccount, err := hs.SQLStore.CloneUserToServiceAccount(c.Req.Context(), c.SignedInUser)
 			if err != nil {
 				return response.Error(500, "Unable to clone user to service account", err)
 			}
 			cmd.ServiceAccountId = serviceAccount.Id
+		} else {
+			//Link the new API key to an existing service account
+
+			//Check if user and service account are in the same org
+			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)
+			}
+			serviceAccountDetails := query.Result
+			if serviceAccountDetails.OrgId != c.OrgId || serviceAccountDetails.OrgId != cmd.OrgId {
+				return response.Error(403, "Target service is not in the same organisation as requesting user or api key", err)
+			}
 		}
 	}
 
@@ -109,3 +123,15 @@ func (hs *HTTPServer) AddAPIKey(c *models.ReqContext, cmd models.AddApiKeyComman
 
 	return response.JSON(200, result)
 }
+
+// AddAPIKey adds an additional API key to a service account
+func (hs *HTTPServer) AdditionalAPIKey(c *models.ReqContext, cmd models.AddApiKeyCommand) response.Response {
+	if !hs.Cfg.FeatureToggles["service-accounts"] {
+		return response.Error(500, "Requires services-accounts feature", errors.New("feature missing"))
+	}
+	if cmd.CreateNewServiceAccount {
+		return response.Error(500, "Can't create service account while adding additional API key", nil)
+	}
+
+	return hs.AddAPIKey(c, cmd)
+}

pkg/services/accesscontrol/accesscontrol.go

@@ -17,12 +17,6 @@ type AccessControl interface {
 	// GetUserRoles returns user roles.
 	GetUserRoles(ctx context.Context, user *models.SignedInUser) ([]*RoleDTO, error)
 
-	// CloneUserToServiceAccount Creates a new service account and assigns it the same roles as the user has
-	CloneUserToServiceAccount(ctx context.Context, user *models.SignedInUser) (*models.User, error)
-
-	// LinkAPIKeyToServiceAccount Connects an APIkey to a service account.  Multiple API keys may be linked to one account.
-	LinkAPIKeyToServiceAccount(ctx context.Context, ApiKey *models.ApiKey, serviceAccount *models.User) error
-
 	//IsDisabled returns if access control is enabled or not
 	IsDisabled() bool
 

pkg/services/accesscontrol/mock/mock.go

@@ -14,7 +14,6 @@ type fullAccessControl interface {
 }
 
 type Calls struct {
-	CloneUserToServiceAccount  []interface{}
 	Evaluate            []interface{}
 	GetUserPermissions  []interface{}
 	GetUserRoles        []interface{}
@@ -22,12 +21,9 @@ type Calls struct {
 	DeclareFixedRoles   []interface{}
 	GetUserBuiltInRoles []interface{}
 	RegisterFixedRoles  []interface{}
-	LinkAPIKeyToServiceAccount []interface{}
 }
 
 type Mock struct {
-	// Unless an override is provided, user will be returned by CloneUserToServiceAccount
-	createduser *models.User
 	// Unless an override is provided, permissions will be returned by GetUserPermissions
 	permissions []*accesscontrol.Permission
 	// Unless an override is provided, roles will be returned by GetUserRoles
@@ -41,8 +37,6 @@ type Mock struct {
 	Calls Calls
 
 	// Override functions
-	CloneUserToServiceAccountFunc  func(context.Context, *models.SignedInUser) (*models.User, error)
-	LinkAPIKeyToServiceAccountFunc func(context.Context, *models.ApiKey, *models.User) error
 	EvaluateFunc            func(context.Context, *models.SignedInUser, accesscontrol.Evaluator) (bool, error)
 	GetUserPermissionsFunc  func(context.Context, *models.SignedInUser) ([]*accesscontrol.Permission, error)
 	GetUserRolesFunc        func(context.Context, *models.SignedInUser) ([]*accesscontrol.RoleDTO, error)
@@ -119,26 +113,6 @@ func (m *Mock) GetUserRoles(ctx context.Context, user *models.SignedInUser) ([]*
 	return m.roles, nil
 }
 
-func (m *Mock) CloneUserToServiceAccount(ctx context.Context, user *models.SignedInUser) (*models.User, error) {
-	m.Calls.CloneUserToServiceAccount = append(m.Calls.CloneUserToServiceAccount, []interface{}{ctx, user})
-	// Use override if provided
-	if m.CloneUserToServiceAccountFunc != nil {
-		return m.CloneUserToServiceAccountFunc(ctx, user)
-	}
-	// Otherwise return the user
-	return m.createduser, nil
-}
-
-func (m *Mock) LinkAPIKeyToServiceAccount(ctx context.Context, apikey *models.ApiKey, service_account *models.User) error {
-	m.Calls.LinkAPIKeyToServiceAccount = append(m.Calls.LinkAPIKeyToServiceAccount, []interface{}{ctx, apikey, service_account})
-	// Use override if provided
-	if m.LinkAPIKeyToServiceAccountFunc != nil {
-		return m.LinkAPIKeyToServiceAccountFunc(ctx, apikey, service_account)
-	}
-	// Otherwise return the default
-	return nil
-}
-
 // Middleware checks if service disabled or not to switch to fallback authorization.
 // This mock return m.disabled unless an override is provided.
 func (m *Mock) IsDisabled() bool {

pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol.go

@@ -77,16 +77,6 @@ func (ac *OSSAccessControlService) GetUserRoles(ctx context.Context, user *model
 	return nil, errors.New("unsupported function") //OSS users will continue to use builtin roles via GetUserPermissions
 }
 
-// CloneUserToServiceAccount creates a service account with permissions based on a user
-func (ac *OSSAccessControlService) CloneUserToServiceAccount(ctx context.Context, user *models.SignedInUser) (*models.User, error) {
-	return nil, errors.New("clone user not implemented yet in service accounts") //Please switch on Enterprise to test this
-}
-
-// Link creates a service account with permissions based on a user
-func (ac *OSSAccessControlService) LinkAPIKeyToServiceAccount(context.Context, *models.ApiKey, *models.User) error {
-	return errors.New("link SA not implemented yet in service accounts") //Please switch on Enterprise to test this
-}
-
 // GetUserPermissions returns user permissions based on built-in roles
 func (ac *OSSAccessControlService) GetUserPermissions(ctx context.Context, user *models.SignedInUser) ([]*accesscontrol.Permission, error) {
 	timer := prometheus.NewTimer(metrics.MAccessPermissionsSummary)

pkg/services/sqlstore/user.go

@@ -8,11 +8,13 @@ import (
 	"strings"
 	"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() {
@@ -183,6 +185,24 @@ func (ss *SQLStore) createUser(ctx context.Context, sess *DBSession, args userCr
 	return user, nil
 }
 
+func (ss *SQLStore) CloneUserToServiceAccount(ctx context.Context, siUser *models.SignedInUser) (*models.User, error) {
+	cmd := models.CreateUserCommand{
+		Login:            "Service-Account-" + uuid.New().String(),
+		Email:            uuid.New().String(),
+		Password:         "Password-" + uuid.New().String(),
+		Name:             siUser.Name + "-Service-Account-" + uuid.New().String(),
+		OrgId:            siUser.OrgId,
+		IsServiceAccount: true,
+	}
+
+	newuser, err := ss.CreateUser(ctx, cmd)
+	if err != nil {
+		return nil, errors.Errorf("Failed to create user: %v", err)
+	}
+
+	return newuser, err
+}
+
 func (ss *SQLStore) CreateUser(ctx context.Context, cmd models.CreateUserCommand) (*models.User, error) {
 	var user *models.User
 	err := ss.WithTransactionalDbSession(ctx, func(sess *DBSession) error {