Serviceaccounts: Add ability to add samename SA for different orgs (#83893)

* add ability to add samename SA for different orgs

* Update pkg/services/user/userimpl/user.go

* fix tests

* refactor name

* removed tests

* add migration

* fix linting
This commit is contained in:
Eric Leijonmarck
2024-03-06 09:53:58 +01:00
committed by GitHub
parent 2653bd8fab
commit e611a736ed
3 changed files with 102 additions and 15 deletions

View File

@ -42,10 +42,19 @@ func ProvideServiceAccountsStore(cfg *setting.Cfg, store db.DB, apiKeyService ap
}
}
// generateLogin makes a generated string to have a ID for the service account across orgs and it's name
// this causes you to create a service account with the same name in different orgs
// not the same name in the same org
func generateLogin(prefix string, orgId int64, name string) string {
generatedLogin := fmt.Sprintf("%v-%v-%v", prefix, orgId, strings.ToLower(name))
// in case the name has multiple spaces or dashes in the prefix or otherwise, replace them with a single dash
generatedLogin = strings.Replace(generatedLogin, "--", "-", 1)
return strings.Replace(generatedLogin, " ", "-", -1)
}
// CreateServiceAccount creates service account
func (s *ServiceAccountsStoreImpl) CreateServiceAccount(ctx context.Context, orgId int64, saForm *serviceaccounts.CreateServiceAccountForm) (*serviceaccounts.ServiceAccountDTO, error) {
generatedLogin := serviceaccounts.ServiceAccountPrefix + strings.ToLower(saForm.Name)
generatedLogin = strings.ReplaceAll(generatedLogin, " ", "-")
login := generateLogin(serviceaccounts.ServiceAccountPrefix, orgId, saForm.Name)
isDisabled := false
role := org.RoleViewer
if saForm.IsDisabled != nil {
@ -56,7 +65,7 @@ func (s *ServiceAccountsStoreImpl) CreateServiceAccount(ctx context.Context, org
}
newSA, err := s.userService.CreateServiceAccount(ctx, &user.CreateUserCommand{
Login: generatedLogin,
Login: login,
OrgID: orgId,
Name: saForm.Name,
IsDisabled: isDisabled,
@ -435,7 +444,7 @@ func (s *ServiceAccountsStoreImpl) MigrateApiKey(ctx context.Context, orgId int6
func (s *ServiceAccountsStoreImpl) CreateServiceAccountFromApikey(ctx context.Context, key *apikey.APIKey) error {
prefix := "sa-autogen"
cmd := user.CreateUserCommand{
Login: fmt.Sprintf("%v-%v-%v", prefix, key.OrgID, key.Name),
Login: generateLogin(prefix, key.OrgID, key.Name),
Name: fmt.Sprintf("%v-%v", prefix, key.Name),
OrgID: key.OrgID,
DefaultOrgRole: string(key.Role),

View File

@ -30,8 +30,8 @@ func TestMain(m *testing.M) {
// Service Account should not create an org on its own
func TestStore_CreateServiceAccountOrgNonExistant(t *testing.T) {
_, store := setupTestDatabase(t)
serviceAccountName := "new Service Account"
t.Run("create service account", func(t *testing.T) {
serviceAccountName := "new Service Account"
serviceAccountOrgId := int64(1)
serviceAccountRole := org.RoleAdmin
isDisabled := true
@ -47,13 +47,12 @@ func TestStore_CreateServiceAccountOrgNonExistant(t *testing.T) {
}
func TestStore_CreateServiceAccount(t *testing.T) {
_, store := setupTestDatabase(t)
orgQuery := &org.CreateOrgCommand{Name: orgimpl.MainOrgName}
orgResult, err := store.orgService.CreateWithMember(context.Background(), orgQuery)
require.NoError(t, err)
serviceAccountName := "new Service Account"
t.Run("create service account", func(t *testing.T) {
serviceAccountName := "new Service Account"
_, store := setupTestDatabase(t)
orgQuery := &org.CreateOrgCommand{Name: orgimpl.MainOrgName}
orgResult, err := store.orgService.CreateWithMember(context.Background(), orgQuery)
require.NoError(t, err)
serviceAccountOrgId := orgResult.ID
serviceAccountRole := org.RoleAdmin
isDisabled := true
@ -65,13 +64,11 @@ func TestStore_CreateServiceAccount(t *testing.T) {
saDTO, err := store.CreateServiceAccount(context.Background(), serviceAccountOrgId, &saForm)
require.NoError(t, err)
assert.Equal(t, "sa-new-service-account", saDTO.Login)
assert.Equal(t, serviceAccountName, saDTO.Name)
assert.Equal(t, 0, int(saDTO.Tokens))
retrieved, err := store.RetrieveServiceAccount(context.Background(), serviceAccountOrgId, saDTO.Id)
require.NoError(t, err)
assert.Equal(t, "sa-new-service-account", retrieved.Login)
assert.Equal(t, serviceAccountName, retrieved.Name)
assert.Equal(t, serviceAccountOrgId, retrieved.OrgId)
assert.Equal(t, string(serviceAccountRole), retrieved.Role)
@ -81,6 +78,82 @@ func TestStore_CreateServiceAccount(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, saDTO.Id, retrievedId)
})
t.Run("create service account twice same org, error", func(t *testing.T) {
_, store := setupTestDatabase(t)
orgQuery := &org.CreateOrgCommand{Name: orgimpl.MainOrgName}
orgResult, err := store.orgService.CreateWithMember(context.Background(), orgQuery)
require.NoError(t, err)
serviceAccountOrgId := orgResult.ID
serviceAccountRole := org.RoleAdmin
isDisabled := true
saForm := serviceaccounts.CreateServiceAccountForm{
Name: serviceAccountName,
Role: &serviceAccountRole,
IsDisabled: &isDisabled,
}
saDTO, err := store.CreateServiceAccount(context.Background(), serviceAccountOrgId, &saForm)
require.NoError(t, err)
assert.Equal(t, serviceAccountName, saDTO.Name)
assert.Equal(t, 0, int(saDTO.Tokens))
retrieved, err := store.RetrieveServiceAccount(context.Background(), serviceAccountOrgId, saDTO.Id)
require.NoError(t, err)
assert.Equal(t, serviceAccountName, retrieved.Name)
assert.Equal(t, serviceAccountOrgId, retrieved.OrgId)
assert.Equal(t, string(serviceAccountRole), retrieved.Role)
assert.True(t, retrieved.IsDisabled)
retrievedId, err := store.RetrieveServiceAccountIdByName(context.Background(), serviceAccountOrgId, serviceAccountName)
require.NoError(t, err)
assert.Equal(t, saDTO.Id, retrievedId)
// should not b able to create the same service account twice in the same org
_, err = store.CreateServiceAccount(context.Background(), serviceAccountOrgId, &saForm)
require.Error(t, err)
})
t.Run("create service account twice different orgs should work", func(t *testing.T) {
_, store := setupTestDatabase(t)
orgQuery := &org.CreateOrgCommand{Name: orgimpl.MainOrgName}
orgResult, err := store.orgService.CreateWithMember(context.Background(), orgQuery)
require.NoError(t, err)
serviceAccountOrgId := orgResult.ID
serviceAccountRole := org.RoleAdmin
isDisabled := true
saForm := serviceaccounts.CreateServiceAccountForm{
Name: serviceAccountName,
Role: &serviceAccountRole,
IsDisabled: &isDisabled,
}
saDTO, err := store.CreateServiceAccount(context.Background(), serviceAccountOrgId, &saForm)
require.NoError(t, err)
assert.Equal(t, serviceAccountName, saDTO.Name)
assert.Equal(t, 0, int(saDTO.Tokens))
retrieved, err := store.RetrieveServiceAccount(context.Background(), serviceAccountOrgId, saDTO.Id)
require.NoError(t, err)
assert.Equal(t, serviceAccountName, retrieved.Name)
assert.Equal(t, serviceAccountOrgId, retrieved.OrgId)
assert.Equal(t, string(serviceAccountRole), retrieved.Role)
assert.True(t, retrieved.IsDisabled)
retrievedId, err := store.RetrieveServiceAccountIdByName(context.Background(), serviceAccountOrgId, serviceAccountName)
require.NoError(t, err)
assert.Equal(t, saDTO.Id, retrievedId)
orgQuerySecond := &org.CreateOrgCommand{Name: "Second Org name"}
orgResultSecond, err := store.orgService.CreateWithMember(context.Background(), orgQuerySecond)
require.NoError(t, err)
serviceAccountOrgIdSecond := orgResultSecond.ID
// should not b able to create the same service account twice in the same org
saDTOSecond, err := store.CreateServiceAccount(context.Background(), serviceAccountOrgIdSecond, &saForm)
require.NoError(t, err)
assert.Equal(t, serviceAccountName, saDTOSecond.Name)
assert.Equal(t, 0, int(saDTOSecond.Tokens))
})
}
func TestStore_CreateServiceAccountRoleNone(t *testing.T) {
@ -100,13 +173,11 @@ func TestStore_CreateServiceAccountRoleNone(t *testing.T) {
saDTO, err := store.CreateServiceAccount(context.Background(), serviceAccountOrgId, &saForm)
require.NoError(t, err)
assert.Equal(t, "sa-new-service-account", saDTO.Login)
assert.Equal(t, serviceAccountName, saDTO.Name)
assert.Equal(t, 0, int(saDTO.Tokens))
retrieved, err := store.RetrieveServiceAccount(context.Background(), serviceAccountOrgId, saDTO.Id)
require.NoError(t, err)
assert.Equal(t, "sa-new-service-account", retrieved.Login)
assert.Equal(t, serviceAccountName, retrieved.Name)
assert.Equal(t, serviceAccountOrgId, retrieved.OrgId)
assert.Equal(t, string(serviceAccountRole), retrieved.Role)

View File

@ -153,6 +153,13 @@ func addUserMigrations(mg *Migrator) {
mg.AddMigration("Add unique index user_uid", NewAddIndexMigration(userV2, &Index{
Cols: []string{"uid"}, Type: UniqueIndex,
}))
// Service accounts login were not unique per org. this migration is part of making it unique per org
// to be able to create service accounts that are unique per org
mg.AddMigration("Update login field for service accounts", NewRawSQLMigration("").
SQLite("UPDATE user SET login = 'sa-' || CAST(org_id AS TEXT) || '-' || REPLACE(login, 'sa-', '') WHERE login IS NOT NULL AND is_service_account = 1;").
Postgres("UPDATE \"user\" SET login = 'sa-' || org_id::text || '-' || REPLACE(login, 'sa-', '') WHERE login IS NOT NULL AND is_service_account = true;").
Mysql("UPDATE user SET login = CONCAT('sa-', CAST(org_id AS CHAR), '-', REPLACE(login, 'sa-', '')) WHERE login IS NOT NULL AND is_service_account = 1;"))
}
const migSQLITEisServiceAccountNullable = `ALTER TABLE user ADD COLUMN tmp_service_account BOOLEAN DEFAULT 0;