RBAC: Refactor GetUserPermissions to use []accesscontrol.Permission (#50683)

* Return slice of permissions instead of slice of pointers for permissions

pkg/services/accesscontrol/accesscontrol.go

@@ -21,7 +21,7 @@ type AccessControl interface {
 	Evaluate(ctx context.Context, user *models.SignedInUser, evaluator Evaluator) (bool, error)
 
 	// GetUserPermissions returns user permissions with only action and scope fields set.
-	GetUserPermissions(ctx context.Context, user *models.SignedInUser, options Options) ([]*Permission, error)
+	GetUserPermissions(ctx context.Context, user *models.SignedInUser, options Options) ([]Permission, error)
 
 	//IsDisabled returns if access control is enabled or not
 	IsDisabled() bool
@@ -42,7 +42,7 @@ type RoleRegistry interface {
 
 type PermissionsStore interface {
 	// GetUserPermissions returns user permissions with only action and scope fields set.
-	GetUserPermissions(ctx context.Context, query GetUserPermissionsQuery) ([]*Permission, error)
+	GetUserPermissions(ctx context.Context, query GetUserPermissionsQuery) ([]Permission, error)
 }
 
 type TeamPermissionsService interface {
@@ -144,7 +144,7 @@ var ReqOrgAdminOrEditor = func(c *models.ReqContext) bool {
 	return c.OrgRole == models.ROLE_ADMIN || c.OrgRole == models.ROLE_EDITOR
 }
 
-func BuildPermissionsMap(permissions []*Permission) map[string]bool {
+func BuildPermissionsMap(permissions []Permission) map[string]bool {
 	permissionsMap := make(map[string]bool)
 	for _, p := range permissions {
 		permissionsMap[p.Action] = true
@@ -154,7 +154,7 @@ func BuildPermissionsMap(permissions []*Permission) map[string]bool {
 }
 
 // GroupScopesByAction will group scopes on action
-func GroupScopesByAction(permissions []*Permission) map[string][]string {
+func GroupScopesByAction(permissions []Permission) map[string][]string {
 	m := make(map[string][]string)
 	for _, p := range permissions {
 		m[p.Action] = append(m[p.Action], p.Scope)

pkg/services/accesscontrol/database/database.go

@@ -20,8 +20,8 @@ type AccessControlStore struct {
 	sql *sqlstore.SQLStore
 }
 
-func (s *AccessControlStore) GetUserPermissions(ctx context.Context, query accesscontrol.GetUserPermissionsQuery) ([]*accesscontrol.Permission, error) {
-	result := make([]*accesscontrol.Permission, 0)
+func (s *AccessControlStore) GetUserPermissions(ctx context.Context, query accesscontrol.GetUserPermissionsQuery) ([]accesscontrol.Permission, error) {
+	result := make([]accesscontrol.Permission, 0)
 	err := s.sql.WithDbSession(ctx, func(sess *sqlstore.DBSession) error {
 		filter, params := userRolesFilter(query.OrgID, query.UserID, query.Roles)
 

pkg/services/accesscontrol/filter_bench_test.go

@@ -48,7 +48,7 @@ func benchmarkFilter(b *testing.B, numDs, numPermissions int) {
 	}
 }
 
-func setupFilterBenchmark(b *testing.B, numDs, numPermissions int) (*sqlstore.SQLStore, []*accesscontrol.Permission) {
+func setupFilterBenchmark(b *testing.B, numDs, numPermissions int) (*sqlstore.SQLStore, []accesscontrol.Permission) {
 	b.Helper()
 	store := sqlstore.InitTestDB(b)
 
@@ -64,9 +64,9 @@ func setupFilterBenchmark(b *testing.B, numDs, numPermissions int) (*sqlstore.SQ
 		numPermissions = numDs
 	}
 
-	permissions := make([]*accesscontrol.Permission, 0, numPermissions)
+	permissions := make([]accesscontrol.Permission, 0, numPermissions)
 	for i := 1; i <= numPermissions; i++ {
-		permissions = append(permissions, &accesscontrol.Permission{
+		permissions = append(permissions, accesscontrol.Permission{
 			Action: "datasources:read",
 			Scope:  accesscontrol.Scope("datasources", "id", strconv.Itoa(i)),
 		})

pkg/services/accesscontrol/middleware_test.go

@@ -34,7 +34,7 @@ func TestMiddleware(t *testing.T) {
 		{
 			desc: "should pass middleware for correct permissions",
 			ac: mock.New().WithPermissions(
-				[]*accesscontrol.Permission{{Action: "users:read", Scope: "users:*"}},
+				[]accesscontrol.Permission{{Action: "users:read", Scope: "users:*"}},
 			),
 			evaluator:      accesscontrol.EvalPermission("users:read", "users:*"),
 			expectFallback: false,
@@ -43,7 +43,7 @@ func TestMiddleware(t *testing.T) {
 		{
 			desc: "should not reach endpoint when missing permissions",
 			ac: mock.New().WithPermissions(
-				[]*accesscontrol.Permission{{Action: "users:read", Scope: "users:1"}},
+				[]accesscontrol.Permission{{Action: "users:read", Scope: "users:1"}},
 			),
 			evaluator:      accesscontrol.EvalPermission("users:read", "users:*"),
 			expectFallback: false,

pkg/services/accesscontrol/mock/mock.go

@@ -25,7 +25,7 @@ type Calls struct {
 
 type Mock struct {
 	// Unless an override is provided, permissions will be returned by GetUserPermissions
-	permissions []*accesscontrol.Permission
+	permissions []accesscontrol.Permission
 	// Unless an override is provided, disabled will be returned by IsDisabled
 	disabled bool
 	// Unless an override is provided, builtInRoles will be returned by GetUserBuiltInRoles
@@ -36,7 +36,7 @@ type Mock struct {
 
 	// Override functions
 	EvaluateFunc                       func(context.Context, *models.SignedInUser, accesscontrol.Evaluator) (bool, error)
-	GetUserPermissionsFunc             func(context.Context, *models.SignedInUser, accesscontrol.Options) ([]*accesscontrol.Permission, error)
+	GetUserPermissionsFunc             func(context.Context, *models.SignedInUser, accesscontrol.Options) ([]accesscontrol.Permission, error)
 	IsDisabledFunc                     func() bool
 	DeclareFixedRolesFunc              func(...accesscontrol.RoleRegistration) error
 	GetUserBuiltInRolesFunc            func(user *models.SignedInUser) []string
@@ -53,7 +53,7 @@ func New() *Mock {
 	mock := &Mock{
 		Calls:          Calls{},
 		disabled:       false,
-		permissions:    []*accesscontrol.Permission{},
+		permissions:    []accesscontrol.Permission{},
 		builtInRoles:   []string{},
 		scopeResolvers: accesscontrol.NewScopeResolvers(),
 	}
@@ -65,7 +65,7 @@ func (m Mock) GetUsageStats(ctx context.Context) map[string]interface{} {
 	return make(map[string]interface{})
 }
 
-func (m Mock) WithPermissions(permissions []*accesscontrol.Permission) *Mock {
+func (m Mock) WithPermissions(permissions []accesscontrol.Permission) *Mock {
 	m.permissions = permissions
 	return &m
 }
@@ -104,7 +104,7 @@ func (m *Mock) Evaluate(ctx context.Context, user *models.SignedInUser, evaluato
 
 // GetUserPermissions returns user permissions.
 // This mock return m.permissions unless an override is provided.
-func (m *Mock) GetUserPermissions(ctx context.Context, user *models.SignedInUser, opts accesscontrol.Options) ([]*accesscontrol.Permission, error) {
+func (m *Mock) GetUserPermissions(ctx context.Context, user *models.SignedInUser, opts accesscontrol.Options) ([]accesscontrol.Permission, error) {
 	m.Calls.GetUserPermissions = append(m.Calls.GetUserPermissions, []interface{}{ctx, user, opts})
 	// Use override if provided
 	if m.GetUserPermissionsFunc != nil {

pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol.go

@@ -103,7 +103,7 @@ func (ac *OSSAccessControlService) Evaluate(ctx context.Context, user *models.Si
 }
 
 // GetUserPermissions returns user permissions based on built-in roles
-func (ac *OSSAccessControlService) GetUserPermissions(ctx context.Context, user *models.SignedInUser, _ accesscontrol.Options) ([]*accesscontrol.Permission, error) {
+func (ac *OSSAccessControlService) GetUserPermissions(ctx context.Context, user *models.SignedInUser, _ accesscontrol.Options) ([]accesscontrol.Permission, error) {
 	timer := prometheus.NewTimer(metrics.MAccessPermissionsSummary)
 	defer timer.ObserveDuration()
 
@@ -120,28 +120,24 @@ func (ac *OSSAccessControlService) GetUserPermissions(ctx context.Context, user
 	}
 
 	permissions = append(permissions, dbPermissions...)
-	resolved := make([]*accesscontrol.Permission, 0, len(permissions))
 	keywordMutator := ac.scopeResolvers.GetScopeKeywordMutator(user)
-	for _, p := range permissions {
+	for i := range permissions {
 		// if the permission has a keyword in its scope it will be resolved
-		p.Scope, err = keywordMutator(ctx, p.Scope)
+		permissions[i].Scope, err = keywordMutator(ctx, permissions[i].Scope)
 		if err != nil {
 			return nil, err
 		}
-		resolved = append(resolved, p)
 	}
 
-	return resolved, nil
+	return permissions, nil
 }
 
-func (ac *OSSAccessControlService) getFixedPermissions(ctx context.Context, user *models.SignedInUser) []*accesscontrol.Permission {
-	permissions := make([]*accesscontrol.Permission, 0)
+func (ac *OSSAccessControlService) getFixedPermissions(ctx context.Context, user *models.SignedInUser) []accesscontrol.Permission {
+	permissions := make([]accesscontrol.Permission, 0)
 
 	for _, builtin := range accesscontrol.GetOrgRoles(ac.cfg, user) {
 		if basicRole, ok := ac.roles[builtin]; ok {
-			for i := range basicRole.Permissions {
-				permissions = append(permissions, &basicRole.Permissions[i])
-			}
+			permissions = append(permissions, basicRole.Permissions...)
 		}
 	}
 

pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol_test.go

@@ -35,10 +35,10 @@ func setupTestEnv(t testing.TB) *OSSAccessControlService {
 }
 
 // extractRawPermissionsHelper extracts action and scope fields only from a permission slice
-func extractRawPermissionsHelper(perms []*accesscontrol.Permission) []*accesscontrol.Permission {
-	res := make([]*accesscontrol.Permission, len(perms))
+func extractRawPermissionsHelper(perms []accesscontrol.Permission) []accesscontrol.Permission {
+	res := make([]accesscontrol.Permission, len(perms))
 	for i, p := range perms {
-		res[i] = &accesscontrol.Permission{Action: p.Action, Scope: p.Scope}
+		res[i] = accesscontrol.Permission{Action: p.Action, Scope: p.Scope}
 	}
 	return res
 }
@@ -421,8 +421,8 @@ func TestOSSAccessControlService_GetUserPermissions(t *testing.T) {
 
 			rawUserPerms := extractRawPermissionsHelper(userPerms)
 
-			assert.Contains(t, rawUserPerms, &tt.wantPerm, "Expected resolution of raw permission")
-			assert.NotContains(t, rawUserPerms, &tt.rawPerm, "Expected raw permission to have been resolved")
+			assert.Contains(t, rawUserPerms, tt.wantPerm, "Expected resolution of raw permission")
+			assert.NotContains(t, rawUserPerms, tt.rawPerm, "Expected raw permission to have been resolved")
 		})
 	}
 }

pkg/services/accesscontrol/resourcepermissions/api_test.go

@@ -26,7 +26,7 @@ import (
 type getDescriptionTestCase struct {
 	desc           string
 	options        Options
-	permissions    []*accesscontrol.Permission
+	permissions    []accesscontrol.Permission
 	expected       Description
 	expectedStatus int
 }
@@ -49,7 +49,7 @@ func TestApi_getDescription(t *testing.T) {
 					"Admin": {"dashboards:read", "dashboards:write", "dashboards:delete", "dashboards.permissions:read", "dashboards:permissions:write"},
 				},
 			},
-			permissions: []*accesscontrol.Permission{
+			permissions: []accesscontrol.Permission{
 				{Action: "dashboards.permissions:read"},
 			},
 			expected: Description{
@@ -76,7 +76,7 @@ func TestApi_getDescription(t *testing.T) {
 					"View": {"dashboards:read"},
 				},
 			},
-			permissions: []*accesscontrol.Permission{
+			permissions: []accesscontrol.Permission{
 				{Action: "dashboards.permissions:read"},
 			},
 			expected: Description{
@@ -103,7 +103,7 @@ func TestApi_getDescription(t *testing.T) {
 					"View": {"dashboards:read"},
 				},
 			},
-			permissions:    []*accesscontrol.Permission{},
+			permissions:    []accesscontrol.Permission{},
 			expected:       Description{},
 			expectedStatus: http.StatusForbidden,
 		},
@@ -132,7 +132,7 @@ func TestApi_getDescription(t *testing.T) {
 type getPermissionsTestCase struct {
 	desc           string
 	resourceID     string
-	permissions    []*accesscontrol.Permission
+	permissions    []accesscontrol.Permission
 	expectedStatus int
 }
 
@@ -141,7 +141,7 @@ func TestApi_getPermissions(t *testing.T) {
 		{
 			desc:       "expect permissions for resource with id 1",
 			resourceID: "1",
-			permissions: []*accesscontrol.Permission{
+			permissions: []accesscontrol.Permission{
 				{Action: "dashboards.permissions:read", Scope: "dashboards:id:1"},
 				{Action: accesscontrol.ActionTeamsRead, Scope: accesscontrol.ScopeTeamsAll},
 				{Action: accesscontrol.ActionOrgUsersRead, Scope: accesscontrol.ScopeUsersAll},
@@ -151,7 +151,7 @@ func TestApi_getPermissions(t *testing.T) {
 		{
 			desc:           "expect http status 403 when missing permission",
 			resourceID:     "1",
-			permissions:    []*accesscontrol.Permission{},
+			permissions:    []accesscontrol.Permission{},
 			expectedStatus: 403,
 		},
 	}
@@ -179,7 +179,7 @@ type setBuiltinPermissionTestCase struct {
 	builtInRole    string
 	expectedStatus int
 	permission     string
-	permissions    []*accesscontrol.Permission
+	permissions    []accesscontrol.Permission
 }
 
 func TestApi_setBuiltinRolePermission(t *testing.T) {
@@ -190,7 +190,7 @@ func TestApi_setBuiltinRolePermission(t *testing.T) {
 			builtInRole:    "Viewer",
 			expectedStatus: 200,
 			permission:     "Edit",
-			permissions: []*accesscontrol.Permission{
+			permissions: []accesscontrol.Permission{
 				{Action: "dashboards.permissions:read", Scope: "dashboards:id:1"},
 				{Action: "dashboards.permissions:write", Scope: "dashboards:id:1"},
 				{Action: accesscontrol.ActionTeamsRead, Scope: accesscontrol.ScopeTeamsAll},
@@ -203,7 +203,7 @@ func TestApi_setBuiltinRolePermission(t *testing.T) {
 			builtInRole:    "Admin",
 			expectedStatus: 200,
 			permission:     "View",
-			permissions: []*accesscontrol.Permission{
+			permissions: []accesscontrol.Permission{
 				{Action: "dashboards.permissions:read", Scope: "dashboards:id:1"},
 				{Action: "dashboards.permissions:write", Scope: "dashboards:id:1"},
 				{Action: accesscontrol.ActionTeamsRead, Scope: accesscontrol.ScopeTeamsAll},
@@ -216,7 +216,7 @@ func TestApi_setBuiltinRolePermission(t *testing.T) {
 			builtInRole:    "Invalid",
 			expectedStatus: http.StatusBadRequest,
 			permission:     "View",
-			permissions: []*accesscontrol.Permission{
+			permissions: []accesscontrol.Permission{
 				{Action: "dashboards.permissions:read", Scope: "dashboards:id:1"},
 				{Action: "dashboards.permissions:write", Scope: "dashboards:id:1"},
 			},
@@ -227,7 +227,7 @@ func TestApi_setBuiltinRolePermission(t *testing.T) {
 			builtInRole:    "Invalid",
 			expectedStatus: http.StatusForbidden,
 			permission:     "View",
-			permissions: []*accesscontrol.Permission{
+			permissions: []accesscontrol.Permission{
 				{Action: "dashboards.permissions:read", Scope: "dashboards:id:1"},
 			},
 		},
@@ -257,7 +257,7 @@ type setTeamPermissionTestCase struct {
 	resourceID     string
 	expectedStatus int
 	permission     string
-	permissions    []*accesscontrol.Permission
+	permissions    []accesscontrol.Permission
 }
 
 func TestApi_setTeamPermission(t *testing.T) {
@@ -268,7 +268,7 @@ func TestApi_setTeamPermission(t *testing.T) {
 			resourceID:     "1",
 			expectedStatus: 200,
 			permission:     "Edit",
-			permissions: []*accesscontrol.Permission{
+			permissions: []accesscontrol.Permission{
 				{Action: "dashboards.permissions:read", Scope: "dashboards:id:1"},
 				{Action: "dashboards.permissions:write", Scope: "dashboards:id:1"},
 				{Action: accesscontrol.ActionTeamsRead, Scope: accesscontrol.ScopeTeamsAll},
@@ -281,7 +281,7 @@ func TestApi_setTeamPermission(t *testing.T) {
 			resourceID:     "1",
 			expectedStatus: 200,
 			permission:     "View",
-			permissions: []*accesscontrol.Permission{
+			permissions: []accesscontrol.Permission{
 				{Action: "dashboards.permissions:read", Scope: "dashboards:id:1"},
 				{Action: "dashboards.permissions:write", Scope: "dashboards:id:1"},
 				{Action: accesscontrol.ActionTeamsRead, Scope: accesscontrol.ScopeTeamsAll},
@@ -294,7 +294,7 @@ func TestApi_setTeamPermission(t *testing.T) {
 			resourceID:     "1",
 			expectedStatus: http.StatusBadRequest,
 			permission:     "View",
-			permissions: []*accesscontrol.Permission{
+			permissions: []accesscontrol.Permission{
 				{Action: "dashboards.permissions:read", Scope: "dashboards:id:1"},
 				{Action: "dashboards.permissions:write", Scope: "dashboards:id:1"},
 			},
@@ -305,7 +305,7 @@ func TestApi_setTeamPermission(t *testing.T) {
 			resourceID:     "1",
 			expectedStatus: http.StatusForbidden,
 			permission:     "View",
-			permissions: []*accesscontrol.Permission{
+			permissions: []accesscontrol.Permission{
 				{Action: "dashboards.permissions:read", Scope: "dashboards:id:1"},
 			},
 		},
@@ -340,7 +340,7 @@ type setUserPermissionTestCase struct {
 	resourceID     string
 	expectedStatus int
 	permission     string
-	permissions    []*accesscontrol.Permission
+	permissions    []accesscontrol.Permission
 }
 
 func TestApi_setUserPermission(t *testing.T) {
@@ -351,7 +351,7 @@ func TestApi_setUserPermission(t *testing.T) {
 			resourceID:     "1",
 			expectedStatus: 200,
 			permission:     "Edit",
-			permissions: []*accesscontrol.Permission{
+			permissions: []accesscontrol.Permission{
 				{Action: "dashboards.permissions:read", Scope: "dashboards:id:1"},
 				{Action: "dashboards.permissions:write", Scope: "dashboards:id:1"},
 				{Action: accesscontrol.ActionTeamsRead, Scope: accesscontrol.ScopeTeamsAll},
@@ -364,7 +364,7 @@ func TestApi_setUserPermission(t *testing.T) {
 			resourceID:     "1",
 			expectedStatus: 200,
 			permission:     "View",
-			permissions: []*accesscontrol.Permission{
+			permissions: []accesscontrol.Permission{
 				{Action: "dashboards.permissions:read", Scope: "dashboards:id:1"},
 				{Action: "dashboards.permissions:write", Scope: "dashboards:id:1"},
 				{Action: accesscontrol.ActionTeamsRead, Scope: accesscontrol.ScopeTeamsAll},
@@ -377,7 +377,7 @@ func TestApi_setUserPermission(t *testing.T) {
 			resourceID:     "1",
 			expectedStatus: http.StatusBadRequest,
 			permission:     "View",
-			permissions: []*accesscontrol.Permission{
+			permissions: []accesscontrol.Permission{
 				{Action: "dashboards.permissions:read", Scope: "dashboards:id:1"},
 				{Action: "dashboards.permissions:write", Scope: "dashboards:id:1"},
 			},
@@ -388,7 +388,7 @@ func TestApi_setUserPermission(t *testing.T) {
 			resourceID:     "1",
 			expectedStatus: http.StatusForbidden,
 			permission:     "View",
-			permissions: []*accesscontrol.Permission{
+			permissions: []accesscontrol.Permission{
 				{Action: "dashboards.permissions:read", Scope: "dashboards:id:1"},
 			},
 		},

pkg/services/accesscontrol/resourcepermissions/service_test.go

@@ -36,7 +36,7 @@ func TestService_SetUserPermission(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.desc, func(t *testing.T) {
-			service, sql := setupTestEnvironment(t, []*accesscontrol.Permission{}, Options{
+			service, sql := setupTestEnvironment(t, []accesscontrol.Permission{}, Options{
 				Resource:             "dashboards",
 				Assignments:          Assignments{Users: true},
 				PermissionsToActions: nil,
@@ -80,7 +80,7 @@ func TestService_SetTeamPermission(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.desc, func(t *testing.T) {
-			service, sql := setupTestEnvironment(t, []*accesscontrol.Permission{}, Options{
+			service, sql := setupTestEnvironment(t, []accesscontrol.Permission{}, Options{
 				Resource:             "dashboards",
 				Assignments:          Assignments{Teams: true},
 				PermissionsToActions: nil,
@@ -124,7 +124,7 @@ func TestService_SetBuiltInRolePermission(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.desc, func(t *testing.T) {
-			service, _ := setupTestEnvironment(t, []*accesscontrol.Permission{}, Options{
+			service, _ := setupTestEnvironment(t, []accesscontrol.Permission{}, Options{
 				Resource:             "dashboards",
 				Assignments:          Assignments{BuiltInRoles: true},
 				PermissionsToActions: nil,
@@ -197,7 +197,7 @@ func TestService_SetPermissions(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.desc, func(t *testing.T) {
-			service, sql := setupTestEnvironment(t, []*accesscontrol.Permission{}, tt.options)
+			service, sql := setupTestEnvironment(t, []accesscontrol.Permission{}, tt.options)
 
 			// seed user
 			_, err := sql.CreateUser(context.Background(), models.CreateUserCommand{Login: "user", OrgId: 1})
@@ -216,7 +216,7 @@ func TestService_SetPermissions(t *testing.T) {
 	}
 }
 
-func setupTestEnvironment(t *testing.T, permissions []*accesscontrol.Permission, ops Options) (*Service, *sqlstore.SQLStore) {
+func setupTestEnvironment(t *testing.T, permissions []accesscontrol.Permission, ops Options) (*Service, *sqlstore.SQLStore) {
 	t.Helper()
 
 	sql := sqlstore.InitTestDB(t)