From 14bee49f9a8d5c9316a92c89c1f5c684a7c7d21f Mon Sep 17 00:00:00 2001 From: Gabriel MABILLE Date: Fri, 18 Feb 2022 11:27:00 +0100 Subject: [PATCH] AccessControl: Compute metadata from context permissions (#45578) * AccessControl: Compute metadata from context permissions * Remove nil Co-authored-by: Jguer * Check user permissions are set Co-authored-by: Jguer --- pkg/api/{roles.go => accesscontrol.go} | 223 ++++++++++-------- pkg/api/datasources.go | 31 +-- pkg/api/org_users.go | 30 +-- pkg/api/team.go | 41 +--- pkg/api/user.go | 25 +- pkg/api/user_test.go | 23 +- pkg/services/accesscontrol/accesscontrol.go | 24 +- .../accesscontrol/accesscontrol_bench_test.go | 30 +-- .../accesscontrol/accesscontrol_test.go | 46 ++-- pkg/services/serviceaccounts/api/api.go | 21 +- 10 files changed, 213 insertions(+), 281 deletions(-) rename pkg/api/{roles.go => accesscontrol.go} (55%) diff --git a/pkg/api/roles.go b/pkg/api/accesscontrol.go similarity index 55% rename from pkg/api/roles.go rename to pkg/api/accesscontrol.go index 8ec62c68ea7..5215869edf8 100644 --- a/pkg/api/roles.go +++ b/pkg/api/accesscontrol.go @@ -1,8 +1,10 @@ package api import ( + "fmt" + "github.com/grafana/grafana/pkg/models" - "github.com/grafana/grafana/pkg/services/accesscontrol" + ac "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/setting" ) @@ -29,49 +31,49 @@ const ( // API related scopes var ( - ScopeProvisionersAll = accesscontrol.Scope("provisioners", "*") - ScopeProvisionersDashboards = accesscontrol.Scope("provisioners", "dashboards") - ScopeProvisionersPlugins = accesscontrol.Scope("provisioners", "plugins") - ScopeProvisionersDatasources = accesscontrol.Scope("provisioners", "datasources") - ScopeProvisionersNotifications = accesscontrol.Scope("provisioners", "notifications") + ScopeProvisionersAll = ac.Scope("provisioners", "*") + ScopeProvisionersDashboards = ac.Scope("provisioners", "dashboards") + ScopeProvisionersPlugins = ac.Scope("provisioners", "plugins") + ScopeProvisionersDatasources = ac.Scope("provisioners", "datasources") + ScopeProvisionersNotifications = ac.Scope("provisioners", "notifications") - ScopeDatasourcesAll = accesscontrol.Scope("datasources", "*") - ScopeDatasourceID = accesscontrol.Scope("datasources", "id", accesscontrol.Parameter(":id")) - ScopeDatasourceUID = accesscontrol.Scope("datasources", "uid", accesscontrol.Parameter(":uid")) - ScopeDatasourceName = accesscontrol.Scope("datasources", "name", accesscontrol.Parameter(":name")) + ScopeDatasourcesAll = ac.Scope("datasources", "*") + ScopeDatasourceID = ac.Scope("datasources", "id", ac.Parameter(":id")) + ScopeDatasourceUID = ac.Scope("datasources", "uid", ac.Parameter(":uid")) + ScopeDatasourceName = ac.Scope("datasources", "name", ac.Parameter(":name")) ) // declareFixedRoles declares to the AccessControl service fixed roles and their // grants to organization roles ("Viewer", "Editor", "Admin") or "Grafana Admin" // that HTTPServer needs func (hs *HTTPServer) declareFixedRoles() error { - provisioningWriterRole := accesscontrol.RoleRegistration{ - Role: accesscontrol.RoleDTO{ + provisioningWriterRole := ac.RoleRegistration{ + Role: ac.RoleDTO{ Version: 3, Name: "fixed:provisioning:writer", DisplayName: "Provisioning writer", Description: "Reload provisioning.", Group: "Provisioning", - Permissions: []accesscontrol.Permission{ + Permissions: []ac.Permission{ { Action: ActionProvisioningReload, Scope: ScopeProvisionersAll, }, }, }, - Grants: []string{accesscontrol.RoleGrafanaAdmin}, + Grants: []string{ac.RoleGrafanaAdmin}, } - datasourcesExplorerRole := accesscontrol.RoleRegistration{ - Role: accesscontrol.RoleDTO{ + datasourcesExplorerRole := ac.RoleRegistration{ + Role: ac.RoleDTO{ Version: 4, Name: "fixed:datasources:explorer", DisplayName: "Data source explorer", Description: "Enable the Explore feature. Data source permissions still apply; you can only query data sources for which you have query permissions.", Group: "Data sources", - Permissions: []accesscontrol.Permission{ + Permissions: []ac.Permission{ { - Action: accesscontrol.ActionDatasourcesExplore, + Action: ac.ActionDatasourcesExplore, }, }, }, @@ -82,14 +84,14 @@ func (hs *HTTPServer) declareFixedRoles() error { datasourcesExplorerRole.Grants = append(datasourcesExplorerRole.Grants, string(models.ROLE_VIEWER)) } - datasourcesReaderRole := accesscontrol.RoleRegistration{ - Role: accesscontrol.RoleDTO{ + datasourcesReaderRole := ac.RoleRegistration{ + Role: ac.RoleDTO{ Version: 3, Name: "fixed:datasources:reader", DisplayName: "Data source reader", Description: "Read and query all data sources.", Group: "Data sources", - Permissions: []accesscontrol.Permission{ + Permissions: []ac.Permission{ { Action: ActionDatasourcesRead, Scope: ScopeDatasourcesAll, @@ -103,14 +105,14 @@ func (hs *HTTPServer) declareFixedRoles() error { Grants: []string{string(models.ROLE_ADMIN)}, } - datasourcesWriterRole := accesscontrol.RoleRegistration{ - Role: accesscontrol.RoleDTO{ + datasourcesWriterRole := ac.RoleRegistration{ + Role: ac.RoleDTO{ Version: 3, Name: "fixed:datasources:writer", DisplayName: "Data source writer", Description: "Create, update, delete, read, or query data sources.", Group: "Data sources", - Permissions: accesscontrol.ConcatPermissions(datasourcesReaderRole.Role.Permissions, []accesscontrol.Permission{ + Permissions: ac.ConcatPermissions(datasourcesReaderRole.Role.Permissions, []ac.Permission{ { Action: ActionDatasourcesWrite, Scope: ScopeDatasourcesAll, @@ -127,14 +129,14 @@ func (hs *HTTPServer) declareFixedRoles() error { Grants: []string{string(models.ROLE_ADMIN)}, } - datasourcesIdReaderRole := accesscontrol.RoleRegistration{ - Role: accesscontrol.RoleDTO{ + datasourcesIdReaderRole := ac.RoleRegistration{ + Role: ac.RoleDTO{ Version: 4, Name: "fixed:datasources.id:reader", DisplayName: "Data source ID reader", Description: "Read the ID of a data source based on its name.", Group: "Infrequently used", - Permissions: []accesscontrol.Permission{ + Permissions: []ac.Permission{ { Action: ActionDatasourcesIDRead, Scope: ScopeDatasourcesAll, @@ -144,14 +146,14 @@ func (hs *HTTPServer) declareFixedRoles() error { Grants: []string{string(models.ROLE_VIEWER)}, } - datasourcesCompatibilityReaderRole := accesscontrol.RoleRegistration{ - Role: accesscontrol.RoleDTO{ + datasourcesCompatibilityReaderRole := ac.RoleRegistration{ + Role: ac.RoleDTO{ Version: 3, Name: "fixed:datasources:compatibility:querier", DisplayName: "Data source compatibility querier", Description: "Only used for open source compatibility. Query data sources.", Group: "Infrequently used", - Permissions: []accesscontrol.Permission{ + Permissions: []ac.Permission{ {Action: ActionDatasourcesQuery}, {Action: ActionDatasourcesRead}, }, @@ -159,29 +161,29 @@ func (hs *HTTPServer) declareFixedRoles() error { Grants: []string{string(models.ROLE_VIEWER)}, } - orgReaderRole := accesscontrol.RoleRegistration{ - Role: accesscontrol.RoleDTO{ + orgReaderRole := ac.RoleRegistration{ + Role: ac.RoleDTO{ Version: 5, Name: "fixed:organization:reader", DisplayName: "Organization reader", Description: "Read an organization, such as its ID, name, address, or quotas.", Group: "Organizations", - Permissions: []accesscontrol.Permission{ + Permissions: []ac.Permission{ {Action: ActionOrgsRead}, {Action: ActionOrgsQuotasRead}, }, }, - Grants: []string{string(models.ROLE_VIEWER), accesscontrol.RoleGrafanaAdmin}, + Grants: []string{string(models.ROLE_VIEWER), ac.RoleGrafanaAdmin}, } - orgWriterRole := accesscontrol.RoleRegistration{ - Role: accesscontrol.RoleDTO{ + orgWriterRole := ac.RoleRegistration{ + Role: ac.RoleDTO{ Version: 5, Name: "fixed:organization:writer", DisplayName: "Organization writer", Description: "Read an organization, its quotas, or its preferences. Update organization properties, or its preferences.", Group: "Organizations", - Permissions: accesscontrol.ConcatPermissions(orgReaderRole.Role.Permissions, []accesscontrol.Permission{ + Permissions: ac.ConcatPermissions(orgReaderRole.Role.Permissions, []ac.Permission{ {Action: ActionOrgsPreferencesRead}, {Action: ActionOrgsWrite}, {Action: ActionOrgsPreferencesWrite}, @@ -190,71 +192,71 @@ func (hs *HTTPServer) declareFixedRoles() error { Grants: []string{string(models.ROLE_ADMIN)}, } - orgMaintainerRole := accesscontrol.RoleRegistration{ - Role: accesscontrol.RoleDTO{ + orgMaintainerRole := ac.RoleRegistration{ + Role: ac.RoleDTO{ Version: 5, Name: "fixed:organization:maintainer", DisplayName: "Organization maintainer", Description: "Create, read, write, or delete an organization. Read or write an organization's quotas. Needs to be assigned globally.", Group: "Organizations", - Permissions: accesscontrol.ConcatPermissions(orgReaderRole.Role.Permissions, []accesscontrol.Permission{ + Permissions: ac.ConcatPermissions(orgReaderRole.Role.Permissions, []ac.Permission{ {Action: ActionOrgsCreate}, {Action: ActionOrgsWrite}, {Action: ActionOrgsDelete}, {Action: ActionOrgsQuotasWrite}, }), }, - Grants: []string{string(accesscontrol.RoleGrafanaAdmin)}, + Grants: []string{string(ac.RoleGrafanaAdmin)}, } teamCreatorGrants := []string{string(models.ROLE_ADMIN)} if hs.Cfg.EditorsCanAdmin { teamCreatorGrants = append(teamCreatorGrants, string(models.ROLE_EDITOR)) } - teamsCreatorRole := accesscontrol.RoleRegistration{ - Role: accesscontrol.RoleDTO{ + teamsCreatorRole := ac.RoleRegistration{ + Role: ac.RoleDTO{ Name: "fixed:teams:creator", DisplayName: "Team creator", Description: "Create teams and read organisation users (required to manage the created teams).", Group: "Teams", Version: 2, - Permissions: []accesscontrol.Permission{ - {Action: accesscontrol.ActionTeamsCreate}, - {Action: accesscontrol.ActionOrgUsersRead, Scope: accesscontrol.ScopeUsersAll}, + Permissions: []ac.Permission{ + {Action: ac.ActionTeamsCreate}, + {Action: ac.ActionOrgUsersRead, Scope: ac.ScopeUsersAll}, }, }, Grants: teamCreatorGrants, } - teamsWriterRole := accesscontrol.RoleRegistration{ - Role: accesscontrol.RoleDTO{ + teamsWriterRole := ac.RoleRegistration{ + Role: ac.RoleDTO{ Name: "fixed:teams:writer", DisplayName: "Team writer", Description: "Create, read, write, or delete a team as well as controlling team memberships.", Group: "Teams", Version: 2, - Permissions: []accesscontrol.Permission{ - {Action: accesscontrol.ActionTeamsCreate}, - {Action: accesscontrol.ActionTeamsDelete, Scope: accesscontrol.ScopeTeamsAll}, - {Action: accesscontrol.ActionTeamsPermissionsRead, Scope: accesscontrol.ScopeTeamsAll}, - {Action: accesscontrol.ActionTeamsPermissionsWrite, Scope: accesscontrol.ScopeTeamsAll}, - {Action: accesscontrol.ActionTeamsRead, Scope: accesscontrol.ScopeTeamsAll}, - {Action: accesscontrol.ActionTeamsWrite, Scope: accesscontrol.ScopeTeamsAll}, + Permissions: []ac.Permission{ + {Action: ac.ActionTeamsCreate}, + {Action: ac.ActionTeamsDelete, Scope: ac.ScopeTeamsAll}, + {Action: ac.ActionTeamsPermissionsRead, Scope: ac.ScopeTeamsAll}, + {Action: ac.ActionTeamsPermissionsWrite, Scope: ac.ScopeTeamsAll}, + {Action: ac.ActionTeamsRead, Scope: ac.ScopeTeamsAll}, + {Action: ac.ActionTeamsWrite, Scope: ac.ScopeTeamsAll}, }, }, Grants: []string{string(models.ROLE_ADMIN)}, } - annotationsReaderRole := accesscontrol.RoleRegistration{ - Role: accesscontrol.RoleDTO{ + annotationsReaderRole := ac.RoleRegistration{ + Role: ac.RoleDTO{ Name: "fixed:annotations:reader", DisplayName: "Annotation reader", Description: "Read annotations and tags", Group: "Annotations", Version: 1, - Permissions: []accesscontrol.Permission{ - {Action: accesscontrol.ActionAnnotationsRead, Scope: accesscontrol.ScopeAnnotationsAll}, - {Action: accesscontrol.ActionAnnotationsTagsRead, Scope: accesscontrol.ScopeAnnotationsTagsAll}, + Permissions: []ac.Permission{ + {Action: ac.ActionAnnotationsRead, Scope: ac.ScopeAnnotationsAll}, + {Action: ac.ActionAnnotationsTagsRead, Scope: ac.ScopeAnnotationsTagsAll}, }, }, Grants: []string{string(models.ROLE_VIEWER)}, @@ -271,69 +273,96 @@ func (hs *HTTPServer) declareFixedRoles() error { // here is the list of complex evaluators we use in this package // dataSourcesConfigurationAccessEvaluator is used to protect the "Configure > Data sources" tab access -var dataSourcesConfigurationAccessEvaluator = accesscontrol.EvalAll( - accesscontrol.EvalPermission(ActionDatasourcesRead), - accesscontrol.EvalAny( - accesscontrol.EvalPermission(ActionDatasourcesCreate), - accesscontrol.EvalPermission(ActionDatasourcesDelete), - accesscontrol.EvalPermission(ActionDatasourcesWrite), +var dataSourcesConfigurationAccessEvaluator = ac.EvalAll( + ac.EvalPermission(ActionDatasourcesRead), + ac.EvalAny( + ac.EvalPermission(ActionDatasourcesCreate), + ac.EvalPermission(ActionDatasourcesDelete), + ac.EvalPermission(ActionDatasourcesWrite), ), ) // dataSourcesNewAccessEvaluator is used to protect the "Configure > Data sources > New" page access -var dataSourcesNewAccessEvaluator = accesscontrol.EvalAll( - accesscontrol.EvalPermission(ActionDatasourcesRead), - accesscontrol.EvalPermission(ActionDatasourcesCreate), - accesscontrol.EvalPermission(ActionDatasourcesWrite), +var dataSourcesNewAccessEvaluator = ac.EvalAll( + ac.EvalPermission(ActionDatasourcesRead), + ac.EvalPermission(ActionDatasourcesCreate), + ac.EvalPermission(ActionDatasourcesWrite), ) // dataSourcesEditAccessEvaluator is used to protect the "Configure > Data sources > Edit" page access -var dataSourcesEditAccessEvaluator = accesscontrol.EvalAll( - accesscontrol.EvalPermission(ActionDatasourcesRead), - accesscontrol.EvalPermission(ActionDatasourcesWrite), +var dataSourcesEditAccessEvaluator = ac.EvalAll( + ac.EvalPermission(ActionDatasourcesRead), + ac.EvalPermission(ActionDatasourcesWrite), ) // orgPreferencesAccessEvaluator is used to protect the "Configure > Preferences" page access -var orgPreferencesAccessEvaluator = accesscontrol.EvalAny( - accesscontrol.EvalAll( - accesscontrol.EvalPermission(ActionOrgsRead), - accesscontrol.EvalPermission(ActionOrgsWrite), +var orgPreferencesAccessEvaluator = ac.EvalAny( + ac.EvalAll( + ac.EvalPermission(ActionOrgsRead), + ac.EvalPermission(ActionOrgsWrite), ), - accesscontrol.EvalAll( - accesscontrol.EvalPermission(ActionOrgsPreferencesRead), - accesscontrol.EvalPermission(ActionOrgsPreferencesWrite), + ac.EvalAll( + ac.EvalPermission(ActionOrgsPreferencesRead), + ac.EvalPermission(ActionOrgsPreferencesWrite), ), ) // orgsAccessEvaluator is used to protect the "Server Admin > Orgs" page access // (you need to have read access to update or delete orgs; read is the minimum) -var orgsAccessEvaluator = accesscontrol.EvalPermission(ActionOrgsRead) +var orgsAccessEvaluator = ac.EvalPermission(ActionOrgsRead) // orgsCreateAccessEvaluator is used to protect the "Server Admin > Orgs > New Org" page access -var orgsCreateAccessEvaluator = accesscontrol.EvalAll( - accesscontrol.EvalPermission(ActionOrgsRead), - accesscontrol.EvalPermission(ActionOrgsCreate), +var orgsCreateAccessEvaluator = ac.EvalAll( + ac.EvalPermission(ActionOrgsRead), + ac.EvalPermission(ActionOrgsCreate), ) // teamsAccessEvaluator is used to protect the "Configuration > Teams" page access // grants access to a user when they can either create teams or can read and update a team -var teamsAccessEvaluator = accesscontrol.EvalAny( - accesscontrol.EvalPermission(accesscontrol.ActionTeamsCreate), - accesscontrol.EvalAll( - accesscontrol.EvalPermission(accesscontrol.ActionTeamsRead), - accesscontrol.EvalAny( - accesscontrol.EvalPermission(accesscontrol.ActionTeamsWrite), - accesscontrol.EvalPermission(accesscontrol.ActionTeamsPermissionsWrite), +var teamsAccessEvaluator = ac.EvalAny( + ac.EvalPermission(ac.ActionTeamsCreate), + ac.EvalAll( + ac.EvalPermission(ac.ActionTeamsRead), + ac.EvalAny( + ac.EvalPermission(ac.ActionTeamsWrite), + ac.EvalPermission(ac.ActionTeamsPermissionsWrite), ), ), ) // teamsEditAccessEvaluator is used to protect the "Configuration > Teams > edit" page access -var teamsEditAccessEvaluator = accesscontrol.EvalAll( - accesscontrol.EvalPermission(accesscontrol.ActionTeamsRead), - accesscontrol.EvalAny( - accesscontrol.EvalPermission(accesscontrol.ActionTeamsCreate), - accesscontrol.EvalPermission(accesscontrol.ActionTeamsWrite), - accesscontrol.EvalPermission(accesscontrol.ActionTeamsPermissionsWrite), +var teamsEditAccessEvaluator = ac.EvalAll( + ac.EvalPermission(ac.ActionTeamsRead), + ac.EvalAny( + ac.EvalPermission(ac.ActionTeamsCreate), + ac.EvalPermission(ac.ActionTeamsWrite), + ac.EvalPermission(ac.ActionTeamsPermissionsWrite), ), ) + +// Metadata helpers +// getAccessControlMetadata returns the accesscontrol metadata associated with a given resource +func (hs *HTTPServer) getAccessControlMetadata(c *models.ReqContext, resource string, id int64) ac.Metadata { + key := fmt.Sprintf("%d", id) + ids := map[string]bool{key: true} + + return hs.getMultiAccessControlMetadata(c, resource, ids)[key] +} + +// getMultiAccessControlMetadata returns the accesscontrol metadata associated with a given set of resources +func (hs *HTTPServer) getMultiAccessControlMetadata(c *models.ReqContext, resource string, ids map[string]bool) map[string]ac.Metadata { + if hs.AccessControl.IsDisabled() || !c.QueryBool("accesscontrol") { + return map[string]ac.Metadata{} + } + + if c.SignedInUser.Permissions == nil { + return map[string]ac.Metadata{} + } + + permissions, ok := c.SignedInUser.Permissions[c.OrgId] + if !ok { + return map[string]ac.Metadata{} + } + + return ac.GetResourcesMetadata(c.Req.Context(), permissions, resource, ids) +} diff --git a/pkg/api/datasources.go b/pkg/api/datasources.go index 138635024c6..9af7d1c247b 100644 --- a/pkg/api/datasources.go +++ b/pkg/api/datasources.go @@ -17,7 +17,6 @@ import ( "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/plugins/adapters" - "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/util" "github.com/grafana/grafana/pkg/web" ) @@ -71,23 +70,6 @@ func (hs *HTTPServer) GetDataSources(c *models.ReqContext) response.Response { return response.JSON(200, &result) } -func (hs *HTTPServer) getDataSourceAccessControlMetadata(c *models.ReqContext, dsID int64) (accesscontrol.Metadata, error) { - if hs.AccessControl.IsDisabled() || !c.QueryBool("accesscontrol") { - return nil, nil - } - - userPermissions, err := hs.AccessControl.GetUserPermissions(c.Req.Context(), c.SignedInUser, - accesscontrol.Options{ReloadCache: false}) - if err != nil || len(userPermissions) == 0 { - return nil, err - } - - key := fmt.Sprintf("%d", dsID) - dsIDs := map[string]bool{key: true} - - return accesscontrol.GetResourcesMetadata(c.Req.Context(), userPermissions, "datasources", dsIDs)[key], nil -} - // GET /api/datasources/:id func (hs *HTTPServer) GetDataSourceById(c *models.ReqContext) response.Response { id, err := strconv.ParseInt(web.Params(c.Req)[":id"], 10, 64) @@ -115,12 +97,9 @@ func (hs *HTTPServer) GetDataSourceById(c *models.ReqContext) response.Response } dto := convertModelToDtos(filtered[0]) + // Add accesscontrol metadata - metadata, err := hs.getDataSourceAccessControlMetadata(c, dto.Id) - if err != nil { - return response.Error(http.StatusInternalServerError, "Failed to query metadata", err) - } - dto.AccessControl = metadata + dto.AccessControl = hs.getAccessControlMetadata(c, "datasources", dto.Id) return response.JSON(200, &dto) } @@ -179,11 +158,7 @@ func (hs *HTTPServer) GetDataSourceByUID(c *models.ReqContext) response.Response dto := convertModelToDtos(filtered[0]) // Add accesscontrol metadata - metadata, err := hs.getDataSourceAccessControlMetadata(c, dto.Id) - if err != nil { - return response.Error(http.StatusInternalServerError, "Failed to query metadata", err) - } - dto.AccessControl = metadata + dto.AccessControl = hs.getAccessControlMetadata(c, "datasources", dto.Id) return response.JSON(200, &dto) } diff --git a/pkg/api/org_users.go b/pkg/api/org_users.go index dfc70933943..cad6b3715f6 100644 --- a/pkg/api/org_users.go +++ b/pkg/api/org_users.go @@ -10,7 +10,6 @@ import ( "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/api/response" "github.com/grafana/grafana/pkg/models" - "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/util" "github.com/grafana/grafana/pkg/web" ) @@ -113,19 +112,6 @@ func (hs *HTTPServer) GetOrgUsersForCurrentOrgLookup(c *models.ReqContext) respo return response.JSON(200, result) } -func (hs *HTTPServer) getUserAccessControlMetadata(c *models.ReqContext, resourceIDs map[string]bool) (map[string]accesscontrol.Metadata, error) { - if hs.AccessControl == nil || hs.AccessControl.IsDisabled() || !c.QueryBool("accesscontrol") { - return nil, nil - } - - userPermissions, err := hs.AccessControl.GetUserPermissions(c.Req.Context(), c.SignedInUser, accesscontrol.Options{ReloadCache: false}) - if err != nil || len(userPermissions) == 0 { - return nil, err - } - - return accesscontrol.GetResourcesMetadata(c.Req.Context(), userPermissions, "users", resourceIDs), nil -} - // GET /api/orgs/:orgId/users func (hs *HTTPServer) GetOrgUsers(c *models.ReqContext) response.Response { orgId, err := strconv.ParseInt(web.Params(c.Req)[":orgId"], 10, 64) @@ -164,17 +150,11 @@ func (hs *HTTPServer) getOrgUsersHelper(c *models.ReqContext, query *models.GetO filteredUsers = append(filteredUsers, user) } - accessControlMetadata, errAC := hs.getUserAccessControlMetadata(c, userIDs) - if errAC != nil { - hs.log.Error("Failed to get access control metadata", "error", errAC) - - return filteredUsers, nil - } else if accessControlMetadata == nil { - return filteredUsers, nil - } - - for i := range filteredUsers { - filteredUsers[i].AccessControl = accessControlMetadata[fmt.Sprint(filteredUsers[i].UserId)] + accessControlMetadata := hs.getMultiAccessControlMetadata(c, "users", userIDs) + if len(accessControlMetadata) > 0 { + for i := range filteredUsers { + filteredUsers[i].AccessControl = accessControlMetadata[fmt.Sprint(filteredUsers[i].UserId)] + } } return filteredUsers, nil diff --git a/pkg/api/team.go b/pkg/api/team.go index 4f9a38ffb27..2b256959e9c 100644 --- a/pkg/api/team.go +++ b/pkg/api/team.go @@ -2,14 +2,12 @@ package api import ( "errors" - "fmt" "net/http" "strconv" "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/api/response" "github.com/grafana/grafana/pkg/models" - "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/util" "github.com/grafana/grafana/pkg/web" @@ -105,20 +103,6 @@ func (hs *HTTPServer) DeleteTeamByID(c *models.ReqContext) response.Response { return response.Success("Team deleted") } -func (hs *HTTPServer) getTeamsAccessControlMetadata(c *models.ReqContext, teamIDs map[string]bool) (map[string]accesscontrol.Metadata, error) { - if hs.AccessControl.IsDisabled() || !c.QueryBool("accesscontrol") { - return nil, nil - } - - userPermissions, err := hs.AccessControl.GetUserPermissions(c.Req.Context(), c.SignedInUser, accesscontrol.Options{ReloadCache: false}) - if err != nil || len(userPermissions) == 0 { - hs.log.Warn("could not fetch accesscontrol metadata for teams", "error", err) - return nil, err - } - - return accesscontrol.GetResourcesMetadata(c.Req.Context(), userPermissions, "teams", teamIDs), nil -} - // GET /api/teams/search func (hs *HTTPServer) SearchTeams(c *models.ReqContext) response.Response { perPage := c.QueryInt("perpage") @@ -157,8 +141,8 @@ func (hs *HTTPServer) SearchTeams(c *models.ReqContext) response.Response { teamIDs[strconv.FormatInt(team.Id, 10)] = true } - metadata, err := hs.getTeamsAccessControlMetadata(c, teamIDs) - if err == nil && len(metadata) != 0 { + metadata := hs.getMultiAccessControlMetadata(c, "teams", teamIDs) + if len(metadata) > 0 { for _, team := range query.Result.Teams { team.AccessControl = metadata[strconv.FormatInt(team.Id, 10)] } @@ -170,23 +154,6 @@ func (hs *HTTPServer) SearchTeams(c *models.ReqContext) response.Response { return response.JSON(200, query.Result) } -func (hs *HTTPServer) getTeamAccessControlMetadata(c *models.ReqContext, teamID int64) (accesscontrol.Metadata, error) { - if hs.AccessControl.IsDisabled() || !c.QueryBool("accesscontrol") { - return nil, nil - } - - userPermissions, err := hs.AccessControl.GetUserPermissions(c.Req.Context(), c.SignedInUser, accesscontrol.Options{ReloadCache: false}) - if err != nil || len(userPermissions) == 0 { - hs.log.Warn("could not fetch accesscontrol metadata", "team", teamID, "error", err) - return nil, err - } - - key := fmt.Sprintf("%d", teamID) - teamIDs := map[string]bool{key: true} - - return accesscontrol.GetResourcesMetadata(c.Req.Context(), userPermissions, "teams", teamIDs)[key], nil -} - // UserFilter returns the user ID used in a filter when querying a team // 1. If the user is a viewer or editor, this will return the user's ID. // 2. If the user is an admin, this will return models.FilterIgnoreUser (0) @@ -227,8 +194,8 @@ func (hs *HTTPServer) GetTeamByID(c *models.ReqContext) response.Response { return response.Error(500, "Failed to get Team", err) } - metadata, _ := hs.getTeamAccessControlMetadata(c, query.Result.Id) - query.Result.AccessControl = metadata + // Add accesscontrol metadata + query.Result.AccessControl = hs.getAccessControlMetadata(c, "teams", query.Result.Id) query.Result.AvatarUrl = dtos.GetGravatarUrlWithDefault(query.Result.Email, query.Result.Name) return response.JSON(200, &query.Result) diff --git a/pkg/api/user.go b/pkg/api/user.go index 64f609fff43..20babbf91cc 100644 --- a/pkg/api/user.go +++ b/pkg/api/user.go @@ -3,14 +3,12 @@ package api import ( "context" "errors" - "fmt" "net/http" "strconv" "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/api/response" "github.com/grafana/grafana/pkg/models" - "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" "github.com/grafana/grafana/pkg/web" @@ -48,33 +46,12 @@ func (hs *HTTPServer) getUserUserProfile(c *models.ReqContext, userID int64) res query.Result.IsExternal = true } - accessControlMetadata, errAC := hs.getGlobalUserAccessControlMetadata(c, userID) - if errAC != nil { - hs.log.Error("Failed to get access control metadata", "error", errAC) - } - - query.Result.AccessControl = accessControlMetadata + query.Result.AccessControl = hs.getAccessControlMetadata(c, "global:users", userID) query.Result.AvatarUrl = dtos.GetGravatarUrl(query.Result.Email) return response.JSON(200, query.Result) } -func (hs *HTTPServer) getGlobalUserAccessControlMetadata(c *models.ReqContext, userID int64) (accesscontrol.Metadata, error) { - if hs.AccessControl == nil || hs.AccessControl.IsDisabled() || !c.QueryBool("accesscontrol") { - return nil, nil - } - - userPermissions, err := hs.AccessControl.GetUserPermissions(c.Req.Context(), c.SignedInUser, accesscontrol.Options{ReloadCache: false}) - if err != nil || len(userPermissions) == 0 { - return nil, err - } - - key := fmt.Sprintf("%d", userID) - userIDs := map[string]bool{key: true} - - return accesscontrol.GetResourcesMetadata(c.Req.Context(), userPermissions, "global:users", userIDs)[key], nil -} - // GET /api/users/lookup func (hs *HTTPServer) GetUserByLoginOrEmail(c *models.ReqContext) response.Response { query := models.GetUserByLoginQuery{LoginOrEmail: c.Query("loginOrEmail")} diff --git a/pkg/api/user_test.go b/pkg/api/user_test.go index 2841d5bd5d2..e52fcf09778 100644 --- a/pkg/api/user_test.go +++ b/pkg/api/user_test.go @@ -8,32 +8,33 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/oauth2" + "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" + acmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock" "github.com/grafana/grafana/pkg/services/login/authinfoservice" authinfostore "github.com/grafana/grafana/pkg/services/login/authinfoservice/database" + "github.com/grafana/grafana/pkg/services/searchusers" "github.com/grafana/grafana/pkg/services/searchusers/filters" "github.com/grafana/grafana/pkg/services/secrets/database" secretsManager "github.com/grafana/grafana/pkg/services/secrets/manager" "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/services/sqlstore/mockstore" "github.com/grafana/grafana/pkg/setting" - "golang.org/x/oauth2" - - "github.com/grafana/grafana/pkg/services/searchusers" - - "github.com/grafana/grafana/pkg/bus" - "github.com/grafana/grafana/pkg/components/simplejson" - "github.com/grafana/grafana/pkg/models" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) func TestUserAPIEndpoint_userLoggedIn(t *testing.T) { settings := setting.NewCfg() sqlStore := sqlstore.InitTestDB(t) hs := &HTTPServer{ - Cfg: settings, - SQLStore: sqlStore, + Cfg: settings, + SQLStore: sqlStore, + AccessControl: &acmock.Mock{}, } mockResult := models.SearchUserQueryResult{ diff --git a/pkg/services/accesscontrol/accesscontrol.go b/pkg/services/accesscontrol/accesscontrol.go index b2deeb36845..03ae32a23a2 100644 --- a/pkg/services/accesscontrol/accesscontrol.go +++ b/pkg/services/accesscontrol/accesscontrol.go @@ -151,7 +151,7 @@ func addActionToMetadata(allMetadata map[string]Metadata, action, id string) map } // GetResourcesMetadata returns a map of accesscontrol metadata, listing for each resource, users available actions -func GetResourcesMetadata(ctx context.Context, permissions []*Permission, resource string, resourceIDs map[string]bool) map[string]Metadata { +func GetResourcesMetadata(ctx context.Context, permissions map[string][]string, resource string, ids map[string]bool) map[string]Metadata { allScope := GetResourceAllScope(resource) allIDScope := GetResourceAllIDScope(resource) @@ -162,16 +162,18 @@ func GetResourcesMetadata(ctx context.Context, permissions []*Permission, resour // Loop through permissions once result := map[string]Metadata{} - for _, p := range permissions { - if p.Scope == "*" || p.Scope == allScope || p.Scope == allIDScope { - // Add global action to all resources - for id := range resourceIDs { - result = addActionToMetadata(result, p.Action, id) - } - } else { - if len(p.Scope) > idIndex && strings.HasPrefix(p.Scope, idPrefix) && resourceIDs[p.Scope[idIndex:]] { - // Add action to a specific resource - result = addActionToMetadata(result, p.Action, p.Scope[idIndex:]) + for action, scopes := range permissions { + for _, scope := range scopes { + if scope == "*" || scope == allScope || scope == allIDScope { + // Add global action to all resources + for id := range ids { + result = addActionToMetadata(result, action, id) + } + } else { + if len(scope) > idIndex && strings.HasPrefix(scope, idPrefix) && ids[scope[idIndex:]] { + // Add action to a specific resource + result = addActionToMetadata(result, action, scope[idIndex:]) + } } } } diff --git a/pkg/services/accesscontrol/accesscontrol_bench_test.go b/pkg/services/accesscontrol/accesscontrol_bench_test.go index 2259508f67e..b6942e664d3 100644 --- a/pkg/services/accesscontrol/accesscontrol_bench_test.go +++ b/pkg/services/accesscontrol/accesscontrol_bench_test.go @@ -9,17 +9,17 @@ import ( "github.com/stretchr/testify/assert" ) -func setupTestEnv(b *testing.B, resourceCount, permissionPerResource int) ([]*Permission, map[string]bool) { - res := make([]*Permission, resourceCount*permissionPerResource) +func setupTestEnv(b *testing.B, resourceCount, permissionPerResource int) (map[string][]string, map[string]bool) { + res := map[string][]string{} ids := make(map[string]bool, resourceCount) - for r := 0; r < resourceCount; r++ { - for p := 0; p < permissionPerResource; p++ { - perm := Permission{Action: fmt.Sprintf("resources:action%v", p), Scope: fmt.Sprintf("resources:id:%v", r)} - id := r*permissionPerResource + p - res[id] = &perm + for p := 0; p < permissionPerResource; p++ { + action := fmt.Sprintf("resources:action%v", p) + for r := 0; r < resourceCount; r++ { + scope := fmt.Sprintf("resources:id:%v", r) + res[action] = append(res[action], scope) + ids[fmt.Sprintf("%d", r)] = true } - ids[fmt.Sprintf("%d", r)] = true } return res, ids @@ -40,23 +40,23 @@ func benchGetMetadata(b *testing.B, resourceCount, permissionPerResource int) { } // Lots of permissions -func BenchmarkGetResourcesMetadata_10_1000(b *testing.B) { benchGetMetadata(b, 10, 1000) } // ~0.0017s/op -func BenchmarkGetResourcesMetadata_10_10000(b *testing.B) { benchGetMetadata(b, 10, 10000) } // ~0.016s/op -func BenchmarkGetResourcesMetadata_10_100000(b *testing.B) { benchGetMetadata(b, 10, 100000) } // ~0.17s/op +func BenchmarkGetResourcesMetadata_10_1000(b *testing.B) { benchGetMetadata(b, 10, 1000) } // ~0.0022s/op +func BenchmarkGetResourcesMetadata_10_10000(b *testing.B) { benchGetMetadata(b, 10, 10000) } // ~0.019s/op +func BenchmarkGetResourcesMetadata_10_100000(b *testing.B) { benchGetMetadata(b, 10, 100000) } // ~0.25s/op func BenchmarkGetResourcesMetadata_10_1000000(b *testing.B) { if testing.Short() { b.Skip("Skipping benchmark in short mode") } benchGetMetadata(b, 10, 1000000) -} // ~3.89s/op +} // ~5.8s/op // Lots of resources func BenchmarkGetResourcesMetadata_1000_10(b *testing.B) { benchGetMetadata(b, 1000, 10) } // ~0,0023s/op -func BenchmarkGetResourcesMetadata_10000_10(b *testing.B) { benchGetMetadata(b, 10000, 10) } // ~0.021s/op -func BenchmarkGetResourcesMetadata_100000_10(b *testing.B) { benchGetMetadata(b, 100000, 10) } // ~0.22s/op +func BenchmarkGetResourcesMetadata_10000_10(b *testing.B) { benchGetMetadata(b, 10000, 10) } // ~0.022s/op +func BenchmarkGetResourcesMetadata_100000_10(b *testing.B) { benchGetMetadata(b, 100000, 10) } // ~0.26s/op func BenchmarkGetResourcesMetadata_1000000_10(b *testing.B) { if testing.Short() { b.Skip("Skipping benchmark in short mode") } benchGetMetadata(b, 1000000, 10) -} // ~2.8s/op +} // ~4.1s/op diff --git a/pkg/services/accesscontrol/accesscontrol_test.go b/pkg/services/accesscontrol/accesscontrol_test.go index e373b08d172..eef8e0b1e11 100644 --- a/pkg/services/accesscontrol/accesscontrol_test.go +++ b/pkg/services/accesscontrol/accesscontrol_test.go @@ -12,7 +12,7 @@ func TestGetResourcesMetadata(t *testing.T) { desc string resource string resourcesIDs map[string]bool - permissions []*Permission + permissions map[string][]string expected map[string]Metadata }{ { @@ -24,10 +24,10 @@ func TestGetResourcesMetadata(t *testing.T) { { desc: "Should return no permission for resources 1,2,3 given the user has permissions for 4 only", resource: "resources", - permissions: []*Permission{ - {Action: "resources:action1", Scope: Scope("resources", "id", "4")}, - {Action: "resources:action2", Scope: Scope("resources", "id", "4")}, - {Action: "resources:action3", Scope: Scope("resources", "id", "4")}, + permissions: map[string][]string{ + "resources:action1": {Scope("resources", "id", "4")}, + "resources:action2": {Scope("resources", "id", "4")}, + "resources:action3": {Scope("resources", "id", "4")}, }, resourcesIDs: map[string]bool{"1": true, "2": true, "3": true}, expected: map[string]Metadata{}, @@ -35,10 +35,10 @@ func TestGetResourcesMetadata(t *testing.T) { { desc: "Should only return permissions for resources 1 and 2, given the user has no permissions for 3", resource: "resources", - permissions: []*Permission{ - {Action: "resources:action1", Scope: Scope("resources", "id", "1")}, - {Action: "resources:action2", Scope: Scope("resources", "id", "2")}, - {Action: "resources:action3", Scope: Scope("resources", "id", "2")}, + permissions: map[string][]string{ + "resources:action1": {Scope("resources", "id", "1")}, + "resources:action2": {Scope("resources", "id", "2")}, + "resources:action3": {Scope("resources", "id", "2")}, }, resourcesIDs: map[string]bool{"1": true, "2": true, "3": true}, expected: map[string]Metadata{ @@ -49,13 +49,13 @@ func TestGetResourcesMetadata(t *testing.T) { { desc: "Should return permissions with global scopes for resources 1,2,3", resource: "resources", - permissions: []*Permission{ - {Action: "resources:action4", Scope: Scope("resources", "id", "*")}, - {Action: "resources:action5", Scope: Scope("resources", "*")}, - {Action: "resources:action6", Scope: "*"}, - {Action: "resources:action1", Scope: Scope("resources", "id", "1")}, - {Action: "resources:action2", Scope: Scope("resources", "id", "2")}, - {Action: "resources:action3", Scope: Scope("resources", "id", "2")}, + permissions: map[string][]string{ + "resources:action1": {Scope("resources", "id", "1")}, + "resources:action2": {Scope("resources", "id", "2")}, + "resources:action3": {Scope("resources", "id", "2")}, + "resources:action4": {Scope("resources", "id", "*")}, + "resources:action5": {Scope("resources", "*")}, + "resources:action6": {"*"}, }, resourcesIDs: map[string]bool{"1": true, "2": true, "3": true}, expected: map[string]Metadata{ @@ -67,11 +67,10 @@ func TestGetResourcesMetadata(t *testing.T) { { desc: "Should correctly filter out irrelevant permissions for resources 1,2,3", resource: "resources", - permissions: []*Permission{ - {Action: "resources:action1", Scope: Scope("resources", "id", "1")}, - {Action: "otherresources:action1", Scope: Scope("resources", "id", "1")}, - {Action: "resources:action2", Scope: Scope("otherresources", "id", "*")}, - {Action: "otherresources:action1", Scope: Scope("otherresources", "id", "*")}, + permissions: map[string][]string{ + "resources:action1": {Scope("resources", "id", "1")}, + "resources:action2": {Scope("otherresources", "id", "*")}, + "otherresources:action1": {Scope("resources", "id", "1"), Scope("otherresources", "id", "*")}, }, resourcesIDs: map[string]bool{"1": true, "2": true, "3": true}, expected: map[string]Metadata{ @@ -81,9 +80,8 @@ func TestGetResourcesMetadata(t *testing.T) { { desc: "Should correctly handle permissions with multilayer scope", resource: "resources:sub", - permissions: []*Permission{ - {Action: "resources:action1", Scope: Scope("resources", "sub", "id", "1")}, - {Action: "resources:action1", Scope: Scope("resources", "sub", "id", "123")}, + permissions: map[string][]string{ + "resources:action1": {Scope("resources", "sub", "id", "1"), Scope("resources", "sub", "id", "123")}, }, resourcesIDs: map[string]bool{"1": true, "123": true}, expected: map[string]Metadata{ diff --git a/pkg/services/serviceaccounts/api/api.go b/pkg/services/serviceaccounts/api/api.go index b8ee87f00fa..1d26c54ce01 100644 --- a/pkg/services/serviceaccounts/api/api.go +++ b/pkg/services/serviceaccounts/api/api.go @@ -150,8 +150,8 @@ func (api *ServiceAccountsAPI) ListServiceAccounts(c *models.ReqContext) respons saIDs[strconv.FormatInt(serviceAccounts[i].Id, 10)] = true } - metadata, err := api.getAccessControlMetadata(c, saIDs) - if err == nil && len(metadata) != 0 { + metadata := api.getAccessControlMetadata(c, saIDs) + if len(metadata) > 0 { for i := range serviceAccounts { serviceAccounts[i].AccessControl = metadata[strconv.FormatInt(serviceAccounts[i].Id, 10)] } @@ -160,18 +160,21 @@ func (api *ServiceAccountsAPI) ListServiceAccounts(c *models.ReqContext) respons return response.JSON(http.StatusOK, serviceAccounts) } -func (api *ServiceAccountsAPI) getAccessControlMetadata(c *models.ReqContext, saIDs map[string]bool) (map[string]accesscontrol.Metadata, error) { +func (api *ServiceAccountsAPI) getAccessControlMetadata(c *models.ReqContext, saIDs map[string]bool) map[string]accesscontrol.Metadata { if api.accesscontrol.IsDisabled() || !c.QueryBool("accesscontrol") { - return nil, nil + return map[string]accesscontrol.Metadata{} } - userPermissions, err := api.accesscontrol.GetUserPermissions(c.Req.Context(), c.SignedInUser, accesscontrol.Options{ReloadCache: false}) - if err != nil || len(userPermissions) == 0 { - api.log.Warn("could not fetch accesscontrol metadata for teams", "error", err) - return nil, err + if c.SignedInUser.Permissions == nil { + return map[string]accesscontrol.Metadata{} } - return accesscontrol.GetResourcesMetadata(c.Req.Context(), userPermissions, "serviceaccounts", saIDs), nil + permissions, ok := c.SignedInUser.Permissions[c.OrgId] + if !ok { + return map[string]accesscontrol.Metadata{} + } + + return accesscontrol.GetResourcesMetadata(c.Req.Context(), permissions, "serviceaccounts", saIDs) } func (api *ServiceAccountsAPI) RetrieveServiceAccount(ctx *models.ReqContext) response.Response {