diff --git a/internal/relationships/validation.go b/internal/relationships/validation.go index f9c968f5f6..1ee322b641 100644 --- a/internal/relationships/validation.go +++ b/internal/relationships/validation.go @@ -3,22 +3,86 @@ package relationships import ( "context" + "github.com/samber/lo" + "github.com/authzed/spicedb/internal/namespace" "github.com/authzed/spicedb/pkg/caveats" "github.com/authzed/spicedb/pkg/datastore" "github.com/authzed/spicedb/pkg/genutil/mapz" ns "github.com/authzed/spicedb/pkg/namespace" core "github.com/authzed/spicedb/pkg/proto/core/v1" + "github.com/authzed/spicedb/pkg/spiceerrors" "github.com/authzed/spicedb/pkg/tuple" ) -// ValidateRelationships performs validation on the given relationships, ensuring that +// ValidateRelationshipUpdates performs validation on the given relationship updates, ensuring that // they can be applied against the datastore. -func ValidateRelationships( +func ValidateRelationshipUpdates( + ctx context.Context, + reader datastore.Reader, + updates []*core.RelationTupleUpdate, +) error { + rels := lo.Map(updates, func(item *core.RelationTupleUpdate, _ int) *core.RelationTuple { + return item.Tuple + }) + + // Load namespaces and caveats. + referencedNamespaceMap, referencedCaveatMap, err := loadNamespacesAndCaveats(ctx, rels, reader) + if err != nil { + return err + } + + // Validate each updates's types. + for _, update := range updates { + option := ValidateRelationshipForWriting + if update.Operation == core.RelationTupleUpdate_DELETE { + option = ValidateRelationshipForDeletion + } + + if err := ValidateOneRelationship( + referencedNamespaceMap, + referencedCaveatMap, + update.Tuple, + option, + ); err != nil { + return err + } + } + + return nil +} + +// ValidateRelationshipsForCreateOrTouch performs validation on the given relationships to be written, ensuring that +// they can be applied against the datastore. +// +// NOTE: This method *cannot* be used for relationships that will be deleted. +func ValidateRelationshipsForCreateOrTouch( ctx context.Context, reader datastore.Reader, rels []*core.RelationTuple, ) error { + // Load namespaces and caveats. + referencedNamespaceMap, referencedCaveatMap, err := loadNamespacesAndCaveats(ctx, rels, reader) + if err != nil { + return err + } + + // Validate each relationship's types. + for _, rel := range rels { + if err := ValidateOneRelationship( + referencedNamespaceMap, + referencedCaveatMap, + rel, + ValidateRelationshipForWriting, + ); err != nil { + return err + } + } + + return nil +} + +func loadNamespacesAndCaveats(ctx context.Context, rels []*core.RelationTuple, reader datastore.Reader) (map[string]*namespace.TypeSystem, map[string]*core.CaveatDefinition, error) { referencedNamespaceNames := mapz.NewSet[string]() referencedCaveatNamesWithContext := mapz.NewSet[string]() for _, rel := range rels { @@ -32,29 +96,27 @@ func ValidateRelationships( var referencedNamespaceMap map[string]*namespace.TypeSystem var referencedCaveatMap map[string]*core.CaveatDefinition - // Load namespaces. if !referencedNamespaceNames.IsEmpty() { foundNamespaces, err := reader.LookupNamespacesWithNames(ctx, referencedNamespaceNames.AsSlice()) if err != nil { - return err + return nil, nil, err } referencedNamespaceMap = make(map[string]*namespace.TypeSystem, len(foundNamespaces)) for _, nsDef := range foundNamespaces { nts, err := namespace.NewNamespaceTypeSystem(nsDef.Definition, namespace.ResolverForDatastoreReader(reader)) if err != nil { - return err + return nil, nil, err } referencedNamespaceMap[nsDef.Definition.Name] = nts } } - // Load caveats, if any. if !referencedCaveatNamesWithContext.IsEmpty() { foundCaveats, err := reader.LookupCaveatsWithNames(ctx, referencedCaveatNamesWithContext.AsSlice()) if err != nil { - return err + return nil, nil, err } referencedCaveatMap = make(map[string]*core.CaveatDefinition, len(foundCaveats)) @@ -62,25 +124,26 @@ func ValidateRelationships( referencedCaveatMap[caveatDef.Definition.Name] = caveatDef.Definition } } + return referencedNamespaceMap, referencedCaveatMap, nil +} - // Validate each relationship's types. - for _, rel := range rels { - if err := ValidateOneRelationship( - referencedNamespaceMap, - referencedCaveatMap, - rel, - ); err != nil { - return err - } - } +// ValidationRelationshipRule is the rule to use for the validation. +type ValidationRelationshipRule int - return nil -} +const ( + // ValidateRelationshipForWriting indicates that the validation should occur for a CREATE or TOUCH operation. + ValidateRelationshipForWriting ValidationRelationshipRule = 0 + // ValidateRelationshipForDeletion indicates that the validation should occur for a DELETE operation. + ValidateRelationshipForDeletion ValidationRelationshipRule = 1 +) + +// ValidateOneRelationship validates a single relationship for CREATE/TOUCH or DELETE. func ValidateOneRelationship( namespaceMap map[string]*namespace.TypeSystem, caveatMap map[string]*core.CaveatDefinition, rel *core.RelationTuple, + rule ValidationRelationshipRule, ) error { // Validate the IDs of the resource and subject. if err := tuple.ValidateResourceID(rel.ResourceAndRelation.ObjectId); err != nil { @@ -119,13 +182,12 @@ func ValidateOneRelationship( } // Validate the subject against the allowed relation(s). - var relationToCheck *core.AllowedRelation var caveat *core.AllowedCaveat - if rel.Caveat != nil { caveat = ns.AllowedCaveat(rel.Caveat.CaveatName) } + var relationToCheck *core.AllowedRelation if rel.Subject.ObjectId == tuple.PublicWildcard { relationToCheck = ns.AllowedPublicNamespaceWithCaveat(rel.Subject.Namespace, caveat) } else { @@ -135,16 +197,44 @@ func ValidateOneRelationship( caveat) } - isAllowed, err := resourceTS.HasAllowedRelation( - rel.ResourceAndRelation.Relation, - relationToCheck, - ) - if err != nil { - return err - } + switch { + case rule == ValidateRelationshipForWriting || caveat != nil: + // For writing or when the caveat was specified, the caveat must be a direct match. + isAllowed, err := resourceTS.HasAllowedRelation( + rel.ResourceAndRelation.Relation, + relationToCheck) + if err != nil { + return err + } + + if isAllowed != namespace.AllowedRelationValid { + return NewInvalidSubjectTypeError(rel, relationToCheck) + } + + case rule == ValidateRelationshipForDeletion && caveat == nil: + // For deletion, the caveat *can* be ignored if not specified. + if rel.Subject.ObjectId == tuple.PublicWildcard { + isAllowed, err := resourceTS.IsAllowedPublicNamespace(rel.ResourceAndRelation.Relation, rel.Subject.Namespace) + if err != nil { + return err + } + + if isAllowed != namespace.PublicSubjectAllowed { + return NewInvalidSubjectTypeError(rel, relationToCheck) + } + } else { + isAllowed, err := resourceTS.IsAllowedDirectRelation(rel.ResourceAndRelation.Relation, rel.Subject.Namespace, rel.Subject.Relation) + if err != nil { + return err + } + + if isAllowed != namespace.DirectRelationValid { + return NewInvalidSubjectTypeError(rel, relationToCheck) + } + } - if isAllowed != namespace.AllowedRelationValid { - return NewInvalidSubjectTypeError(rel, relationToCheck) + default: + return spiceerrors.MustBugf("unknown validate rule") } // Validate caveat and its context, if applicable. diff --git a/internal/relationships/validation_test.go b/internal/relationships/validation_test.go new file mode 100644 index 0000000000..a4b6cdb6d7 --- /dev/null +++ b/internal/relationships/validation_test.go @@ -0,0 +1,238 @@ +package relationships + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/authzed/spicedb/internal/datastore/memdb" + "github.com/authzed/spicedb/internal/testfixtures" + core "github.com/authzed/spicedb/pkg/proto/core/v1" + "github.com/authzed/spicedb/pkg/tuple" +) + +const basicSchema = `definition user {} + +caveat somecaveat(somecondition int) { + somecondition == 42 +} + +caveat anothercaveat(somecondition int) { + somecondition == 42 +} + +definition folder {} + +definition resource { + relation folder: folder + relation viewer: user | user with somecaveat | user:* + relation editor: user with somecaveat + relation viewer2: user:* with somecaveat + + permission view = viewer +}` + +func TestValidateRelationshipOperations(t *testing.T) { + tcs := []struct { + name string + schema string + relationship string + operation core.RelationTupleUpdate_Operation + expectedError string + }{ + { + "basic create", + basicSchema, + "resource:foo#viewer@user:tom", + core.RelationTupleUpdate_CREATE, + "", + }, + { + "basic delete", + basicSchema, + "resource:foo#viewer@user:tom", + core.RelationTupleUpdate_DELETE, + "", + }, + { + "create over permission error", + basicSchema, + "resource:foo#view@user:tom", + core.RelationTupleUpdate_CREATE, + "cannot write a relationship to permission", + }, + { + "delete over permission error", + basicSchema, + "resource:foo#view@user:tom", + core.RelationTupleUpdate_DELETE, + "cannot write a relationship to permission", + }, + { + "create wrong subject type", + basicSchema, + "resource:foo#folder@user:tom", + core.RelationTupleUpdate_CREATE, + "subjects of type `user` are not allowed on relation", + }, + { + "delete wrong subject type", + basicSchema, + "resource:foo#folder@user:tom", + core.RelationTupleUpdate_DELETE, + "subjects of type `user` are not allowed on relation", + }, + { + "unknown subject type", + basicSchema, + "resource:foo#folder@foobar:tom", + core.RelationTupleUpdate_CREATE, + "object definition `foobar` not found", + }, + { + "unknown resource type", + basicSchema, + "foobar:foo#folder@user:tom", + core.RelationTupleUpdate_CREATE, + "object definition `foobar` not found", + }, + { + "create with wrong caveat", + basicSchema, + "resource:fo#viewer@user:tom[anothercaveat]", + core.RelationTupleUpdate_CREATE, + "subjects of type `user with anothercaveat` are not allowed on relation `resource#viewer`", + }, + { + "delete with wrong caveat", + basicSchema, + "resource:fo#viewer@user:tom[anothercaveat]", + core.RelationTupleUpdate_DELETE, + "subjects of type `user with anothercaveat` are not allowed on relation `resource#viewer`", + }, + { + "create with correct caveat", + basicSchema, + "resource:fo#viewer@user:tom[somecaveat]", + core.RelationTupleUpdate_CREATE, + "", + }, + { + "delete with correct caveat", + basicSchema, + "resource:fo#viewer@user:tom[somecaveat]", + core.RelationTupleUpdate_DELETE, + "", + }, + { + "create with no caveat should error", + basicSchema, + "resource:fo#editor@user:tom", + core.RelationTupleUpdate_CREATE, + "subjects of type `user` are not allowed on relation `resource#editor`", + }, + { + "delete with no caveat should be okay", + basicSchema, + "resource:fo#editor@user:tom", + core.RelationTupleUpdate_DELETE, + "", + }, + { + "create with wildcard", + basicSchema, + "resource:fo#viewer@user:*", + core.RelationTupleUpdate_CREATE, + "", + }, + { + "delete with wildcard", + basicSchema, + "resource:fo#viewer@user:*", + core.RelationTupleUpdate_DELETE, + "", + }, + { + "create with invalid wildcard", + basicSchema, + "resource:fo#editor@user:*", + core.RelationTupleUpdate_CREATE, + "subjects of type `user:*` are not allowed on relation `resource#editor`", + }, + { + "delete with invalid wildcard", + basicSchema, + "resource:fo#editor@user:*", + core.RelationTupleUpdate_DELETE, + "subjects of type `user:*` are not allowed on relation `resource#editor`", + }, + { + "create with no caveat over wildcard should error", + basicSchema, + "resource:fo#viewer2@user:*", + core.RelationTupleUpdate_CREATE, + "subjects of type `user:*` are not allowed on relation `resource#viewer2`", + }, + { + "delete with no caveat over wildcard should be okay", + basicSchema, + "resource:fo#viewer2@user:*", + core.RelationTupleUpdate_DELETE, + "", + }, + { + "create with no caveat over concrete subject should error", + basicSchema, + "resource:fo#viewer2@user:tom", + core.RelationTupleUpdate_CREATE, + "subjects of type `user` are not allowed on relation `resource#viewer2`", + }, + { + "delete with no caveat over concrete subject should error", + basicSchema, + "resource:fo#viewer2@user:tom", + core.RelationTupleUpdate_DELETE, + "subjects of type `user` are not allowed on relation `resource#viewer2`", + }, + } + + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + req := require.New(t) + + ds, err := memdb.NewMemdbDatastore(0, 0, memdb.DisableGC) + req.NoError(err) + + uds, rev := testfixtures.DatastoreFromSchemaAndTestRelationships(ds, tc.schema, nil, req) + reader := uds.SnapshotReader(rev) + + op := tuple.Create + if tc.operation == core.RelationTupleUpdate_DELETE { + op = tuple.Delete + } + + // Validate update. + err = ValidateRelationshipUpdates(context.Background(), reader, []*core.RelationTupleUpdate{ + op(tuple.MustParse(tc.relationship)), + }) + if tc.expectedError != "" { + req.ErrorContains(err, tc.expectedError) + } else { + req.NoError(err) + } + + // Validate create/touch. + if tc.operation != core.RelationTupleUpdate_DELETE { + err = ValidateRelationshipsForCreateOrTouch(context.Background(), reader, []*core.RelationTuple{ + tuple.MustParse(tc.relationship), + }) + if tc.expectedError != "" { + req.ErrorContains(err, tc.expectedError) + } else { + req.NoError(err) + } + } + }) + } +} diff --git a/internal/services/v1/experimental.go b/internal/services/v1/experimental.go index d9ec981e1b..c6ff0471b1 100644 --- a/internal/services/v1/experimental.go +++ b/internal/services/v1/experimental.go @@ -146,6 +146,7 @@ func (a *bulkLoadAdapter) Next(_ context.Context) (*core.RelationTuple, error) { a.referencedNamespaceMap, a.referencedCaveatMap, &a.current, + relationships.ValidateRelationshipForWriting, ); err != nil { return nil, err } diff --git a/internal/services/v1/relationships.go b/internal/services/v1/relationships.go index 711850a9c0..3d0a7c5528 100644 --- a/internal/services/v1/relationships.go +++ b/internal/services/v1/relationships.go @@ -10,7 +10,6 @@ import ( "github.com/jzelinskie/stringz" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" - "github.com/samber/lo" "google.golang.org/protobuf/proto" "github.com/authzed/spicedb/internal/dispatch" @@ -292,12 +291,8 @@ func (ps *permissionServer) WriteRelationships(ctx context.Context, req *v1.Writ } } - tuples := lo.Map(tupleUpdates, func(item *core.RelationTupleUpdate, _ int) *core.RelationTuple { - return item.Tuple - }) - // Validate the updates. - err := relationships.ValidateRelationships(ctx, rwt, tuples) + err := relationships.ValidateRelationshipUpdates(ctx, rwt, tupleUpdates) if err != nil { return ps.rewriteError(ctx, err) } diff --git a/internal/services/v1/relationships_test.go b/internal/services/v1/relationships_test.go index 359c8c53d6..f608d0254a 100644 --- a/internal/services/v1/relationships_test.go +++ b/internal/services/v1/relationships_test.go @@ -405,44 +405,78 @@ func TestDeleteRelationshipViaWriteNoop(t *testing.T) { } func TestWriteCaveatedRelationships(t *testing.T) { - req := require.New(t) + for _, deleteWithCaveat := range []bool{true, false} { + t.Run(fmt.Sprintf("with-caveat-%v", deleteWithCaveat), func(t *testing.T) { + req := require.New(t) - conn, cleanup, _, _ := testserver.NewTestServer(req, 0, memdb.DisableGC, true, tf.StandardDatastoreWithData) - client := v1.NewPermissionsServiceClient(conn) - t.Cleanup(cleanup) + conn, cleanup, _, _ := testserver.NewTestServer(req, 0, memdb.DisableGC, true, tf.StandardDatastoreWithData) + client := v1.NewPermissionsServiceClient(conn) + t.Cleanup(cleanup) - toWrite := tuple.MustParse("document:companyplan#caveated_viewer@user:johndoe#...") - caveatCtx, err := structpb.NewStruct(map[string]any{"expectedSecret": "hi"}) - req.NoError(err) + toWrite := tuple.MustParse("document:companyplan#caveated_viewer@user:johndoe#...") + caveatCtx, err := structpb.NewStruct(map[string]any{"expectedSecret": "hi"}) + req.NoError(err) - toWrite.Caveat = &core.ContextualizedCaveat{ - CaveatName: "doesnotexist", - Context: caveatCtx, - } - toWrite.Caveat.Context = caveatCtx - relWritten := tuple.MustToRelationship(toWrite) - writeReq := &v1.WriteRelationshipsRequest{ - Updates: []*v1.RelationshipUpdate{{ - Operation: v1.RelationshipUpdate_OPERATION_CREATE, - Relationship: relWritten, - }}, - } + toWrite.Caveat = &core.ContextualizedCaveat{ + CaveatName: "doesnotexist", + Context: caveatCtx, + } + toWrite.Caveat.Context = caveatCtx + relWritten := tuple.MustToRelationship(toWrite) + writeReq := &v1.WriteRelationshipsRequest{ + Updates: []*v1.RelationshipUpdate{{ + Operation: v1.RelationshipUpdate_OPERATION_CREATE, + Relationship: relWritten, + }}, + } - // Should fail due to non-existing caveat - ctx := context.Background() - _, err = client.WriteRelationships(ctx, writeReq) - grpcutil.RequireStatus(t, codes.InvalidArgument, err) + // Should fail due to non-existing caveat + ctx := context.Background() + _, err = client.WriteRelationships(ctx, writeReq) + grpcutil.RequireStatus(t, codes.InvalidArgument, err) - req.Contains(err.Error(), "subjects of type `user with doesnotexist` are not allowed on relation `document#caveated_viewer`") + req.Contains(err.Error(), "subjects of type `user with doesnotexist` are not allowed on relation `document#caveated_viewer`") - // should succeed - relWritten.OptionalCaveat.CaveatName = "test" - resp, err := client.WriteRelationships(context.Background(), writeReq) - req.NoError(err) + // should succeed + relWritten.OptionalCaveat.CaveatName = "test" + resp, err := client.WriteRelationships(context.Background(), writeReq) + req.NoError(err) - // read relationship back - relRead := readFirst(req, client, resp.WrittenAt, relWritten) - req.True(proto.Equal(relWritten, relRead)) + // read relationship back + relRead := readFirst(req, client, resp.WrittenAt, relWritten) + req.True(proto.Equal(relWritten, relRead)) + + // issue the deletion + relToDelete := tuple.MustToRelationship(tuple.MustParse("document:companyplan#caveated_viewer@user:johndoe#...")) + if deleteWithCaveat { + relToDelete = tuple.MustToRelationship(tuple.MustParse("document:companyplan#caveated_viewer@user:johndoe#...[test]")) + } + + deleteReq := &v1.WriteRelationshipsRequest{ + Updates: []*v1.RelationshipUpdate{{ + Operation: v1.RelationshipUpdate_OPERATION_DELETE, + Relationship: relToDelete, + }}, + } + + resp, err = client.WriteRelationships(context.Background(), deleteReq) + req.NoError(err) + + // ensure the relationship is no longer present. + stream, err := client.ReadRelationships(context.Background(), &v1.ReadRelationshipsRequest{ + Consistency: &v1.Consistency{ + Requirement: &v1.Consistency_AtExactSnapshot{ + AtExactSnapshot: resp.WrittenAt, + }, + }, + RelationshipFilter: tuple.RelToFilter(relWritten), + }) + require.NoError(t, err) + + _, err = stream.Recv() + require.True(t, errors.Is(err, io.EOF)) + }) + } } func readFirst(require *require.Assertions, client v1.PermissionsServiceClient, token *v1.ZedToken, rel *v1.Relationship) *v1.Relationship { diff --git a/pkg/development/devcontext.go b/pkg/development/devcontext.go index f26f43b4cd..7a39b30221 100644 --- a/pkg/development/devcontext.go +++ b/pkg/development/devcontext.go @@ -201,7 +201,7 @@ func loadTuples(ctx context.Context, tuples []*core.RelationTuple, rwt datastore continue } - err = relationships.ValidateRelationships(ctx, rwt, []*core.RelationTuple{tpl}) + err = relationships.ValidateRelationshipsForCreateOrTouch(ctx, rwt, []*core.RelationTuple{tpl}) if err != nil { devErr, wireErr := distinguishGraphError(ctx, err, devinterface.DeveloperError_RELATIONSHIP, 0, 0, tplString) if devErr != nil { diff --git a/pkg/validationfile/loader.go b/pkg/validationfile/loader.go index ef25b5cccc..4bfac58bab 100644 --- a/pkg/validationfile/loader.go +++ b/pkg/validationfile/loader.go @@ -154,7 +154,7 @@ func PopulateFromFilesContents(ctx context.Context, ds datastore.Datastore, file chunkedTuples = append(chunkedTuples, update.Tuple) } revision, err = ds.ReadWriteTx(ctx, func(rwt datastore.ReadWriteTransaction) error { - err = relationships.ValidateRelationships(ctx, rwt, chunkedTuples) + err = relationships.ValidateRelationshipsForCreateOrTouch(ctx, rwt, chunkedTuples) if err != nil { return err }