From 2788817107df8bb7021b7b7bcd195cfa451f1da7 Mon Sep 17 00:00:00 2001 From: Gabriel MABILLE Date: Fri, 25 Oct 2024 14:57:39 +0200 Subject: [PATCH] AuthZ: Implement Check (#95162) * AuthZ: Implement Check --------- Co-authored-by: Alexander Zobnin --- pkg/services/authz/client.go | 9 +- pkg/services/authz/mappers/rbac_mapper.go | 66 +++++++ pkg/services/authz/server.go | 153 +++++++++++++-- pkg/services/authz/server_test.go | 218 ++++++++++++++++++++++ 4 files changed, 429 insertions(+), 17 deletions(-) create mode 100644 pkg/services/authz/mappers/rbac_mapper.go create mode 100644 pkg/services/authz/server_test.go diff --git a/pkg/services/authz/client.go b/pkg/services/authz/client.go index df0e3d6ae44..d0074e9e7d2 100644 --- a/pkg/services/authz/client.go +++ b/pkg/services/authz/client.go @@ -14,7 +14,9 @@ import ( "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/authn" "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/grpcserver" "github.com/grafana/grafana/pkg/setting" ) @@ -28,8 +30,9 @@ type Client interface { // ProvideAuthZClient provides an AuthZ client and creates the AuthZ service. func ProvideAuthZClient( - cfg *setting.Cfg, features featuremgmt.FeatureToggles, acSvc accesscontrol.Service, - grpcServer grpcserver.Provider, tracer tracing.Tracer, + cfg *setting.Cfg, features featuremgmt.FeatureToggles, ac accesscontrol.AccessControl, + authnSvc authn.Service, folderSvc folder.Service, grpcServer grpcserver.Provider, + tracer tracing.Tracer, ) (Client, error) { if !features.IsEnabledGlobally(featuremgmt.FlagAuthZGRPCServer) { return nil, nil @@ -43,7 +46,7 @@ func ProvideAuthZClient( var client Client // Register the server - server, err := newLegacyServer(acSvc, features, grpcServer, tracer, authCfg) + server, err := newLegacyServer(authnSvc, ac, folderSvc, features, grpcServer, tracer, authCfg) if err != nil { return nil, err } diff --git a/pkg/services/authz/mappers/rbac_mapper.go b/pkg/services/authz/mappers/rbac_mapper.go new file mode 100644 index 00000000000..e12e5d0ebcc --- /dev/null +++ b/pkg/services/authz/mappers/rbac_mapper.go @@ -0,0 +1,66 @@ +package mappers + +type VerbToAction map[string]string // e.g. "get" -> "read" +type ResourceVerbToAction map[string]VerbToAction // e.g. "dashboards" -> VerbToAction +type GroupResourceVerbToAction map[string]ResourceVerbToAction // e.g. "dashboards.grafana.app" -> ResourceVerbToAction + +type ResourceToAttribute map[string]string // e.g. "dashboards" -> "uid" +type GroupResourceToAttribute map[string]ResourceToAttribute // e.g. "dashboards.grafana.app" -> ResourceToAttribute + +type K8sRbacMapper struct { + DefaultActions VerbToAction + DefaultAttribute string + Actions GroupResourceVerbToAction + Attributes GroupResourceToAttribute +} + +func NewK8sRbacMapper() *K8sRbacMapper { + return &K8sRbacMapper{ + DefaultActions: VerbToAction{ + "get": "read", + "list": "read", + "watch": "read", + "create": "create", + "update": "write", + "patch": "write", + "delete": "delete", + "deletecollection": "delete", + }, + DefaultAttribute: "uid", + Actions: GroupResourceVerbToAction{ + "dashboards.grafana.app": ResourceVerbToAction{"dashboards": VerbToAction{}}, + "folders.grafana.app": ResourceVerbToAction{"folders": VerbToAction{}}, + }, + } +} + +func (m *K8sRbacMapper) Action(group, resource, verb string) (string, bool) { + if resourceActions, ok := m.Actions[group]; ok { + if actions, ok := resourceActions[resource]; ok { + if action, ok := actions[verb]; ok { + // If the action is explicitly set empty + // it means that the action is not allowed + if action == "" { + return "", false + } + return action, true + } + if defaultAction, ok := m.DefaultActions[verb]; ok { + return resource + ":" + defaultAction, true + } + } + } + return "", false +} + +func (m *K8sRbacMapper) Scope(group, resource, name string) (string, bool) { + if resourceAttributes, ok := m.Attributes[group]; ok { + if attribute, ok := resourceAttributes[resource]; ok { + return resource + ":" + attribute + ":" + name, true + } + } + if m.DefaultAttribute != "" { + return resource + ":" + m.DefaultAttribute + ":" + name, true + } + return "", false +} diff --git a/pkg/services/authz/server.go b/pkg/services/authz/server.go index d345021f34e..3ac60ebc5e0 100644 --- a/pkg/services/authz/server.go +++ b/pkg/services/authz/server.go @@ -2,35 +2,44 @@ package authz import ( "context" - "errors" + "fmt" - "github.com/grafana/authlib/authz" + authzlib "github.com/grafana/authlib/authz" authzv1 "github.com/grafana/authlib/authz/proto/v1" + "github.com/grafana/authlib/claims" grpc_auth "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/auth" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/authn" + "github.com/grafana/grafana/pkg/services/authz/mappers" "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/services/folder" "github.com/grafana/grafana/pkg/services/grpcserver" ) var _ authzv1.AuthzServiceServer = (*legacyServer)(nil) var _ grpc_auth.ServiceAuthFuncOverride = (*legacyServer)(nil) -var _ authz.ServiceAuthorizeFuncOverride = (*legacyServer)(nil) +var _ authzlib.ServiceAuthorizeFuncOverride = (*legacyServer)(nil) func newLegacyServer( - acSvc accesscontrol.Service, features featuremgmt.FeatureToggles, - grpcServer grpcserver.Provider, tracer tracing.Tracer, cfg *Cfg, + authnSvc authn.Service, ac accesscontrol.AccessControl, folderSvc folder.Service, + features featuremgmt.FeatureToggles, grpcServer grpcserver.Provider, tracer tracing.Tracer, cfg *Cfg, ) (*legacyServer, error) { if !features.IsEnabledGlobally(featuremgmt.FlagAuthZGRPCServer) { return nil, nil } l := &legacyServer{ - acSvc: acSvc, - logger: log.New("authz-grpc-server"), - tracer: tracer, + ac: ac.WithoutResolvers(), // We want to skip the folder tree resolution as it's done by the service + authnSvc: authnSvc, + folderSvc: folderSvc, + logger: log.New("authz-grpc-server"), + tracer: tracer, + mapper: mappers.NewK8sRbacMapper(), } if cfg.listen { @@ -47,9 +56,12 @@ func newLegacyServer( type legacyServer struct { authzv1.UnimplementedAuthzServiceServer - acSvc accesscontrol.Service - logger log.Logger - tracer tracing.Tracer + ac accesscontrol.AccessControl + authnSvc authn.Service + folderSvc folder.Service + logger log.Logger + tracer tracing.Tracer + mapper *mappers.K8sRbacMapper } // AuthFuncOverride is a function that allows to override the default auth function. @@ -70,7 +82,120 @@ func (l *legacyServer) AuthorizeFuncOverride(ctx context.Context) error { return nil } -func (l *legacyServer) Check(context.Context, *authzv1.CheckRequest) (*authzv1.CheckResponse, error) { - // FIXME: implement for legacy access control - return nil, errors.New("unimplemented") +func wrapErr(err error) error { + return status.Error(codes.Internal, fmt.Errorf("authz check failed: %w", err).Error()) +} + +func validateRequest(req *authzv1.CheckRequest) error { + if req.GetGroup() == "" { + return status.Error(codes.InvalidArgument, "group is required") + } + if req.GetResource() == "" { + return status.Error(codes.InvalidArgument, "resource is required") + } + if req.GetVerb() == "" { + return status.Error(codes.InvalidArgument, "verb is required") + } + if req.GetSubject() == "" { + return status.Error(codes.InvalidArgument, "subject is required") + } + return nil +} + +func (l *legacyServer) Check(ctx context.Context, req *authzv1.CheckRequest) (*authzv1.CheckResponse, error) { + ctx, span := l.tracer.Start(ctx, "authz.Check") + defer span.End() + ctxLogger := l.logger.FromContext(ctx) + + deny := &authzv1.CheckResponse{Allowed: false} + if err := validateRequest(req); err != nil { + ctxLogger.Error("invalid request", "error", err) + return deny, err + } + + namespace := req.GetNamespace() + info, err := claims.ParseNamespace(namespace) + // We have to check the stackID as ParseNamespace returns orgID 1 for stacks namespaces + if err != nil || info.OrgID == 0 || info.StackID != 0 { + ctxLogger.Error("invalid namespace", "namespace", namespace, "error", err) + return deny, status.Error(codes.InvalidArgument, "invalid namespace: "+namespace) + } + + // Get the RBAC action associated with the request + action, ok := l.mapper.Action(req.Group, req.Resource, req.Verb) + if !ok { + ctxLogger.Error("could not find associated rbac action", "group", req.Group, "resource", req.Resource, "verb", req.Verb) + return deny, wrapErr(fmt.Errorf("could not find associated rbac action")) + } + + // Get the user from the subject + user, err := l.authnSvc.ResolveIdentity(ctx, info.OrgID, req.Subject) + if err != nil { + // TODO: should probably distinguish between not found and other errors + ctxLogger.Error("could not resolve identity", "subject", req.Subject, "orgId", info.OrgID) + return deny, wrapErr(fmt.Errorf("could not resolve identity")) + } + + // Check if the user has the action solely + if req.Name == "" && req.Folder == "" { + ev := accesscontrol.EvalPermission(action) + hasAccess, err := l.ac.Evaluate(ctx, user, ev) + if err != nil { + ctxLogger.Error("could not evaluate permission", "subject", req.Subject, "orgId", info.OrgID, "action", action) + return deny, wrapErr(fmt.Errorf("could not evaluate permission")) + } + + return &authzv1.CheckResponse{Allowed: hasAccess}, nil + } + + scopes := make([]string, 0, 1) + // If a parent is specified: Check if the user has access to any of the parent folders + if req.Folder != "" { + scopes, err = l.getFolderTree(ctx, info.OrgID, req.Folder) + if err != nil { + ctxLogger.Error("could not get folder tree", "folder", req.Folder, "orgId", info.OrgID, "error", err) + return nil, wrapErr(err) + } + } + // If a resource is specified: Check if the user has access to the requested resource + if req.Name != "" { + scope, ok := l.mapper.Scope(req.Group, req.Resource, req.Name) + if !ok { + ctxLogger.Error("could not get attribute for resource", "resource", req.Resource) + return deny, wrapErr(fmt.Errorf("could not get attribute for resource")) + } + scopes = append(scopes, scope) + } + + ev := accesscontrol.EvalPermission(action, scopes...) + allowed, err := l.ac.Evaluate(ctx, user, ev) + if err != nil { + ctxLogger.Error("could not evaluate permission", + "subject", req.Subject, + "orgId", info.OrgID, + "action", action, + "folder", req.Folder, + "scopes_count", len(scopes)) + return deny, fmt.Errorf("could not evaluate permission") + } + + return &authzv1.CheckResponse{Allowed: allowed}, nil +} + +func (l *legacyServer) getFolderTree(ctx context.Context, orgID int64, parent string) ([]string, error) { + ctx, span := l.tracer.Start(ctx, "authz.getFolderTree") + defer span.End() + + scopes := make([]string, 0, 6) + // Get the folder tree + folders, err := l.folderSvc.GetParents(ctx, folder.GetParentsQuery{UID: parent, OrgID: orgID}) + if err != nil { + return nil, err + } + scopes = append(scopes, "folders:uid:"+parent) + for _, f := range folders { + scopes = append(scopes, "folders:uid:"+f.UID) + } + + return scopes, nil } diff --git a/pkg/services/authz/server_test.go b/pkg/services/authz/server_test.go new file mode 100644 index 00000000000..4f5514b1aa2 --- /dev/null +++ b/pkg/services/authz/server_test.go @@ -0,0 +1,218 @@ +package authz + +import ( + "context" + "testing" + + authzv1 "github.com/grafana/authlib/authz/proto/v1" + "github.com/grafana/authlib/claims" + "github.com/stretchr/testify/require" + + "github.com/grafana/grafana/pkg/apimachinery/identity" + "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/infra/tracing" + "github.com/grafana/grafana/pkg/services/accesscontrol/acimpl" + "github.com/grafana/grafana/pkg/services/authn" + "github.com/grafana/grafana/pkg/services/authn/authntest" + "github.com/grafana/grafana/pkg/services/authz/mappers" + "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/services/folder" + "github.com/grafana/grafana/pkg/services/folder/foldertest" +) + +var folderTree = []*folder.Folder{ + { + ID: 1, + UID: "top", + Title: "top", + }, + { + ID: 2, + UID: "sub", + Title: "sub", + ParentUID: "top", + }, + { + ID: 3, + UID: "sub2", + Title: "sub2", + ParentUID: "sub", + }, + { + ID: 4, + UID: "sub3", + Title: "sub3", + ParentUID: "sub2", + }, +} + +func Test_legacyServer_Check(t *testing.T) { + tests := []struct { + name string + req *authzv1.CheckRequest + parents []*folder.Folder + userPerms map[string][]string + wantAllowed bool + wantErr bool + }{ + { + name: "should not allow access to a dashboard without read permission", + req: &authzv1.CheckRequest{ + Subject: "user:1", + Verb: "get", + Group: "dashboards.grafana.app", + Resource: "dashboards", + Name: "dash1", + Namespace: "org-2", + }, + userPerms: map[string][]string{}, + wantAllowed: false, + wantErr: false, + }, + { + name: "should allow access to a dashboard with read permission", + req: &authzv1.CheckRequest{ + Subject: "user:1", + Verb: "get", + Group: "dashboards.grafana.app", + Resource: "dashboards", + Name: "dash1", + Namespace: "org-2", + }, + userPerms: map[string][]string{"dashboards:read": {"dashboards:uid:dash1"}}, + wantAllowed: true, + wantErr: false, + }, + { + name: "should allow access to a dashboard through read permission on a parent folder", + req: &authzv1.CheckRequest{ + Subject: "user:1", + Verb: "get", + Group: "dashboards.grafana.app", + Resource: "dashboards", + Name: "dash1", + Namespace: "org-2", + Folder: "sub4", + }, + parents: folderTree, + userPerms: map[string][]string{ + "dashboards:read": {"folders:uid:sub"}, + }, + wantAllowed: true, + wantErr: false, + }, + { + name: "should check action only", + req: &authzv1.CheckRequest{ + Subject: "user:1", + Verb: "get", + Group: "dashboards.grafana.app", + Resource: "dashboards", + Namespace: "org-2", + }, + userPerms: map[string][]string{"dashboards:read": {"dashboards:uid:dash1"}}, + wantAllowed: true, + wantErr: false, + }, + // Input validation + { + name: "should return error when group is not set", + req: &authzv1.CheckRequest{ + Subject: "user:1", + Verb: "get", + Resource: "dashboards", + Name: "dash1", + Namespace: "org-2", + }, + wantErr: true, + }, + { + name: "should return error when resource is not set", + req: &authzv1.CheckRequest{ + Subject: "user:1", + Verb: "get", + Group: "dashboards.grafana.app", + Name: "dash1", + Namespace: "org-2", + }, + wantErr: true, + }, + { + name: "should return error when verb is not set", + req: &authzv1.CheckRequest{ + Subject: "user:1", + Group: "dashboards.grafana.app", + Resource: "dashboards", + Name: "dash1", + Namespace: "org-2", + }, + wantErr: true, + }, + { + name: "should return error when subject is not set", + req: &authzv1.CheckRequest{ + Verb: "get", + Group: "dashboards.grafana.app", + Resource: "dashboards", + Name: "dash1", + Namespace: "org-2", + }, + wantErr: true, + }, + { + name: "should return error when namespace is incorrect", + req: &authzv1.CheckRequest{ + Subject: "user:1", + Verb: "get", + Group: "dashboards.grafana.app", + Resource: "dashboards", + Name: "dash1", + Namespace: "stacks-2", + }, + wantErr: true, + }, + { + name: "should return error when action is not found", + req: &authzv1.CheckRequest{ + Subject: "user:1", + Verb: "get", + Group: "unknown.grafana.app", + Resource: "unknown", + Name: "unknown", + Namespace: "org-2", + }, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := featuremgmt.WithFeatures() + l := &legacyServer{ + ac: acimpl.ProvideAccessControl(f, nil), + authnSvc: &authntest.FakeService{ + ExpectedIdentity: &authn.Identity{ + ID: "user:1", + UID: "1", + Type: claims.TypeUser, + OrgID: 2, + OrgRoles: map[int64]identity.RoleType{2: identity.RoleNone}, + Login: "user1", + Permissions: map[int64]map[string][]string{2: tt.userPerms}, + }, + }, + folderSvc: &foldertest.FakeService{ExpectedFolders: tt.parents}, + logger: log.New("authz-grpc-server.test"), + tracer: tracing.InitializeTracerForTest(), + mapper: mappers.NewK8sRbacMapper(), + } + got, err := l.Check(context.Background(), tt.req) + if tt.wantErr { + require.Error(t, err) + return + } + require.NoError(t, err) + require.NotNil(t, got) + require.Equal(t, tt.wantAllowed, got.Allowed) + }) + } +}