From 6a9c4579ab536b14a4db4ee0b58ff4b4e1a7faa6 Mon Sep 17 00:00:00 2001 From: Alexander Zobnin Date: Wed, 11 Sep 2024 14:41:08 +0200 Subject: [PATCH] refactor: move Check and ListObjects to AccessControl implementation --- pkg/services/accesscontrol/accesscontrol.go | 6 +++--- pkg/services/accesscontrol/acimpl/accesscontrol.go | 8 ++++++++ pkg/services/accesscontrol/acimpl/service.go | 9 --------- pkg/services/accesscontrol/actest/fake.go | 10 ++++++++++ pkg/services/dashboards/service/dashboard_service.go | 4 +--- pkg/services/dashboards/service/zanzana.go | 4 ++-- pkg/services/ngalert/accesscontrol/testing.go | 10 ++++++++++ pkg/services/ngalert/api/testing.go | 11 ++++++++++- 8 files changed, 44 insertions(+), 18 deletions(-) diff --git a/pkg/services/accesscontrol/accesscontrol.go b/pkg/services/accesscontrol/accesscontrol.go index f00b74fbc8c..8c9145964e0 100644 --- a/pkg/services/accesscontrol/accesscontrol.go +++ b/pkg/services/accesscontrol/accesscontrol.go @@ -28,11 +28,13 @@ type AccessControl interface { // RegisterScopeAttributeResolver allows the caller to register a scope resolver for a // specific scope prefix (ex: datasources:name:) RegisterScopeAttributeResolver(prefix string, resolver ScopeAttributeResolver) + + Check(ctx context.Context, in *openfgav1.CheckRequest) (*openfgav1.CheckResponse, error) + ListObjects(ctx context.Context, in *openfgav1.ListObjectsRequest) (*openfgav1.ListObjectsResponse, error) } type Service interface { registry.ProvidesUsageStats - ZanzanaChecker // GetRoleByName returns a role by name GetRoleByName(ctx context.Context, orgID int64, roleName string) (*RoleDTO, error) // GetUserPermissions returns user permissions with only action and scope fields set. @@ -61,8 +63,6 @@ type Service interface { } type ZanzanaChecker interface { - Check(ctx context.Context, in *openfgav1.CheckRequest) (*openfgav1.CheckResponse, error) - ListObjects(ctx context.Context, in *openfgav1.ListObjectsRequest) (*openfgav1.ListObjectsResponse, error) } //go:generate mockery --name Store --structname MockStore --outpkg actest --filename store_mock.go --output ./actest/ diff --git a/pkg/services/accesscontrol/acimpl/accesscontrol.go b/pkg/services/accesscontrol/acimpl/accesscontrol.go index 4a6b8a99e3e..107ca76e0a6 100644 --- a/pkg/services/accesscontrol/acimpl/accesscontrol.go +++ b/pkg/services/accesscontrol/acimpl/accesscontrol.go @@ -211,3 +211,11 @@ func (a *AccessControl) debug(ctx context.Context, ident identity.Requester, msg a.log.FromContext(ctx).Debug(msg, "id", ident.GetID(), "orgID", ident.GetOrgID(), "permissions", eval.GoString()) } + +func (a *AccessControl) Check(ctx context.Context, in *openfgav1.CheckRequest) (*openfgav1.CheckResponse, error) { + return a.zclient.Check(ctx, in) +} + +func (a *AccessControl) ListObjects(ctx context.Context, in *openfgav1.ListObjectsRequest) (*openfgav1.ListObjectsResponse, error) { + return a.zclient.ListObjects(ctx, in) +} diff --git a/pkg/services/accesscontrol/acimpl/service.go b/pkg/services/accesscontrol/acimpl/service.go index b9e1d2a1fc5..7a37f5b6a76 100644 --- a/pkg/services/accesscontrol/acimpl/service.go +++ b/pkg/services/accesscontrol/acimpl/service.go @@ -9,7 +9,6 @@ import ( "time" "github.com/grafana/authlib/claims" - openfgav1 "github.com/openfga/api/proto/openfga/v1" "github.com/prometheus/client_golang/prometheus" "go.opentelemetry.io/otel/attribute" @@ -792,11 +791,3 @@ func (s *Service) GetRoleByName(ctx context.Context, orgID int64, roleName strin }) return role, err } - -func (s *Service) Check(ctx context.Context, in *openfgav1.CheckRequest) (*openfgav1.CheckResponse, error) { - return s.zclient.Check(ctx, in) -} - -func (s *Service) ListObjects(ctx context.Context, in *openfgav1.ListObjectsRequest) (*openfgav1.ListObjectsResponse, error) { - return s.zclient.ListObjects(ctx, in) -} diff --git a/pkg/services/accesscontrol/actest/fake.go b/pkg/services/accesscontrol/actest/fake.go index d916f553f32..3d9638ff9cd 100644 --- a/pkg/services/accesscontrol/actest/fake.go +++ b/pkg/services/accesscontrol/actest/fake.go @@ -3,6 +3,8 @@ package actest import ( "context" + openfgav1 "github.com/openfga/api/proto/openfga/v1" + "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/services/accesscontrol" ) @@ -75,6 +77,14 @@ func (f FakeAccessControl) Evaluate(ctx context.Context, user identity.Requester func (f FakeAccessControl) RegisterScopeAttributeResolver(prefix string, resolver accesscontrol.ScopeAttributeResolver) { } +func (f FakeAccessControl) Check(ctx context.Context, in *openfgav1.CheckRequest) (*openfgav1.CheckResponse, error) { + return nil, nil +} + +func (f FakeAccessControl) ListObjects(ctx context.Context, in *openfgav1.ListObjectsRequest) (*openfgav1.ListObjectsResponse, error) { + return nil, nil +} + type FakeStore struct { ExpectedUserPermissions []accesscontrol.Permission ExpectedBasicRolesPermissions []accesscontrol.Permission diff --git a/pkg/services/dashboards/service/dashboard_service.go b/pkg/services/dashboards/service/dashboard_service.go index a466717ffcb..eeeba8edd21 100644 --- a/pkg/services/dashboards/service/dashboard_service.go +++ b/pkg/services/dashboards/service/dashboard_service.go @@ -58,7 +58,6 @@ type DashboardServiceImpl struct { folderPermissions accesscontrol.FolderPermissionsService dashboardPermissions accesscontrol.DashboardPermissionsService ac accesscontrol.AccessControl - acService accesscontrol.Service metrics *dashboardsMetrics } @@ -67,7 +66,7 @@ func ProvideDashboardServiceImpl( cfg *setting.Cfg, dashboardStore dashboards.Store, folderStore folder.FolderStore, features featuremgmt.FeatureToggles, folderPermissionsService accesscontrol.FolderPermissionsService, dashboardPermissionsService accesscontrol.DashboardPermissionsService, ac accesscontrol.AccessControl, - folderSvc folder.Service, r prometheus.Registerer, acService accesscontrol.Service, + folderSvc folder.Service, r prometheus.Registerer, ) (*DashboardServiceImpl, error) { dashSvc := &DashboardServiceImpl{ cfg: cfg, @@ -77,7 +76,6 @@ func ProvideDashboardServiceImpl( folderPermissions: folderPermissionsService, dashboardPermissions: dashboardPermissionsService, ac: ac, - acService: acService, folderStore: folderStore, folderService: folderSvc, metrics: newDashboardsMetrics(r), diff --git a/pkg/services/dashboards/service/zanzana.go b/pkg/services/dashboards/service/zanzana.go index 736a486e5f2..a6228e326e2 100644 --- a/pkg/services/dashboards/service/zanzana.go +++ b/pkg/services/dashboards/service/zanzana.go @@ -170,7 +170,7 @@ func (dr *DashboardServiceImpl) findDashboardsZanzanaList(ctx context.Context, q } func (dr *DashboardServiceImpl) listResources(ctx context.Context, query *dashboards.FindPersistedDashboardsQuery, resourceType string) ([]string, error) { - res, err := dr.acService.ListObjects(ctx, &openfgav1.ListObjectsRequest{ + res, err := dr.ac.ListObjects(ctx, &openfgav1.ListObjectsRequest{ User: query.SignedInUser.GetUID(), Type: resourceType, Relation: "read", @@ -240,7 +240,7 @@ func (dr *DashboardServiceImpl) checkDashboards(ctx context.Context, query *dash Object: object, } - checkRes, err := dr.acService.Check(ctx, &openfgav1.CheckRequest{ + checkRes, err := dr.ac.Check(ctx, &openfgav1.CheckRequest{ TupleKey: key, }) if err != nil { diff --git a/pkg/services/ngalert/accesscontrol/testing.go b/pkg/services/ngalert/accesscontrol/testing.go index 38909578d30..f0bfc6a9d1c 100644 --- a/pkg/services/ngalert/accesscontrol/testing.go +++ b/pkg/services/ngalert/accesscontrol/testing.go @@ -3,6 +3,8 @@ package accesscontrol import ( "context" + openfgav1 "github.com/openfga/api/proto/openfga/v1" + "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/services/accesscontrol" ) @@ -36,4 +38,12 @@ func (a *recordingAccessControlFake) IsDisabled() bool { return a.Disabled } +func (a *recordingAccessControlFake) Check(ctx context.Context, in *openfgav1.CheckRequest) (*openfgav1.CheckResponse, error) { + return nil, nil +} + +func (a *recordingAccessControlFake) ListObjects(ctx context.Context, in *openfgav1.ListObjectsRequest) (*openfgav1.ListObjectsResponse, error) { + return nil, nil +} + var _ accesscontrol.AccessControl = &recordingAccessControlFake{} diff --git a/pkg/services/ngalert/api/testing.go b/pkg/services/ngalert/api/testing.go index 8b85525de78..b85fcf10de6 100644 --- a/pkg/services/ngalert/api/testing.go +++ b/pkg/services/ngalert/api/testing.go @@ -7,8 +7,9 @@ import ( "testing" "time" - "github.com/grafana/grafana-plugin-sdk-go/data" + openfgav1 "github.com/openfga/api/proto/openfga/v1" + "github.com/grafana/grafana-plugin-sdk-go/data" "github.com/grafana/grafana/pkg/apimachinery/identity" ac "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/ngalert/eval" @@ -133,6 +134,14 @@ func (a *recordingAccessControlFake) IsDisabled() bool { return a.Disabled } +func (a *recordingAccessControlFake) Check(ctx context.Context, in *openfgav1.CheckRequest) (*openfgav1.CheckResponse, error) { + return nil, nil +} + +func (a *recordingAccessControlFake) ListObjects(ctx context.Context, in *openfgav1.ListObjectsRequest) (*openfgav1.ListObjectsResponse, error) { + return nil, nil +} + var _ ac.AccessControl = &recordingAccessControlFake{} type fakeRuleAccessControlService struct {