Skip to content

Commit c39bf5f

Browse files
authored
feat: add internal flag for running CheckPermission with a query plan (#2663)
1 parent 268bc53 commit c39bf5f

17 files changed

+245
-142
lines changed

internal/services/integrationtesting/query_plan_consistency_test.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,7 @@ func (q *queryPlanConsistencyHandle) buildContext(t *testing.T) *query.Context {
5050
return &query.Context{
5151
Context: t.Context(),
5252
Executor: query.LocalExecutor{},
53-
Datastore: q.ds,
54-
Revision: q.revision,
53+
Reader: q.ds.SnapshotReader(q.revision),
5554
CaveatRunner: caveats.NewCaveatRunner(caveattypes.Default.TypeSet),
5655
TraceLogger: query.NewTraceLogger(), // Enable tracing for debugging
5756
}

internal/services/v1/permissions.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,10 @@ func (ps *permissionServer) CheckPermission(ctx context.Context, req *v1.CheckPe
7070

7171
telemetry.LogicalChecks.Inc()
7272

73+
if ps.config.ExperimentalQueryPlan {
74+
return ps.checkPermissionWithQueryPlan(ctx, req)
75+
}
76+
7377
atRevision, checkedAt, err := consistency.RevisionFromContext(ctx)
7478
if err != nil {
7579
return nil, ps.rewriteError(ctx, err)
Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
package v1
2+
3+
import (
4+
"context"
5+
6+
v1 "github.com/authzed/authzed-go/proto/authzed/api/v1"
7+
8+
caveatsimpl "github.com/authzed/spicedb/internal/caveats"
9+
datastoremw "github.com/authzed/spicedb/internal/middleware/datastore"
10+
"github.com/authzed/spicedb/pkg/datastore"
11+
"github.com/authzed/spicedb/pkg/middleware/consistency"
12+
"github.com/authzed/spicedb/pkg/query"
13+
"github.com/authzed/spicedb/pkg/schema/v2"
14+
)
15+
16+
// checkPermissionWithQueryPlan executes a permission check using the query plan API.
17+
// This builds an iterator tree from the schema and executes it against the datastore.
18+
func (ps *permissionServer) checkPermissionWithQueryPlan(ctx context.Context, req *v1.CheckPermissionRequest) (*v1.CheckPermissionResponse, error) {
19+
atRevision, checkedAt, err := consistency.RevisionFromContext(ctx)
20+
if err != nil {
21+
return nil, ps.rewriteError(ctx, err)
22+
}
23+
24+
ds := datastoremw.MustFromContext(ctx)
25+
reader := ds.SnapshotReader(atRevision)
26+
27+
// Load all namespace and caveat definitions to build the schema
28+
// TODO: Better schema caching
29+
namespaces, err := reader.ListAllNamespaces(ctx)
30+
if err != nil {
31+
return nil, ps.rewriteError(ctx, err)
32+
}
33+
34+
caveats, err := reader.ListAllCaveats(ctx)
35+
if err != nil {
36+
return nil, ps.rewriteError(ctx, err)
37+
}
38+
39+
// Build schema from definitions
40+
fullSchema, err := schema.BuildSchemaFromDefinitions(
41+
datastore.DefinitionsOf(namespaces),
42+
datastore.DefinitionsOf(caveats),
43+
)
44+
if err != nil {
45+
return nil, ps.rewriteError(ctx, err)
46+
}
47+
48+
// Build iterator tree from schema
49+
// TODO: Better iterator caching
50+
it, err := query.BuildIteratorFromSchema(fullSchema, req.Resource.ObjectType, req.Permission)
51+
if err != nil {
52+
return nil, ps.rewriteError(ctx, err)
53+
}
54+
55+
// Parse caveat context if provided
56+
caveatContext, err := GetCaveatContext(ctx, req.Context, ps.config.MaxCaveatContextSize)
57+
if err != nil {
58+
return nil, ps.rewriteError(ctx, err)
59+
}
60+
61+
// Create query context with optional tracing
62+
qctx := &query.Context{
63+
Context: ctx,
64+
Executor: query.LocalExecutor{},
65+
Reader: reader,
66+
CaveatContext: caveatContext,
67+
CaveatRunner: caveatsimpl.NewCaveatRunner(ps.config.CaveatTypeSet),
68+
}
69+
70+
// Execute the check
71+
resource := query.Object{
72+
ObjectType: req.Resource.ObjectType,
73+
ObjectID: req.Resource.ObjectId,
74+
}
75+
76+
subject := query.ObjectAndRelation{
77+
ObjectType: req.Subject.Object.ObjectType,
78+
ObjectID: req.Subject.Object.ObjectId,
79+
Relation: normalizeSubjectRelation(req.Subject),
80+
}
81+
82+
pathSeq, err := qctx.Check(it, []query.Object{resource}, subject)
83+
if err != nil {
84+
return nil, ps.rewriteError(ctx, err)
85+
}
86+
87+
// Collect results and convert to response
88+
permissionship, partialCaveat, err := convertPathsToPermissionship(pathSeq)
89+
if err != nil {
90+
return nil, ps.rewriteError(ctx, err)
91+
}
92+
93+
resp := &v1.CheckPermissionResponse{
94+
CheckedAt: checkedAt,
95+
Permissionship: permissionship,
96+
PartialCaveatInfo: partialCaveat,
97+
}
98+
99+
return resp, nil
100+
}
101+
102+
// convertPathsToPermissionship iterates over paths and determines the permissionship result.
103+
// Returns the first path's permissionship status, as any path indicates access.
104+
func convertPathsToPermissionship(pathSeq query.PathSeq) (v1.CheckPermissionResponse_Permissionship, *v1.PartialCaveatInfo, error) {
105+
// Iterate over paths to find the first valid result
106+
for path, err := range pathSeq {
107+
if err != nil {
108+
return v1.CheckPermissionResponse_PERMISSIONSHIP_UNSPECIFIED, nil, err
109+
}
110+
111+
// Found a path - determine permissionship based on caveat presence
112+
if path.Caveat != nil {
113+
// TODO: Extract missing required context from caveat expression
114+
return v1.CheckPermissionResponse_PERMISSIONSHIP_CONDITIONAL_PERMISSION, &v1.PartialCaveatInfo{
115+
MissingRequiredContext: []string{},
116+
}, nil
117+
}
118+
119+
// Path exists without caveat - has permission
120+
return v1.CheckPermissionResponse_PERMISSIONSHIP_HAS_PERMISSION, nil, nil
121+
}
122+
123+
// No paths found - no permission
124+
return v1.CheckPermissionResponse_PERMISSIONSHIP_NO_PERMISSION, nil, nil
125+
}

internal/services/v1/relationships.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,9 @@ type PermissionsServerConfig struct {
115115

116116
// EnableExperimentalLookupResources3 is used to enable LookupResources v3 for testing.
117117
EnableExperimentalLookupResources3 bool // TODO: remove when LookupResources v3 is fully enabled
118+
119+
// ExperimentalQueryPlan enables the experimental query plan for API calls.
120+
ExperimentalQueryPlan bool
118121
}
119122

120123
// NewPermissionsServer creates a PermissionsServiceServer instance.
@@ -140,6 +143,7 @@ func NewPermissionsServer(
140143
ExpiringRelationshipsEnabled: config.ExpiringRelationshipsEnabled,
141144
PerformanceInsightMetricsEnabled: config.PerformanceInsightMetricsEnabled,
142145
EnableExperimentalLookupResources3: config.EnableExperimentalLookupResources3,
146+
ExperimentalQueryPlan: config.ExperimentalQueryPlan,
143147
}
144148

145149
return &permissionServer{

pkg/cmd/serve.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,10 @@ func RegisterServeFlags(cmd *cobra.Command, config *server.Config) error {
167167

168168
experimentalFlags := nfs.FlagSet(BoldBlue("Experimental"))
169169
// Flags for experimental features
170+
experimentalFlags.StringVar(&config.ExperimentalQueryPlan, "experimental-query-plan", "", "if non-empty, the version of the experimental query plan to use: `check` or empty")
171+
if err := experimentalFlags.MarkHidden("experimental-query-plan"); err != nil {
172+
return fmt.Errorf("failed to mark flag as hidden: %w", err)
173+
}
170174
experimentalFlags.StringVar(&config.ExperimentalLookupResourcesVersion, "experimental-lookup-resources-version", "", "if non-empty, the version of the experimental lookup resources API to use: `lr3` or empty")
171175
experimentalFlags.BoolVar(&config.EnableRelationshipExpiration, "enable-experimental-relationship-expiration", true, "enables experimental support for relationship expiration")
172176
if err := experimentalFlags.MarkHidden("enable-experimental-relationship-expiration"); err != nil {

pkg/cmd/server/server.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@ type Config struct {
128128
MaxBulkExportRelationshipsLimit uint32 `debugmap:"visible"`
129129
EnableExperimentalLookupResources bool `debugmap:"visible"`
130130
ExperimentalLookupResourcesVersion string `debugmap:"visible"`
131+
ExperimentalQueryPlan string `debugmap:"visible"`
131132
EnableRelationshipExpiration bool `debugmap:"visible" default:"true"`
132133
EnableRevisionHeartbeat bool `debugmap:"visible"`
133134
EnablePerformanceInsightMetrics bool `debugmap:"visible"`
@@ -498,6 +499,7 @@ func (c *Config) Complete(ctx context.Context) (RunnableServer, error) {
498499
CaveatTypeSet: c.DatastoreConfig.CaveatTypeSet,
499500
PerformanceInsightMetricsEnabled: c.EnablePerformanceInsightMetrics,
500501
EnableExperimentalLookupResources3: c.ExperimentalLookupResourcesVersion == "lr3",
502+
ExperimentalQueryPlan: c.ExperimentalQueryPlan == "check",
501503
}
502504

503505
healthManager := health.NewHealthManager(dispatcher, ds)

pkg/cmd/server/zz_generated.options.go

Lines changed: 9 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/query/build_tree_test.go

Lines changed: 21 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,9 @@ func TestBuildTree(t *testing.T) {
3131
require.NoError(err)
3232

3333
ctx := &Context{
34-
Context: t.Context(),
35-
Executor: LocalExecutor{},
36-
Datastore: ds,
37-
Revision: revision,
34+
Context: t.Context(),
35+
Executor: LocalExecutor{},
36+
Reader: ds.SnapshotReader(revision),
3837
}
3938

4039
relSeq, err := ctx.Check(it, NewObjects("document", "specialplan"), NewObject("user", "multiroleguy").WithEllipses())
@@ -65,10 +64,9 @@ func TestBuildTreeMultipleRelations(t *testing.T) {
6564
require.Contains(explain.String(), "Union", "edit permission should create a union iterator")
6665

6766
ctx := &Context{
68-
Context: t.Context(),
69-
Executor: LocalExecutor{},
70-
Datastore: ds,
71-
Revision: revision,
67+
Context: t.Context(),
68+
Executor: LocalExecutor{},
69+
Reader: ds.SnapshotReader(revision),
7270
}
7371

7472
relSeq, err := ctx.Check(it, NewObjects("document", "specialplan"), NewObject("user", "multiroleguy").WithEllipses())
@@ -120,10 +118,9 @@ func TestBuildTreeSubRelations(t *testing.T) {
120118
require.NotEmpty(explain.String())
121119

122120
ctx := &Context{
123-
Context: t.Context(),
124-
Executor: LocalExecutor{},
125-
Datastore: ds,
126-
Revision: revision,
121+
Context: t.Context(),
122+
Executor: LocalExecutor{},
123+
Reader: ds.SnapshotReader(revision),
127124
}
128125

129126
// Just test that the iterator can be executed without error
@@ -223,10 +220,9 @@ func TestBuildTreeIntersectionOperation(t *testing.T) {
223220
require.Contains(explain.String(), "Intersection", "should create intersection iterator")
224221

225222
ctx := &Context{
226-
Context: t.Context(),
227-
Executor: LocalExecutor{},
228-
Datastore: ds,
229-
Revision: revision,
223+
Context: t.Context(),
224+
Executor: LocalExecutor{},
225+
Reader: ds.SnapshotReader(revision),
230226
}
231227

232228
// Test execution
@@ -290,10 +286,9 @@ func TestBuildTreeExclusionEdgeCases(t *testing.T) {
290286
ds, revision := testfixtures.StandardDatastoreWithData(rawDS, require)
291287

292288
ctx := &Context{
293-
Context: t.Context(),
294-
Executor: LocalExecutor{},
295-
Datastore: ds,
296-
Revision: revision,
289+
Context: t.Context(),
290+
Executor: LocalExecutor{},
291+
Reader: ds.SnapshotReader(revision),
297292
}
298293

299294
userDef := testfixtures.UserNS.CloneVT()
@@ -554,10 +549,9 @@ func TestBuildTreeSingleRelationOptimization(t *testing.T) {
554549
require.Contains(explain.String(), "Relation", "should create relation iterator")
555550

556551
ctx := &Context{
557-
Context: t.Context(),
558-
Executor: LocalExecutor{},
559-
Datastore: ds,
560-
Revision: revision,
552+
Context: t.Context(),
553+
Executor: LocalExecutor{},
554+
Reader: ds.SnapshotReader(revision),
561555
}
562556

563557
// Test execution
@@ -578,10 +572,9 @@ func TestBuildTreeSubrelationHandling(t *testing.T) {
578572
ds, revision := testfixtures.StandardDatastoreWithData(rawDS, require)
579573

580574
ctx := &Context{
581-
Context: t.Context(),
582-
Executor: LocalExecutor{},
583-
Datastore: ds,
584-
Revision: revision,
575+
Context: t.Context(),
576+
Executor: LocalExecutor{},
577+
Reader: ds.SnapshotReader(revision),
585578
}
586579

587580
userDef := testfixtures.UserNS.CloneVT()

pkg/query/caveat.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -125,16 +125,13 @@ func (c *CaveatIterator) simplifyCaveat(ctx *Context, path Path) (*core.CaveatEx
125125
return nil, false, errors.New("no caveat runner available for caveat evaluation")
126126
}
127127

128-
// Get a snapshot reader which should implement CaveatReader
129-
reader := ctx.Datastore.SnapshotReader(ctx.Revision)
130-
131128
// Use the SimplifyCaveatExpression function to properly handle AND/OR logic
132129
simplified, passes, err := SimplifyCaveatExpression(
133130
ctx,
134131
ctx.CaveatRunner,
135132
path.Caveat,
136133
ctx.CaveatContext,
137-
reader,
134+
ctx.Reader,
138135
)
139136
if err != nil {
140137
return nil, false, fmt.Errorf("failed to simplify caveat: %w", err)

pkg/query/caveat_test.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -111,8 +111,7 @@ func TestCaveatIteratorNoCaveat(t *testing.T) {
111111
queryCtx := &Context{
112112
Context: context.Background(),
113113
Executor: LocalExecutor{},
114-
Datastore: ds,
115-
Revision: rev,
114+
Reader: ds.SnapshotReader(rev),
116115
CaveatContext: tc.caveatContext,
117116
CaveatRunner: caveats.NewCaveatRunner(types.NewTypeSet()),
118117
}
@@ -207,8 +206,7 @@ func TestCaveatIteratorWithCaveat(t *testing.T) {
207206
queryCtx := &Context{
208207
Context: context.Background(),
209208
Executor: LocalExecutor{},
210-
Datastore: ds,
211-
Revision: rev,
209+
Reader: ds.SnapshotReader(rev),
212210
CaveatContext: tc.caveatContext,
213211
CaveatRunner: caveats.NewCaveatRunner(types.NewTypeSet()),
214212
}

0 commit comments

Comments
 (0)