Skip to content

Commit

Permalink
feat: map subject/object strings to UUIDs (#792)
Browse files Browse the repository at this point in the history
In relation tuples and related API objects such as queries, subjects
(including subject sets) and objects are now automatically mapped to and
from UUIDs.  The mapping is done in each handler and for each protocol
(HTTP, gRPC) separately.

Below the handler, all objects and subjects are now UUIDs, but still
handled as strings, for the following reasons:
 * No duplication of datastructures (one for type `string`, one for type
   `uuid.UUID`).
 * No unnecessary copying, the mapping is done in-place and batched
   across multiple objects.
  • Loading branch information
hperl committed May 9, 2022
1 parent 26428f6 commit 44f442f
Show file tree
Hide file tree
Showing 25 changed files with 465 additions and 153 deletions.
9 changes: 6 additions & 3 deletions cmd/expand/root_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"testing"

"github.com/ory/x/cmdx"

"github.com/stretchr/testify/assert"

"github.com/ory/keto/cmd/client"
Expand All @@ -18,12 +17,16 @@ func TestExpandCommand(t *testing.T) {

t.Run("case=unknown tuple", func(t *testing.T) {
t.Run("format=JSON", func(t *testing.T) {
stdOut := ts.Cmd.ExecNoErr(t, "access", nspace.Name, "object", "--"+cmdx.FlagFormat, string(cmdx.FormatJSON))
stdOut := ts.Cmd.ExecNoErr(t,
"access", nspace.Name,
"object", "--"+cmdx.FlagFormat, string(cmdx.FormatJSON))
assert.Equal(t, "null\n", stdOut)
})

t.Run("format=default", func(t *testing.T) {
stdOut := ts.Cmd.ExecNoErr(t, "access", nspace.Name, "object", "--"+cmdx.FlagFormat, string(cmdx.FormatDefault))
stdOut := ts.Cmd.ExecNoErr(t,
"access", nspace.Name,
"object", "--"+cmdx.FlagFormat, string(cmdx.FormatDefault))
assert.Contains(t, stdOut, "empty tree")
})
})
Expand Down
17 changes: 9 additions & 8 deletions internal/check/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,14 @@ import (
"encoding/json"
"net/http"

"github.com/julienschmidt/httprouter"
"github.com/ory/herodot"
"github.com/pkg/errors"

rts "github.com/ory/keto/proto/ory/keto/relation_tuples/v1alpha2"

"google.golang.org/grpc"

"github.com/ory/keto/internal/relationtuple"

"github.com/julienschmidt/httprouter"

"github.com/ory/keto/internal/x"
rts "github.com/ory/keto/proto/ory/keto/relation_tuples/v1alpha2"
)

type (
Expand All @@ -30,7 +26,10 @@ type (
}
)

var _ rts.CheckServiceServer = (*Handler)(nil)
var (
_ rts.CheckServiceServer = (*Handler)(nil)
_ *getCheckRequest = nil
)

func NewHandler(d handlerDependencies) *Handler {
return &Handler{d: d}
Expand Down Expand Up @@ -64,7 +63,6 @@ type RESTResponse struct {
}

// swagger:parameters getCheck postCheck
// nolint:deadcode,unused
type getCheckRequest struct {
// in:query
MaxDepth int `json:"max-depth"`
Expand Down Expand Up @@ -105,6 +103,7 @@ func (h *Handler) getCheck(w http.ResponseWriter, r *http.Request, _ httprouter.
return
}

h.d.PermissionEngine().d.UUIDMappingManager().MapFieldsToUUID(r.Context(), tuple)
allowed, err := h.d.PermissionEngine().SubjectIsAllowed(r.Context(), tuple, maxDepth)
if err != nil {
h.d.Writer().WriteError(w, r, err)
Expand Down Expand Up @@ -151,6 +150,7 @@ func (h *Handler) postCheck(w http.ResponseWriter, r *http.Request, _ httprouter
return
}

h.d.PermissionEngine().d.UUIDMappingManager().MapFieldsToUUID(r.Context(), &tuple)
allowed, err := h.d.PermissionEngine().SubjectIsAllowed(r.Context(), &tuple, maxDepth)
if err != nil {
h.d.Writer().WriteError(w, r, err)
Expand All @@ -171,6 +171,7 @@ func (h *Handler) Check(ctx context.Context, req *rts.CheckRequest) (*rts.CheckR
return nil, err
}

h.d.PermissionEngine().d.UUIDMappingManager().MapFieldsToUUID(ctx, tuple)
allowed, err := h.d.PermissionEngine().SubjectIsAllowed(ctx, tuple, int(req.MaxDepth))
// TODO add content change handling
if err != nil {
Expand Down
4 changes: 3 additions & 1 deletion internal/check/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import (
)

func assertAllowed(t *testing.T, resp *http.Response) {
t.Helper()

body, err := io.ReadAll(resp.Body)
require.NoError(t, err)

Expand Down Expand Up @@ -97,7 +99,7 @@ func TestRESTHandler(t *testing.T) {
Relation: "r",
Subject: &relationtuple.SubjectID{ID: "s"},
}
require.NoError(t, reg.RelationTupleManager().WriteRelationTuples(context.Background(), rt))
relationtuple.MapAndWriteTuples(t, reg, rt)

q, err := rt.ToURLQuery()
require.NoError(t, err)
Expand Down
2 changes: 1 addition & 1 deletion internal/driver/config/namespace_memory.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func (s *memoryNamespaceManager) GetNamespaceByName(_ context.Context, name stri
}
}

return nil, errors.WithStack(herodot.ErrNotFound.WithReasonf("Unknown namespace with name %s.", name))
return nil, errors.WithStack(herodot.ErrNotFound.WithReasonf("Unknown namespace with name %q.", name))
}

func (s *memoryNamespaceManager) GetNamespaceByConfigID(_ context.Context, id int32) (*namespace.Namespace, error) {
Expand Down
15 changes: 4 additions & 11 deletions internal/driver/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,15 @@ import (
"context"
"net/http"

prometheus "github.com/ory/x/prometheusx"

"github.com/gobuffalo/pop/v6"

"github.com/ory/keto/internal/driver/config"
"github.com/ory/keto/internal/uuidmapping"

"github.com/spf13/cobra"

"google.golang.org/grpc"

"github.com/ory/x/healthx"
prometheus "github.com/ory/x/prometheusx"
"github.com/ory/x/tracing"
"github.com/spf13/cobra"
"google.golang.org/grpc"

"github.com/ory/keto/internal/check"
"github.com/ory/keto/internal/driver/config"
"github.com/ory/keto/internal/expand"
"github.com/ory/keto/internal/persistence"
"github.com/ory/keto/internal/relationtuple"
Expand All @@ -34,7 +28,6 @@ type (
x.WriterProvider

relationtuple.ManagerProvider
uuidmapping.ManagerProvider
expand.EngineProvider
check.EngineProvider
persistence.Migrator
Expand Down
3 changes: 1 addition & 2 deletions internal/driver/registry_default.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
"github.com/ory/keto/internal/persistence"
"github.com/ory/keto/internal/persistence/sql"
"github.com/ory/keto/internal/relationtuple"
"github.com/ory/keto/internal/uuidmapping"
"github.com/ory/keto/internal/x"
"github.com/ory/keto/ketoctx"
rts "github.com/ory/keto/proto/ory/keto/relation_tuples/v1alpha2"
Expand Down Expand Up @@ -152,7 +151,7 @@ func (r *RegistryDefault) RelationTupleManager() relationtuple.Manager {
return r.p
}

func (r *RegistryDefault) UUIDMappingManager() uuidmapping.Manager {
func (r *RegistryDefault) UUIDMappingManager() relationtuple.UUIDMappingManager {
if r.p == nil {
panic("no relation tuple manager, but expected to have one")
}
Expand Down
9 changes: 3 additions & 6 deletions internal/e2e/cases_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,14 @@ import (
"strconv"
"testing"

"github.com/stretchr/testify/require"

"github.com/ory/herodot"

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

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/ory/keto/internal/expand"
"github.com/ory/keto/internal/namespace"
"github.com/ory/keto/internal/relationtuple"
"github.com/ory/keto/internal/x"
)

func runCases(c client, addNamespace func(*testing.T, ...*namespace.Namespace)) func(*testing.T) {
Expand Down Expand Up @@ -188,7 +185,7 @@ func runCases(c client, addNamespace func(*testing.T, ...*namespace.Namespace))
Relation: "rel",
}
resp := c.queryTuple(t, q)
require.Equal(t, resp.RelationTuples, rts)
require.ElementsMatch(t, resp.RelationTuples, rts)

c.deleteAllTuples(t, q)
resp = c.queryTuple(t, q)
Expand Down
29 changes: 17 additions & 12 deletions internal/expand/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,13 @@ import (
"context"
"net/http"

"github.com/julienschmidt/httprouter"
"github.com/ory/herodot"

rts "github.com/ory/keto/proto/ory/keto/relation_tuples/v1alpha2"

"google.golang.org/grpc"

"github.com/ory/keto/internal/relationtuple"

"github.com/julienschmidt/httprouter"

"github.com/ory/keto/internal/x"
rts "github.com/ory/keto/proto/ory/keto/relation_tuples/v1alpha2"
)

type (
Expand All @@ -28,7 +24,10 @@ type (
}
)

var _ rts.ExpandServiceServer = (*handler)(nil)
var (
_ rts.ExpandServiceServer = (*handler)(nil)
_ *getExpandRequest = nil
)

const RouteBase = "/relation-tuples/expand"

Expand All @@ -49,7 +48,6 @@ func (h *handler) RegisterReadGRPC(s *grpc.Server) {
func (h *handler) RegisterWriteGRPC(s *grpc.Server) {}

// swagger:parameters getExpand
// nolint:deadcode,unused
type getExpandRequest struct {
// in:query
MaxDepth int `json:"max-depth"`
Expand Down Expand Up @@ -81,24 +79,31 @@ func (h *handler) getExpand(w http.ResponseWriter, r *http.Request, _ httprouter
return
}

res, err := h.d.ExpandEngine().BuildTree(r.Context(), (&relationtuple.SubjectSet{}).FromURLQuery(r.URL.Query()), maxDepth)
subject := (&relationtuple.SubjectSet{}).FromURLQuery(r.URL.Query())

h.d.ExpandEngine().d.UUIDMappingManager().MapFieldsToUUID(r.Context(), subject)
res, err := h.d.ExpandEngine().BuildTree(r.Context(), subject, maxDepth)
if err != nil {
h.d.Writer().WriteError(w, r, err)
return
}
h.d.ExpandEngine().d.UUIDMappingManager().MapFieldsFromUUID(r.Context(), res)

h.d.Writer().Write(w, r, res)
}

func (h *handler) Expand(ctx context.Context, req *rts.ExpandRequest) (*rts.ExpandResponse, error) {
sub, err := relationtuple.SubjectFromProto(req.Subject)
subject, err := relationtuple.SubjectFromProto(req.Subject)
if err != nil {
return nil, err
}
tree, err := h.d.ExpandEngine().BuildTree(ctx, sub, int(req.MaxDepth))

h.d.ExpandEngine().d.UUIDMappingManager().MapFieldsToUUID(ctx, subject)
res, err := h.d.ExpandEngine().BuildTree(ctx, subject, int(req.MaxDepth))
if err != nil {
return nil, err
}
h.d.ExpandEngine().d.UUIDMappingManager().MapFieldsFromUUID(ctx, res)

return &rts.ExpandResponse{Tree: tree.ToProto()}, nil
return &rts.ExpandResponse{Tree: res.ToProto()}, nil
}
48 changes: 43 additions & 5 deletions internal/expand/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,20 +79,20 @@ func TestRESTHandler(t *testing.T) {
},
}

require.NoError(t, reg.RelationTupleManager().WriteRelationTuples(context.Background(), []*relationtuple.InternalRelationTuple{
{
relationtuple.MapAndWriteTuples(t, reg,
&relationtuple.InternalRelationTuple{
Namespace: nspace.Name,
Object: rootSub.Object,
Relation: rootSub.Relation,
Subject: expectedTree.Children[0].Subject,
},
{
&relationtuple.InternalRelationTuple{
Namespace: nspace.Name,
Object: rootSub.Object,
Relation: rootSub.Relation,
Subject: expectedTree.Children[1].Subject,
},
}...))
)

qs := rootSub.ToURLQuery()
qs.Set("max-depth", "2")
Expand All @@ -103,6 +103,44 @@ func TestRESTHandler(t *testing.T) {

actualTree := expand.Tree{}
require.NoError(t, json.NewDecoder(resp.Body).Decode(&actualTree))
assert.Equal(t, expectedTree, &actualTree)
assertEqualTrees(t, expectedTree, &actualTree)
})
}

func assertEqualTrees(t *testing.T, expected, actual *expand.Tree) {
t.Helper()
assert.Truef(t, treesAreEqual(t, expected, actual),
"expected:\n%s\n\nactual:\n%s", expected.String(), actual.String())
}

func treesAreEqual(t *testing.T, expected, actual *expand.Tree) bool {
if expected == nil || actual == nil {
return expected == actual
}

if expected.Type != actual.Type {
t.Logf("expected type %q, actual type %q", expected.Type, actual.Type)
return false
}
if expected.Subject.String() != actual.Subject.String() {
t.Logf("expected subject: %q, actual subject: %q", expected.Subject.String(), actual.Subject.String())
return false
}
if len(expected.Children) != len(actual.Children) {
t.Logf("expected len(children)=%d, actual len(children)=%d", len(expected.Children), len(actual.Children))
return false
}

// For children, we check for equality disregarding the order
outer:
for _, expectedChild := range expected.Children {
for _, actualChild := range actual.Children {
if treesAreEqual(t, expectedChild, actualChild) {
continue outer
}
}
t.Logf("expected child:\n%s\n\nactual child:\n%s", expectedChild.String(), actual.String())
return false
}
return true
}
15 changes: 14 additions & 1 deletion internal/expand/tree.go
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ func TreeFromProto(t *rts.SubjectTree) (*Tree, error) {

func (t *Tree) String() string {
if t == nil {
return ""
return "(nil)"
}

sub := t.Subject.String()
Expand All @@ -233,3 +233,16 @@ func (t *Tree) String() string {

return fmt.Sprintf("∪ %s\n├─ %s", sub, strings.Join(children, "\n├─ "))
}

func (t *Tree) UUIDMappableFields() (res []*string) {
if t == nil {
return
}
if t.Subject != nil {
res = append(res, t.Subject.UUIDMappableFields()...)
}
for _, c := range t.Children {
res = append(res, c.UUIDMappableFields()...)
}
return
}
3 changes: 1 addition & 2 deletions internal/persistence/definitions.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,12 @@ import (
"github.com/gobuffalo/pop/v6"

"github.com/ory/keto/internal/relationtuple"
"github.com/ory/keto/internal/uuidmapping"
)

type (
Persister interface {
relationtuple.Manager
uuidmapping.Manager
relationtuple.UUIDMappingManager

Connection(ctx context.Context) *pop.Connection
}
Expand Down
Loading

0 comments on commit 44f442f

Please sign in to comment.