RBAC: Inheritance permission migration should handle empty managed roles (#50611)
* Make inheritance permission migration more robust * Better fix * Add more tests to the migration * Add removed test case * Add test case for empty role in empty org * Handling the role.ID 0 case with a log Co-authored-by: Jguer <joao.guerreiro@grafana.com> Co-authored-by: Jguer <joao.guerreiro@grafana.com>
This commit is contained in:
Gabriel MABILLE
GitHub
+109
-18
@@ -1,6 +1,7 @@
|
||||
package test
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"strconv"
|
||||
"strings"
|
||||
"testing"
|
||||
@@ -14,19 +15,17 @@ import (
|
||||
"xorm.io/xorm"
|
||||
)
|
||||
|
||||
func TestManagedPermissionsMigration(t *testing.T) {
|
||||
// Run initial migration to have a working DB
|
||||
x := setupTestDB(t)
|
||||
type inheritanceTestCase struct {
|
||||
desc string
|
||||
putRolePerms map[int64]map[string][]rawPermission
|
||||
wantRolePerms map[int64]map[string][]rawPermission
|
||||
}
|
||||
|
||||
func inheritanceTestCases() []inheritanceTestCase {
|
||||
team1Scope := accesscontrol.Scope("teams", "id", "1")
|
||||
team2Scope := accesscontrol.Scope("teams", "id", "2")
|
||||
|
||||
type teamMigrationTestCase struct {
|
||||
desc string
|
||||
putRolePerms map[int64]map[string][]rawPermission
|
||||
wantRolePerms map[int64]map[string][]rawPermission
|
||||
}
|
||||
testCases := []teamMigrationTestCase{
|
||||
return []inheritanceTestCase{
|
||||
{
|
||||
desc: "empty perms",
|
||||
putRolePerms: map[int64]map[string][]rawPermission{},
|
||||
@@ -74,6 +73,16 @@ func TestManagedPermissionsMigration(t *testing.T) {
|
||||
{Action: "teams:write", Scope: team1Scope},
|
||||
},
|
||||
},
|
||||
3: {
|
||||
"managed:builtins:editor:permissions": {
|
||||
{Action: "teams.permissions:read", Scope: team1Scope},
|
||||
{Action: "teams.permissions:write", Scope: team2Scope},
|
||||
},
|
||||
"managed:builtins:admin:permissions": {}, //Role existed with no permission
|
||||
},
|
||||
4: {
|
||||
"managed:builtins:admin:permissions": {}, //Role existed with no permission
|
||||
},
|
||||
},
|
||||
wantRolePerms: map[int64]map[string][]rawPermission{
|
||||
1: {
|
||||
@@ -111,16 +120,37 @@ func TestManagedPermissionsMigration(t *testing.T) {
|
||||
{Action: "teams:write", Scope: team1Scope},
|
||||
},
|
||||
},
|
||||
3: {
|
||||
"managed:builtins:editor:permissions": {
|
||||
{Action: "teams.permissions:read", Scope: team1Scope},
|
||||
{Action: "teams.permissions:write", Scope: team2Scope},
|
||||
},
|
||||
"managed:builtins:admin:permissions": {
|
||||
{Action: "teams.permissions:read", Scope: team1Scope},
|
||||
{Action: "teams.permissions:write", Scope: team2Scope},
|
||||
},
|
||||
},
|
||||
4: {
|
||||
"managed:builtins:admin:permissions": {}, //Role existed with no permission
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
for _, tc := range testCases {
|
||||
func TestManagedPermissionsMigration(t *testing.T) {
|
||||
// Run initial migration to have a working DB
|
||||
x := setupTestDB(t)
|
||||
|
||||
for _, tc := range inheritanceTestCases() {
|
||||
t.Run(tc.desc, func(t *testing.T) {
|
||||
// Remove migration
|
||||
_, errDeleteMig := x.Exec(`DELETE FROM migration_log WHERE migration_id = ?;
|
||||
DELETE FROM permission; DELETE FROM role`, acmig.ManagedPermissionsMigrationID)
|
||||
_, errDeleteMig := x.Exec(`DELETE FROM migration_log WHERE migration_id = ?`, acmig.ManagedPermissionsMigrationID)
|
||||
require.NoError(t, errDeleteMig)
|
||||
_, errDeletePerm := x.Exec(`DELETE FROM permission`)
|
||||
require.NoError(t, errDeletePerm)
|
||||
_, errDeleteRole := x.Exec(`DELETE FROM role`)
|
||||
require.NoError(t, errDeleteRole)
|
||||
|
||||
// put permissions
|
||||
putTestPermissions(t, x, tc.putRolePerms)
|
||||
@@ -163,6 +193,65 @@ DELETE FROM permission; DELETE FROM role`, acmig.ManagedPermissionsMigrationID)
|
||||
}
|
||||
}
|
||||
|
||||
func TestManagedPermissionsMigrationRunTwice(t *testing.T) {
|
||||
// Run initial migration to have a working DB
|
||||
x := setupTestDB(t)
|
||||
|
||||
for _, tc := range inheritanceTestCases() {
|
||||
t.Run(tc.desc, func(t *testing.T) {
|
||||
// Remove migration
|
||||
_, errDeleteMig := x.Exec(`DELETE FROM migration_log WHERE migration_id LIKE ?`, acmig.ManagedPermissionsMigrationID+"%")
|
||||
require.NoError(t, errDeleteMig)
|
||||
_, errDeletePerm := x.Exec(`DELETE FROM permission`)
|
||||
require.NoError(t, errDeletePerm)
|
||||
_, errDeleteRole := x.Exec(`DELETE FROM role`)
|
||||
require.NoError(t, errDeleteRole)
|
||||
|
||||
for i := 0; i < 2; i++ {
|
||||
if i == 0 {
|
||||
// put permissions
|
||||
putTestPermissions(t, x, tc.putRolePerms)
|
||||
}
|
||||
|
||||
// Run accesscontrol migration (permissions insertion should not have conflicted)
|
||||
acmigrator := migrator.NewMigrator(x, &setting.Cfg{Logger: log.New("acmigration.test")})
|
||||
acmig.AddManagedPermissionsMigration(acmigrator, acmig.ManagedPermissionsMigrationID+fmt.Sprint(i))
|
||||
|
||||
errRunningMig := acmigrator.Start(false, 0)
|
||||
require.NoError(t, errRunningMig)
|
||||
|
||||
// verify got == want
|
||||
for orgID, roles := range tc.wantRolePerms {
|
||||
for roleName := range roles {
|
||||
// Check managed roles exist
|
||||
role := accesscontrol.Role{}
|
||||
hasRole, errManagedRoleSearch := x.Table("role").Where("org_id = ? AND name = ?", orgID, roleName).Get(&role)
|
||||
|
||||
require.NoError(t, errManagedRoleSearch)
|
||||
require.True(t, hasRole, "expected role to exist", "orgID", orgID, "role", roleName)
|
||||
|
||||
// Check permissions associated with each role
|
||||
perms := []accesscontrol.Permission{}
|
||||
count, errManagedPermsSearch := x.Table("permission").Where("role_id = ?", role.ID).FindAndCount(&perms)
|
||||
|
||||
require.NoError(t, errManagedPermsSearch)
|
||||
require.Equal(t, int64(len(tc.wantRolePerms[orgID][roleName])), count, "expected role to be tied to permissions", "orgID", orgID, "role", roleName)
|
||||
|
||||
gotRawPerms := convertToRawPermissions(perms)
|
||||
require.ElementsMatch(t, gotRawPerms, tc.wantRolePerms[orgID][roleName], "expected role to have permissions", "orgID", orgID, "role", roleName)
|
||||
|
||||
// Check assignment of the roles
|
||||
br := accesscontrol.BuiltinRole{}
|
||||
has, errAssignmentSearch := x.Table("builtin_role").Where("role_id = ? AND role = ? AND org_id = ?", role.ID, acmig.ParseRoleFromName(roleName), orgID).Get(&br)
|
||||
require.NoError(t, errAssignmentSearch)
|
||||
require.True(t, has, "expected assignment of role to builtin role", "orgID", orgID, "role", roleName)
|
||||
}
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func putTestPermissions(t *testing.T, x *xorm.Engine, rolePerms map[int64]map[string][]rawPermission) {
|
||||
for orgID, roles := range rolePerms {
|
||||
for roleName, perms := range roles {
|
||||
@@ -190,13 +279,15 @@ func putTestPermissions(t *testing.T, x *xorm.Engine, rolePerms map[int64]map[st
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, int64(1), brCount)
|
||||
|
||||
permissions := []accesscontrol.Permission{}
|
||||
for _, p := range perms {
|
||||
permissions = append(permissions, p.toPermission(role.ID, now))
|
||||
if len(perms) > 0 {
|
||||
permissions := []accesscontrol.Permission{}
|
||||
for _, p := range perms {
|
||||
permissions = append(permissions, p.toPermission(role.ID, now))
|
||||
}
|
||||
permissionsCount, err := x.Insert(permissions)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, int64(len(perms)), permissionsCount)
|
||||
}
|
||||
permissionsCount, err := x.Insert(permissions)
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, int64(len(perms)), permissionsCount)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user