Service accounts: Split user and service account database (#46442)

* ServiceAccounts: remove unused endpoint

* ServiceAccounts: remove usage of getOrgUsers from service accounts

* use dialect for boolean str true in delete

* return service account results directly

* Move Service Account Deletions to sa package

Co-authored-by: Gabriel MABILLE <gamab@users.noreply.github.com>
Co-authored-by: gamab <gabi.mabs@gmail.com>

* Move service account methods to service accounts

Co-authored-by: Gabriel MABILLE <gamab@users.noreply.github.com>
Co-authored-by: gamab <gabi.mabs@gmail.com>

* Service accounts should not interfere with users

Co-authored-by: Gabriel MABILLE <gamab@users.noreply.github.com>

* filter service accounts in user services

* mispell fix

* fix overextended lines

Co-authored-by: Gabriel MABILLE <gamab@users.noreply.github.com>

* fix variable

Co-authored-by: Gabriel MABILLE <gamab@users.noreply.github.com>
Co-authored-by: gamab <gabi.mabs@gmail.com>

pkg/services/sqlstore/apikey.go

@@ -41,13 +41,12 @@ 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 {
+// GetAllOrgsAPIKeys queries the database for valid non SA APIKeys across all orgs
+func (ss *SQLStore) GetAllOrgsAPIKeys(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")
+					Where("(expires IS NULL OR expires >= ?) AND service_account_id IS NULL", timeNow().Unix()).Asc("name")
 		return sess.Find(&result)
 	})
 	if err != nil {
@@ -114,30 +113,6 @@ 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/mockstore/mockstore.go

@@ -136,10 +136,6 @@ func (m *SQLStoreMock) DeleteOldLoginAttempts(ctx context.Context, cmd *models.D
 	return m.ExpectedError
 }
 
-func (m *SQLStoreMock) CreateServiceAccountForApikey(ctx context.Context, orgId int64, keyname string, role models.RoleType) (*models.User, error) {
-	return nil, m.ExpectedError
-}
-
 func (m *SQLStoreMock) CreateUser(ctx context.Context, cmd models.CreateUserCommand) (*models.User, error) {
 	return nil, m.ExpectedError
 }
@@ -595,7 +591,7 @@ func (m *SQLStoreMock) GetAPIKeys(ctx context.Context, query *models.GetApiKeysQ
 	return m.ExpectedError
 }
 
-func (m *SQLStoreMock) GetNonServiceAccountAPIKeys(ctx context.Context) []*models.ApiKey {
+func (m *SQLStoreMock) GetAllOrgsAPIKeys(ctx context.Context) []*models.ApiKey {
 	return nil
 }
 
@@ -607,10 +603,6 @@ func (m *SQLStoreMock) AddAPIKey(ctx context.Context, cmd *models.AddApiKeyComma
 	return m.ExpectedError
 }
 
-func (m *SQLStoreMock) UpdateApikeyServiceAccount(ctx context.Context, apikeyId int64, saccountId int64) error {
-	return m.ExpectedError
-}
-
 func (m *SQLStoreMock) GetApiKeyById(ctx context.Context, query *models.GetApiKeyByIdQuery) error {
 	return m.ExpectedError
 }

pkg/services/sqlstore/org_users.go

@@ -24,7 +24,7 @@ func (ss *SQLStore) AddOrgUser(ctx context.Context, cmd *models.AddOrgUserComman
 	return ss.WithTransactionalDbSession(ctx, func(sess *DBSession) error {
 		// check if user exists
 		var user models.User
-		if exists, err := sess.ID(cmd.UserId).Get(&user); err != nil {
+		if exists, err := sess.ID(cmd.UserId).Where(notServiceAccountFilter(ss)).Get(&user); err != nil {
 			return err
 		} else if !exists {
 			return models.ErrUserNotFound
@@ -114,9 +114,8 @@ func (ss *SQLStore) GetOrgUsers(ctx context.Context, query *models.GetOrgUsersQu
 		whereParams = append(whereParams, query.UserID)
 	}
 
-	// TODO: add to chore, for cleaning up after we have created
-	// service accounts table in the modelling
-	whereConditions = append(whereConditions, fmt.Sprintf("%s.is_service_account = %t", x.Dialect().Quote("user"), query.IsServiceAccount))
+	whereConditions = append(whereConditions, fmt.Sprintf("%s.is_service_account = ?", dialect.Quote("user")))
+	whereParams = append(whereParams, dialect.BooleanStr(false))
 
 	if ss.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagAccesscontrol) && query.User != nil {
 		acFilter, err := accesscontrol.Filter(query.User, "org_user.user_id", "users", accesscontrol.ActionOrgUsersRead)
@@ -179,9 +178,7 @@ func (ss *SQLStore) SearchOrgUsers(ctx context.Context, query *models.SearchOrgU
 	whereConditions = append(whereConditions, "org_user.org_id = ?")
 	whereParams = append(whereParams, query.OrgID)
 
-	// TODO: add to chore, for cleaning up after we have created
-	// service accounts table in the modelling
-	whereConditions = append(whereConditions, fmt.Sprintf("%s.is_service_account = %t", x.Dialect().Quote("user"), query.IsServiceAccount))
+	whereConditions = append(whereConditions, fmt.Sprintf("%s.is_service_account = %s", x.Dialect().Quote("user"), ss.Dialect.BooleanStr(false)))
 
 	if ss.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagAccesscontrol) {
 		acFilter, err := accesscontrol.Filter(query.User, "org_user.user_id", "users", accesscontrol.ActionOrgUsersRead)
@@ -248,7 +245,7 @@ func (ss *SQLStore) RemoveOrgUser(ctx context.Context, cmd *models.RemoveOrgUser
 	return ss.WithTransactionalDbSession(ctx, func(sess *DBSession) error {
 		// check if user exists
 		var user models.User
-		if exists, err := sess.ID(cmd.UserId).Get(&user); err != nil {
+		if exists, err := sess.ID(cmd.UserId).Where(notServiceAccountFilter(ss)).Get(&user); err != nil {
 			return err
 		} else if !exists {
 			return models.ErrUserNotFound
@@ -301,7 +298,7 @@ func (ss *SQLStore) RemoveOrgUser(ctx context.Context, cmd *models.RemoveOrgUser
 			}
 		} else if cmd.ShouldDeleteOrphanedUser {
 			// no other orgs, delete the full user
-			if err := deleteUserInTransaction(sess, &models.DeleteUserCommand{UserId: user.Id}); err != nil {
+			if err := deleteUserInTransaction(ss, sess, &models.DeleteUserCommand{UserId: user.Id}); err != nil {
 				return err
 			}
 
@@ -312,8 +309,8 @@ func (ss *SQLStore) RemoveOrgUser(ctx context.Context, cmd *models.RemoveOrgUser
 	})
 }
 
+// validate that there is an org admin user left
 func validateOneAdminLeftInOrg(orgId int64, sess *DBSession) error {
-	// validate that there is an admin user left
 	res, err := sess.Query("SELECT 1 from org_user WHERE org_id=? and role='Admin'", orgId)
 	if err != nil {
 		return err

pkg/services/sqlstore/store.go

@@ -28,7 +28,6 @@ type Store interface {
 	GetOrgByNameHandler(ctx context.Context, query *models.GetOrgByNameQuery) error
 	CreateLoginAttempt(ctx context.Context, cmd *models.CreateLoginAttemptCommand) error
 	DeleteOldLoginAttempts(ctx context.Context, cmd *models.DeleteOldLoginAttemptsCommand) error
-	CreateServiceAccountForApikey(ctx context.Context, orgId int64, keyname string, role models.RoleType) (*models.User, error)
 	CreateUser(ctx context.Context, cmd models.CreateUserCommand) (*models.User, error)
 	GetUserById(ctx context.Context, query *models.GetUserByIdQuery) error
 	GetUserByLogin(ctx context.Context, query *models.GetUserByLoginQuery) error
@@ -134,10 +133,9 @@ type Store interface {
 	SetAlertNotificationStateToPendingCommand(ctx context.Context, cmd *models.SetAlertNotificationStateToPendingCommand) error
 	GetOrCreateAlertNotificationState(ctx context.Context, cmd *models.GetOrCreateNotificationStateQuery) error
 	GetAPIKeys(ctx context.Context, query *models.GetApiKeysQuery) error
-	GetNonServiceAccountAPIKeys(ctx context.Context) []*models.ApiKey
+	GetAllOrgsAPIKeys(ctx context.Context) []*models.ApiKey
 	DeleteApiKey(ctx context.Context, cmd *models.DeleteApiKeyCommand) error
 	AddAPIKey(ctx context.Context, cmd *models.AddApiKeyCommand) error
-	UpdateApikeyServiceAccount(ctx context.Context, apikeyId int64, saccountId int64) error
 	GetApiKeyById(ctx context.Context, query *models.GetApiKeyByIdQuery) error
 	GetApiKeyByName(ctx context.Context, query *models.GetApiKeyByNameQuery) error
 	UpdateTempUserStatus(ctx context.Context, cmd *models.UpdateTempUserStatusCommand) error

pkg/services/sqlstore/user.go

@@ -184,24 +184,6 @@ func (ss *SQLStore) createUser(ctx context.Context, sess *DBSession, args userCr
 	return user, nil
 }
 
-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
-}
-
 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 {
@@ -300,10 +282,19 @@ func (ss *SQLStore) CreateUser(ctx context.Context, cmd models.CreateUserCommand
 	return user, err
 }
 
-func (ss SQLStore) GetUserById(ctx context.Context, query *models.GetUserByIdQuery) error {
+func notServiceAccountFilter(ss *SQLStore) string {
+	return fmt.Sprintf("%s.is_service_account = %s",
+		ss.Dialect.Quote("user"),
+		ss.Dialect.BooleanStr(false))
+}
+
+func (ss *SQLStore) GetUserById(ctx context.Context, query *models.GetUserByIdQuery) error {
 	return ss.WithDbSession(ctx, func(sess *DBSession) error {
 		user := new(models.User)
-		has, err := sess.ID(query.Id).Get(user)
+
+		has, err := sess.ID(query.Id).
+			Where(notServiceAccountFilter(ss)).
+			Get(user)
 
 		if err != nil {
 			return err
@@ -326,7 +317,7 @@ func (ss *SQLStore) GetUserByLogin(ctx context.Context, query *models.GetUserByL
 		// Try and find the user by login first.
 		// It's not sufficient to assume that a LoginOrEmail with an "@" is an email.
 		user := &models.User{Login: query.LoginOrEmail}
-		has, err := sess.Get(user)
+		has, err := sess.Where(notServiceAccountFilter(ss)).Get(user)
 
 		if err != nil {
 			return err
@@ -358,7 +349,7 @@ func (ss *SQLStore) GetUserByEmail(ctx context.Context, query *models.GetUserByE
 		}
 
 		user := &models.User{Email: query.Email}
-		has, err := sess.Get(user)
+		has, err := sess.Where(notServiceAccountFilter(ss)).Get(user)
 
 		if err != nil {
 			return err
@@ -382,7 +373,7 @@ func (ss *SQLStore) UpdateUser(ctx context.Context, cmd *models.UpdateUserComman
 			Updated: time.Now(),
 		}
 
-		if _, err := sess.ID(cmd.UserId).Update(&user); err != nil {
+		if _, err := sess.ID(cmd.UserId).Where(notServiceAccountFilter(ss)).Update(&user); err != nil {
 			return err
 		}
 
@@ -405,7 +396,7 @@ func (ss *SQLStore) ChangeUserPassword(ctx context.Context, cmd *models.ChangeUs
 			Updated:  time.Now(),
 		}
 
-		_, err := sess.ID(cmd.UserId).Update(&user)
+		_, err := sess.ID(cmd.UserId).Where(notServiceAccountFilter(ss)).Update(&user)
 		return err
 	})
 }
@@ -456,7 +447,7 @@ func setUsingOrgInTransaction(sess *DBSession, userID int64, orgID int64) error
 func (ss *SQLStore) GetUserProfile(ctx context.Context, query *models.GetUserProfileQuery) error {
 	return ss.WithDbSession(ctx, func(sess *DBSession) error {
 		var user models.User
-		has, err := sess.ID(query.UserId).Get(&user)
+		has, err := sess.ID(query.UserId).Where(notServiceAccountFilter(ss)).Get(&user)
 
 		if err != nil {
 			return err
@@ -506,7 +497,9 @@ func (ss *SQLStore) GetUserOrgList(ctx context.Context, query *models.GetUserOrg
 	query.Result = make([]*models.UserOrgDTO, 0)
 	sess := x.Table("org_user")
 	sess.Join("INNER", "org", "org_user.org_id=org.id")
+	sess.Join("INNER", x.Dialect().Quote("user"), fmt.Sprintf("org_user.user_id=%s.id", x.Dialect().Quote("user")))
 	sess.Where("org_user.user_id=?", query.UserId)
+	sess.Where(notServiceAccountFilter(ss))
 	sess.Cols("org.name", "org_user.role", "org_user.org_id")
 	sess.OrderBy("org.name")
 	err := sess.Find(&query.Result)
@@ -612,8 +605,6 @@ func SearchUsers(ctx context.Context, query *models.SearchUsersQuery) error {
 	whereParams := make([]interface{}, 0)
 	sess := x.Table("user").Alias("u")
 
-	// TODO: add to chore, for cleaning up after we have created
-	// service accounts table in the modelling
 	whereConditions = append(whereConditions, "u.is_service_account = ?")
 	whereParams = append(whereParams, dialect.BooleanStr(false))
 
@@ -710,7 +701,7 @@ func (ss *SQLStore) DisableUser(ctx context.Context, cmd *models.DisableUserComm
 	user := models.User{}
 	sess := x.Table("user")
 
-	if has, err := sess.ID(cmd.UserId).Get(&user); err != nil {
+	if has, err := sess.ID(cmd.UserId).Where(notServiceAccountFilter(ss)).Get(&user); err != nil {
 		return err
 	} else if !has {
 		return models.ErrUserNotFound
@@ -739,32 +730,28 @@ func (ss *SQLStore) BatchDisableUsers(ctx context.Context, cmd *models.BatchDisa
 			disableParams = append(disableParams, v)
 		}
 
-		_, err := sess.Exec(disableParams...)
-		if err != nil {
-			return err
-		}
-
-		return nil
+		_, err := sess.Where(notServiceAccountFilter(ss)).Exec(disableParams...)
+		return err
 	})
 }
 
 func (ss *SQLStore) DeleteUser(ctx context.Context, cmd *models.DeleteUserCommand) error {
 	return ss.WithTransactionalDbSession(ctx, func(sess *DBSession) error {
-		return deleteUserInTransaction(sess, cmd)
+		return deleteUserInTransaction(ss, sess, cmd)
 	})
 }
 
-func deleteUserInTransaction(sess *DBSession, cmd *models.DeleteUserCommand) error {
+func deleteUserInTransaction(ss *SQLStore, sess *DBSession, cmd *models.DeleteUserCommand) error {
 	// Check if user exists
 	user := models.User{Id: cmd.UserId}
-	has, err := sess.Get(&user)
+	has, err := sess.Where(notServiceAccountFilter(ss)).Get(&user)
 	if err != nil {
 		return err
 	}
 	if !has {
 		return models.ErrUserNotFound
 	}
-	for _, sql := range userDeletions() {
+	for _, sql := range UserDeletions() {
 		_, err := sess.Exec(sql, cmd.UserId)
 		if err != nil {
 			return err
@@ -773,7 +760,7 @@ func deleteUserInTransaction(sess *DBSession, cmd *models.DeleteUserCommand) err
 	return nil
 }
 
-func userDeletions() []string {
+func UserDeletions() []string {
 	deletes := []string{
 		"DELETE FROM star WHERE user_id = ?",
 		"DELETE FROM " + dialect.Quote("user") + " WHERE id = ?",
@@ -788,18 +775,11 @@ func userDeletions() []string {
 	return deletes
 }
 
-func ServiceAccountDeletions() []string {
-	deletes := []string{
-		"DELETE FROM api_key WHERE service_account_id = ?",
-	}
-	deletes = append(deletes, userDeletions()...)
-	return deletes
-}
-
+// UpdateUserPermissions sets the user Server Admin flag
 func (ss *SQLStore) UpdateUserPermissions(userID int64, isAdmin bool) error {
 	return ss.WithTransactionalDbSession(context.Background(), func(sess *DBSession) error {
 		var user models.User
-		if _, err := sess.ID(userID).Get(&user); err != nil {
+		if _, err := sess.ID(userID).Where(notServiceAccountFilter(ss)).Get(&user); err != nil {
 			return err
 		}
 
@@ -833,8 +813,8 @@ func (ss *SQLStore) SetUserHelpFlag(ctx context.Context, cmd *models.SetUserHelp
 	})
 }
 
+// validateOneAdminLeft validate that there is an admin user left
 func validateOneAdminLeft(sess *DBSession) error {
-	// validate that there is an admin user left
 	count, err := sess.Where("is_admin=?", true).Count(&models.User{})
 	if err != nil {
 		return err