From 21792fdf37d84c73424cfd563e27775e35c5870c Mon Sep 17 00:00:00 2001 From: Karl Persson Date: Mon, 17 Oct 2022 12:15:20 +0200 Subject: [PATCH] RBAC: Make uid for managed role names deterministic during migrations (#56620) * RBAC: Change the generate uid function to be deterministic so we can avoid collision * RBAC: Use fmt.Errorf * RBAC: Add comment * RBAC: Export GenerateManagedRoleUID --- .../managed_permission_migrator.go | 2 +- .../accesscontrol/permission_migrator.go | 27 ++++++------------- 2 files changed, 9 insertions(+), 20 deletions(-) diff --git a/pkg/services/sqlstore/migrations/accesscontrol/managed_permission_migrator.go b/pkg/services/sqlstore/migrations/accesscontrol/managed_permission_migrator.go index 361f7c2f346..2a2a285c2c0 100644 --- a/pkg/services/sqlstore/migrations/accesscontrol/managed_permission_migrator.go +++ b/pkg/services/sqlstore/migrations/accesscontrol/managed_permission_migrator.go @@ -106,7 +106,7 @@ func (sp *managedPermissionMigrator) Exec(sess *xorm.Session, mg *migrator.Migra } if !ok { - uid, err := generateNewRoleUID(sess, orgID) + uid, err := GenerateManagedRoleUID(orgID, managedRole) if err != nil { return err } diff --git a/pkg/services/sqlstore/migrations/accesscontrol/permission_migrator.go b/pkg/services/sqlstore/migrations/accesscontrol/permission_migrator.go index 5e5fbcc8354..35821ae8894 100644 --- a/pkg/services/sqlstore/migrations/accesscontrol/permission_migrator.go +++ b/pkg/services/sqlstore/migrations/accesscontrol/permission_migrator.go @@ -10,7 +10,6 @@ import ( "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/sqlstore/migrator" - "github.com/grafana/grafana/pkg/util" ) var ( @@ -137,11 +136,10 @@ func (m *permissionMigrator) createRoles(roles []*accesscontrol.Role) ([]*access args := make([]interface{}, 0, len(roles)*5) for i, r := range roles { - uid, err := generateNewRoleUID(m.sess, r.OrgID) + uid, err := GenerateManagedRoleUID(r.OrgID, r.Name) if err != nil { return nil, err } - valueStrings[i] = "(?, ?, ?, 1, ?, ?)" args = append(args, r.OrgID, uid, r.Name, ts, ts) } @@ -165,11 +163,10 @@ func (m *permissionMigrator) createRolesMySQL(roles []*accesscontrol.Role) ([]*a args := make([]interface{}, 0, len(roles)*2) for i := range roles { - uid, err := generateNewRoleUID(m.sess, roles[i].OrgID) + uid, err := GenerateManagedRoleUID(roles[i].OrgID, roles[i].Name) if err != nil { return nil, err } - roles[i].UID = uid roles[i].Created = ts roles[i].Updated = ts @@ -210,19 +207,11 @@ func batch(count, batchSize int, eachFn func(start, end int) error) error { return nil } -func generateNewRoleUID(sess *xorm.Session, orgID int64) (string, error) { - for i := 0; i < 3; i++ { - uid := util.GenerateShortUID() - - exists, err := sess.Where("org_id=? AND uid=?", orgID, uid).Get(&accesscontrol.Role{}) - if err != nil { - return "", err - } - - if !exists { - return uid, nil - } +// GenerateManagedRoleUID generated a deterministic uid of the form `managed_{org_id}_{type}_{id}`. +func GenerateManagedRoleUID(orgID int64, name string) (string, error) { + parts := strings.Split(name, ":") + if len(parts) != 4 { + return "", fmt.Errorf("unexpected role name: %s", name) } - - return "", fmt.Errorf("failed to generate uid") + return fmt.Sprintf("managed_%d_%s_%s", orgID, parts[1], parts[2]), nil }