Skip to content

Commit

Permalink
feat: add max-depth for check command
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
doodlesbykumbi authored Dec 5, 2021
1 parent 87e4d1d commit ac543a5
Show file tree
Hide file tree
Showing 14 changed files with 606 additions and 1,861 deletions.
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", 100, "maximum depth of the tree")

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 tree (default 100)
-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
5 changes: 5 additions & 0 deletions docs/docs/concepts/api-overview.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@ 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 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).

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 Down
502 changes: 342 additions & 160 deletions docs/docs/reference/proto-api.mdx

Large diffs are not rendered by default.

34 changes: 28 additions & 6 deletions internal/check/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package check
import (
"context"
"errors"
"fmt"

"github.com/ory/keto/internal/x/graph"

Expand Down Expand Up @@ -30,7 +31,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 +60,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 +77,18 @@ 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) {
// TODO: Clarify the semantics of restDepth. To me it means the number of recursive calls of checkOneIndirectionFurther, without counting the first
if restDepth <= 0 {
// TODO: Figure out how to decorate this error
return false, fmt.Errorf("max-depth exhausted")
}

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

Expand All @@ -79,7 +101,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 +112,6 @@ 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) {
return e.checkOneIndirectionFurther(ctx, r, &relationtuple.RelationQuery{Object: r.Object, Relation: r.Relation, Namespace: r.Namespace}, restDepth)
}
79 changes: 66 additions & 13 deletions internal/check/engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,59 @@ func newDepsProvider(t *testing.T, namespaces []*namespace.Namespace, pageOpts .
}

func TestEngine(t *testing.T) {
t.Run("respects max depth", func(t *testing.T) {
// "user" has relation "access" through being an "owner" through being an "admin"
// which requires at least 2 units of depth. If max-depth is 2 then we hit max-depth
ns := "test"
user := &relationtuple.SubjectID{ID: "user"}
object := "object"

adminRel := relationtuple.InternalRelationTuple{
Relation: "admin",
Object: object,
Namespace: ns,
Subject: user,
}

adminIsOwnerRel := relationtuple.InternalRelationTuple{
Relation: "owner",
Object: object,
Namespace: ns,
Subject: &relationtuple.SubjectSet{
Relation: "admin",
Object: object,
Namespace: ns,
},
}

accessRel := relationtuple.InternalRelationTuple{
Relation: "access",
Object: object,
Namespace: ns,
Subject: &relationtuple.SubjectSet{
Relation: "owner",
Object: object,
Namespace: ns,
},
}
reg := newDepsProvider(t, []*namespace.Namespace{
{Name: ns, ID: 1},
})
require.NoError(t, reg.RelationTupleManager().WriteRelationTuples(context.Background(), &adminRel, &adminIsOwnerRel, &accessRel))

e := check.NewEngine(reg)

res, err := e.SubjectIsAllowed(context.Background(), &relationtuple.InternalRelationTuple{
Relation: "access",
Object: object,
Namespace: ns,
Subject: user,
}, 2)
require.Error(t, err)
assert.Contains(t, err.Error(), "max-depth exhausted")
assert.False(t, res)
})

t.Run("direct inclusion", func(t *testing.T) {
rel := relationtuple.InternalRelationTuple{
Relation: "access",
Expand All @@ -42,7 +95,7 @@ func TestEngine(t *testing.T) {

e := check.NewEngine(reg)

res, err := e.SubjectIsAllowed(context.Background(), &rel)
res, err := e.SubjectIsAllowed(context.Background(), &rel, 100)
require.NoError(t, err)
assert.True(t, res)
})
Expand Down Expand Up @@ -83,7 +136,7 @@ func TestEngine(t *testing.T) {
Object: dust,
Subject: &mark,
Namespace: sofaNamespace,
})
}, 100)
require.NoError(t, err)
assert.True(t, res)
})
Expand Down Expand Up @@ -111,7 +164,7 @@ func TestEngine(t *testing.T) {
Object: rel.Object,
Namespace: rel.Namespace,
Subject: &relationtuple.SubjectID{ID: "not " + user.ID},
})
}, 100)
require.NoError(t, err)
assert.False(t, res)
})
Expand Down Expand Up @@ -143,7 +196,7 @@ func TestEngine(t *testing.T) {
Relation: access.Relation,
Object: object,
Subject: user.Subject,
})
}, 100)
require.NoError(t, err)
assert.False(t, res)
})
Expand Down Expand Up @@ -181,7 +234,7 @@ func TestEngine(t *testing.T) {
Object: diaryEntry,
Namespace: diaryNamespace,
Subject: user.Subject,
})
}, 100)
require.NoError(t, err)
assert.False(t, res)
})
Expand Down Expand Up @@ -239,7 +292,7 @@ func TestEngine(t *testing.T) {
Relation: writeRel.Relation,
Object: object,
Subject: &user,
})
}, 100)
require.NoError(t, err)
assert.True(t, res)

Expand All @@ -249,7 +302,7 @@ func TestEngine(t *testing.T) {
Relation: orgMembers.Relation,
Object: organization,
Subject: &user,
})
}, 100)
require.NoError(t, err)
assert.True(t, res)
})
Expand Down Expand Up @@ -289,7 +342,7 @@ func TestEngine(t *testing.T) {
Relation: directoryAccess.Relation,
Object: file,
Subject: &user,
})
}, 100)
require.NoError(t, err)
assert.False(t, res)
})
Expand Down Expand Up @@ -333,7 +386,7 @@ func TestEngine(t *testing.T) {
Object: obj,
Relation: ownerRel,
Subject: &relationtuple.SubjectID{ID: directOwner},
})
}, 100)
require.NoError(t, err)
assert.True(t, res)

Expand All @@ -342,7 +395,7 @@ func TestEngine(t *testing.T) {
Object: obj,
Relation: ownerRel,
Subject: &relationtuple.SubjectID{ID: indirectOwner},
})
}, 100)
require.NoError(t, err)
assert.True(t, res)
})
Expand Down Expand Up @@ -375,7 +428,7 @@ func TestEngine(t *testing.T) {
Object: obj,
Relation: access,
Subject: &relationtuple.SubjectID{ID: user},
})
}, 100)
require.NoError(t, err)
assert.True(t, allowed)

Expand Down Expand Up @@ -429,7 +482,7 @@ func TestEngine(t *testing.T) {
Relation: access,
Subject: &relationtuple.SubjectID{ID: user},
}
allowed, err := e.SubjectIsAllowed(context.Background(), req)
allowed, err := e.SubjectIsAllowed(context.Background(), req, 100)
require.NoError(t, err)
assert.True(t, allowed, req.String())
}
Expand Down Expand Up @@ -483,7 +536,7 @@ func TestEngine(t *testing.T) {
Subject: &relationtuple.SubjectID{
ID: stations[2],
},
})
}, 100)
require.NoError(t, err)
assert.False(t, res)
})
Expand Down
Loading

0 comments on commit ac543a5

Please sign in to comment.