From eaea073889905ffdbf9ca6056db9bde226a66d8e Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 10 Dec 2021 16:31:32 -0500 Subject: [PATCH 01/11] Update authzed-go dependency to pull in proto changes --- go.mod | 2 +- go.sum | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 105b65ec15..65ea09d4b8 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,7 @@ go 1.16 require ( github.com/Masterminds/squirrel v1.5.2 github.com/alecthomas/units v0.0.0-20210927113745-59d0afb8317a - github.com/authzed/authzed-go v0.3.1-0.20211130221323-9d59da6e55da + github.com/authzed/authzed-go v0.3.1-0.20211210193835-3ada8db7bc94 github.com/authzed/grpcutil v0.0.0-20211020204402-aba1876830e6 github.com/aws/aws-sdk-go v1.42.16 github.com/benbjohnson/clock v1.3.0 diff --git a/go.sum b/go.sum index 3e1ac30845..3a2dd4a6e0 100644 --- a/go.sum +++ b/go.sum @@ -75,6 +75,10 @@ github.com/armon/go-radix v0.0.0-20180808171621-7fddfc383310/go.mod h1:ufUuZ+zHj github.com/armon/go-radix v1.0.0/go.mod h1:ufUuZ+zHj4x4TnLV4JWEpy2hxWSpsRywHrMgIH9cCH8= github.com/authzed/authzed-go v0.3.1-0.20211130221323-9d59da6e55da h1:dcjIyi2Om9CDAmEKiwQZsF7bV0x4PXuJeoN11BbbXXI= github.com/authzed/authzed-go v0.3.1-0.20211130221323-9d59da6e55da/go.mod h1:bsUniBRroq4l5WZMYLO+T9osQa/P2qMwZ+Af8zoJK8Y= +github.com/authzed/authzed-go v0.3.1-0.20211209213733-6dfdd048c960 h1:C8e4cBlOetPhqNCMgE4fhBb+wzhQLziHfHJgzRFZd3s= +github.com/authzed/authzed-go v0.3.1-0.20211209213733-6dfdd048c960/go.mod h1:bsUniBRroq4l5WZMYLO+T9osQa/P2qMwZ+Af8zoJK8Y= +github.com/authzed/authzed-go v0.3.1-0.20211210193835-3ada8db7bc94 h1:4la5GstRfU268cich5RwPe7+Smkz/4BUcHDGSOH8yhY= +github.com/authzed/authzed-go v0.3.1-0.20211210193835-3ada8db7bc94/go.mod h1:bsUniBRroq4l5WZMYLO+T9osQa/P2qMwZ+Af8zoJK8Y= github.com/authzed/grpcutil v0.0.0-20210913124023-cad23ae5a9e8/go.mod h1:HwO/KbRU3fWXEYHE96kvXnwxzi97tkXD1hfi5UaZ71Y= github.com/authzed/grpcutil v0.0.0-20211020204402-aba1876830e6 h1:izP/rEris51ZmomXb5J0ShyJKqsxTfVKDRnJz0QGbgg= github.com/authzed/grpcutil v0.0.0-20211020204402-aba1876830e6/go.mod h1:rqjY3zyK/YP7NID9+B2BdIRRkvnK+cdf9/qya/zaFZE= From 947cc9b339af534d1ae8ec6469e0db2b64cad58b Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 10 Dec 2021 16:33:47 -0500 Subject: [PATCH 02/11] Add middleware to run the handwritten validation methods in the protos --- .../handwrittenvalidation.go | 54 +++++++++++++++++++ internal/services/v0/acl.go | 2 + internal/services/v0/acl_test.go | 12 +++++ internal/services/v1/permissions_test.go | 7 +++ internal/services/v1/relationships.go | 3 ++ internal/services/v1/relationships_test.go | 7 +++ 6 files changed, 85 insertions(+) create mode 100644 internal/middleware/handwrittenvalidation/handwrittenvalidation.go diff --git a/internal/middleware/handwrittenvalidation/handwrittenvalidation.go b/internal/middleware/handwrittenvalidation/handwrittenvalidation.go new file mode 100644 index 0000000000..019afce56e --- /dev/null +++ b/internal/middleware/handwrittenvalidation/handwrittenvalidation.go @@ -0,0 +1,54 @@ +package handwrittenvalidation + +import ( + "context" + + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +type handwrittenValidator interface { + HandwrittenValidate() error +} + +// UnaryServerInterceptor returns a new unary server interceptor that runs the handwritten validation +// on the incoming request, if any. +func UnaryServerInterceptor(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) { + validator, ok := req.(handwrittenValidator) + if ok { + err := validator.HandwrittenValidate() + if err != nil { + return nil, status.Errorf(codes.InvalidArgument, "%s", err) + } + } + + return handler(ctx, req) +} + +// StreamServerInterceptor returns a new stream server interceptor that runs the handwritten validation +// on the incoming request messages, if any. +func StreamServerInterceptor(srv interface{}, stream grpc.ServerStream, info *grpc.StreamServerInfo, handler grpc.StreamHandler) error { + wrapper := &recvWrapper{stream} + return handler(srv, wrapper) +} + +type recvWrapper struct { + grpc.ServerStream +} + +func (s *recvWrapper) RecvMsg(m interface{}) error { + if err := s.ServerStream.RecvMsg(m); err != nil { + return err + } + + validator, ok := m.(handwrittenValidator) + if ok { + err := validator.HandwrittenValidate() + if err != nil { + return err + } + } + + return nil +} diff --git a/internal/services/v0/acl.go b/internal/services/v0/acl.go index f090e94254..557f7d18a9 100644 --- a/internal/services/v0/acl.go +++ b/internal/services/v0/acl.go @@ -18,6 +18,7 @@ import ( "github.com/authzed/spicedb/internal/datastore" "github.com/authzed/spicedb/internal/dispatch" "github.com/authzed/spicedb/internal/graph" + "github.com/authzed/spicedb/internal/middleware/handwrittenvalidation" "github.com/authzed/spicedb/internal/middleware/usagemetrics" "github.com/authzed/spicedb/internal/namespace" v1 "github.com/authzed/spicedb/internal/proto/dispatch/v1" @@ -49,6 +50,7 @@ var errInvalidZookie = errors.New("invalid revision requested") func NewACLServer(ds datastore.Datastore, nsm namespace.Manager, dispatch dispatch.Dispatcher, defaultDepth uint32) v0.ACLServiceServer { middleware := []grpc.UnaryServerInterceptor{ usagemetrics.UnaryServerInterceptor(), + handwrittenvalidation.UnaryServerInterceptor, } middleware = append(middleware, grpcutil.DefaultUnaryMiddleware...) diff --git a/internal/services/v0/acl_test.go b/internal/services/v0/acl_test.go index 3ed93f2efd..0e30f16338 100644 --- a/internal/services/v0/acl_test.go +++ b/internal/services/v0/acl_test.go @@ -417,6 +417,18 @@ func TestInvalidWriteArguments(t *testing.T) { []string{"document:newdoc#parent@user:someuser"}, codes.InvalidArgument, }, + { + "invalid object", + nil, + []string{"document:*#parent@user:someuser#..."}, + codes.InvalidArgument, + }, + { + "wildcard userset not allowed", + nil, + []string{"document:somedoc#parent@user:*#..."}, + codes.InvalidArgument, + }, } for _, tc := range testCases { diff --git a/internal/services/v1/permissions_test.go b/internal/services/v1/permissions_test.go index a5d99d6ae1..2df1db8c46 100644 --- a/internal/services/v1/permissions_test.go +++ b/internal/services/v1/permissions_test.go @@ -219,6 +219,13 @@ func TestCheckPermissions(t *testing.T) { v1.CheckPermissionResponse_PERMISSIONSHIP_UNSPECIFIED, codes.FailedPrecondition, }, + { + obj("document", "*"), + "viewer_and_editor_derived", + sub("invalidnamespace", "someuser", ""), + v1.CheckPermissionResponse_PERMISSIONSHIP_UNSPECIFIED, + codes.InvalidArgument, + }, } for _, delta := range testTimedeltas { diff --git a/internal/services/v1/relationships.go b/internal/services/v1/relationships.go index 86d895aa90..4516545d8d 100644 --- a/internal/services/v1/relationships.go +++ b/internal/services/v1/relationships.go @@ -17,6 +17,7 @@ import ( "github.com/authzed/spicedb/internal/dispatch" "github.com/authzed/spicedb/internal/graph" "github.com/authzed/spicedb/internal/middleware/consistency" + "github.com/authzed/spicedb/internal/middleware/handwrittenvalidation" "github.com/authzed/spicedb/internal/middleware/usagemetrics" "github.com/authzed/spicedb/internal/namespace" "github.com/authzed/spicedb/internal/services/serviceerrors" @@ -40,11 +41,13 @@ func NewPermissionsServer(ds datastore.Datastore, WithServiceSpecificInterceptors: shared.WithServiceSpecificInterceptors{ Unary: grpcmw.ChainUnaryServer( grpcvalidate.UnaryServerInterceptor(), + handwrittenvalidation.UnaryServerInterceptor, usagemetrics.UnaryServerInterceptor(), consistency.UnaryServerInterceptor(ds), ), Stream: grpcmw.ChainStreamServer( grpcvalidate.StreamServerInterceptor(), + handwrittenvalidation.StreamServerInterceptor, usagemetrics.StreamServerInterceptor(), consistency.StreamServerInterceptor(ds), ), diff --git a/internal/services/v1/relationships_test.go b/internal/services/v1/relationships_test.go index 4f87489ae2..79e1cf3c91 100644 --- a/internal/services/v1/relationships_test.go +++ b/internal/services/v1/relationships_test.go @@ -435,6 +435,13 @@ func TestInvalidWriteRelationshipArgs(t *testing.T) { codes.InvalidArgument, "user:someuser is not allowed", }, + { + "bad write wildcard object", + nil, + []*v1.Relationship{rel("document", "*", "parent", "user", "someuser", "")}, + codes.InvalidArgument, + "alphanumeric", + }, } for _, delta := range testTimedeltas { From bffc0a7902265f77887b7f23bb1727dabaf2bc1d Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 10 Dec 2021 16:34:36 -0500 Subject: [PATCH 03/11] Add validation methods for resource ID and subject ID in tuple package, and allow wildcards in subject IDs --- pkg/tuple/onr.go | 2 +- pkg/tuple/onr_test.go | 8 ++++++++ pkg/tuple/tuple.go | 40 +++++++++++++++++++++++++++++++++------- pkg/tuple/tuple_test.go | 21 +++++++++++++++++++++ 4 files changed, 63 insertions(+), 8 deletions(-) diff --git a/pkg/tuple/onr.go b/pkg/tuple/onr.go index 9aa150390e..051cb591ea 100644 --- a/pkg/tuple/onr.go +++ b/pkg/tuple/onr.go @@ -74,7 +74,7 @@ func StringONR(onr *v0.ObjectAndRelation) string { return "" } - if onr.Relation == ellipsis { + if onr.Relation == Ellipsis { return fmt.Sprintf(formatImplicitSubjectRel, onr.Namespace, onr.ObjectId) } diff --git a/pkg/tuple/onr_test.go b/pkg/tuple/onr_test.go index 0c833a47f0..916f4f9fc7 100644 --- a/pkg/tuple/onr_test.go +++ b/pkg/tuple/onr_test.go @@ -15,6 +15,10 @@ var onrTestCases = []struct { serialized: "tenant/testns:testobj#testrel", objectFormat: ObjectAndRelation("tenant/testns", "testobj", "testrel"), }, + { + serialized: "tenant/testns:*#testrel", + objectFormat: nil, + }, { serialized: "tenant/testns:testobj#...", objectFormat: nil, @@ -66,6 +70,10 @@ var subjectOnrTestCases = []struct { serialized: "tenant/testns:testobj#...", objectFormat: ObjectAndRelation("tenant/testns", "testobj", "..."), }, + { + serialized: "tenant/testns:*#...", + objectFormat: ObjectAndRelation("tenant/testns", "*", "..."), + }, { serialized: "tenant/testns:testobj", objectFormat: ObjectAndRelation("tenant/testns", "testobj", "..."), diff --git a/pkg/tuple/tuple.go b/pkg/tuple/tuple.go index cf8b1f90ec..eaad3a8078 100644 --- a/pkg/tuple/tuple.go +++ b/pkg/tuple/tuple.go @@ -10,32 +10,40 @@ import ( ) const ( - ellipsis = "..." + // Ellipsis is the ellipsis relation in v0 style subjects. + Ellipsis = "..." + + // PublicWildcard is the wildcard value for subject object IDs that indicates public access + // for the subject type. + PublicWildcard = "*" ) const ( namespaceNameExpr = "([a-z][a-z0-9_]{2,61}[a-z0-9]/)?[a-z][a-z0-9_]{2,62}[a-z0-9]" - objectIDExpr = "[a-zA-Z0-9_][a-zA-Z0-9/_-]{0,127}" + resourceIDExpr = "[a-zA-Z0-9_][a-zA-Z0-9/_-]{0,127}" + subjectIDExpr = "([a-zA-Z0-9_][a-zA-Z0-9/_-]{0,127})|\\*" relationExpr = "[a-z][a-z0-9_]{2,62}[a-z0-9]" ) var onrExpr = fmt.Sprintf( `(?P(%s)):(?P%s)#(?P%s)`, namespaceNameExpr, - objectIDExpr, + resourceIDExpr, relationExpr, ) var subjectExpr = fmt.Sprintf( `(?P(%s)):(?P%s)(#(?P%s|\.\.\.))?`, namespaceNameExpr, - objectIDExpr, + subjectIDExpr, relationExpr, ) var ( - onrRegex = regexp.MustCompile(fmt.Sprintf("^%s$", onrExpr)) - subjectRegex = regexp.MustCompile(fmt.Sprintf("^%s$", subjectExpr)) + onrRegex = regexp.MustCompile(fmt.Sprintf("^%s$", onrExpr)) + subjectRegex = regexp.MustCompile(fmt.Sprintf("^%s$", subjectExpr)) + resourceIDRegex = regexp.MustCompile(fmt.Sprintf("^%s$", resourceIDExpr)) + subjectIDRegex = regexp.MustCompile(fmt.Sprintf("^%s$", subjectIDExpr)) ) var parserRegex = regexp.MustCompile( @@ -46,6 +54,24 @@ var parserRegex = regexp.MustCompile( ), ) +// ValidateResourceID ensures that the given resource ID is valid. Returns an error if not. +func ValidateResourceID(objectID string) error { + if !resourceIDRegex.MatchString(objectID) { + return fmt.Errorf("invalid resource id; must be alphanumeric and between 1 and 127 characters") + } + + return nil +} + +// ValidateSubjectID ensures that the given object ID (under a subject reference) is valid. Returns an error if not. +func ValidateSubjectID(subjectID string) error { + if !subjectIDRegex.MatchString(subjectID) { + return fmt.Errorf("invalid subject id; must be alphanumeric and between 1 and 127 characters or a star for public") + } + + return nil +} + // String converts a tuple to a string. If the tuple is nil or empty, returns empty string. func String(tpl *v0.RelationTuple) string { if tpl == nil || tpl.ObjectAndRelation == nil || tpl.User == nil || tpl.User.GetUserset() == nil { @@ -85,7 +111,7 @@ func Parse(tpl string) *v0.RelationTuple { return nil } - subjectRelation := ellipsis + subjectRelation := Ellipsis subjectRelIndex := stringz.SliceIndex(parserRegex.SubexpNames(), "subjectRel") if len(groups[subjectRelIndex]) > 0 { subjectRelation = groups[subjectRelIndex] diff --git a/pkg/tuple/tuple_test.go b/pkg/tuple/tuple_test.go index 9cf2d87d4b..0152c741b7 100644 --- a/pkg/tuple/tuple_test.go +++ b/pkg/tuple/tuple_test.go @@ -116,6 +116,15 @@ var testCases = []struct { ), relFormat: rel("foos", "bar", "bazzy", "groo", "grar", ""), }, + { + input: "tenant/testns:testobj#testrel@tenant/user:*#...", + expectedOutput: "tenant/testns:testobj#testrel@tenant/user:*", + tupleFormat: makeTuple( + ObjectAndRelation("tenant/testns", "testobj", "testrel"), + ObjectAndRelation("tenant/user", "*", "..."), + ), + relFormat: rel("tenant/testns", "testobj", "testrel", "tenant/user", "*", ""), + }, } func TestSerialize(t *testing.T) { @@ -179,3 +188,15 @@ func TestConvert(t *testing.T) { }) } } + +func TestValidate(t *testing.T) { + for _, tc := range testCases { + t.Run("validate/"+tc.input, func(t *testing.T) { + parsed := ParseRel(tc.input) + if parsed != nil { + require.NoError(t, ValidateResourceID(parsed.Resource.ObjectId)) + require.NoError(t, ValidateSubjectID(parsed.Subject.Object.ObjectId)) + } + }) + } +} From 19ae1b8f98342d9f6b238060ce92f0f2ef2aa3eb Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 10 Dec 2021 16:42:35 -0500 Subject: [PATCH 04/11] Implement wildcard support in the type system - Change to AllowedRelation type - Add methods for filtered lookup of direct relations - Add IsAllowedPublicNamespace method - Add wildcard type validation in Validate - Update namespace diff to support wildcards --- internal/datastore/test/namespace.go | 6 +- internal/graph/lookup.go | 14 +- internal/namespace/diff.go | 99 ++++++++++---- internal/namespace/diff_test.go | 58 +++++++- internal/namespace/typesystem.go | 171 ++++++++++++++++++++++-- internal/namespace/typesystem_test.go | 51 +++++-- internal/services/v0/namespace_test.go | 30 ++--- internal/testfixtures/datastore.go | 22 +-- pkg/namespace/builder.go | 24 +++- pkg/schemadsl/compiler/compiler_test.go | 29 +++- 10 files changed, 404 insertions(+), 100 deletions(-) diff --git a/internal/datastore/test/namespace.go b/internal/datastore/test/namespace.go index c85e00c6f2..ee3baf22db 100644 --- a/internal/datastore/test/namespace.go +++ b/internal/datastore/test/namespace.go @@ -17,12 +17,12 @@ import ( var ( testNamespace = ns.Namespace("test/test", - ns.Relation("editor", nil, ns.RelationReference(testUserNS.Name, "...")), + ns.Relation("editor", nil, ns.AllowedRelation(testUserNS.Name, "...")), ) updatedNamespace = ns.Namespace(testNamespace.Name, - ns.Relation("reader", nil, ns.RelationReference(testUserNS.Name, "...")), - ns.Relation("editor", nil, ns.RelationReference(testUserNS.Name, "...")), + ns.Relation("reader", nil, ns.AllowedRelation(testUserNS.Name, "...")), + ns.Relation("editor", nil, ns.AllowedRelation(testUserNS.Name, "...")), ) ) diff --git a/internal/graph/lookup.go b/internal/graph/lookup.go index 8c23f41ed3..cbf0ac9fbe 100644 --- a/internal/graph/lookup.go +++ b/internal/graph/lookup.go @@ -198,9 +198,9 @@ func (cl *ConcurrentLookup) lookupDirect(ctx context.Context, req ValidatedLooku }) } - // Dispatch to any allowed direct relation types that don't match the target ONR, collect + // Dispatch to any allowed subject relation types that don't match the target ONR, collect // the found object IDs, and then search for those. - allowedDirect, err := typeSystem.AllowedDirectRelations(req.ObjectRelation.Relation) + allowedDirect, err := typeSystem.AllowedSubjectRelations(req.ObjectRelation.Relation) if err != nil { return returnResult(lookupResultError(req, err, emptyMetadata)) } @@ -212,19 +212,19 @@ func (cl *ConcurrentLookup) lookupDirect(ctx context.Context, req ValidatedLooku var excludedDirect []*v0.RelationReference for _, allowedDirectType := range allowedDirect { - if allowedDirectType.Relation == Ellipsis { + if allowedDirectType.GetRelation() == Ellipsis { continue } if allowedDirectType.Namespace == req.ObjectRelation.Namespace && - allowedDirectType.Relation == req.ObjectRelation.Relation { + allowedDirectType.GetRelation() == req.ObjectRelation.Relation { continue } // Prevent recursive inferred lookups, which can cause an infinite loop. rr := &v0.RelationReference{ Namespace: allowedDirectType.Namespace, - Relation: allowedDirectType.Relation, + Relation: allowedDirectType.GetRelation(), } if checkStackContains(directStack, rr) { excludedDirect = append(excludedDirect, rr) @@ -241,7 +241,7 @@ func (cl *ConcurrentLookup) lookupDirect(ctx context.Context, req ValidatedLooku Subject: req.Subject, ObjectRelation: &v0.RelationReference{ Namespace: allowedDirectType.Namespace, - Relation: allowedDirectType.Relation, + Relation: allowedDirectType.GetRelation(), }, Limit: noLimit, // Since this is an inferred lookup, we can't limit. Metadata: decrementDepth(req.Metadata), @@ -350,7 +350,7 @@ func (cl *ConcurrentLookup) processTupleToUserset(ctx context.Context, req Valid return returnResult(result) } - tuplesetDirectRelations, err := typeSystem.AllowedDirectRelations(ttu.Tupleset.Relation) + tuplesetDirectRelations, err := typeSystem.AllowedSubjectRelations(ttu.Tupleset.Relation) if err != nil { return returnResult(lookupResultError(req, err, emptyMetadata)) } diff --git a/internal/namespace/diff.go b/internal/namespace/diff.go index b3d10a210e..10969a5e95 100644 --- a/internal/namespace/diff.go +++ b/internal/namespace/diff.go @@ -37,6 +37,14 @@ const ( // RelationDirectTypeRemoved indicates that an allowed direct relation type has been removed from // the relation. RelationDirectTypeRemoved DeltaType = "relation-direct-type-removed" + + // RelationDirectWildcardTypeAdded indicates that an allowed relation wildcard type has been added to + // the relation. + RelationDirectWildcardTypeAdded DeltaType = "relation-direct-type-added" + + // RelationDirectWildcardTypeRemoved indicates that an allowed relation wildcard type has been removed from + // the relation. + RelationDirectWildcardTypeRemoved DeltaType = "relation-direct-type-removed" ) // NamespaceDiff holds the diff between two namespaces. @@ -60,6 +68,9 @@ type Delta struct { // DirectType is the direct relation type added or removed, if any. DirectType *v0.RelationReference + + // WildcardType is the wildcard type added or removed, if any. + WildcardType string } // DiffNamespaces performs a diff between two namespace definitions. One or both of the definitions @@ -167,41 +178,77 @@ func DiffNamespaces(existing *v0.NamespaceDefinition, updated *v0.NamespaceDefin updatedAllowedRels := tuple.NewONRSet() for _, existingAllowed := range existingTypeInfo.AllowedDirectRelations { - existingAllowedRels.Add(&v0.ObjectAndRelation{ - Namespace: existingAllowed.Namespace, - Relation: existingAllowed.Relation, - ObjectId: "", - }) + if existingAllowed.GetRelation() != "" { + existingAllowedRels.Add(&v0.ObjectAndRelation{ + Namespace: existingAllowed.Namespace, + Relation: existingAllowed.GetRelation(), + ObjectId: "", + }) + } + + if existingAllowed.GetPublicWildcard() != nil { + existingAllowedRels.Add(&v0.ObjectAndRelation{ + Namespace: existingAllowed.Namespace, + Relation: "-", + ObjectId: tuple.PublicWildcard, + }) + } } for _, updatedAllowed := range updatedTypeInfo.AllowedDirectRelations { - updatedAllowedRels.Add(&v0.ObjectAndRelation{ - Namespace: updatedAllowed.Namespace, - Relation: updatedAllowed.Relation, - ObjectId: "", - }) + if updatedAllowed.GetRelation() != "" { + updatedAllowedRels.Add(&v0.ObjectAndRelation{ + Namespace: updatedAllowed.Namespace, + Relation: updatedAllowed.GetRelation(), + ObjectId: "", + }) + } + + if updatedAllowed.GetPublicWildcard() != nil { + updatedAllowedRels.Add(&v0.ObjectAndRelation{ + Namespace: updatedAllowed.Namespace, + Relation: "-", + ObjectId: tuple.PublicWildcard, + }) + } } for _, removed := range existingAllowedRels.Subtract(updatedAllowedRels).AsSlice() { - deltas = append(deltas, Delta{ - Type: RelationDirectTypeRemoved, - RelationName: shared, - DirectType: &v0.RelationReference{ - Namespace: removed.Namespace, - Relation: removed.Relation, - }, - }) + if removed.ObjectId == tuple.PublicWildcard { + deltas = append(deltas, Delta{ + Type: RelationDirectWildcardTypeRemoved, + RelationName: shared, + WildcardType: removed.Namespace, + }) + } else { + deltas = append(deltas, Delta{ + Type: RelationDirectTypeRemoved, + RelationName: shared, + DirectType: &v0.RelationReference{ + Namespace: removed.Namespace, + Relation: removed.Relation, + }, + }) + } } for _, added := range updatedAllowedRels.Subtract(existingAllowedRels).AsSlice() { - deltas = append(deltas, Delta{ - Type: RelationDirectTypeAdded, - RelationName: shared, - DirectType: &v0.RelationReference{ - Namespace: added.Namespace, - Relation: added.Relation, - }, - }) + if added.ObjectId == tuple.PublicWildcard { + deltas = append(deltas, Delta{ + Type: RelationDirectWildcardTypeAdded, + RelationName: shared, + WildcardType: added.Namespace, + }) + } else { + deltas = append(deltas, Delta{ + Type: RelationDirectTypeAdded, + RelationName: shared, + DirectType: &v0.RelationReference{ + Namespace: added.Namespace, + Relation: added.Relation, + }, + }) + } } } diff --git a/internal/namespace/diff_test.go b/internal/namespace/diff_test.go index 519af76b96..d04c20eefc 100644 --- a/internal/namespace/diff_test.go +++ b/internal/namespace/diff_test.go @@ -126,7 +126,7 @@ func TestNamespaceDiff(t *testing.T) { ns.Relation("somerel", ns.Union( ns.This(), ns.ComputedUserset("owner"), - ), ns.RelationReference("foo", "bar")), + ), ns.AllowedRelation("foo", "bar")), ), []Delta{ {Type: RelationDirectTypeAdded, RelationName: "somerel", DirectType: &v0.RelationReference{ @@ -142,7 +142,7 @@ func TestNamespaceDiff(t *testing.T) { ns.Relation("somerel", ns.Union( ns.This(), ns.ComputedUserset("owner"), - ), ns.RelationReference("foo", "bar")), + ), ns.AllowedRelation("foo", "bar")), ), ns.Namespace( "document", @@ -165,14 +165,14 @@ func TestNamespaceDiff(t *testing.T) { ns.Relation("somerel", ns.Union( ns.This(), ns.ComputedUserset("owner"), - ), ns.RelationReference("foo", "bar")), + ), ns.AllowedRelation("foo", "bar")), ), ns.Namespace( "document", ns.Relation("somerel", ns.Union( ns.This(), ns.ComputedUserset("owner"), - ), ns.RelationReference("foo", "bar")), + ), ns.AllowedRelation("foo", "bar")), ), []Delta{}, }, @@ -183,14 +183,14 @@ func TestNamespaceDiff(t *testing.T) { ns.Relation("somerel", ns.Union( ns.This(), ns.ComputedUserset("owner"), - ), ns.RelationReference("foo", "bar")), + ), ns.AllowedRelation("foo", "bar")), ), ns.Namespace( "document", ns.Relation("somerel", ns.Union( ns.This(), ns.ComputedUserset("owner"), - ), ns.RelationReference("foo2", "bar")), + ), ns.AllowedRelation("foo2", "bar")), ), []Delta{ {Type: RelationDirectTypeRemoved, RelationName: "somerel", DirectType: &v0.RelationReference{ @@ -203,6 +203,52 @@ func TestNamespaceDiff(t *testing.T) { }}, }, }, + { + "wildcard type added and removed", + ns.Namespace( + "document", + ns.Relation("somerel", ns.Union( + ns.This(), + ns.ComputedUserset("owner"), + ), ns.AllowedPublicNamespace("foo")), + ), + ns.Namespace( + "document", + ns.Relation("somerel", ns.Union( + ns.This(), + ns.ComputedUserset("owner"), + ), ns.AllowedPublicNamespace("foo2")), + ), + []Delta{ + {Type: RelationDirectWildcardTypeRemoved, RelationName: "somerel", WildcardType: "foo"}, + {Type: RelationDirectWildcardTypeAdded, RelationName: "somerel", WildcardType: "foo2"}, + }, + }, + + { + "wildcard type changed", + ns.Namespace( + "document", + ns.Relation("somerel", ns.Union( + ns.This(), + ns.ComputedUserset("owner"), + ), ns.AllowedPublicNamespace("foo")), + ), + ns.Namespace( + "document", + ns.Relation("somerel", ns.Union( + ns.This(), + ns.ComputedUserset("owner"), + ), ns.AllowedRelation("foo", "something")), + ), + []Delta{ + {Type: RelationDirectWildcardTypeRemoved, RelationName: "somerel", WildcardType: "foo"}, + {Type: RelationDirectTypeAdded, RelationName: "somerel", DirectType: &v0.RelationReference{ + Namespace: "foo", + Relation: "something", + }}, + }, + }, } for _, tc := range testCases { diff --git a/internal/namespace/typesystem.go b/internal/namespace/typesystem.go index d180d65c49..313525c06c 100644 --- a/internal/namespace/typesystem.go +++ b/internal/namespace/typesystem.go @@ -10,8 +10,10 @@ import ( iv1 "github.com/authzed/spicedb/internal/proto/impl/v1" "github.com/authzed/spicedb/pkg/graph" nspkg "github.com/authzed/spicedb/pkg/namespace" + "github.com/authzed/spicedb/pkg/tuple" ) +// AllowedDirectRelation indicates whether a relation is allowed on the right side of another relation. type AllowedDirectRelation int const ( @@ -28,6 +30,23 @@ const ( DirectRelationNotValid ) +// AllowedPublicSubject indicates whether a public subject of a particular kind is allowed on the right side of another relation. +type AllowedPublicSubject int + +const ( + // UnknownIfPublicAllowed indicates that no type information is defined for + // this relation. + UnknownIfPublicAllowed AllowedPublicSubject = iota + + // PublicSubjectAllowed indicates that the specified subject wildcard is valid as + // part of a *direct* tuple on the relation. + PublicSubjectAllowed + + // PublicSubjectNotAllowed indicates that the specified subject wildcard is not + // valid as part of a *direct* tuple on the relation. + PublicSubjectNotAllowed +) + // LookupNamespace is a function used to lookup a namespace. type LookupNamespace func(ctx context.Context, name string) (*v0.NamespaceDefinition, error) @@ -87,17 +106,19 @@ func BuildNamespaceTypeSystem(nsDef *v0.NamespaceDefinition, lookupNamespace Loo } return &NamespaceTypeSystem{ - lookupNamespace: lookupNamespace, - nsDef: nsDef, - relationMap: relationMap, + lookupNamespace: lookupNamespace, + nsDef: nsDef, + relationMap: relationMap, + wildcardCheckCache: map[string]*v0.AllowedRelation{}, }, nil } // NamespaceTypeSystem represents typing information found in a namespace. type NamespaceTypeSystem struct { - lookupNamespace LookupNamespace - nsDef *v0.NamespaceDefinition - relationMap map[string]*v0.Relation + lookupNamespace LookupNamespace + nsDef *v0.NamespaceDefinition + relationMap map[string]*v0.Relation + wildcardCheckCache map[string]*v0.AllowedRelation } // HasTypeInformation returns true if the relation with the given name exists and has type @@ -124,6 +145,28 @@ func (nts *NamespaceTypeSystem) IsPermission(relationName string) bool { return nspkg.GetRelationKind(found) == iv1.RelationMetadata_PERMISSION } +// IsAllowedPublicNamespace returns whether the target namespace is defined as public on the source relation. +func (nts *NamespaceTypeSystem) IsAllowedPublicNamespace(sourceRelationName string, targetNamespaceName string) (AllowedPublicSubject, error) { + found, ok := nts.relationMap[sourceRelationName] + if !ok { + return UnknownIfPublicAllowed, fmt.Errorf("unknown relation/permission `%s` under permissions system `%s`", sourceRelationName, nts.nsDef.Name) + } + + typeInfo := found.GetTypeInformation() + if typeInfo == nil { + return UnknownIfPublicAllowed, nil + } + + allowedRelations := typeInfo.GetAllowedDirectRelations() + for _, allowedRelation := range allowedRelations { + if allowedRelation.GetNamespace() == targetNamespaceName && allowedRelation.GetPublicWildcard() != nil { + return PublicSubjectAllowed, nil + } + } + + return PublicSubjectNotAllowed, nil +} + // IsAllowedDirectRelation returns whether the subject relation is allowed to appear on the right // hand side of a tuple placed in the source relation with the given name. func (nts *NamespaceTypeSystem) IsAllowedDirectRelation(sourceRelationName string, targetNamespaceName string, targetRelationName string) (AllowedDirectRelation, error) { @@ -147,21 +190,104 @@ func (nts *NamespaceTypeSystem) IsAllowedDirectRelation(sourceRelationName strin return DirectRelationNotValid, nil } -// AllowedDirectRelations returns the allowed subject relations for a source relation. -func (nts *NamespaceTypeSystem) AllowedDirectRelations(sourceRelationName string) ([]*v0.RelationReference, error) { +// AllowedDirectRelationsAndWildcards returns the allowed subject relations for a source relation. Note that this function will return +// wildcards. +func (nts *NamespaceTypeSystem) AllowedDirectRelationsAndWildcards(sourceRelationName string) ([]*v0.AllowedRelation, error) { found, ok := nts.relationMap[sourceRelationName] if !ok { - return []*v0.RelationReference{}, fmt.Errorf("unknown relation/permission `%s` under permissions system `%s`", sourceRelationName, nts.nsDef.Name) + return []*v0.AllowedRelation{}, fmt.Errorf("unknown relation/permission `%s` under permissions system `%s`", sourceRelationName, nts.nsDef.Name) } typeInfo := found.GetTypeInformation() if typeInfo == nil { - return []*v0.RelationReference{}, nil + return []*v0.AllowedRelation{}, nil } return typeInfo.GetAllowedDirectRelations(), nil } +// AllowedSubjectRelations returns the allowed subject relations for a source relation. Note that this function will *not* +// return wildcards. +func (nts *NamespaceTypeSystem) AllowedSubjectRelations(sourceRelationName string) ([]*v0.RelationReference, error) { + allowedDirect, err := nts.AllowedDirectRelationsAndWildcards(sourceRelationName) + if err != nil { + return []*v0.RelationReference{}, err + } + + filtered := make([]*v0.RelationReference, 0, len(allowedDirect)) + for _, allowed := range allowedDirect { + if allowed.GetPublicWildcard() != nil { + continue + } + + if allowed.GetRelation() == "" { + panic("Got an empty relation for a non-wildcard type definition under namespace") + } + + filtered = append(filtered, &v0.RelationReference{ + Namespace: allowed.GetNamespace(), + Relation: allowed.GetRelation(), + }) + } + return filtered, nil +} + +// ReferencesWildcardType returns true if the relation references a wildcard type, either directly or via +// another relation. +func (nts *NamespaceTypeSystem) ReferencesWildcardType(ctx context.Context, relationName string) (*v0.AllowedRelation, error) { + return nts.referencesWildcardType(ctx, relationName, map[string]bool{}) +} + +func (nts *NamespaceTypeSystem) referencesWildcardType(ctx context.Context, relationName string, encountered map[string]bool) (*v0.AllowedRelation, error) { + cached, isCached := nts.wildcardCheckCache[relationName] + if isCached { + return cached, nil + } + + computed, err := nts.computeReferencesWildcardType(ctx, relationName, encountered) + if err != nil { + return nil, err + } + + nts.wildcardCheckCache[relationName] = computed + return computed, nil +} + +func (nts *NamespaceTypeSystem) computeReferencesWildcardType(ctx context.Context, relationName string, encountered map[string]bool) (*v0.AllowedRelation, error) { + relString := fmt.Sprintf("%s#%s", nts.nsDef.Name, relationName) + _, ok := encountered[relString] + if ok { + return nil, nil + } + encountered[relString] = true + + allowedRels, err := nts.AllowedDirectRelationsAndWildcards(relationName) + if err != nil { + return nil, err + } + + for _, allowedRelation := range allowedRels { + if allowedRelation.GetPublicWildcard() != nil { + return allowedRelation, nil + } + + if allowedRelation.GetRelation() != tuple.Ellipsis { + if allowedRelation.GetNamespace() == nts.nsDef.Name { + return nts.referencesWildcardType(ctx, allowedRelation.GetRelation(), encountered) + } + + subjectTS, err := nts.typeSystemForNamespace(ctx, allowedRelation.GetNamespace()) + if err != nil { + return nil, err + } + + return subjectTS.referencesWildcardType(ctx, allowedRelation.GetRelation(), encountered) + } + } + + return nil, nil +} + // Validate runs validation on the type system for the namespace to ensure it is consistent. func (nts *NamespaceTypeSystem) Validate(ctx context.Context) error { for _, relation := range nts.relationMap { @@ -195,6 +321,16 @@ func (nts *NamespaceTypeSystem) Validate(ctx context.Context) error { if nspkg.GetRelationKind(found) == iv1.RelationMetadata_PERMISSION { return fmt.Errorf("under permission `%s`: permissions cannot be used on the left hand side of an arrow (found `%s`)", relation.Name, relationName) } + + // Ensure the tupleset relation doesn't itself import wildcard. + wildcardType, err := nts.ReferencesWildcardType(ctx, relationName) + if err != nil { + return err + } + + if wildcardType != nil { + return fmt.Errorf("for arrow under relation `%s`: relation `%s#%s` includes wildcard (public) type `%s`: wildcard relations cannot be used on the left side of arrows", relation.Name, nts.nsDef.Name, relationName, wildcardType.GetNamespace()) + } } return nil }) @@ -228,7 +364,7 @@ func (nts *NamespaceTypeSystem) Validate(ctx context.Context) error { // they exist within the namespace. for _, allowedRelation := range allowedDirectRelations { if allowedRelation.GetNamespace() == nts.nsDef.Name { - if allowedRelation.GetRelation() != "..." { + if allowedRelation.GetPublicWildcard() == nil && allowedRelation.GetRelation() != tuple.Ellipsis { _, ok := nts.relationMap[allowedRelation.GetRelation()] if !ok { return fmt.Errorf("for relation `%s`: relation/permission `%s` was not found under definition `%s`", relation.Name, allowedRelation.GetRelation(), allowedRelation.GetNamespace()) @@ -240,11 +376,22 @@ func (nts *NamespaceTypeSystem) Validate(ctx context.Context) error { return fmt.Errorf("could not lookup definition `%s` for relation `%s`: %w", allowedRelation.GetNamespace(), relation.Name, err) } - if allowedRelation.GetRelation() != "..." { + if allowedRelation.GetPublicWildcard() == nil && allowedRelation.GetRelation() != tuple.Ellipsis { + // Ensure the relation exists. ok := subjectTS.HasRelation(allowedRelation.GetRelation()) if !ok { return fmt.Errorf("for relation `%s`: relation/permission `%s` was not found under definition `%s`", relation.Name, allowedRelation.GetRelation(), allowedRelation.GetNamespace()) } + + // Ensure the relation doesn't itself import wildcard. + wildcardType, err := subjectTS.ReferencesWildcardType(ctx, allowedRelation.GetRelation()) + if err != nil { + return err + } + + if wildcardType != nil { + return fmt.Errorf("for relation `%s`: relation/permission `%s#%s` includes wildcard (public) type `%s`: wildcard relations cannot be transitively included", relation.Name, allowedRelation.GetNamespace(), allowedRelation.GetRelation(), wildcardType.GetNamespace()) + } } } } diff --git a/internal/namespace/typesystem_test.go b/internal/namespace/typesystem_test.go index 606aa81b32..907fcabfac 100644 --- a/internal/namespace/typesystem_test.go +++ b/internal/namespace/typesystem_test.go @@ -84,7 +84,7 @@ func TestTypeSystem(t *testing.T) { ns.Relation("owner", nil), ns.Relation("editor", ns.Union( ns.ComputedUserset("owner"), - ), ns.RelationReference("document", "owner")), + ), ns.AllowedRelation("document", "owner")), ), []*v0.NamespaceDefinition{}, "direct relations are not allowed under relation `editor`", @@ -93,7 +93,7 @@ func TestTypeSystem(t *testing.T) { "relation in relation types has invalid namespace", ns.Namespace( "document", - ns.Relation("owner", nil, ns.RelationReference("someinvalidns", "...")), + ns.Relation("owner", nil, ns.AllowedRelation("someinvalidns", "...")), ns.Relation("editor", ns.Union( ns.This(), ns.ComputedUserset("owner"), @@ -113,7 +113,7 @@ func TestTypeSystem(t *testing.T) { "relation in relation types has invalid relation", ns.Namespace( "document", - ns.Relation("owner", nil, ns.RelationReference("anotherns", "foobar")), + ns.Relation("owner", nil, ns.AllowedRelation("anotherns", "foobar")), ns.Relation("editor", ns.Union( ns.This(), ns.ComputedUserset("owner"), @@ -137,36 +137,65 @@ func TestTypeSystem(t *testing.T) { "full type check", ns.Namespace( "document", - ns.Relation("owner", nil, ns.RelationReference("user", "...")), + ns.Relation("owner", nil, ns.AllowedRelation("user", "...")), ns.Relation("can_comment", nil, - ns.RelationReference("user", "..."), - ns.RelationReference("folder", "can_comment"), + ns.AllowedRelation("user", "..."), + ns.AllowedRelation("folder", "can_comment"), ), ns.Relation("editor", ns.Union( ns.ComputedUserset("owner"), ns.This(), ), - ns.RelationReference("user", "..."), + ns.AllowedRelation("user", "..."), ), - ns.Relation("parent", nil, ns.RelationReference("folder", "...")), + ns.Relation("parent", nil, ns.AllowedRelation("folder", "...")), ns.Relation("viewer", ns.Union( ns.This(), ns.ComputedUserset("editor"), ns.TupleToUserset("parent", "viewer"), - ), ns.RelationReference("user", "...")), + ), ns.AllowedRelation("user", "..."), ns.AllowedPublicNamespace("user")), ), []*v0.NamespaceDefinition{ ns.Namespace("user"), ns.Namespace( "folder", - ns.Relation("can_comment", nil, ns.RelationReference("user", "...")), - ns.Relation("parent", nil, ns.RelationReference("folder", "...")), + ns.Relation("can_comment", nil, ns.AllowedRelation("user", "...")), + ns.Relation("parent", nil, ns.AllowedRelation("folder", "...")), ), }, "", }, + { + "transitive wildcard type check", + ns.Namespace( + "document", + ns.Relation("viewer", nil, ns.AllowedRelation("user", "..."), ns.AllowedRelation("group", "member")), + ), + []*v0.NamespaceDefinition{ + ns.Namespace("user"), + ns.Namespace( + "group", + ns.Relation("member", nil, ns.AllowedRelation("user", "..."), ns.AllowedPublicNamespace("user")), + ), + }, + "for relation `viewer`: relation/permission `group#member` includes wildcard (public) type `user`: wildcard relations cannot be transitively included", + }, + { + "ttu wildcard type check", + ns.Namespace( + "folder", + ns.Relation("parent", nil, ns.AllowedRelation("folder", "..."), ns.AllowedPublicNamespace("folder")), + ns.Relation("viewer", ns.Union( + ns.TupleToUserset("parent", "viewer"), + )), + ), + []*v0.NamespaceDefinition{ + ns.Namespace("user"), + }, + "for arrow under relation `viewer`: relation `folder#parent` includes wildcard (public) type `folder`: wildcard relations cannot be used on the left side of arrows", + }, } for _, tc := range testCases { diff --git a/internal/services/v0/namespace_test.go b/internal/services/v0/namespace_test.go index e0760a25bf..afc7ec67fa 100644 --- a/internal/services/v0/namespace_test.go +++ b/internal/services/v0/namespace_test.go @@ -67,7 +67,7 @@ func TestNamespaceChanged(t *testing.T) { "folder", ns.Relation("viewer", nil, - ns.RelationReference("user", "..."), + ns.AllowedRelation("user", "..."), ), ), []*v0.RelationTuple{}, @@ -82,7 +82,7 @@ func TestNamespaceChanged(t *testing.T) { "folder", ns.Relation("viewer", nil, - ns.RelationReference("user", "..."), + ns.AllowedRelation("user", "..."), ), ), []*v0.RelationTuple{tuple.MustParse("folder:somefolder#viewer@user:someuser#...")}, @@ -97,11 +97,11 @@ func TestNamespaceChanged(t *testing.T) { "folder", ns.Relation("anotherrel", nil, - ns.RelationReference("folder", "..."), + ns.AllowedRelation("folder", "..."), ), ns.Relation("viewer", nil, - ns.RelationReference("user", "..."), + ns.AllowedRelation("user", "..."), ), ), []*v0.RelationTuple{tuple.MustParse("folder:somefolder#anotherrel@folder:somefolder#viewer")}, @@ -109,7 +109,7 @@ func TestNamespaceChanged(t *testing.T) { "folder", ns.Relation("anotherrel", nil, - ns.RelationReference("folder", "..."), + ns.AllowedRelation("folder", "..."), ), ), "cannot delete relation `viewer` in definition `folder`, as a relationship references it", @@ -120,7 +120,7 @@ func TestNamespaceChanged(t *testing.T) { "folder", ns.Relation("viewer", nil, - ns.RelationReference("user", "..."), + ns.AllowedRelation("user", "..."), ), ), []*v0.RelationTuple{tuple.MustParse("folder:somefolder#viewer@user:someuser#...")}, @@ -128,7 +128,7 @@ func TestNamespaceChanged(t *testing.T) { "folder", ns.Relation("viewer", nil, - ns.RelationReference("folder", "..."), + ns.AllowedRelation("folder", "..."), ), ), "cannot remove allowed relation/permission `user#...` from relation `viewer` in definition `folder`, as a relationship exists with it", @@ -139,7 +139,7 @@ func TestNamespaceChanged(t *testing.T) { "folder", ns.Relation("viewer", nil, - ns.RelationReference("user", "..."), + ns.AllowedRelation("user", "..."), ), ), []*v0.RelationTuple{}, @@ -147,7 +147,7 @@ func TestNamespaceChanged(t *testing.T) { "folder", ns.Relation("viewer", nil, - ns.RelationReference("folder", "..."), + ns.AllowedRelation("folder", "..."), ), ), "", @@ -158,7 +158,7 @@ func TestNamespaceChanged(t *testing.T) { "folder", ns.Relation("viewer", nil, - ns.RelationReference("user", "..."), + ns.AllowedRelation("user", "..."), ), ), []*v0.RelationTuple{tuple.MustParse("folder:somefolder#viewer@user:someuser#...")}, @@ -166,8 +166,8 @@ func TestNamespaceChanged(t *testing.T) { "folder", ns.Relation("viewer", nil, - ns.RelationReference("user", "..."), - ns.RelationReference("folder", "..."), + ns.AllowedRelation("user", "..."), + ns.AllowedRelation("folder", "..."), ), ), "", @@ -236,7 +236,7 @@ func TestDeleteNamespace(t *testing.T) { "folder", ns.Relation("viewer", nil, - ns.RelationReference("user", "..."), + ns.AllowedRelation("user", "..."), ), ), []string{"folder", "user"}, @@ -249,7 +249,7 @@ func TestDeleteNamespace(t *testing.T) { "folder", ns.Relation("viewer", nil, - ns.RelationReference("user", "..."), + ns.AllowedRelation("user", "..."), ), ), []string{"folder"}, @@ -264,7 +264,7 @@ func TestDeleteNamespace(t *testing.T) { "folder", ns.Relation("viewer", nil, - ns.RelationReference("user", "..."), + ns.AllowedRelation("user", "..."), ), ), []string{"user"}, diff --git a/internal/testfixtures/datastore.go b/internal/testfixtures/datastore.go index 8bdeffc218..63a693c157 100644 --- a/internal/testfixtures/datastore.go +++ b/internal/testfixtures/datastore.go @@ -18,16 +18,16 @@ var DocumentNS = ns.Namespace( "document", ns.Relation("owner", nil, - ns.RelationReference("user", "..."), + ns.AllowedRelation("user", "..."), ), ns.Relation("editor", ns.Union( ns.This(), ns.ComputedUserset("owner"), ), - ns.RelationReference("user", "..."), + ns.AllowedRelation("user", "..."), ), - ns.Relation("parent", nil, ns.RelationReference("folder", "...")), + ns.Relation("parent", nil, ns.AllowedRelation("folder", "...")), ns.Relation("lock", nil), ns.Relation("viewer", ns.Union( @@ -35,21 +35,21 @@ var DocumentNS = ns.Namespace( ns.ComputedUserset("editor"), ns.TupleToUserset("parent", "viewer"), ), - ns.RelationReference("user", "..."), + ns.AllowedRelation("user", "..."), ), ns.Relation("viewer_and_editor", ns.Intersection( ns.This(), ns.ComputedUserset("editor"), ), - ns.RelationReference("user", "..."), + ns.AllowedRelation("user", "..."), ), ns.Relation("viewer_and_editor_derived", ns.Union( ns.This(), ns.ComputedUserset("viewer_and_editor"), ), - ns.RelationReference("user", "..."), + ns.AllowedRelation("user", "..."), ), ) @@ -57,15 +57,15 @@ var FolderNS = ns.Namespace( "folder", ns.Relation("owner", nil, - ns.RelationReference("user", "..."), + ns.AllowedRelation("user", "..."), ), - ns.Relation("parent", nil, ns.RelationReference("folder", "...")), + ns.Relation("parent", nil, ns.AllowedRelation("folder", "...")), ns.Relation("editor", ns.Union( ns.This(), ns.ComputedUserset("owner"), ), - ns.RelationReference("user", "..."), + ns.AllowedRelation("user", "..."), ), ns.Relation("viewer", ns.Union( @@ -73,8 +73,8 @@ var FolderNS = ns.Namespace( ns.ComputedUserset("editor"), ns.TupleToUserset("parent", "viewer"), ), - ns.RelationReference("user", "..."), - ns.RelationReference("folder", "viewer"), + ns.AllowedRelation("user", "..."), + ns.AllowedRelation("folder", "viewer"), ), ) diff --git a/pkg/namespace/builder.go b/pkg/namespace/builder.go index 9377aee42d..298a207a8d 100644 --- a/pkg/namespace/builder.go +++ b/pkg/namespace/builder.go @@ -22,7 +22,7 @@ func NamespaceWithComment(name string, comment string, relations ...*v0.Relation } // Relation creates a relation definition with an optional rewrite definition. -func Relation(name string, rewrite *v0.UsersetRewrite, allowedDirectRelations ...*v0.RelationReference) *v0.Relation { +func Relation(name string, rewrite *v0.UsersetRewrite, allowedDirectRelations ...*v0.AllowedRelation) *v0.Relation { var typeInfo *v0.TypeInformation if len(allowedDirectRelations) > 0 { typeInfo = &v0.TypeInformation{ @@ -57,12 +57,32 @@ func Relation(name string, rewrite *v0.UsersetRewrite, allowedDirectRelations .. } // RelationWithComment creates a relation definition with an optional rewrite definition. -func RelationWithComment(name string, comment string, rewrite *v0.UsersetRewrite, allowedDirectRelations ...*v0.RelationReference) *v0.Relation { +func RelationWithComment(name string, comment string, rewrite *v0.UsersetRewrite, allowedDirectRelations ...*v0.AllowedRelation) *v0.Relation { rel := Relation(name, rewrite, allowedDirectRelations...) rel.Metadata, _ = AddComment(rel.Metadata, comment) return rel } +// AllowedRelation creates a relation reference to an allowed relation. +func AllowedRelation(namespaceName string, relationName string) *v0.AllowedRelation { + return &v0.AllowedRelation{ + Namespace: namespaceName, + RelationOrWildcard: &v0.AllowedRelation_Relation{ + Relation: relationName, + }, + } +} + +// AllowedPublicNamespace creates a relation reference to an allowed public namespace. +func AllowedPublicNamespace(namespaceName string) *v0.AllowedRelation { + return &v0.AllowedRelation{ + Namespace: namespaceName, + RelationOrWildcard: &v0.AllowedRelation_PublicWildcard_{ + PublicWildcard: &v0.AllowedRelation_PublicWildcard{}, + }, + } +} + // RelationReference creates a relation reference. func RelationReference(namespaceName string, relationName string) *v0.RelationReference { return &v0.RelationReference{ diff --git a/pkg/schemadsl/compiler/compiler_test.go b/pkg/schemadsl/compiler/compiler_test.go index 4c2ff42c31..4b97215a77 100644 --- a/pkg/schemadsl/compiler/compiler_test.go +++ b/pkg/schemadsl/compiler/compiler_test.go @@ -64,7 +64,7 @@ func TestCompile(t *testing.T) { []*v0.NamespaceDefinition{ namespace.Namespace("sometenant/simple", namespace.Relation("foos", nil, - namespace.RelationReference("sometenant/bars", "..."), + namespace.AllowedRelation("sometenant/bars", "..."), ), ), }, @@ -79,7 +79,22 @@ func TestCompile(t *testing.T) { []*v0.NamespaceDefinition{ namespace.Namespace("sometenant/simple", namespace.Relation("foos", nil, - namespace.RelationReference("sometenant/bars", "mehs"), + namespace.AllowedRelation("sometenant/bars", "mehs"), + ), + ), + }, + }, + { + "wildcard relation", + &someTenant, + `definition simple { + relation foos: bars:* + }`, + "", + []*v0.NamespaceDefinition{ + namespace.Namespace("sometenant/simple", + namespace.Relation("foos", nil, + namespace.AllowedPublicNamespace("sometenant/bars"), ), ), }, @@ -94,7 +109,7 @@ func TestCompile(t *testing.T) { []*v0.NamespaceDefinition{ namespace.Namespace("sometenant/simple", namespace.Relation("foos", nil, - namespace.RelationReference("anothertenant/bars", "mehs"), + namespace.AllowedRelation("anothertenant/bars", "mehs"), ), ), }, @@ -110,11 +125,11 @@ func TestCompile(t *testing.T) { []*v0.NamespaceDefinition{ namespace.Namespace("sometenant/simple", namespace.Relation("foos", nil, - namespace.RelationReference("sometenant/bars", "mehs"), + namespace.AllowedRelation("sometenant/bars", "mehs"), ), namespace.Relation("hello", nil, - namespace.RelationReference("sometenant/there", "..."), - namespace.RelationReference("sometenant/world", "..."), + namespace.AllowedRelation("sometenant/there", "..."), + namespace.AllowedRelation("sometenant/world", "..."), ), ), }, @@ -423,7 +438,7 @@ func TestCompile(t *testing.T) { namespace.Relation( "somerel", nil, - namespace.RelationReference("some_tenant/bars", "..."), + namespace.AllowedRelation("some_tenant/bars", "..."), ), ), }, From 607de2d3db4b86e67a3fe560b540dc776d247cc9 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 10 Dec 2021 16:42:52 -0500 Subject: [PATCH 05/11] Implement support for wildcards in the schema package --- pkg/schemadsl/compiler/translator.go | 42 +++++++--- pkg/schemadsl/dslshape/dslshape.go | 3 + pkg/schemadsl/generator/generator.go | 15 ++-- pkg/schemadsl/generator/generator_test.go | 29 ++----- pkg/schemadsl/lexer/lex_def.go | 6 ++ pkg/schemadsl/lexer/lex_test.go | 22 ++++++ pkg/schemadsl/lexer/tokentype_string.go | 5 +- pkg/schemadsl/parser/parser.go | 13 +++- pkg/schemadsl/parser/parser_test.go | 2 + pkg/schemadsl/parser/tests/brokenwildcard.zed | 6 ++ .../parser/tests/brokenwildcard.zed.expected | 59 ++++++++++++++ pkg/schemadsl/parser/tests/wildcard.zed | 7 ++ .../parser/tests/wildcard.zed.expected | 76 +++++++++++++++++++ 13 files changed, 242 insertions(+), 43 deletions(-) create mode 100644 pkg/schemadsl/parser/tests/brokenwildcard.zed create mode 100644 pkg/schemadsl/parser/tests/brokenwildcard.zed.expected create mode 100644 pkg/schemadsl/parser/tests/wildcard.zed create mode 100644 pkg/schemadsl/parser/tests/wildcard.zed.expected diff --git a/pkg/schemadsl/compiler/translator.go b/pkg/schemadsl/compiler/translator.go index 1f78a4fed9..cca7445400 100644 --- a/pkg/schemadsl/compiler/translator.go +++ b/pkg/schemadsl/compiler/translator.go @@ -148,14 +148,14 @@ func translateRelation(tctx translationContext, relationNode *dslNode) (*v0.Rela return nil, relationNode.Errorf("invalid relation name: %w", err) } - allowedDirectTypes := []*v0.RelationReference{} + allowedDirectTypes := []*v0.AllowedRelation{} for _, typeRef := range relationNode.List(dslshape.NodeRelationPredicateAllowedTypes) { - relReferences, err := translateTypeReference(tctx, typeRef) + allowedRelations, err := translateAllowedRelations(tctx, typeRef) if err != nil { return nil, err } - allowedDirectTypes = append(allowedDirectTypes, relReferences...) + allowedDirectTypes = append(allowedDirectTypes, allowedRelations...) } relation := namespace.Relation(relationName, nil, allowedDirectTypes...) @@ -304,14 +304,14 @@ func translateExpressionOperation(tctx translationContext, expressionOpNode *dsl } } -func translateTypeReference(tctx translationContext, typeRefNode *dslNode) ([]*v0.RelationReference, error) { +func translateAllowedRelations(tctx translationContext, typeRefNode *dslNode) ([]*v0.AllowedRelation, error) { switch typeRefNode.GetType() { case dslshape.NodeTypeTypeReference: - references := []*v0.RelationReference{} + references := []*v0.AllowedRelation{} for _, subRefNode := range typeRefNode.List(dslshape.NodeTypeReferencePredicateType) { - subReferences, err := translateTypeReference(tctx, subRefNode) + subReferences, err := translateAllowedRelations(tctx, subRefNode) if err != nil { - return []*v0.RelationReference{}, err + return []*v0.AllowedRelation{}, err } references = append(references, subReferences...) @@ -321,16 +321,16 @@ func translateTypeReference(tctx translationContext, typeRefNode *dslNode) ([]*v case dslshape.NodeTypeSpecificTypeReference: ref, err := translateSpecificTypeReference(tctx, typeRefNode) if err != nil { - return []*v0.RelationReference{}, err + return []*v0.AllowedRelation{}, err } - return []*v0.RelationReference{ref}, nil + return []*v0.AllowedRelation{ref}, nil default: return nil, typeRefNode.Errorf("unknown type ref node type %s", typeRefNode.GetType()) } } -func translateSpecificTypeReference(tctx translationContext, typeRefNode *dslNode) (*v0.RelationReference, error) { +func translateSpecificTypeReference(tctx translationContext, typeRefNode *dslNode) (*v0.AllowedRelation, error) { typePath, err := typeRefNode.GetString(dslshape.NodeSpecificReferencePredicateType) if err != nil { return nil, typeRefNode.Errorf("invalid type name: %w", err) @@ -341,6 +341,22 @@ func translateSpecificTypeReference(tctx translationContext, typeRefNode *dslNod return nil, typeRefNode.Errorf("%w", err) } + if typeRefNode.Has(dslshape.NodeSpecificReferencePredicateWildcard) { + ref := &v0.AllowedRelation{ + Namespace: nspath, + RelationOrWildcard: &v0.AllowedRelation_PublicWildcard_{ + PublicWildcard: &v0.AllowedRelation_PublicWildcard{}, + }, + } + + err = ref.Validate() + if err != nil { + return nil, typeRefNode.Errorf("invalid type relation: %w", err) + } + + return ref, nil + } + relationName := Ellipsis if typeRefNode.Has(dslshape.NodeSpecificReferencePredicateRelation) { relationName, err = typeRefNode.GetString(dslshape.NodeSpecificReferencePredicateRelation) @@ -349,9 +365,11 @@ func translateSpecificTypeReference(tctx translationContext, typeRefNode *dslNod } } - ref := &v0.RelationReference{ + ref := &v0.AllowedRelation{ Namespace: nspath, - Relation: relationName, + RelationOrWildcard: &v0.AllowedRelation_Relation{ + Relation: relationName, + }, } err = ref.Validate() diff --git a/pkg/schemadsl/dslshape/dslshape.go b/pkg/schemadsl/dslshape/dslshape.go index 0da1622a8e..53657bc7d7 100644 --- a/pkg/schemadsl/dslshape/dslshape.go +++ b/pkg/schemadsl/dslshape/dslshape.go @@ -98,6 +98,9 @@ const ( // A relation under a type reference. NodeSpecificReferencePredicateRelation = "relation-name" + // A wildcard under a type reference. + NodeSpecificReferencePredicateWildcard = "type-wildcard" + // // NodeTypePermission // diff --git a/pkg/schemadsl/generator/generator.go b/pkg/schemadsl/generator/generator.go index 66816b08c7..4ace77b4d8 100644 --- a/pkg/schemadsl/generator/generator.go +++ b/pkg/schemadsl/generator/generator.go @@ -70,12 +70,12 @@ func (sg *sourceGenerator) emitRelation(relation *v0.Relation) { if relation.TypeInformation == nil || relation.TypeInformation.AllowedDirectRelations == nil || len(relation.TypeInformation.AllowedDirectRelations) == 0 { sg.appendIssue("missing allowed types") } else { - for index, relationRef := range relation.TypeInformation.AllowedDirectRelations { + for index, allowedRelation := range relation.TypeInformation.AllowedDirectRelations { if index > 0 { sg.append(" | ") } - sg.emitRelationReference(relationRef) + sg.emitAllowedRelation(allowedRelation) } } } @@ -88,11 +88,14 @@ func (sg *sourceGenerator) emitRelation(relation *v0.Relation) { sg.appendLine() } -func (sg *sourceGenerator) emitRelationReference(relationReference *v0.RelationReference) { - sg.append(relationReference.Namespace) - if relationReference.Relation != Ellipsis { +func (sg *sourceGenerator) emitAllowedRelation(allowedRelation *v0.AllowedRelation) { + sg.append(allowedRelation.Namespace) + if allowedRelation.GetRelation() != "" && allowedRelation.GetRelation() != Ellipsis { sg.append("#") - sg.append(relationReference.Relation) + sg.append(allowedRelation.GetRelation()) + } + if allowedRelation.GetPublicWildcard() != nil { + sg.append(":*") } } diff --git a/pkg/schemadsl/generator/generator_test.go b/pkg/schemadsl/generator/generator_test.go index 8244f7296e..feb3780f7d 100644 --- a/pkg/schemadsl/generator/generator_test.go +++ b/pkg/schemadsl/generator/generator_test.go @@ -29,10 +29,7 @@ func TestGenerator(t *testing.T) { { "simple relation", namespace.Namespace("foos/test", - namespace.Relation("somerel", nil, &v0.RelationReference{ - Namespace: "foos/bars", - Relation: "hiya", - }), + namespace.Relation("somerel", nil, namespace.AllowedRelation("foos/bars", "hiya")), ), `definition foos/test { relation somerel: foos/bars#hiya @@ -76,10 +73,7 @@ func TestGenerator(t *testing.T) { namespace.Relation("somerel", namespace.Union( namespace.This(), namespace.ComputedUserset("anotherrel"), - ), &v0.RelationReference{ - Namespace: "foos/bars", - Relation: "hiya", - }), + ), namespace.AllowedRelation("foos/bars", "hiya")), ), `definition foos/test { relation somerel: foos/bars#hiya = /* _this unsupported here. Please rewrite into a relation and permission */ + anotherrel @@ -103,20 +97,11 @@ func TestGenerator(t *testing.T) { * Some comment goes here */`, namespace.Relation("owner", nil, - &v0.RelationReference{ - Namespace: "foos/user", - Relation: "...", - }, + namespace.AllowedRelation("foos/user", "..."), ), namespace.RelationWithComment("reader", "//foobar", nil, - &v0.RelationReference{ - Namespace: "foos/user", - Relation: "...", - }, - &v0.RelationReference{ - Namespace: "foos/group", - Relation: "member", - }, + namespace.AllowedRelation("foos/user", "..."), + namespace.AllowedRelation("foos/group", "member"), ), namespace.Relation("read", namespace.Union( namespace.ComputedUserset("reader"), @@ -231,7 +216,7 @@ definition foos/test { /** the document */ definition foos/document { /** some super long comment goes here and therefore should be made into a multiline comment */ - relation reader: foos/user + relation reader: foos/user | foos/user:* /** multiline comment */ @@ -248,7 +233,7 @@ definition foos/document { /** * some super long comment goes here and therefore should be made into a multiline comment */ - relation reader: foos/user + relation reader: foos/user | foos/user:* /** * multiline diff --git a/pkg/schemadsl/lexer/lex_def.go b/pkg/schemadsl/lexer/lex_def.go index 3341c63bb2..117291b65b 100644 --- a/pkg/schemadsl/lexer/lex_def.go +++ b/pkg/schemadsl/lexer/lex_def.go @@ -49,6 +49,7 @@ const ( TokenTypeRightArrow // -> TokenTypeHash // # TokenTypeEllipsis // ... + TokenTypeStar // * ) // keywords contains the full set of keywords supported. @@ -66,6 +67,8 @@ var syntheticPredecessors = map[TokenType]bool{ TokenTypeRightBrace: true, TokenTypeRightParen: true, + + TokenTypeStar: true, } // lexerEntrypoint scans until EOFRUNE @@ -109,6 +112,9 @@ Loop: case r == '#': l.emit(TokenTypeHash) + case r == '*': + l.emit(TokenTypeStar) + case r == '.': if l.acceptString("..") { l.emit(TokenTypeEllipsis) diff --git a/pkg/schemadsl/lexer/lex_test.go b/pkg/schemadsl/lexer/lex_test.go index e431348e4d..15049a4ea1 100644 --- a/pkg/schemadsl/lexer/lex_test.go +++ b/pkg/schemadsl/lexer/lex_test.go @@ -39,6 +39,7 @@ var lexerTests = []lexerTest{ {"right paren", ")", []Lexeme{{TokenTypeRightParen, 0, ")"}, tEOF}}, {"semicolon", ";", []Lexeme{{TokenTypeSemicolon, 0, ";"}, tEOF}}, + {"star", "*", []Lexeme{{TokenTypeStar, 0, "*"}, tEOF}}, {"right arrow", "->", []Lexeme{{TokenTypeRightArrow, 0, "->"}, tEOF}}, @@ -64,6 +65,13 @@ var lexerTests = []lexerTest{ tEOF, }}, + {"type star", "foo:*", []Lexeme{ + {TokenTypeIdentifier, 0, "foo"}, + {TokenTypeColon, 0, ":"}, + {TokenTypeStar, 0, "*"}, + tEOF, + }}, + {"expression", "foo->bar", []Lexeme{ {TokenTypeIdentifier, 0, "foo"}, {TokenTypeRightArrow, 0, "->"}, @@ -103,6 +111,20 @@ var lexerTests = []lexerTest{ tEOF, }}, + {"relation", "/* foo */relation parent: namespace:*\n", []Lexeme{ + {TokenTypeMultilineComment, 0, "/* foo */"}, + {TokenTypeKeyword, 0, "relation"}, + tWhitespace, + {TokenTypeIdentifier, 0, "parent"}, + {TokenTypeColon, 0, ":"}, + tWhitespace, + {TokenTypeIdentifier, 0, "namespace"}, + {TokenTypeColon, 0, ":"}, + {TokenTypeStar, 0, "*"}, + {TokenTypeSyntheticSemicolon, 0, "\n"}, + tEOF, + }}, + {"expression with parens", "(foo->bar)\n", []Lexeme{ {TokenTypeLeftParen, 0, "("}, {TokenTypeIdentifier, 0, "foo"}, diff --git a/pkg/schemadsl/lexer/tokentype_string.go b/pkg/schemadsl/lexer/tokentype_string.go index 9faaebac70..5d195f462d 100644 --- a/pkg/schemadsl/lexer/tokentype_string.go +++ b/pkg/schemadsl/lexer/tokentype_string.go @@ -33,11 +33,12 @@ func _() { _ = x[TokenTypeRightArrow-22] _ = x[TokenTypeHash-23] _ = x[TokenTypeEllipsis-24] + _ = x[TokenTypeStar-25] } -const _TokenType_name = "TokenTypeErrorTokenTypeSyntheticSemicolonTokenTypeEOFTokenTypeWhitespaceTokenTypeSinglelineCommentTokenTypeMultilineCommentTokenTypeNewlineTokenTypeKeywordTokenTypeIdentifierTokenTypeNumberTokenTypeLeftBraceTokenTypeRightBraceTokenTypeLeftParenTokenTypeRightParenTokenTypePipeTokenTypePlusTokenTypeMinusTokenTypeAndTokenTypeDivTokenTypeEqualsTokenTypeColonTokenTypeSemicolonTokenTypeRightArrowTokenTypeHashTokenTypeEllipsis" +const _TokenType_name = "TokenTypeErrorTokenTypeSyntheticSemicolonTokenTypeEOFTokenTypeWhitespaceTokenTypeSinglelineCommentTokenTypeMultilineCommentTokenTypeNewlineTokenTypeKeywordTokenTypeIdentifierTokenTypeNumberTokenTypeLeftBraceTokenTypeRightBraceTokenTypeLeftParenTokenTypeRightParenTokenTypePipeTokenTypePlusTokenTypeMinusTokenTypeAndTokenTypeDivTokenTypeEqualsTokenTypeColonTokenTypeSemicolonTokenTypeRightArrowTokenTypeHashTokenTypeEllipsisTokenTypeStar" -var _TokenType_index = [...]uint16{0, 14, 41, 53, 72, 98, 123, 139, 155, 174, 189, 207, 226, 244, 263, 276, 289, 303, 315, 327, 342, 356, 374, 393, 406, 423} +var _TokenType_index = [...]uint16{0, 14, 41, 53, 72, 98, 123, 139, 155, 174, 189, 207, 226, 244, 263, 276, 289, 303, 315, 327, 342, 356, 374, 393, 406, 423, 436} func (i TokenType) String() string { if i < 0 || i >= TokenType(len(_TokenType_index)-1) { diff --git a/pkg/schemadsl/parser/parser.go b/pkg/schemadsl/parser/parser.go index 1c3259be6d..d661c976f4 100644 --- a/pkg/schemadsl/parser/parser.go +++ b/pkg/schemadsl/parser/parser.go @@ -142,7 +142,7 @@ func (p *sourceParser) consumeRelation() AstNode { } // consumeTypeReference consumes a reference to a type or types of relations. -// ```somerel | anotherrel``` +// ```sometype | anothertype | anothertype:* ``` func (p *sourceParser) consumeTypeReference() AstNode { refNode := p.startNode(dslshape.NodeTypeTypeReference) defer p.finishNode() @@ -169,6 +169,17 @@ func (p *sourceParser) consumeSpecificType() AstNode { specificNode.Decorate(dslshape.NodeSpecificReferencePredicateType, typeName) + // Check for a wildcard + if _, ok := p.tryConsume(lexer.TokenTypeColon); ok { + _, ok := p.consume(lexer.TokenTypeStar) + if !ok { + return specificNode + } + + specificNode.Decorate(dslshape.NodeSpecificReferencePredicateWildcard, "true") + return specificNode + } + // Check for a relation specified. if _, ok := p.tryConsume(lexer.TokenTypeHash); !ok { return specificNode diff --git a/pkg/schemadsl/parser/parser_test.go b/pkg/schemadsl/parser/parser_test.go index c359c34bf9..c9f2089a79 100644 --- a/pkg/schemadsl/parser/parser_test.go +++ b/pkg/schemadsl/parser/parser_test.go @@ -106,6 +106,8 @@ func TestParser(t *testing.T) { {"indented comments test", "indentedcomments"}, {"parens test", "parens"}, {"multiple parens test", "multiparen"}, + {"wildcard test", "wildcard"}, + {"broken wildcard test", "brokenwildcard"}, } for _, test := range parserTests { diff --git a/pkg/schemadsl/parser/tests/brokenwildcard.zed b/pkg/schemadsl/parser/tests/brokenwildcard.zed new file mode 100644 index 0000000000..bbd464c2a0 --- /dev/null +++ b/pkg/schemadsl/parser/tests/brokenwildcard.zed @@ -0,0 +1,6 @@ +definition user {} + +definition resource { + relation viewer: user | user: | anothertype + permission view = viewer +} \ No newline at end of file diff --git a/pkg/schemadsl/parser/tests/brokenwildcard.zed.expected b/pkg/schemadsl/parser/tests/brokenwildcard.zed.expected new file mode 100644 index 0000000000..61ed6f1c02 --- /dev/null +++ b/pkg/schemadsl/parser/tests/brokenwildcard.zed.expected @@ -0,0 +1,59 @@ +NodeTypeFile + end-rune = 119 + input-source = broken wildcard test + start-rune = 0 + child-node => + NodeTypeDefinition + definition-name = user + end-rune = 17 + input-source = broken wildcard test + start-rune = 0 + NodeTypeDefinition + definition-name = resource + end-rune = 119 + input-source = broken wildcard test + start-rune = 20 + child-node => + NodeTypeRelation + end-rune = 88 + input-source = broken wildcard test + relation-name = viewer + start-rune = 46 + allowed-types => + NodeTypeTypeReference + end-rune = 88 + input-source = broken wildcard test + start-rune = 63 + type-ref-type => + NodeTypeSpecificTypeReference + end-rune = 66 + input-source = broken wildcard test + start-rune = 63 + type-name = user + NodeTypeSpecificTypeReference + end-rune = 74 + input-source = broken wildcard test + start-rune = 70 + type-name = user + child-node => + NodeTypeError + end-rune = 74 + error-message = Expected one of: [TokenTypeStar], found: TokenTypePipe + input-source = broken wildcard test + start-rune = 76 + NodeTypeSpecificTypeReference + end-rune = 88 + input-source = broken wildcard test + start-rune = 78 + type-name = anothertype + NodeTypePermission + end-rune = 117 + input-source = broken wildcard test + relation-name = view + start-rune = 94 + compute-expression => + NodeTypeIdentifier + end-rune = 117 + identifier-value = viewer + input-source = broken wildcard test + start-rune = 112 \ No newline at end of file diff --git a/pkg/schemadsl/parser/tests/wildcard.zed b/pkg/schemadsl/parser/tests/wildcard.zed new file mode 100644 index 0000000000..04d84dbfca --- /dev/null +++ b/pkg/schemadsl/parser/tests/wildcard.zed @@ -0,0 +1,7 @@ +definition user {} + +definition resource { + relation viewer: user | user:* | anothertype + relation another: user | user:* + permission view = viewer +} \ No newline at end of file diff --git a/pkg/schemadsl/parser/tests/wildcard.zed.expected b/pkg/schemadsl/parser/tests/wildcard.zed.expected new file mode 100644 index 0000000000..b158bde0d6 --- /dev/null +++ b/pkg/schemadsl/parser/tests/wildcard.zed.expected @@ -0,0 +1,76 @@ +NodeTypeFile + end-rune = 156 + input-source = wildcard test + start-rune = 0 + child-node => + NodeTypeDefinition + definition-name = user + end-rune = 17 + input-source = wildcard test + start-rune = 0 + NodeTypeDefinition + definition-name = resource + end-rune = 156 + input-source = wildcard test + start-rune = 20 + child-node => + NodeTypeRelation + end-rune = 89 + input-source = wildcard test + relation-name = viewer + start-rune = 46 + allowed-types => + NodeTypeTypeReference + end-rune = 89 + input-source = wildcard test + start-rune = 63 + type-ref-type => + NodeTypeSpecificTypeReference + end-rune = 66 + input-source = wildcard test + start-rune = 63 + type-name = user + NodeTypeSpecificTypeReference + end-rune = 75 + input-source = wildcard test + start-rune = 70 + type-name = user + type-wildcard = true + NodeTypeSpecificTypeReference + end-rune = 89 + input-source = wildcard test + start-rune = 79 + type-name = anothertype + NodeTypeRelation + end-rune = 125 + input-source = wildcard test + relation-name = another + start-rune = 95 + allowed-types => + NodeTypeTypeReference + end-rune = 125 + input-source = wildcard test + start-rune = 113 + type-ref-type => + NodeTypeSpecificTypeReference + end-rune = 116 + input-source = wildcard test + start-rune = 113 + type-name = user + NodeTypeSpecificTypeReference + end-rune = 125 + input-source = wildcard test + start-rune = 120 + type-name = user + type-wildcard = true + NodeTypePermission + end-rune = 154 + input-source = wildcard test + relation-name = view + start-rune = 131 + compute-expression => + NodeTypeIdentifier + end-rune = 154 + identifier-value = viewer + input-source = wildcard test + start-rune = 149 \ No newline at end of file From c30bcfedc370f2e680c0f62b2a1a92e31b95cbdf Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 10 Dec 2021 16:43:16 -0500 Subject: [PATCH 06/11] Add assertions support to consistency tests --- internal/services/consistency_test.go | 27 +++++++++++++++++++ .../services/testconfigs/teamwitharrow.yaml | 6 +++++ pkg/validationfile/fileformat.go | 16 +++++++++-- pkg/validationfile/loader.go | 8 +++++- 4 files changed, 54 insertions(+), 3 deletions(-) diff --git a/internal/services/consistency_test.go b/internal/services/consistency_test.go index 978c5a637b..7425476465 100644 --- a/internal/services/consistency_test.go +++ b/internal/services/consistency_test.go @@ -111,6 +111,7 @@ func TestConsistency(t *testing.T) { for _, tester := range testers { t.Run(tester.Name(), func(t *testing.T) { runConsistencyTests(t, tester, dispatcher, fullyResolved, tuplesPerNamespace, revision) + runAssertions(t, tester, dispatcher, fullyResolved, revision) }) } }) @@ -121,6 +122,32 @@ func TestConsistency(t *testing.T) { } } +func runAssertions(t *testing.T, + tester serviceTester, + dispatch dispatch.Dispatcher, + fullyResolved *validationfile.FullyParsedValidationFile, + revision decimal.Decimal) { + for _, parsedFile := range fullyResolved.ParsedFiles { + for _, assertTrueRel := range parsedFile.Assertions.AssertTrue { + rel := tuple.Parse(assertTrueRel) + require.NotNil(t, rel) + + result, err := tester.Check(context.Background(), rel.ObjectAndRelation, rel.User.GetUserset(), revision) + require.NoError(t, err) + require.True(t, result, "Assertion `%s` returned false; true expected", tuple.String(rel)) + } + + for _, assertFalseRel := range parsedFile.Assertions.AssertFalse { + rel := tuple.Parse(assertFalseRel) + require.NotNil(t, rel) + + result, err := tester.Check(context.Background(), rel.ObjectAndRelation, rel.User.GetUserset(), revision) + require.NoError(t, err) + require.False(t, result, "Assertion `%s` returned true; false expected", tuple.String(rel)) + } + } +} + func runCrossVersionTests(t *testing.T, testers []serviceTester, dispatch dispatch.Dispatcher, diff --git a/internal/services/testconfigs/teamwitharrow.yaml b/internal/services/testconfigs/teamwitharrow.yaml index f269f62d3e..2dfc8e47c5 100644 --- a/internal/services/testconfigs/teamwitharrow.yaml +++ b/internal/services/testconfigs/teamwitharrow.yaml @@ -77,3 +77,9 @@ relationships: | test/team:support_engineers#parent@test/organization:authzed#... test/team:emea_support_engineers#direct_member@test/user:iona#... test/team:emea_support_engineers#parent@test/team:support_engineers#... +assertions: + assertTrue: + - "test/repository:authzed_go#read@test/user:jake#..." + - "test/repository:authzed_go#read@test/user:jimmy#..." + assertFalse: + - "test/repository:authzed_go#admin@test/user:jake#..." diff --git a/pkg/validationfile/fileformat.go b/pkg/validationfile/fileformat.go index 17412d1f9e..7b4c68960c 100644 --- a/pkg/validationfile/fileformat.go +++ b/pkg/validationfile/fileformat.go @@ -15,8 +15,8 @@ import ( // ValidationFile represents the contents of a YAML validation file, as // exported by the playground. // -// NOTE: This struct does not contain the `assertions` or `validation` blocks produced -// by the playground, as they are currently unused in Go-side code. +// NOTE: This struct does not contain the `validation` block produced +// by the playground, as it is currently unused in Go-side code. // // Parsing for those blocks' *contents* can be found in this module, since they are parsed // by the developer API. @@ -35,6 +35,9 @@ type ValidationFile struct { // ValidationTuples are the validation tuples, in tuple string syntax. Optional if Relationships // are specified. ValidationTuples []string `yaml:"validation_tuples"` + + // Assertions are the (optional) assertions for the validation file. + Assertions SimpleAssertions `yaml:"assertions"` } // ErrorWithSource is an error that includes the source text and position information. @@ -199,6 +202,15 @@ func (vs ValidationString) ONRS() ([]*v0.ObjectAndRelation, *ErrorWithSource) { return onrs, nil } +// SimpleAssertions is a parsed assertions block. +type SimpleAssertions struct { + // AssertTrue is the set of relationships to assert true. + AssertTrue []string `yaml:"assertTrue"` + + // AssertFalse is the set of relationships to assert false. + AssertFalse []string `yaml:"assertFalse"` +} + // Assertion is an unparsed assertion. type Assertion struct { relationshipString string diff --git a/pkg/validationfile/loader.go b/pkg/validationfile/loader.go index 2dbb387a65..43e4b18b43 100644 --- a/pkg/validationfile/loader.go +++ b/pkg/validationfile/loader.go @@ -27,6 +27,9 @@ type FullyParsedValidationFile struct { // Tuples are the relation tuples defined in the validation file, either directly // or in the relationships block. Tuples []*v0.RelationTuple + + // ParsedFiles are the underlying parsed validation files. + ParsedFiles []ValidationFile } // PopulateFromFiles populates the given datastore with the namespaces and tuples found in @@ -35,6 +38,7 @@ func PopulateFromFiles(ds datastore.Datastore, filePaths []string) (*FullyParsed var revision decimal.Decimal nsDefs := []*v0.NamespaceDefinition{} tuples := []*v0.RelationTuple{} + files := []ValidationFile{} for _, filePath := range filePaths { fileContents, err := os.ReadFile(filePath) @@ -47,6 +51,8 @@ func PopulateFromFiles(ds datastore.Datastore, filePaths []string) (*FullyParsed return nil, decimal.Zero, fmt.Errorf("Error when parsing config file %s: %w", filePath, err) } + files = append(files, parsed) + // Parse the schema, if any. if parsed.Schema != "" { defs, err := compiler.Compile([]compiler.InputSchema{{ @@ -145,5 +151,5 @@ func PopulateFromFiles(ds datastore.Datastore, filePaths []string) (*FullyParsed revision = wrevision } - return &FullyParsedValidationFile{nsDefs, tuples}, revision, nil + return &FullyParsedValidationFile{nsDefs, tuples, files}, revision, nil } From e30d135cd4a281a561cdac53b7198f0af1f0a089 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 10 Dec 2021 16:43:47 -0500 Subject: [PATCH 07/11] Add additional validation to relationship writes in datastores --- internal/datastore/common/validation.go | 28 +++++++++++++++++++++++++ internal/datastore/crdb/tuple.go | 4 ++++ internal/datastore/memdb/tuple.go | 5 +++++ internal/datastore/postgres/tuple.go | 4 ++++ 4 files changed, 41 insertions(+) create mode 100644 internal/datastore/common/validation.go diff --git a/internal/datastore/common/validation.go b/internal/datastore/common/validation.go new file mode 100644 index 0000000000..7e18cbdc56 --- /dev/null +++ b/internal/datastore/common/validation.go @@ -0,0 +1,28 @@ +package common + +import ( + "fmt" + + v1 "github.com/authzed/authzed-go/proto/authzed/api/v1" + + "github.com/authzed/spicedb/pkg/tuple" +) + +// ValidateUpdatesToWrite performs basic validation on relationship updates going into datastores. +func ValidateUpdatesToWrite(updates []*v1.RelationshipUpdate) error { + for _, update := range updates { + err := update.HandwrittenValidate() + if err != nil { + return err + } + + if update.Relationship.Subject.Object.ObjectId == tuple.PublicWildcard && update.Relationship.Subject.OptionalRelation != "" { + return fmt.Errorf( + "Attempt to write a wildcard relationship (`%s`) with a non-empty relation. Please report this bug", + tuple.RelString(update.Relationship), + ) + } + } + + return nil +} diff --git a/internal/datastore/crdb/tuple.go b/internal/datastore/crdb/tuple.go index 3a5e253f19..4a2ba61c16 100644 --- a/internal/datastore/crdb/tuple.go +++ b/internal/datastore/crdb/tuple.go @@ -127,6 +127,10 @@ func (cds *crdbDatastore) WriteTuples(ctx context.Context, preconditions []*v1.P defer span.End() var nowRevision datastore.Revision + if err := common.ValidateUpdatesToWrite(mutations); err != nil { + return datastore.NoRevision, fmt.Errorf(errUnableToWriteTuples, err) + } + if err := cds.execute(ctx, cds.conn, pgx.TxOptions{}, func(tx pgx.Tx) error { keySet := newKeySet() if err := cds.checkPreconditions(ctx, tx, keySet, preconditions); err != nil { diff --git a/internal/datastore/memdb/tuple.go b/internal/datastore/memdb/tuple.go index 88c0cb2aff..cafac9430a 100644 --- a/internal/datastore/memdb/tuple.go +++ b/internal/datastore/memdb/tuple.go @@ -12,6 +12,7 @@ import ( "github.com/jzelinskie/stringz" "github.com/authzed/spicedb/internal/datastore" + "github.com/authzed/spicedb/internal/datastore/common" ) const ( @@ -59,6 +60,10 @@ func (mds *memdbDatastore) WriteTuples(ctx context.Context, preconditions []*v1. return datastore.NoRevision, fmt.Errorf(errUnableToWriteTuples, err) } + if err := common.ValidateUpdatesToWrite(mutations); err != nil { + return datastore.NoRevision, fmt.Errorf(errUnableToWriteTuples, err) + } + newChangelogID, err := mds.write(ctx, txn, mutations) if err != nil { return datastore.NoRevision, fmt.Errorf(errUnableToWriteTuples, err) diff --git a/internal/datastore/postgres/tuple.go b/internal/datastore/postgres/tuple.go index 3b337f87c7..202a05ac5c 100644 --- a/internal/datastore/postgres/tuple.go +++ b/internal/datastore/postgres/tuple.go @@ -98,6 +98,10 @@ func (pgd *pgDatastore) WriteTuples(ctx context.Context, preconditions []*v1.Pre ctx, span := tracer.Start(datastore.SeparateContextWithTracing(ctx), "WriteTuples") defer span.End() + if err := common.ValidateUpdatesToWrite(mutations); err != nil { + return datastore.NoRevision, fmt.Errorf(errUnableToWriteTuples, err) + } + tx, err := pgd.dbpool.Begin(ctx) if err != nil { return datastore.NoRevision, fmt.Errorf(errUnableToWriteTuples, err) From 2febca1147b0d064ee46c7e5f2bb29237cbc3e32 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 10 Dec 2021 16:44:21 -0500 Subject: [PATCH 08/11] Add in-progress wildcard support --- internal/services/testconfigs/public.yaml | 34 +++++++++++ internal/services/v0/acl_test.go | 7 +++ internal/services/v0/developer_test.go | 62 ++++++++++++++++++++ internal/services/v0/validation.go | 64 ++++++++++++++++---- internal/services/v1/relationships.go | 68 +++++++++++++++++----- internal/services/v1/relationships_test.go | 14 +++++ 6 files changed, 222 insertions(+), 27 deletions(-) create mode 100644 internal/services/testconfigs/public.yaml diff --git a/internal/services/testconfigs/public.yaml b/internal/services/testconfigs/public.yaml new file mode 100644 index 0000000000..84f21e3489 --- /dev/null +++ b/internal/services/testconfigs/public.yaml @@ -0,0 +1,34 @@ +--- +schema: >- + definition test/user {} + + definition test/anotherkindofuser {} + + definition test/resource { + relation editor: test/user + relation viewer: test/user | test/user:* | test/anotherkindofuser | test/anotherkindofuser:* + + permission view = viewer + editor + } +relationships: | + test/resource:first#editor@test/user:editordude + test/resource:first#viewer@test/user:somegal + test/resource:first#viewer@test/user:* + test/resource:first#viewer@test/anotherkindofuser:editordude +assertions: + assertTrue: + - "test/resource:first#view@test/user:editordude" + - "test/resource:first#editor@test/user:editordude" + - "test/resource:first#view@test/user:somegal" + - "test/resource:first#view@test/user:editordude" + # TODO(evan): Uncomment once wildcard dispatch is in + # - "test/resource:first#view@test/user:anotheruser" + # - "test/resource:first#view@test/user:aseconduser" + # - "test/resource:first#view@test/user:athirduser" + assertFalse: + - "test/resource:first#editor@test/user:somegal" + - "test/resource:first#editor@test/user:anotheruser" + - "test/resource:first#editor@test/anotherkindofuser:editordude" + - "test/resource:first#viewer@test/anotherkindofuser:anotheruser" + - "test/resource:first#viewer@test/anotherkindofuser:aseconduser" + - "test/resource:first#viewer@test/anotherkindofuser:athirduser" diff --git a/internal/services/v0/acl_test.go b/internal/services/v0/acl_test.go index 0e30f16338..43609aaf57 100644 --- a/internal/services/v0/acl_test.go +++ b/internal/services/v0/acl_test.go @@ -515,6 +515,13 @@ func TestCheck(t *testing.T) { {ONR("user", "", "..."), false}, }, }, + { + ONR("document", "*", "fakerelation"), + codes.InvalidArgument, + []checkTest{ + {ONR("user", "aasdasd", "..."), false}, + }, + }, } for _, delta := range testTimedeltas { diff --git a/internal/services/v0/developer_test.go b/internal/services/v0/developer_test.go index 581d1baae4..323bec8df3 100644 --- a/internal/services/v0/developer_test.go +++ b/internal/services/v0/developer_test.go @@ -144,6 +144,7 @@ func TestEditCheck(t *testing.T) { nil, []*v0.EditCheckResult{ { + Relationship: tuple.MustParse("somenamespace:someobj#anotherrel@user:foo"), Error: &v0.DeveloperError{ Message: "relation/permission `anotherrel` not found under definition `somenamespace`", Kind: v0.DeveloperError_UNKNOWN_RELATION, @@ -180,6 +181,35 @@ func TestEditCheck(t *testing.T) { }, }, }, + /* TODO(evan): Uncomment once dispatch for wildcard is supported. + { + "valid wildcard checks", + ` + definition user {} + definition somenamespace { + relation somerel: user | user:* + } + `, + []*v0.RelationTuple{ + tuple.MustParse("somenamespace:someobj#somerel@user:*"), + }, + []*v0.RelationTuple{ + tuple.MustParse("somenamespace:someobj#somerel@user:foo"), + tuple.MustParse("somenamespace:someobj#somerel@user:anotheruser"), + }, + nil, + []*v0.EditCheckResult{ + { + Relationship: tuple.MustParse("somenamespace:someobj#somerel@user:foo"), + IsMember: true, + }, + { + Relationship: tuple.MustParse("somenamespace:someobj#somerel@user:anotheruser"), + IsMember: true, + }, + }, + }, + */ } for _, tc := range tests { @@ -203,6 +233,7 @@ func TestEditCheck(t *testing.T) { require.Equal(tc.expectedResults, resp.CheckResults) } else { require.Equal(0, len(resp.RequestErrors), "Found error(s): %v", resp.RequestErrors) + require.Equal(tc.expectedResults, resp.CheckResults) } }) } @@ -580,6 +611,37 @@ assertFalse: }, ``, }, + /* TODO(evan): Uncomment once dispatch for wildcard is supported. + { + "wildcard relationship", + ` + definition user {} + definition document { + relation writer: user + relation viewer: user | user:* + permission view = viewer + writer + } + `, + []*v0.RelationTuple{ + tuple.MustParse("document:somedoc#writer@user:jimmy"), + tuple.MustParse("document:somedoc#viewer@user:*"), + }, + `"document:somedoc#view": + - "[user:*] is " + - "[user:jimmy] is "`, + `assertTrue: + - document:somedoc#writer@user:jimmy + - document:somedoc#viewer@user:jimmy + - document:somedoc#viewer@user:somegal + assertFalse: + - document:somedoc#writer@user:somegal + `, + nil, + `document:somedoc#view: + - '[user:*] is ' + - '[user:jimmy] is /' + `, + },*/ } for _, tc := range tests { diff --git a/internal/services/v0/validation.go b/internal/services/v0/validation.go index a9c26cf0db..9833b0076c 100644 --- a/internal/services/v0/validation.go +++ b/internal/services/v0/validation.go @@ -8,6 +8,7 @@ import ( "github.com/shopspring/decimal" "github.com/authzed/spicedb/internal/namespace" + "github.com/authzed/spicedb/pkg/tuple" ) type invalidRelationError struct { @@ -22,6 +23,17 @@ func validateTupleWrite( nsm namespace.Manager, revision decimal.Decimal, ) error { + + err := tuple.ValidateResourceID(tpl.ObjectAndRelation.ObjectId) + if err != nil { + return err + } + + err = tuple.ValidateSubjectID(tpl.User.GetUserset().ObjectId) + if err != nil { + return err + } + if err := nsm.CheckNamespaceAndRelation( ctx, tpl.ObjectAndRelation.Namespace, @@ -32,6 +44,17 @@ func validateTupleWrite( return err } + // Ensure wildcard writes have no subject relation. + if tpl.User.GetUserset().ObjectId == tuple.PublicWildcard { + if tpl.User.GetUserset().Relation != tuple.Ellipsis { + return invalidRelationError{ + error: fmt.Errorf("wildcard (public) relationships require a subject relation of `...` on %v", tpl.ObjectAndRelation), + subject: tpl.User, + onr: tpl.ObjectAndRelation, + } + } + } + if err := nsm.CheckNamespaceAndRelation( ctx, tpl.User.GetUserset().Namespace, @@ -55,19 +78,36 @@ func validateTupleWrite( } } - isAllowed, err := ts.IsAllowedDirectRelation( - tpl.ObjectAndRelation.Relation, - tpl.User.GetUserset().Namespace, - tpl.User.GetUserset().Relation) - if err != nil { - return err - } + if tpl.User.GetUserset().ObjectId == tuple.PublicWildcard { + isAllowed, err := ts.IsAllowedPublicNamespace( + tpl.ObjectAndRelation.Relation, + tpl.User.GetUserset().Namespace) + if err != nil { + return err + } - if isAllowed == namespace.DirectRelationNotValid { - return invalidRelationError{ - error: fmt.Errorf("relation/permission %v is not allowed as the subject of %v", tpl.User, tpl.ObjectAndRelation), - subject: tpl.User, - onr: tpl.ObjectAndRelation, + if isAllowed != namespace.PublicSubjectAllowed { + return invalidRelationError{ + error: fmt.Errorf("wildcard (public) subjects of type %s are not allowed on %v", tpl.User.GetUserset().Namespace, tpl.ObjectAndRelation), + subject: tpl.User, + onr: tpl.ObjectAndRelation, + } + } + } else { + isAllowed, err := ts.IsAllowedDirectRelation( + tpl.ObjectAndRelation.Relation, + tpl.User.GetUserset().Namespace, + tpl.User.GetUserset().Relation) + if err != nil { + return err + } + + if isAllowed == namespace.DirectRelationNotValid { + return invalidRelationError{ + error: fmt.Errorf("relation/permission %v is not allowed as the subject of %v", tpl.User, tpl.ObjectAndRelation), + subject: tpl.User, + onr: tpl.ObjectAndRelation, + } } } diff --git a/internal/services/v1/relationships.go b/internal/services/v1/relationships.go index 4516545d8d..fd6e2a8906 100644 --- a/internal/services/v1/relationships.go +++ b/internal/services/v1/relationships.go @@ -162,6 +162,16 @@ func (ps *permissionServer) WriteRelationships(ctx context.Context, req *v1.Writ } for _, update := range req.Updates { + err := tuple.ValidateResourceID(update.Relationship.Resource.ObjectId) + if err != nil { + return nil, rewritePermissionsError(ctx, err) + } + + err = tuple.ValidateSubjectID(update.Relationship.Subject.Object.ObjectId) + if err != nil { + return nil, rewritePermissionsError(ctx, err) + } + if err := ps.nsm.CheckNamespaceAndRelation( ctx, update.Relationship.Resource.ObjectType, @@ -172,6 +182,16 @@ func (ps *permissionServer) WriteRelationships(ctx context.Context, req *v1.Writ return nil, rewritePermissionsError(ctx, err) } + // Ensure wildcard writes have no subject relation. + if update.Relationship.Subject.Object.ObjectId == tuple.PublicWildcard { + if update.Relationship.Subject.OptionalRelation != "" { + return nil, status.Errorf( + codes.InvalidArgument, + "wildcard (public) relationships require an empty subject relation", + ) + } + } + if err := ps.nsm.CheckNamespaceAndRelation( ctx, update.Relationship.Subject.Object.ObjectType, @@ -195,22 +215,40 @@ func (ps *permissionServer) WriteRelationships(ctx context.Context, req *v1.Writ ) } - isAllowed, err := ts.IsAllowedDirectRelation( - update.Relationship.Relation, - update.Relationship.Subject.Object.ObjectType, - stringz.DefaultEmpty(update.Relationship.Subject.OptionalRelation, datastore.Ellipsis), - ) - if err != nil { - return nil, err - } - - if isAllowed == namespace.DirectRelationNotValid { - return nil, status.Errorf( - codes.InvalidArgument, - "subject %s is not allowed for the resource %s", - tuple.StringSubjectRef(update.Relationship.Subject), - tuple.StringObjectRef(update.Relationship.Resource), + if update.Relationship.Subject.Object.ObjectId == tuple.PublicWildcard { + isAllowed, err := ts.IsAllowedPublicNamespace( + update.Relationship.Relation, + update.Relationship.Subject.Object.ObjectType) + if err != nil { + return nil, err + } + + if isAllowed != namespace.PublicSubjectAllowed { + return nil, status.Errorf( + codes.InvalidArgument, + "wildcard (public) subjects of type %s are not allowed on %v", + update.Relationship.Subject.Object.ObjectType, + tuple.StringObjectRef(update.Relationship.Resource), + ) + } + } else { + isAllowed, err := ts.IsAllowedDirectRelation( + update.Relationship.Relation, + update.Relationship.Subject.Object.ObjectType, + stringz.DefaultEmpty(update.Relationship.Subject.OptionalRelation, datastore.Ellipsis), ) + if err != nil { + return nil, err + } + + if isAllowed == namespace.DirectRelationNotValid { + return nil, status.Errorf( + codes.InvalidArgument, + "subject %s is not allowed for the resource %s", + tuple.StringSubjectRef(update.Relationship.Subject), + tuple.StringObjectRef(update.Relationship.Resource), + ) + } } } diff --git a/internal/services/v1/relationships_test.go b/internal/services/v1/relationships_test.go index 79e1cf3c91..4d9605d854 100644 --- a/internal/services/v1/relationships_test.go +++ b/internal/services/v1/relationships_test.go @@ -442,6 +442,20 @@ func TestInvalidWriteRelationshipArgs(t *testing.T) { codes.InvalidArgument, "alphanumeric", }, + { + "disallowed wildcard relation", + nil, + []*v1.Relationship{rel("document", "somedoc", "parent", "user", "*", "somerelation")}, + codes.InvalidArgument, + "empty subject relation", + }, + { + "disallowed wildcard subject", + nil, + []*v1.Relationship{rel("document", "somedoc", "parent", "user", "*", "")}, + codes.InvalidArgument, + "wildcard", + }, } for _, delta := range testTimedeltas { From ba6a5b2ce8781e0d613ada3319a2afe6d94493ae Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 10 Dec 2021 16:58:35 -0500 Subject: [PATCH 09/11] Update go.sum --- go.sum | 4 ---- 1 file changed, 4 deletions(-) diff --git a/go.sum b/go.sum index 3a2dd4a6e0..51ff2b497f 100644 --- a/go.sum +++ b/go.sum @@ -73,10 +73,6 @@ github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5 github.com/armon/go-metrics v0.0.0-20180917152333-f0300d1749da/go.mod h1:Q73ZrmVTwzkszR9V5SSuryQ31EELlFMUz1kKyl939pY= github.com/armon/go-radix v0.0.0-20180808171621-7fddfc383310/go.mod h1:ufUuZ+zHj4x4TnLV4JWEpy2hxWSpsRywHrMgIH9cCH8= github.com/armon/go-radix v1.0.0/go.mod h1:ufUuZ+zHj4x4TnLV4JWEpy2hxWSpsRywHrMgIH9cCH8= -github.com/authzed/authzed-go v0.3.1-0.20211130221323-9d59da6e55da h1:dcjIyi2Om9CDAmEKiwQZsF7bV0x4PXuJeoN11BbbXXI= -github.com/authzed/authzed-go v0.3.1-0.20211130221323-9d59da6e55da/go.mod h1:bsUniBRroq4l5WZMYLO+T9osQa/P2qMwZ+Af8zoJK8Y= -github.com/authzed/authzed-go v0.3.1-0.20211209213733-6dfdd048c960 h1:C8e4cBlOetPhqNCMgE4fhBb+wzhQLziHfHJgzRFZd3s= -github.com/authzed/authzed-go v0.3.1-0.20211209213733-6dfdd048c960/go.mod h1:bsUniBRroq4l5WZMYLO+T9osQa/P2qMwZ+Af8zoJK8Y= github.com/authzed/authzed-go v0.3.1-0.20211210193835-3ada8db7bc94 h1:4la5GstRfU268cich5RwPe7+Smkz/4BUcHDGSOH8yhY= github.com/authzed/authzed-go v0.3.1-0.20211210193835-3ada8db7bc94/go.mod h1:bsUniBRroq4l5WZMYLO+T9osQa/P2qMwZ+Af8zoJK8Y= github.com/authzed/grpcutil v0.0.0-20210913124023-cad23ae5a9e8/go.mod h1:HwO/KbRU3fWXEYHE96kvXnwxzi97tkXD1hfi5UaZ71Y= From 5245dfc302a3e46c015d090fe9166e92a10b03cd Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Fri, 10 Dec 2021 18:19:16 -0500 Subject: [PATCH 10/11] Update authzed-go reference --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 65ea09d4b8..c027e6ae2e 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,7 @@ go 1.16 require ( github.com/Masterminds/squirrel v1.5.2 github.com/alecthomas/units v0.0.0-20210927113745-59d0afb8317a - github.com/authzed/authzed-go v0.3.1-0.20211210193835-3ada8db7bc94 + github.com/authzed/authzed-go v0.3.1-0.20211210231744-4126c5fdd0a5 github.com/authzed/grpcutil v0.0.0-20211020204402-aba1876830e6 github.com/aws/aws-sdk-go v1.42.16 github.com/benbjohnson/clock v1.3.0 diff --git a/go.sum b/go.sum index 51ff2b497f..a5266dd96e 100644 --- a/go.sum +++ b/go.sum @@ -73,8 +73,8 @@ github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5 github.com/armon/go-metrics v0.0.0-20180917152333-f0300d1749da/go.mod h1:Q73ZrmVTwzkszR9V5SSuryQ31EELlFMUz1kKyl939pY= github.com/armon/go-radix v0.0.0-20180808171621-7fddfc383310/go.mod h1:ufUuZ+zHj4x4TnLV4JWEpy2hxWSpsRywHrMgIH9cCH8= github.com/armon/go-radix v1.0.0/go.mod h1:ufUuZ+zHj4x4TnLV4JWEpy2hxWSpsRywHrMgIH9cCH8= -github.com/authzed/authzed-go v0.3.1-0.20211210193835-3ada8db7bc94 h1:4la5GstRfU268cich5RwPe7+Smkz/4BUcHDGSOH8yhY= -github.com/authzed/authzed-go v0.3.1-0.20211210193835-3ada8db7bc94/go.mod h1:bsUniBRroq4l5WZMYLO+T9osQa/P2qMwZ+Af8zoJK8Y= +github.com/authzed/authzed-go v0.3.1-0.20211210231744-4126c5fdd0a5 h1:avwpP+RWe7YaA6IisYzpfejb42/dDzhb7S4pwF4tmWs= +github.com/authzed/authzed-go v0.3.1-0.20211210231744-4126c5fdd0a5/go.mod h1:bsUniBRroq4l5WZMYLO+T9osQa/P2qMwZ+Af8zoJK8Y= github.com/authzed/grpcutil v0.0.0-20210913124023-cad23ae5a9e8/go.mod h1:HwO/KbRU3fWXEYHE96kvXnwxzi97tkXD1hfi5UaZ71Y= github.com/authzed/grpcutil v0.0.0-20211020204402-aba1876830e6 h1:izP/rEris51ZmomXb5J0ShyJKqsxTfVKDRnJz0QGbgg= github.com/authzed/grpcutil v0.0.0-20211020204402-aba1876830e6/go.mod h1:rqjY3zyK/YP7NID9+B2BdIRRkvnK+cdf9/qya/zaFZE= From 46f931b573fa2e3e19dc9251d1771edaa72934e3 Mon Sep 17 00:00:00 2001 From: Evan Cordell Date: Fri, 10 Dec 2021 18:29:11 -0500 Subject: [PATCH 11/11] support wildcard in check requests --- internal/graph/check.go | 6 +++++- internal/services/v0/developer_test.go | 2 -- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/internal/graph/check.go b/internal/graph/check.go index 35dcffc2e4..ffe0addf3f 100644 --- a/internal/graph/check.go +++ b/internal/graph/check.go @@ -32,6 +32,10 @@ func onrEqual(lhs, rhs *v0.ObjectAndRelation) bool { return lhs.ObjectId == rhs.ObjectId && lhs.Relation == rhs.Relation && lhs.Namespace == rhs.Namespace } +func onrEqualOrPublic(tuple, target *v0.ObjectAndRelation) bool { + return onrEqual(tuple, target) || (target.Namespace == tuple.Namespace && tuple.ObjectId == "*") +} + // ValidatedCheckRequest represents a request after it has been validated and parsed for internal // consumption. type ValidatedCheckRequest struct { @@ -82,7 +86,7 @@ func (cc *ConcurrentChecker) checkDirect(ctx context.Context, req ValidatedCheck var requestsToDispatch []ReduceableCheckFunc for tpl := it.Next(); tpl != nil; tpl = it.Next() { tplUserset := tpl.User.GetUserset() - if onrEqual(tplUserset, req.Subject) { + if onrEqualOrPublic(tplUserset, req.Subject) { resultChan <- checkResult(v1.DispatchCheckResponse_MEMBER, emptyMetadata) return } diff --git a/internal/services/v0/developer_test.go b/internal/services/v0/developer_test.go index 323bec8df3..f518a514d2 100644 --- a/internal/services/v0/developer_test.go +++ b/internal/services/v0/developer_test.go @@ -181,7 +181,6 @@ func TestEditCheck(t *testing.T) { }, }, }, - /* TODO(evan): Uncomment once dispatch for wildcard is supported. { "valid wildcard checks", ` @@ -209,7 +208,6 @@ func TestEditCheck(t *testing.T) { }, }, }, - */ } for _, tc := range tests {