Skip to content

Commit 20855de

Browse files
josephschorrjosephschorr
committed
Ensure all resources are returned for relation check when caveats are specified
Fixes #2026
1 parent 881a32e commit 20855de

File tree

5 files changed

+161
-11
lines changed

5 files changed

+161
-11
lines changed

‎internal/dispatch/graph/check_test.go

+124-9
Original file line numberDiff line numberDiff line change
@@ -288,13 +288,14 @@ func TestCheckMetadata(t *testing.T) {
288288

289289
func TestCheckPermissionOverSchema(t *testing.T) {
290290
testCases := []struct {
291-
name string
292-
schema string
293-
relationships []*core.RelationTuple
294-
resource *core.ObjectAndRelation
295-
subject *core.ObjectAndRelation
296-
expectedPermissionship v1.ResourceCheckResult_Membership
297-
expectedCaveat *core.CaveatExpression
291+
name string
292+
schema string
293+
relationships []*core.RelationTuple
294+
resource *core.ObjectAndRelation
295+
subject *core.ObjectAndRelation
296+
expectedPermissionship v1.ResourceCheckResult_Membership
297+
expectedCaveat *core.CaveatExpression
298+
alternativeExpectedCaveat *core.CaveatExpression
298299
}{
299300
{
300301
"basic union",
@@ -312,6 +313,7 @@ func TestCheckPermissionOverSchema(t *testing.T) {
312313
ONR("user", "tom", "..."),
313314
v1.ResourceCheckResult_MEMBER,
314315
nil,
316+
nil,
315317
},
316318
{
317319
"basic intersection",
@@ -330,6 +332,7 @@ func TestCheckPermissionOverSchema(t *testing.T) {
330332
ONR("user", "tom", "..."),
331333
v1.ResourceCheckResult_MEMBER,
332334
nil,
335+
nil,
333336
},
334337
{
335338
"basic exclusion",
@@ -347,6 +350,7 @@ func TestCheckPermissionOverSchema(t *testing.T) {
347350
ONR("user", "tom", "..."),
348351
v1.ResourceCheckResult_MEMBER,
349352
nil,
353+
nil,
350354
},
351355
{
352356
"basic union, multiple branches",
@@ -365,6 +369,7 @@ func TestCheckPermissionOverSchema(t *testing.T) {
365369
ONR("user", "tom", "..."),
366370
v1.ResourceCheckResult_MEMBER,
367371
nil,
372+
nil,
368373
},
369374
{
370375
"basic union no permission",
@@ -380,6 +385,7 @@ func TestCheckPermissionOverSchema(t *testing.T) {
380385
ONR("user", "tom", "..."),
381386
v1.ResourceCheckResult_NOT_MEMBER,
382387
nil,
388+
nil,
383389
},
384390
{
385391
"basic intersection no permission",
@@ -397,6 +403,7 @@ func TestCheckPermissionOverSchema(t *testing.T) {
397403
ONR("user", "tom", "..."),
398404
v1.ResourceCheckResult_NOT_MEMBER,
399405
nil,
406+
nil,
400407
},
401408
{
402409
"basic exclusion no permission",
@@ -415,6 +422,7 @@ func TestCheckPermissionOverSchema(t *testing.T) {
415422
ONR("user", "tom", "..."),
416423
v1.ResourceCheckResult_NOT_MEMBER,
417424
nil,
425+
nil,
418426
},
419427
{
420428
"exclusion with multiple branches",
@@ -441,6 +449,7 @@ func TestCheckPermissionOverSchema(t *testing.T) {
441449
ONR("user", "tom", "..."),
442450
v1.ResourceCheckResult_MEMBER,
443451
nil,
452+
nil,
444453
},
445454
{
446455
"intersection with multiple branches",
@@ -467,6 +476,7 @@ func TestCheckPermissionOverSchema(t *testing.T) {
467476
ONR("user", "tom", "..."),
468477
v1.ResourceCheckResult_MEMBER,
469478
nil,
479+
nil,
470480
},
471481
{
472482
"exclusion with multiple branches no permission",
@@ -494,6 +504,7 @@ func TestCheckPermissionOverSchema(t *testing.T) {
494504
ONR("user", "tom", "..."),
495505
v1.ResourceCheckResult_NOT_MEMBER,
496506
nil,
507+
nil,
497508
},
498509
{
499510
"intersection with multiple branches no permission",
@@ -519,6 +530,7 @@ func TestCheckPermissionOverSchema(t *testing.T) {
519530
ONR("user", "tom", "..."),
520531
v1.ResourceCheckResult_NOT_MEMBER,
521532
nil,
533+
nil,
522534
},
523535
{
524536
"basic arrow",
@@ -541,6 +553,7 @@ func TestCheckPermissionOverSchema(t *testing.T) {
541553
ONR("user", "tom", "..."),
542554
v1.ResourceCheckResult_MEMBER,
543555
nil,
556+
nil,
544557
},
545558
{
546559
"basic any arrow",
@@ -563,6 +576,7 @@ func TestCheckPermissionOverSchema(t *testing.T) {
563576
ONR("user", "tom", "..."),
564577
v1.ResourceCheckResult_MEMBER,
565578
nil,
579+
nil,
566580
},
567581
{
568582
"basic all arrow negative",
@@ -585,6 +599,7 @@ func TestCheckPermissionOverSchema(t *testing.T) {
585599
ONR("user", "tom", "..."),
586600
v1.ResourceCheckResult_NOT_MEMBER,
587601
nil,
602+
nil,
588603
},
589604
{
590605
"basic all arrow positive",
@@ -608,6 +623,7 @@ func TestCheckPermissionOverSchema(t *testing.T) {
608623
ONR("user", "tom", "..."),
609624
v1.ResourceCheckResult_MEMBER,
610625
nil,
626+
nil,
611627
},
612628
{
613629
"basic all arrow positive with different types",
@@ -635,6 +651,7 @@ func TestCheckPermissionOverSchema(t *testing.T) {
635651
ONR("user", "tom", "..."),
636652
v1.ResourceCheckResult_MEMBER,
637653
nil,
654+
nil,
638655
},
639656
{
640657
"basic all arrow negative over different types",
@@ -663,6 +680,7 @@ func TestCheckPermissionOverSchema(t *testing.T) {
663680
ONR("user", "tom", "..."),
664681
v1.ResourceCheckResult_NOT_MEMBER,
665682
nil,
683+
nil,
666684
},
667685
{
668686
"basic all arrow positive over different types",
@@ -692,6 +710,7 @@ func TestCheckPermissionOverSchema(t *testing.T) {
692710
ONR("user", "tom", "..."),
693711
v1.ResourceCheckResult_MEMBER,
694712
nil,
713+
nil,
695714
},
696715
{
697716
"all arrow for single org",
@@ -713,6 +732,7 @@ func TestCheckPermissionOverSchema(t *testing.T) {
713732
ONR("user", "tom", "..."),
714733
v1.ResourceCheckResult_MEMBER,
715734
nil,
735+
nil,
716736
},
717737
{
718738
"all arrow for no orgs",
@@ -733,6 +753,7 @@ func TestCheckPermissionOverSchema(t *testing.T) {
733753
ONR("user", "tom", "..."),
734754
v1.ResourceCheckResult_NOT_MEMBER,
735755
nil,
756+
nil,
736757
},
737758
{
738759
"view_by_all negative",
@@ -766,6 +787,7 @@ func TestCheckPermissionOverSchema(t *testing.T) {
766787
ONR("user", "fred", "..."),
767788
v1.ResourceCheckResult_NOT_MEMBER,
768789
nil,
790+
nil,
769791
},
770792
{
771793
"view_by_any positive",
@@ -801,6 +823,7 @@ func TestCheckPermissionOverSchema(t *testing.T) {
801823
ONR("user", "fred", "..."),
802824
v1.ResourceCheckResult_MEMBER,
803825
nil,
826+
nil,
804827
},
805828
{
806829
"view_by_any positive directly",
@@ -836,6 +859,7 @@ func TestCheckPermissionOverSchema(t *testing.T) {
836859
ONR("user", "rachel", "..."),
837860
v1.ResourceCheckResult_MEMBER,
838861
nil,
862+
nil,
839863
},
840864
{
841865
"caveated intersection arrow",
@@ -862,6 +886,7 @@ func TestCheckPermissionOverSchema(t *testing.T) {
862886
ONR("user", "tom", "..."),
863887
v1.ResourceCheckResult_CAVEATED_MEMBER,
864888
caveatAndCtx("somecaveat", nil),
889+
nil,
865890
},
866891
{
867892
"intersection arrow with caveated member",
@@ -888,6 +913,7 @@ func TestCheckPermissionOverSchema(t *testing.T) {
888913
ONR("user", "tom", "..."),
889914
v1.ResourceCheckResult_CAVEATED_MEMBER,
890915
caveatAndCtx("somecaveat", nil),
916+
nil,
891917
},
892918
{
893919
"caveated intersection arrow with caveated member",
@@ -914,6 +940,7 @@ func TestCheckPermissionOverSchema(t *testing.T) {
914940
ONR("user", "tom", "..."),
915941
v1.ResourceCheckResult_CAVEATED_MEMBER,
916942
caveatAndCtx("somecaveat", nil),
943+
nil,
917944
},
918945
{
919946
"caveated intersection arrow with caveated member, different context",
@@ -947,6 +974,7 @@ func TestCheckPermissionOverSchema(t *testing.T) {
947974
caveatAndCtx("anothercaveat", map[string]any{"someparam": int64(43)}),
948975
caveatAndCtx("somecaveat", map[string]any{"someparam": int64(42)}),
949976
),
977+
nil,
950978
},
951979
{
952980
"caveated intersection arrow with multiple caveated branches",
@@ -978,8 +1006,8 @@ func TestCheckPermissionOverSchema(t *testing.T) {
9781006
caveatAndCtx("somecaveat", map[string]any{"someparam": int64(41)}),
9791007
caveatAndCtx("somecaveat", map[string]any{"someparam": int64(42)}),
9801008
),
1009+
nil,
9811010
},
982-
9831011
{
9841012
"caveated intersection arrow with multiple caveated members",
9851013
`definition user {}
@@ -1010,6 +1038,7 @@ func TestCheckPermissionOverSchema(t *testing.T) {
10101038
caveatAndCtx("somecaveat", map[string]any{"someparam": int64(41)}),
10111039
caveatAndCtx("somecaveat", map[string]any{"someparam": int64(42)}),
10121040
),
1041+
nil,
10131042
},
10141043
{
10151044
"caveated intersection arrow with one caveated branch",
@@ -1038,6 +1067,7 @@ func TestCheckPermissionOverSchema(t *testing.T) {
10381067
ONR("user", "tom", "..."),
10391068
v1.ResourceCheckResult_CAVEATED_MEMBER,
10401069
caveatAndCtx("somecaveat", map[string]any{"someparam": int64(42)}),
1070+
nil,
10411071
},
10421072
{
10431073
"caveated intersection arrow with one caveated member",
@@ -1066,6 +1096,7 @@ func TestCheckPermissionOverSchema(t *testing.T) {
10661096
ONR("user", "tom", "..."),
10671097
v1.ResourceCheckResult_CAVEATED_MEMBER,
10681098
caveatAndCtx("somecaveat", map[string]any{"someparam": int64(42)}),
1099+
nil,
10691100
},
10701101
{
10711102
"caveated intersection arrow multiple paths to the same subject",
@@ -1093,6 +1124,7 @@ func TestCheckPermissionOverSchema(t *testing.T) {
10931124
ONR("user", "tom", "..."),
10941125
v1.ResourceCheckResult_CAVEATED_MEMBER,
10951126
caveatAndCtx("somecaveat", nil),
1127+
nil,
10961128
},
10971129
{
10981130
"recursive all arrow positive result",
@@ -1129,6 +1161,7 @@ func TestCheckPermissionOverSchema(t *testing.T) {
11291161
ONR("user", "fred", "..."),
11301162
v1.ResourceCheckResult_MEMBER,
11311163
nil,
1164+
nil,
11321165
},
11331166
{
11341167
"recursive all arrow negative result",
@@ -1165,6 +1198,7 @@ func TestCheckPermissionOverSchema(t *testing.T) {
11651198
ONR("user", "tom", "..."),
11661199
v1.ResourceCheckResult_NOT_MEMBER,
11671200
nil,
1201+
nil,
11681202
},
11691203
{
11701204
"recursive all arrow negative result due to recursion missing a folder",
@@ -1202,6 +1236,79 @@ func TestCheckPermissionOverSchema(t *testing.T) {
12021236
ONR("user", "fred", "..."),
12031237
v1.ResourceCheckResult_NOT_MEMBER,
12041238
nil,
1239+
nil,
1240+
},
1241+
{
1242+
"caveated over multiple branches",
1243+
`
1244+
caveat somecaveat(somevalue int) {
1245+
somevalue == 42
1246+
}
1247+
1248+
definition user {}
1249+
1250+
definition role {
1251+
relation member: user
1252+
}
1253+
1254+
definition resource {
1255+
relation viewer: role#member with somecaveat
1256+
permission view = viewer
1257+
}
1258+
`,
1259+
[]*core.RelationTuple{
1260+
tuple.MustParse(`role:firstrole#member@user:tom[somecaveat:{"somevalue":40}]`),
1261+
tuple.MustParse(`role:secondrole#member@user:tom[somecaveat:{"somevalue":42}]`),
1262+
tuple.MustParse(`resource:doc1#viewer@role:firstrole#member`),
1263+
tuple.MustParse(`resource:doc1#viewer@role:secondrole#member`),
1264+
},
1265+
ONR("resource", "doc1", "view"),
1266+
ONR("user", "tom", "..."),
1267+
v1.ResourceCheckResult_CAVEATED_MEMBER,
1268+
caveatOr(
1269+
caveatAndCtx("somecaveat", map[string]any{"somevalue": int64(40)}),
1270+
caveatAndCtx("somecaveat", map[string]any{"somevalue": int64(42)}),
1271+
),
1272+
caveatOr(
1273+
caveatAndCtx("somecaveat", map[string]any{"somevalue": int64(42)}),
1274+
caveatAndCtx("somecaveat", map[string]any{"somevalue": int64(40)}),
1275+
),
1276+
},
1277+
{
1278+
"caveated over multiple branches reversed",
1279+
`
1280+
caveat somecaveat(somevalue int) {
1281+
somevalue == 42
1282+
}
1283+
1284+
definition user {}
1285+
1286+
definition role {
1287+
relation member: user
1288+
}
1289+
1290+
definition resource {
1291+
relation viewer: role#member with somecaveat
1292+
permission view = viewer
1293+
}
1294+
`,
1295+
[]*core.RelationTuple{
1296+
tuple.MustParse(`role:secondrole#member@user:tom[somecaveat:{"somevalue":42}]`),
1297+
tuple.MustParse(`role:firstrole#member@user:tom[somecaveat:{"somevalue":40}]`),
1298+
tuple.MustParse(`resource:doc1#viewer@role:secondrole#member`),
1299+
tuple.MustParse(`resource:doc1#viewer@role:firstrole#member`),
1300+
},
1301+
ONR("resource", "doc1", "view"),
1302+
ONR("user", "tom", "..."),
1303+
v1.ResourceCheckResult_CAVEATED_MEMBER,
1304+
caveatOr(
1305+
caveatAndCtx("somecaveat", map[string]any{"somevalue": int64(40)}),
1306+
caveatAndCtx("somecaveat", map[string]any{"somevalue": int64(42)}),
1307+
),
1308+
caveatOr(
1309+
caveatAndCtx("somecaveat", map[string]any{"somevalue": int64(42)}),
1310+
caveatAndCtx("somecaveat", map[string]any{"somevalue": int64(40)}),
1311+
),
12051312
},
12061313
}
12071314

@@ -1239,10 +1346,18 @@ func TestCheckPermissionOverSchema(t *testing.T) {
12391346

12401347
require.Equal(tc.expectedPermissionship, membership)
12411348

1242-
if tc.expectedCaveat != nil {
1349+
if tc.expectedCaveat != nil && tc.alternativeExpectedCaveat == nil {
12431350
require.NotEmpty(resp.ResultsByResourceId[tc.resource.ObjectId].Expression)
12441351
testutil.RequireProtoEqual(t, tc.expectedCaveat, resp.ResultsByResourceId[tc.resource.ObjectId].Expression, "mismatch in caveat")
12451352
}
1353+
1354+
if tc.expectedCaveat != nil && tc.alternativeExpectedCaveat != nil {
1355+
require.NotEmpty(resp.ResultsByResourceId[tc.resource.ObjectId].Expression)
1356+
1357+
if testutil.AreProtoEqual(tc.expectedCaveat, resp.ResultsByResourceId[tc.resource.ObjectId].Expression, "mismatch in caveat") != nil {
1358+
testutil.RequireProtoEqual(t, tc.alternativeExpectedCaveat, resp.ResultsByResourceId[tc.resource.ObjectId].Expression, "mismatch in caveat")
1359+
}
1360+
}
12461361
})
12471362
}
12481363
}

0 commit comments

Comments
 (0)