feat: add max-depth parameter for check and global max-depth (#791)

The parameter `max-depth` for the check command limits the depth of the search, a safeguard against particularly expensive queries. This allows users more fine-grain control. Furthermore, there is now a global max-depth configuration value that limits the overall max-depth of check and expand operations. It defaults to `5`, which is considered a very safe value.
ory · Dec 10, 2021 · 1e3b63f · 1e3b63f
1 parent f9c4123
commit 1e3b63f
Show file tree

Hide file tree

Showing 25 changed files with 341 additions and 78 deletions.
diff --git a/cmd/check/root.go b/cmd/check/root.go
@@ -22,6 +22,8 @@ func (o *checkOutput) String() string {
 	return "Denied\n"
 }
 
+const FlagMaxDepth = "max-depth"
+
 func newCheckCmd() *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "check <subject> <relation> <namespace> <object>",
@@ -35,6 +37,11 @@ func newCheckCmd() *cobra.Command {
 			}
 			defer conn.Close()
 
+			maxDepth, err := cmd.Flags().GetInt32(FlagMaxDepth)
+			if err != nil {
+				return err
+			}
+
 			cl := acl.NewCheckServiceClient(conn)
 			resp, err := cl.Check(cmd.Context(), &acl.CheckRequest{
 				Subject: &acl.Subject{
@@ -43,6 +50,7 @@ func newCheckCmd() *cobra.Command {
 				Relation:  args[1],
 				Namespace: args[2],
 				Object:    args[3],
+				MaxDepth:  maxDepth,
 			})
 			if err != nil {
 				_, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Could not make request: %s\n", err)
@@ -56,6 +64,7 @@ func newCheckCmd() *cobra.Command {
 
 	client.RegisterRemoteURLFlags(cmd.Flags())
 	cmdx.RegisterFormatFlags(cmd.Flags())
+	cmd.Flags().Int32P(FlagMaxDepth, "d", 0, "Maximum depth of the search tree. If the value is less than 1 or greater than the global max-depth then the global max-depth will be used instead.")
 
 	return cmd
 }

diff --git a/cmd/expand/root.go b/cmd/expand/root.go
@@ -72,7 +72,7 @@ func NewExpandCmd() *cobra.Command {
 	client.RegisterRemoteURLFlags(cmd.Flags())
 	cmdx.RegisterJSONFormatFlags(cmd.Flags())
 	cmdx.RegisterNoiseFlags(cmd.Flags())
-	cmd.Flags().Int32P(FlagMaxDepth, "d", 100, "maximum depth of the tree")
+	cmd.Flags().Int32P(FlagMaxDepth, "d", 0, "Maximum depth of the tree to be returned. If the value is less than 1 or greater than the global max-depth then the global max-depth will be used instead.")
 
 	return cmd
 }

diff --git a/docs/docs/cli/keto-check.md b/docs/docs/cli/keto-check.md
@@ -28,6 +28,7 @@ keto check &lt;subject&gt; &lt;relation&gt; &lt;namespace&gt; &lt;object&gt; [fl
 ```
   -f, --format string         Set the output format. One of table, json, and json-pretty. (default &#34;default&#34;)
   -h, --help                  help for check
+  -d, --max-depth int32       Maximum depth of the search tree. If the value is less than 1 or greater than the global max-depth then the global max-depth will be used instead.
   -q, --quiet                 Be quiet with output printing.
       --read-remote string    Remote URL of the read API endpoint. (default &#34;127.0.0.1:4466&#34;)
       --write-remote string   Remote URL of the write API endpoint. (default &#34;127.0.0.1:4467&#34;)

diff --git a/docs/docs/cli/keto-expand.md b/docs/docs/cli/keto-expand.md
@@ -27,7 +27,7 @@ keto expand &lt;relation&gt; &lt;namespace&gt; &lt;object&gt; [flags]
 ```
   -f, --format string         Set the output format. One of default, json, and json-pretty. (default &#34;default&#34;)
   -h, --help                  help for expand
-  -d, --max-depth int32       maximum depth of the tree (default 100)
+  -d, --max-depth int32       Maximum depth of the tree to be returned. If the value is less than 1 or greater than the global max-depth then the global max-depth will be used instead.
   -q, --quiet                 Be quiet with output printing.
       --read-remote string    Remote URL of the read API endpoint. (default &#34;127.0.0.1:4466&#34;)
       --write-remote string   Remote URL of the write API endpoint. (default &#34;127.0.0.1:4467&#34;)

diff --git a/docs/docs/concepts/api-overview.mdx b/docs/docs/concepts/api-overview.mdx
@@ -41,6 +41,12 @@ This API resolves [subject sets](./subjects.mdx#subject-sets) and
 This API is primarily used to
 [check permissions to restrict actions](../guides/simple-access-check-guide.mdx).
 
+A check-request can include the maximum depth of the search tree. If the value
+is less than 1 or greater than the global max-depth then the global max-depth
+will be used instead. This is to ensure low latency and limit the resource usage
+per request. To find out more about Ory Keto's performance, head over to the
+[performance considerations](../performance.mdx).
+
 For more details, head over to the
 [gRPC API reference](../reference/proto-api.mdx#checkservice) or
 [REST API reference](../reference/rest-api.mdx#check-a-relation-tuple).
@@ -56,10 +62,11 @@ tuples including the operands as defined in the
 - determine why someone has access to an object
 - audit permissions in the system
 
-An expand-request has to include the maximum depth of the tree to be returned.
-This is required to ensure low latency and limit the resource usage per request.
-To find out more about Ory Keto's performance, head over to the
-[performance considerations](../performance.mdx).
+An expand-request can include the maximum depth of the tree to be returned. If
+the value is less than 1 or greater than the global max-depth then the global
+max-depth will be used instead. This is required to ensure low latency and limit
+the resource usage per request. To find out more about Ory Keto's performance,
+head over to the [performance considerations](../performance.mdx).
 
 For more details, head over to the
 [gRPC API reference](../reference/proto-api.mdx#expandservice) or

diff --git a/docs/docs/reference/proto-api.mdx b/docs/docs/reference/proto-api.mdx
@@ -67,6 +67,7 @@ related to an object.
 | subject   | [Subject](#ory.keto.acl.v1alpha1.Subject) |       | The related subject in this check.                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                              |
 | latest    | [bool](#bool)                             |       | This field is not implemented yet and has no effect. <!-- Set this field to `true` in case your application needs to authorize depending on up to date ACLs, also called a "content-change check".<br/>If set to `true` the `snaptoken` field is ignored, the check is evaluated at the latest snapshot (globally consistent) and the response includes a snaptoken for clients to store along with object contents that can be used for subsequent checks of the same content version.<br/>Example use case: - You need to authorize a user to modify/delete some resource and it is unacceptable that if the permission to do that had just been revoked some seconds ago so that the change had not yet been fully replicated to all availability zones. --> |
 | snaptoken | [string](#string)                         |       | This field is not implemented yet and has no effect. <!-- Optional. Like reads, a check is always evaluated at a consistent snapshot no earlier than the given snaptoken.<br/>Leave this field blank if you want to evaluate the check based on eventually consistent ACLs, benefiting from very low latency, but possibly slightly stale results.<br/>If the specified token is too old and no longer known, the server falls back as if no snaptoken had been specified.<br/>If not specified the server tries to evaluate the check on the best snapshot version where it is very likely that ACLs had already been replicated to all availability zones. -->                                                                                                |
+| max_depth | [int32](#int32)                           |       | The maximum depth to search for a relation.<br/>If the value is less than 1 or greater than the global max-depth then the global max-depth will be used instead.                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                |
 
 ### CheckResponse
 
@@ -97,7 +98,7 @@ The request for an ExpandService.Expand RPC. Expands the given subject set.
 | Field     | Type                                      | Label | Description                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                       |
 | --------- | ----------------------------------------- | ----- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
 | subject   | [Subject](#ory.keto.acl.v1alpha1.Subject) |       | The subject to expand.                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                            |
-| max_depth | [int32](#int32)                           |       | The maximum depth of tree to build. It is important to set this parameter to a meaningful value. Ponder how deep you really want to display this.                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                 |
+| max_depth | [int32](#int32)                           |       | The maximum depth of tree to build.<br/>If the value is less than 1 or greater than the global max-depth then the global max-depth will be used instead.<br/>It is important to set this parameter to a meaningful value. Ponder how deep you really want to display this.                                                                                                                                                                                                                                                                                                                                                                        |
 | snaptoken | [string](#string)                         |       | This field is not implemented yet and has no effect. <!-- Optional. Like reads, a expand is always evaluated at a consistent snapshot no earlier than the given snaptoken.<br/>Leave this field blank if you want to expand based on eventually consistent ACLs, benefiting from very low latency, but possibly slightly stale results.<br/>If the specified token is too old and no longer known, the server falls back as if no snaptoken had been specified.<br/>If not specified the server tries to build the tree on the best snapshot version where it is very likely that ACLs had already been replicated to all availability zones. --> |
 
 ### ExpandResponse

diff --git a/internal/check/engine.go b/internal/check/engine.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 
+	"github.com/ory/keto/internal/driver/config"
 	"github.com/ory/keto/internal/x/graph"
 
 	"github.com/ory/herodot"
@@ -21,6 +22,8 @@ type (
 	}
 	EngineDependencies interface {
 		relationtuple.ManagerProvider
+		config.Provider
+		x.LoggerProvider
 	}
 )
 
@@ -30,7 +33,12 @@ func NewEngine(d EngineDependencies) *Engine {
 	}
 }
 
-func (e *Engine) subjectIsAllowed(ctx context.Context, requested *relationtuple.InternalRelationTuple, rels []*relationtuple.InternalRelationTuple) (bool, error) {
+func (e *Engine) subjectIsAllowed(
+	ctx context.Context,
+	requested *relationtuple.InternalRelationTuple,
+	rels []*relationtuple.InternalRelationTuple,
+	restDepth int,
+) (bool, error) {
 	// This is the same as the graph problem "can requested.Subject be reached from requested.Object through the first outgoing edge requested.Relation"
 	//
 	// We implement recursive depth-first search here.
@@ -54,7 +62,12 @@ func (e *Engine) subjectIsAllowed(ctx context.Context, requested *relationtuple.
 		}
 
 		// expand the set by one indirection; paginated
-		allowed, err := e.checkOneIndirectionFurther(ctx, requested, &relationtuple.RelationQuery{Object: sub.Object, Relation: sub.Relation, Namespace: sub.Namespace})
+		allowed, err := e.checkOneIndirectionFurther(
+			ctx,
+			requested,
+			&relationtuple.RelationQuery{Object: sub.Object, Relation: sub.Relation, Namespace: sub.Namespace},
+			restDepth-1,
+		)
 		if err != nil {
 			return false, err
 		}
@@ -66,7 +79,17 @@ func (e *Engine) subjectIsAllowed(ctx context.Context, requested *relationtuple.
 	return false, nil
 }
 
-func (e *Engine) checkOneIndirectionFurther(ctx context.Context, requested *relationtuple.InternalRelationTuple, expandQuery *relationtuple.RelationQuery) (bool, error) {
+func (e *Engine) checkOneIndirectionFurther(
+	ctx context.Context,
+	requested *relationtuple.InternalRelationTuple,
+	expandQuery *relationtuple.RelationQuery,
+	restDepth int,
+) (bool, error) {
+	if restDepth <= 0 {
+		e.d.Logger().WithFields(requested.ToLoggerFields()).Debug("reached max-depth, therefore this query will not be further expanded")
+		return false, nil
+	}
+
 	// an empty page token denotes the first page (as tokens are opaque)
 	var prevPage string
 
@@ -79,7 +102,7 @@ func (e *Engine) checkOneIndirectionFurther(ctx context.Context, requested *rela
 			return false, err
 		}
 
-		allowed, err := e.subjectIsAllowed(ctx, requested, nextRels)
+		allowed, err := e.subjectIsAllowed(ctx, requested, nextRels, restDepth)
 
 		// loop through pages until either allowed, end of pages, or an error occurred
 		if allowed || nextPage == "" || err != nil {
@@ -90,6 +113,11 @@ func (e *Engine) checkOneIndirectionFurther(ctx context.Context, requested *rela
 	}
 }
 
-func (e *Engine) SubjectIsAllowed(ctx context.Context, r *relationtuple.InternalRelationTuple) (bool, error) {
-	return e.checkOneIndirectionFurther(ctx, r, &relationtuple.RelationQuery{Object: r.Object, Relation: r.Relation, Namespace: r.Namespace})
+func (e *Engine) SubjectIsAllowed(ctx context.Context, r *relationtuple.InternalRelationTuple, restDepth int) (bool, error) {
+	// global max-depth takes precedence when it is the lesser or if the request max-depth is less than or equal to 0
+	if globalMaxDepth := e.d.Config().ReadAPIMaxDepth(); restDepth <= 0 || globalMaxDepth < restDepth {
+		restDepth = globalMaxDepth
+	}
+
+	return e.checkOneIndirectionFurther(ctx, r, &relationtuple.RelationQuery{Object: r.Object, Relation: r.Relation, Namespace: r.Namespace}, restDepth)
 }