From 4496ae496ea68d45e281cd4be3d811d9ff2e293a Mon Sep 17 00:00:00 2001 From: Emil Tullstedt Date: Mon, 3 May 2021 10:27:12 +0200 Subject: [PATCH] Access control: Clean up users scopes (#33532) Following discussion in grafana/grafana-enterprise#1292, removing org-scoped users scopes to make it clear that the local organization is the default and the alternative to that is a global scope (for a select few endpoints) --- pkg/api/api.go | 14 +-- pkg/api/index.go | 4 +- pkg/services/accesscontrol/models.go | 7 +- .../ossaccesscontrol/ossaccesscontrol_test.go | 6 +- pkg/services/accesscontrol/roles.go | 96 ++++++------------- 5 files changed, 45 insertions(+), 82 deletions(-) diff --git a/pkg/api/api.go b/pkg/api/api.go index c3131958f32..4a0643de8ea 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -56,7 +56,7 @@ func (hs *HTTPServer) registerRoutes() { r.Get("/datasources/", reqOrgAdmin, hs.Index) r.Get("/datasources/new", reqOrgAdmin, hs.Index) r.Get("/datasources/edit/*", reqOrgAdmin, hs.Index) - r.Get("/org/users", authorize(reqOrgAdmin, accesscontrol.ActionOrgUsersRead, accesscontrol.ScopeOrgCurrentUsersAll), hs.Index) + r.Get("/org/users", authorize(reqOrgAdmin, accesscontrol.ActionOrgUsersRead, accesscontrol.ScopeUsersAll), hs.Index) r.Get("/org/users/new", reqOrgAdmin, hs.Index) r.Get("/org/users/invite", authorize(reqOrgAdmin, accesscontrol.ActionUsersCreate), hs.Index) r.Get("/org/teams", reqCanAccessTeams, hs.Index) @@ -66,7 +66,7 @@ func (hs *HTTPServer) registerRoutes() { r.Get("/configuration", reqGrafanaAdmin, hs.Index) r.Get("/admin", reqGrafanaAdmin, hs.Index) r.Get("/admin/settings", reqGrafanaAdmin, hs.Index) - r.Get("/admin/users", authorize(reqGrafanaAdmin, accesscontrol.ActionUsersRead, accesscontrol.ScopeUsersAll), hs.Index) + r.Get("/admin/users", authorize(reqGrafanaAdmin, accesscontrol.ActionUsersRead, accesscontrol.ScopeGlobalUsersAll), hs.Index) r.Get("/admin/users/create", authorize(reqGrafanaAdmin, accesscontrol.ActionUsersCreate), hs.Index) r.Get("/admin/users/edit/:id", authorize(reqGrafanaAdmin, accesscontrol.ActionUsersRead), hs.Index) r.Get("/admin/orgs", reqGrafanaAdmin, hs.Index) @@ -161,13 +161,13 @@ func (hs *HTTPServer) registerRoutes() { // users (admin permission required) apiRoute.Group("/users", func(usersRoute routing.RouteRegister) { const userIDScope = `users:{{ index . ":id" }}` - usersRoute.Get("/", authorize(reqGrafanaAdmin, accesscontrol.ActionUsersRead, accesscontrol.ScopeUsersAll), routing.Wrap(SearchUsers)) - usersRoute.Get("/search", authorize(reqGrafanaAdmin, accesscontrol.ActionUsersRead, accesscontrol.ScopeUsersAll), routing.Wrap(SearchUsersWithPaging)) + usersRoute.Get("/", authorize(reqGrafanaAdmin, accesscontrol.ActionUsersRead, accesscontrol.ScopeGlobalUsersAll), routing.Wrap(SearchUsers)) + usersRoute.Get("/search", authorize(reqGrafanaAdmin, accesscontrol.ActionUsersRead, accesscontrol.ScopeGlobalUsersAll), routing.Wrap(SearchUsersWithPaging)) usersRoute.Get("/:id", authorize(reqGrafanaAdmin, accesscontrol.ActionUsersRead, userIDScope), routing.Wrap(GetUserByID)) usersRoute.Get("/:id/teams", authorize(reqGrafanaAdmin, accesscontrol.ActionUsersTeamRead, userIDScope), routing.Wrap(GetUserTeams)) usersRoute.Get("/:id/orgs", authorize(reqGrafanaAdmin, accesscontrol.ActionUsersRead, userIDScope), routing.Wrap(GetUserOrgList)) // query parameters /users/lookup?loginOrEmail=admin@example.com - usersRoute.Get("/lookup", authorize(reqGrafanaAdmin, accesscontrol.ActionUsersRead, accesscontrol.ScopeUsersAll), routing.Wrap(GetUserByLoginOrEmail)) + usersRoute.Get("/lookup", authorize(reqGrafanaAdmin, accesscontrol.ActionUsersRead, accesscontrol.ScopeGlobalUsersAll), routing.Wrap(GetUserByLoginOrEmail)) usersRoute.Put("/:id", authorize(reqGrafanaAdmin, accesscontrol.ActionUsersWrite, userIDScope), bind(models.UpdateUserCommand{}), routing.Wrap(UpdateUser)) usersRoute.Post("/:id/using/:orgId", authorize(reqGrafanaAdmin, accesscontrol.ActionUsersWrite, userIDScope), routing.Wrap(UpdateUserActiveOrg)) }) @@ -202,8 +202,8 @@ func (hs *HTTPServer) registerRoutes() { const orgScope = `org:current/users:{{ index . ":userId" }}` orgRoute.Put("/", reqOrgAdmin, bind(dtos.UpdateOrgForm{}), routing.Wrap(UpdateOrgCurrent)) orgRoute.Put("/address", reqOrgAdmin, bind(dtos.UpdateOrgAddressForm{}), routing.Wrap(UpdateOrgAddressCurrent)) - orgRoute.Get("/users", authorize(reqOrgAdmin, accesscontrol.ActionOrgUsersRead, accesscontrol.ScopeOrgCurrentUsersAll), routing.Wrap(hs.GetOrgUsersForCurrentOrg)) - orgRoute.Post("/users", authorize(reqOrgAdmin, accesscontrol.ActionOrgUsersAdd, accesscontrol.ScopeOrgCurrentUsersAll), quota("user"), bind(models.AddOrgUserCommand{}), routing.Wrap(AddOrgUserToCurrentOrg)) + orgRoute.Get("/users", authorize(reqOrgAdmin, accesscontrol.ActionOrgUsersRead, accesscontrol.ScopeUsersAll), routing.Wrap(hs.GetOrgUsersForCurrentOrg)) + orgRoute.Post("/users", authorize(reqOrgAdmin, accesscontrol.ActionOrgUsersAdd, accesscontrol.ScopeUsersAll), quota("user"), bind(models.AddOrgUserCommand{}), routing.Wrap(AddOrgUserToCurrentOrg)) orgRoute.Patch("/users/:userId", authorize(reqOrgAdmin, accesscontrol.ActionOrgUsersRoleUpdate, orgScope), bind(models.UpdateOrgUserCommand{}), routing.Wrap(UpdateOrgUserForCurrentOrg)) orgRoute.Delete("/users/:userId", authorize(reqOrgAdmin, accesscontrol.ActionOrgUsersRemove, orgScope), routing.Wrap(RemoveOrgUserForCurrentOrg)) diff --git a/pkg/api/index.go b/pkg/api/index.go index a4956484293..ca4395da85d 100644 --- a/pkg/api/index.go +++ b/pkg/api/index.go @@ -254,7 +254,7 @@ func (hs *HTTPServer) getNavTree(c *models.ReqContext, hasEditPerm bool) ([]*dto }) } - if hasAccess(ac.ReqOrgAdmin, ac.ActionOrgUsersRead, ac.ScopeOrgCurrentUsersAll) { + if hasAccess(ac.ReqOrgAdmin, ac.ActionOrgUsersRead, ac.ScopeUsersAll) { configNodes = append(configNodes, &dtos.NavLink{ Text: "Users", Id: "users", @@ -349,7 +349,7 @@ func (hs *HTTPServer) buildAdminNavLinks(c *models.ReqContext) []*dtos.NavLink { hasAccess := ac.HasAccess(hs.AccessControl, c) adminNavLinks := []*dtos.NavLink{} - if hasAccess(ac.ReqGrafanaAdmin, ac.ActionUsersRead, ac.ScopeUsersAll) { + if hasAccess(ac.ReqGrafanaAdmin, ac.ActionUsersRead, ac.ScopeGlobalUsersAll) { adminNavLinks = append(adminNavLinks, &dtos.NavLink{ Text: "Users", Id: "global-users", Url: hs.Cfg.AppSubURL + "/admin/users", Icon: "user", }) diff --git a/pkg/services/accesscontrol/models.go b/pkg/services/accesscontrol/models.go index 486c47c8e19..ac9b8ea5e24 100644 --- a/pkg/services/accesscontrol/models.go +++ b/pkg/services/accesscontrol/models.go @@ -76,11 +76,10 @@ const ( ActionLDAPStatusRead = "ldap.status:read" // Global Scopes - ScopeUsersAll = "users:*" - ScopeUsersSelf = "users:self" + ScopeGlobalUsersAll = "global:users:*" - ScopeOrgAllUsersAll = "org:*/users:*" - ScopeOrgCurrentUsersAll = "org:current/users:*" + ScopeUsersSelf = "users:self" + ScopeUsersAll = "users:*" ) const RoleGrafanaAdmin = "Grafana Admin" diff --git a/pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol_test.go b/pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol_test.go index 09c81a182aa..61d5ee8a549 100644 --- a/pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol_test.go +++ b/pkg/services/accesscontrol/ossaccesscontrol/ossaccesscontrol_test.go @@ -58,8 +58,8 @@ func TestEvaluatingPermissions(t *testing.T) { isGrafanaAdmin: false, }, endpoints: []endpointTestCase{ - {permission: accesscontrol.ActionUsersDisable, scope: []string{accesscontrol.ScopeUsersAll}}, - {permission: accesscontrol.ActionUsersEnable, scope: []string{accesscontrol.ScopeUsersAll}}, + {permission: accesscontrol.ActionUsersDisable, scope: []string{accesscontrol.ScopeGlobalUsersAll}}, + {permission: accesscontrol.ActionUsersEnable, scope: []string{accesscontrol.ScopeGlobalUsersAll}}, }, evalResult: true, }, @@ -71,7 +71,7 @@ func TestEvaluatingPermissions(t *testing.T) { isGrafanaAdmin: false, }, endpoints: []endpointTestCase{ - {permission: accesscontrol.ActionUsersCreate, scope: []string{accesscontrol.ScopeUsersAll}}, + {permission: accesscontrol.ActionUsersCreate, scope: []string{accesscontrol.ScopeGlobalUsersAll}}, }, evalResult: false, }, diff --git a/pkg/services/accesscontrol/roles.go b/pkg/services/accesscontrol/roles.go index f10c4cf141e..074059c5b6b 100644 --- a/pkg/services/accesscontrol/roles.go +++ b/pkg/services/accesscontrol/roles.go @@ -25,62 +25,32 @@ var ldapAdminEditRole = RoleDTO{ }), } -var orgsAdminReadRole = RoleDTO{ - Name: orgsAdminRead, +var usersOrgReadRole = RoleDTO{ + Name: usersOrgRead, Version: 1, Permissions: []Permission{ { Action: ActionOrgUsersRead, - Scope: ScopeOrgAllUsersAll, - }, - }, -} - -var orgsAdminEditRole = RoleDTO{ - Name: orgsAdminEdit, - Version: 1, - Permissions: ConcatPermissions(orgsAdminReadRole.Permissions, []Permission{ - { - Action: ActionOrgUsersAdd, - Scope: ScopeOrgAllUsersAll, - }, - { - Action: ActionOrgUsersRemove, - Scope: ScopeOrgAllUsersAll, - }, - { - Action: ActionOrgUsersRoleUpdate, - Scope: ScopeOrgAllUsersAll, - }, - }), -} - -var orgsCurrentReadRole = RoleDTO{ - Name: orgsCurrentRead, - Version: 1, - Permissions: []Permission{ - { - Action: ActionOrgUsersRead, - Scope: ScopeOrgCurrentUsersAll, + Scope: ScopeUsersAll, }, }, } -var orgsCurrentEditRole = RoleDTO{ - Name: orgsCurrentEdit, +var usersOrgEditRole = RoleDTO{ + Name: usersOrgEdit, Version: 1, - Permissions: ConcatPermissions(orgsCurrentReadRole.Permissions, []Permission{ + Permissions: ConcatPermissions(usersOrgReadRole.Permissions, []Permission{ { Action: ActionOrgUsersAdd, - Scope: ScopeOrgCurrentUsersAll, + Scope: ScopeUsersAll, }, { Action: ActionOrgUsersRoleUpdate, - Scope: ScopeOrgCurrentUsersAll, + Scope: ScopeUsersAll, }, { Action: ActionOrgUsersRemove, - Scope: ScopeOrgCurrentUsersAll, + Scope: ScopeUsersAll, }, }), } @@ -91,19 +61,19 @@ var usersAdminReadRole = RoleDTO{ Permissions: []Permission{ { Action: ActionUsersRead, - Scope: ScopeUsersAll, + Scope: ScopeGlobalUsersAll, }, { Action: ActionUsersTeamRead, - Scope: ScopeUsersAll, + Scope: ScopeGlobalUsersAll, }, { Action: ActionUsersAuthTokenList, - Scope: ScopeUsersAll, + Scope: ScopeGlobalUsersAll, }, { Action: ActionUsersQuotasList, - Scope: ScopeUsersAll, + Scope: ScopeGlobalUsersAll, }, }, } @@ -114,42 +84,42 @@ var usersAdminEditRole = RoleDTO{ Permissions: ConcatPermissions(usersAdminReadRole.Permissions, []Permission{ { Action: ActionUsersPasswordUpdate, - Scope: ScopeUsersAll, + Scope: ScopeGlobalUsersAll, }, { Action: ActionUsersCreate, }, { Action: ActionUsersWrite, - Scope: ScopeUsersAll, + Scope: ScopeGlobalUsersAll, }, { Action: ActionUsersDelete, - Scope: ScopeUsersAll, + Scope: ScopeGlobalUsersAll, }, { Action: ActionUsersEnable, - Scope: ScopeUsersAll, + Scope: ScopeGlobalUsersAll, }, { Action: ActionUsersDisable, - Scope: ScopeUsersAll, + Scope: ScopeGlobalUsersAll, }, { Action: ActionUsersPermissionsUpdate, - Scope: ScopeUsersAll, + Scope: ScopeGlobalUsersAll, }, { Action: ActionUsersLogout, - Scope: ScopeUsersAll, + Scope: ScopeGlobalUsersAll, }, { Action: ActionUsersAuthTokenUpdate, - Scope: ScopeUsersAll, + Scope: ScopeGlobalUsersAll, }, { Action: ActionUsersQuotasUpdate, - Scope: ScopeUsersAll, + Scope: ScopeGlobalUsersAll, }, }), } @@ -164,11 +134,8 @@ var PredefinedRoles = map[string]RoleDTO{ usersAdminRead: usersAdminReadRole, usersAdminEdit: usersAdminEditRole, - orgsAdminRead: orgsAdminReadRole, - orgsAdminEdit: orgsAdminEditRole, - - orgsCurrentRead: orgsCurrentReadRole, - orgsCurrentEdit: orgsCurrentEditRole, + usersOrgRead: usersOrgReadRole, + usersOrgEdit: usersOrgEditRole, ldapAdminRead: ldapAdminReadRole, ldapAdminEdit: ldapAdminEditRole, @@ -178,11 +145,8 @@ const ( usersAdminEdit = "grafana:roles:users:admin:edit" usersAdminRead = "grafana:roles:users:admin:read" - orgsAdminEdit = "grafana:roles:orgs:admin:edit" - orgsAdminRead = "grafana:roles:orgs:admin:read" - - orgsCurrentEdit = "grafana:roles:orgs:current:edit" - orgsCurrentRead = "grafana:roles:orgs:current:read" + usersOrgEdit = "grafana:roles:users:org:edit" + usersOrgRead = "grafana:roles:users:org:read" ldapAdminEdit = "grafana:roles:ldap:admin:edit" ldapAdminRead = "grafana:roles:ldap:admin:read" @@ -194,14 +158,14 @@ var PredefinedRoleGrants = map[string][]string{ RoleGrafanaAdmin: { ldapAdminEdit, ldapAdminRead, - orgsAdminEdit, - orgsAdminRead, usersAdminEdit, usersAdminRead, + usersOrgEdit, + usersOrgRead, }, string(models.ROLE_ADMIN): { - orgsCurrentEdit, - orgsCurrentRead, + usersOrgEdit, + usersOrgRead, }, }