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.

Additionally, the migration (adding the string values to the mapping
table and replacing the values with UUIDs) is now performed
automatically as part of the migrations.  Tests are in `migratest`,
which now also handles `GoMigrations` properly.  This method replaces
the dedicated command in `cmd/migrate/...` for the UUID mappings.
  • Loading branch information
hperl committed May 16, 2022
1 parent 75fb3ae commit 46e25e0
Show file tree
Hide file tree
Showing 58 changed files with 1,020 additions and 529 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
4 changes: 2 additions & 2 deletions cmd/migrate/migrate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ func TestMigrate(t *testing.T) {

t.Cleanup(func() {
// migrate all down
t.Log(cmd.ExecNoErr(t, "down", "0", "--"+FlagYes))
t.Logf("cleanup:\n%s\n", cmd.ExecNoErr(t, "down", "0", "--"+FlagYes))
})

parts := strings.Split(stdOut, "Are you sure that you want to apply this migration?")
Expand All @@ -120,7 +120,7 @@ func TestMigrate(t *testing.T) {

t.Cleanup(func() {
// migrate all down
t.Log(cmd.ExecNoErr(t, "down", "0", "--"+FlagYes))
t.Logf("cleanup:\n%s\n", cmd.ExecNoErr(t, "down", "0", "--"+FlagYes))
})

parts := strings.Split(out, "Applying migrations...")
Expand Down
49 changes: 0 additions & 49 deletions cmd/migrate/migrate_uuid_mapping.go

This file was deleted.

1 change: 0 additions & 1 deletion cmd/migrate/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ func newMigrateCmd(opts []ketoctx.Option) *cobra.Command {
newStatusCmd(opts),
newUpCmd(opts),
newDownCmd(opts),
newMigrateUUIDMappingCmd(),
)
return cmd
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ readClient.listRelationTuples(readRequest, (err, resp) => {
writeClient.transactRelationTuples(writeRequest, (err) => {
if (err) {
console.log('Unexpected err', err)
return 1
}
})
})
25 changes: 17 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,10 @@ func (h *Handler) getCheck(w http.ResponseWriter, r *http.Request, _ httprouter.
return
}

if err := h.d.PermissionEngine().d.UUIDMappingManager().MapFieldsToUUID(r.Context(), tuple); err != nil {
h.d.Writer().WriteError(w, r, err)
return
}
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 +153,10 @@ func (h *Handler) postCheck(w http.ResponseWriter, r *http.Request, _ httprouter
return
}

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

if err := h.d.PermissionEngine().d.UUIDMappingManager().MapFieldsToUUID(ctx, tuple); err != nil {
return nil, err
}
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
6 changes: 6 additions & 0 deletions internal/driver/pop_connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,12 @@ func (r *RegistryDefault) PopConnectionWithOpts(ctx context.Context, popOpts ...
return nil, errors.WithStack(err)
}

// Close this connection when the context is closed.
go func() {
<-ctx.Done()
conn.Close()
}()

return conn.WithContext(ctx), nil
}

Expand Down
13 changes: 3 additions & 10 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"

"github.com/gobuffalo/pop/v6"
"github.com/ory/x/healthx"
"github.com/ory/x/otelx"
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"

"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
14 changes: 11 additions & 3 deletions internal/driver/registry_default.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/gobuffalo/pop/v6"
"github.com/ory/herodot"
"github.com/ory/x/dbal"
"github.com/ory/x/fsx"
"github.com/ory/x/healthx"
"github.com/ory/x/logrusx"
"github.com/ory/x/metricsx"
Expand All @@ -24,8 +25,9 @@ import (
"github.com/ory/keto/internal/expand"
"github.com/ory/keto/internal/persistence"
"github.com/ory/keto/internal/persistence/sql"
_ "github.com/ory/keto/internal/persistence/sql/migrations"
"github.com/ory/keto/internal/persistence/sql/migrations/uuidmapping"
"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 +154,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 Expand Up @@ -186,10 +188,16 @@ func (r *RegistryDefault) MigrationBox(ctx context.Context) (*popx.MigrationBox,
if err != nil {
return nil, err
}
mb, err := sql.NewMigrationBox(c, r.Logger(), r.Tracer(ctx))

mb, err := popx.NewMigrationBox(
fsx.Merge(sql.Migrations, networkx.Migrations),
popx.NewMigrator(c, r.Logger(), r.Tracer(ctx), 0),
popx.WithGoMigrations(uuidmapping.Migrations),
)
if err != nil {
return nil, err
}

r.mb = mb
}
return r.mb, nil
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
Loading

0 comments on commit 46e25e0

Please sign in to comment.