diff --git a/pkg/services/sqlstore/migrations/usermig/service_account_multiple_org_login_migrator.go b/pkg/services/sqlstore/migrations/usermig/service_account_multiple_org_login_migrator.go index d51cfacd5a6..0737091bda4 100644 --- a/pkg/services/sqlstore/migrations/usermig/service_account_multiple_org_login_migrator.go +++ b/pkg/services/sqlstore/migrations/usermig/service_account_multiple_org_login_migrator.go @@ -9,12 +9,16 @@ import ( const ( AllowSameLoginCrossOrgs = "update login field with orgid to allow for multiple service accounts with same name across orgs" + DedupOrgInLogin = "update service accounts login field orgid to appear only once" ) // 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 func AddServiceAccountsAllowSameLoginCrossOrgs(mg *migrator.Migrator) { mg.AddMigration(AllowSameLoginCrossOrgs, &ServiceAccountsSameLoginCrossOrgs{}) + // Before it was fixed, the previous migration introduced the org_id again in logins that already had it. + // This migration removes the duplicate org_id from the login. + mg.AddMigration(DedupOrgInLogin, &ServiceAccountsDeduplicateOrgInLogin{}) } var _ migrator.CodeMigration = new(ServiceAccountsSameLoginCrossOrgs) @@ -76,3 +80,48 @@ func (p *ServiceAccountsSameLoginCrossOrgs) Exec(sess *xorm.Session, mg *migrato } return err } + +type ServiceAccountsDeduplicateOrgInLogin struct { + migrator.MigrationBase +} + +func (p *ServiceAccountsDeduplicateOrgInLogin) SQL(dialect migrator.Dialect) string { + return "code migration" +} + +func (p *ServiceAccountsDeduplicateOrgInLogin) Exec(sess *xorm.Session, mg *migrator.Migrator) error { + dialect := mg.Dialect + var err error + + // var logins []Login + switch dialect.DriverName() { + case migrator.Postgres: + _, err = sess.Exec(` + UPDATE "user" + SET login = 'sa-' || org_id::text || SUBSTRING(login FROM LENGTH('sa-' || org_id::text || '-' || org_id::text)+1) + WHERE login IS NOT NULL + AND is_service_account = true + AND login LIKE 'sa-' || org_id::text || '-' || org_id::text || '-%'; + `) + case migrator.MySQL: + _, err = sess.Exec(` + UPDATE user + SET login = CONCAT('sa-', org_id, SUBSTRING(login, LENGTH(CONCAT('sa-', org_id, '-', org_id))+1)) + WHERE login IS NOT NULL + AND is_service_account = 1 + AND login LIKE CONCAT('sa-', org_id, '-', org_id, '-%'); + `) + case migrator.SQLite: + _, err = sess.Exec(` + UPDATE ` + dialect.Quote("user") + ` + SET login = 'sa-' || CAST(org_id AS TEXT) || SUBSTRING(login, LENGTH('sa-'||CAST(org_id AS TEXT)||'-'||CAST(org_id AS TEXT))+1) + WHERE login IS NOT NULL + AND is_service_account = 1 + AND login LIKE 'sa-'||CAST(org_id AS TEXT)||'-'||CAST(org_id AS TEXT)||'-%'; + `) + default: + return fmt.Errorf("dialect not supported: %s", dialect) + } + + return err +} diff --git a/pkg/services/sqlstore/migrations/usermig/test/service_account_test.go b/pkg/services/sqlstore/migrations/usermig/test/service_account_test.go index 17330453cce..7ef8b475855 100644 --- a/pkg/services/sqlstore/migrations/usermig/test/service_account_test.go +++ b/pkg/services/sqlstore/migrations/usermig/test/service_account_test.go @@ -285,3 +285,125 @@ func TestIntegrationServiceAccountMigration(t *testing.T) { }) } } + +func TestIntegrationServiceAccountDedupOrgMigration(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in short mode") + } + // Run initial migration to have a working DB + x := setupTestDB(t) + + type migrationTestCase struct { + desc string + serviceAccounts []*user.User + wantServiceAccounts []*user.User + } + testCases := []migrationTestCase{ + { + desc: "no change", + serviceAccounts: []*user.User{ + { + ID: 1, + UID: "u1", + Name: "sa-1-nochange", + Login: "sa-1-nochange", + Email: "sa-1-nochange@example.org", + OrgID: 1, + Created: now, + Updated: now, + IsServiceAccount: true, + }, + { + ID: 2, + UID: "u2", + Name: "sa-2-nochange", + Login: "sa-2-nochange", + Email: "sa-2-nochange@example.org", + OrgID: 2, + Created: now, + Updated: now, + IsServiceAccount: true, + }, + }, + wantServiceAccounts: []*user.User{ + { + ID: 1, + Login: "sa-1-nochange", + }, + { + ID: 2, + Login: "sa-2-nochange", + }, + }, + }, + { + desc: "dedup org in login", + serviceAccounts: []*user.User{ + { + ID: 3, + UID: "u3", + Name: "sa-1-dedup", + Login: "sa-1-1-dedup", + Email: "sa-1-dedup@example.org", + OrgID: 1, + Created: now, + Updated: now, + IsServiceAccount: true, + }, + { + ID: 4, + UID: "u4", + Name: "sa-6480-dedup", + Login: "sa-6480-6480-dedup", + Email: "sa-6480-dedup@example.org", + OrgID: 6480, + Created: now, + Updated: now, + IsServiceAccount: true, + }, + }, + wantServiceAccounts: []*user.User{ + { + ID: 3, + Login: "sa-1-dedup", + }, + { + ID: 4, + Login: "sa-6480-dedup", + }, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + // Remove migration and permissions + _, errDeleteMig := x.Exec(`DELETE FROM migration_log WHERE migration_id = ?`, usermig.DedupOrgInLogin) + require.NoError(t, errDeleteMig) + + // insert service accounts + serviceAccoutsCount, err := x.Insert(tc.serviceAccounts) + require.NoError(t, err) + require.Equal(t, int64(len(tc.serviceAccounts)), serviceAccoutsCount) + + // run the migration + usermigrator := migrator.NewMigrator(x, &setting.Cfg{Logger: log.New("usermigration.test")}) + usermigrator.AddMigration(usermig.DedupOrgInLogin, &usermig.ServiceAccountsDeduplicateOrgInLogin{}) + errRunningMig := usermigrator.Start(false, 0) + require.NoError(t, errRunningMig) + + // Check service accounts + resultingServiceAccounts := []user.User{} + err = x.Table("user").Find(&resultingServiceAccounts) + require.NoError(t, err) + + for i := range tc.wantServiceAccounts { + for _, sa := range resultingServiceAccounts { + if sa.ID == tc.wantServiceAccounts[i].ID { + assert.Equal(t, tc.wantServiceAccounts[i].Login, sa.Login) + } + } + } + }) + } +}