From 7ab1ef8d6e8484813cce6efaf86114903be80352 Mon Sep 17 00:00:00 2001 From: Karl Persson Date: Mon, 21 Mar 2022 17:58:18 +0100 Subject: [PATCH] Access Control: Support other attributes than id for resource permissions (#46727) * Add option to set ResourceAttribute for a permissions service * Use prefix in access control sql filter to parse scopes * Use prefix in access control metadata to check access --- pkg/api/accesscontrol.go | 14 +- pkg/api/datasources.go | 5 +- pkg/api/org_users.go | 2 +- pkg/api/team.go | 4 +- pkg/api/user.go | 2 +- pkg/services/accesscontrol/accesscontrol.go | 36 +- .../accesscontrol/accesscontrol_bench_test.go | 2 +- .../accesscontrol/accesscontrol_test.go | 34 +- .../database/resource_permissions.go | 28 +- .../resource_permissions_bench_test.go | 22 +- .../database/resource_permissions_test.go | 307 ++++++++++-------- pkg/services/accesscontrol/filter.go | 42 ++- .../accesscontrol/filter_bench_test.go | 2 +- pkg/services/accesscontrol/filter_test.go | 22 +- pkg/services/accesscontrol/models.go | 1 + .../ossaccesscontrol/permissions_services.go | 11 +- .../accesscontrol/resourcepermissions/api.go | 10 +- .../resourcepermissions/api_test.go | 12 +- .../resourcepermissions/options.go | 2 + .../resourcepermissions/service.go | 53 +-- .../resourcepermissions/types/models.go | 22 +- pkg/services/serviceaccounts/api/api.go | 2 +- .../serviceaccounts/database/database.go | 2 +- pkg/services/sqlstore/org_users.go | 4 +- .../sqlstore/permissions/dashboard.go | 6 +- pkg/services/sqlstore/team.go | 4 +- 26 files changed, 363 insertions(+), 288 deletions(-) diff --git a/pkg/api/accesscontrol.go b/pkg/api/accesscontrol.go index ed5ffc53ef5..4b4311490ac 100644 --- a/pkg/api/accesscontrol.go +++ b/pkg/api/accesscontrol.go @@ -1,8 +1,6 @@ package api import ( - "fmt" - "github.com/grafana/grafana/pkg/models" ac "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/dashboards" @@ -463,15 +461,13 @@ var teamsEditAccessEvaluator = ac.EvalAll( // Metadata helpers // getAccessControlMetadata returns the accesscontrol metadata associated with a given resource -func (hs *HTTPServer) getAccessControlMetadata(c *models.ReqContext, resource string, id int64) ac.Metadata { - key := fmt.Sprintf("%d", id) - ids := map[string]bool{key: true} - - return hs.getMultiAccessControlMetadata(c, resource, ids)[key] +func (hs *HTTPServer) getAccessControlMetadata(c *models.ReqContext, prefix string, resourceID string) ac.Metadata { + ids := map[string]bool{resourceID: true} + return hs.getMultiAccessControlMetadata(c, prefix, ids)[resourceID] } // getMultiAccessControlMetadata returns the accesscontrol metadata associated with a given set of resources -func (hs *HTTPServer) getMultiAccessControlMetadata(c *models.ReqContext, resource string, ids map[string]bool) map[string]ac.Metadata { +func (hs *HTTPServer) getMultiAccessControlMetadata(c *models.ReqContext, prefix string, resourceIDs map[string]bool) map[string]ac.Metadata { if hs.AccessControl.IsDisabled() || !c.QueryBool("accesscontrol") { return map[string]ac.Metadata{} } @@ -485,5 +481,5 @@ func (hs *HTTPServer) getMultiAccessControlMetadata(c *models.ReqContext, resour return map[string]ac.Metadata{} } - return ac.GetResourcesMetadata(c.Req.Context(), permissions, resource, ids) + return ac.GetResourcesMetadata(c.Req.Context(), permissions, prefix, resourceIDs) } diff --git a/pkg/api/datasources.go b/pkg/api/datasources.go index 31efaf43192..fb746ed45a8 100644 --- a/pkg/api/datasources.go +++ b/pkg/api/datasources.go @@ -100,7 +100,7 @@ func (hs *HTTPServer) GetDataSourceById(c *models.ReqContext) response.Response dto := convertModelToDtos(filtered[0]) // Add accesscontrol metadata - dto.AccessControl = hs.getAccessControlMetadata(c, "datasources", dto.Id) + dto.AccessControl = hs.getAccessControlMetadata(c, "datasources:id:", strconv.FormatInt(dto.Id, 10)) return response.JSON(200, &dto) } @@ -159,8 +159,7 @@ func (hs *HTTPServer) GetDataSourceByUID(c *models.ReqContext) response.Response dto := convertModelToDtos(filtered[0]) // Add accesscontrol metadata - dto.AccessControl = hs.getAccessControlMetadata(c, "datasources", dto.Id) - + dto.AccessControl = hs.getAccessControlMetadata(c, "datasources:id:", strconv.FormatInt(dto.Id, 10)) return response.JSON(200, &dto) } diff --git a/pkg/api/org_users.go b/pkg/api/org_users.go index cad6b3715f6..39a7f77ce74 100644 --- a/pkg/api/org_users.go +++ b/pkg/api/org_users.go @@ -150,7 +150,7 @@ func (hs *HTTPServer) getOrgUsersHelper(c *models.ReqContext, query *models.GetO filteredUsers = append(filteredUsers, user) } - accessControlMetadata := hs.getMultiAccessControlMetadata(c, "users", userIDs) + accessControlMetadata := hs.getMultiAccessControlMetadata(c, "users:id:", userIDs) if len(accessControlMetadata) > 0 { for i := range filteredUsers { filteredUsers[i].AccessControl = accessControlMetadata[fmt.Sprint(filteredUsers[i].UserId)] diff --git a/pkg/api/team.go b/pkg/api/team.go index 2b256959e9c..f2d0ae1b7ba 100644 --- a/pkg/api/team.go +++ b/pkg/api/team.go @@ -141,7 +141,7 @@ func (hs *HTTPServer) SearchTeams(c *models.ReqContext) response.Response { teamIDs[strconv.FormatInt(team.Id, 10)] = true } - metadata := hs.getMultiAccessControlMetadata(c, "teams", teamIDs) + metadata := hs.getMultiAccessControlMetadata(c, "teams:id:", teamIDs) if len(metadata) > 0 { for _, team := range query.Result.Teams { team.AccessControl = metadata[strconv.FormatInt(team.Id, 10)] @@ -195,7 +195,7 @@ func (hs *HTTPServer) GetTeamByID(c *models.ReqContext) response.Response { } // Add accesscontrol metadata - query.Result.AccessControl = hs.getAccessControlMetadata(c, "teams", query.Result.Id) + query.Result.AccessControl = hs.getAccessControlMetadata(c, "teams:id:", strconv.FormatInt(query.Result.Id, 10)) query.Result.AvatarUrl = dtos.GetGravatarUrlWithDefault(query.Result.Email, query.Result.Name) return response.JSON(200, &query.Result) diff --git a/pkg/api/user.go b/pkg/api/user.go index 20babbf91cc..25861433948 100644 --- a/pkg/api/user.go +++ b/pkg/api/user.go @@ -46,7 +46,7 @@ func (hs *HTTPServer) getUserUserProfile(c *models.ReqContext, userID int64) res query.Result.IsExternal = true } - query.Result.AccessControl = hs.getAccessControlMetadata(c, "global:users", userID) + query.Result.AccessControl = hs.getAccessControlMetadata(c, "global:users:id:", strconv.FormatInt(userID, 10)) query.Result.AvatarUrl = dtos.GetGravatarUrl(query.Result.Email) return response.JSON(200, query.Result) diff --git a/pkg/services/accesscontrol/accesscontrol.go b/pkg/services/accesscontrol/accesscontrol.go index 1e036468068..161d9178cdb 100644 --- a/pkg/services/accesscontrol/accesscontrol.go +++ b/pkg/services/accesscontrol/accesscontrol.go @@ -164,28 +164,32 @@ func addActionToMetadata(allMetadata map[string]Metadata, action, id string) map } // GetResourcesMetadata returns a map of accesscontrol metadata, listing for each resource, users available actions -func GetResourcesMetadata(ctx context.Context, permissions map[string][]string, resource string, ids map[string]bool) map[string]Metadata { - allScope := GetResourceAllScope(resource) - allIDScope := GetResourceAllIDScope(resource) +func GetResourcesMetadata(ctx context.Context, permissions map[string][]string, prefix string, resourceIDs map[string]bool) map[string]Metadata { + rootPrefix, attributePrefix, ok := extractPrefixes(prefix) + if !ok { + return map[string]Metadata{} + } - // prefix of ID based scopes (resource:id) - idPrefix := Scope(resource, "id") - // index of the ID in the scope - idIndex := len(idPrefix) + 1 + allScope := GetResourceAllScope(strings.TrimSuffix(rootPrefix, ":")) + allAttributeScope := Scope(strings.TrimSuffix(attributePrefix, ":"), "*") + + // index of the attribute in the scope + attributeIndex := len(attributePrefix) // Loop through permissions once result := map[string]Metadata{} + for action, scopes := range permissions { for _, scope := range scopes { - if scope == "*" || scope == allScope || scope == allIDScope { + if scope == "*" || scope == allScope || scope == allAttributeScope { // Add global action to all resources - for id := range ids { + for id := range resourceIDs { result = addActionToMetadata(result, action, id) } } else { - if len(scope) > idIndex && strings.HasPrefix(scope, idPrefix) && ids[scope[idIndex:]] { + if len(scope) > attributeIndex && strings.HasPrefix(scope, attributePrefix) && resourceIDs[scope[attributeIndex:]] { // Add action to a specific resource - result = addActionToMetadata(result, action, scope[idIndex:]) + result = addActionToMetadata(result, action, scope[attributeIndex:]) } } } @@ -205,3 +209,13 @@ func ManagedTeamRoleName(teamID int64) string { func ManagedBuiltInRoleName(builtInRole string) string { return fmt.Sprintf("managed:builtins:%s:permissions", strings.ToLower(builtInRole)) } + +func extractPrefixes(prefix string) (string, string, bool) { + parts := strings.Split(strings.TrimSuffix(prefix, ":"), ":") + if len(parts) != 2 { + return "", "", false + } + rootPrefix := parts[0] + ":" + attributePrefix := rootPrefix + parts[1] + ":" + return rootPrefix, attributePrefix, true +} diff --git a/pkg/services/accesscontrol/accesscontrol_bench_test.go b/pkg/services/accesscontrol/accesscontrol_bench_test.go index b6942e664d3..044b67153b3 100644 --- a/pkg/services/accesscontrol/accesscontrol_bench_test.go +++ b/pkg/services/accesscontrol/accesscontrol_bench_test.go @@ -31,7 +31,7 @@ func benchGetMetadata(b *testing.B, resourceCount, permissionPerResource int) { var metadata map[string]Metadata for n := 0; n < b.N; n++ { - metadata = GetResourcesMetadata(context.Background(), permissions, "resources", ids) + metadata = GetResourcesMetadata(context.Background(), permissions, "resources:id:", ids) assert.Len(b, metadata, resourceCount) for _, resourceMetadata := range metadata { assert.Len(b, resourceMetadata, permissionPerResource) diff --git a/pkg/services/accesscontrol/accesscontrol_test.go b/pkg/services/accesscontrol/accesscontrol_test.go index eef8e0b1e11..4b49976591e 100644 --- a/pkg/services/accesscontrol/accesscontrol_test.go +++ b/pkg/services/accesscontrol/accesscontrol_test.go @@ -10,20 +10,20 @@ import ( func TestGetResourcesMetadata(t *testing.T) { tests := []struct { desc string - resource string + prefix string resourcesIDs map[string]bool permissions map[string][]string expected map[string]Metadata }{ { desc: "Should return no permission for resources 1,2,3 given the user has no permission", - resource: "resources", + prefix: "resources:id:", resourcesIDs: map[string]bool{"1": true, "2": true, "3": true}, expected: map[string]Metadata{}, }, { - desc: "Should return no permission for resources 1,2,3 given the user has permissions for 4 only", - resource: "resources", + desc: "Should return no permission for resources 1,2,3 given the user has permissions for 4 only", + prefix: "resources:id:", permissions: map[string][]string{ "resources:action1": {Scope("resources", "id", "4")}, "resources:action2": {Scope("resources", "id", "4")}, @@ -33,8 +33,8 @@ func TestGetResourcesMetadata(t *testing.T) { expected: map[string]Metadata{}, }, { - desc: "Should only return permissions for resources 1 and 2, given the user has no permissions for 3", - resource: "resources", + desc: "Should only return permissions for resources 1 and 2, given the user has no permissions for 3", + prefix: "resources:id:", permissions: map[string][]string{ "resources:action1": {Scope("resources", "id", "1")}, "resources:action2": {Scope("resources", "id", "2")}, @@ -47,8 +47,8 @@ func TestGetResourcesMetadata(t *testing.T) { }, }, { - desc: "Should return permissions with global scopes for resources 1,2,3", - resource: "resources", + desc: "Should return permissions with global scopes for resources 1,2,3", + prefix: "resources:id:", permissions: map[string][]string{ "resources:action1": {Scope("resources", "id", "1")}, "resources:action2": {Scope("resources", "id", "2")}, @@ -65,8 +65,8 @@ func TestGetResourcesMetadata(t *testing.T) { }, }, { - desc: "Should correctly filter out irrelevant permissions for resources 1,2,3", - resource: "resources", + desc: "Should correctly filter out irrelevant permissions for resources 1,2,3", + prefix: "resources:id:", permissions: map[string][]string{ "resources:action1": {Scope("resources", "id", "1")}, "resources:action2": {Scope("otherresources", "id", "*")}, @@ -77,22 +77,10 @@ func TestGetResourcesMetadata(t *testing.T) { "1": {"resources:action1": true, "otherresources:action1": true}, }, }, - { - desc: "Should correctly handle permissions with multilayer scope", - resource: "resources:sub", - permissions: map[string][]string{ - "resources:action1": {Scope("resources", "sub", "id", "1"), Scope("resources", "sub", "id", "123")}, - }, - resourcesIDs: map[string]bool{"1": true, "123": true}, - expected: map[string]Metadata{ - "1": {"resources:action1": true}, - "123": {"resources:action1": true}, - }, - }, } for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { - metadata := GetResourcesMetadata(context.Background(), tt.permissions, tt.resource, tt.resourcesIDs) + metadata := GetResourcesMetadata(context.Background(), tt.permissions, tt.prefix, tt.resourcesIDs) assert.EqualValues(t, tt.expected, metadata) }) } diff --git a/pkg/services/accesscontrol/database/resource_permissions.go b/pkg/services/accesscontrol/database/resource_permissions.go index ba1d9f269f1..f54c9584bd9 100644 --- a/pkg/services/accesscontrol/database/resource_permissions.go +++ b/pkg/services/accesscontrol/database/resource_permissions.go @@ -210,7 +210,7 @@ func (s *AccessControlStore) setResourcePermission( ` var current []accesscontrol.Permission - if err := sess.SQL(rawSQL, role.ID, accesscontrol.GetResourceScope(cmd.Resource, cmd.ResourceID)).Find(¤t); err != nil { + if err := sess.SQL(rawSQL, role.ID, accesscontrol.Scope(cmd.Resource, cmd.ResourceAttribute, cmd.ResourceID)).Find(¤t); err != nil { return nil, err } @@ -235,14 +235,14 @@ func (s *AccessControlStore) setResourcePermission( } for action := range missing { - id, err := s.createResourcePermission(sess, role.ID, action, cmd.Resource, cmd.ResourceID) + id, err := s.createResourcePermission(sess, role.ID, action, cmd.Resource, cmd.ResourceID, cmd.ResourceAttribute) if err != nil { return nil, err } keep = append(keep, id) } - permissions, err := s.getResourcePermissionsByIds(sess, cmd.Resource, cmd.ResourceID, keep) + permissions, err := s.getResourcePermissionsByIds(sess, cmd.Resource, cmd.ResourceID, cmd.ResourceAttribute, keep) if err != nil { return nil, err } @@ -267,8 +267,8 @@ func (s *AccessControlStore) GetResourcePermissions(ctx context.Context, orgID i return result, err } -func (s *AccessControlStore) createResourcePermission(sess *sqlstore.DBSession, roleID int64, action, resource string, resourceID string) (int64, error) { - permission := managedPermission(action, resource, resourceID) +func (s *AccessControlStore) createResourcePermission(sess *sqlstore.DBSession, roleID int64, action, resource, resourceID, resourceAttribute string) (int64, error) { + permission := managedPermission(action, resource, resourceID, resourceAttribute) permission.RoleID = roleID permission.Created = time.Now() permission.Updated = time.Now() @@ -340,14 +340,14 @@ func (s *AccessControlStore) getResourcePermissions(sess *sqlstore.DBSession, or where := `WHERE (r.org_id = ? OR r.org_id = 0) AND (p.scope = '*' OR p.scope = ? OR p.scope = ? OR p.scope = ?` - scope := accesscontrol.GetResourceScope(query.Resource, query.ResourceID) + scope := accesscontrol.Scope(query.Resource, query.ResourceAttribute, query.ResourceID) args := []interface{}{ scope, orgID, orgID, - accesscontrol.GetResourceAllScope(query.Resource), - accesscontrol.GetResourceAllIDScope(query.Resource), + accesscontrol.Scope(query.Resource, "*"), + accesscontrol.Scope(query.Resource, query.ResourceAttribute, "*"), scope, } @@ -370,14 +370,14 @@ func (s *AccessControlStore) getResourcePermissions(sess *sqlstore.DBSession, or initialLength := len(args) - userFilter, err := accesscontrol.Filter(query.User, "u.id", "users", accesscontrol.ActionOrgUsersRead) + userFilter, err := accesscontrol.Filter(query.User, "u.id", "users:id:", accesscontrol.ActionOrgUsersRead) if err != nil { return nil, err } user := userSelect + userFrom + where + " AND " + userFilter.Where args = append(args, userFilter.Args...) - teamFilter, err := accesscontrol.Filter(query.User, "t.id", "teams", accesscontrol.ActionTeamsRead) + teamFilter, err := accesscontrol.Filter(query.User, "t.id", "teams:id:", accesscontrol.ActionTeamsRead) if err != nil { return nil, err } @@ -574,7 +574,7 @@ func (s *AccessControlStore) getOrCreateManagedRole(sess *sqlstore.DBSession, or return &role, nil } -func (s *AccessControlStore) getResourcePermissionsByIds(sess *sqlstore.DBSession, resource, resourceID string, ids []int64) ([]flatResourcePermission, error) { +func (s *AccessControlStore) getResourcePermissionsByIds(sess *sqlstore.DBSession, resource, resourceID, resourceAttribute string, ids []int64) ([]flatResourcePermission, error) { var result []flatResourcePermission if len(ids) == 0 { return result, nil @@ -602,7 +602,7 @@ func (s *AccessControlStore) getResourcePermissionsByIds(sess *sqlstore.DBSessio ` args := make([]interface{}, 0, len(ids)+1) - args = append(args, accesscontrol.GetResourceScope(resource, resourceID)) + args = append(args, accesscontrol.Scope(resource, resourceAttribute, resourceID)) for _, id := range ids { args = append(args, id) } @@ -614,9 +614,9 @@ func (s *AccessControlStore) getResourcePermissionsByIds(sess *sqlstore.DBSessio return result, nil } -func managedPermission(action, resource string, resourceID string) accesscontrol.Permission { +func managedPermission(action, resource string, resourceID, resourceAttribute string) accesscontrol.Permission { return accesscontrol.Permission{ Action: action, - Scope: accesscontrol.GetResourceScope(resource, resourceID), + Scope: accesscontrol.Scope(resource, resourceAttribute, resourceID), } } diff --git a/pkg/services/accesscontrol/database/resource_permissions_bench_test.go b/pkg/services/accesscontrol/database/resource_permissions_bench_test.go index 8fddf4446a2..769ae8ec982 100644 --- a/pkg/services/accesscontrol/database/resource_permissions_bench_test.go +++ b/pkg/services/accesscontrol/database/resource_permissions_bench_test.go @@ -56,9 +56,11 @@ func getDSPermissions(b *testing.B, store *AccessControlStore, dataSources []int dsId := dataSources[0] permissions, err := store.GetResourcePermissions(context.Background(), accesscontrol.GlobalOrgID, types.GetResourcePermissionsQuery{ - Actions: []string{dsAction}, - Resource: dsResource, - ResourceID: strconv.Itoa(int(dsId)), + User: &models.SignedInUser{OrgId: 1, Permissions: map[int64]map[string][]string{1: {"org.users:read": {"users:*"}, "teams:read": {"teams:*"}}}}, + Actions: []string{dsAction}, + Resource: dsResource, + ResourceID: strconv.Itoa(int(dsId)), + ResourceAttribute: "id", }) require.NoError(b, err) assert.GreaterOrEqual(b, len(permissions), 2) @@ -96,9 +98,10 @@ func GenerateDatasourcePermissions(b *testing.B, db *sqlstore.SQLStore, ac *Acce accesscontrol.GlobalOrgID, accesscontrol.User{ID: userIds[i]}, types.SetResourcePermissionCommand{ - Actions: []string{dsAction}, - Resource: dsResource, - ResourceID: strconv.Itoa(int(dsID)), + Actions: []string{dsAction}, + Resource: dsResource, + ResourceID: strconv.Itoa(int(dsID)), + ResourceAttribute: "id", }, nil, ) @@ -113,9 +116,10 @@ func GenerateDatasourcePermissions(b *testing.B, db *sqlstore.SQLStore, ac *Acce accesscontrol.GlobalOrgID, teamIds[i], types.SetResourcePermissionCommand{ - Actions: []string{"datasources:query"}, - Resource: "datasources", - ResourceID: strconv.Itoa(int(dsID)), + Actions: []string{"datasources:query"}, + Resource: "datasources", + ResourceID: strconv.Itoa(int(dsID)), + ResourceAttribute: "id", }, nil, ) diff --git a/pkg/services/accesscontrol/database/resource_permissions_test.go b/pkg/services/accesscontrol/database/resource_permissions_test.go index f7ad93de442..6d6d06bb21f 100644 --- a/pkg/services/accesscontrol/database/resource_permissions_test.go +++ b/pkg/services/accesscontrol/database/resource_permissions_test.go @@ -16,31 +16,34 @@ import ( ) type setUserResourcePermissionTest struct { - desc string - orgID int64 - userID int64 - actions []string - resource string - resourceID string - seeds []types.SetResourcePermissionCommand + desc string + orgID int64 + userID int64 + actions []string + resource string + resourceID string + resourceAttribute string + seeds []types.SetResourcePermissionCommand } func TestAccessControlStore_SetUserResourcePermission(t *testing.T) { tests := []setUserResourcePermissionTest{ { - desc: "should set resource permission for user", - userID: 1, - actions: []string{"datasources:query"}, - resource: "datasources", - resourceID: "1", + desc: "should set resource permission for user", + userID: 1, + actions: []string{"datasources:query"}, + resource: "datasources", + resourceID: "1", + resourceAttribute: "uid", }, { - desc: "should remove resource permission for user", - orgID: 1, - userID: 1, - actions: []string{}, - resource: "datasources", - resourceID: "1", + desc: "should remove resource permission for user", + orgID: 1, + userID: 1, + actions: []string{}, + resource: "datasources", + resourceID: "1", + resourceAttribute: "uid", seeds: []types.SetResourcePermissionCommand{ { Actions: []string{"datasources:query"}, @@ -50,12 +53,13 @@ func TestAccessControlStore_SetUserResourcePermission(t *testing.T) { }, }, { - desc: "should add new resource permission for user", - orgID: 1, - userID: 1, - actions: []string{"datasources:query", "datasources:write"}, - resource: "datasources", - resourceID: "1", + desc: "should add new resource permission for user", + orgID: 1, + userID: 1, + actions: []string{"datasources:query", "datasources:write"}, + resource: "datasources", + resourceID: "1", + resourceAttribute: "uid", seeds: []types.SetResourcePermissionCommand{ { Actions: []string{"datasources:write"}, @@ -76,9 +80,10 @@ func TestAccessControlStore_SetUserResourcePermission(t *testing.T) { } added, err := store.SetUserResourcePermission(context.Background(), test.userID, accesscontrol.User{ID: test.userID}, types.SetResourcePermissionCommand{ - Actions: test.actions, - Resource: test.resource, - ResourceID: test.resourceID, + Actions: test.actions, + Resource: test.resource, + ResourceID: test.resourceID, + ResourceAttribute: test.resourceAttribute, }, nil) require.NoError(t, err) @@ -86,59 +91,65 @@ func TestAccessControlStore_SetUserResourcePermission(t *testing.T) { assert.Equal(t, accesscontrol.ResourcePermission{}, *added) } else { assert.Len(t, added.Actions, len(test.actions)) - assert.Equal(t, accesscontrol.GetResourceScope(test.resource, test.resourceID), added.Scope) + assert.Equal(t, accesscontrol.Scope(test.resource, test.resourceAttribute, test.resourceID), added.Scope) } }) } } type setTeamResourcePermissionTest struct { - desc string - orgID int64 - teamID int64 - actions []string - resource string - resourceID string - seeds []types.SetResourcePermissionCommand + desc string + orgID int64 + teamID int64 + actions []string + resource string + resourceID string + resourceAttribute string + seeds []types.SetResourcePermissionCommand } func TestAccessControlStore_SetTeamResourcePermission(t *testing.T) { tests := []setTeamResourcePermissionTest{ { - desc: "should add new resource permission for team", - orgID: 1, - teamID: 1, - actions: []string{"datasources:query"}, - resource: "datasources", - resourceID: "1", + desc: "should add new resource permission for team", + orgID: 1, + teamID: 1, + actions: []string{"datasources:query"}, + resource: "datasources", + resourceID: "1", + resourceAttribute: "uid", }, { - desc: "should add new resource permission when others exist", - orgID: 1, - teamID: 1, - actions: []string{"datasources:query", "datasources:write"}, - resource: "datasources", - resourceID: "1", + desc: "should add new resource permission when others exist", + orgID: 1, + teamID: 1, + actions: []string{"datasources:query", "datasources:write"}, + resource: "datasources", + resourceID: "1", + resourceAttribute: "uid", seeds: []types.SetResourcePermissionCommand{ { - Actions: []string{"datasources:query"}, - Resource: "datasources", - ResourceID: "1", + Actions: []string{"datasources:query"}, + Resource: "datasources", + ResourceID: "1", + ResourceAttribute: "uid", }, }, }, { - desc: "should remove permissions for team", - orgID: 1, - teamID: 1, - actions: []string{}, - resource: "datasources", - resourceID: "1", + desc: "should remove permissions for team", + orgID: 1, + teamID: 1, + actions: []string{}, + resource: "datasources", + resourceID: "1", + resourceAttribute: "uid", seeds: []types.SetResourcePermissionCommand{ { - Actions: []string{"datasources:query"}, - Resource: "datasources", - ResourceID: "1", + Actions: []string{"datasources:query"}, + Resource: "datasources", + ResourceID: "1", + ResourceAttribute: "uid", }, }, }, @@ -154,9 +165,10 @@ func TestAccessControlStore_SetTeamResourcePermission(t *testing.T) { } added, err := store.SetTeamResourcePermission(context.Background(), test.orgID, test.teamID, types.SetResourcePermissionCommand{ - Actions: test.actions, - Resource: test.resource, - ResourceID: test.resourceID, + Actions: test.actions, + Resource: test.resource, + ResourceID: test.resourceID, + ResourceAttribute: test.resourceAttribute, }, nil) require.NoError(t, err) @@ -164,59 +176,65 @@ func TestAccessControlStore_SetTeamResourcePermission(t *testing.T) { assert.Equal(t, accesscontrol.ResourcePermission{}, *added) } else { assert.Len(t, added.Actions, len(test.actions)) - assert.Equal(t, accesscontrol.GetResourceScope(test.resource, test.resourceID), added.Scope) + assert.Equal(t, accesscontrol.Scope(test.resource, test.resourceAttribute, test.resourceID), added.Scope) } }) } } type setBuiltInResourcePermissionTest struct { - desc string - orgID int64 - builtInRole string - actions []string - resource string - resourceID string - seeds []types.SetResourcePermissionCommand + desc string + orgID int64 + builtInRole string + actions []string + resource string + resourceID string + resourceAttribute string + seeds []types.SetResourcePermissionCommand } func TestAccessControlStore_SetBuiltInResourcePermission(t *testing.T) { tests := []setBuiltInResourcePermissionTest{ { - desc: "should add new resource permission for builtin role", - orgID: 1, - builtInRole: "Viewer", - actions: []string{"datasources:query"}, - resource: "datasources", - resourceID: "1", + desc: "should add new resource permission for builtin role", + orgID: 1, + builtInRole: "Viewer", + actions: []string{"datasources:query"}, + resource: "datasources", + resourceID: "1", + resourceAttribute: "uid", }, { - desc: "should add new resource permission when others exist", - orgID: 1, - builtInRole: "Viewer", - actions: []string{"datasources:query", "datasources:write"}, - resource: "datasources", - resourceID: "1", + desc: "should add new resource permission when others exist", + orgID: 1, + builtInRole: "Viewer", + actions: []string{"datasources:query", "datasources:write"}, + resource: "datasources", + resourceID: "1", + resourceAttribute: "uid", seeds: []types.SetResourcePermissionCommand{ { - Actions: []string{"datasources:query"}, - Resource: "datasources", - ResourceID: "1", + Actions: []string{"datasources:query"}, + Resource: "datasources", + ResourceID: "1", + ResourceAttribute: "uid", }, }, }, { - desc: "should remove permissions for builtin role", - orgID: 1, - builtInRole: "Viewer", - actions: []string{}, - resource: "datasources", - resourceID: "1", + desc: "should remove permissions for builtin role", + orgID: 1, + builtInRole: "Viewer", + actions: []string{}, + resource: "datasources", + resourceID: "1", + resourceAttribute: "uid", seeds: []types.SetResourcePermissionCommand{ { - Actions: []string{"datasources:query"}, - Resource: "datasources", - ResourceID: "1", + Actions: []string{"datasources:query"}, + Resource: "datasources", + ResourceID: "1", + ResourceAttribute: "uid", }, }, }, @@ -232,9 +250,10 @@ func TestAccessControlStore_SetBuiltInResourcePermission(t *testing.T) { } added, err := store.SetBuiltInResourcePermission(context.Background(), test.orgID, test.builtInRole, types.SetResourcePermissionCommand{ - Actions: test.actions, - Resource: test.resource, - ResourceID: test.resourceID, + Actions: test.actions, + Resource: test.resource, + ResourceID: test.resourceID, + ResourceAttribute: test.resourceAttribute, }, nil) require.NoError(t, err) @@ -242,46 +261,51 @@ func TestAccessControlStore_SetBuiltInResourcePermission(t *testing.T) { assert.Equal(t, accesscontrol.ResourcePermission{}, *added) } else { assert.Len(t, added.Actions, len(test.actions)) - assert.Equal(t, accesscontrol.GetResourceScope(test.resource, test.resourceID), added.Scope) + assert.Equal(t, accesscontrol.Scope(test.resource, test.resourceAttribute, test.resourceID), added.Scope) } }) } } type setResourcePermissionsTest struct { - desc string - orgID int64 - commands []types.SetResourcePermissionsCommand + desc string + orgID int64 + resourceAttribute string + commands []types.SetResourcePermissionsCommand } func TestAccessControlStore_SetResourcePermissions(t *testing.T) { tests := []setResourcePermissionsTest{ { - desc: "should set all permissions provided", - orgID: 1, + desc: "should set all permissions provided", + orgID: 1, + resourceAttribute: "uid", commands: []types.SetResourcePermissionsCommand{ { User: accesscontrol.User{ID: 1}, SetResourcePermissionCommand: types.SetResourcePermissionCommand{ - Actions: []string{"datasources:query"}, - Resource: "datasources", - ResourceID: "1", + Actions: []string{"datasources:query"}, + Resource: "datasources", + ResourceID: "1", + ResourceAttribute: "uid", }, }, { TeamID: 3, SetResourcePermissionCommand: types.SetResourcePermissionCommand{ - Actions: []string{"datasources:query"}, - Resource: "datasources", - ResourceID: "1", + Actions: []string{"datasources:query"}, + Resource: "datasources", + ResourceID: "1", + ResourceAttribute: "uid", }, }, { BuiltinRole: "Admin", SetResourcePermissionCommand: types.SetResourcePermissionCommand{ - Actions: []string{"datasources:query"}, - Resource: "datasources", - ResourceID: "1", + Actions: []string{"datasources:query"}, + Resource: "datasources", + ResourceID: "1", + ResourceAttribute: "uid", }, }, }, @@ -304,7 +328,7 @@ func TestAccessControlStore_SetResourcePermissions(t *testing.T) { assert.Equal(t, c.TeamID, permissions[i].TeamId) assert.Equal(t, c.User.ID, permissions[i].UserId) assert.Equal(t, c.BuiltinRole, permissions[i].BuiltInRole) - assert.Equal(t, accesscontrol.GetResourceScope(c.Resource, c.ResourceID), permissions[i].Scope) + assert.Equal(t, accesscontrol.Scope(c.Resource, tt.resourceAttribute, c.ResourceID), permissions[i].Scope) } } }) @@ -312,13 +336,14 @@ func TestAccessControlStore_SetResourcePermissions(t *testing.T) { } type getResourcePermissionsTest struct { - desc string - user *models.SignedInUser - numUsers int - actions []string - resource string - resourceID string - onlyManaged bool + desc string + user *models.SignedInUser + numUsers int + actions []string + resource string + resourceID string + resourceAttribute string + onlyManaged bool } func TestAccessControlStore_GetResourcePermissions(t *testing.T) { @@ -330,10 +355,11 @@ func TestAccessControlStore_GetResourcePermissions(t *testing.T) { Permissions: map[int64]map[string][]string{ 1: {accesscontrol.ActionOrgUsersRead: {accesscontrol.ScopeUsersAll}}, }}, - numUsers: 3, - actions: []string{"datasources:query"}, - resource: "datasources", - resourceID: "1", + numUsers: 3, + actions: []string{"datasources:query"}, + resource: "datasources", + resourceID: "1", + resourceAttribute: "uid", }, { desc: "should return manage permissions for all resource ids", @@ -342,11 +368,12 @@ func TestAccessControlStore_GetResourcePermissions(t *testing.T) { Permissions: map[int64]map[string][]string{ 1: {accesscontrol.ActionOrgUsersRead: {accesscontrol.ScopeUsersAll}}, }}, - numUsers: 3, - actions: []string{"datasources:query"}, - resource: "datasources", - resourceID: "1", - onlyManaged: true, + numUsers: 3, + actions: []string{"datasources:query"}, + resource: "datasources", + resourceID: "1", + resourceAttribute: "uid", + onlyManaged: true, }, } @@ -389,14 +416,15 @@ func TestAccessControlStore_GetResourcePermissions(t *testing.T) { }) require.NoError(t, err) - seedResourcePermissions(t, store, sql, test.actions, test.resource, test.resourceID, test.numUsers) + seedResourcePermissions(t, store, sql, test.actions, test.resource, test.resourceID, test.resourceAttribute, test.numUsers) permissions, err := store.GetResourcePermissions(context.Background(), test.user.OrgId, types.GetResourcePermissionsQuery{ - User: test.user, - Actions: test.actions, - Resource: test.resource, - ResourceID: test.resourceID, - OnlyManaged: test.onlyManaged, + User: test.user, + Actions: test.actions, + Resource: test.resource, + ResourceID: test.resourceID, + ResourceAttribute: test.resourceAttribute, + OnlyManaged: test.onlyManaged, }) require.NoError(t, err) @@ -409,7 +437,7 @@ func TestAccessControlStore_GetResourcePermissions(t *testing.T) { } } -func seedResourcePermissions(t *testing.T, store *AccessControlStore, sql *sqlstore.SQLStore, actions []string, resource, resourceID string, numUsers int) { +func seedResourcePermissions(t *testing.T, store *AccessControlStore, sql *sqlstore.SQLStore, actions []string, resource, resourceID, resourceAttribute string, numUsers int) { t.Helper() for i := 0; i < numUsers; i++ { org, _ := sql.GetOrgByName("test") @@ -427,9 +455,10 @@ func seedResourcePermissions(t *testing.T, store *AccessControlStore, sql *sqlst require.NoError(t, err) _, err = store.SetUserResourcePermission(context.Background(), 1, accesscontrol.User{ID: u.Id}, types.SetResourcePermissionCommand{ - Actions: actions, - Resource: resource, - ResourceID: resourceID, + Actions: actions, + Resource: resource, + ResourceID: resourceID, + ResourceAttribute: resourceAttribute, }, nil) require.NoError(t, err) } diff --git a/pkg/services/accesscontrol/filter.go b/pkg/services/accesscontrol/filter.go index b8fb8675fbb..605e39cec78 100644 --- a/pkg/services/accesscontrol/filter.go +++ b/pkg/services/accesscontrol/filter.go @@ -32,6 +32,7 @@ type SQLFilter struct { // Filter creates a where clause to restrict the view of a query based on a users permissions // Scopes that exists for all actions will be parsed and compared against the supplied sqlID +// Prefix parameter is the prefix of the scope that we support (e.g. "users:id:") func Filter(user *models.SignedInUser, sqlID, prefix string, actions ...string) (SQLFilter, error) { if _, ok := sqlIDAcceptList[sqlID]; !ok { return denyQuery, errors.New("sqlID is not in the accept list") @@ -41,7 +42,7 @@ func Filter(user *models.SignedInUser, sqlID, prefix string, actions ...string) } wildcards := 0 - result := make(map[int64]int) + result := make(map[interface{}]int) for _, a := range actions { ids, hasWildcard := parseScopes(prefix, user.Permissions[user.OrgId][a]) if hasWildcard { @@ -84,14 +85,37 @@ func Filter(user *models.SignedInUser, sqlID, prefix string, actions ...string) return SQLFilter{query.String(), ids}, nil } -func parseScopes(prefix string, scopes []string) (ids map[int64]struct{}, hasWildcard bool) { - ids = make(map[int64]struct{}) +func parseScopes(prefix string, scopes []string) (ids map[interface{}]struct{}, hasWildcard bool) { + ids = make(map[interface{}]struct{}) + + rootPrefix, attributePrefix, ok := extractPrefixes(prefix) + if !ok { + return nil, false + } + + parser := parseStringAttribute + if strings.HasSuffix(prefix, "id:") { + parser = parseIntAttribute + } + + allScope := rootPrefix + "*" + allAttributeScope := attributePrefix + "*" + for _, scope := range scopes { - if strings.HasPrefix(scope, prefix) || scope == "*" { - if id := strings.TrimPrefix(scope, prefix); id == "*" || id == ":*" || id == ":id:*" { + if scope == "*" { + return nil, true + } + + if strings.HasPrefix(scope, rootPrefix) { + if scope == allScope || scope == allAttributeScope { return nil, true } - if id, err := parseScopeID(scope); err == nil { + + if !strings.HasPrefix(scope, prefix) { + continue + } + + if id, err := parser(scope); err == nil { ids[id] = struct{}{} } } @@ -99,10 +123,14 @@ func parseScopes(prefix string, scopes []string) (ids map[int64]struct{}, hasWil return ids, false } -func parseScopeID(scope string) (int64, error) { +func parseIntAttribute(scope string) (interface{}, error) { return strconv.ParseInt(scope[strings.LastIndex(scope, ":")+1:], 10, 64) } +func parseStringAttribute(scope string) (interface{}, error) { + return scope[strings.LastIndex(scope, ":")+1:], nil +} + // SetAcceptListForTest allow us to mutate the list for blackbox testing func SetAcceptListForTest(list map[string]struct{}) func() { original := sqlIDAcceptList diff --git a/pkg/services/accesscontrol/filter_bench_test.go b/pkg/services/accesscontrol/filter_bench_test.go index 4fbb816a1f2..a6799e917e2 100644 --- a/pkg/services/accesscontrol/filter_bench_test.go +++ b/pkg/services/accesscontrol/filter_bench_test.go @@ -34,7 +34,7 @@ func benchmarkFilter(b *testing.B, numDs, numPermissions int) { acFilter, err := accesscontrol.Filter( &models.SignedInUser{OrgId: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(permissions)}}, "data_source.id", - "datasources", + "datasources:id:", "datasources:read", ) require.NoError(b, err) diff --git a/pkg/services/accesscontrol/filter_test.go b/pkg/services/accesscontrol/filter_test.go index ac5dc67adf2..ce26e71efaa 100644 --- a/pkg/services/accesscontrol/filter_test.go +++ b/pkg/services/accesscontrol/filter_test.go @@ -29,7 +29,7 @@ func TestFilter_Datasources(t *testing.T) { { desc: "expect all data sources to be returned", sqlID: "data_source.id", - prefix: "datasources", + prefix: "datasources:id:", actions: []string{"datasources:read"}, permissions: map[string][]string{ "datasources:read": {"datasources:*"}, @@ -39,7 +39,7 @@ func TestFilter_Datasources(t *testing.T) { { desc: "expect all data sources for wildcard id scope to be returned", sqlID: "data_source.id", - prefix: "datasources", + prefix: "datasources:id:", actions: []string{"datasources:read"}, permissions: map[string][]string{ "datasources:read": {"datasources:id:*"}, @@ -49,7 +49,7 @@ func TestFilter_Datasources(t *testing.T) { { desc: "expect all data sources for wildcard scope to be returned", sqlID: "data_source.id", - prefix: "datasources", + prefix: "datasources:id:", actions: []string{"datasources:read"}, permissions: map[string][]string{ "datasources:read": {"*"}, @@ -59,7 +59,7 @@ func TestFilter_Datasources(t *testing.T) { { desc: "expect no data sources to be returned", sqlID: "data_source.id", - prefix: "datasources", + prefix: "datasources:id:", actions: []string{"datasources:read"}, permissions: map[string][]string{}, expectedDataSources: []string{}, @@ -67,7 +67,7 @@ func TestFilter_Datasources(t *testing.T) { { desc: "expect data sources with id 3, 7 and 8 to be returned", sqlID: "data_source.id", - prefix: "datasources", + prefix: "datasources:id:", actions: []string{"datasources:read"}, permissions: map[string][]string{ "datasources:read": {"datasources:id:3", "datasources:id:7", "datasources:id:8"}, @@ -77,7 +77,7 @@ func TestFilter_Datasources(t *testing.T) { { desc: "expect no data sources to be returned for malformed scope", sqlID: "data_source.id", - prefix: "datasources", + prefix: "datasources:id:", actions: []string{"datasources:read"}, permissions: map[string][]string{ "datasources:read": {"datasources:id:1*"}, @@ -86,7 +86,7 @@ func TestFilter_Datasources(t *testing.T) { { desc: "expect error if sqlID is not in the accept list", sqlID: "other.id", - prefix: "datasources", + prefix: "datasources:id:", actions: []string{"datasources:read"}, permissions: map[string][]string{ "datasources:read": {"datasources:id:3", "datasources:id:7", "datasources:id:8"}, @@ -96,7 +96,7 @@ func TestFilter_Datasources(t *testing.T) { { desc: "expect data sources that users has several actions for", sqlID: "data_source.id", - prefix: "datasources", + prefix: "datasources:id:", actions: []string{"datasources:read", "datasources:write"}, permissions: map[string][]string{ "datasources:read": {"datasources:id:3", "datasources:id:7", "datasources:id:8"}, @@ -108,7 +108,7 @@ func TestFilter_Datasources(t *testing.T) { { desc: "expect data sources that users has several actions for", sqlID: "data_source.id", - prefix: "datasources", + prefix: "datasources:id:", actions: []string{"datasources:read", "datasources:write"}, permissions: map[string][]string{ "datasources:read": {"datasources:id:3", "datasources:id:7", "datasources:id:8"}, @@ -120,7 +120,7 @@ func TestFilter_Datasources(t *testing.T) { { desc: "expect no data sources when scopes does not match", sqlID: "data_source.id", - prefix: "datasources", + prefix: "datasources:id:", actions: []string{"datasources:read", "datasources:write"}, permissions: map[string][]string{ "datasources:read": {"datasources:id:3", "datasources:id:7", "datasources:id:8"}, @@ -132,7 +132,7 @@ func TestFilter_Datasources(t *testing.T) { { desc: "expect to not crash if duplicates in the scope", sqlID: "data_source.id", - prefix: "datasources", + prefix: "datasources:id:", actions: []string{"datasources:read", "datasources:write"}, permissions: map[string][]string{ "datasources:read": {"datasources:id:3", "datasources:id:7", "datasources:id:8", "datasources:id:3", "datasources:id:8"}, diff --git a/pkg/services/accesscontrol/models.go b/pkg/services/accesscontrol/models.go index ac20cca91f7..30068ab85ba 100644 --- a/pkg/services/accesscontrol/models.go +++ b/pkg/services/accesscontrol/models.go @@ -242,6 +242,7 @@ type SetResourcePermissionCommand struct { const ( GlobalOrgID = 0 + // Permission actions ActionAPIKeyRead = "apikeys:read" diff --git a/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go b/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go index 58b5391a3fd..3ff02acbdea 100644 --- a/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go +++ b/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go @@ -82,8 +82,9 @@ func ProvideTeamPermissions( ac accesscontrol.AccessControl, store resourcepermissions.Store, ) (*resourcepermissions.Service, error) { options := resourcepermissions.Options{ - Resource: "teams", - OnlyManaged: true, + Resource: "teams", + ResourceAttribute: "id", + OnlyManaged: true, ResourceValidator: func(ctx context.Context, orgID int64, resourceID string) error { id, err := strconv.ParseInt(resourceID, 10, 64) if err != nil { @@ -161,7 +162,8 @@ func provideDashboardService( } options := resourcepermissions.Options{ - Resource: "dashboards", + Resource: "dashboards", + ResourceAttribute: "id", ResourceValidator: func(ctx context.Context, orgID int64, resourceID string) error { dashboard, err := getDashboard(ctx, orgID, resourceID) if err != nil { @@ -217,7 +219,8 @@ func provideFolderService( accesscontrol accesscontrol.AccessControl, store resourcepermissions.Store, ) (*resourcepermissions.Service, error) { options := resourcepermissions.Options{ - Resource: "folders", + Resource: "folders", + ResourceAttribute: "id", ResourceValidator: func(ctx context.Context, orgID int64, resourceID string) error { id, err := strconv.ParseInt(resourceID, 10, 64) if err != nil { diff --git a/pkg/services/accesscontrol/resourcepermissions/api.go b/pkg/services/accesscontrol/resourcepermissions/api.go index 8f01da76ba7..582e410b6b1 100644 --- a/pkg/services/accesscontrol/resourcepermissions/api.go +++ b/pkg/services/accesscontrol/resourcepermissions/api.go @@ -35,13 +35,13 @@ func (a *api) registerEndpoints() { uidSolver := solveUID(a.service.options.UidSolver) disable := middleware.Disable(a.ac.IsDisabled()) a.router.Group(fmt.Sprintf("/api/access-control/%s", a.service.options.Resource), func(r routing.RouteRegister) { - idScope := accesscontrol.Scope(a.service.options.Resource, "id", accesscontrol.Parameter(":resourceID")) + scope := accesscontrol.Scope(a.service.options.Resource, a.service.options.ResourceAttribute, accesscontrol.Parameter(":resourceID")) actionWrite, actionRead := fmt.Sprintf("%s.permissions:write", a.service.options.Resource), fmt.Sprintf("%s.permissions:read", a.service.options.Resource) r.Get("/description", auth(disable, accesscontrol.EvalPermission(actionRead)), routing.Wrap(a.getDescription)) - r.Get("/:resourceID", uidSolver, auth(disable, accesscontrol.EvalPermission(actionRead, idScope)), routing.Wrap(a.getPermissions)) - r.Post("/:resourceID/users/:userID", uidSolver, auth(disable, accesscontrol.EvalPermission(actionWrite, idScope)), routing.Wrap(a.setUserPermission)) - r.Post("/:resourceID/teams/:teamID", uidSolver, auth(disable, accesscontrol.EvalPermission(actionWrite, idScope)), routing.Wrap(a.setTeamPermission)) - r.Post("/:resourceID/builtInRoles/:builtInRole", uidSolver, auth(disable, accesscontrol.EvalPermission(actionWrite, idScope)), routing.Wrap(a.setBuiltinRolePermission)) + r.Get("/:resourceID", uidSolver, auth(disable, accesscontrol.EvalPermission(actionRead, scope)), routing.Wrap(a.getPermissions)) + r.Post("/:resourceID/users/:userID", uidSolver, auth(disable, accesscontrol.EvalPermission(actionWrite, scope)), routing.Wrap(a.setUserPermission)) + r.Post("/:resourceID/teams/:teamID", uidSolver, auth(disable, accesscontrol.EvalPermission(actionWrite, scope)), routing.Wrap(a.setTeamPermission)) + r.Post("/:resourceID/builtInRoles/:builtInRole", uidSolver, auth(disable, accesscontrol.EvalPermission(actionWrite, scope)), routing.Wrap(a.setBuiltinRolePermission)) }) } diff --git a/pkg/services/accesscontrol/resourcepermissions/api_test.go b/pkg/services/accesscontrol/resourcepermissions/api_test.go index d1cd6d979e3..db357923d85 100644 --- a/pkg/services/accesscontrol/resourcepermissions/api_test.go +++ b/pkg/services/accesscontrol/resourcepermissions/api_test.go @@ -36,7 +36,8 @@ func TestApi_getDescription(t *testing.T) { { desc: "should return description", options: Options{ - Resource: "dashboards", + Resource: "dashboards", + ResourceAttribute: "uid", Assignments: Assignments{ Users: true, Teams: true, @@ -64,7 +65,8 @@ func TestApi_getDescription(t *testing.T) { { desc: "should only return user assignment", options: Options{ - Resource: "dashboards", + Resource: "dashboards", + ResourceAttribute: "uid", Assignments: Assignments{ Users: true, Teams: false, @@ -90,7 +92,8 @@ func TestApi_getDescription(t *testing.T) { { desc: "should return 403 when missing read permission", options: Options{ - Resource: "dashboards", + Resource: "dashboards", + ResourceAttribute: "uid", Assignments: Assignments{ Users: true, Teams: false, @@ -514,7 +517,8 @@ func contextProvider(tc *testContext) web.Handler { } var testOptions = Options{ - Resource: "dashboards", + Resource: "dashboards", + ResourceAttribute: "id", Assignments: Assignments{ Users: true, Teams: true, diff --git a/pkg/services/accesscontrol/resourcepermissions/options.go b/pkg/services/accesscontrol/resourcepermissions/options.go index ac70712486c..16939e8a3e7 100644 --- a/pkg/services/accesscontrol/resourcepermissions/options.go +++ b/pkg/services/accesscontrol/resourcepermissions/options.go @@ -14,6 +14,8 @@ type InheritedScopesSolver func(ctx context.Context, orgID int64, resourceID str type Options struct { // Resource is the action and scope prefix that is generated Resource string + // ResourceAttribute is the attribute the scope should be based on (e.g. id or uid) + ResourceAttribute string // OnlyManaged will tell the service to return all permissions if set to false and only managed permissions if set to true OnlyManaged bool // ResourceValidator is a validator function that will be called before each assignment. diff --git a/pkg/services/accesscontrol/resourcepermissions/service.go b/pkg/services/accesscontrol/resourcepermissions/service.go index c2039a3dfa8..90291f16a26 100644 --- a/pkg/services/accesscontrol/resourcepermissions/service.go +++ b/pkg/services/accesscontrol/resourcepermissions/service.go @@ -111,12 +111,13 @@ func (s *Service) GetPermissions(ctx context.Context, user *models.SignedInUser, } return s.store.GetResourcePermissions(ctx, user.OrgId, types.GetResourcePermissionsQuery{ - User: user, - Actions: s.actions, - Resource: s.options.Resource, - ResourceID: resourceID, - InheritedScopes: inheritedScopes, - OnlyManaged: s.options.OnlyManaged, + User: user, + Actions: s.actions, + Resource: s.options.Resource, + ResourceID: resourceID, + ResourceAttribute: s.options.ResourceAttribute, + InheritedScopes: inheritedScopes, + OnlyManaged: s.options.OnlyManaged, }) } @@ -135,10 +136,11 @@ func (s *Service) SetUserPermission(ctx context.Context, orgID int64, user acces } return s.store.SetUserResourcePermission(ctx, orgID, user, types.SetResourcePermissionCommand{ - Actions: actions, - Permission: permission, - ResourceID: resourceID, - Resource: s.options.Resource, + Actions: actions, + Permission: permission, + Resource: s.options.Resource, + ResourceID: resourceID, + ResourceAttribute: s.options.ResourceAttribute, }, s.options.OnSetUser) } @@ -157,10 +159,11 @@ func (s *Service) SetTeamPermission(ctx context.Context, orgID, teamID int64, re } return s.store.SetTeamResourcePermission(ctx, orgID, teamID, types.SetResourcePermissionCommand{ - Actions: actions, - Permission: permission, - ResourceID: resourceID, - Resource: s.options.Resource, + Actions: actions, + Permission: permission, + Resource: s.options.Resource, + ResourceID: resourceID, + ResourceAttribute: s.options.ResourceAttribute, }, s.options.OnSetTeam) } @@ -179,10 +182,11 @@ func (s *Service) SetBuiltInRolePermission(ctx context.Context, orgID int64, bui } return s.store.SetBuiltInResourcePermission(ctx, orgID, builtInRole, types.SetResourcePermissionCommand{ - Actions: actions, - Permission: permission, - ResourceID: resourceID, - Resource: s.options.Resource, + Actions: actions, + Permission: permission, + Resource: s.options.Resource, + ResourceID: resourceID, + ResourceAttribute: s.options.ResourceAttribute, }, s.options.OnSetBuiltInRole) } @@ -220,10 +224,11 @@ func (s *Service) SetPermissions( TeamID: cmd.TeamID, BuiltinRole: cmd.BuiltinRole, SetResourcePermissionCommand: types.SetResourcePermissionCommand{ - Actions: actions, - Resource: s.options.Resource, - ResourceID: resourceID, - Permission: cmd.Permission, + Actions: actions, + Resource: s.options.Resource, + ResourceID: resourceID, + ResourceAttribute: s.options.ResourceAttribute, + Permission: cmd.Permission, }, }) } @@ -301,7 +306,7 @@ func (s *Service) declareFixedRoles() error { scopeAll := accesscontrol.Scope(s.options.Resource, "*") readerRole := accesscontrol.RoleRegistration{ Role: accesscontrol.RoleDTO{ - Version: 5, + Version: 6, Name: fmt.Sprintf("fixed:%s.permissions:reader", s.options.Resource), DisplayName: s.options.ReaderRoleName, Group: s.options.RoleGroup, @@ -314,7 +319,7 @@ func (s *Service) declareFixedRoles() error { writerRole := accesscontrol.RoleRegistration{ Role: accesscontrol.RoleDTO{ - Version: 5, + Version: 6, Name: fmt.Sprintf("fixed:%s.permissions:writer", s.options.Resource), DisplayName: s.options.WriterRoleName, Group: s.options.RoleGroup, diff --git a/pkg/services/accesscontrol/resourcepermissions/types/models.go b/pkg/services/accesscontrol/resourcepermissions/types/models.go index 53b52426d67..7733f2394d3 100644 --- a/pkg/services/accesscontrol/resourcepermissions/types/models.go +++ b/pkg/services/accesscontrol/resourcepermissions/types/models.go @@ -6,10 +6,11 @@ import ( ) type SetResourcePermissionCommand struct { - Actions []string - Resource string - ResourceID string - Permission string + Actions []string + Resource string + ResourceID string + ResourceAttribute string + Permission string } type SetResourcePermissionsCommand struct { @@ -21,10 +22,11 @@ type SetResourcePermissionsCommand struct { } type GetResourcePermissionsQuery struct { - Actions []string - Resource string - ResourceID string - OnlyManaged bool - InheritedScopes []string - User *models.SignedInUser + Actions []string + Resource string + ResourceID string + ResourceAttribute string + OnlyManaged bool + InheritedScopes []string + User *models.SignedInUser } diff --git a/pkg/services/serviceaccounts/api/api.go b/pkg/services/serviceaccounts/api/api.go index 6781a76e6d6..90d4b4f2817 100644 --- a/pkg/services/serviceaccounts/api/api.go +++ b/pkg/services/serviceaccounts/api/api.go @@ -147,7 +147,7 @@ func (api *ServiceAccountsAPI) getAccessControlMetadata(c *models.ReqContext, sa return map[string]accesscontrol.Metadata{} } - return accesscontrol.GetResourcesMetadata(c.Req.Context(), permissions, "serviceaccounts", saIDs) + return accesscontrol.GetResourcesMetadata(c.Req.Context(), permissions, "serviceaccounts:id:", saIDs) } func (api *ServiceAccountsAPI) RetrieveServiceAccount(ctx *models.ReqContext) response.Response { diff --git a/pkg/services/serviceaccounts/database/database.go b/pkg/services/serviceaccounts/database/database.go index 21c5bc60200..61647755700 100644 --- a/pkg/services/serviceaccounts/database/database.go +++ b/pkg/services/serviceaccounts/database/database.go @@ -314,7 +314,7 @@ func (s *ServiceAccountsStoreImpl) SearchOrgServiceAccounts( s.sqlStore.Dialect.BooleanStr(true))) if s.sqlStore.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagAccesscontrol) { - acFilter, err := accesscontrol.Filter(signedInUser, "org_user.user_id", "serviceaccounts", serviceaccounts.ActionRead) + acFilter, err := accesscontrol.Filter(signedInUser, "org_user.user_id", "serviceaccounts:id:", serviceaccounts.ActionRead) if err != nil { return err } diff --git a/pkg/services/sqlstore/org_users.go b/pkg/services/sqlstore/org_users.go index 7a36bbe7657..0620f8da89e 100644 --- a/pkg/services/sqlstore/org_users.go +++ b/pkg/services/sqlstore/org_users.go @@ -118,7 +118,7 @@ func (ss *SQLStore) GetOrgUsers(ctx context.Context, query *models.GetOrgUsersQu whereParams = append(whereParams, dialect.BooleanStr(false)) if ss.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagAccesscontrol) && query.User != nil { - acFilter, err := accesscontrol.Filter(query.User, "org_user.user_id", "users", accesscontrol.ActionOrgUsersRead) + acFilter, err := accesscontrol.Filter(query.User, "org_user.user_id", "users:id:", accesscontrol.ActionOrgUsersRead) if err != nil { return err } @@ -181,7 +181,7 @@ func (ss *SQLStore) SearchOrgUsers(ctx context.Context, query *models.SearchOrgU whereConditions = append(whereConditions, fmt.Sprintf("%s.is_service_account = %s", x.Dialect().Quote("user"), ss.Dialect.BooleanStr(false))) if ss.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagAccesscontrol) { - acFilter, err := accesscontrol.Filter(query.User, "org_user.user_id", "users", accesscontrol.ActionOrgUsersRead) + acFilter, err := accesscontrol.Filter(query.User, "org_user.user_id", "users:id:", accesscontrol.ActionOrgUsersRead) if err != nil { return err } diff --git a/pkg/services/sqlstore/permissions/dashboard.go b/pkg/services/sqlstore/permissions/dashboard.go index 3431d9dd563..707fce60812 100644 --- a/pkg/services/sqlstore/permissions/dashboard.go +++ b/pkg/services/sqlstore/permissions/dashboard.go @@ -110,13 +110,13 @@ func (f AccessControlDashboardPermissionFilter) Where() (string, []interface{}) if len(f.dashboardActions) > 0 { builder.WriteString("((") - dashFilter, _ := accesscontrol.Filter(f.User, "dashboard.id", "dashboards", f.dashboardActions...) + dashFilter, _ := accesscontrol.Filter(f.User, "dashboard.id", "dashboards:id:", f.dashboardActions...) builder.WriteString(dashFilter.Where) args = append(args, dashFilter.Args...) builder.WriteString(" OR ") - dashFolderFilter, _ := accesscontrol.Filter(f.User, "dashboard.folder_id", "folders", f.dashboardActions...) + dashFolderFilter, _ := accesscontrol.Filter(f.User, "dashboard.folder_id", "folders:id:", f.dashboardActions...) builder.WriteString(dashFolderFilter.Where) builder.WriteString(") AND NOT dashboard.is_folder)") args = append(args, dashFolderFilter.Args...) @@ -127,7 +127,7 @@ func (f AccessControlDashboardPermissionFilter) Where() (string, []interface{}) builder.WriteString(" OR ") } builder.WriteString("(") - folderFilter, _ := accesscontrol.Filter(f.User, "dashboard.id", "folders", f.folderActions...) + folderFilter, _ := accesscontrol.Filter(f.User, "dashboard.id", "folders:id:", f.folderActions...) builder.WriteString(folderFilter.Where) builder.WriteString(" AND dashboard.is_folder)") args = append(args, folderFilter.Args...) diff --git a/pkg/services/sqlstore/team.go b/pkg/services/sqlstore/team.go index 541b48d5484..abedc2110b3 100644 --- a/pkg/services/sqlstore/team.go +++ b/pkg/services/sqlstore/team.go @@ -229,7 +229,7 @@ func (ss *SQLStore) SearchTeams(ctx context.Context, query *models.SearchTeamsQu err error ) if ss.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagAccesscontrol) { - acFilter, err = ac.Filter(query.SignedInUser, "team.id", "teams", ac.ActionTeamsRead) + acFilter, err = ac.Filter(query.SignedInUser, "team.id", "teams:id:", ac.ActionTeamsRead) if err != nil { return err } @@ -529,7 +529,7 @@ func (ss *SQLStore) GetTeamMembers(ctx context.Context, query *models.GetTeamMem // If the signed in user is not set no member will be returned if ss.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagAccesscontrol) { sqlID := fmt.Sprintf("%s.%s", x.Dialect().Quote("user"), x.Dialect().Quote("id")) - *acFilter, err = ac.Filter(query.SignedInUser, sqlID, "users", ac.ActionOrgUsersRead) + *acFilter, err = ac.Filter(query.SignedInUser, sqlID, "users:id:", ac.ActionOrgUsersRead) if err != nil { return err }