public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[v11.0.x] Fix: Deduplicate OrgID in SA logins (#94400)

Browse Source 
* Fix: Deduplicate OrgID in SA logins (#94378)

* Fix: Deduplicate OrgID in SA logins

(cherry picked from commit b90e09e966)

* Fix: Actually call the DedupOrgInLogin migration (#94520)

* Fix: Account for conflicting logins in dedupOrgInlogin migration (#94669)

---------

Co-authored-by: Gabriel MABILLE <gamab@users.noreply.github.com>
This commit is contained in:
grafana-delivery-bot[bot]
2024-10-23 15:47:42 +02:00
committed by GitHub
co-authored by Gabriel MABILLE
parent 10e0b16e42
commit 829e425b4d
3 changed files with 220 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/services/sqlstore/migrations/user_mig.go
+3
View File
@@ -158,6 +158,9 @@ func addUserMigrations(mg *Migrator) {
// 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(usermig.AllowSameLoginCrossOrgs, &usermig.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(usermig.DedupOrgInLogin, &usermig.ServiceAccountsDeduplicateOrgInLogin{})
// Users login and email should be in lower case
mg.AddMigration(usermig.LowerCaseUserLoginAndEmail, &usermig.UsersLowerCaseLoginAndEmail{})
pkg/services/sqlstore/migrations/usermig/service_account_multiple_org_login_migrator.go
+58
View File
@@ -9,6 +9,7 @@ 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
@@ -76,3 +77,60 @@ 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" AS u
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 || '-%'
AND NOT EXISTS (
SELECT 1
FROM "user" AS u2
WHERE u2.login = 'sa-' || u.org_id::text || SUBSTRING(u.login FROM LENGTH('sa-' || u.org_id::text || '-' || u.org_id::text)+1)
);;
`)
case migrator.MySQL:
_, err = sess.Exec(`
UPDATE user AS u
LEFT JOIN user AS u2 ON u2.login = CONCAT('sa-', u.org_id, SUBSTRING(u.login, LENGTH(CONCAT('sa-', u.org_id, '-', u.org_id))+1))
SET u.login = CONCAT('sa-', u.org_id, SUBSTRING(u.login, LENGTH(CONCAT('sa-', u.org_id, '-', u.org_id))+1))
WHERE u.login IS NOT NULL
AND u.is_service_account = 1
AND u.login LIKE CONCAT('sa-', u.org_id, '-', u.org_id, '-%')
AND u2.login IS NULL;
`)
case migrator.SQLite:
_, err = sess.Exec(`
UPDATE ` + dialect.Quote("user") + ` AS u
SET login = 'sa-' || CAST(u.org_id AS TEXT) || SUBSTRING(u.login, LENGTH('sa-'||CAST(u.org_id AS TEXT)||'-'||CAST(u.org_id AS TEXT))+1)
WHERE u.login IS NOT NULL
AND u.is_service_account = 1
AND u.login LIKE 'sa-'||CAST(u.org_id AS TEXT)||'-'||CAST(u.org_id AS TEXT)||'-%'
AND NOT EXISTS (
SELECT 1
FROM ` + dialect.Quote("user") + `AS u2
WHERE u2.login = 'sa-' || CAST(u.org_id AS TEXT) || SUBSTRING(u.login, LENGTH('sa-'||CAST(u.org_id AS TEXT)||'-'||CAST(u.org_id AS TEXT))+1)
);;
`)
default:
return fmt.Errorf("dialect not supported: %s", dialect)
}
return err
}
pkg/services/sqlstore/migrations/usermig/test/service_account_test.go
+159
View File
@@ -285,3 +285,162 @@ 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",
},
},
},
{
desc: "handle conflicts",
serviceAccounts: []*user.User{
{
ID: 5,
UID: "u5",
Name: "sa-2-conflict",
Login: "sa-2-conflict",
Email: "sa-2-conflict@example.org",
OrgID: 2,
Created: now,
Updated: now,
IsServiceAccount: true,
},
{
ID: 6,
UID: "u6",
Name: "sa-2b-conflict",
Login: "sa-2-2-conflict",
Email: "sa-2b-conflict@example.org",
OrgID: 2,
Created: now,
Updated: now,
IsServiceAccount: true,
},
},
wantServiceAccounts: []*user.User{
{
ID: 5,
Login: "sa-2-conflict",
},
{
ID: 6,
Login: "sa-2-2-conflict",
},
},
},
}
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)
}
}
}
})
}
}