From b9cc1548f4596a4ed339b68678bd78472cb3121f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Rold=C3=A1n=20Betancort?= Date: Thu, 20 Oct 2022 15:50:33 +0100 Subject: [PATCH 1/3] test delete caveated rel with RelationTupleUpdate_DELETE the caveat is not part of the primary key of the relationship and so it's ignored when it comes to delete operations --- pkg/datastore/test/caveat.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/pkg/datastore/test/caveat.go b/pkg/datastore/test/caveat.go index e98154d0bb..5aae970454 100644 --- a/pkg/datastore/test/caveat.go +++ b/pkg/datastore/test/caveat.go @@ -146,6 +146,17 @@ func WriteCaveatedRelationshipTest(t *testing.T, tester DatastoreTester) { req.NoError(err) assertTupleCorrectlyStored(req, ds, rev, tpl) + // RelationTupleUpdate_DELETE ignores caveat part of the request + tpl.Caveat.CaveatName = "rando" + rev, err = common.WriteTuples(ctx, sds, core.RelationTupleUpdate_DELETE, tpl) + req.NoError(err) + iter, err := ds.SnapshotReader(rev).QueryRelationships(context.Background(), datastore.RelationshipsFilter{ + ResourceType: tpl.ResourceAndRelation.Namespace, + }) + req.NoError(err) + defer iter.Close() + req.Nil(iter.Next()) + // Caveated tuple can reference non-existing caveat - controller layer is responsible for validation tpl = createTestCaveatedTuple(t, "document:rando#parent@folder:company#...", "rando") _, err = common.WriteTuples(ctx, sds, core.RelationTupleUpdate_CREATE, tpl) From 31dd8d05459efdbaa00c907715d1473c760d16b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Rold=C3=A1n=20Betancort?= Date: Thu, 20 Oct 2022 18:09:29 +0100 Subject: [PATCH 2/3] fix usages of zerolog global logger --- internal/dispatch/graph/computecheck_test.go | 4 ++-- internal/services/v1/errors.go | 2 +- internal/services/v1alpha1/errors.go | 2 +- pkg/caveats/env.go | 2 ++ pkg/spiceerrors/withstatus.go | 3 ++- 5 files changed, 8 insertions(+), 5 deletions(-) diff --git a/internal/dispatch/graph/computecheck_test.go b/internal/dispatch/graph/computecheck_test.go index 2ff19e53fa..3276fa9d1e 100644 --- a/internal/dispatch/graph/computecheck_test.go +++ b/internal/dispatch/graph/computecheck_test.go @@ -7,6 +7,7 @@ import ( "google.golang.org/protobuf/types/known/structpb" "github.com/authzed/spicedb/internal/datastore/memdb" + log "github.com/authzed/spicedb/internal/logging" datastoremw "github.com/authzed/spicedb/internal/middleware/datastore" "github.com/authzed/spicedb/pkg/caveats" "github.com/authzed/spicedb/pkg/caveats/types" @@ -16,7 +17,6 @@ import ( "github.com/authzed/spicedb/pkg/schemadsl/compiler" "github.com/authzed/spicedb/pkg/tuple" - "github.com/rs/zerolog/log" "github.com/stretchr/testify/require" ) @@ -540,7 +540,7 @@ func TestComputeCheckWithCaveats(t *testing.T) { "provided": map[string]any{ "type": "backend", "region": "us", "team": "shop", "additional_attrs": map[string]any{ - "tag1": 200, + "tag1": 200.0, "tag2": false, }, }, diff --git a/internal/services/v1/errors.go b/internal/services/v1/errors.go index b58480f50a..39de517dfe 100644 --- a/internal/services/v1/errors.go +++ b/internal/services/v1/errors.go @@ -7,13 +7,13 @@ import ( "strconv" "github.com/rs/zerolog" - "github.com/rs/zerolog/log" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" v1 "github.com/authzed/authzed-go/proto/authzed/api/v1" "github.com/authzed/spicedb/internal/graph" + log "github.com/authzed/spicedb/internal/logging" "github.com/authzed/spicedb/internal/namespace" "github.com/authzed/spicedb/internal/services/shared" "github.com/authzed/spicedb/internal/sharederrors" diff --git a/internal/services/v1alpha1/errors.go b/internal/services/v1alpha1/errors.go index 93c570510a..46ac130797 100644 --- a/internal/services/v1alpha1/errors.go +++ b/internal/services/v1alpha1/errors.go @@ -5,10 +5,10 @@ import ( "errors" v1 "github.com/authzed/authzed-go/proto/authzed/api/v1" - "github.com/rs/zerolog/log" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" + log "github.com/authzed/spicedb/internal/logging" "github.com/authzed/spicedb/internal/namespace" "github.com/authzed/spicedb/internal/services/shared" "github.com/authzed/spicedb/internal/sharederrors" diff --git a/pkg/caveats/env.go b/pkg/caveats/env.go index 7d0cfdf938..19014c929a 100644 --- a/pkg/caveats/env.go +++ b/pkg/caveats/env.go @@ -66,6 +66,8 @@ func (e *Environment) asCelEnvironment() (*cel.Env, error) { opts = append(opts, customTypeOpts...) } + opts = append(opts, types.TypeMethods...) + // Set options. // DefaultUTCTimeZone: ensure all timestamps are evaluated at UTC opts = append(opts, cel.DefaultUTCTimeZone(true)) diff --git a/pkg/spiceerrors/withstatus.go b/pkg/spiceerrors/withstatus.go index 84d594f56b..7f2c3a43ff 100644 --- a/pkg/spiceerrors/withstatus.go +++ b/pkg/spiceerrors/withstatus.go @@ -3,7 +3,8 @@ package spiceerrors import ( "errors" - "github.com/rs/zerolog/log" + log "github.com/authzed/spicedb/internal/logging" + "google.golang.org/genproto/googleapis/rpc/errdetails" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" From aed105fdef9b8295e226585f1859adc3224a178d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Rold=C3=A1n=20Betancort?= Date: Thu, 20 Oct 2022 18:16:49 +0100 Subject: [PATCH 3/3] implements "isSubtreeOf" function for maps --- internal/dispatch/graph/computecheck_test.go | 5 +- pkg/caveats/env.go | 3 +- pkg/caveats/types/map.go | 45 +++++++++++++++ pkg/caveats/types/map_test.go | 61 ++++++++++++++++++++ pkg/caveats/types/registration.go | 13 +++++ 5 files changed, 123 insertions(+), 4 deletions(-) create mode 100644 pkg/caveats/types/map.go create mode 100644 pkg/caveats/types/map_test.go diff --git a/internal/dispatch/graph/computecheck_test.go b/internal/dispatch/graph/computecheck_test.go index 3276fa9d1e..9fbc95c861 100644 --- a/internal/dispatch/graph/computecheck_test.go +++ b/internal/dispatch/graph/computecheck_test.go @@ -469,7 +469,7 @@ func TestComputeCheckWithCaveats(t *testing.T) { }`, map[string]caveatDefinition{ "attributes_match": { - "expected.all(x, expected[x] == provided[x])", + "expected.isSubtreeOf(provided)", map[string]types.VariableType{ "expected": types.MapType(types.AnyType), "provided": types.MapType(types.AnyType), @@ -525,8 +525,9 @@ func TestComputeCheckWithCaveats(t *testing.T) { "provided": map[string]any{ "type": "backend", "region": "us", "team": "shop", "additional_attrs": map[string]any{ - "tag1": 100, + "tag1": 100.0, "tag2": false, + "tag3": "hi", }, }, }, diff --git a/pkg/caveats/env.go b/pkg/caveats/env.go index 19014c929a..2a50af6131 100644 --- a/pkg/caveats/env.go +++ b/pkg/caveats/env.go @@ -65,8 +65,7 @@ func (e *Environment) asCelEnvironment() (*cel.Env, error) { for _, customTypeOpts := range types.CustomTypes { opts = append(opts, customTypeOpts...) } - - opts = append(opts, types.TypeMethods...) + opts = append(opts, types.CustomMethodsOnTypes...) // Set options. // DefaultUTCTimeZone: ensure all timestamps are evaluated at UTC diff --git a/pkg/caveats/types/map.go b/pkg/caveats/types/map.go new file mode 100644 index 0000000000..a54905c3e5 --- /dev/null +++ b/pkg/caveats/types/map.go @@ -0,0 +1,45 @@ +package types + +import ( + "github.com/google/cel-go/cel" + "github.com/google/cel-go/common/types" + "github.com/google/cel-go/common/types/ref" +) + +func init() { + registerMethodOnDefinedType(cel.MapType(cel.StringType, cel.AnyType), + "isSubtreeOf", + []*cel.Type{cel.MapType(cel.StringType, cel.AnyType)}, + cel.BoolType, + func(arg ...ref.Val) ref.Val { + map0 := arg[0].Value().(map[string]any) + map1 := arg[1].Value().(map[string]any) + return types.Bool(subtree(map0, map1)) + }, + ) +} + +func subtree(map0 map[string]any, map1 map[string]any) bool { + for k, v := range map0 { + val, ok := map1[k] + if !ok { + return false + } + nestedMap0, ok := v.(map[string]any) + if ok { + nestedMap1, ok := val.(map[string]any) + if !ok { + return false + } + nestedResult := subtree(nestedMap0, nestedMap1) + if !nestedResult { + return false + } + } else { + if v != val { + return false + } + } + } + return true +} diff --git a/pkg/caveats/types/map_test.go b/pkg/caveats/types/map_test.go new file mode 100644 index 0000000000..c409508f53 --- /dev/null +++ b/pkg/caveats/types/map_test.go @@ -0,0 +1,61 @@ +package types + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestMapSubtree(t *testing.T) { + tcs := []struct { + map1 map[string]any + map2 map[string]any + subtree bool + }{ + { + map[string]any{"a": 1}, + map[string]any{"a": 1}, + true, + }, + { + map[string]any{"a": 1, "b": 2}, + map[string]any{"a": 1}, + false, + }, + { + map[string]any{"a": 1}, + map[string]any{"a": 1, "b": 1}, + true, + }, + { + map[string]any{"a": 1, "b": map[string]any{"a": 1}}, + map[string]any{"a": 1}, + false, + }, + { + map[string]any{"a": 1, "b": map[string]any{"a": 1}}, + map[string]any{"a": 1, "b": map[string]any{"a": 1}}, + true, + }, + { + map[string]any{"a": 1, "b": map[string]any{"a": 1}}, + map[string]any{"a": 1, "b": map[string]any{"a": 1, "b": 1}}, + true, + }, + { + map[string]any{"a": 1, "b": map[string]any{"a": 1}}, + map[string]any{"a": 1, "b": map[string]any{"a": "1", "b": 1}}, + false, + }, + { + map[string]any{"a": 1, "b": map[string]any{"a": 1}}, + map[string]any{"a": 1, "b": map[string]any{"a": 1, "b": map[string]any{}}}, + true, + }, + } + for _, tt := range tcs { + t.Run("", func(t *testing.T) { + require.Equal(t, tt.subtree, subtree(tt.map1, tt.map2)) + }) + } +} diff --git a/pkg/caveats/types/registration.go b/pkg/caveats/types/registration.go index de58ac82fe..ee376a40e7 100644 --- a/pkg/caveats/types/registration.go +++ b/pkg/caveats/types/registration.go @@ -2,6 +2,7 @@ package types import ( "github.com/google/cel-go/cel" + "github.com/google/cel-go/common/types/ref" ) var definitions = map[string]typeDefinition{} @@ -11,6 +12,10 @@ var definitions = map[string]typeDefinition{} // types. var CustomTypes = map[string][]cel.EnvOption{} +// CustomMethodsOnTypes holds a set of new methods applied over defined types. This is exported +// so that the CEL environment construction can apply the necessary env options to support these methods +var CustomMethodsOnTypes []cel.EnvOption + type ( typedValueConverter func(value any) (any, error) ) @@ -70,3 +75,11 @@ func registerCustomType(keyword string, baseCelType *cel.Type, converter typedVa CustomTypes[keyword] = opts return registerBasicType(keyword, baseCelType, converter) } + +func registerMethodOnDefinedType(baseType *cel.Type, name string, args []*cel.Type, returnType *cel.Type, binding func(arg ...ref.Val) ref.Val) { + finalArgs := make([]*cel.Type, 0, len(args)+1) + finalArgs = append(finalArgs, baseType) + finalArgs = append(finalArgs, args...) + method := cel.Function(name, cel.MemberOverload(name, finalArgs, returnType, cel.FunctionBinding(binding))) + CustomMethodsOnTypes = append(CustomMethodsOnTypes, method) +}