From 18cbfba596623991da2cb9b2c7d2bbf6d455157f Mon Sep 17 00:00:00 2001 From: Karl Persson Date: Tue, 1 Mar 2022 10:58:41 +0100 Subject: [PATCH] Access control: Filter users and teams by read permissions (#45968) * pass signed in user and filter based on permissions --- pkg/services/accesscontrol/accesscontrol.go | 2 +- .../database/resource_permissions.go | 24 ++++++++--- .../database/resource_permissions_test.go | 20 +++++++-- pkg/services/accesscontrol/filter.go | 2 + .../accesscontrol/mock/service_mock.go | 5 ++- .../ossaccesscontrol/permissions_services.go | 2 +- .../accesscontrol/resourcepermissions/api.go | 2 +- .../resourcepermissions/api_test.go | 42 +++++++++++++++---- .../resourcepermissions/service.go | 5 ++- .../resourcepermissions/types/models.go | 6 ++- 10 files changed, 83 insertions(+), 27 deletions(-) diff --git a/pkg/services/accesscontrol/accesscontrol.go b/pkg/services/accesscontrol/accesscontrol.go index 03ae32a23a2..015dcd2ab90 100644 --- a/pkg/services/accesscontrol/accesscontrol.go +++ b/pkg/services/accesscontrol/accesscontrol.go @@ -44,7 +44,7 @@ type PermissionsServices interface { type PermissionsService interface { // GetPermissions returns all permissions for given resourceID - GetPermissions(ctx context.Context, orgID int64, resourceID string) ([]ResourcePermission, error) + GetPermissions(ctx context.Context, user *models.SignedInUser, resourceID string) ([]ResourcePermission, error) // SetUserPermission sets permission on resource for a user SetUserPermission(ctx context.Context, orgID int64, user User, resourceID, permission string) (*ResourcePermission, error) // SetTeamPermission sets permission on resource for a team diff --git a/pkg/services/accesscontrol/database/resource_permissions.go b/pkg/services/accesscontrol/database/resource_permissions.go index 68c266db351..ff2c3b39b93 100644 --- a/pkg/services/accesscontrol/database/resource_permissions.go +++ b/pkg/services/accesscontrol/database/resource_permissions.go @@ -359,16 +359,28 @@ func (s *AccessControlStore) getResourcesPermissions(sess *sqlstore.DBSession, o args = append(args, a) } - // Need args x3 due to union initialLength := len(args) - args = append(args, args[:initialLength]...) - args = append(args, args[:initialLength]...) - user := userSelect + userFrom + where - team := teamSelect + teamFrom + where + userFilter, err := accesscontrol.Filter(context.Background(), "u.id", "users", accesscontrol.ActionOrgUsersRead, query.User) + if err != nil { + return nil, err + } + user := userSelect + userFrom + where + " AND " + userFilter.Where + args = append(args, userFilter.Args...) + + teamFilter, err := accesscontrol.Filter(context.Background(), "t.id", "teams", accesscontrol.ActionTeamsRead, query.User) + if err != nil { + return nil, err + } + + team := teamSelect + teamFrom + where + " AND " + teamFilter.Where + args = append(args, args[:initialLength]...) + args = append(args, teamFilter.Args...) + builtin := builtinSelect + builtinFrom + where - sql := user + "UNION" + team + "UNION" + builtin + args = append(args, args[:initialLength]...) + sql := user + " UNION " + team + " UNION " + builtin queryResults := make([]flatResourcePermission, 0) if err := sess.SQL(sql, args...).Find(&queryResults); err != nil { return nil, err diff --git a/pkg/services/accesscontrol/database/resource_permissions_test.go b/pkg/services/accesscontrol/database/resource_permissions_test.go index aa2ac0e35be..415a0b621d7 100644 --- a/pkg/services/accesscontrol/database/resource_permissions_test.go +++ b/pkg/services/accesscontrol/database/resource_permissions_test.go @@ -313,6 +313,7 @@ func TestAccessControlStore_SetResourcePermissions(t *testing.T) { type getResourcesPermissionsTest struct { desc string + user *models.SignedInUser numUsers int actions []string resource string @@ -323,14 +324,24 @@ type getResourcesPermissionsTest struct { func TestAccessControlStore_GetResourcesPermissions(t *testing.T) { tests := []getResourcesPermissionsTest{ { - desc: "should return permissions for all resource ids", + desc: "should return permissions for all resource ids", + user: &models.SignedInUser{ + OrgId: 1, + Permissions: map[int64]map[string][]string{ + 1: {accesscontrol.ActionOrgUsersRead: {accesscontrol.ScopeUsersAll}}, + }}, numUsers: 3, actions: []string{"datasources:query"}, resource: "datasources", resourceIDs: []string{"1", "2"}, }, { - desc: "should return manage permissions for all resource ids", + desc: "should return manage permissions for all resource ids", + user: &models.SignedInUser{ + OrgId: 1, + Permissions: map[int64]map[string][]string{ + 1: {accesscontrol.ActionOrgUsersRead: {accesscontrol.ScopeUsersAll}}, + }}, numUsers: 3, actions: []string{"datasources:query"}, resource: "datasources", @@ -345,7 +356,7 @@ func TestAccessControlStore_GetResourcesPermissions(t *testing.T) { err := sql.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error { role := &accesscontrol.Role{ - OrgID: 1, + OrgID: test.user.OrgId, UID: "seeded", Name: "seeded", Updated: time.Now(), @@ -382,7 +393,8 @@ func TestAccessControlStore_GetResourcesPermissions(t *testing.T) { seedResourcePermissions(t, store, sql, test.actions, test.resource, id, test.numUsers) } - permissions, err := store.GetResourcesPermissions(context.Background(), 1, types.GetResourcesPermissionsQuery{ + permissions, err := store.GetResourcesPermissions(context.Background(), test.user.OrgId, types.GetResourcesPermissionsQuery{ + User: test.user, Actions: test.actions, Resource: test.resource, ResourceIDs: test.resourceIDs, diff --git a/pkg/services/accesscontrol/filter.go b/pkg/services/accesscontrol/filter.go index 26d18d731a5..01e2a83184a 100644 --- a/pkg/services/accesscontrol/filter.go +++ b/pkg/services/accesscontrol/filter.go @@ -12,7 +12,9 @@ import ( var sqlIDAcceptList = map[string]struct{}{ "org_user.user_id": {}, "role.id": {}, + "t.id": {}, "team.id": {}, + "u.id": {}, "\"user\".\"id\"": {}, // For Postgres "`user`.`id`": {}, // For MySQL and SQLite } diff --git a/pkg/services/accesscontrol/mock/service_mock.go b/pkg/services/accesscontrol/mock/service_mock.go index c5f57caf056..fae65edeb72 100644 --- a/pkg/services/accesscontrol/mock/service_mock.go +++ b/pkg/services/accesscontrol/mock/service_mock.go @@ -5,6 +5,7 @@ import ( "github.com/stretchr/testify/mock" + "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/accesscontrol" ) @@ -14,8 +15,8 @@ type MockPermissionsService struct { mock.Mock } -func (m *MockPermissionsService) GetPermissions(ctx context.Context, orgID int64, resourceID string) ([]accesscontrol.ResourcePermission, error) { - mockedArgs := m.Called(ctx, orgID, resourceID) +func (m *MockPermissionsService) GetPermissions(ctx context.Context, user *models.SignedInUser, resourceID string) ([]accesscontrol.ResourcePermission, error) { + mockedArgs := m.Called(ctx, user, resourceID) return mockedArgs.Get(0).([]accesscontrol.ResourcePermission), mockedArgs.Error(1) } diff --git a/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go b/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go index 578819c5cfa..a0babb86346 100644 --- a/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go +++ b/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go @@ -113,7 +113,7 @@ var _ accesscontrol.PermissionsService = new(emptyPermissionsService) type emptyPermissionsService struct{} -func (e emptyPermissionsService) GetPermissions(ctx context.Context, orgID int64, resourceID string) ([]accesscontrol.ResourcePermission, error) { +func (e emptyPermissionsService) GetPermissions(ctx context.Context, user *models.SignedInUser, resourceID string) ([]accesscontrol.ResourcePermission, error) { return nil, nil } diff --git a/pkg/services/accesscontrol/resourcepermissions/api.go b/pkg/services/accesscontrol/resourcepermissions/api.go index 0f90577282d..78b6f6c0a34 100644 --- a/pkg/services/accesscontrol/resourcepermissions/api.go +++ b/pkg/services/accesscontrol/resourcepermissions/api.go @@ -82,7 +82,7 @@ type resourcePermissionDTO struct { func (a *api) getPermissions(c *models.ReqContext) response.Response { resourceID := web.Params(c.Req)[":resourceID"] - permissions, err := a.service.GetPermissions(c.Req.Context(), c.OrgId, resourceID) + permissions, err := a.service.GetPermissions(c.Req.Context(), c.SignedInUser, resourceID) if err != nil { return response.Error(http.StatusInternalServerError, "failed to get permissions", err) } diff --git a/pkg/services/accesscontrol/resourcepermissions/api_test.go b/pkg/services/accesscontrol/resourcepermissions/api_test.go index 515b37f6e75..b64a9b0bcb5 100644 --- a/pkg/services/accesscontrol/resourcepermissions/api_test.go +++ b/pkg/services/accesscontrol/resourcepermissions/api_test.go @@ -136,9 +136,13 @@ type getPermissionsTestCase struct { func TestApi_getPermissions(t *testing.T) { tests := []getPermissionsTestCase{ { - desc: "expect permissions for resource with id 1", - resourceID: "1", - permissions: []*accesscontrol.Permission{{Action: "dashboards.permissions:read", Scope: "dashboards:id:1"}}, + desc: "expect permissions for resource with id 1", + resourceID: "1", + permissions: []*accesscontrol.Permission{ + {Action: "dashboards.permissions:read", Scope: "dashboards:id:1"}, + {Action: accesscontrol.ActionTeamsRead, Scope: accesscontrol.ScopeTeamsAll}, + {Action: accesscontrol.ActionOrgUsersRead, Scope: accesscontrol.ScopeUsersAll}, + }, expectedStatus: 200, }, { @@ -152,7 +156,7 @@ func TestApi_getPermissions(t *testing.T) { for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { service, sql := setupTestEnvironment(t, tt.permissions, testOptions) - server := setupTestServer(t, &models.SignedInUser{OrgId: 1}, service) + server := setupTestServer(t, &models.SignedInUser{OrgId: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.permissions)}}, service) seedPermissions(t, tt.resourceID, sql, service) @@ -195,6 +199,8 @@ func TestApi_setBuiltinRolePermission(t *testing.T) { 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}, + {Action: accesscontrol.ActionOrgUsersRead, Scope: accesscontrol.ScopeUsersAll}, }, }, { @@ -206,6 +212,8 @@ func TestApi_setBuiltinRolePermission(t *testing.T) { 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}, + {Action: accesscontrol.ActionOrgUsersRead, Scope: accesscontrol.ScopeUsersAll}, }, }, { @@ -234,7 +242,7 @@ func TestApi_setBuiltinRolePermission(t *testing.T) { for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { service, _ := setupTestEnvironment(t, tt.permissions, testOptions) - server := setupTestServer(t, &models.SignedInUser{OrgId: 1}, service) + server := setupTestServer(t, &models.SignedInUser{OrgId: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.permissions)}}, service) recorder := setPermission(t, server, testOptions.Resource, tt.resourceID, tt.permission, "builtInRoles", tt.builtInRole) assert.Equal(t, tt.expectedStatus, recorder.Code) @@ -269,6 +277,8 @@ func TestApi_setTeamPermission(t *testing.T) { 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}, + {Action: accesscontrol.ActionOrgUsersRead, Scope: accesscontrol.ScopeUsersAll}, }, }, { @@ -280,6 +290,8 @@ func TestApi_setTeamPermission(t *testing.T) { 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}, + {Action: accesscontrol.ActionOrgUsersRead, Scope: accesscontrol.ScopeUsersAll}, }, }, { @@ -308,7 +320,7 @@ func TestApi_setTeamPermission(t *testing.T) { for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { service, sql := setupTestEnvironment(t, tt.permissions, testOptions) - server := setupTestServer(t, &models.SignedInUser{OrgId: 1}, service) + server := setupTestServer(t, &models.SignedInUser{OrgId: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.permissions)}}, service) // seed team _, err := sql.CreateTeam("test", "test@test.com", 1) @@ -348,6 +360,8 @@ func TestApi_setUserPermission(t *testing.T) { 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}, + {Action: accesscontrol.ActionOrgUsersRead, Scope: accesscontrol.ScopeUsersAll}, }, }, { @@ -359,6 +373,8 @@ func TestApi_setUserPermission(t *testing.T) { 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}, + {Action: accesscontrol.ActionOrgUsersRead, Scope: accesscontrol.ScopeUsersAll}, }, }, { @@ -387,7 +403,7 @@ func TestApi_setUserPermission(t *testing.T) { for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { service, sql := setupTestEnvironment(t, tt.permissions, testOptions) - server := setupTestServer(t, &models.SignedInUser{OrgId: 1}, service) + server := setupTestServer(t, &models.SignedInUser{OrgId: 1, Permissions: map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tt.permissions)}}, service) // seed user _, err := sql.CreateUser(context.Background(), models.CreateUserCommand{Login: "test", OrgId: 1}) @@ -432,9 +448,17 @@ func TestApi_UidSolver(t *testing.T) { for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { - userPermissions := []*accesscontrol.Permission{{Action: "dashboards.permissions:read", Scope: "dashboards:id:1"}} + userPermissions := []*accesscontrol.Permission{ + {Action: "dashboards.permissions:read", Scope: "dashboards:id:1"}, + {Action: accesscontrol.ActionTeamsRead, Scope: accesscontrol.ScopeTeamsAll}, + {Action: accesscontrol.ActionOrgUsersRead, Scope: accesscontrol.ScopeUsersAll}, + } + service, sql := setupTestEnvironment(t, userPermissions, withSolver(testOptions, testSolver)) - server := setupTestServer(t, &models.SignedInUser{OrgId: 1}, service) + server := setupTestServer(t, &models.SignedInUser{OrgId: 1, Permissions: map[int64]map[string][]string{ + 1: accesscontrol.GroupScopesByAction(userPermissions), + }}, service) + seedPermissions(t, tt.resourceID, sql, service) permissions, recorder := getPermission(t, server, testOptions.Resource, tt.uid) diff --git a/pkg/services/accesscontrol/resourcepermissions/service.go b/pkg/services/accesscontrol/resourcepermissions/service.go index 191799af0c5..f0acd494b67 100644 --- a/pkg/services/accesscontrol/resourcepermissions/service.go +++ b/pkg/services/accesscontrol/resourcepermissions/service.go @@ -98,8 +98,9 @@ type Service struct { sqlStore *sqlstore.SQLStore } -func (s *Service) GetPermissions(ctx context.Context, orgID int64, resourceID string) ([]accesscontrol.ResourcePermission, error) { - return s.store.GetResourcesPermissions(ctx, orgID, types.GetResourcesPermissionsQuery{ +func (s *Service) GetPermissions(ctx context.Context, user *models.SignedInUser, resourceID string) ([]accesscontrol.ResourcePermission, error) { + return s.store.GetResourcesPermissions(ctx, user.OrgId, types.GetResourcesPermissionsQuery{ + User: user, Actions: s.actions, Resource: s.options.Resource, ResourceIDs: []string{resourceID}, diff --git a/pkg/services/accesscontrol/resourcepermissions/types/models.go b/pkg/services/accesscontrol/resourcepermissions/types/models.go index 36f4072dde0..7f3b662d9ad 100644 --- a/pkg/services/accesscontrol/resourcepermissions/types/models.go +++ b/pkg/services/accesscontrol/resourcepermissions/types/models.go @@ -1,6 +1,9 @@ package types -import "github.com/grafana/grafana/pkg/services/accesscontrol" +import ( + "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/accesscontrol" +) type SetResourcePermissionCommand struct { Actions []string @@ -22,4 +25,5 @@ type GetResourcesPermissionsQuery struct { Resource string ResourceIDs []string OnlyManaged bool + User *models.SignedInUser }