Skip to content

Commit

Permalink
Fix exclusion operation in Check dispatch to *always* request the ful…
Browse files Browse the repository at this point in the history
…l set of results for the first branch
  • Loading branch information
josephschorr committed Jun 20, 2024
1 parent c50701a commit 684f857
Show file tree
Hide file tree
Showing 4 changed files with 327 additions and 1 deletion.
261 changes: 261 additions & 0 deletions internal/dispatch/graph/check_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,267 @@ func addFrame(trace *v1.CheckDebugTrace, foundFrames *mapz.Set[string]) {
}
}

func TestCheckPermissionOverSchema(t *testing.T) {
testCases := []struct {
name string
schema string
relationships []*core.RelationTuple
resource *core.ObjectAndRelation
subject *core.ObjectAndRelation
expectedPermissionship v1.ResourceCheckResult_Membership
}{
{
"basic union",
`definition user {}
definition document {
relation editor: user
relation viewer: user
permission view = viewer + editor
}`,
[]*core.RelationTuple{
tuple.MustParse("document:first#viewer@user:tom"),
},
ONR("document", "first", "view"),
ONR("user", "tom", "..."),
v1.ResourceCheckResult_MEMBER,
},
{
"basic intersection",
`definition user {}
definition document {
relation editor: user
relation viewer: user
permission view = viewer & editor
}`,
[]*core.RelationTuple{
tuple.MustParse("document:first#viewer@user:tom"),
tuple.MustParse("document:first#editor@user:tom"),
},
ONR("document", "first", "view"),
ONR("user", "tom", "..."),
v1.ResourceCheckResult_MEMBER,
},
{
"basic exclusion",
`definition user {}
definition document {
relation editor: user
relation viewer: user
permission view = viewer - editor
}`,
[]*core.RelationTuple{
tuple.MustParse("document:first#viewer@user:tom"),
},
ONR("document", "first", "view"),
ONR("user", "tom", "..."),
v1.ResourceCheckResult_MEMBER,
},
{
"basic union, multiple branches",
`definition user {}
definition document {
relation editor: user
relation viewer: user
permission view = viewer + editor
}`,
[]*core.RelationTuple{
tuple.MustParse("document:first#viewer@user:tom"),
tuple.MustParse("document:first#editor@user:tom"),
},
ONR("document", "first", "view"),
ONR("user", "tom", "..."),
v1.ResourceCheckResult_MEMBER,
},
{
"basic union no permission",
`definition user {}
definition document {
relation editor: user
relation viewer: user
permission view = viewer + editor
}`,
[]*core.RelationTuple{},
ONR("document", "first", "view"),
ONR("user", "tom", "..."),
v1.ResourceCheckResult_NOT_MEMBER,
},
{
"basic intersection no permission",
`definition user {}
definition document {
relation editor: user
relation viewer: user
permission view = viewer & editor
}`,
[]*core.RelationTuple{
tuple.MustParse("document:first#viewer@user:tom"),
},
ONR("document", "first", "view"),
ONR("user", "tom", "..."),
v1.ResourceCheckResult_NOT_MEMBER,
},
{
"basic exclusion no permission",
`definition user {}
definition document {
relation banned: user
relation viewer: user
permission view = viewer - banned
}`,
[]*core.RelationTuple{
tuple.MustParse("document:first#viewer@user:tom"),
tuple.MustParse("document:first#banned@user:tom"),
},
ONR("document", "first", "view"),
ONR("user", "tom", "..."),
v1.ResourceCheckResult_NOT_MEMBER,
},
{
"exclusion with multiple branches",
`definition user {}
definition group {
relation member: user
relation banned: user
permission view = member - banned
}
definition document {
relation group: group
permission view = group->view
}`,
[]*core.RelationTuple{
tuple.MustParse("document:first#group@group:first"),
tuple.MustParse("document:first#group@group:second"),
tuple.MustParse("group:first#member@user:tom"),
tuple.MustParse("group:first#banned@user:tom"),
tuple.MustParse("group:second#member@user:tom"),
},
ONR("document", "first", "view"),
ONR("user", "tom", "..."),
v1.ResourceCheckResult_MEMBER,
},
{
"intersection with multiple branches",
`definition user {}
definition group {
relation member: user
relation other: user
permission view = member & other
}
definition document {
relation group: group
permission view = group->view
}`,
[]*core.RelationTuple{
tuple.MustParse("document:first#group@group:first"),
tuple.MustParse("document:first#group@group:second"),
tuple.MustParse("group:first#member@user:tom"),
tuple.MustParse("group:first#other@user:tom"),
tuple.MustParse("group:second#member@user:tom"),
},
ONR("document", "first", "view"),
ONR("user", "tom", "..."),
v1.ResourceCheckResult_MEMBER,
},
{
"exclusion with multiple branches no permission",
`definition user {}
definition group {
relation member: user
relation banned: user
permission view = member - banned
}
definition document {
relation group: group
permission view = group->view
}`,
[]*core.RelationTuple{
tuple.MustParse("document:first#group@group:first"),
tuple.MustParse("document:first#group@group:second"),
tuple.MustParse("group:first#member@user:tom"),
tuple.MustParse("group:first#banned@user:tom"),
tuple.MustParse("group:second#member@user:tom"),
tuple.MustParse("group:second#banned@user:tom"),
},
ONR("document", "first", "view"),
ONR("user", "tom", "..."),
v1.ResourceCheckResult_NOT_MEMBER,
},
{
"intersection with multiple branches no permission",
`definition user {}
definition group {
relation member: user
relation other: user
permission view = member & other
}
definition document {
relation group: group
permission view = group->view
}`,
[]*core.RelationTuple{
tuple.MustParse("document:first#group@group:first"),
tuple.MustParse("document:first#group@group:second"),
tuple.MustParse("group:first#member@user:tom"),
tuple.MustParse("group:second#member@user:tom"),
},
ONR("document", "first", "view"),
ONR("user", "tom", "..."),
v1.ResourceCheckResult_NOT_MEMBER,
},
}

for _, tc := range testCases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
require := require.New(t)

dispatcher := NewLocalOnlyDispatcher(10)

ds, err := memdb.NewMemdbDatastore(0, 0, memdb.DisableGC)
require.NoError(err)

ds, revision := testfixtures.DatastoreFromSchemaAndTestRelationships(ds, tc.schema, tc.relationships, require)

ctx := datastoremw.ContextWithHandle(context.Background())
require.NoError(datastoremw.SetInContext(ctx, ds))

resp, err := dispatcher.DispatchCheck(ctx, &v1.DispatchCheckRequest{
ResourceRelation: RR(tc.resource.Namespace, tc.resource.Relation),
ResourceIds: []string{tc.resource.ObjectId},
Subject: tc.subject,
Metadata: &v1.ResolverMeta{
AtRevision: revision.String(),
DepthRemaining: 50,
},
ResultsSetting: v1.DispatchCheckRequest_ALLOW_SINGLE_RESULT,
})
require.NoError(err)

membership := v1.ResourceCheckResult_NOT_MEMBER
if r, ok := resp.ResultsByResourceId[tc.resource.ObjectId]; ok {
membership = r.Membership
}

require.Equal(tc.expectedPermissionship, membership)
})
}
}

