From 318a0ebb366a5892aac2529bbffccdc25f9cc68b Mon Sep 17 00:00:00 2001 From: Jo Date: Wed, 31 Dec 2025 10:15:00 +0100 Subject: [PATCH] IAM: Authorize writes to zanzana on token permissions (#115645) * validate writes to zanzana, not reads * lint ignore --- pkg/services/authz/zanzana.go | 3 ++ pkg/services/authz/zanzana/server/auth.go | 19 ++++++++ .../authz/zanzana/server/server_bench_test.go | 2 +- .../authz/zanzana/server/server_mutate.go | 6 ++- .../server/server_mutate_folder_test.go | 8 ++-- .../server/server_mutate_org_role_test.go | 4 +- .../server_mutate_resourcepermissions_test.go | 4 +- .../server/server_mutate_rolebindings_test.go | 4 +- .../server/server_mutate_roles_test.go | 2 +- .../server/server_mutate_teambindings_test.go | 2 +- .../zanzana/server/server_mutate_test.go | 23 +++++++++- .../authz/zanzana/server/server_test.go | 13 +++++- .../authz/zanzana/server/server_write.go | 6 ++- .../authz/zanzana/server/server_write_test.go | 46 +++++++++++++++++++ pkg/services/authz/zanzana/zanzana.go | 3 ++ 15 files changed, 128 insertions(+), 17 deletions(-) create mode 100644 pkg/services/authz/zanzana/server/server_write_test.go diff --git a/pkg/services/authz/zanzana.go b/pkg/services/authz/zanzana.go index f6751258f2d..79eb19a8b5b 100644 --- a/pkg/services/authz/zanzana.go +++ b/pkg/services/authz/zanzana.go @@ -78,6 +78,9 @@ func ProvideZanzanaClient(cfg *setting.Cfg, db db.DB, tracer tracing.Tracer, fea ctx = types.WithAuthInfo(ctx, authnlib.NewAccessTokenAuthInfo(authnlib.Claims[authnlib.AccessTokenClaims]{ Rest: authnlib.AccessTokenClaims{ Namespace: "*", + Permissions: []string{ + zanzana.TokenPermissionUpdate, + }, }, })) return ctx, nil diff --git a/pkg/services/authz/zanzana/server/auth.go b/pkg/services/authz/zanzana/server/auth.go index f137bdca47f..5ea412c35b8 100644 --- a/pkg/services/authz/zanzana/server/auth.go +++ b/pkg/services/authz/zanzana/server/auth.go @@ -4,7 +4,9 @@ import ( "context" "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/services/authz/zanzana" "github.com/grafana/grafana/pkg/setting" + "golang.org/x/exp/slices" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -30,3 +32,20 @@ func authorize(ctx context.Context, namespace string, ss setting.ZanzanaServerSe } return nil } + +func authorizeWrite(ctx context.Context, namespace string, ss setting.ZanzanaServerSettings) error { + if err := authorize(ctx, namespace, ss); err != nil { + return err + } + + c, ok := claims.AuthInfoFrom(ctx) + if !ok { + return status.Errorf(codes.Unauthenticated, "unauthenticated") + } + + if !slices.Contains(c.GetTokenPermissions(), zanzana.TokenPermissionUpdate) { + return status.Errorf(codes.PermissionDenied, "missing token permission %s", zanzana.TokenPermissionUpdate) + } + + return nil +} diff --git a/pkg/services/authz/zanzana/server/server_bench_test.go b/pkg/services/authz/zanzana/server/server_bench_test.go index 98ec58560b0..1d76dbea66b 100644 --- a/pkg/services/authz/zanzana/server/server_bench_test.go +++ b/pkg/services/authz/zanzana/server/server_bench_test.go @@ -391,7 +391,7 @@ func setupBenchmarkServer(b *testing.B) (*Server, *benchmarkData) { b.Logf("Total tuples to write: %d", len(allTuples)) // Get store info - ctx := newContextWithNamespace() + ctx := newContextWithZanzanaUpdatePermission() storeInf, err := srv.getStoreInfo(ctx, benchNamespace) require.NoError(b, err) diff --git a/pkg/services/authz/zanzana/server/server_mutate.go b/pkg/services/authz/zanzana/server/server_mutate.go index 15a57404bbc..3af0d06180d 100644 --- a/pkg/services/authz/zanzana/server/server_mutate.go +++ b/pkg/services/authz/zanzana/server/server_mutate.go @@ -8,6 +8,7 @@ import ( openfgav1 "github.com/openfga/api/proto/openfga/v1" "go.opentelemetry.io/otel/codes" + "google.golang.org/grpc/status" authzextv1 "github.com/grafana/grafana/pkg/services/authz/proto/v1" ) @@ -35,6 +36,9 @@ func (s *Server) Mutate(ctx context.Context, req *authzextv1.MutateRequest) (*au if err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) + if _, ok := status.FromError(err); ok { + return nil, err + } s.logger.Error("failed to perform mutate request", "error", err, "namespace", req.GetNamespace()) return nil, errors.New("failed to perform mutate request") } @@ -43,7 +47,7 @@ func (s *Server) Mutate(ctx context.Context, req *authzextv1.MutateRequest) (*au } func (s *Server) mutate(ctx context.Context, req *authzextv1.MutateRequest) (*authzextv1.MutateResponse, error) { - if err := authorize(ctx, req.GetNamespace(), s.cfg); err != nil { + if err := authorizeWrite(ctx, req.GetNamespace(), s.cfg); err != nil { return nil, err } diff --git a/pkg/services/authz/zanzana/server/server_mutate_folder_test.go b/pkg/services/authz/zanzana/server/server_mutate_folder_test.go index a01c3b08740..410e70099d8 100644 --- a/pkg/services/authz/zanzana/server/server_mutate_folder_test.go +++ b/pkg/services/authz/zanzana/server/server_mutate_folder_test.go @@ -30,7 +30,7 @@ func testMutateFolders(t *testing.T, srv *Server) { setupMutateFolders(t, srv) t.Run("should create new folder parent relation", func(t *testing.T) { - _, err := srv.Mutate(newContextWithNamespace(), &v1.MutateRequest{ + _, err := srv.Mutate(newContextWithZanzanaUpdatePermission(), &v1.MutateRequest{ Namespace: "default", Operations: []*v1.MutateOperation{ { @@ -61,7 +61,7 @@ func testMutateFolders(t *testing.T, srv *Server) { }) t.Run("should delete folder parent relation", func(t *testing.T) { - _, err := srv.Mutate(newContextWithNamespace(), &v1.MutateRequest{ + _, err := srv.Mutate(newContextWithZanzanaUpdatePermission(), &v1.MutateRequest{ Namespace: "default", Operations: []*v1.MutateOperation{ { @@ -88,7 +88,7 @@ func testMutateFolders(t *testing.T, srv *Server) { }) t.Run("should clean up all parent relations", func(t *testing.T) { - _, err := srv.Mutate(newContextWithNamespace(), &v1.MutateRequest{ + _, err := srv.Mutate(newContextWithZanzanaUpdatePermission(), &v1.MutateRequest{ Namespace: "default", Operations: []*v1.MutateOperation{ { @@ -115,7 +115,7 @@ func testMutateFolders(t *testing.T, srv *Server) { }) t.Run("should perform batch mutate if multiple operations are provided", func(t *testing.T) { - _, err := srv.Mutate(newContextWithNamespace(), &v1.MutateRequest{ + _, err := srv.Mutate(newContextWithZanzanaUpdatePermission(), &v1.MutateRequest{ Namespace: "default", Operations: []*v1.MutateOperation{ { diff --git a/pkg/services/authz/zanzana/server/server_mutate_org_role_test.go b/pkg/services/authz/zanzana/server/server_mutate_org_role_test.go index 7c7472cf6ac..899ba4be86f 100644 --- a/pkg/services/authz/zanzana/server/server_mutate_org_role_test.go +++ b/pkg/services/authz/zanzana/server/server_mutate_org_role_test.go @@ -25,7 +25,7 @@ func testMutateOrgRoles(t *testing.T, srv *Server) { setupMutateOrgRoles(t, srv) t.Run("should update user org role and delete old role", func(t *testing.T) { - _, err := srv.Mutate(newContextWithNamespace(), &v1.MutateRequest{ + _, err := srv.Mutate(newContextWithZanzanaUpdatePermission(), &v1.MutateRequest{ Namespace: "default", Operations: []*v1.MutateOperation{ { @@ -63,7 +63,7 @@ func testMutateOrgRoles(t *testing.T, srv *Server) { }) t.Run("should add user org role and delete old role", func(t *testing.T) { - _, err := srv.Mutate(newContextWithNamespace(), &v1.MutateRequest{ + _, err := srv.Mutate(newContextWithZanzanaUpdatePermission(), &v1.MutateRequest{ Namespace: "default", Operations: []*v1.MutateOperation{ { diff --git a/pkg/services/authz/zanzana/server/server_mutate_resourcepermissions_test.go b/pkg/services/authz/zanzana/server/server_mutate_resourcepermissions_test.go index 3336d9d813b..296ed9e2be3 100644 --- a/pkg/services/authz/zanzana/server/server_mutate_resourcepermissions_test.go +++ b/pkg/services/authz/zanzana/server/server_mutate_resourcepermissions_test.go @@ -28,7 +28,7 @@ func testMutateResourcePermissions(t *testing.T, srv *Server) { setupMutateResourcePermissions(t, srv) t.Run("should create new resource permission", func(t *testing.T) { - _, err := srv.Mutate(newContextWithNamespace(), &v1.MutateRequest{ + _, err := srv.Mutate(newContextWithZanzanaUpdatePermission(), &v1.MutateRequest{ Namespace: "default", Operations: []*v1.MutateOperation{ { @@ -76,7 +76,7 @@ func testMutateResourcePermissions(t *testing.T, srv *Server) { require.NoError(t, err) require.Len(t, res.Tuples, 2) - _, err = srv.Mutate(newContextWithNamespace(), &v1.MutateRequest{ + _, err = srv.Mutate(newContextWithZanzanaUpdatePermission(), &v1.MutateRequest{ Namespace: "default", Operations: []*v1.MutateOperation{ { diff --git a/pkg/services/authz/zanzana/server/server_mutate_rolebindings_test.go b/pkg/services/authz/zanzana/server/server_mutate_rolebindings_test.go index 87db285c360..3d4b0c5db51 100644 --- a/pkg/services/authz/zanzana/server/server_mutate_rolebindings_test.go +++ b/pkg/services/authz/zanzana/server/server_mutate_rolebindings_test.go @@ -25,7 +25,7 @@ func testMutateRoleBindings(t *testing.T, srv *Server) { setupMutateRoleBindings(t, srv) t.Run("should update user role and delete old role", func(t *testing.T) { - _, err := srv.Mutate(newContextWithNamespace(), &v1.MutateRequest{ + _, err := srv.Mutate(newContextWithZanzanaUpdatePermission(), &v1.MutateRequest{ Namespace: "default", Operations: []*v1.MutateOperation{ { @@ -75,7 +75,7 @@ func testMutateRoleBindings(t *testing.T, srv *Server) { }) t.Run("should assign role to basic role", func(t *testing.T) { - _, err := srv.Mutate(newContextWithNamespace(), &v1.MutateRequest{ + _, err := srv.Mutate(newContextWithZanzanaUpdatePermission(), &v1.MutateRequest{ Namespace: "default", Operations: []*v1.MutateOperation{ { diff --git a/pkg/services/authz/zanzana/server/server_mutate_roles_test.go b/pkg/services/authz/zanzana/server/server_mutate_roles_test.go index 1128c0c0f72..e6c92dc66f8 100644 --- a/pkg/services/authz/zanzana/server/server_mutate_roles_test.go +++ b/pkg/services/authz/zanzana/server/server_mutate_roles_test.go @@ -25,7 +25,7 @@ func testMutateRoles(t *testing.T, srv *Server) { setupMutateRoles(t, srv) t.Run("should update role and delete old role permissions", func(t *testing.T) { - _, err := srv.Mutate(newContextWithNamespace(), &v1.MutateRequest{ + _, err := srv.Mutate(newContextWithZanzanaUpdatePermission(), &v1.MutateRequest{ Namespace: "default", Operations: []*v1.MutateOperation{ { diff --git a/pkg/services/authz/zanzana/server/server_mutate_teambindings_test.go b/pkg/services/authz/zanzana/server/server_mutate_teambindings_test.go index 5103b142fc5..a4c8d73d856 100644 --- a/pkg/services/authz/zanzana/server/server_mutate_teambindings_test.go +++ b/pkg/services/authz/zanzana/server/server_mutate_teambindings_test.go @@ -25,7 +25,7 @@ func testMutateTeamBindings(t *testing.T, srv *Server) { setupMutateTeamBindings(t, srv) t.Run("should update user team binding and delete old team binding", func(t *testing.T) { - _, err := srv.Mutate(newContextWithNamespace(), &v1.MutateRequest{ + _, err := srv.Mutate(newContextWithZanzanaUpdatePermission(), &v1.MutateRequest{ Namespace: "default", Operations: []*v1.MutateOperation{ { diff --git a/pkg/services/authz/zanzana/server/server_mutate_test.go b/pkg/services/authz/zanzana/server/server_mutate_test.go index c1fcfabbe43..57949b9ef12 100644 --- a/pkg/services/authz/zanzana/server/server_mutate_test.go +++ b/pkg/services/authz/zanzana/server/server_mutate_test.go @@ -5,6 +5,8 @@ import ( openfgav1 "github.com/openfga/api/proto/openfga/v1" "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" "google.golang.org/protobuf/types/known/structpb" iamv0 "github.com/grafana/grafana/apps/iam/pkg/apis/iam/v0alpha1" @@ -33,7 +35,7 @@ func testMutate(t *testing.T, srv *Server) { setupMutate(t, srv) t.Run("should perform multiple mutate operations", func(t *testing.T) { - _, err := srv.Mutate(newContextWithNamespace(), &v1.MutateRequest{ + _, err := srv.Mutate(newContextWithZanzanaUpdatePermission(), &v1.MutateRequest{ Namespace: "default", Operations: []*v1.MutateOperation{ { @@ -133,6 +135,25 @@ func testMutate(t *testing.T, srv *Server) { require.NoError(t, err) require.Len(t, res.Tuples, 0) }) + + t.Run("should reject mutate without zanzana:update", func(t *testing.T) { + _, err := srv.Mutate(newContextWithNamespace(), &v1.MutateRequest{ + Namespace: "default", + Operations: []*v1.MutateOperation{ + { + Operation: &v1.MutateOperation_SetFolderParent{ + SetFolderParent: &v1.SetFolderParentOperation{ + Folder: "new-folder", + Parent: "1", + DeleteExisting: false, + }, + }, + }, + }, + }) + require.Error(t, err) + require.Equal(t, codes.PermissionDenied, status.Code(err)) + }) } func TestDeduplicateTupleKeys(t *testing.T) { diff --git a/pkg/services/authz/zanzana/server/server_test.go b/pkg/services/authz/zanzana/server/server_test.go index 3f3a7e2cad6..b910d1f492a 100644 --- a/pkg/services/authz/zanzana/server/server_test.go +++ b/pkg/services/authz/zanzana/server/server_test.go @@ -14,6 +14,7 @@ import ( "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/authz/zanzana" "github.com/grafana/grafana/pkg/services/authz/zanzana/common" "github.com/grafana/grafana/pkg/services/authz/zanzana/store" "github.com/grafana/grafana/pkg/services/sqlstore" @@ -218,11 +219,21 @@ func setupOpenFGADatabase(t *testing.T, srv *Server, tuples []*openfgav1.TupleKe } func newContextWithNamespace() context.Context { + return newContextWithNamespaceAndPermissions() +} + +func newContextWithNamespaceAndPermissions(perms ...string) context.Context { ctx := context.Background() ctx = claims.WithAuthInfo(ctx, authnlib.NewAccessTokenAuthInfo(authnlib.Claims[authnlib.AccessTokenClaims]{ Rest: authnlib.AccessTokenClaims{ - Namespace: "*", + Namespace: "*", + Permissions: perms, + DelegatedPermissions: perms, }, })) return ctx } + +func newContextWithZanzanaUpdatePermission() context.Context { + return newContextWithNamespaceAndPermissions(zanzana.TokenPermissionUpdate) +} diff --git a/pkg/services/authz/zanzana/server/server_write.go b/pkg/services/authz/zanzana/server/server_write.go index d16d706a320..dd0e4b11f0d 100644 --- a/pkg/services/authz/zanzana/server/server_write.go +++ b/pkg/services/authz/zanzana/server/server_write.go @@ -8,6 +8,7 @@ import ( openfgav1 "github.com/openfga/api/proto/openfga/v1" "go.opentelemetry.io/otel/codes" + "google.golang.org/grpc/status" authzextv1 "github.com/grafana/grafana/pkg/services/authz/proto/v1" "github.com/grafana/grafana/pkg/services/authz/zanzana/common" @@ -25,6 +26,9 @@ func (s *Server) Write(ctx context.Context, req *authzextv1.WriteRequest) (*auth if err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) + if _, ok := status.FromError(err); ok { + return nil, err + } s.logger.Error("failed to perform write request", "error", err, "namespace", req.GetNamespace()) return nil, errors.New("failed to perform write request") } @@ -33,7 +37,7 @@ func (s *Server) Write(ctx context.Context, req *authzextv1.WriteRequest) (*auth } func (s *Server) write(ctx context.Context, req *authzextv1.WriteRequest) (*authzextv1.WriteResponse, error) { - if err := authorize(ctx, req.GetNamespace(), s.cfg); err != nil { + if err := authorizeWrite(ctx, req.GetNamespace(), s.cfg); err != nil { return nil, err } diff --git a/pkg/services/authz/zanzana/server/server_write_test.go b/pkg/services/authz/zanzana/server/server_write_test.go new file mode 100644 index 00000000000..892edcb694c --- /dev/null +++ b/pkg/services/authz/zanzana/server/server_write_test.go @@ -0,0 +1,46 @@ +package server + +import ( + "testing" + + "github.com/grafana/grafana/pkg/services/sqlstore" + "github.com/grafana/grafana/pkg/setting" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + + authzextv1 "github.com/grafana/grafana/pkg/services/authz/proto/v1" + "github.com/grafana/grafana/pkg/services/authz/zanzana/common" + "github.com/stretchr/testify/require" +) + +func TestWriteAuthorization(t *testing.T) { + cfg := setting.NewCfg() + testStore := sqlstore.NewTestStore(t, sqlstore.WithCfg(cfg)) + srv := setupOpenFGAServer(t, testStore, cfg) + setup(t, srv) + + req := &authzextv1.WriteRequest{ + Namespace: namespace, + Writes: &authzextv1.WriteRequestWrites{ + TupleKeys: []*authzextv1.TupleKey{ + { + // Folder parent tuples are valid without any relationship condition. + User: "folder:1", + Relation: common.RelationParent, + Object: "folder:write-authz-test", + }, + }, + }, + } + + t.Run("denies Write without zanzana:update", func(t *testing.T) { + _, err := srv.Write(newContextWithNamespace(), req) + require.Error(t, err) + require.Equal(t, codes.PermissionDenied, status.Code(err)) + }) + + t.Run("allows Write with zanzana:update", func(t *testing.T) { + _, err := srv.Write(newContextWithZanzanaUpdatePermission(), req) + require.NoError(t, err) + }) +} diff --git a/pkg/services/authz/zanzana/zanzana.go b/pkg/services/authz/zanzana/zanzana.go index 6cadfc0c199..261ea78830d 100644 --- a/pkg/services/authz/zanzana/zanzana.go +++ b/pkg/services/authz/zanzana/zanzana.go @@ -16,6 +16,9 @@ const ( TypeNamespace = common.TypeGroupResouce ) +// TokenPermissionUpdate is required for callers to perform write operations against Zanzana (Mutate/Write). +const TokenPermissionUpdate = "zanzana:update" //nolint:gosec // G101: permission identifier, not a credential. + const ( RelationTeamMember = common.RelationTeamMember RelationTeamAdmin = common.RelationTeamAdmin