From 26339f978bc8d8441ee09d6825ad999927231b08 Mon Sep 17 00:00:00 2001 From: Jo Date: Fri, 18 Aug 2023 12:42:18 +0200 Subject: [PATCH] Auth: Move access control API to SignedInUser interface (#73144) * move access control api to SignedInUser interface * remove unused code * add logic for reading perms from a specific org * move the specific org logic to org_user.go * add a comment --------- Co-authored-by: IevaVasiljeva --- pkg/api/accesscontrol.go | 13 ++++--------- pkg/api/admin.go | 6 +++--- pkg/api/apikey.go | 2 +- pkg/api/folder.go | 2 +- pkg/api/org_users.go | 10 +++++++++- pkg/api/plugins.go | 3 +-- pkg/api/team.go | 2 +- pkg/services/accesscontrol/accesscontrol.go | 8 ++++---- pkg/services/accesscontrol/actest/fake.go | 3 +-- pkg/services/accesscontrol/api/api.go | 3 ++- pkg/services/accesscontrol/middleware.go | 11 +++++++---- pkg/services/accesscontrol/mock/service_mock.go | 4 ++-- .../ossaccesscontrol/permissions_services.go | 3 ++- .../accesscontrol/resourcepermissions/api.go | 8 ++++---- .../accesscontrol/resourcepermissions/models.go | 4 ++-- .../accesscontrol/resourcepermissions/service.go | 7 ++++--- pkg/services/auth/identity/requester.go | 2 ++ 17 files changed, 50 insertions(+), 41 deletions(-) diff --git a/pkg/api/accesscontrol.go b/pkg/api/accesscontrol.go index 858c7bad723..c2cbec08a44 100644 --- a/pkg/api/accesscontrol.go +++ b/pkg/api/accesscontrol.go @@ -463,25 +463,20 @@ func (hs *HTTPServer) declareFixedRoles() error { func (hs *HTTPServer) getAccessControlMetadata(c *contextmodel.ReqContext, orgID int64, prefix string, resourceID string) ac.Metadata { ids := map[string]bool{resourceID: true} - return hs.getMultiAccessControlMetadata(c, orgID, prefix, ids)[resourceID] + return hs.getMultiAccessControlMetadata(c, prefix, ids)[resourceID] } // getMultiAccessControlMetadata returns the accesscontrol metadata associated with a given set of resources // Context must contain permissions in the given org (see LoadPermissionsMiddleware or AuthorizeInOrgMiddleware) func (hs *HTTPServer) getMultiAccessControlMetadata(c *contextmodel.ReqContext, - orgID int64, prefix string, resourceIDs map[string]bool) map[string]ac.Metadata { + prefix string, resourceIDs map[string]bool) map[string]ac.Metadata { if !c.QueryBool("accesscontrol") { return map[string]ac.Metadata{} } - if c.SignedInUser.Permissions == nil { + if len(c.SignedInUser.GetPermissions()) == 0 { return map[string]ac.Metadata{} } - permissions, ok := c.SignedInUser.Permissions[orgID] - if !ok { - return map[string]ac.Metadata{} - } - - return ac.GetResourcesMetadata(c.Req.Context(), permissions, prefix, resourceIDs) + return ac.GetResourcesMetadata(c.Req.Context(), c.SignedInUser.GetPermissions(), prefix, resourceIDs) } diff --git a/pkg/api/admin.go b/pkg/api/admin.go index ae3354ffa98..d0ccd476cc1 100644 --- a/pkg/api/admin.go +++ b/pkg/api/admin.go @@ -6,9 +6,9 @@ import ( "github.com/grafana/grafana/pkg/api/response" ac "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/auth/identity" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/services/stats" - "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/setting" ) @@ -67,7 +67,7 @@ func (hs *HTTPServer) AdminGetStats(c *contextmodel.ReqContext) response.Respons return response.JSON(http.StatusOK, adminStats) } -func (hs *HTTPServer) getAuthorizedSettings(ctx context.Context, user *user.SignedInUser, bag setting.SettingsBag) (setting.SettingsBag, error) { +func (hs *HTTPServer) getAuthorizedSettings(ctx context.Context, user identity.Requester, bag setting.SettingsBag) (setting.SettingsBag, error) { eval := func(scope string) (bool, error) { return hs.AccessControl.Evaluate(ctx, user, ac.EvalPermission(ac.ActionSettingsRead, scope)) } @@ -108,7 +108,7 @@ func (hs *HTTPServer) getAuthorizedSettings(ctx context.Context, user *user.Sign return authorizedBag, nil } -func (hs *HTTPServer) getAuthorizedVerboseSettings(ctx context.Context, user *user.SignedInUser, bag setting.VerboseSettingsBag) (setting.VerboseSettingsBag, error) { +func (hs *HTTPServer) getAuthorizedVerboseSettings(ctx context.Context, user identity.Requester, bag setting.VerboseSettingsBag) (setting.VerboseSettingsBag, error) { eval := func(scope string) (bool, error) { return hs.AccessControl.Evaluate(ctx, user, ac.EvalPermission(ac.ActionSettingsRead, scope)) } diff --git a/pkg/api/apikey.go b/pkg/api/apikey.go index ecd2b1e03a0..894078633b6 100644 --- a/pkg/api/apikey.go +++ b/pkg/api/apikey.go @@ -57,7 +57,7 @@ func (hs *HTTPServer) GetAPIKeys(c *contextmodel.ReqContext) response.Response { } } - metadata := hs.getMultiAccessControlMetadata(c, c.OrgID, "apikeys:id", ids) + metadata := hs.getMultiAccessControlMetadata(c, "apikeys:id", ids) if len(metadata) > 0 { for _, key := range result { key.AccessControl = metadata[strconv.FormatInt(key.ID, 10)] diff --git a/pkg/api/folder.go b/pkg/api/folder.go index 46fc794395b..95f876d144c 100644 --- a/pkg/api/folder.go +++ b/pkg/api/folder.go @@ -390,7 +390,7 @@ func (hs *HTTPServer) getFolderACMetadata(c *contextmodel.ReqContext, f *folder. folderIDs[p.UID] = true } - allMetadata := hs.getMultiAccessControlMetadata(c, c.OrgID, dashboards.ScopeFoldersPrefix, folderIDs) + allMetadata := hs.getMultiAccessControlMetadata(c, dashboards.ScopeFoldersPrefix, folderIDs) metadata := allMetadata[f.UID] // Flatten metadata - if any parent has a permission, the child folder inherits it diff --git a/pkg/api/org_users.go b/pkg/api/org_users.go index 20b1ba2d7f7..1069bd71dd9 100644 --- a/pkg/api/org_users.go +++ b/pkg/api/org_users.go @@ -310,7 +310,15 @@ func (hs *HTTPServer) searchOrgUsersHelper(c *contextmodel.ReqContext, query *or } // Get accesscontrol metadata and IPD labels for users in the target org - accessControlMetadata := hs.getMultiAccessControlMetadata(c, query.OrgID, "users:id:", userIDs) + accessControlMetadata := map[string]accesscontrol.Metadata{} + if c.QueryBool("accesscontrol") && c.SignedInUser.Permissions != nil { + // TODO https://github.com/grafana/grafana-authnz-team/issues/268 - user access control service for fetching permissions from another organization + permissions, ok := c.SignedInUser.Permissions[query.OrgID] + if ok { + accessControlMetadata = accesscontrol.GetResourcesMetadata(c.Req.Context(), permissions, "users:id:", userIDs) + } + } + for i := range filteredUsers { filteredUsers[i].AccessControl = accessControlMetadata[fmt.Sprint(filteredUsers[i].UserID)] if module, ok := modules[filteredUsers[i].UserID]; ok { diff --git a/pkg/api/plugins.go b/pkg/api/plugins.go index 0d60aaddf2b..1053157c2bb 100644 --- a/pkg/api/plugins.go +++ b/pkg/api/plugins.go @@ -121,8 +121,7 @@ func (hs *HTTPServer) GetPluginList(c *contextmodel.ReqContext) response.Respons } // Compute metadata - pluginsMetadata := hs.getMultiAccessControlMetadata(c, c.OrgID, - pluginaccesscontrol.ScopeProvider.GetResourceScope(""), filteredPluginIDs) + pluginsMetadata := hs.getMultiAccessControlMetadata(c, pluginaccesscontrol.ScopeProvider.GetResourceScope(""), filteredPluginIDs) // Prepare DTO result := make(dtos.PluginList, 0) diff --git a/pkg/api/team.go b/pkg/api/team.go index fbee7f8fb39..fb81673c84f 100644 --- a/pkg/api/team.go +++ b/pkg/api/team.go @@ -167,7 +167,7 @@ func (hs *HTTPServer) SearchTeams(c *contextmodel.ReqContext) response.Response teamIDs[strconv.FormatInt(team.ID, 10)] = true } - metadata := hs.getMultiAccessControlMetadata(c, c.SignedInUser.GetOrgID(), "teams:id:", teamIDs) + metadata := hs.getMultiAccessControlMetadata(c, "teams:id:", teamIDs) if len(metadata) > 0 { for _, team := range queryResult.Teams { team.AccessControl = metadata[strconv.FormatInt(team.ID, 10)] diff --git a/pkg/services/accesscontrol/accesscontrol.go b/pkg/services/accesscontrol/accesscontrol.go index 084beb0515c..323f58e52ce 100644 --- a/pkg/services/accesscontrol/accesscontrol.go +++ b/pkg/services/accesscontrol/accesscontrol.go @@ -64,7 +64,7 @@ type SearchOptions struct { } type TeamPermissionsService interface { - GetPermissions(ctx context.Context, user *user.SignedInUser, resourceID string) ([]ResourcePermission, error) + GetPermissions(ctx context.Context, user identity.Requester, resourceID string) ([]ResourcePermission, error) SetUserPermission(ctx context.Context, orgID int64, user User, resourceID, permission string) (*ResourcePermission, error) } @@ -86,7 +86,7 @@ type ServiceAccountPermissionsService interface { type PermissionsService interface { // GetPermissions returns all permissions for given resourceID - GetPermissions(ctx context.Context, user *user.SignedInUser, resourceID string) ([]ResourcePermission, error) + GetPermissions(ctx context.Context, user identity.Requester, resourceID string) ([]ResourcePermission, error) // SetUserPermission sets permission on resource for a user SetUserPermission(ctx context.Context, orgID int64, user User, resourceID, permission string) (*ResourcePermission, error) // SetTeamPermission sets permission on resource for a team @@ -151,13 +151,13 @@ var ReqSignedIn = func(c *contextmodel.ReqContext) bool { } var ReqGrafanaAdmin = func(c *contextmodel.ReqContext) bool { - return c.IsGrafanaAdmin + return c.SignedInUser.GetIsGrafanaAdmin() } // ReqHasRole generates a fallback to check whether the user has a role // ReqHasRole(org.RoleAdmin) will always return true for Grafana server admins, eg, a Grafana Admin / Viewer role combination func ReqHasRole(role org.RoleType) func(c *contextmodel.ReqContext) bool { - return func(c *contextmodel.ReqContext) bool { return c.HasRole(role) } + return func(c *contextmodel.ReqContext) bool { return c.SignedInUser.HasRole(role) } } func BuildPermissionsMap(permissions []Permission) map[string]bool { diff --git a/pkg/services/accesscontrol/actest/fake.go b/pkg/services/accesscontrol/actest/fake.go index e6540b59eb4..8bd019d1aa5 100644 --- a/pkg/services/accesscontrol/actest/fake.go +++ b/pkg/services/accesscontrol/actest/fake.go @@ -5,7 +5,6 @@ import ( "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/auth/identity" - "github.com/grafana/grafana/pkg/services/user" ) var _ accesscontrol.Service = new(FakeService) @@ -121,7 +120,7 @@ type FakePermissionsService struct { ExpectedMappedAction string } -func (f *FakePermissionsService) GetPermissions(ctx context.Context, user *user.SignedInUser, resourceID string) ([]accesscontrol.ResourcePermission, error) { +func (f *FakePermissionsService) GetPermissions(ctx context.Context, user identity.Requester, resourceID string) ([]accesscontrol.ResourcePermission, error) { return f.ExpectedPermissions, f.ExpectedErr } diff --git a/pkg/services/accesscontrol/api/api.go b/pkg/services/accesscontrol/api/api.go index 9c5f41b8fbf..0f64ab7553c 100644 --- a/pkg/services/accesscontrol/api/api.go +++ b/pkg/services/accesscontrol/api/api.go @@ -114,7 +114,8 @@ func (api *AccessControlAPI) searchUserPermissions(c *contextmodel.ReqContext) r return response.JSON(http.StatusBadRequest, "provide one of 'action' or 'actionPrefix'") } - permissions, err := api.Service.SearchUserPermissions(c.Req.Context(), c.OrgID, searchOptions) + permissions, err := api.Service.SearchUserPermissions(c.Req.Context(), + c.SignedInUser.GetOrgID(), searchOptions) if err != nil { response.Error(http.StatusInternalServerError, "could not search user permissions", err) } diff --git a/pkg/services/accesscontrol/middleware.go b/pkg/services/accesscontrol/middleware.go index e93a0ca5e82..34d2550b96f 100644 --- a/pkg/services/accesscontrol/middleware.go +++ b/pkg/services/accesscontrol/middleware.go @@ -15,6 +15,7 @@ import ( "github.com/grafana/grafana/pkg/middleware/cookies" "github.com/grafana/grafana/pkg/models/usertoken" + "github.com/grafana/grafana/pkg/services/auth/identity" "github.com/grafana/grafana/pkg/services/authn" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/services/org" @@ -30,7 +31,7 @@ func Middleware(ac AccessControl) func(Evaluator) web.Handler { if c.AllowAnonymous { forceLogin, _ := strconv.ParseBool(c.Req.URL.Query().Get("forceLogin")) // ignoring error, assuming false for non-true values is ok. orgID, err := strconv.ParseInt(c.Req.URL.Query().Get("orgId"), 10, 64) - if err == nil && orgID > 0 && orgID != c.OrgID { + if err == nil && orgID > 0 && orgID != c.SignedInUser.GetOrgID() { forceLogin = true } @@ -56,9 +57,9 @@ func Middleware(ac AccessControl) func(Evaluator) web.Handler { } } -func authorize(c *contextmodel.ReqContext, ac AccessControl, user *user.SignedInUser, evaluator Evaluator) { +func authorize(c *contextmodel.ReqContext, ac AccessControl, user identity.Requester, evaluator Evaluator) { injected, err := evaluator.MutateScopes(c.Req.Context(), scopeInjector(scopeParams{ - OrgID: c.OrgID, + OrgID: user.GetOrgID(), URLParams: web.Params(c.Req), })) if err != nil { @@ -78,9 +79,11 @@ func deny(c *contextmodel.ReqContext, evaluator Evaluator, err error) { if err != nil { c.Logger.Error("Error from access control system", "error", err, "accessErrorID", id) } else { + namespace, identifier := c.SignedInUser.GetNamespacedID() c.Logger.Info( "Access denied", - "userID", c.UserID, + "namespace", namespace, + "userID", identifier, "accessErrorID", id, "permissions", evaluator.GoString(), ) diff --git a/pkg/services/accesscontrol/mock/service_mock.go b/pkg/services/accesscontrol/mock/service_mock.go index 0701bc1218c..618b0cf14ed 100644 --- a/pkg/services/accesscontrol/mock/service_mock.go +++ b/pkg/services/accesscontrol/mock/service_mock.go @@ -6,7 +6,7 @@ import ( "github.com/stretchr/testify/mock" "github.com/grafana/grafana/pkg/services/accesscontrol" - "github.com/grafana/grafana/pkg/services/user" + "github.com/grafana/grafana/pkg/services/auth/identity" ) var _ accesscontrol.PermissionsService = new(MockPermissionsService) @@ -19,7 +19,7 @@ type MockPermissionsService struct { mock.Mock } -func (m *MockPermissionsService) GetPermissions(ctx context.Context, user *user.SignedInUser, resourceID string) ([]accesscontrol.ResourcePermission, error) { +func (m *MockPermissionsService) GetPermissions(ctx context.Context, user identity.Requester, resourceID string) ([]accesscontrol.ResourcePermission, error) { mockedArgs := m.Called(ctx, user, resourceID) return mockedArgs.Get(0).([]accesscontrol.ResourcePermission), mockedArgs.Error(1) } diff --git a/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go b/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go index 4da58168c98..4c13bbae6df 100644 --- a/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go +++ b/pkg/services/accesscontrol/ossaccesscontrol/permissions_services.go @@ -10,6 +10,7 @@ import ( "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol/resourcepermissions" + "github.com/grafana/grafana/pkg/services/auth/identity" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/folder" @@ -253,7 +254,7 @@ var _ accesscontrol.DatasourcePermissionsService = new(DatasourcePermissionsServ type DatasourcePermissionsService struct{} -func (e DatasourcePermissionsService) GetPermissions(ctx context.Context, user *user.SignedInUser, resourceID string) ([]accesscontrol.ResourcePermission, error) { +func (e DatasourcePermissionsService) GetPermissions(ctx context.Context, user identity.Requester, resourceID string) ([]accesscontrol.ResourcePermission, error) { return nil, nil } diff --git a/pkg/services/accesscontrol/resourcepermissions/api.go b/pkg/services/accesscontrol/resourcepermissions/api.go index 3c2b48be8e1..a92e1f1475a 100644 --- a/pkg/services/accesscontrol/resourcepermissions/api.go +++ b/pkg/services/accesscontrol/resourcepermissions/api.go @@ -155,7 +155,7 @@ func (a *api) setUserPermission(c *contextmodel.ReqContext) response.Response { return response.Error(http.StatusBadRequest, "bad request data", err) } - _, err = a.service.SetUserPermission(c.Req.Context(), c.OrgID, accesscontrol.User{ID: userID}, resourceID, cmd.Permission) + _, err = a.service.SetUserPermission(c.Req.Context(), c.SignedInUser.GetOrgID(), accesscontrol.User{ID: userID}, resourceID, cmd.Permission) if err != nil { return response.Error(http.StatusBadRequest, "failed to set user permission", err) } @@ -175,7 +175,7 @@ func (a *api) setTeamPermission(c *contextmodel.ReqContext) response.Response { return response.Error(http.StatusBadRequest, "bad request data", err) } - _, err = a.service.SetTeamPermission(c.Req.Context(), c.OrgID, teamID, resourceID, cmd.Permission) + _, err = a.service.SetTeamPermission(c.Req.Context(), c.SignedInUser.GetOrgID(), teamID, resourceID, cmd.Permission) if err != nil { return response.Error(http.StatusBadRequest, "failed to set team permission", err) } @@ -192,7 +192,7 @@ func (a *api) setBuiltinRolePermission(c *contextmodel.ReqContext) response.Resp return response.Error(http.StatusBadRequest, "bad request data", err) } - _, err := a.service.SetBuiltInRolePermission(c.Req.Context(), c.OrgID, builtInRole, resourceID, cmd.Permission) + _, err := a.service.SetBuiltInRolePermission(c.Req.Context(), c.SignedInUser.GetOrgID(), builtInRole, resourceID, cmd.Permission) if err != nil { return response.Error(http.StatusBadRequest, "failed to set role permission", err) } @@ -208,7 +208,7 @@ func (a *api) setPermissions(c *contextmodel.ReqContext) response.Response { return response.Error(http.StatusBadRequest, "bad request data", err) } - _, err := a.service.SetPermissions(c.Req.Context(), c.OrgID, resourceID, cmd.Permissions...) + _, err := a.service.SetPermissions(c.Req.Context(), c.SignedInUser.GetOrgID(), resourceID, cmd.Permissions...) if err != nil { return response.Error(http.StatusBadRequest, "failed to set permissions", err) } diff --git a/pkg/services/accesscontrol/resourcepermissions/models.go b/pkg/services/accesscontrol/resourcepermissions/models.go index f3b5425f0e9..35b6f584e20 100644 --- a/pkg/services/accesscontrol/resourcepermissions/models.go +++ b/pkg/services/accesscontrol/resourcepermissions/models.go @@ -2,7 +2,7 @@ package resourcepermissions import ( "github.com/grafana/grafana/pkg/services/accesscontrol" - "github.com/grafana/grafana/pkg/services/user" + "github.com/grafana/grafana/pkg/services/auth/identity" ) type SetResourcePermissionCommand struct { @@ -29,5 +29,5 @@ type GetResourcePermissionsQuery struct { OnlyManaged bool InheritedScopes []string EnforceAccessControl bool - User *user.SignedInUser + User identity.Requester } diff --git a/pkg/services/accesscontrol/resourcepermissions/service.go b/pkg/services/accesscontrol/resourcepermissions/service.go index 8f80c461450..235e2240dd0 100644 --- a/pkg/services/accesscontrol/resourcepermissions/service.go +++ b/pkg/services/accesscontrol/resourcepermissions/service.go @@ -8,6 +8,7 @@ import ( "github.com/grafana/grafana/pkg/api/routing" "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/auth/identity" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/licensing" "github.com/grafana/grafana/pkg/services/org" @@ -115,17 +116,17 @@ type Service struct { userService user.Service } -func (s *Service) GetPermissions(ctx context.Context, user *user.SignedInUser, resourceID string) ([]accesscontrol.ResourcePermission, error) { +func (s *Service) GetPermissions(ctx context.Context, user identity.Requester, resourceID string) ([]accesscontrol.ResourcePermission, error) { var inheritedScopes []string if s.options.InheritedScopesSolver != nil { var err error - inheritedScopes, err = s.options.InheritedScopesSolver(ctx, user.OrgID, resourceID) + inheritedScopes, err = s.options.InheritedScopesSolver(ctx, user.GetOrgID(), resourceID) if err != nil { return nil, err } } - return s.store.GetResourcePermissions(ctx, user.OrgID, GetResourcePermissionsQuery{ + return s.store.GetResourcePermissions(ctx, user.GetOrgID(), GetResourcePermissionsQuery{ User: user, Actions: s.actions, Resource: s.options.Resource, diff --git a/pkg/services/auth/identity/requester.go b/pkg/services/auth/identity/requester.go index 066d4936a1e..6b0ba901024 100644 --- a/pkg/services/auth/identity/requester.go +++ b/pkg/services/auth/identity/requester.go @@ -53,6 +53,8 @@ type Requester interface { // Legacy + // HasRole returns true if the active entity has the given role in the active organization. + HasRole(role roletype.RoleType) bool // GetCacheKey returns a unique key for the entity. // Add an extra prefix to avoid collisions with other caches GetCacheKey() (string, error)