diff --git a/pkg/api/api.go b/pkg/api/api.go index 6f06156f0ae..2931c9027de 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -176,8 +176,8 @@ func (hs *HTTPServer) registerRoutes() { apiRoute.Group("/users", func(usersRoute routing.RouteRegister) { userIDScope := ac.Scope("global.users", "id", ac.Parameter(":id")) - usersRoute.Get("/", authorize(reqGrafanaAdmin, ac.EvalPermission(ac.ActionUsersRead, ac.ScopeGlobalUsersAll)), routing.Wrap(hs.searchUsersService.SearchUsers)) - usersRoute.Get("/search", authorize(reqGrafanaAdmin, ac.EvalPermission(ac.ActionUsersRead, ac.ScopeGlobalUsersAll)), routing.Wrap(hs.searchUsersService.SearchUsersWithPaging)) + usersRoute.Get("/", authorize(reqGrafanaAdmin, ac.EvalPermission(ac.ActionUsersRead)), routing.Wrap(hs.searchUsersService.SearchUsers)) + usersRoute.Get("/search", authorize(reqGrafanaAdmin, ac.EvalPermission(ac.ActionUsersRead)), routing.Wrap(hs.searchUsersService.SearchUsersWithPaging)) usersRoute.Get("/:id", authorize(reqGrafanaAdmin, ac.EvalPermission(ac.ActionUsersRead, userIDScope)), routing.Wrap(hs.GetUserByID)) usersRoute.Get("/:id/teams", authorize(reqGrafanaAdmin, ac.EvalPermission(ac.ActionUsersTeamRead, userIDScope)), routing.Wrap(hs.GetUserTeams)) usersRoute.Get("/:id/orgs", authorize(reqGrafanaAdmin, ac.EvalPermission(ac.ActionUsersRead, userIDScope)), routing.Wrap(hs.GetUserOrgList)) @@ -277,7 +277,7 @@ func (hs *HTTPServer) registerRoutes() { orgsRoute.Put("/", authorizeInOrg(reqGrafanaAdmin, ac.UseOrgFromContextParams, ac.EvalPermission(ActionOrgsWrite)), routing.Wrap(hs.UpdateOrg)) orgsRoute.Put("/address", authorizeInOrg(reqGrafanaAdmin, ac.UseOrgFromContextParams, ac.EvalPermission(ActionOrgsWrite)), routing.Wrap(hs.UpdateOrgAddress)) orgsRoute.Delete("/", authorizeInOrg(reqGrafanaAdmin, ac.UseOrgFromContextParams, ac.EvalPermission(ActionOrgsDelete)), routing.Wrap(hs.DeleteOrgByID)) - orgsRoute.Get("/users", authorizeInOrg(reqGrafanaAdmin, ac.UseOrgFromContextParams, ac.EvalPermission(ac.ActionOrgUsersRead, ac.ScopeUsersAll)), routing.Wrap(hs.GetOrgUsers)) + orgsRoute.Get("/users", authorizeInOrg(reqGrafanaAdmin, ac.UseOrgFromContextParams, ac.EvalPermission(ac.ActionOrgUsersRead)), routing.Wrap(hs.GetOrgUsers)) orgsRoute.Post("/users", authorizeInOrg(reqGrafanaAdmin, ac.UseOrgFromContextParams, ac.EvalPermission(ac.ActionOrgUsersAdd, ac.ScopeUsersAll)), routing.Wrap(hs.AddOrgUser)) orgsRoute.Patch("/users/:userId", authorizeInOrg(reqGrafanaAdmin, ac.UseOrgFromContextParams, ac.EvalPermission(ac.ActionOrgUsersRoleUpdate, userIDScope)), routing.Wrap(hs.UpdateOrgUser)) orgsRoute.Delete("/users/:userId", authorizeInOrg(reqGrafanaAdmin, ac.UseOrgFromContextParams, ac.EvalPermission(ac.ActionOrgUsersRemove, userIDScope)), routing.Wrap(hs.RemoveOrgUser)) diff --git a/pkg/models/user.go b/pkg/models/user.go index b2419637364..9ed0996020b 100644 --- a/pkg/models/user.go +++ b/pkg/models/user.go @@ -141,12 +141,13 @@ type GetUserProfileQuery struct { } type SearchUsersQuery struct { - OrgId int64 - Query string - Page int - Limit int - AuthModule string - Filters []Filter + SignedInUser *SignedInUser + OrgId int64 + Query string + Page int + Limit int + AuthModule string + Filters []Filter IsDisabled *bool diff --git a/pkg/services/comments/handlers.go b/pkg/services/comments/handlers.go index 8773f19b4ff..c98fd691944 100644 --- a/pkg/services/comments/handlers.go +++ b/pkg/services/comments/handlers.go @@ -146,7 +146,13 @@ func (s *Service) Get(ctx context.Context, orgID int64, signedInUser *models.Sig } // NOTE: probably replace with comment and user table join. - query := &models.SearchUsersQuery{Query: "", Filters: []models.Filter{NewIDFilter(userIds)}, Page: 0, Limit: len(userIds)} + query := &models.SearchUsersQuery{ + Query: "", + Page: 0, + Limit: len(userIds), + SignedInUser: signedInUser, + Filters: []models.Filter{NewIDFilter(userIds)}, + } if err := s.sqlStore.SearchUsers(ctx, query); err != nil { return nil, err } diff --git a/pkg/services/searchusers/searchusers.go b/pkg/services/searchusers/searchusers.go index fbed2051dae..78b59735c1d 100644 --- a/pkg/services/searchusers/searchusers.go +++ b/pkg/services/searchusers/searchusers.go @@ -61,7 +61,14 @@ func (s *OSSService) SearchUser(c *models.ReqContext) (*models.SearchUsersQuery, } } - query := &models.SearchUsersQuery{Query: searchQuery, Filters: filters, Page: page, Limit: perPage} + query := &models.SearchUsersQuery{ + // added SignedInUser to the query, as to only list the users that the user has permission to read + SignedInUser: c.SignedInUser, + Query: searchQuery, + Filters: filters, + Page: page, + Limit: perPage, + } if err := s.sqlStore.SearchUsers(c.Req.Context(), query); err != nil { return nil, err } diff --git a/pkg/services/sqlstore/user.go b/pkg/services/sqlstore/user.go index 2a029912a37..51b21360002 100644 --- a/pkg/services/sqlstore/user.go +++ b/pkg/services/sqlstore/user.go @@ -620,6 +620,16 @@ func (ss *SQLStore) SearchUsers(ctx context.Context, query *models.SearchUsersQu whereParams = append(whereParams, query.OrgId) } + // user only sees the users for which it has read permissions + if !ac.IsDisabled(ss.Cfg) { + acFilter, err := ac.Filter(query.SignedInUser, "u.id", "global.users:id:", ac.ActionUsersRead) + if err != nil { + return err + } + whereConditions = append(whereConditions, acFilter.Where) + whereParams = append(whereParams, acFilter.Args...) + } + if query.Query != "" { whereConditions = append(whereConditions, "(email "+dialect.LikeStr()+" ? OR name "+dialect.LikeStr()+" ? OR login "+dialect.LikeStr()+" ?)") whereParams = append(whereParams, queryWithWildcards, queryWithWildcards, queryWithWildcards) diff --git a/pkg/services/sqlstore/user_test.go b/pkg/services/sqlstore/user_test.go index bd971fa6f66..7c02e9cfe68 100644 --- a/pkg/services/sqlstore/user_test.go +++ b/pkg/services/sqlstore/user_test.go @@ -9,7 +9,9 @@ import ( "testing" "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/setting" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -336,6 +338,26 @@ func TestUserDataAccess(t *testing.T) { require.Len(t, permQuery.Result, 0) }) + t.Run("Testing DB - return list of users that the SignedInUser has permission to read", func(t *testing.T) { + ss := InitTestDB(t, InitTestDBOpt{FeatureFlags: []string{featuremgmt.FlagAccesscontrol}}) + createFiveTestUsers(t, ss, func(i int) *models.CreateUserCommand { + return &models.CreateUserCommand{ + Email: fmt.Sprint("user", i, "@test.com"), + Name: fmt.Sprint("user", i), + Login: fmt.Sprint("loginuser", i), + } + }) + + testUser := &models.SignedInUser{ + OrgId: 1, + Permissions: map[int64]map[string][]string{1: {"users:read": {"global.users:id:1", "global.users:id:3"}}}, + } + query := models.SearchUsersQuery{SignedInUser: testUser} + err := ss.SearchUsers(context.Background(), &query) + assert.Nil(t, err) + assert.Len(t, query.Result.Users, 2) + }) + ss = InitTestDB(t) t.Run("Testing DB - enable all users", func(t *testing.T) {