From 4d699d4810a60c6b105633a3e33711ad4a927574 Mon Sep 17 00:00:00 2001 From: Gabriel MABILLE Date: Mon, 13 Jan 2025 16:03:14 +0100 Subject: [PATCH] AuthZ: Use M3 AuthZ Service (#98621) * AuthZ: Use M3 AuthZ Service Co-authored-by: ievaVasiljeva * Fix oss * fake auth info --------- Co-authored-by: ievaVasiljeva --- pkg/services/authz/client.go | 34 +++-- pkg/services/authz/server.go | 192 -------------------------- pkg/services/authz/server_test.go | 218 ------------------------------ 3 files changed, 20 insertions(+), 424 deletions(-) delete mode 100644 pkg/services/authz/server_test.go diff --git a/pkg/services/authz/client.go b/pkg/services/authz/client.go index c6cf6525803..0b8a35a053e 100644 --- a/pkg/services/authz/client.go +++ b/pkg/services/authz/client.go @@ -8,17 +8,20 @@ import ( authnlib "github.com/grafana/authlib/authn" authzlib "github.com/grafana/authlib/authz" authzv1 "github.com/grafana/authlib/authz/proto/v1" + "github.com/grafana/authlib/claims" grpcAuth "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/auth" "google.golang.org/grpc" "google.golang.org/grpc/credentials/insecure" + "github.com/grafana/grafana/pkg/infra/db" + "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/registry/apis/iam/legacy" + "github.com/grafana/grafana/pkg/services/authz/rbac" "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" + "github.com/grafana/grafana/pkg/storage/legacysql" ) // `authzService` is hardcoded in authz-service @@ -30,9 +33,8 @@ type Client interface { // ProvideAuthZClient provides an AuthZ client and creates the AuthZ service. func ProvideAuthZClient( - cfg *setting.Cfg, features featuremgmt.FeatureToggles, ac accesscontrol.AccessControl, - authnSvc authn.Service, folderSvc folder.Service, grpcServer grpcserver.Provider, - tracer tracing.Tracer, + cfg *setting.Cfg, features featuremgmt.FeatureToggles, grpcServer grpcserver.Provider, + tracer tracing.Tracer, db db.DB, ) (Client, error) { if !features.IsEnabledGlobally(featuremgmt.FlagAuthZGRPCServer) { return nil, nil @@ -46,10 +48,8 @@ func ProvideAuthZClient( var client Client // Register the server - server, err := newLegacyServer(authnSvc, ac, folderSvc, features, grpcServer, tracer, authCfg) - if err != nil { - return nil, err - } + sql := legacysql.NewDatabaseProvider(db) + server := rbac.NewService(sql, legacy.NewLegacySQLStores(sql), log.New("authz-grpc-server"), tracer) switch authCfg.mode { case ModeInProc: @@ -92,8 +92,14 @@ func ProvideStandaloneAuthZClient( return newCloudLegacyClient(authCfg, tracer) } -func newInProcLegacyClient(server *legacyServer, tracer tracing.Tracer) (authzlib.AccessChecker, error) { - noAuth := func(ctx context.Context) (context.Context, error) { +func newInProcLegacyClient(server *rbac.Service, tracer tracing.Tracer) (authzlib.AccessChecker, error) { + // For in-proc use-case authorize add fake service claims - it should be able to access every namespace, as there is only one + staticAuth := func(ctx context.Context) (context.Context, error) { + ctx = claims.WithClaims(ctx, authnlib.NewAccessTokenAuthInfo(authnlib.Claims[authnlib.AccessTokenClaims]{ + Rest: authnlib.AccessTokenClaims{ + Namespace: "*", + }, + })) return ctx, nil } @@ -101,8 +107,8 @@ func newInProcLegacyClient(server *legacyServer, tracer tracing.Tracer) (authzli channel.RegisterService( grpchan.InterceptServer( &authzv1.AuthzService_ServiceDesc, - grpcAuth.UnaryServerInterceptor(noAuth), - grpcAuth.StreamServerInterceptor(noAuth), + grpcAuth.UnaryServerInterceptor(staticAuth), + grpcAuth.StreamServerInterceptor(staticAuth), ), server, ) diff --git a/pkg/services/authz/server.go b/pkg/services/authz/server.go index 3225248f7f6..274dc61976c 100644 --- a/pkg/services/authz/server.go +++ b/pkg/services/authz/server.go @@ -1,26 +1,13 @@ package authz import ( - "context" - "fmt" - - 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/registry/apis/iam/legacy" - "github.com/grafana/grafana/pkg/services/accesscontrol" - "github.com/grafana/grafana/pkg/services/authn" - "github.com/grafana/grafana/pkg/services/authz/mappers" authzextv1 "github.com/grafana/grafana/pkg/services/authz/proto/v1" "github.com/grafana/grafana/pkg/services/authz/rbac" - "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/storage/legacysql" ) @@ -32,182 +19,3 @@ func RegisterRBACAuthZService(handler grpcserver.Provider, db legacysql.LegacyDa authzv1.RegisterAuthzServiceServer(srv, server) authzextv1.RegisterAuthzExtentionServiceServer(srv, server) } - -var _ authzv1.AuthzServiceServer = (*legacyServer)(nil) -var _ grpc_auth.ServiceAuthFuncOverride = (*legacyServer)(nil) -var _ authzlib.ServiceAuthorizeFuncOverride = (*legacyServer)(nil) - -func newLegacyServer( - 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{ - 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 { - if !cfg.allowInsecure { - l.logger.Error("Not allowing the authz service to run in insecure mode as Auth is skipped") - } else { - grpcServer.GetServer().RegisterService(&authzv1.AuthzService_ServiceDesc, l) - } - } - - return l, nil -} - -type legacyServer struct { - authzv1.UnimplementedAuthzServiceServer - - 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. -// This is ok for now since we don't have on-prem access token support. -func (l *legacyServer) AuthFuncOverride(ctx context.Context, _ string) (context.Context, error) { - ctx, span := l.tracer.Start(ctx, "authz.AuthFuncOverride") - defer span.End() - - return ctx, nil -} - -// AuthorizeFuncOverride is a function that allows to override the default authorize function that checks the namespace of the caller. -// This is ok for now since we don't have on-prem access token support. -func (l *legacyServer) AuthorizeFuncOverride(ctx context.Context) error { - _, span := l.tracer.Start(ctx, "authz.AuthorizeFuncOverride") - defer span.End() - - return nil -} - -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 deleted file mode 100644 index 1bd617ce6ed..00000000000 --- a/pkg/services/authz/server_test.go +++ /dev/null @@ -1,218 +0,0 @@ -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: "dashboard.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: "dashboard.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: "dashboard.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: "dashboard.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: "dashboard.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: "dashboard.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: "dashboard.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: "dashboard.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) - }) - } -}