Skip to content

Commit

Permalink
Ensure caveat context is sent to all LR2 dispatches
Browse files Browse the repository at this point in the history
  • Loading branch information
@josephschorr @vroldanbet
josephschorr authored and vroldanbet committed Oct 14, 2024
1 parent 0d882c7 commit 2f3cf77
Show file tree
Hide file tree
Showing 8 changed files with 224 additions and 23 deletions.
84 changes: 78 additions & 6 deletions internal/dispatch/graph/lookupresources2_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/ccoveille/go-safecast"
"github.com/stretchr/testify/require"
"go.uber.org/goleak"
"google.golang.org/protobuf/types/known/structpb"

"github.com/authzed/spicedb/internal/datastore/memdb"
"github.com/authzed/spicedb/internal/dispatch"
Expand Down Expand Up @@ -339,12 +340,14 @@ func TestMaxDepthLookup2(t *testing.T) {

func TestLookupResources2OverSchemaWithCursors(t *testing.T) {
testCases := []struct {
name string
schema string
relationships []*core.RelationTuple
permission *core.RelationReference
subject *core.ObjectAndRelation
expectedResourceIDs []string
name string
schema string
relationships []*core.RelationTuple
permission *core.RelationReference
subject *core.ObjectAndRelation
optionalCaveatContext map[string]any
expectedResourceIDs []string
expectedMissingFields []string
}{
{
"basic union",
Expand All @@ -361,7 +364,9 @@ func TestLookupResources2OverSchemaWithCursors(t *testing.T) {
),
RR("document", "view"),
ONR("user", "tom", "..."),
nil,
genResourceIds("document", 1510),
nil,
},
{
"basic exclusion",
Expand All @@ -375,7 +380,9 @@ func TestLookupResources2OverSchemaWithCursors(t *testing.T) {
genTuples("document", "viewer", "user", "tom", 1010),
RR("document", "view"),
ONR("user", "tom", "..."),
nil,
genResourceIds("document", 1010),
nil,
},
{
"basic intersection",
Expand All @@ -392,7 +399,9 @@ func TestLookupResources2OverSchemaWithCursors(t *testing.T) {
),
RR("document", "view"),
ONR("user", "tom", "..."),
nil,
genResourceIds("document", 510),
nil,
},
{
"union and excluded union",
Expand All @@ -411,7 +420,9 @@ func TestLookupResources2OverSchemaWithCursors(t *testing.T) {
),
RR("document", "view"),
ONR("user", "tom", "..."),
nil,
genResourceIds("document", 2450),
nil,
},
{
"basic caveats",
Expand All @@ -428,7 +439,9 @@ func TestLookupResources2OverSchemaWithCursors(t *testing.T) {
genTuplesWithCaveat("document", "viewer", "user", "tom", "somecaveat", map[string]any{"somecondition": 42}, 0, 2450),
RR("document", "view"),
ONR("user", "tom", "..."),
nil,
genResourceIds("document", 2450),
nil,
},
{
"excluded items",
Expand All @@ -445,7 +458,9 @@ func TestLookupResources2OverSchemaWithCursors(t *testing.T) {
),
RR("document", "view"),
ONR("user", "tom", "..."),
nil,
genResourceIds("document", 1210),
nil,
},
{
"basic caveats with missing field",
Expand All @@ -462,7 +477,9 @@ func TestLookupResources2OverSchemaWithCursors(t *testing.T) {
genTuplesWithCaveat("document", "viewer", "user", "tom", "somecaveat", map[string]any{}, 0, 2450),
RR("document", "view"),
ONR("user", "tom", "..."),
nil,
genResourceIds("document", 2450),
[]string{"somecondition"},
},
{
"larger arrow dispatch",
Expand All @@ -482,7 +499,9 @@ func TestLookupResources2OverSchemaWithCursors(t *testing.T) {
),
RR("document", "view"),
ONR("user", "tom", "..."),
nil,
genResourceIds("document", 150),
nil,
},
{
"big",
Expand All @@ -499,7 +518,9 @@ func TestLookupResources2OverSchemaWithCursors(t *testing.T) {
),
RR("document", "view"),
ONR("user", "tom", "..."),
nil,
genResourceIds("document", 15100),
nil,
},
{
"arrow under intersection",
Expand All @@ -523,7 +544,9 @@ func TestLookupResources2OverSchemaWithCursors(t *testing.T) {
),
RR("document", "view"),
ONR("user", "tom", "..."),
nil,
genResourceIds("document", 510),
nil,
},
{
"all arrow",
Expand Down Expand Up @@ -563,7 +586,9 @@ func TestLookupResources2OverSchemaWithCursors(t *testing.T) {
},
RR("document", "view"),
ONR("user", "tom", "..."),
nil,
[]string{"doc0", "doc1", "doc4"},
nil,
},
{
"indirect intersection and exclusion",
Expand All @@ -583,7 +608,9 @@ func TestLookupResources2OverSchemaWithCursors(t *testing.T) {
),
RR("document", "view"),
ONR("user", "tom", "..."),
nil,
genResourceIds("document", 1410),
nil,
},
{
"indirect intersections",
Expand Down Expand Up @@ -611,7 +638,9 @@ func TestLookupResources2OverSchemaWithCursors(t *testing.T) {
),
RR("document", "view"),
ONR("user", "tom", "..."),
nil,
genResourceIds("document", 1410),
nil,
},
{
"indirect over arrow",
Expand Down Expand Up @@ -639,7 +668,9 @@ func TestLookupResources2OverSchemaWithCursors(t *testing.T) {
),
RR("document", "view"),
ONR("user", "tom", "..."),
nil,
genResourceIds("document", 1410),
nil,
},
{
"root indirect with intermediate shearing",
Expand Down Expand Up @@ -675,7 +706,40 @@ func TestLookupResources2OverSchemaWithCursors(t *testing.T) {
),
RR("document", "view"),
ONR("user", "tom", "..."),
nil,
genResourceIds("document", 2000),
nil,
},
{
"indirect caveats",
`caveat somecaveat(somevalue int) {
somevalue == 42
}
definition user {}
definition container {
relation access: user with somecaveat
permission accesses = access
}
definition document {
relation container: container
relation viewer: user with somecaveat
permission view = viewer & container->accesses
}`,
joinTuples(
[]*core.RelationTuple{
tuple.MustParse("container:somecontainer#access@user:tom[somecaveat]"),
},
genTuplesWithCaveat("document", "viewer", "user", "tom", "somecaveat", map[string]any{}, 0, 2450),
genTuples("document", "container", "container", "somecontainer", 2450),
),
RR("document", "view"),
ONR("user", "tom", "..."),
map[string]any{"somevalue": 42},
genResourceIds("document", 2450),
nil,
},
}

Expand Down Expand Up @@ -706,6 +770,12 @@ func TestLookupResources2OverSchemaWithCursors(t *testing.T) {
uintPageSize, err := safecast.ToUint32(pageSize)
require.NoError(err)

var caveatContext *structpb.Struct
if tc.optionalCaveatContext != nil {
caveatContext, err = structpb.NewStruct(tc.optionalCaveatContext)
require.NoError(err)
}

err = dispatcher.DispatchLookupResources2(&v1.DispatchLookupResources2Request{
ResourceRelation: tc.permission,
SubjectRelation: RR(tc.subject.Namespace, "..."),
Expand All @@ -717,6 +787,7 @@ func TestLookupResources2OverSchemaWithCursors(t *testing.T) {
},
OptionalLimit: uintPageSize,
OptionalCursor: currentCursor,
Context: caveatContext,
}, stream)
require.NoError(err)

Expand All @@ -727,6 +798,7 @@ func TestLookupResources2OverSchemaWithCursors(t *testing.T) {
foundChunks = append(foundChunks, stream.Results())

for _, result := range stream.Results() {
require.ElementsMatch(tc.expectedMissingFields, result.Resource.MissingContextParams)
foundResourceIDs.Insert(result.Resource.ResourceId)
currentCursor = result.AfterResponseCursor
}
Expand Down
1 change: 1 addition & 0 deletions internal/graph/lookupresources2.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -612,6 +612,7 @@ func (crr *CursoredLookupResources2) redispatchOrReport(
},
OptionalCursor: ci.currentCursor,
OptionalLimit: parentRequest.OptionalLimit,
Context: parentRequest.Context,
}, stream)
}

Expand Down
1 change: 1 addition & 0 deletions internal/graph/lr2streams.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ func (rdc *checkAndDispatchRunner) runDispatch(
},
OptionalCursor: updatedCi.currentCursor,
OptionalLimit: rdc.ci.limits.currentLimit,
Context: rdc.parentRequest.Context,
}, wrappedStream)
}

Expand Down
60 changes: 45 additions & 15 deletions internal/services/integrationtesting/consistency_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,8 @@ func requireSubsetOf(t *testing.T, found []string, expected []string) {
// validateLookupResources ensures that a lookup resources call returns the expected objects and
// only those expected.
func validateLookupResources(t *testing.T, vctx validationContext) {
// Run a lookup resources for each resource type and ensure that the returned objects are those
// that are accessible to the subject.
testForEachResourceType(t, vctx, "validate_lookup_resources",
func(t *testing.T, resourceRelation *core.RelationReference) {
for _, subject := range vctx.accessibilitySet.AllSubjectsNoWildcards() {
Expand All @@ -375,7 +377,7 @@ func validateLookupResources(t *testing.T, vctx validationContext) {
var currentCursor *v1.Cursor
resolvedResources := map[string]*v1.LookupResourcesResponse{}
for i := 0; i < 100; i++ {
foundResources, lastCursor, err := vctx.serviceTester.LookupResources(context.Background(), resourceRelation, subject, vctx.revision, currentCursor, pageSize)
foundResources, lastCursor, err := vctx.serviceTester.LookupResources(context.Background(), resourceRelation, subject, vctx.revision, currentCursor, pageSize, nil)
require.NoError(t, err)

if pageSize > 0 {
Expand Down Expand Up @@ -676,45 +678,73 @@ func runAssertions(t *testing.T, vctx validationContext) {
require.NoError(t, err)
require.Equal(t, entry.expectedPermissionship, permissionship, "Assertion `%s` returned %s; expected %s", tuple.MustString(rel), permissionship, entry.expectedPermissionship)

// Ensure the assertion passes LookupResources.
resolvedResources, _, err := vctx.serviceTester.LookupResources(context.Background(), &core.RelationReference{
// Ensure the assertion passes LookupResources with context, directly.
resolvedDirectResources, _, err := vctx.serviceTester.LookupResources(context.Background(), &core.RelationReference{
Namespace: rel.ResourceAndRelation.Namespace,
Relation: rel.ResourceAndRelation.Relation,
}, rel.Subject, vctx.revision, nil, 0)
}, rel.Subject, vctx.revision, nil, 0, assertion.CaveatContext)
require.NoError(t, err)

resolvedResourcesMap := map[string]*v1.LookupResourcesResponse{}
for _, resource := range resolvedResources {
resolvedResourcesMap[resource.ResourceObjectId] = resource
resolvedDirectResourcesMap := map[string]*v1.LookupResourcesResponse{}
for _, resource := range resolvedDirectResources {
resolvedDirectResourcesMap[resource.ResourceObjectId] = resource
}

resolvedResourceIds := maps.Keys(resolvedResourcesMap)
// Ensure the assertion passes LookupResources without context, indirectly.
resolvedIndirectResources, _, err := vctx.serviceTester.LookupResources(context.Background(), &core.RelationReference{
Namespace: rel.ResourceAndRelation.Namespace,
Relation: rel.ResourceAndRelation.Relation,
}, rel.Subject, vctx.revision, nil, 0, nil)
require.NoError(t, err)

resolvedIndirectResourcesMap := map[string]*v1.LookupResourcesResponse{}
for _, resource := range resolvedIndirectResources {
resolvedIndirectResourcesMap[resource.ResourceObjectId] = resource
}

// Check the assertion was returned for a direct (with context) lookup.
resolvedDirectResourceIds := maps.Keys(resolvedDirectResourcesMap)
switch permissionship {
case v1.CheckPermissionResponse_PERMISSIONSHIP_NO_PERMISSION:
require.NotContains(t, resolvedDirectResourceIds, rel.ResourceAndRelation.ObjectId, "Found unexpected object %s in direct lookup for assertion %s", rel.ResourceAndRelation, rel)

case v1.CheckPermissionResponse_PERMISSIONSHIP_HAS_PERMISSION:
require.Contains(t, resolvedDirectResourceIds, rel.ResourceAndRelation.ObjectId, "Missing object %s in lookup for assertion %s", rel.ResourceAndRelation, rel)
require.Equal(t, v1.LookupPermissionship_LOOKUP_PERMISSIONSHIP_HAS_PERMISSION, resolvedDirectResourcesMap[rel.ResourceAndRelation.ObjectId].Permissionship)

case v1.CheckPermissionResponse_PERMISSIONSHIP_CONDITIONAL_PERMISSION:
require.Contains(t, resolvedDirectResourceIds, rel.ResourceAndRelation.ObjectId, "Missing object %s in lookup for assertion %s", rel.ResourceAndRelation, rel)
require.Equal(t, v1.LookupPermissionship_LOOKUP_PERMISSIONSHIP_CONDITIONAL_PERMISSION, resolvedDirectResourcesMap[rel.ResourceAndRelation.ObjectId].Permissionship)
}

// Check the assertion was returned for an indirect (without context) lookup.
resolvedIndirectResourceIds := maps.Keys(resolvedIndirectResourcesMap)
accessibility, _, _ := vctx.accessibilitySet.AccessibiliyAndPermissionshipFor(rel.ResourceAndRelation, rel.Subject)

switch permissionship {
case v1.CheckPermissionResponse_PERMISSIONSHIP_NO_PERMISSION:
// If the caveat context given is empty, then the lookup result must not exist at all.
// Otherwise, it *could* be caveated or not exist, depending on the context given.
if len(assertion.CaveatContext) == 0 {
require.NotContains(t, resolvedResourceIds, rel.ResourceAndRelation.ObjectId, "Found unexpected object %s in lookup for assertion %s", rel.ResourceAndRelation, rel)
require.NotContains(t, resolvedIndirectResourceIds, rel.ResourceAndRelation.ObjectId, "Found unexpected object %s in indirect lookup for assertion %s", rel.ResourceAndRelation, rel)
} else if accessibility == consistencytestutil.NotAccessible {
found, ok := resolvedResourcesMap[rel.ResourceAndRelation.ObjectId]
found, ok := resolvedIndirectResourcesMap[rel.ResourceAndRelation.ObjectId]
require.True(t, !ok || found.Permissionship != v1.LookupPermissionship_LOOKUP_PERMISSIONSHIP_HAS_PERMISSION) // LookupResources can be caveated, since we didn't rerun LookupResources with the context
} else if accessibility != consistencytestutil.NotAccessibleDueToPrespecifiedCaveat {
require.Equal(t, v1.LookupPermissionship_LOOKUP_PERMISSIONSHIP_CONDITIONAL_PERMISSION, resolvedResourcesMap[rel.ResourceAndRelation.ObjectId].Permissionship)
require.Equal(t, v1.LookupPermissionship_LOOKUP_PERMISSIONSHIP_CONDITIONAL_PERMISSION, resolvedIndirectResourcesMap[rel.ResourceAndRelation.ObjectId].Permissionship)
}

case v1.CheckPermissionResponse_PERMISSIONSHIP_HAS_PERMISSION:
require.Contains(t, resolvedResourceIds, rel.ResourceAndRelation.ObjectId, "Missing object %s in lookup for assertion %s", rel.ResourceAndRelation, rel)
require.Contains(t, resolvedIndirectResourceIds, rel.ResourceAndRelation.ObjectId, "Missing object %s in lookup for assertion %s", rel.ResourceAndRelation, rel)
// If the caveat context given is empty, then the lookup result must be fully permissioned.
// Otherwise, it *could* be caveated or fully permissioned, depending on the context given.
if len(assertion.CaveatContext) == 0 {
require.Equal(t, v1.LookupPermissionship_LOOKUP_PERMISSIONSHIP_HAS_PERMISSION, resolvedResourcesMap[rel.ResourceAndRelation.ObjectId].Permissionship)
require.Equal(t, v1.LookupPermissionship_LOOKUP_PERMISSIONSHIP_HAS_PERMISSION, resolvedIndirectResourcesMap[rel.ResourceAndRelation.ObjectId].Permissionship)
}

case v1.CheckPermissionResponse_PERMISSIONSHIP_CONDITIONAL_PERMISSION:
require.Contains(t, resolvedResourceIds, rel.ResourceAndRelation.ObjectId, "Missing object %s in lookup for assertion %s", rel.ResourceAndRelation, rel)
require.Equal(t, v1.LookupPermissionship_LOOKUP_PERMISSIONSHIP_CONDITIONAL_PERMISSION, resolvedResourcesMap[rel.ResourceAndRelation.ObjectId].Permissionship)
require.Contains(t, resolvedIndirectResourceIds, rel.ResourceAndRelation.ObjectId, "Missing object %s in lookup for assertion %s", rel.ResourceAndRelation, rel)
require.Equal(t, v1.LookupPermissionship_LOOKUP_PERMISSIONSHIP_CONDITIONAL_PERMISSION, resolvedIndirectResourcesMap[rel.ResourceAndRelation.ObjectId].Permissionship)

default:
panic("unknown permissionship")
Expand Down
14 changes: 12 additions & 2 deletions internal/services/integrationtesting/consistencytestutil/servicetester.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ type ServiceTester interface {
Expand(ctx context.Context, resource *core.ObjectAndRelation, atRevision datastore.Revision) (*core.RelationTupleTreeNode, error)
Write(ctx context.Context, relationship *core.RelationTuple) error
Read(ctx context.Context, namespaceName string, atRevision datastore.Revision) ([]*core.RelationTuple, error)
LookupResources(ctx context.Context, resourceRelation *core.RelationReference, subject *core.ObjectAndRelation, atRevision datastore.Revision, cursor *v1.Cursor, limit uint32) ([]*v1.LookupResourcesResponse, *v1.Cursor, error)
LookupResources(ctx context.Context, resourceRelation *core.RelationReference, subject *core.ObjectAndRelation, atRevision datastore.Revision, cursor *v1.Cursor, limit uint32, caveatContext map[string]any) ([]*v1.LookupResourcesResponse, *v1.Cursor, error)
LookupSubjects(ctx context.Context, resource *core.ObjectAndRelation, subjectRelation *core.RelationReference, atRevision datastore.Revision, caveatContext map[string]any) (map[string]*v1.LookupSubjectsResponse, error)
// NOTE: ExperimentalService/BulkCheckPermission has been promoted to PermissionsService/CheckBulkPermissions
BulkCheck(ctx context.Context, items []*v1.BulkCheckPermissionRequestItem, atRevision datastore.Revision) ([]*v1.BulkCheckPermissionPair, error)
Expand Down Expand Up @@ -153,7 +153,16 @@ func (v1st v1ServiceTester) Read(_ context.Context, namespaceName string, atRevi
return tuples, nil
}

func (v1st v1ServiceTester) LookupResources(_ context.Context, resourceRelation *core.RelationReference, subject *core.ObjectAndRelation, atRevision datastore.Revision, cursor *v1.Cursor, limit uint32) ([]*v1.LookupResourcesResponse, *v1.Cursor, error) {
func (v1st v1ServiceTester) LookupResources(_ context.Context, resourceRelation *core.RelationReference, subject *core.ObjectAndRelation, atRevision datastore.Revision, cursor *v1.Cursor, limit uint32, caveatContext map[string]any) ([]*v1.LookupResourcesResponse, *v1.Cursor, error) {
var builtContext *structpb.Struct
if caveatContext != nil {
built, err := structpb.NewStruct(caveatContext)
if err != nil {
return nil, nil, err
}
builtContext = built
}

lookupResp, err := v1st.permClient.LookupResources(context.Background(), &v1.LookupResourcesRequest{
ResourceObjectType: resourceRelation.Namespace,
Permission: resourceRelation.Relation,
Expand All @@ -171,6 +180,7 @@ func (v1st v1ServiceTester) LookupResources(_ context.Context, resourceRelation
},
OptionalLimit: limit,
OptionalCursor: cursor,
Context: builtContext,
})
if err != nil {
return nil, nil, err
Expand Down
Loading

0 comments on commit 2f3cf77

Please sign in to comment.