Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add max-depth for check command + global max-depth #791

Merged
merged 13 commits into from
Dec 10, 2021
Merged
9 changes: 9 additions & 0 deletions cmd/check/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -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>",
Expand All @@ -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{
Expand All @@ -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)
Expand All @@ -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.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this 👍


return cmd
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/expand/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
1 change: 1 addition & 0 deletions docs/docs/cli/keto-check.md
Original file line number Diff line number Diff line change
Expand Up @@ -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;)
Expand Down
2 changes: 1 addition & 1 deletion docs/docs/cli/keto-expand.md
Original file line number Diff line number Diff line change
Expand Up @@ -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;)
Expand Down
15 changes: 11 additions & 4 deletions docs/docs/concepts/api-overview.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand All @@ -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
Expand Down
3 changes: 2 additions & 1 deletion docs/docs/reference/proto-api.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
40 changes: 34 additions & 6 deletions internal/check/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -21,6 +22,8 @@ type (
}
EngineDependencies interface {
relationtuple.ManagerProvider
config.Provider
x.LoggerProvider
}
)

Expand All @@ -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.
Expand All @@ -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
}
Expand All @@ -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
zepatrik marked this conversation as resolved.
Show resolved Hide resolved
}

// an empty page token denotes the first page (as tokens are opaque)
var prevPage string

Expand All @@ -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 {
Expand All @@ -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 {
zepatrik marked this conversation as resolved.
Show resolved Hide resolved
restDepth = globalMaxDepth
}

return e.checkOneIndirectionFurther(ctx, r, &relationtuple.RelationQuery{Object: r.Object, Relation: r.Relation, Namespace: r.Namespace}, restDepth)
zepatrik marked this conversation as resolved.
Show resolved Hide resolved
}
Loading