From 22788d1d861094170b993d3f1a659d2a79876826 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Agn=C3=A8s=20Toulet?= <35176601+AgnesToulet@users.noreply.github.com> Date: Tue, 24 Nov 2020 12:10:32 +0100 Subject: [PATCH] Add an option to hide certain users in the UI (#28942) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add an option to hide certain users in the UI * revert changes for admin users routes * fix sqlstore function name * Improve slice management Co-authored-by: Emil Tullstedt * Hidden users: convert slice to map * filter with user logins instead of IDs * put HiddenUsers in Cfg struct * hide hidden users from dashboards/folders permissions list * Update conf/defaults.ini Co-authored-by: Torkel Ödegaard * fix params order * fix tests * fix dashboard/folder update with hidden user * add team tests * add dashboard and folder permissions tests * fixes after merge * fix tests * API: add test for org users endpoints * update hidden users management for dashboard / folder permissions * improve dashboard / folder permissions tests * fixes after merge * Guardian: add hidden acl tests * API: add team members tests * fix team sql syntax for postgres * api tests update * fix linter error * fix tests errors after merge Co-authored-by: Emil Tullstedt Co-authored-by: Torkel Ödegaard Co-authored-by: Leonard Gram --- conf/defaults.ini | 3 + conf/sample.ini | 3 + pkg/api/api.go | 16 +- pkg/api/common_test.go | 1 + pkg/api/dashboard_permission.go | 19 ++- pkg/api/dashboard_permission_test.go | 191 ++++++++++++++++++---- pkg/api/datasources_test.go | 5 +- pkg/api/dtos/models.go | 12 ++ pkg/api/folder_permission.go | 19 ++- pkg/api/folder_permission_test.go | 213 ++++++++++++++++++++----- pkg/api/org_users.go | 47 ++++-- pkg/api/org_users_test.go | 126 +++++++++++++++ pkg/api/team.go | 11 +- pkg/api/team_members.go | 9 +- pkg/api/team_members_test.go | 73 +++++++++ pkg/models/team.go | 10 +- pkg/services/guardian/guardian.go | 38 +++++ pkg/services/guardian/guardian_test.go | 51 ++++++ pkg/services/sqlstore/team.go | 83 +++++++--- pkg/services/sqlstore/team_test.go | 47 ++++++ pkg/setting/setting.go | 8 + 21 files changed, 852 insertions(+), 133 deletions(-) create mode 100644 pkg/api/org_users_test.go create mode 100644 pkg/api/team_members_test.go diff --git a/conf/defaults.ini b/conf/defaults.ini index 2a5dbf66609..1d6ee4d9156 100644 --- a/conf/defaults.ini +++ b/conf/defaults.ini @@ -296,6 +296,9 @@ editors_can_admin = false # The duration in time a user invitation remains valid before expiring. This setting should be expressed as a duration. Examples: 6h (hours), 2d (days), 1w (week). Default is 24h (24 hours). The minimum supported duration is 15m (15 minutes). user_invite_max_lifetime_duration = 24h +# Enter a comma-separated list of usernames to hide them in the Grafana UI. These users are shown to Grafana admins and to themselves. +hidden_users = + [auth] # Login cookie name login_cookie_name = grafana_session diff --git a/conf/sample.ini b/conf/sample.ini index ee5106154f3..b35247e72ac 100644 --- a/conf/sample.ini +++ b/conf/sample.ini @@ -295,6 +295,9 @@ # The duration in time a user invitation remains valid before expiring. This setting should be expressed as a duration. Examples: 6h (hours), 2d (days), 1w (week). Default is 24h (24 hours). The minimum supported duration is 15m (15 minutes). ;user_invite_max_lifetime_duration = 24h +# Enter a comma-separated list of users login to hide them in the Grafana UI. These users are shown to Grafana admins and themselves. +; hidden_users = + [auth] # Login cookie name ;login_cookie_name = grafana_session diff --git a/pkg/api/api.go b/pkg/api/api.go index 86a36468ba7..d61ca822feb 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -170,7 +170,7 @@ func (hs *HTTPServer) registerRoutes() { // team without requirement of user to be org admin apiRoute.Group("/teams", func(teamsRoute routing.RouteRegister) { - teamsRoute.Get("/:teamId", Wrap(GetTeamByID)) + teamsRoute.Get("/:teamId", Wrap(hs.GetTeamByID)) teamsRoute.Get("/search", Wrap(hs.SearchTeams)) }) @@ -184,7 +184,7 @@ func (hs *HTTPServer) registerRoutes() { apiRoute.Group("/org", func(orgRoute routing.RouteRegister) { orgRoute.Put("/", bind(dtos.UpdateOrgForm{}), Wrap(UpdateOrgCurrent)) orgRoute.Put("/address", bind(dtos.UpdateOrgAddressForm{}), Wrap(UpdateOrgAddressCurrent)) - orgRoute.Get("/users", Wrap(GetOrgUsersForCurrentOrg)) + orgRoute.Get("/users", Wrap(hs.GetOrgUsersForCurrentOrg)) orgRoute.Post("/users", quota("user"), bind(models.AddOrgUserCommand{}), Wrap(AddOrgUserToCurrentOrg)) orgRoute.Patch("/users/:userId", bind(models.UpdateOrgUserCommand{}), Wrap(UpdateOrgUserForCurrentOrg)) orgRoute.Delete("/users/:userId", Wrap(RemoveOrgUserForCurrentOrg)) @@ -201,7 +201,7 @@ func (hs *HTTPServer) registerRoutes() { // current org without requirement of user to be org admin apiRoute.Group("/org", func(orgRoute routing.RouteRegister) { - orgRoute.Get("/users/lookup", Wrap(GetOrgUsersForCurrentOrgLookup)) + orgRoute.Get("/users/lookup", Wrap(hs.GetOrgUsersForCurrentOrgLookup)) }) // create new org @@ -216,7 +216,7 @@ func (hs *HTTPServer) registerRoutes() { orgsRoute.Put("/", bind(dtos.UpdateOrgForm{}), Wrap(UpdateOrg)) orgsRoute.Put("/address", bind(dtos.UpdateOrgAddressForm{}), Wrap(UpdateOrgAddress)) orgsRoute.Delete("/", Wrap(DeleteOrgByID)) - orgsRoute.Get("/users", Wrap(GetOrgUsers)) + orgsRoute.Get("/users", Wrap(hs.GetOrgUsers)) orgsRoute.Post("/users", bind(models.AddOrgUserCommand{}), Wrap(AddOrgUser)) orgsRoute.Patch("/users/:userId", bind(models.UpdateOrgUserCommand{}), Wrap(UpdateOrgUser)) orgsRoute.Delete("/users/:userId", Wrap(RemoveOrgUser)) @@ -287,8 +287,8 @@ func (hs *HTTPServer) registerRoutes() { folderUidRoute.Delete("/", Wrap(DeleteFolder)) folderUidRoute.Group("/permissions", func(folderPermissionRoute routing.RouteRegister) { - folderPermissionRoute.Get("/", Wrap(GetFolderPermissionList)) - folderPermissionRoute.Post("/", bind(dtos.UpdateDashboardAclCommand{}), Wrap(UpdateFolderPermissions)) + folderPermissionRoute.Get("/", Wrap(hs.GetFolderPermissionList)) + folderPermissionRoute.Post("/", bind(dtos.UpdateDashboardAclCommand{}), Wrap(hs.UpdateFolderPermissions)) }) }) }) @@ -314,8 +314,8 @@ func (hs *HTTPServer) registerRoutes() { dashIdRoute.Post("/restore", bind(dtos.RestoreDashboardVersionCommand{}), Wrap(hs.RestoreDashboardVersion)) dashIdRoute.Group("/permissions", func(dashboardPermissionRoute routing.RouteRegister) { - dashboardPermissionRoute.Get("/", Wrap(GetDashboardPermissionList)) - dashboardPermissionRoute.Post("/", bind(dtos.UpdateDashboardAclCommand{}), Wrap(UpdateDashboardPermissions)) + dashboardPermissionRoute.Get("/", Wrap(hs.GetDashboardPermissionList)) + dashboardPermissionRoute.Post("/", bind(dtos.UpdateDashboardAclCommand{}), Wrap(hs.UpdateDashboardPermissions)) }) }) }) diff --git a/pkg/api/common_test.go b/pkg/api/common_test.go index dd951fab78f..c1a4c358f76 100644 --- a/pkg/api/common_test.go +++ b/pkg/api/common_test.go @@ -28,6 +28,7 @@ func loggedInUserScenarioWithRole(t *testing.T, desc string, method string, url sc.context = c sc.context.UserId = testUserID sc.context.OrgId = testOrgID + sc.context.Login = testUserLogin sc.context.OrgRole = role if sc.handlerFunc != nil { return sc.handlerFunc(sc.context) diff --git a/pkg/api/dashboard_permission.go b/pkg/api/dashboard_permission.go index ca5641444cc..ec907f0f869 100644 --- a/pkg/api/dashboard_permission.go +++ b/pkg/api/dashboard_permission.go @@ -10,7 +10,7 @@ import ( "github.com/grafana/grafana/pkg/services/guardian" ) -func GetDashboardPermissionList(c *models.ReqContext) Response { +func (hs *HTTPServer) GetDashboardPermissionList(c *models.ReqContext) Response { dashID := c.ParamsInt64(":dashboardId") _, rsp := getDashboardHelper(c.OrgId, "", dashID, "") @@ -29,7 +29,12 @@ func GetDashboardPermissionList(c *models.ReqContext) Response { return Error(500, "Failed to get dashboard permissions", err) } + filteredAcls := make([]*models.DashboardAclInfoDTO, 0, len(acl)) for _, perm := range acl { + if dtos.IsHiddenUser(perm.UserLogin, c.SignedInUser, hs.Cfg) { + continue + } + perm.UserAvatarUrl = dtos.GetGravatarUrl(perm.UserEmail) if perm.TeamId > 0 { @@ -38,12 +43,14 @@ func GetDashboardPermissionList(c *models.ReqContext) Response { if perm.Slug != "" { perm.Url = models.GetDashboardFolderUrl(perm.IsFolder, perm.Uid, perm.Slug) } + + filteredAcls = append(filteredAcls, perm) } - return JSON(200, acl) + return JSON(200, filteredAcls) } -func UpdateDashboardPermissions(c *models.ReqContext, apiCmd dtos.UpdateDashboardAclCommand) Response { +func (hs *HTTPServer) UpdateDashboardPermissions(c *models.ReqContext, apiCmd dtos.UpdateDashboardAclCommand) Response { if err := validatePermissionsUpdate(apiCmd); err != nil { return Error(400, err.Error(), err) } @@ -76,6 +83,12 @@ func UpdateDashboardPermissions(c *models.ReqContext, apiCmd dtos.UpdateDashboar }) } + hiddenACL, err := g.GetHiddenACL(hs.Cfg) + if err != nil { + return Error(500, "Error while retrieving hidden permissions", err) + } + cmd.Items = append(cmd.Items, hiddenACL...) + if okToUpdate, err := g.CheckPermissionBeforeUpdate(models.PERMISSION_ADMIN, cmd.Items); err != nil || !okToUpdate { if err != nil { if errors.Is(err, guardian.ErrGuardianPermissionExists) || errors.Is(err, guardian.ErrGuardianOverride) { diff --git a/pkg/api/dashboard_permission_test.go b/pkg/api/dashboard_permission_test.go index 8acdee935de..01738250c68 100644 --- a/pkg/api/dashboard_permission_test.go +++ b/pkg/api/dashboard_permission_test.go @@ -1,20 +1,24 @@ package api import ( + "encoding/json" "fmt" "testing" "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/bus" - "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/guardian" + "github.com/grafana/grafana/pkg/setting" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) func TestDashboardPermissionAPIEndpoint(t *testing.T) { t.Run("Dashboard permissions test", func(t *testing.T) { + settings := setting.NewCfg() + hs := &HTTPServer{Cfg: settings} + t.Run("Given dashboard not exists", func(t *testing.T) { setUp := func() { bus.AddHandler("test", func(query *models.GetDashboardQuery) error { @@ -25,7 +29,7 @@ func TestDashboardPermissionAPIEndpoint(t *testing.T) { loggedInUserScenarioWithRole(t, "When calling GET on", "GET", "/api/dashboards/id/1/permissions", "/api/dashboards/id/:id/permissions", models.ROLE_EDITOR, func(sc *scenarioContext) { setUp() - callGetDashboardPermissions(sc) + callGetDashboardPermissions(sc, hs) assert.Equal(t, 404, sc.resp.Code) }) @@ -35,12 +39,17 @@ func TestDashboardPermissionAPIEndpoint(t *testing.T) { }, } - updateDashboardPermissionScenario(t, "When calling POST on", "/api/dashboards/id/1/permissions", - "/api/dashboards/id/:id/permissions", cmd, func(sc *scenarioContext) { + updateDashboardPermissionScenario(t, updatePermissionContext{ + desc: "When calling POST on", + url: "/api/dashboards/id/1/permissions", + routePattern: "/api/dashboards/id/:id/permissions", + cmd: cmd, + fn: func(sc *scenarioContext) { setUp() callUpdateDashboardPermissions(sc) assert.Equal(t, 404, sc.resp.Code) - }) + }, + }, hs) }) t.Run("Given user has no admin permissions", func(t *testing.T) { @@ -63,7 +72,7 @@ func TestDashboardPermissionAPIEndpoint(t *testing.T) { loggedInUserScenarioWithRole(t, "When calling GET on", "GET", "/api/dashboards/id/1/permissions", "/api/dashboards/id/:id/permissions", models.ROLE_EDITOR, func(sc *scenarioContext) { setUp() - callGetDashboardPermissions(sc) + callGetDashboardPermissions(sc, hs) assert.Equal(t, 403, sc.resp.Code) }) @@ -73,12 +82,17 @@ func TestDashboardPermissionAPIEndpoint(t *testing.T) { }, } - updateDashboardPermissionScenario(t, "When calling POST on", "/api/dashboards/id/1/permissions", - "/api/dashboards/id/:id/permissions", cmd, func(sc *scenarioContext) { + updateDashboardPermissionScenario(t, updatePermissionContext{ + desc: "When calling POST on", + url: "/api/dashboards/id/1/permissions", + routePattern: "/api/dashboards/id/:id/permissions", + cmd: cmd, + fn: func(sc *scenarioContext) { setUp() callUpdateDashboardPermissions(sc) assert.Equal(t, 403, sc.resp.Code) - }) + }, + }, hs) }) t.Run("Given user has admin permissions and permissions to update", func(t *testing.T) { @@ -110,13 +124,16 @@ func TestDashboardPermissionAPIEndpoint(t *testing.T) { loggedInUserScenarioWithRole(t, "When calling GET on", "GET", "/api/dashboards/id/1/permissions", "/api/dashboards/id/:id/permissions", models.ROLE_ADMIN, func(sc *scenarioContext) { setUp() - callGetDashboardPermissions(sc) + callGetDashboardPermissions(sc, hs) assert.Equal(t, 200, sc.resp.Code) - respJSON, err := simplejson.NewJson(sc.resp.Body.Bytes()) + + var resp []*models.DashboardAclInfoDTO + err := json.Unmarshal(sc.resp.Body.Bytes(), &resp) require.NoError(t, err) - assert.Equal(t, 5, len(respJSON.MustArray())) - assert.Equal(t, 2, respJSON.GetIndex(0).Get("userId").MustInt()) - assert.Equal(t, int(models.PERMISSION_VIEW), respJSON.GetIndex(0).Get("permission").MustInt()) + + assert.Len(t, resp, 5) + assert.Equal(t, int64(2), resp[0].UserId) + assert.Equal(t, models.PERMISSION_VIEW, resp[0].Permission) }) cmd := dtos.UpdateDashboardAclCommand{ @@ -125,12 +142,17 @@ func TestDashboardPermissionAPIEndpoint(t *testing.T) { }, } - updateDashboardPermissionScenario(t, "When calling POST on", "/api/dashboards/id/1/permissions", - "/api/dashboards/id/:id/permissions", cmd, func(sc *scenarioContext) { + updateDashboardPermissionScenario(t, updatePermissionContext{ + desc: "When calling POST on", + url: "/api/dashboards/id/1/permissions", + routePattern: "/api/dashboards/id/:id/permissions", + cmd: cmd, + fn: func(sc *scenarioContext) { setUp() callUpdateDashboardPermissions(sc) assert.Equal(t, 200, sc.resp.Code) - }) + }, + }, hs) }) t.Run("When trying to update permissions with duplicate permissions", func(t *testing.T) { @@ -158,12 +180,17 @@ func TestDashboardPermissionAPIEndpoint(t *testing.T) { }, } - updateDashboardPermissionScenario(t, "When calling POST on", "/api/dashboards/id/1/permissions", - "/api/dashboards/id/:id/permissions", cmd, func(sc *scenarioContext) { + updateDashboardPermissionScenario(t, updatePermissionContext{ + desc: "When calling POST on", + url: "/api/dashboards/id/1/permissions", + routePattern: "/api/dashboards/id/:id/permissions", + cmd: cmd, + fn: func(sc *scenarioContext) { setUp() callUpdateDashboardPermissions(sc) assert.Equal(t, 400, sc.resp.Code) - }) + }, + }, hs) }) t.Run("When trying to update team or user permissions with a role", func(t *testing.T) { @@ -182,14 +209,19 @@ func TestDashboardPermissionAPIEndpoint(t *testing.T) { } for _, cmd := range cmds { - updateDashboardPermissionScenario(t, "When calling POST on", "/api/dashboards/id/1/permissions", - "/api/dashboards/id/:id/permissions", cmd, func(sc *scenarioContext) { + updateDashboardPermissionScenario(t, updatePermissionContext{ + desc: "When calling POST on", + url: "/api/dashboards/id/1/permissions", + routePattern: "/api/dashboards/id/:id/permissions", + cmd: cmd, + fn: func(sc *scenarioContext) { callUpdateDashboardPermissions(sc) assert.Equal(t, 400, sc.resp.Code) respJSON, err := jsonMap(sc.resp.Body.Bytes()) require.NoError(t, err) assert.Equal(t, models.ErrPermissionsWithRoleNotAllowed.Error(), respJSON["error"]) - }) + }, + }, hs) } }) @@ -219,18 +251,103 @@ func TestDashboardPermissionAPIEndpoint(t *testing.T) { }, } - updateDashboardPermissionScenario(t, "When calling POST on", "/api/dashboards/id/1/permissions", - "/api/dashboards/id/:id/permissions", cmd, func(sc *scenarioContext) { + updateDashboardPermissionScenario(t, updatePermissionContext{ + desc: "When calling POST on", + url: "/api/dashboards/id/1/permissions", + routePattern: "/api/dashboards/id/:id/permissions", + cmd: cmd, + fn: func(sc *scenarioContext) { setUp() callUpdateDashboardPermissions(sc) assert.Equal(t, 400, sc.resp.Code) + }, + }, hs) + }) + + t.Run("Getting and updating dashboard permissions with hidden users", func(t *testing.T) { + origNewGuardian := guardian.New + settings.HiddenUsers = map[string]struct{}{ + "hiddenUser": {}, + testUserLogin: {}, + } + t.Cleanup(func() { + guardian.New = origNewGuardian + settings.HiddenUsers = make(map[string]struct{}) + }) + + guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{ + CanAdminValue: true, + CheckPermissionBeforeUpdateValue: true, + GetAclValue: []*models.DashboardAclInfoDTO{ + {OrgId: 1, DashboardId: 1, UserId: 2, UserLogin: "hiddenUser", Permission: models.PERMISSION_VIEW}, + {OrgId: 1, DashboardId: 1, UserId: 3, UserLogin: testUserLogin, Permission: models.PERMISSION_EDIT}, + {OrgId: 1, DashboardId: 1, UserId: 4, UserLogin: "user_1", Permission: models.PERMISSION_ADMIN}, + }, + GetHiddenAclValue: []*models.DashboardAcl{ + {OrgID: 1, DashboardID: 1, UserID: 2, Permission: models.PERMISSION_VIEW}, + }, + }) + + setUp := func() { + getDashboardQueryResult := models.NewDashboard("Dash") + bus.AddHandler("test", func(query *models.GetDashboardQuery) error { + query.Result = getDashboardQueryResult + return nil }) + } + + var resp []*models.DashboardAclInfoDTO + loggedInUserScenarioWithRole(t, "When calling GET on", "GET", "/api/dashboards/id/1/permissions", + "/api/dashboards/id/:id/permissions", models.ROLE_ADMIN, func(sc *scenarioContext) { + setUp() + callGetDashboardPermissions(sc, hs) + assert.Equal(t, 200, sc.resp.Code) + + err := json.Unmarshal(sc.resp.Body.Bytes(), &resp) + require.NoError(t, err) + + assert.Len(t, resp, 2) + assert.Equal(t, int64(3), resp[0].UserId) + assert.Equal(t, models.PERMISSION_EDIT, resp[0].Permission) + assert.Equal(t, int64(4), resp[1].UserId) + assert.Equal(t, models.PERMISSION_ADMIN, resp[1].Permission) + }) + + cmd := dtos.UpdateDashboardAclCommand{ + Items: []dtos.DashboardAclUpdateItem{ + {UserID: 1000, Permission: models.PERMISSION_ADMIN}, + }, + } + for _, acl := range resp { + cmd.Items = append(cmd.Items, dtos.DashboardAclUpdateItem{ + UserID: acl.UserId, + Permission: acl.Permission, + }) + } + assert.Len(t, cmd.Items, 3) + + updateDashboardPermissionScenario(t, updatePermissionContext{ + desc: "When calling POST on", + url: "/api/dashboards/id/1/permissions", + routePattern: "/api/dashboards/id/:id/permissions", + cmd: cmd, + fn: func(sc *scenarioContext) { + setUp() + bus.AddHandler("test", func(cmd *models.UpdateDashboardAclCommand) error { + assert.Len(t, cmd.Items, 4) + return nil + }) + + sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec() + assert.Equal(t, 200, sc.resp.Code) + }, + }, hs) }) }) } -func callGetDashboardPermissions(sc *scenarioContext) { - sc.handlerFunc = GetDashboardPermissionList +func callGetDashboardPermissions(sc *scenarioContext, hs *HTTPServer) { + sc.handlerFunc = hs.GetDashboardPermissionList sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec() } @@ -242,22 +359,30 @@ func callUpdateDashboardPermissions(sc *scenarioContext) { sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec() } -func updateDashboardPermissionScenario(t *testing.T, desc string, url string, routePattern string, cmd dtos.UpdateDashboardAclCommand, fn scenarioFunc) { - t.Run(fmt.Sprintf("%s %s", desc, url), func(t *testing.T) { +type updatePermissionContext struct { + desc string + url string + routePattern string + cmd dtos.UpdateDashboardAclCommand + fn scenarioFunc +} + +func updateDashboardPermissionScenario(t *testing.T, ctx updatePermissionContext, hs *HTTPServer) { + t.Run(fmt.Sprintf("%s %s", ctx.desc, ctx.url), func(t *testing.T) { t.Cleanup(bus.ClearBusHandlers) - sc := setupScenarioContext(t, url) + sc := setupScenarioContext(t, ctx.url) sc.defaultHandler = Wrap(func(c *models.ReqContext) Response { sc.context = c sc.context.OrgId = testOrgID sc.context.UserId = testUserID - return UpdateDashboardPermissions(c, cmd) + return hs.UpdateDashboardPermissions(c, ctx.cmd) }) - sc.m.Post(routePattern, sc.defaultHandler) + sc.m.Post(ctx.routePattern, sc.defaultHandler) - fn(sc) + ctx.fn(sc) }) } diff --git a/pkg/api/datasources_test.go b/pkg/api/datasources_test.go index d4f1056a0e2..2e5019bf85f 100644 --- a/pkg/api/datasources_test.go +++ b/pkg/api/datasources_test.go @@ -12,8 +12,9 @@ import ( ) const ( - testOrgID int64 = 1 - testUserID int64 = 1 + testOrgID int64 = 1 + testUserID int64 = 1 + testUserLogin string = "testUser" ) func TestDataSourcesProxy_userLoggedIn(t *testing.T) { diff --git a/pkg/api/dtos/models.go b/pkg/api/dtos/models.go index 5a1dbb2e796..48131ccfab5 100644 --- a/pkg/api/dtos/models.go +++ b/pkg/api/dtos/models.go @@ -75,3 +75,15 @@ func GetGravatarUrlWithDefault(text string, defaultText string) string { return GetGravatarUrl(text) } + +func IsHiddenUser(userLogin string, signedInUser *models.SignedInUser, cfg *setting.Cfg) bool { + if signedInUser.IsGrafanaAdmin || userLogin == signedInUser.Login { + return false + } + + if _, hidden := cfg.HiddenUsers[userLogin]; hidden { + return true + } + + return false +} diff --git a/pkg/api/folder_permission.go b/pkg/api/folder_permission.go index 1824fda5bb8..cd1b3a7bac7 100644 --- a/pkg/api/folder_permission.go +++ b/pkg/api/folder_permission.go @@ -12,7 +12,7 @@ import ( "github.com/grafana/grafana/pkg/util" ) -func GetFolderPermissionList(c *models.ReqContext) Response { +func (hs *HTTPServer) GetFolderPermissionList(c *models.ReqContext) Response { s := dashboards.NewFolderService(c.OrgId, c.SignedInUser) folder, err := s.GetFolderByUID(c.Params(":uid")) @@ -31,7 +31,12 @@ func GetFolderPermissionList(c *models.ReqContext) Response { return Error(500, "Failed to get folder permissions", err) } + filteredAcls := make([]*models.DashboardAclInfoDTO, 0, len(acl)) for _, perm := range acl { + if dtos.IsHiddenUser(perm.UserLogin, c.SignedInUser, hs.Cfg) { + continue + } + perm.FolderId = folder.Id perm.DashboardId = 0 @@ -44,12 +49,14 @@ func GetFolderPermissionList(c *models.ReqContext) Response { if perm.Slug != "" { perm.Url = models.GetDashboardFolderUrl(perm.IsFolder, perm.Uid, perm.Slug) } + + filteredAcls = append(filteredAcls, perm) } - return JSON(200, acl) + return JSON(200, filteredAcls) } -func UpdateFolderPermissions(c *models.ReqContext, apiCmd dtos.UpdateDashboardAclCommand) Response { +func (hs *HTTPServer) UpdateFolderPermissions(c *models.ReqContext, apiCmd dtos.UpdateDashboardAclCommand) Response { if err := validatePermissionsUpdate(apiCmd); err != nil { return Error(400, err.Error(), err) } @@ -87,6 +94,12 @@ func UpdateFolderPermissions(c *models.ReqContext, apiCmd dtos.UpdateDashboardAc }) } + hiddenACL, err := g.GetHiddenACL(hs.Cfg) + if err != nil { + return Error(500, "Error while retrieving hidden permissions", err) + } + cmd.Items = append(cmd.Items, hiddenACL...) + if okToUpdate, err := g.CheckPermissionBeforeUpdate(models.PERMISSION_ADMIN, cmd.Items); err != nil || !okToUpdate { if err != nil { if errors.Is(err, guardian.ErrGuardianPermissionExists) || diff --git a/pkg/api/folder_permission_test.go b/pkg/api/folder_permission_test.go index d7a8985571e..ca22340d6c7 100644 --- a/pkg/api/folder_permission_test.go +++ b/pkg/api/folder_permission_test.go @@ -1,20 +1,24 @@ package api import ( + "encoding/json" "fmt" "testing" "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/bus" - "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/guardian" + "github.com/grafana/grafana/pkg/setting" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) func TestFolderPermissionAPIEndpoint(t *testing.T) { + settings := setting.NewCfg() + hs := &HTTPServer{Cfg: settings} + t.Run("Given folder not exists", func(t *testing.T) { mock := &fakeFolderService{ GetFolderByUIDError: models.ErrFolderNotFound, @@ -27,7 +31,7 @@ func TestFolderPermissionAPIEndpoint(t *testing.T) { mockFolderService(mock) loggedInUserScenarioWithRole(t, "When calling GET on", "GET", "/api/folders/uid/permissions", "/api/folders/:uid/permissions", models.ROLE_EDITOR, func(sc *scenarioContext) { - callGetFolderPermissions(sc) + callGetFolderPermissions(sc, hs) assert.Equal(t, 404, sc.resp.Code) }) @@ -37,10 +41,16 @@ func TestFolderPermissionAPIEndpoint(t *testing.T) { }, } - updateFolderPermissionScenario(t, "When calling POST on", "/api/folders/uid/permissions", "/api/folders/:uid/permissions", cmd, func(sc *scenarioContext) { - callUpdateFolderPermissions(sc) - assert.Equal(t, 404, sc.resp.Code) - }) + updateFolderPermissionScenario(t, updatePermissionContext{ + desc: "When calling POST on", + url: "/api/folders/uid/permissions", + routePattern: "/api/folders/:uid/permissions", + cmd: cmd, + fn: func(sc *scenarioContext) { + callUpdateFolderPermissions(sc) + assert.Equal(t, 404, sc.resp.Code) + }, + }, hs) }) t.Run("Given user has no admin permissions", func(t *testing.T) { @@ -64,7 +74,7 @@ func TestFolderPermissionAPIEndpoint(t *testing.T) { mockFolderService(mock) loggedInUserScenarioWithRole(t, "When calling GET on", "GET", "/api/folders/uid/permissions", "/api/folders/:uid/permissions", models.ROLE_EDITOR, func(sc *scenarioContext) { - callGetFolderPermissions(sc) + callGetFolderPermissions(sc, hs) assert.Equal(t, 403, sc.resp.Code) }) @@ -74,10 +84,16 @@ func TestFolderPermissionAPIEndpoint(t *testing.T) { }, } - updateFolderPermissionScenario(t, "When calling POST on", "/api/folders/uid/permissions", "/api/folders/:uid/permissions", cmd, func(sc *scenarioContext) { - callUpdateFolderPermissions(sc) - assert.Equal(t, 403, sc.resp.Code) - }) + updateFolderPermissionScenario(t, updatePermissionContext{ + desc: "When calling POST on", + url: "/api/folders/uid/permissions", + routePattern: "/api/folders/:uid/permissions", + cmd: cmd, + fn: func(sc *scenarioContext) { + callUpdateFolderPermissions(sc) + assert.Equal(t, 403, sc.resp.Code) + }, + }, hs) }) t.Run("Given user has admin permissions and permissions to update", func(t *testing.T) { @@ -111,13 +127,16 @@ func TestFolderPermissionAPIEndpoint(t *testing.T) { mockFolderService(mock) loggedInUserScenarioWithRole(t, "When calling GET on", "GET", "/api/folders/uid/permissions", "/api/folders/:uid/permissions", models.ROLE_ADMIN, func(sc *scenarioContext) { - callGetFolderPermissions(sc) + callGetFolderPermissions(sc, hs) assert.Equal(t, 200, sc.resp.Code) - respJSON, err := simplejson.NewJson(sc.resp.Body.Bytes()) + + var resp []*models.DashboardAclInfoDTO + err := json.Unmarshal(sc.resp.Body.Bytes(), &resp) require.NoError(t, err) - assert.Equal(t, 5, len(respJSON.MustArray())) - assert.Equal(t, 2, respJSON.GetIndex(0).Get("userId").MustInt()) - assert.Equal(t, int(models.PERMISSION_VIEW), respJSON.GetIndex(0).Get("permission").MustInt()) + + assert.Len(t, resp, 5) + assert.Equal(t, int64(2), resp[0].UserId) + assert.Equal(t, models.PERMISSION_VIEW, resp[0].Permission) }) cmd := dtos.UpdateDashboardAclCommand{ @@ -126,14 +145,26 @@ func TestFolderPermissionAPIEndpoint(t *testing.T) { }, } - updateFolderPermissionScenario(t, "When calling POST on", "/api/folders/uid/permissions", "/api/folders/:uid/permissions", cmd, func(sc *scenarioContext) { - callUpdateFolderPermissions(sc) - assert.Equal(t, 200, sc.resp.Code) - respJSON, err := simplejson.NewJson(sc.resp.Body.Bytes()) - require.NoError(t, err) - assert.Equal(t, 1, respJSON.Get("id").MustInt()) - assert.Equal(t, "Folder", respJSON.Get("title").MustString()) - }) + updateFolderPermissionScenario(t, updatePermissionContext{ + desc: "When calling POST on", + url: "/api/folders/uid/permissions", + routePattern: "/api/folders/:uid/permissions", + cmd: cmd, + fn: func(sc *scenarioContext) { + callUpdateFolderPermissions(sc) + assert.Equal(t, 200, sc.resp.Code) + + var resp struct { + ID int64 + Title string + } + err := json.Unmarshal(sc.resp.Body.Bytes(), &resp) + require.NoError(t, err) + + assert.Equal(t, int64(1), resp.ID) + assert.Equal(t, "Folder", resp.Title) + }, + }, hs) }) t.Run("When trying to update permissions with duplicate permissions", func(t *testing.T) { @@ -166,10 +197,16 @@ func TestFolderPermissionAPIEndpoint(t *testing.T) { }, } - updateFolderPermissionScenario(t, "When calling POST on", "/api/folders/uid/permissions", "/api/folders/:uid/permissions", cmd, func(sc *scenarioContext) { - callUpdateFolderPermissions(sc) - assert.Equal(t, 400, sc.resp.Code) - }) + updateFolderPermissionScenario(t, updatePermissionContext{ + desc: "When calling POST on", + url: "/api/folders/uid/permissions", + routePattern: "/api/folders/:uid/permissions", + cmd: cmd, + fn: func(sc *scenarioContext) { + callUpdateFolderPermissions(sc) + assert.Equal(t, 400, sc.resp.Code) + }, + }, hs) }) t.Run("When trying to update team or user permissions with a role", func(t *testing.T) { @@ -188,14 +225,19 @@ func TestFolderPermissionAPIEndpoint(t *testing.T) { } for _, cmd := range cmds { - updateFolderPermissionScenario(t, "When calling POST on", "/api/folders/uid/permissions", - "/api/folders/:uid/permissions", cmd, func(sc *scenarioContext) { + updateFolderPermissionScenario(t, updatePermissionContext{ + desc: "When calling POST on", + url: "/api/folders/uid/permissions", + routePattern: "/api/folders/:uid/permissions", + cmd: cmd, + fn: func(sc *scenarioContext) { callUpdateFolderPermissions(sc) assert.Equal(t, 400, sc.resp.Code) respJSON, err := jsonMap(sc.resp.Body.Bytes()) require.NoError(t, err) assert.Equal(t, models.ErrPermissionsWithRoleNotAllowed.Error(), respJSON["error"]) - }) + }, + }, hs) } }) @@ -229,15 +271,102 @@ func TestFolderPermissionAPIEndpoint(t *testing.T) { }, } - updateFolderPermissionScenario(t, "When calling POST on", "/api/folders/uid/permissions", "/api/folders/:uid/permissions", cmd, func(sc *scenarioContext) { - callUpdateFolderPermissions(sc) - assert.Equal(t, 400, sc.resp.Code) + updateFolderPermissionScenario(t, updatePermissionContext{ + desc: "When calling POST on", + url: "/api/folders/uid/permissions", + routePattern: "/api/folders/:uid/permissions", + cmd: cmd, + fn: func(sc *scenarioContext) { + callUpdateFolderPermissions(sc) + assert.Equal(t, 400, sc.resp.Code) + }, + }, hs) + }) + + t.Run("Getting and updating folder permissions with hidden users", func(t *testing.T) { + origNewGuardian := guardian.New + origNewFolderService := dashboards.NewFolderService + settings.HiddenUsers = map[string]struct{}{ + "hiddenUser": {}, + testUserLogin: {}, + } + t.Cleanup(func() { + guardian.New = origNewGuardian + dashboards.NewFolderService = origNewFolderService + settings.HiddenUsers = make(map[string]struct{}) }) + + guardian.MockDashboardGuardian(&guardian.FakeDashboardGuardian{ + CanAdminValue: true, + CheckPermissionBeforeUpdateValue: true, + GetAclValue: []*models.DashboardAclInfoDTO{ + {OrgId: 1, DashboardId: 1, UserId: 2, UserLogin: "hiddenUser", Permission: models.PERMISSION_VIEW}, + {OrgId: 1, DashboardId: 1, UserId: 3, UserLogin: testUserLogin, Permission: models.PERMISSION_EDIT}, + {OrgId: 1, DashboardId: 1, UserId: 4, UserLogin: "user_1", Permission: models.PERMISSION_ADMIN}, + }, + GetHiddenAclValue: []*models.DashboardAcl{ + {OrgID: 1, DashboardID: 1, UserID: 2, Permission: models.PERMISSION_VIEW}, + }, + }) + + mock := &fakeFolderService{ + GetFolderByUIDResult: &models.Folder{ + Id: 1, + Uid: "uid", + Title: "Folder", + }, + } + + mockFolderService(mock) + + var resp []*models.DashboardAclInfoDTO + loggedInUserScenarioWithRole(t, "When calling GET on", "GET", "/api/folders/uid/permissions", "/api/folders/:uid/permissions", models.ROLE_ADMIN, func(sc *scenarioContext) { + callGetFolderPermissions(sc, hs) + assert.Equal(t, 200, sc.resp.Code) + + err := json.Unmarshal(sc.resp.Body.Bytes(), &resp) + require.NoError(t, err) + + assert.Len(t, resp, 2) + assert.Equal(t, int64(3), resp[0].UserId) + assert.Equal(t, models.PERMISSION_EDIT, resp[0].Permission) + assert.Equal(t, int64(4), resp[1].UserId) + assert.Equal(t, models.PERMISSION_ADMIN, resp[1].Permission) + }) + + cmd := dtos.UpdateDashboardAclCommand{ + Items: []dtos.DashboardAclUpdateItem{ + {UserID: 1000, Permission: models.PERMISSION_ADMIN}, + }, + } + for _, acl := range resp { + cmd.Items = append(cmd.Items, dtos.DashboardAclUpdateItem{ + UserID: acl.UserId, + Permission: acl.Permission, + }) + } + assert.Len(t, cmd.Items, 3) + + updateFolderPermissionScenario(t, updatePermissionContext{ + desc: "When calling POST on", + url: "/api/folders/uid/permissions", + routePattern: "/api/folders/:uid/permissions", + cmd: cmd, + fn: func(sc *scenarioContext) { + bus.AddHandler("test", func(cmd *models.UpdateDashboardAclCommand) error { + assert.Len(t, cmd.Items, 4) + return nil + }) + + sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec() + assert.Equal(t, 200, sc.resp.Code) + }, + }, hs) }) } -func callGetFolderPermissions(sc *scenarioContext) { - sc.handlerFunc = GetFolderPermissionList +func callGetFolderPermissions(sc *scenarioContext, hs *HTTPServer) { + sc.handlerFunc = hs.GetFolderPermissionList sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec() } @@ -249,22 +378,22 @@ func callUpdateFolderPermissions(sc *scenarioContext) { sc.fakeReqWithParams("POST", sc.url, map[string]string{}).exec() } -func updateFolderPermissionScenario(t *testing.T, desc string, url string, routePattern string, cmd dtos.UpdateDashboardAclCommand, fn scenarioFunc) { - t.Run(fmt.Sprintf("%s %s", desc, url), func(t *testing.T) { +func updateFolderPermissionScenario(t *testing.T, ctx updatePermissionContext, hs *HTTPServer) { + t.Run(fmt.Sprintf("%s %s", ctx.desc, ctx.url), func(t *testing.T) { defer bus.ClearBusHandlers() - sc := setupScenarioContext(t, url) + sc := setupScenarioContext(t, ctx.url) sc.defaultHandler = Wrap(func(c *models.ReqContext) Response { sc.context = c sc.context.OrgId = testOrgID sc.context.UserId = testUserID - return UpdateFolderPermissions(c, cmd) + return hs.UpdateFolderPermissions(c, ctx.cmd) }) - sc.m.Post(routePattern, sc.defaultHandler) + sc.m.Post(ctx.routePattern, sc.defaultHandler) - fn(sc) + ctx.fn(sc) }) } diff --git a/pkg/api/org_users.go b/pkg/api/org_users.go index 8d9137e35e3..376502d6d03 100644 --- a/pkg/api/org_users.go +++ b/pkg/api/org_users.go @@ -53,8 +53,13 @@ func addOrgUserHelper(cmd models.AddOrgUserCommand) Response { } // GET /api/org/users -func GetOrgUsersForCurrentOrg(c *models.ReqContext) Response { - result, err := getOrgUsersHelper(c.OrgId, c.Query("query"), c.QueryInt("limit")) +func (hs *HTTPServer) GetOrgUsersForCurrentOrg(c *models.ReqContext) Response { + result, err := hs.getOrgUsersHelper(&models.GetOrgUsersQuery{ + OrgId: c.OrgId, + Query: c.Query("query"), + Limit: c.QueryInt("limit"), + }, c.SignedInUser) + if err != nil { return Error(500, "Failed to get users for current organization", err) } @@ -63,7 +68,7 @@ func GetOrgUsersForCurrentOrg(c *models.ReqContext) Response { } // GET /api/org/users/lookup -func GetOrgUsersForCurrentOrgLookup(c *models.ReqContext) Response { +func (hs *HTTPServer) GetOrgUsersForCurrentOrgLookup(c *models.ReqContext) Response { isAdmin, err := isOrgAdminFolderAdminOrTeamAdmin(c) if err != nil { return Error(500, "Failed to get users for current organization", err) @@ -73,7 +78,12 @@ func GetOrgUsersForCurrentOrgLookup(c *models.ReqContext) Response { return Error(403, "Permission denied", nil) } - orgUsers, err := getOrgUsersHelper(c.OrgId, c.Query("query"), c.QueryInt("limit")) + orgUsers, err := hs.getOrgUsersHelper(&models.GetOrgUsersQuery{ + OrgId: c.OrgId, + Query: c.Query("query"), + Limit: c.QueryInt("limit"), + }, c.SignedInUser) + if err != nil { return Error(500, "Failed to get users for current organization", err) } @@ -114,8 +124,13 @@ func isOrgAdminFolderAdminOrTeamAdmin(c *models.ReqContext) (bool, error) { } // GET /api/orgs/:orgId/users -func GetOrgUsers(c *models.ReqContext) Response { - result, err := getOrgUsersHelper(c.ParamsInt64(":orgId"), "", 0) +func (hs *HTTPServer) GetOrgUsers(c *models.ReqContext) Response { + result, err := hs.getOrgUsersHelper(&models.GetOrgUsersQuery{ + OrgId: c.ParamsInt64(":orgId"), + Query: "", + Limit: 0, + }, c.SignedInUser) + if err != nil { return Error(500, "Failed to get users for organization", err) } @@ -123,22 +138,22 @@ func GetOrgUsers(c *models.ReqContext) Response { return JSON(200, result) } -func getOrgUsersHelper(orgID int64, query string, limit int) ([]*models.OrgUserDTO, error) { - q := models.GetOrgUsersQuery{ - OrgId: orgID, - Query: query, - Limit: limit, - } - - if err := bus.Dispatch(&q); err != nil { +func (hs *HTTPServer) getOrgUsersHelper(query *models.GetOrgUsersQuery, signedInUser *models.SignedInUser) ([]*models.OrgUserDTO, error) { + if err := bus.Dispatch(query); err != nil { return nil, err } - for _, user := range q.Result { + filteredUsers := make([]*models.OrgUserDTO, 0, len(query.Result)) + for _, user := range query.Result { + if dtos.IsHiddenUser(user.Login, signedInUser, hs.Cfg) { + continue + } user.AvatarUrl = dtos.GetGravatarUrl(user.Email) + + filteredUsers = append(filteredUsers, user) } - return q.Result, nil + return filteredUsers, nil } // PATCH /api/org/users/:userId diff --git a/pkg/api/org_users_test.go b/pkg/api/org_users_test.go new file mode 100644 index 00000000000..34445e128b9 --- /dev/null +++ b/pkg/api/org_users_test.go @@ -0,0 +1,126 @@ +package api + +import ( + "encoding/json" + "net/http" + "testing" + + "github.com/grafana/grafana/pkg/api/dtos" + "github.com/grafana/grafana/pkg/bus" + "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/setting" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func setUpGetOrgUsersHandler() { + bus.AddHandler("test", func(query *models.GetOrgUsersQuery) error { + query.Result = []*models.OrgUserDTO{ + {Email: "testUser@grafana.com", Login: testUserLogin}, + {Email: "user1@grafana.com", Login: "user1"}, + {Email: "user2@grafana.com", Login: "user2"}, + } + return nil + }) +} + +func TestOrgUsersAPIEndpoint_userLoggedIn(t *testing.T) { + settings := setting.NewCfg() + hs := &HTTPServer{Cfg: settings} + + loggedInUserScenario(t, "When calling GET on", "api/org/users", func(sc *scenarioContext) { + setUpGetOrgUsersHandler() + + sc.handlerFunc = hs.GetOrgUsersForCurrentOrg + sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec() + + require.Equal(t, http.StatusOK, sc.resp.Code) + + var resp []models.OrgUserDTO + err := json.Unmarshal(sc.resp.Body.Bytes(), &resp) + require.NoError(t, err) + assert.Len(t, resp, 3) + }) + + loggedInUserScenario(t, "When calling GET as an editor with no team / folder permissions on", + "api/org/users/lookup", func(sc *scenarioContext) { + setUpGetOrgUsersHandler() + bus.AddHandler("test", func(query *models.HasAdminPermissionInFoldersQuery) error { + query.Result = false + return nil + }) + bus.AddHandler("test", func(query *models.IsAdminOfTeamsQuery) error { + query.Result = false + return nil + }) + + sc.handlerFunc = hs.GetOrgUsersForCurrentOrgLookup + sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec() + + assert.Equal(t, http.StatusForbidden, sc.resp.Code) + + var resp struct { + Message string + } + err := json.Unmarshal(sc.resp.Body.Bytes(), &resp) + require.NoError(t, err) + + assert.Equal(t, "Permission denied", resp.Message) + }) + + loggedInUserScenarioWithRole(t, "When calling GET as an admin on", "GET", "api/org/users/lookup", + "api/org/users/lookup", models.ROLE_ADMIN, func(sc *scenarioContext) { + setUpGetOrgUsersHandler() + + sc.handlerFunc = hs.GetOrgUsersForCurrentOrgLookup + sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec() + + require.Equal(t, http.StatusOK, sc.resp.Code) + + var resp []dtos.UserLookupDTO + err := json.Unmarshal(sc.resp.Body.Bytes(), &resp) + require.NoError(t, err) + assert.Len(t, resp, 3) + }) + + t.Run("Given there is two hidden users", func(t *testing.T) { + settings.HiddenUsers = map[string]struct{}{ + "user1": {}, + testUserLogin: {}, + } + t.Cleanup(func() { settings.HiddenUsers = make(map[string]struct{}) }) + + loggedInUserScenario(t, "When calling GET on", "api/org/users", func(sc *scenarioContext) { + setUpGetOrgUsersHandler() + + sc.handlerFunc = hs.GetOrgUsersForCurrentOrg + sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec() + + require.Equal(t, http.StatusOK, sc.resp.Code) + + var resp []models.OrgUserDTO + err := json.Unmarshal(sc.resp.Body.Bytes(), &resp) + require.NoError(t, err) + assert.Len(t, resp, 2) + assert.Equal(t, testUserLogin, resp[0].Login) + assert.Equal(t, "user2", resp[1].Login) + }) + + loggedInUserScenarioWithRole(t, "When calling GET as an admin on", "GET", "api/org/users/lookup", + "api/org/users/lookup", models.ROLE_ADMIN, func(sc *scenarioContext) { + setUpGetOrgUsersHandler() + + sc.handlerFunc = hs.GetOrgUsersForCurrentOrgLookup + sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec() + + require.Equal(t, http.StatusOK, sc.resp.Code) + + var resp []dtos.UserLookupDTO + err := json.Unmarshal(sc.resp.Body.Bytes(), &resp) + require.NoError(t, err) + assert.Len(t, resp, 2) + assert.Equal(t, testUserLogin, resp[0].Login) + assert.Equal(t, "user2", resp[1].Login) + }) + }) +} diff --git a/pkg/api/team.go b/pkg/api/team.go index 333e9a7a0f1..f858253cb92 100644 --- a/pkg/api/team.go +++ b/pkg/api/team.go @@ -112,6 +112,8 @@ func (hs *HTTPServer) SearchTeams(c *models.ReqContext) Response { UserIdFilter: userIdFilter, Page: page, Limit: perPage, + SignedInUser: c.SignedInUser, + HiddenUsers: hs.Cfg.HiddenUsers, } if err := bus.Dispatch(&query); err != nil { @@ -129,8 +131,13 @@ func (hs *HTTPServer) SearchTeams(c *models.ReqContext) Response { } // GET /api/teams/:teamId -func GetTeamByID(c *models.ReqContext) Response { - query := models.GetTeamByIdQuery{OrgId: c.OrgId, Id: c.ParamsInt64(":teamId")} +func (hs *HTTPServer) GetTeamByID(c *models.ReqContext) Response { + query := models.GetTeamByIdQuery{ + OrgId: c.OrgId, + Id: c.ParamsInt64(":teamId"), + SignedInUser: c.SignedInUser, + HiddenUsers: hs.Cfg.HiddenUsers, + } if err := bus.Dispatch(&query); err != nil { if errors.Is(err, models.ErrTeamNotFound) { diff --git a/pkg/api/team_members.go b/pkg/api/team_members.go index ba6ea82cd39..aa9cd2e6852 100644 --- a/pkg/api/team_members.go +++ b/pkg/api/team_members.go @@ -18,7 +18,12 @@ func (hs *HTTPServer) GetTeamMembers(c *models.ReqContext) Response { return Error(500, "Failed to get Team Members", err) } + filteredMembers := make([]*models.TeamMemberDTO, 0, len(query.Result)) for _, member := range query.Result { + if dtos.IsHiddenUser(member.Login, c.SignedInUser, hs.Cfg) { + continue + } + member.AvatarUrl = dtos.GetGravatarUrl(member.Email) member.Labels = []string{} @@ -26,9 +31,11 @@ func (hs *HTTPServer) GetTeamMembers(c *models.ReqContext) Response { authProvider := GetAuthProviderLabel(member.AuthModule) member.Labels = append(member.Labels, authProvider) } + + filteredMembers = append(filteredMembers, member) } - return JSON(200, query.Result) + return JSON(200, filteredMembers) } // POST /api/teams/:teamId/members diff --git a/pkg/api/team_members_test.go b/pkg/api/team_members_test.go new file mode 100644 index 00000000000..397a105a514 --- /dev/null +++ b/pkg/api/team_members_test.go @@ -0,0 +1,73 @@ +package api + +import ( + "encoding/json" + "net/http" + "testing" + + "github.com/grafana/grafana/pkg/bus" + "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/licensing" + "github.com/grafana/grafana/pkg/setting" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func setUpGetTeamMembersHandler() { + bus.AddHandler("test", func(query *models.GetTeamMembersQuery) error { + query.Result = []*models.TeamMemberDTO{ + {Email: "testUser@grafana.com", Login: testUserLogin}, + {Email: "user1@grafana.com", Login: "user1"}, + {Email: "user2@grafana.com", Login: "user2"}, + } + return nil + }) +} + +func TestTeamMembersAPIEndpoint_userLoggedIn(t *testing.T) { + settings := setting.NewCfg() + hs := &HTTPServer{ + Cfg: settings, + License: &licensing.OSSLicensingService{}, + } + + loggedInUserScenarioWithRole(t, "When calling GET on", "GET", "api/teams/1/members", + "api/teams/:teamId/members", models.ROLE_ADMIN, func(sc *scenarioContext) { + setUpGetTeamMembersHandler() + + sc.handlerFunc = hs.GetTeamMembers + sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec() + + require.Equal(t, http.StatusOK, sc.resp.Code) + + var resp []models.TeamMemberDTO + err := json.Unmarshal(sc.resp.Body.Bytes(), &resp) + require.NoError(t, err) + assert.Len(t, resp, 3) + }) + + t.Run("Given there is two hidden users", func(t *testing.T) { + settings.HiddenUsers = map[string]struct{}{ + "user1": {}, + testUserLogin: {}, + } + t.Cleanup(func() { settings.HiddenUsers = make(map[string]struct{}) }) + + loggedInUserScenarioWithRole(t, "When calling GET on", "GET", "api/teams/1/members", + "api/teams/:teamId/members", models.ROLE_ADMIN, func(sc *scenarioContext) { + setUpGetTeamMembersHandler() + + sc.handlerFunc = hs.GetTeamMembers + sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec() + + require.Equal(t, http.StatusOK, sc.resp.Code) + + var resp []models.TeamMemberDTO + err := json.Unmarshal(sc.resp.Body.Bytes(), &resp) + require.NoError(t, err) + assert.Len(t, resp, 2) + assert.Equal(t, testUserLogin, resp[0].Login) + assert.Equal(t, "user2", resp[1].Login) + }) + }) +} diff --git a/pkg/models/team.go b/pkg/models/team.go index fc6e20d4668..328e1815b90 100644 --- a/pkg/models/team.go +++ b/pkg/models/team.go @@ -50,9 +50,11 @@ type DeleteTeamCommand struct { } type GetTeamByIdQuery struct { - OrgId int64 - Id int64 - Result *TeamDTO + OrgId int64 + Id int64 + SignedInUser *SignedInUser + HiddenUsers map[string]struct{} + Result *TeamDTO } type GetTeamsByUserQuery struct { @@ -68,6 +70,8 @@ type SearchTeamsQuery struct { Page int OrgId int64 UserIdFilter int64 + SignedInUser *SignedInUser + HiddenUsers map[string]struct{} Result SearchTeamQueryResult } diff --git a/pkg/services/guardian/guardian.go b/pkg/services/guardian/guardian.go index 8cef02155c2..1f9fe85d3eb 100644 --- a/pkg/services/guardian/guardian.go +++ b/pkg/services/guardian/guardian.go @@ -23,6 +23,7 @@ type DashboardGuardian interface { HasPermission(permission models.PermissionType) (bool, error) CheckPermissionBeforeUpdate(permission models.PermissionType, updatePermissions []*models.DashboardAcl) (bool, error) GetAcl() ([]*models.DashboardAclInfoDTO, error) + GetHiddenACL(*setting.Cfg) ([]*models.DashboardAcl, error) } type dashboardGuardianImpl struct { @@ -213,6 +214,38 @@ func (g *dashboardGuardianImpl) getTeams() ([]*models.TeamDTO, error) { return query.Result, err } +func (g *dashboardGuardianImpl) GetHiddenACL(cfg *setting.Cfg) ([]*models.DashboardAcl, error) { + hiddenACL := make([]*models.DashboardAcl, 0) + if g.user.IsGrafanaAdmin { + return hiddenACL, nil + } + + existingPermissions, err := g.GetAcl() + if err != nil { + return hiddenACL, err + } + + for _, item := range existingPermissions { + if item.Inherited || item.UserLogin == g.user.Login { + continue + } + + if _, hidden := cfg.HiddenUsers[item.UserLogin]; hidden { + hiddenACL = append(hiddenACL, &models.DashboardAcl{ + OrgID: item.OrgId, + DashboardID: item.DashboardId, + UserID: item.UserId, + TeamID: item.TeamId, + Role: item.Role, + Permission: item.Permission, + Created: item.Created, + Updated: item.Updated, + }) + } + } + return hiddenACL, nil +} + // nolint:unused type FakeDashboardGuardian struct { DashId int64 @@ -226,6 +259,7 @@ type FakeDashboardGuardian struct { CheckPermissionBeforeUpdateValue bool CheckPermissionBeforeUpdateError error GetAclValue []*models.DashboardAclInfoDTO + GetHiddenAclValue []*models.DashboardAcl } func (g *FakeDashboardGuardian) CanSave() (bool, error) { @@ -256,6 +290,10 @@ func (g *FakeDashboardGuardian) GetAcl() ([]*models.DashboardAclInfoDTO, error) return g.GetAclValue, nil } +func (g *FakeDashboardGuardian) GetHiddenACL(cfg *setting.Cfg) ([]*models.DashboardAcl, error) { + return g.GetHiddenAclValue, nil +} + // nolint:unused func MockDashboardGuardian(mock *FakeDashboardGuardian) { New = func(dashId int64, orgId int64, user *models.SignedInUser) DashboardGuardian { diff --git a/pkg/services/guardian/guardian_test.go b/pkg/services/guardian/guardian_test.go index 740db79877c..657df6b2bdc 100644 --- a/pkg/services/guardian/guardian_test.go +++ b/pkg/services/guardian/guardian_test.go @@ -6,7 +6,10 @@ import ( "runtime" "testing" + "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/setting" + . "github.com/smartystreets/goconvey/convey" "github.com/stretchr/testify/require" ) @@ -673,3 +676,51 @@ func (sc *scenarioContext) verifyUpdateChildDashboardPermissionsWithOverrideShou }) } } + +func TestGuardianGetHiddenACL(t *testing.T) { + Convey("Get hidden ACL tests", t, func() { + bus.ClearBusHandlers() + + bus.AddHandler("test", func(query *models.GetDashboardAclInfoListQuery) error { + query.Result = []*models.DashboardAclInfoDTO{ + {Inherited: false, UserId: 1, UserLogin: "user1", Permission: models.PERMISSION_EDIT}, + {Inherited: false, UserId: 2, UserLogin: "user2", Permission: models.PERMISSION_ADMIN}, + {Inherited: true, UserId: 3, UserLogin: "user3", Permission: models.PERMISSION_VIEW}, + } + return nil + }) + + cfg := setting.NewCfg() + cfg.HiddenUsers = map[string]struct{}{"user2": {}} + + Convey("Should get hidden acl", func() { + user := &models.SignedInUser{ + OrgId: orgID, + UserId: 1, + Login: "user1", + } + g := New(dashboardID, orgID, user) + + hiddenACL, err := g.GetHiddenACL(cfg) + So(err, ShouldBeNil) + + So(hiddenACL, ShouldHaveLength, 1) + So(hiddenACL[0].UserID, ShouldEqual, 2) + }) + + Convey("Grafana admin should not get hidden acl", func() { + user := &models.SignedInUser{ + OrgId: orgID, + UserId: 1, + Login: "user1", + IsGrafanaAdmin: true, + } + g := New(dashboardID, orgID, user) + + hiddenACL, err := g.GetHiddenACL(cfg) + So(err, ShouldBeNil) + + So(hiddenACL, ShouldHaveLength, 0) + }) + }) +} diff --git a/pkg/services/sqlstore/team.go b/pkg/services/sqlstore/team.go index a9bdf995fc1..b04ec56cc3c 100644 --- a/pkg/services/sqlstore/team.go +++ b/pkg/services/sqlstore/team.go @@ -3,6 +3,7 @@ package sqlstore import ( "bytes" "fmt" + "strings" "time" "github.com/grafana/grafana/pkg/bus" @@ -24,26 +25,54 @@ func init() { bus.AddHandler("sql", IsAdminOfTeams) } -func getTeamSearchSQLBase() string { - return `SELECT - team.id as id, - team.org_id, - team.name as name, - team.email as email, - (SELECT COUNT(*) from team_member where team_member.team_id = team.id) as member_count, - team_member.permission - FROM team as team - INNER JOIN team_member on team.id = team_member.team_id AND team_member.user_id = ? ` +func getFilteredUsers(signedInUser *models.SignedInUser, hiddenUsers map[string]struct{}) []string { + filteredUsers := make([]string, 0, len(hiddenUsers)) + if signedInUser == nil || signedInUser.IsGrafanaAdmin { + return filteredUsers + } + + for u := range hiddenUsers { + if u == signedInUser.Login { + continue + } + filteredUsers = append(filteredUsers, u) + } + + return filteredUsers } -func getTeamSelectSQLBase() string { +func getTeamMemberCount(filteredUsers []string) string { + if len(filteredUsers) > 0 { + return `(SELECT COUNT(*) FROM team_member + INNER JOIN ` + dialect.Quote("user") + ` ON team_member.user_id = ` + dialect.Quote("user") + `.id + WHERE team_member.team_id = team.id AND ` + dialect.Quote("user") + `.login NOT IN (?` + + strings.Repeat(",?", len(filteredUsers)-1) + ")" + + `) AS member_count ` + } + + return "(SELECT COUNT(*) FROM team_member WHERE team_member.team_id = team.id) AS member_count " +} + +func getTeamSearchSQLBase(filteredUsers []string) string { + return `SELECT + team.id AS id, + team.org_id, + team.name AS name, + team.email AS email, + team_member.permission, ` + + getTeamMemberCount(filteredUsers) + + ` FROM team AS team + INNER JOIN team_member ON team.id = team_member.team_id AND team_member.user_id = ? ` +} + +func getTeamSelectSQLBase(filteredUsers []string) string { return `SELECT team.id as id, team.org_id, team.name as name, - team.email as email, - (SELECT COUNT(*) from team_member where team_member.team_id = team.id) as member_count - FROM team as team ` + team.email as email, ` + + getTeamMemberCount(filteredUsers) + + ` FROM team as team ` } func CreateTeam(cmd *models.CreateTeamCommand) error { @@ -157,14 +186,21 @@ func SearchTeams(query *models.SearchTeamsQuery) error { var sql bytes.Buffer params := make([]interface{}, 0) + filteredUsers := getFilteredUsers(query.SignedInUser, query.HiddenUsers) if query.UserIdFilter > 0 { - sql.WriteString(getTeamSearchSQLBase()) + sql.WriteString(getTeamSearchSQLBase(filteredUsers)) + for _, user := range filteredUsers { + params = append(params, user) + } params = append(params, query.UserIdFilter) } else { - sql.WriteString(getTeamSelectSQLBase()) + sql.WriteString(getTeamSelectSQLBase(filteredUsers)) + for _, user := range filteredUsers { + params = append(params, user) + } } - sql.WriteString(` WHERE team.org_id = ?`) + sql.WriteString(` WHERE team.org_id = ?`) params = append(params, query.OrgId) if query.Query != "" { @@ -206,12 +242,19 @@ func SearchTeams(query *models.SearchTeamsQuery) error { func GetTeamById(query *models.GetTeamByIdQuery) error { var sql bytes.Buffer + params := make([]interface{}, 0) + + filteredUsers := getFilteredUsers(query.SignedInUser, query.HiddenUsers) + sql.WriteString(getTeamSelectSQLBase(filteredUsers)) + for _, user := range filteredUsers { + params = append(params, user) + } - sql.WriteString(getTeamSelectSQLBase()) sql.WriteString(` WHERE team.org_id = ? and team.id = ?`) + params = append(params, query.OrgId, query.Id) var team models.TeamDTO - exists, err := x.SQL(sql.String(), query.OrgId, query.Id).Get(&team) + exists, err := x.SQL(sql.String(), params...).Get(&team) if err != nil { return err @@ -231,7 +274,7 @@ func GetTeamsByUser(query *models.GetTeamsByUserQuery) error { var sql bytes.Buffer - sql.WriteString(getTeamSelectSQLBase()) + sql.WriteString(getTeamSelectSQLBase([]string{})) sql.WriteString(` INNER JOIN team_member on team.id = team_member.team_id`) sql.WriteString(` WHERE team.org_id = ? and team_member.user_id = ?`) diff --git a/pkg/services/sqlstore/team_test.go b/pkg/services/sqlstore/team_test.go index 88f9d5d52fe..e56985ee7ba 100644 --- a/pkg/services/sqlstore/team_test.go +++ b/pkg/services/sqlstore/team_test.go @@ -48,6 +48,7 @@ func TestTeamCommandsAndQueries(t *testing.T) { So(team1.Name, ShouldEqual, "group1 name") So(team1.Email, ShouldEqual, "test1@test.com") So(team1.OrgId, ShouldEqual, testOrgId) + So(team1.MemberCount, ShouldEqual, 0) err = AddTeamMember(&models.AddTeamMemberCommand{OrgId: testOrgId, TeamId: team1.Id, UserId: userIds[0]}) So(err, ShouldBeNil) @@ -74,6 +75,20 @@ func TestTeamCommandsAndQueries(t *testing.T) { So(q2.Result[0].Login, ShouldEqual, "loginuser1") So(q2.Result[0].OrgId, ShouldEqual, testOrgId) So(q2.Result[0].External, ShouldEqual, true) + + err = SearchTeams(query) + So(err, ShouldBeNil) + team1 = query.Result.Teams[0] + So(team1.MemberCount, ShouldEqual, 2) + + getTeamQuery := &models.GetTeamByIdQuery{OrgId: testOrgId, Id: team1.Id} + err = GetTeamById(getTeamQuery) + So(err, ShouldBeNil) + team1 = getTeamQuery.Result + So(team1.Name, ShouldEqual, "group1 name") + So(team1.Email, ShouldEqual, "test1@test.com") + So(team1.OrgId, ShouldEqual, testOrgId) + So(team1.MemberCount, ShouldEqual, 2) }) Convey("Should return latest auth module for users when getting team members", func() { @@ -275,6 +290,38 @@ func TestTeamCommandsAndQueries(t *testing.T) { So(err, ShouldBeNil) So(query.Result, ShouldBeTrue) }) + + Convey("Should not return hidden users in team member count", func() { + signedInUser := &models.SignedInUser{Login: "loginuser0"} + hiddenUsers := map[string]struct{}{"loginuser0": {}, "loginuser1": {}} + + teamId := group1.Result.Id + err = AddTeamMember(&models.AddTeamMemberCommand{OrgId: testOrgId, TeamId: teamId, UserId: userIds[0]}) + So(err, ShouldBeNil) + err = AddTeamMember(&models.AddTeamMemberCommand{OrgId: testOrgId, TeamId: teamId, UserId: userIds[1]}) + So(err, ShouldBeNil) + err = AddTeamMember(&models.AddTeamMemberCommand{OrgId: testOrgId, TeamId: teamId, UserId: userIds[2]}) + So(err, ShouldBeNil) + + searchQuery := &models.SearchTeamsQuery{OrgId: testOrgId, Page: 1, Limit: 10, SignedInUser: signedInUser, HiddenUsers: hiddenUsers} + err = SearchTeams(searchQuery) + So(err, ShouldBeNil) + So(searchQuery.Result.Teams, ShouldHaveLength, 2) + team1 := searchQuery.Result.Teams[0] + So(team1.MemberCount, ShouldEqual, 2) + + searchQueryFilteredByUser := &models.SearchTeamsQuery{OrgId: testOrgId, Page: 1, Limit: 10, UserIdFilter: userIds[0], SignedInUser: signedInUser, HiddenUsers: hiddenUsers} + err = SearchTeams(searchQueryFilteredByUser) + So(err, ShouldBeNil) + So(searchQueryFilteredByUser.Result.Teams, ShouldHaveLength, 1) + team1 = searchQuery.Result.Teams[0] + So(team1.MemberCount, ShouldEqual, 2) + + getTeamQuery := &models.GetTeamByIdQuery{OrgId: testOrgId, Id: teamId, SignedInUser: signedInUser, HiddenUsers: hiddenUsers} + err = GetTeamById(getTeamQuery) + So(err, ShouldBeNil) + So(getTeamQuery.Result.MemberCount, ShouldEqual, 2) + }) }) }) } diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index 09eba061858..75b88df4f35 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -308,6 +308,7 @@ type Cfg struct { // User UserInviteMaxLifetime time.Duration + HiddenUsers map[string]struct{} // Annotations AlertingAnnotationCleanupSetting AnnotationCleanupSettings @@ -1118,6 +1119,13 @@ func readUserSettings(iniFile *ini.File, cfg *Cfg) error { return errors.New("the minimum supported value for the `user_invite_max_lifetime_duration` configuration is 15m (15 minutes)") } + cfg.HiddenUsers = make(map[string]struct{}) + hiddenUsers := users.Key("hidden_users").MustString("") + for _, user := range strings.Split(hiddenUsers, ",") { + user = strings.TrimSpace(user) + cfg.HiddenUsers[user] = struct{}{} + } + return nil }