func TestCheckDebugging(t *testing.T) {
type expectedFrame struct {
resourceType *core.RelationReference
Expand Down
7 changes: 6 additions & 1 deletion internal/graph/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -770,7 +770,12 @@ func difference[T any](
othersChan := make(chan CheckResult, len(children)-1)

go func() {
result := handler(childCtx, crc, children[0])
result := handler(childCtx, currentRequestContext{
parentReq: crc.parentReq,
filteredResourceIDs: crc.filteredResourceIDs,
resultsSetting: v1.DispatchCheckRequest_REQUIRE_ALL_RESULTS,
maxDispatchCount: crc.maxDispatchCount,
}, children[0])
baseChan <- result
}()

Expand Down
28 changes: 28 additions & 0 deletions internal/graph/membershipset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -705,6 +705,34 @@ func TestMembershipSetSubtract(t *testing.T) {
false,
false,
},
{
"non overlapping",
map[string]*core.CaveatExpression{
"resource1": nil,
"resource2": nil,
},
map[string]*core.CaveatExpression{
"resource2": nil,
},
map[string]*core.CaveatExpression{
"resource1": nil,
},
true,
false,
},
{
"non overlapping reversed",
map[string]*core.CaveatExpression{
"resource2": nil,
},
map[string]*core.CaveatExpression{
"resource1": nil,
"resource2": nil,
},
map[string]*core.CaveatExpression{},
false,
true,
},
}

for _, tc := range tcs {
Expand Down
32 changes: 32 additions & 0 deletions internal/services/integrationtesting/arrowovermultiexclusion.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
---
schema: |+
definition user {}
definition group {
relation direct_member: user
relation excluded: user:*
permission view = direct_member - excluded
}
definition resource {
relation group: group
permission view = group->view
}
relationships: >-
resource:one#group@group:group1
resource:one#group@group:group2
group:group1#direct_member@user:fred
group:group1#excluded@user:*
group:group2#direct_member@user:fred
assertions:
assertTrue:
- "group:group1#direct_member@user:fred"
- "group:group2#direct_member@user:fred"
- "group:group2#view@user:fred"
assertFalse:
- "group:group1#view@user:fred"

0 comments on commit 684f857

Please sign in to comment.