Access control: Filter users and teams by read permissions (#45968)

* pass signed in user and filter based on permissions
This commit is contained in:
Karl Persson
2022-03-01 10:58:41 +01:00
committed by GitHub
parent 07dda8a299
commit 18cbfba596
10 changed files with 83 additions and 27 deletions

View File

@@ -44,7 +44,7 @@ type PermissionsServices interface {
type PermissionsService interface { type PermissionsService interface {
// GetPermissions returns all permissions for given resourceID // 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 sets permission on resource for a user
SetUserPermission(ctx context.Context, orgID int64, user User, resourceID, permission string) (*ResourcePermission, error) SetUserPermission(ctx context.Context, orgID int64, user User, resourceID, permission string) (*ResourcePermission, error)
// SetTeamPermission sets permission on resource for a team // SetTeamPermission sets permission on resource for a team

View File

@@ -359,16 +359,28 @@ func (s *AccessControlStore) getResourcesPermissions(sess *sqlstore.DBSession, o
args = append(args, a) args = append(args, a)
} }
// Need args x3 due to union
initialLength := len(args) initialLength := len(args)
args = append(args, args[:initialLength]...)
args = append(args, args[:initialLength]...)
user := userSelect + userFrom + where userFilter, err := accesscontrol.Filter(context.Background(), "u.id", "users", accesscontrol.ActionOrgUsersRead, query.User)
team := teamSelect + teamFrom + where 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 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) queryResults := make([]flatResourcePermission, 0)
if err := sess.SQL(sql, args...).Find(&queryResults); err != nil { if err := sess.SQL(sql, args...).Find(&queryResults); err != nil {
return nil, err return nil, err

View File

@@ -313,6 +313,7 @@ func TestAccessControlStore_SetResourcePermissions(t *testing.T) {
type getResourcesPermissionsTest struct { type getResourcesPermissionsTest struct {
desc string desc string
user *models.SignedInUser
numUsers int numUsers int
actions []string actions []string
resource string resource string
@@ -324,6 +325,11 @@ func TestAccessControlStore_GetResourcesPermissions(t *testing.T) {
tests := []getResourcesPermissionsTest{ 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, numUsers: 3,
actions: []string{"datasources:query"}, actions: []string{"datasources:query"},
resource: "datasources", resource: "datasources",
@@ -331,6 +337,11 @@ func TestAccessControlStore_GetResourcesPermissions(t *testing.T) {
}, },
{ {
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, numUsers: 3,
actions: []string{"datasources:query"}, actions: []string{"datasources:query"},
resource: "datasources", resource: "datasources",
@@ -345,7 +356,7 @@ func TestAccessControlStore_GetResourcesPermissions(t *testing.T) {
err := sql.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error { err := sql.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
role := &accesscontrol.Role{ role := &accesscontrol.Role{
OrgID: 1, OrgID: test.user.OrgId,
UID: "seeded", UID: "seeded",
Name: "seeded", Name: "seeded",
Updated: time.Now(), Updated: time.Now(),
@@ -382,7 +393,8 @@ func TestAccessControlStore_GetResourcesPermissions(t *testing.T) {
seedResourcePermissions(t, store, sql, test.actions, test.resource, id, test.numUsers) 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, Actions: test.actions,
Resource: test.resource, Resource: test.resource,
ResourceIDs: test.resourceIDs, ResourceIDs: test.resourceIDs,

View File

@@ -12,7 +12,9 @@ import (
var sqlIDAcceptList = map[string]struct{}{ var sqlIDAcceptList = map[string]struct{}{
"org_user.user_id": {}, "org_user.user_id": {},
"role.id": {}, "role.id": {},
"t.id": {},
"team.id": {}, "team.id": {},
"u.id": {},
"\"user\".\"id\"": {}, // For Postgres "\"user\".\"id\"": {}, // For Postgres
"`user`.`id`": {}, // For MySQL and SQLite "`user`.`id`": {}, // For MySQL and SQLite
} }

View File

@@ -5,6 +5,7 @@ import (
"github.com/stretchr/testify/mock" "github.com/stretchr/testify/mock"
"github.com/grafana/grafana/pkg/models"
"github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol"
) )
@@ -14,8 +15,8 @@ type MockPermissionsService struct {
mock.Mock mock.Mock
} }
func (m *MockPermissionsService) GetPermissions(ctx context.Context, orgID int64, resourceID string) ([]accesscontrol.ResourcePermission, error) { func (m *MockPermissionsService) GetPermissions(ctx context.Context, user *models.SignedInUser, resourceID string) ([]accesscontrol.ResourcePermission, error) {
mockedArgs := m.Called(ctx, orgID, resourceID) mockedArgs := m.Called(ctx, user, resourceID)
return mockedArgs.Get(0).([]accesscontrol.ResourcePermission), mockedArgs.Error(1) return mockedArgs.Get(0).([]accesscontrol.ResourcePermission), mockedArgs.Error(1)
} }

View File

@@ -113,7 +113,7 @@ var _ accesscontrol.PermissionsService = new(emptyPermissionsService)
type emptyPermissionsService struct{} 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 return nil, nil
} }

View File

@@ -82,7 +82,7 @@ type resourcePermissionDTO struct {
func (a *api) getPermissions(c *models.ReqContext) response.Response { func (a *api) getPermissions(c *models.ReqContext) response.Response {
resourceID := web.Params(c.Req)[":resourceID"] 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 { if err != nil {
return response.Error(http.StatusInternalServerError, "failed to get permissions", err) return response.Error(http.StatusInternalServerError, "failed to get permissions", err)
} }

View File

@@ -138,7 +138,11 @@ func TestApi_getPermissions(t *testing.T) {
{ {
desc: "expect permissions for resource with id 1", desc: "expect permissions for resource with id 1",
resourceID: "1", resourceID: "1",
permissions: []*accesscontrol.Permission{{Action: "dashboards.permissions:read", Scope: "dashboards:id: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, expectedStatus: 200,
}, },
{ {
@@ -152,7 +156,7 @@ func TestApi_getPermissions(t *testing.T) {
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) { t.Run(tt.desc, func(t *testing.T) {
service, sql := setupTestEnvironment(t, tt.permissions, testOptions) 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) seedPermissions(t, tt.resourceID, sql, service)
@@ -195,6 +199,8 @@ func TestApi_setBuiltinRolePermission(t *testing.T) {
permissions: []*accesscontrol.Permission{ permissions: []*accesscontrol.Permission{
{Action: "dashboards.permissions:read", Scope: "dashboards:id:1"}, {Action: "dashboards.permissions:read", Scope: "dashboards:id:1"},
{Action: "dashboards.permissions:write", 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{ permissions: []*accesscontrol.Permission{
{Action: "dashboards.permissions:read", Scope: "dashboards:id:1"}, {Action: "dashboards.permissions:read", Scope: "dashboards:id:1"},
{Action: "dashboards.permissions:write", 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 { for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) { t.Run(tt.desc, func(t *testing.T) {
service, _ := setupTestEnvironment(t, tt.permissions, testOptions) 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) recorder := setPermission(t, server, testOptions.Resource, tt.resourceID, tt.permission, "builtInRoles", tt.builtInRole)
assert.Equal(t, tt.expectedStatus, recorder.Code) assert.Equal(t, tt.expectedStatus, recorder.Code)
@@ -269,6 +277,8 @@ func TestApi_setTeamPermission(t *testing.T) {
permissions: []*accesscontrol.Permission{ permissions: []*accesscontrol.Permission{
{Action: "dashboards.permissions:read", Scope: "dashboards:id:1"}, {Action: "dashboards.permissions:read", Scope: "dashboards:id:1"},
{Action: "dashboards.permissions:write", 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{ permissions: []*accesscontrol.Permission{
{Action: "dashboards.permissions:read", Scope: "dashboards:id:1"}, {Action: "dashboards.permissions:read", Scope: "dashboards:id:1"},
{Action: "dashboards.permissions:write", 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 { for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) { t.Run(tt.desc, func(t *testing.T) {
service, sql := setupTestEnvironment(t, tt.permissions, testOptions) 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 // seed team
_, err := sql.CreateTeam("test", "test@test.com", 1) _, err := sql.CreateTeam("test", "test@test.com", 1)
@@ -348,6 +360,8 @@ func TestApi_setUserPermission(t *testing.T) {
permissions: []*accesscontrol.Permission{ permissions: []*accesscontrol.Permission{
{Action: "dashboards.permissions:read", Scope: "dashboards:id:1"}, {Action: "dashboards.permissions:read", Scope: "dashboards:id:1"},
{Action: "dashboards.permissions:write", 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{ permissions: []*accesscontrol.Permission{
{Action: "dashboards.permissions:read", Scope: "dashboards:id:1"}, {Action: "dashboards.permissions:read", Scope: "dashboards:id:1"},
{Action: "dashboards.permissions:write", 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 { for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) { t.Run(tt.desc, func(t *testing.T) {
service, sql := setupTestEnvironment(t, tt.permissions, testOptions) 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 // seed user
_, err := sql.CreateUser(context.Background(), models.CreateUserCommand{Login: "test", OrgId: 1}) _, 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 { for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) { 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)) 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) seedPermissions(t, tt.resourceID, sql, service)
permissions, recorder := getPermission(t, server, testOptions.Resource, tt.uid) permissions, recorder := getPermission(t, server, testOptions.Resource, tt.uid)

View File

@@ -98,8 +98,9 @@ type Service struct {
sqlStore *sqlstore.SQLStore sqlStore *sqlstore.SQLStore
} }
func (s *Service) GetPermissions(ctx context.Context, orgID int64, resourceID string) ([]accesscontrol.ResourcePermission, error) { func (s *Service) GetPermissions(ctx context.Context, user *models.SignedInUser, resourceID string) ([]accesscontrol.ResourcePermission, error) {
return s.store.GetResourcesPermissions(ctx, orgID, types.GetResourcesPermissionsQuery{ return s.store.GetResourcesPermissions(ctx, user.OrgId, types.GetResourcesPermissionsQuery{
User: user,
Actions: s.actions, Actions: s.actions,
Resource: s.options.Resource, Resource: s.options.Resource,
ResourceIDs: []string{resourceID}, ResourceIDs: []string{resourceID},

View File

@@ -1,6 +1,9 @@
package types 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 { type SetResourcePermissionCommand struct {
Actions []string Actions []string
@@ -22,4 +25,5 @@ type GetResourcesPermissionsQuery struct {
Resource string Resource string
ResourceIDs []string ResourceIDs []string
OnlyManaged bool OnlyManaged bool
User *models.SignedInUser
} }