From 883fc2cd97fbc715fe8a5a9f6837d235e3cd4aa1 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Tue, 11 Jan 2022 14:45:21 -0500 Subject: [PATCH] Fixes for lint issues --- internal/graph/lookup.go | 4 + internal/membership/foundsubject_test.go | 3 +- internal/membership/membership.go | 4 +- internal/membership/membership_test.go | 172 +++++++++--------- .../services/testconfigs/simplewildcard.yaml | 8 +- .../services/testconfigs/wildcardnested.yaml | 26 +-- .../testconfigs/wildcardwithintersection.yaml | 6 +- .../wildcardwithrightsideexclusion.yaml | 6 +- 8 files changed, 113 insertions(+), 116 deletions(-) diff --git a/internal/graph/lookup.go b/internal/graph/lookup.go index 52bedd10b1..3d226906a2 100644 --- a/internal/graph/lookup.go +++ b/internal/graph/lookup.go @@ -211,6 +211,10 @@ func (cl *ConcurrentLookup) lookupDirect(ctx context.Context, req ValidatedLooku // Dispatch a check for the subject wildcard, if allowed. isWildcardAllowed, err := typeSystem.IsAllowedPublicNamespace(req.ObjectRelation.Relation, req.Subject.Namespace) + if err != nil { + return returnResult(lookupResultError(req, err, emptyMetadata)) + } + if isWildcardAllowed == namespace.PublicSubjectAllowed { requests = append(requests, func(ctx context.Context, resultChan chan<- LookupResult) { objects := tuple.NewONRSet() diff --git a/internal/membership/foundsubject_test.go b/internal/membership/foundsubject_test.go index 7f87a6d882..83c791841a 100644 --- a/internal/membership/foundsubject_test.go +++ b/internal/membership/foundsubject_test.go @@ -4,8 +4,9 @@ import ( "fmt" "testing" - "github.com/authzed/spicedb/pkg/validationfile" "github.com/stretchr/testify/require" + + "github.com/authzed/spicedb/pkg/validationfile" ) func TestToValidationString(t *testing.T) { diff --git a/internal/membership/membership.go b/internal/membership/membership.go index 123846bea6..0b73716333 100644 --- a/internal/membership/membership.go +++ b/internal/membership/membership.go @@ -78,7 +78,7 @@ func populateFoundSubjects(rootONR *v0.ObjectAndRelation, treeNode *v0.RelationT case v0.SetOperationUserset_INTERSECTION: if len(typed.IntermediateNode.ChildNodes) == 0 { - return nil, fmt.Errorf("Found intersection with no children") + return nil, fmt.Errorf("found intersection with no children") } firstChildSet, err := populateFoundSubjects(rootONR, typed.IntermediateNode.ChildNodes[0]) @@ -100,7 +100,7 @@ func populateFoundSubjects(rootONR *v0.ObjectAndRelation, treeNode *v0.RelationT case v0.SetOperationUserset_EXCLUSION: if len(typed.IntermediateNode.ChildNodes) == 0 { - return nil, fmt.Errorf("Found exclusion with no children") + return nil, fmt.Errorf("found exclusion with no children") } firstChildSet, err := populateFoundSubjects(rootONR, typed.IntermediateNode.ChildNodes[0]) diff --git a/internal/membership/membership_test.go b/internal/membership/membership_test.go index f5c27de483..9be0e285eb 100644 --- a/internal/membership/membership_test.go +++ b/internal/membership/membership_test.go @@ -192,16 +192,15 @@ func TestMembershipSetIntersectionWithOneWildcard(t *testing.T) { require := require.New(t) ms := NewMembershipSet() - intersection := - graph.Intersection(ONR("folder", "company", "viewer"), - graph.Leaf(_this, - tuple.User(ONR("user", "owner", "...")), - tuple.User(ONR("user", "*", "...")), - ), - graph.Leaf(_this, - tuple.User(ONR("user", "legal", "...")), - ), - ) + intersection := graph.Intersection(ONR("folder", "company", "viewer"), + graph.Leaf(_this, + tuple.User(ONR("user", "owner", "...")), + tuple.User(ONR("user", "*", "...")), + ), + graph.Leaf(_this, + tuple.User(ONR("user", "legal", "...")), + ), + ) fso, ok, err := ms.AddExpansion(ONR("folder", "company", "viewer"), intersection) require.True(ok) @@ -213,16 +212,15 @@ func TestMembershipSetIntersectionWithAllWildcardLeft(t *testing.T) { require := require.New(t) ms := NewMembershipSet() - intersection := - graph.Intersection(ONR("folder", "company", "viewer"), - graph.Leaf(_this, - tuple.User(ONR("user", "owner", "...")), - tuple.User(ONR("user", "*", "...")), - ), - graph.Leaf(_this, - tuple.User(ONR("user", "*", "...")), - ), - ) + intersection := graph.Intersection(ONR("folder", "company", "viewer"), + graph.Leaf(_this, + tuple.User(ONR("user", "owner", "...")), + tuple.User(ONR("user", "*", "...")), + ), + graph.Leaf(_this, + tuple.User(ONR("user", "*", "...")), + ), + ) fso, ok, err := ms.AddExpansion(ONR("folder", "company", "viewer"), intersection) require.True(ok) @@ -234,16 +232,15 @@ func TestMembershipSetIntersectionWithAllWildcardRight(t *testing.T) { require := require.New(t) ms := NewMembershipSet() - intersection := - graph.Intersection(ONR("folder", "company", "viewer"), - graph.Leaf(_this, - tuple.User(ONR("user", "*", "...")), - ), - graph.Leaf(_this, - tuple.User(ONR("user", "owner", "...")), - tuple.User(ONR("user", "*", "...")), - ), - ) + intersection := graph.Intersection(ONR("folder", "company", "viewer"), + graph.Leaf(_this, + tuple.User(ONR("user", "*", "...")), + ), + graph.Leaf(_this, + tuple.User(ONR("user", "owner", "...")), + tuple.User(ONR("user", "*", "...")), + ), + ) fso, ok, err := ms.AddExpansion(ONR("folder", "company", "viewer"), intersection) require.True(ok) @@ -255,16 +252,15 @@ func TestMembershipSetExclusionWithLeftWildcard(t *testing.T) { require := require.New(t) ms := NewMembershipSet() - exclusion := - graph.Exclusion(ONR("folder", "company", "viewer"), - graph.Leaf(_this, - tuple.User(ONR("user", "owner", "...")), - tuple.User(ONR("user", "*", "...")), - ), - graph.Leaf(_this, - tuple.User(ONR("user", "legal", "...")), - ), - ) + exclusion := graph.Exclusion(ONR("folder", "company", "viewer"), + graph.Leaf(_this, + tuple.User(ONR("user", "owner", "...")), + tuple.User(ONR("user", "*", "...")), + ), + graph.Leaf(_this, + tuple.User(ONR("user", "legal", "...")), + ), + ) fso, ok, err := ms.AddExpansion(ONR("folder", "company", "viewer"), exclusion) require.True(ok) @@ -276,16 +272,15 @@ func TestMembershipSetExclusionWithRightWildcard(t *testing.T) { require := require.New(t) ms := NewMembershipSet() - exclusion := - graph.Exclusion(ONR("folder", "company", "viewer"), - graph.Leaf(_this, - tuple.User(ONR("user", "owner", "...")), - tuple.User(ONR("user", "legal", "...")), - ), - graph.Leaf(_this, - tuple.User(ONR("user", "*", "...")), - ), - ) + exclusion := graph.Exclusion(ONR("folder", "company", "viewer"), + graph.Leaf(_this, + tuple.User(ONR("user", "owner", "...")), + tuple.User(ONR("user", "legal", "...")), + ), + graph.Leaf(_this, + tuple.User(ONR("user", "*", "...")), + ), + ) fso, ok, err := ms.AddExpansion(ONR("folder", "company", "viewer"), exclusion) require.True(ok) @@ -297,19 +292,18 @@ func TestMembershipSetIntersectionWithThreeWildcards(t *testing.T) { require := require.New(t) ms := NewMembershipSet() - intersection := - graph.Intersection(ONR("folder", "company", "viewer"), - graph.Leaf(_this, - tuple.User(ONR("user", "owner", "...")), - tuple.User(ONR("user", "legal", "...")), - ), - graph.Leaf(_this, - tuple.User(ONR("user", "*", "...")), - ), - graph.Leaf(_this, - tuple.User(ONR("user", "*", "...")), - ), - ) + intersection := graph.Intersection(ONR("folder", "company", "viewer"), + graph.Leaf(_this, + tuple.User(ONR("user", "owner", "...")), + tuple.User(ONR("user", "legal", "...")), + ), + graph.Leaf(_this, + tuple.User(ONR("user", "*", "...")), + ), + graph.Leaf(_this, + tuple.User(ONR("user", "*", "...")), + ), + ) fso, ok, err := ms.AddExpansion(ONR("folder", "company", "viewer"), intersection) require.True(ok) @@ -321,20 +315,19 @@ func TestMembershipSetIntersectionWithOneBranchMissingWildcards(t *testing.T) { require := require.New(t) ms := NewMembershipSet() - intersection := - graph.Intersection(ONR("folder", "company", "viewer"), - graph.Leaf(_this, - tuple.User(ONR("user", "owner", "...")), - tuple.User(ONR("user", "legal", "...")), - tuple.User(ONR("user", "*", "...")), - ), - graph.Leaf(_this, - tuple.User(ONR("user", "owner", "...")), - ), - graph.Leaf(_this, - tuple.User(ONR("user", "*", "...")), - ), - ) + intersection := graph.Intersection(ONR("folder", "company", "viewer"), + graph.Leaf(_this, + tuple.User(ONR("user", "owner", "...")), + tuple.User(ONR("user", "legal", "...")), + tuple.User(ONR("user", "*", "...")), + ), + graph.Leaf(_this, + tuple.User(ONR("user", "owner", "...")), + ), + graph.Leaf(_this, + tuple.User(ONR("user", "*", "...")), + ), + ) fso, ok, err := ms.AddExpansion(ONR("folder", "company", "viewer"), intersection) require.True(ok) @@ -346,19 +339,18 @@ func TestMembershipSetIntersectionWithTwoBranchesMissingWildcards(t *testing.T) require := require.New(t) ms := NewMembershipSet() - intersection := - graph.Intersection(ONR("folder", "company", "viewer"), - graph.Leaf(_this, - tuple.User(ONR("user", "owner", "...")), - tuple.User(ONR("user", "legal", "...")), - ), - graph.Leaf(_this, - tuple.User(ONR("user", "another", "...")), - ), - graph.Leaf(_this, - tuple.User(ONR("user", "*", "...")), - ), - ) + intersection := graph.Intersection(ONR("folder", "company", "viewer"), + graph.Leaf(_this, + tuple.User(ONR("user", "owner", "...")), + tuple.User(ONR("user", "legal", "...")), + ), + graph.Leaf(_this, + tuple.User(ONR("user", "another", "...")), + ), + graph.Leaf(_this, + tuple.User(ONR("user", "*", "...")), + ), + ) fso, ok, err := ms.AddExpansion(ONR("folder", "company", "viewer"), intersection) require.True(ok) diff --git a/internal/services/testconfigs/simplewildcard.yaml b/internal/services/testconfigs/simplewildcard.yaml index 7af9470557..0de5c3d994 100644 --- a/internal/services/testconfigs/simplewildcard.yaml +++ b/internal/services/testconfigs/simplewildcard.yaml @@ -10,7 +10,7 @@ relationships: | test/resource:first#viewer@test/user:concreteguy assertions: assertTrue: - - test/resource:first#viewer@test/user:concreteguy - - test/resource:first#viewer@test/user:anotheruser - - test/resource:first#viewer@test/user:aseconduser - - test/resource:first#viewer@test/user:athirduser + - "test/resource:first#viewer@test/user:concreteguy" + - "test/resource:first#viewer@test/user:anotheruser" + - "test/resource:first#viewer@test/user:aseconduser" + - "test/resource:first#viewer@test/user:athirduser" diff --git a/internal/services/testconfigs/wildcardnested.yaml b/internal/services/testconfigs/wildcardnested.yaml index 63ec04e20c..66097dcd0f 100644 --- a/internal/services/testconfigs/wildcardnested.yaml +++ b/internal/services/testconfigs/wildcardnested.yaml @@ -16,20 +16,20 @@ relationships: | test/resource:first#mustbehere@test/user:somegal assertions: assertTrue: - - test/resource:first#viewer@test/user:somegal - - test/resource:first#viewer@test/user:anotherperson - - test/resource:first#viewer@test/user:thirduser - - test/resource:first#viewer@test/user:bannedguy + - "test/resource:first#viewer@test/user:somegal" + - "test/resource:first#viewer@test/user:anotherperson" + - "test/resource:first#viewer@test/user:thirduser" + - "test/resource:first#viewer@test/user:bannedguy" - - test/resource:first#view@test/user:somegal - - test/resource:first#view@test/user:anotherperson - - test/resource:first#view@test/user:thirduser + - "test/resource:first#view@test/user:somegal" + - "test/resource:first#view@test/user:anotherperson" + - "test/resource:first#view@test/user:thirduser" - - test/resource:first#mustbehere@test/user:somegal - - test/resource:first#specialview@test/user:somegal + - "test/resource:first#mustbehere@test/user:somegal" + - "test/resource:first#specialview@test/user:somegal" assertFalse: - - test/resource:first#view@test/user:bannedguy + - "test/resource:first#view@test/user:bannedguy" - - test/resource:first#specialview@test/user:bannedguy - - test/resource:first#specialview@test/user:anotherperson - - test/resource:first#specialview@test/user:thirduser + - "test/resource:first#specialview@test/user:bannedguy" + - "test/resource:first#specialview@test/user:anotherperson" + - "test/resource:first#specialview@test/user:thirduser" diff --git a/internal/services/testconfigs/wildcardwithintersection.yaml b/internal/services/testconfigs/wildcardwithintersection.yaml index c87ef6c0a2..3e28a80864 100644 --- a/internal/services/testconfigs/wildcardwithintersection.yaml +++ b/internal/services/testconfigs/wildcardwithintersection.yaml @@ -16,9 +16,9 @@ relationships: | test/resource:second#viewer@test/user:* assertions: assertTrue: - - test/resource:first#view@test/user:somegal - - test/resource:second#view@test/user:editordude - - test/resource:second#view@test/user:seconduser + - "test/resource:first#view@test/user:somegal" + - "test/resource:second#view@test/user:editordude" + - "test/resource:second#view@test/user:seconduser" assertFalse: - "test/resource:first#view@test/user:editordude" - "test/resource:first#view@test/user:anotheruser" diff --git a/internal/services/testconfigs/wildcardwithrightsideexclusion.yaml b/internal/services/testconfigs/wildcardwithrightsideexclusion.yaml index 2304e1d1e9..321ba7de93 100644 --- a/internal/services/testconfigs/wildcardwithrightsideexclusion.yaml +++ b/internal/services/testconfigs/wildcardwithrightsideexclusion.yaml @@ -3,7 +3,7 @@ schema: >- definition test/user {} definition test/resource { - relation viewer: test/user + relation viewer: test/user relation banned: test/user | test/user:* permission view = viewer - banned @@ -16,8 +16,8 @@ relationships: | test/resource:second#viewer@test/user:somegal assertions: assertTrue: - - test/resource:first#viewer@test/user:somegal - - test/resource:second#viewer@test/user:somegal + - "test/resource:first#viewer@test/user:somegal" + - "test/resource:second#viewer@test/user:somegal" assertFalse: - "test/resource:first#view@test/user:editordude" - "test/resource:first#view@test/user:anotheruser"