From 8e078900043de695094667ab1bdb4e4b0a55a6ab Mon Sep 17 00:00:00 2001 From: Henning Perl Date: Tue, 17 Jan 2023 17:38:11 +0100 Subject: [PATCH] feat: faster SQL queries for checks and strict check mode (#1171) With this change we introduce an experimental strict mode that drastically reduces the number of SQL queries performed during checks. This is experimental to allow adjusting its behavior in a breaking manner, but it is ready for production usage. Also some of the non-strict queries are optimized. Co-authored-by: Patrik --- .schema/README.md | 4 +- benchtest.new.txt | 7 + embedx/config.schema.json | 6 + go.mod | 4 +- go.sum | 7 + internal/check/2023-01-09-benchtest.txt | 7 + internal/check/bench_test.go | 56 +++++- internal/check/engine.go | 156 ++++++++-------- internal/check/engine_test.go | 109 +++++------ internal/check/performance_test.go | 161 ---------------- internal/check/rewrites.go | 80 +++++--- internal/check/rewrites_test.go | 2 +- internal/check/testfixtures/project_opl.ts | 29 +++ internal/check/testmain_test.go | 1 + internal/driver/config/provider.go | 7 +- internal/driver/config/provider_test.go | 14 ++ internal/driver/registry_default.go | 12 +- internal/driver/registry_factory.go | 62 +++++- internal/namespace/definitions.go | 28 +++ internal/persistence/definitions.go | 3 + internal/persistence/sql/persister.go | 2 + internal/persistence/sql/relationtuples.go | 20 +- internal/persistence/sql/traverser.go | 207 +++++++++++++++++++++ internal/relationtuple/definitions.go | 25 +++ package-lock.json | 47 ++++- package.json | 1 + 26 files changed, 714 insertions(+), 343 deletions(-) create mode 100644 benchtest.new.txt create mode 100644 internal/check/2023-01-09-benchtest.txt delete mode 100644 internal/check/performance_test.go create mode 100644 internal/check/testfixtures/project_opl.ts create mode 100644 internal/persistence/sql/traverser.go diff --git a/.schema/README.md b/.schema/README.md index 2de087297..57d689bec 100644 --- a/.schema/README.md +++ b/.schema/README.md @@ -1,5 +1,5 @@ The schemas in this directory are meant for external and public use. The config schema is generated from the internal one at -`internal/driver/config/config.schema.json`, so in case of changes to the config -schema, please edit that internal schema instead. +`embedx/config.schema.json`, so in case of changes to the config schema, please +edit that internal schema instead. diff --git a/benchtest.new.txt b/benchtest.new.txt new file mode 100644 index 000000000..ce12ba92b --- /dev/null +++ b/benchtest.new.txt @@ -0,0 +1,7 @@ +goos: linux +goarch: amd64 +pkg: github.com/ory/keto/internal/check +cpu: 11th Gen Intel(R) Core(TM) i7-1185G7 @ 3.00GHz +BenchmarkComputedUsersets/Computed_userset-8 14576 81280 ns/op 1.000 queries/op 17675 B/op 364 allocs/op +PASS +ok github.com/ory/keto/internal/check 2.055s diff --git a/embedx/config.schema.json b/embedx/config.schema.json index 005afd051..a29f68789 100644 --- a/embedx/config.schema.json +++ b/embedx/config.schema.json @@ -347,8 +347,14 @@ "file:///etc/configs/keto_namespaces.ts", "ws://my.websocket.server/keto_namespaces.ts" ] + }, + "experimental_strict_mode": { + "type": "boolean", + "title": "Strict permission checking mode", + "description": "EXPERIMENTAL: If strict mode is enabled, then relation tuples for permits are not checked directly (but the rewrites are applied). Similarly, subject sets are only expanded if they were declared with SubjectSet<...>. These stricter rules result in much faster checks with fewer queries to the underlying database. The behavior of strict mode might change while it is experimental." } }, + "additionalProperties": false, "required": ["location"] } ] diff --git a/go.mod b/go.mod index cda6f6256..72b55d467 100644 --- a/go.mod +++ b/go.mod @@ -23,7 +23,7 @@ require ( github.com/ory/herodot v0.9.13 github.com/ory/jsonschema/v3 v3.0.7 github.com/ory/keto/proto v0.10.0-alpha.0 - github.com/ory/x v0.0.524 + github.com/ory/x v0.0.531 github.com/pelletier/go-toml v1.9.5 github.com/phayes/freeport v0.0.0-20220201140144-74d24b5ae9f5 github.com/pkg/errors v0.9.1 @@ -77,6 +77,7 @@ require ( github.com/felixge/fgprof v0.9.3 // indirect github.com/felixge/httpsnoop v1.0.3 // indirect github.com/fsnotify/fsnotify v1.6.0 // indirect + github.com/go-bindata/go-bindata v3.1.2+incompatible // indirect github.com/go-logr/logr v1.2.3 // indirect github.com/go-logr/stdr v1.2.2 // indirect github.com/go-openapi/jsonpointer v0.19.5 // indirect @@ -94,6 +95,7 @@ require ( github.com/gofrs/flock v0.8.1 // indirect github.com/gogo/protobuf v1.3.2 // indirect github.com/golang/glog v1.0.0 // indirect + github.com/golang/mock v1.6.0 // indirect github.com/golang/protobuf v1.5.2 // indirect github.com/google/pprof v0.0.0-20221010195024-131d412537ea // indirect github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 // indirect diff --git a/go.sum b/go.sum index 8e3fa6b10..d59861666 100644 --- a/go.sum +++ b/go.sum @@ -192,6 +192,8 @@ github.com/fsnotify/fsnotify v1.6.0 h1:n+5WquG0fcWoWp6xPWfHdbskMCQaFnG6PfBrh1Ky4 github.com/fsnotify/fsnotify v1.6.0/go.mod h1:sl3t1tCWJFWoRz9R8WJCbQihKKwmorjAbSClcnxKAGw= github.com/ghodss/yaml v1.0.0 h1:wQHKEahhL6wmXdzwWG11gIVCkOv05bNOh+Rxn0yngAk= github.com/ghodss/yaml v1.0.0/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04= +github.com/go-bindata/go-bindata v3.1.2+incompatible h1:5vjJMVhowQdPzjE1LdxyFF7YFTXg5IgGVW4gBr5IbvE= +github.com/go-bindata/go-bindata v3.1.2+incompatible/go.mod h1:xK8Dsgwmeed+BBsSy2XTopBn/8uK2HWuGSnA11C3Joo= github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1/go.mod h1:vR7hzQXu2zJy9AVAgeJqvqgH9Q5CA+iKCZ2gyEVpxRU= github.com/go-gl/glfw/v3.3/glfw v0.0.0-20191125211704-12ad95a8df72/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4/go.mod h1:tQ2UAYgL5IevRw8kRxooKSPJfGvJ9fJQFa0TUsXzTg8= @@ -278,6 +280,8 @@ github.com/golang/mock v1.4.0/go.mod h1:UOMv5ysSaYNkG+OFQykRIcU/QvvxJf3p21QfJ2Bt github.com/golang/mock v1.4.1/go.mod h1:UOMv5ysSaYNkG+OFQykRIcU/QvvxJf3p21QfJ2Bt3cw= github.com/golang/mock v1.4.3/go.mod h1:UOMv5ysSaYNkG+OFQykRIcU/QvvxJf3p21QfJ2Bt3cw= github.com/golang/mock v1.4.4/go.mod h1:l3mdAwkq5BuhzHwde/uurv3sEJeZMXNpwsxVWU71h+4= +github.com/golang/mock v1.6.0 h1:ErTB+efbowRARo13NNdxyJji2egdxLGQhRaY+DUumQc= +github.com/golang/mock v1.6.0/go.mod h1:p6yTPP+5HYm5mzsMV8JkE6ZKdX+/wYM6Hr+LicevLPs= github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= @@ -643,6 +647,8 @@ github.com/ory/viper v1.7.5 h1:+xVdq7SU3e1vNaCsk/ixsfxE4zylk1TJUiJrY647jUE= github.com/ory/viper v1.7.5/go.mod h1:ypOuyJmEUb3oENywQZRgeAMwqgOyDqwboO1tj3DjTaM= github.com/ory/x v0.0.524 h1:U7JQKiaz+JpWWJvYYqdwVCqXcvI3W9uYO+4v7ew98Vk= github.com/ory/x v0.0.524/go.mod h1:XBqhPZRppPHTxtsE0l0oI/B2Onf1QJtMRGPh3CpEpA0= +github.com/ory/x v0.0.531 h1:ndkhfzgX7y1ChNnYPS5ioqVuvyRENXKUBrNnkmoPOFQ= +github.com/ory/x v0.0.531/go.mod h1:XBqhPZRppPHTxtsE0l0oI/B2Onf1QJtMRGPh3CpEpA0= github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc= github.com/pascaldekloe/goe v0.1.0/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc= github.com/pborman/uuid v1.2.0/go.mod h1:X/NO0urCmaxf9VXbdlT7C2Yzkj2IKimNn4k+gtPdI/k= @@ -1205,6 +1211,7 @@ golang.org/x/tools v0.0.0-20210105154028-b0ab187a4818/go.mod h1:emZCQorbCU4vsT4f golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/tools v0.0.0-20210108195828-e2f9c7f1fc8e/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/tools v0.1.0/go.mod h1:xkSsbof2nBLbhDlRMhhhyNLN/zl3eTqcnHD5viDpcZ0= +golang.org/x/tools v0.1.1/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/tools v0.1.2/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/tools v0.1.7/go.mod h1:LGqMHiF4EqQNHR1JncWGqT5BVaXmza+X+BDGol+dOxo= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= diff --git a/internal/check/2023-01-09-benchtest.txt b/internal/check/2023-01-09-benchtest.txt new file mode 100644 index 000000000..cf97a9d44 --- /dev/null +++ b/internal/check/2023-01-09-benchtest.txt @@ -0,0 +1,7 @@ +goos: linux +goarch: amd64 +pkg: github.com/ory/keto/internal/check +cpu: 11th Gen Intel(R) Core(TM) i7-1185G7 @ 3.00GHz +BenchmarkComputedUsersets/Computed_userset-8 2155 583306 ns/op 9.000 queries/op 109370 B/op 1556 allocs/op +PASS +ok github.com/ory/keto/internal/check 1.351s diff --git a/internal/check/bench_test.go b/internal/check/bench_test.go index 23b962bfa..e16b220b8 100644 --- a/internal/check/bench_test.go +++ b/internal/check/bench_test.go @@ -5,15 +5,20 @@ package check_test import ( "context" + _ "embed" "fmt" + "strings" "testing" "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.opentelemetry.io/otel/sdk/trace" + "go.opentelemetry.io/otel/sdk/trace/tracetest" "github.com/ory/keto/internal/check" "github.com/ory/keto/internal/check/checkgroup" + "github.com/ory/keto/internal/driver" "github.com/ory/keto/internal/driver/config" "github.com/ory/keto/internal/namespace" "github.com/ory/keto/internal/namespace/ast" @@ -96,7 +101,6 @@ func BenchmarkCheckEngine(b *testing.B) { require.NoError(b, reg.Config(ctx).Set(config.KeyLimitMaxReadDepth, 100*maxDepth)) e := check.NewEngine(reg) - b.ResetTimer() b.Run("case=deep tree", func(b *testing.B) { for _, depth := range depths { b.Run(fmt.Sprintf("depth=%03d", depth), func(b *testing.B) { @@ -127,3 +131,53 @@ func BenchmarkCheckEngine(b *testing.B) { } }) } + +//go:embed testfixtures/project_opl.ts +var ProjectOPLConfig string + +func BenchmarkComputedUsersets(b *testing.B) { + ctx := context.Background() + + spans := tracetest.NewSpanRecorder() + tracer := trace.NewTracerProvider(trace.WithSpanProcessor(spans)).Tracer("") + reg := driver.NewSqliteTestRegistry(b, false, + driver.WithLogLevel("debug"), + driver.WithOPL(ProjectOPLConfig), + driver.WithTracer(tracer), + driver.WithConfig(config.KeyNamespacesExperimentalStrictMode, true)) + reg.Logger().Logger.SetLevel(logrus.DebugLevel) + + insertFixtures(b, reg.RelationTupleManager(), []string{ + "Project:Ory#owner@User:Admin", + "Project:Ory#developer@User:Dev", + }) + + e := check.NewEngine(reg) + + query := tupleFromString(b, "Project:Ory#readProject@User:Dev") + + b.Run("Computed userset", func(b *testing.B) { + initialDBSpans := dbSpans(spans) + for i := 0; i < b.N; i++ { + res := e.CheckRelationTuple(ctx, query, 0) + assert.NoError(b, res.Err) + if res.Err != nil { + b.Errorf("got unexpected error: %v", res.Err) + } + if res.Membership != checkgroup.IsMember { + b.Error("check failed") + } + } + b.ReportMetric((float64(dbSpans(spans)-initialDBSpans))/float64(b.N), "queries/op") + }) + +} + +func dbSpans(spans *tracetest.SpanRecorder) (count int) { + for _, s := range spans.Started() { + if strings.HasPrefix(s.Name(), "persistence.sql") { + count++ + } + } + return +} diff --git a/internal/check/engine.go b/internal/check/engine.go index 5b7e6ec80..5fd24e6c4 100644 --- a/internal/check/engine.go +++ b/internal/check/engine.go @@ -5,7 +5,6 @@ package check import ( "context" - "fmt" "github.com/ory/herodot" "github.com/pkg/errors" @@ -17,6 +16,7 @@ import ( "github.com/ory/keto/internal/driver/config" "github.com/ory/keto/internal/namespace" "github.com/ory/keto/internal/namespace/ast" + "github.com/ory/keto/internal/persistence" "github.com/ory/keto/internal/relationtuple" "github.com/ory/keto/internal/x" "github.com/ory/keto/internal/x/graph" @@ -32,6 +32,7 @@ type ( } EngineDependencies interface { relationtuple.ManagerProvider + persistence.Provider config.Provider x.LoggerProvider } @@ -86,7 +87,7 @@ func (e *Engine) CheckRelationTuple(ctx context.Context, r *relationTuple, restD } resultCh := make(chan checkgroup.Result) - go e.checkIsAllowed(ctx, r, restDepth)(ctx, resultCh) + go e.checkIsAllowed(ctx, r, restDepth, false)(ctx, resultCh) select { case result := <-resultCh: return result @@ -98,10 +99,10 @@ func (e *Engine) CheckRelationTuple(ctx context.Context, r *relationTuple, restD // checkExpandSubject checks the expansions of the subject set of the tuple. // // For a relation tuple n:obj#rel@user, checkExpandSubject first queries for all -// subjects that match n:obj#rel@* (arbitrary subjects), and then for each -// subject set checks subject@user. +// tuples that match n:obj#rel@* (arbitrary subjects), and then for each +// subject set checks subject_set@user. func (e *Engine) checkExpandSubject(r *relationTuple, restDepth int) checkgroup.CheckFunc { - if restDepth < 0 { + if restDepth <= 0 { e.d.Logger(). WithField("request", r.String()). Debug("reached max-depth, therefore this query will not be further expanded") @@ -113,56 +114,45 @@ func (e *Engine) checkExpandSubject(r *relationTuple, restDepth int) checkgroup. Trace("check expand subject") g := checkgroup.New(ctx) + defer func() { resultCh <- g.Result() }() var ( - subjects []*relationTuple - pageToken string - err error - visited bool - innerCtx = graph.InitVisited(ctx) - query = &query{Namespace: &r.Namespace, Object: &r.Object, Relation: &r.Relation} + visited bool + innerCtx = graph.InitVisited(ctx) ) - for { - subjects, pageToken, err = e.d.RelationTupleManager().GetRelationTuples(innerCtx, query, x.WithToken(pageToken)) - if errors.Is(err, herodot.ErrNotFound) { - g.Add(checkgroup.NotMemberFunc) - break - } else if err != nil { - g.Add(checkgroup.ErrorFunc(err)) - break - } - for _, s := range subjects { - innerCtx, visited = graph.CheckAndAddVisited(innerCtx, s.Subject) - if visited { - continue - } - subjectSet, ok := s.Subject.(*relationtuple.SubjectSet) - if !ok || subjectSet.Relation == "" { - continue - } - g.Add(e.checkIsAllowed( - innerCtx, - &relationTuple{ - Namespace: subjectSet.Namespace, - Object: subjectSet.Object, - Relation: subjectSet.Relation, - Subject: r.Subject, - }, - restDepth-1, - )) - } - if pageToken == "" || g.Done() { - break + + results, err := e.d.Traverser().TraverseSubjectSetExpansion(ctx, r) + + if errors.Is(err, herodot.ErrNotFound) { + g.Add(checkgroup.NotMemberFunc) + return + } else if err != nil { + g.Add(checkgroup.ErrorFunc(err)) + return + } + + // See if the current hop was already enough to answer the check + for _, result := range results { + if result.Found { + g.Add(checkgroup.IsMemberFunc) + return } } - resultCh <- g.Result() + // If not, we must go another hop: + for _, result := range results { + innerCtx, visited = graph.CheckAndAddVisited(innerCtx, result.To.Subject) + if visited { + continue + } + g.Add(e.checkIsAllowed(innerCtx, result.To, restDepth, true)) + } } } // checkDirect checks if the relation tuple is in the database directly. func (e *Engine) checkDirect(r *relationTuple, restDepth int) checkgroup.CheckFunc { - if restDepth < 0 { + if restDepth <= 0 { e.d.Logger(). WithField("method", "checkDirect"). Debug("reached max-depth, therefore this query will not be further expanded") @@ -172,11 +162,22 @@ func (e *Engine) checkDirect(r *relationTuple, restDepth int) checkgroup.CheckFu e.d.Logger(). WithField("request", r.String()). Trace("check direct") - if rels, _, err := e.d.RelationTupleManager().GetRelationTuples( + found, err := e.d.RelationTupleManager().ExistsRelationTuples( ctx, r.ToQuery(), - x.WithSize(1), - ); err == nil && len(rels) > 0 { + ) + + switch { + case err != nil: + e.d.Logger(). + WithField("method", "checkDirect"). + WithError(err). + Error("failed to look up direct access in db") + resultCh <- checkgroup.Result{ + Membership: checkgroup.NotMember, + } + + case found: resultCh <- checkgroup.Result{ Membership: checkgroup.IsMember, Tree: &ketoapi.Tree[*relationtuple.RelationTuple]{ @@ -184,7 +185,8 @@ func (e *Engine) checkDirect(r *relationTuple, restDepth int) checkgroup.CheckFu Tuple: r, }, } - } else { + + default: resultCh <- checkgroup.Result{ Membership: checkgroup.NotMember, } @@ -196,8 +198,8 @@ func (e *Engine) checkDirect(r *relationTuple, restDepth int) checkgroup.CheckFu // the relation tuple subject to the namespace, object and relation) either // directly (in the database), or through subject-set expansions, or through // user-set rewrites. -func (e *Engine) checkIsAllowed(ctx context.Context, r *relationTuple, restDepth int) checkgroup.CheckFunc { - if restDepth < 0 { +func (e *Engine) checkIsAllowed(ctx context.Context, r *relationTuple, restDepth int, skipDirect bool) checkgroup.CheckFunc { + if restDepth <= 0 { e.d.Logger(). WithField("method", "checkIsAllowed"). Debug("reached max-depth, therefore this query will not be further expanded") @@ -209,55 +211,43 @@ func (e *Engine) checkIsAllowed(ctx context.Context, r *relationTuple, restDepth Trace("check is allowed") g := checkgroup.New(ctx) - g.Add(e.checkDirect(r, restDepth-1)) - g.Add(e.checkExpandSubject(r, restDepth)) relation, err := e.astRelationFor(ctx, r) if err != nil { g.Add(checkgroup.ErrorFunc(err)) - } else if relation != nil && relation.SubjectSetRewrite != nil { + return g.CheckFunc() + } + hasRewrite := relation != nil && relation.SubjectSetRewrite != nil + strictMode := e.d.Config(ctx).StrictMode() + canHaveSubjectSets := !strictMode || relation == nil || containsSubjectSetExpand(relation) + if hasRewrite { g.Add(e.checkSubjectSetRewrite(ctx, r, relation.SubjectSetRewrite, restDepth)) } - - return g.CheckFunc() -} - -func (e *Engine) astRelationFor(ctx context.Context, r *relationTuple) (*ast.Relation, error) { - // Special case: If the relationTuple's relation is empty, then it is not an - // error that the relation was not found. - if r.Relation == "" { - return nil, nil + if (!strictMode || !hasRewrite) && !skipDirect { + // In strict mode, add a direct check only if there is no subject set rewrite for this relation. + // Rewrites are added as 'permits'. + g.Add(e.checkDirect(r, restDepth-1)) } - - ns, err := e.namespaceFor(ctx, r) - if err != nil { - // On an unknown namespace the answer should be "not allowed", not "not - // found". Therefore we don't return the error here. - return nil, nil + if canHaveSubjectSets { + g.Add(e.checkExpandSubject(r, restDepth-1)) } - // Special case: If Relations is empty, then there is no namespace - // configuration, and it is not an error that the relation was not found. - if len(ns.Relations) == 0 { - return nil, nil - } + return g.CheckFunc() +} - for _, rel := range ns.Relations { - if rel.Name == r.Relation { - return &rel, nil +func containsSubjectSetExpand(relation *ast.Relation) bool { + for _, t := range relation.Types { + if t.Relation != "" { + return true } } - return nil, fmt.Errorf("relation %q not found", r.Relation) + return false } -func (e *Engine) namespaceFor(ctx context.Context, r *relationTuple) (*namespace.Namespace, error) { +func (e *Engine) astRelationFor(ctx context.Context, r *relationTuple) (*ast.Relation, error) { namespaceManager, err := e.d.Config(ctx).NamespaceManager() if err != nil { return nil, err } - ns, err := namespaceManager.GetNamespaceByName(ctx, r.Namespace) - if err != nil { - return nil, err - } - return ns, nil + return namespace.ASTRelationFor(ctx, namespaceManager, r.Namespace, r.Relation) } diff --git a/internal/check/engine_test.go b/internal/check/engine_test.go index 4d31da900..9534a96b3 100644 --- a/internal/check/engine_test.go +++ b/internal/check/engine_test.go @@ -15,19 +15,21 @@ import ( "github.com/ory/keto/internal/driver" "github.com/ory/keto/internal/driver/config" "github.com/ory/keto/internal/namespace" + "github.com/ory/keto/internal/persistence" "github.com/ory/keto/internal/relationtuple" "github.com/ory/keto/internal/x" "github.com/ory/keto/ketoapi" ) type configProvider = config.Provider -type loggerProvider = x.LoggerProvider -// deps is defined to capture engine dependencies in a single struct +// deps are defined to capture engine dependencies in a single struct type deps struct { *relationtuple.ManagerWrapper // managerProvider + persistence.Provider configProvider - loggerProvider + x.LoggerProvider + x.TracingProvider } func newDepsProvider(t testing.TB, namespaces []*namespace.Namespace, pageOpts ...x.PaginationOptionSetter) *deps { @@ -36,9 +38,11 @@ func newDepsProvider(t testing.TB, namespaces []*namespace.Namespace, pageOpts . mr := relationtuple.NewManagerWrapper(t, reg, pageOpts...) return &deps{ - ManagerWrapper: mr, - configProvider: reg, - loggerProvider: reg, + ManagerWrapper: mr, + Provider: reg, + configProvider: reg, + LoggerProvider: reg, + TracingProvider: reg, } } @@ -107,7 +111,7 @@ func TestEngine(t *testing.T) { // global max-depth takes precedence and max-depth=2 is not enough require.NoError(t, reg.Config(ctx).Set(config.KeyLimitMaxReadDepth, 2)) - res, err = e.CheckIsMember(ctx, userHasAccess, 3) + res, err = e.CheckIsMember(ctx, userHasAccess, 2) require.NoError(t, err) assert.False(t, res) @@ -284,67 +288,26 @@ func TestEngine(t *testing.T) { }) t.Run("indirect inclusion level 2", func(t *testing.T) { - object := uuid.Must(uuid.NewV4()) - someNamespace := "some_namespaces" - user := relationtuple.SubjectID{ID: uuid.Must(uuid.NewV4())} - organization := uuid.Must(uuid.NewV4()) - orgNamespace := "organizations" - - ownerUserSet := relationtuple.SubjectSet{ - Namespace: someNamespace, - Relation: "owner", - Object: object, - } - orgMembers := relationtuple.SubjectSet{ - Namespace: orgNamespace, - Relation: "member", - Object: organization, - } - - writeRel := relationtuple.RelationTuple{ - Namespace: someNamespace, - Relation: "write", - Object: object, - Subject: &ownerUserSet, - } - orgOwnerRel := relationtuple.RelationTuple{ - Namespace: someNamespace, - Relation: ownerUserSet.Relation, - Object: object, - Subject: &orgMembers, - } - userMembershipRel := relationtuple.RelationTuple{ - Namespace: orgNamespace, - Relation: orgMembers.Relation, - Object: organization, - Subject: &user, - } - reg := newDepsProvider(t, []*namespace.Namespace{ - {Name: someNamespace}, - {Name: orgNamespace}, + {Name: "obj"}, + {Name: "org"}, + }) + //require.NoError(t, reg.RelationTupleManager().WriteRelationTuples(ctx, &writeRel, &orgOwnerRel, &userMembershipRel)) + insertFixtures(t, reg.RelationTupleManager(), []string{ + "obj:object#write@obj:object#owner", + "obj:object#owner@org:organization#member", + "org:organization#member@user", }) - require.NoError(t, reg.RelationTupleManager().WriteRelationTuples(ctx, &writeRel, &orgOwnerRel, &userMembershipRel)) e := check.NewEngine(reg) // user can write object - res, err := e.CheckIsMember(ctx, &relationtuple.RelationTuple{ - Namespace: someNamespace, - Relation: writeRel.Relation, - Object: object, - Subject: &user, - }, 0) + res, err := e.CheckIsMember(ctx, tupleFromString(t, "obj:object#write@user"), 0) require.NoError(t, err) assert.True(t, res) // user is member of the organization - res, err = e.CheckIsMember(ctx, &relationtuple.RelationTuple{ - Namespace: orgNamespace, - Relation: orgMembers.Relation, - Object: organization, - Subject: &user, - }, 0) + res, err = e.CheckIsMember(ctx, tupleFromString(t, "org:organization#member@user"), 0) require.NoError(t, err) assert.True(t, res) }) @@ -536,4 +499,34 @@ func TestEngine(t *testing.T) { require.NoError(t, err) assert.False(t, res) }) + + t.Run("case=strict mode", func(t *testing.T) { + reg := driver.NewSqliteTestRegistry(t, false, + driver.WithOPL(ProjectOPLConfig), + driver.WithConfig(config.KeyNamespacesExperimentalStrictMode, true)) + + insertFixtures(t, reg.RelationTupleManager(), []string{ + "Project:abc#owner@User:1", + "Project:abc#owner@User1", + // The following tuples are ignored in strict mode + "Project:abc#isOwner@User:isOwner", + "Project:abc#readProject@readProjectUser", + "Project:abc#readProject@User:ReadProject", + }) + + e := check.NewEngine(reg) + + for _, sub := range []string{"readProjectUser", "User:ReadProject", "User:isOwner"} { + // These checks should return false, even though the exact tuple is in the db. + res, err := e.CheckIsMember(ctx, tupleFromString(t, "Project:abc#readProject@"+sub), 10) + require.NoError(t, err) + assert.False(t, res) + } + + for _, sub := range []string{"User:1", "User1"} { + res, err := e.CheckIsMember(ctx, tupleFromString(t, "Project:abc#readProject@"+sub), 10) + require.NoError(t, err) + assert.True(t, res) + } + }) } diff --git a/internal/check/performance_test.go b/internal/check/performance_test.go deleted file mode 100644 index 2bb37da16..000000000 --- a/internal/check/performance_test.go +++ /dev/null @@ -1,161 +0,0 @@ -// Copyright © 2023 Ory Corp -// SPDX-License-Identifier: Apache-2.0 - -package check_test - -//import ( -// "context" -// "fmt" -// "strconv" -// "testing" -// -// "github.com/stretchr/testify/assert" -// "github.com/stretchr/testify/require" -// -// "github.com/ory/keto/internal/driver" -// "github.com/ory/keto/internal/e2e" -// "github.com/ory/keto/internal/expand" -// "github.com/ory/keto/internal/namespace" -// "github.com/ory/keto/internal/relationtuple" -//) -// -//const ( -// defaultNamespace = "default" -// indirectRelation = "indirect" -// root = "root" -// -// tuplesPerLevel = 10 -//) -// -//func createDataset(b testing.TB, reg driver.Registry, depth int, parent string) (created int) { -// newTuples := make([]*relationtuple.InternalRelationTuple, tuplesPerLevel) -// -// for p := 0; p < tuplesPerLevel; p++ { -// me := parent + " " + strconv.Itoa(p) -// newTuples[p] = &relationtuple.InternalRelationTuple{ -// Namespace: defaultNamespace, -// Object: parent, -// Relation: indirectRelation, -// Subject: &relationtuple.SubjectSet{ -// Namespace: defaultNamespace, -// Object: me, -// Relation: indirectRelation, -// }, -// } -// -// if depth >= 1 { -// created += createDataset(b, reg, depth-1, me) -// } -// } -// -// require.NoError(b, reg.Persister().WriteRelationTuples(context.Background(), newTuples...)) -// return created + tuplesPerLevel -//} -// -//type noErrButResult struct { -// res bool -// cid int -// o string -//} -// -//func (e *noErrButResult) Error() string { -// return "why did you call me, wtf?" -//} -// -//func XBenchmarkCheckEngine(b *testing.B) { -// for _, dsn := range e2e.GetDSNs(b) { -// func(dsn *e2e.DsnT) { -// nspaces := []*namespace.Namespace{{Name: defaultNamespace, ID: 1}} -// ctx, reg := e2e.NewInitializedReg(b, dsn, nspaces) -// -// b.ResetTimer() -// -// clientC := make([]chan int, 100) -// clientErrs := make(chan error, 100) -// for ci := range clientC { -// clientC[ci] = make(chan int) -// -// go func(ci int) { -// for d := range clientC[ci] { -// o := root -// for i := 0; i < d; i++ { -// o += " " + strconv.Itoa(ci%tuplesPerLevel) -// } -// -// res, err := reg.PermissionEngine().SubjectIsAllowed(ctx, &relationtuple.InternalRelationTuple{ -// Namespace: defaultNamespace, -// Object: root, -// Relation: indirectRelation, -// Subject: &relationtuple.SubjectSet{ -// Namespace: defaultNamespace, -// Object: o, -// Relation: indirectRelation, -// }, -// }) -// if err == nil { -// err = &noErrButResult{res: res, cid: ci, o: o} -// } -// clientErrs <- err -// } -// }(ci) -// } -// -// defer func() { -// for _, c := range clientC { -// close(c) -// } -// }() -// -// for depth := 2; depth <= 4; depth += 1 { -// b.StopTimer() -// -// b.Logf("created %d tuples", createDataset(b, reg, depth, root)) -// -// b.StartTimer() -// -// for clients := 1; clients <= 5; clients++ { -// b.Run(fmt.Sprintf("dsn=%s depth=%d clients=%d", dsn.Name, depth, clients), func(b *testing.B) { -// for i := 0; i < b.N; i++ { -// for ci := 0; ci < clients; ci++ { -// clientC[ci] <- depth -// } -// -// for ci := 0; ci < clients; ci++ { -// err := <-clientErrs -// if res, ok := err.(*noErrButResult); ok { -// require.True(b, res.res) -// continue -// } -// -// b.Logf("got err %+v", err) -// b.FailNow() -// } -// } -// }) -// } -// } -// }(dsn) -// } -//} -// -//func XTestCreateDataset(t *testing.T) { -// for _, dsn := range e2e.GetDSNs(t) { -// nspaces := []*namespace.Namespace{{Name: defaultNamespace, ID: 0}} -// -// ctx, reg := e2e.NewInitializedReg(t, dsn, nspaces) -// createDataset(t, reg, 4, "root") -// -// tree, err := reg.ExpandEngine().BuildTree(ctx, &relationtuple.SubjectSet{ -// Namespace: defaultNamespace, -// Object: root, -// Relation: indirectRelation, -// }, 5) -// require.NoError(t, err) -// -// for d := 0; d < 4; d++ { -// assert.Equal(t, expand.Union, tree.Type) -// assert.Len(t, tree.Children, tuplesPerLevel) -// tree = tree.Children[0] -// } -// } -//} diff --git a/internal/check/rewrites.go b/internal/check/rewrites.go index d70dd4c85..9b2008af5 100644 --- a/internal/check/rewrites.go +++ b/internal/check/rewrites.go @@ -36,7 +36,7 @@ func (e *Engine) checkSubjectSetRewrite( rewrite *ast.SubjectSetRewrite, restDepth int, ) checkgroup.CheckFunc { - if restDepth < 0 { + if restDepth <= 0 { e.d.Logger().Debug("reached max-depth, therefore this query will not be further expanded") return checkgroup.UnknownMemberFunc } @@ -46,8 +46,9 @@ func (e *Engine) checkSubjectSetRewrite( Trace("check subject-set rewrite") var ( - op binaryOperator - checks []checkgroup.CheckFunc + op binaryOperator + checks []checkgroup.CheckFunc + handled = make(map[int]struct{}) ) switch rewrite.Operation { case ast.OperatorOr: @@ -58,7 +59,44 @@ func (e *Engine) checkSubjectSetRewrite( return checkNotImplemented } - for _, child := range rewrite.Children { + // Shortcut for ORs of ComputedSubjectSets + if rewrite.Operation == ast.OperatorOr { + var computedSubjectSets []string + for i, child := range rewrite.Children { + switch c := child.(type) { + case *ast.ComputedSubjectSet: + handled[i] = struct{}{} + computedSubjectSets = append(computedSubjectSets, c.Relation) + } + } + if len(computedSubjectSets) > 0 { + checks = append(checks, func(ctx context.Context, resultCh chan<- checkgroup.Result) { + res, err := e.d.Traverser().TraverseSubjectSetRewrite(ctx, tuple, computedSubjectSets) + if err != nil { + resultCh <- checkgroup.Result{Err: errors.WithStack(err)} + return + } + g := checkgroup.New(ctx) + defer func() { resultCh <- g.Result() }() + for _, result := range res { + if result.Found { + g.SetIsMember() + return + } + } + // If not, we must go another hop: + for _, result := range res { + g.Add(e.checkIsAllowed(ctx, result.To, restDepth-1, true)) + } + }) + } + } + + for i, child := range rewrite.Children { + if _, found := handled[i]; found { + continue + } + switch c := child.(type) { case *ast.TupleToSubjectSet: @@ -77,7 +115,7 @@ func (e *Engine) checkSubjectSetRewrite( checks = append(checks, checkgroup.WithEdge(checkgroup.Edge{ Tuple: *tuple, Type: toTreeNodeType(c.Operation), - }, e.checkSubjectSetRewrite(ctx, tuple, c, restDepth))) + }, e.checkSubjectSetRewrite(ctx, tuple, c, restDepth-1))) case *ast.InvertResult: checks = append(checks, checkgroup.WithEdge(checkgroup.Edge{ @@ -183,16 +221,12 @@ func (e *Engine) checkComputedSubjectSet( WithField("computed subjectSet relation", subjectSet.Relation). Trace("check computed subjectSet") - return e.checkIsAllowed( - ctx, - &relationTuple{ - Namespace: r.Namespace, - Object: r.Object, - Relation: subjectSet.Relation, - Subject: r.Subject, - }, - restDepth, - ) + return e.checkIsAllowed(ctx, &relationTuple{ + Namespace: r.Namespace, + Object: r.Object, + Relation: subjectSet.Relation, + Subject: r.Subject, + }, restDepth, false) } // checkTupleToSubjectSet rewrites the relation tuple to use the subject-set relation. @@ -244,16 +278,12 @@ func (e *Engine) checkTupleToSubjectSet( for _, t := range tuples { if subSet, ok := t.Subject.(*relationtuple.SubjectSet); ok { - g.Add(e.checkIsAllowed( - ctx, - &relationTuple{ - Namespace: subSet.Namespace, - Object: subSet.Object, - Relation: subjectSet.ComputedSubjectSetRelation, - Subject: tuple.Subject, - }, - restDepth-1, - )) + g.Add(e.checkIsAllowed(ctx, &relationTuple{ + Namespace: subSet.Namespace, + Object: subSet.Object, + Relation: subjectSet.ComputedSubjectSetRelation, + Subject: tuple.Subject, + }, restDepth-1, false)) } } diff --git a/internal/check/rewrites_test.go b/internal/check/rewrites_test.go index 48cab738b..ba629338c 100644 --- a/internal/check/rewrites_test.go +++ b/internal/check/rewrites_test.go @@ -234,7 +234,7 @@ func TestUsersetRewrites(t *testing.T) { res := e.CheckRelationTuple(ctx, rt, 100) require.NoError(t, res.Err) t.Logf("tree:\n%s", res.Tree) - assert.Equal(t, tc.expected.Membership, res.Membership) + assert.Equal(t, tc.expected.Membership.String(), res.Membership.String()) if len(tc.expectedPaths) > 0 { for _, path := range tc.expectedPaths { diff --git a/internal/check/testfixtures/project_opl.ts b/internal/check/testfixtures/project_opl.ts new file mode 100644 index 000000000..6a8d5465b --- /dev/null +++ b/internal/check/testfixtures/project_opl.ts @@ -0,0 +1,29 @@ +// Copyright © 2023 Ory Corp +// SPDX-License-Identifier: Apache-2.0 + +import {Namespace, Context} from '@ory/keto-namespace-types' + +class User implements Namespace {} + +class Project implements Namespace { + related: { + owner: User[] + developer: User[] + } + + permits = { + isOwner: (ctx: Context) => this.related.owner.includes(ctx.subject), + isOwnerOrDeveloper: (ctx: Context) => + this.related.owner.includes(ctx.subject) || + this.related.developer.includes(ctx.subject), + writeCollaborator: (ctx: Context) => + this.permits.isOwner(ctx), + readCollaborator: (ctx: Context) => + this.permits.isOwnerOrDeveloper(ctx), + deleteProject: (ctx: Context) => this.permits.isOwner(ctx), + writeProject: (ctx: Context) => + this.permits.isOwnerOrDeveloper(ctx), + readProject: (ctx: Context) => + this.permits.isOwnerOrDeveloper(ctx), + } +} diff --git a/internal/check/testmain_test.go b/internal/check/testmain_test.go index a0065a88b..66f7774ee 100644 --- a/internal/check/testmain_test.go +++ b/internal/check/testmain_test.go @@ -16,5 +16,6 @@ func TestMain(m *testing.M) { goleak.IgnoreTopFunction("github.com/ory/keto/internal/check/checkgroup.worker"), goleak.IgnoreTopFunction("net/http.(*persistConn).readLoop"), goleak.IgnoreTopFunction("net/http.(*persistConn).writeLoop"), + goleak.IgnoreTopFunction("database/sql.(*DB).connectionOpener"), ) } diff --git a/internal/driver/config/provider.go b/internal/driver/config/provider.go index e7748c1db..377cc6c6c 100644 --- a/internal/driver/config/provider.go +++ b/internal/driver/config/provider.go @@ -49,7 +49,8 @@ const ( KeyMetricsHost = "serve." + string(EndpointMetrics) + ".host" KeyMetricsPort = "serve." + string(EndpointMetrics) + ".port" - KeyNamespaces = "namespaces" + KeyNamespaces = "namespaces" + KeyNamespacesExperimentalStrictMode = KeyNamespaces + ".experimental_strict_mode" DSNMemory = "sqlite://file::memory:?_fk=true&cache=shared" ) @@ -247,6 +248,10 @@ func (k *Config) NamespaceManager() (namespace.Manager, error) { return k.nm, nil } +func (k *Config) StrictMode() bool { + return k.p.BoolF(KeyNamespacesExperimentalStrictMode, false) +} + type ( buildNamespaceFn func(context.Context, *Config) (namespace.Manager, error) diff --git a/internal/driver/config/provider_test.go b/internal/driver/config/provider_test.go index 302d39958..1c2e11a88 100644 --- a/internal/driver/config/provider_test.go +++ b/internal/driver/config/provider_test.go @@ -105,6 +105,7 @@ namespaces: require.NoError(t, err) _, ok := nm.(*memoryNamespaceManager) assert.True(t, ok) + assert.False(t, p.StrictMode()) } } @@ -152,6 +153,7 @@ namespaces: file://%s`, require.NoError(t, err) _, ok := nm.(*NamespaceWatcher) assert.True(t, ok) + assert.False(t, p.StrictMode()) }) t.Run("case=uses passed configx provider", func(t *testing.T) { @@ -232,8 +234,20 @@ namespaces: names, relationNames := []string{namespaces[0].Name, namespaces[1].Name}, []string{namespaces[0].Relations[0].Name, namespaces[1].Relations[0].Name} + assert.False(t, p.StrictMode()) assert.ElementsMatch(t, names, []string{"User", "Group"}) assert.ElementsMatch(t, relationNames, []string{"manager", "members"}) }) } + + t.Run("case=strict_mode=true", func(t *testing.T) { + config := createFileF(t, ` +dsn: memory +namespaces: + location: file://%s + experimental_strict_mode: true`, oplConfigFile) + + _, p := setup(t, config) + assert.True(t, p.StrictMode()) + }) } diff --git a/internal/driver/registry_default.go b/internal/driver/registry_default.go index bc8518fc6..db621fd68 100644 --- a/internal/driver/registry_default.go +++ b/internal/driver/registry_default.go @@ -50,6 +50,7 @@ var ( type ( RegistryDefault struct { p persistence.Persister + traverser relationtuple.Traverser mb *popx.MigrationBox l *logrusx.Logger w herodot.Writer @@ -198,6 +199,13 @@ func (r *RegistryDefault) Persister() persistence.Persister { return r.p } +func (r *RegistryDefault) Traverser() relationtuple.Traverser { + if r.traverser == nil { + panic("no traverser, but expected to have one") + } + return r.traverser +} + func (r *RegistryDefault) PermissionEngine() *check.Engine { if r.ce == nil { r.ce = check.NewEngine(r) @@ -305,10 +313,12 @@ func (r *RegistryDefault) Init(ctx context.Context) (err error) { return err } - r.p, err = sql.NewPersister(ctx, r, network.ID) + p, err := sql.NewPersister(ctx, r, network.ID) if err != nil { return err } + r.p = p + r.traverser = sql.NewTraverser(p) return nil }() diff --git a/internal/driver/registry_factory.go b/internal/driver/registry_factory.go index a0781f565..dac6381cf 100644 --- a/internal/driver/registry_factory.go +++ b/internal/driver/registry_factory.go @@ -10,11 +10,14 @@ import ( "crypto/rand" "crypto/tls" "fmt" + "os" "sync" "testing" "github.com/ory/x/configx" + "github.com/ory/x/otelx" "github.com/ory/x/tlsx" + "go.opentelemetry.io/otel/trace" "google.golang.org/grpc" "google.golang.org/grpc/credentials" @@ -34,6 +37,29 @@ import ( "github.com/ory/keto/internal/driver/config" ) +// createFile writes the content to a temporary file, returning the path. +// Good for testing config files. +func createFile(t testing.TB, content string) (path string) { + t.Helper() + + f, err := os.CreateTemp(t.TempDir(), "config-*.yaml") + if err != nil { + t.Fatal(err) + } + + t.Cleanup(func() { _ = os.Remove(f.Name()) }) + + n, err := f.WriteString(content) + if err != nil { + t.Fatal(err) + } + if n != len(content) { + t.Fatal("failed to write the complete content") + } + + return f.Name() +} + func NewDefaultRegistry(ctx context.Context, flags *pflag.FlagSet, withoutNetwork bool, opts []ketoctx.Option) (Registry, error) { reg, ok := ctx.Value(RegistryContextKey).(Registry) if ok { @@ -83,24 +109,56 @@ func NewSqliteTestRegistry(t testing.TB, debugOnDisk bool, opts ...TestRegistryO return NewTestRegistry(t, dbx.GetSqlite(t, mode), opts...) } +func NewCRDBTestRegistry(t testing.TB) *RegistryDefault { + var buf [20]byte + _, _ = rand.Read(buf[:]) + testdb := fmt.Sprintf("testdb_%x", buf) + return NewTestRegistry(t, &dbx.DsnT{ + Name: "cockroach", + Conn: dbx.RunCockroach(t, testdb), + MigrateUp: true, + MigrateDown: true, + }) +} + type TestRegistryOption func(t testing.TB, r *RegistryDefault) +func WithConfig(key string, value any) TestRegistryOption { + return func(t testing.TB, r *RegistryDefault) { + require.NoError(t, r.c.Set(key, value)) + } +} func WithNamespaces(namespaces []*namespace.Namespace) TestRegistryOption { return func(t testing.TB, r *RegistryDefault) { require.NoError(t, r.c.Set(config.KeyNamespaces, namespaces)) } } +func WithOPL(opl string) TestRegistryOption { + return func(t testing.TB, r *RegistryDefault) { + f := createFile(t, opl) + require.NoError(t, r.c.Set(config.KeyNamespaces+".location", "file://"+f)) + } +} func WithGRPCUnaryInterceptors(i ...grpc.UnaryServerInterceptor) TestRegistryOption { return func(_ testing.TB, r *RegistryDefault) { r.defaultUnaryInterceptors = i } } - func WithGRPCStreamInterceptors(i ...grpc.StreamServerInterceptor) TestRegistryOption { return func(_ testing.TB, r *RegistryDefault) { r.defaultStreamInterceptors = i } } +func WithTracer(tracer trace.Tracer) TestRegistryOption { + return func(t testing.TB, r *RegistryDefault) { + r.tracer = new(otelx.Tracer).WithOTLP(tracer) + } +} +func WithLogLevel(level string) TestRegistryOption { + return func(t testing.TB, r *RegistryDefault) { + require.NoError(t, r.c.Set("log.level", level)) + } +} type selfSignedCert struct { once sync.Once @@ -144,7 +202,7 @@ func NewTestRegistry(t testing.TB, dsn *dbx.DsnT, opts ...TestRegistryOption) *R ctx = configx.ContextWithConfigOptions(ctx, configx.WithValues(map[string]interface{}{ config.KeyDSN: dsn.Conn, - "log.level": "debug", + "log.level": "info", config.KeyNamespaces: []*namespace.Namespace{}, })) c, err := config.NewDefault(ctx, nil, l) diff --git a/internal/namespace/definitions.go b/internal/namespace/definitions.go index 25e2073b6..386610ea6 100644 --- a/internal/namespace/definitions.go +++ b/internal/namespace/definitions.go @@ -6,6 +6,7 @@ package namespace import ( "context" "encoding/json" + "fmt" "github.com/ory/keto/internal/namespace/ast" ) @@ -31,3 +32,30 @@ type ( NamespaceManager() (Manager, error) } ) + +func ASTRelationFor(ctx context.Context, m Manager, namespace, relation string) (*ast.Relation, error) { + // Special case: If the relationTuple's relation is empty, then it is not an + // error that the relation was not found. + if relation == "" { + return nil, nil + } + ns, err := m.GetNamespaceByName(ctx, namespace) + if err != nil { + // On an unknown namespace the answer should be "not allowed", not "not + // found". Therefore, we don't return the error here. + return nil, nil + } + + // Special case: If Relations is empty, then there is no namespace + // configuration, and it is not an error that the relation was not found. + if len(ns.Relations) == 0 { + return nil, nil + } + + for _, rel := range ns.Relations { + if rel.Name == relation { + return &rel, nil + } + } + return nil, fmt.Errorf("relation %q not found", relation) +} diff --git a/internal/persistence/definitions.go b/internal/persistence/definitions.go index 58118129d..631d93b60 100644 --- a/internal/persistence/definitions.go +++ b/internal/persistence/definitions.go @@ -7,6 +7,7 @@ import ( "context" "errors" + "github.com/gofrs/uuid" "github.com/ory/x/popx" "github.com/gobuffalo/pop/v6" @@ -20,6 +21,7 @@ type ( relationtuple.MappingManager Connection(ctx context.Context) *pop.Connection + NetworkID(ctx context.Context) uuid.UUID } Migrator interface { MigrationBox(ctx context.Context) (*popx.MigrationBox, error) @@ -28,6 +30,7 @@ type ( } Provider interface { Persister() Persister + Traverser() relationtuple.Traverser } ) diff --git a/internal/persistence/sql/persister.go b/internal/persistence/sql/persister.go index 706038452..dc0e7aceb 100644 --- a/internal/persistence/sql/persister.go +++ b/internal/persistence/sql/persister.go @@ -14,6 +14,7 @@ import ( "github.com/ory/x/popx" "github.com/pkg/errors" + "github.com/ory/keto/internal/driver/config" "github.com/ory/keto/internal/persistence" "github.com/ory/keto/internal/x" "github.com/ory/keto/ketoctx" @@ -33,6 +34,7 @@ type ( x.LoggerProvider x.TracingProvider ketoctx.ContextualizerProvider + config.Provider PopConnection(ctx context.Context) (*pop.Connection, error) } diff --git a/internal/persistence/sql/relationtuples.go b/internal/persistence/sql/relationtuples.go index 53b78875e..f497eb298 100644 --- a/internal/persistence/sql/relationtuples.go +++ b/internal/persistence/sql/relationtuples.go @@ -41,11 +41,11 @@ func (relationTuples) TableName() string { return "keto_relation_tuples" } -func (RelationTuple) TableName() string { +func (*RelationTuple) TableName() string { return "keto_relation_tuples" } -func (r *RelationTuple) toInternal() (*relationtuple.RelationTuple, error) { +func (r *RelationTuple) ToInternal() (*relationtuple.RelationTuple, error) { if r == nil { return nil, nil } @@ -237,7 +237,7 @@ func (p *Persister) GetRelationTuples(ctx context.Context, query *relationtuple. internalRes := make([]*relationtuple.RelationTuple, 0, len(res)) for _, r := range res { - if rt, err := r.toInternal(); err == nil { + if rt, err := r.ToInternal(); err == nil { // Ignore error here, which stems from a deleted namespace. internalRes = append(internalRes, rt) } @@ -246,6 +246,20 @@ func (p *Persister) GetRelationTuples(ctx context.Context, query *relationtuple. return internalRes, nextPageToken, nil } +func (p *Persister) ExistsRelationTuples(ctx context.Context, query *relationtuple.RelationQuery) (_ bool, err error) { + ctx, span := p.d.Tracer(ctx).Tracer().Start(ctx, "persistence.sql.ExistsRelationTuples") + defer otelx.End(span, &err) + + sqlQuery := p.queryWithNetwork(ctx) + + err = p.whereQuery(ctx, sqlQuery, query) + if err != nil { + return false, err + } + exists, err := sqlQuery.Exists(&RelationTuple{}) + return exists, sqlcon.HandleError(err) +} + func (p *Persister) WriteRelationTuples(ctx context.Context, rs ...*relationtuple.RelationTuple) (err error) { ctx, span := p.d.Tracer(ctx).Tracer().Start(ctx, "persistence.sql.WriteRelationTuples") defer otelx.End(span, &err) diff --git a/internal/persistence/sql/traverser.go b/internal/persistence/sql/traverser.go new file mode 100644 index 000000000..4d38479b6 --- /dev/null +++ b/internal/persistence/sql/traverser.go @@ -0,0 +1,207 @@ +// Copyright © 2023 Ory Corp +// SPDX-License-Identifier: Apache-2.0 + +package sql + +import ( + "context" + "fmt" + + "github.com/gobuffalo/pop/v6" + "github.com/gofrs/uuid" + "github.com/ory/x/otelx" + "github.com/ory/x/sqlcon" + "github.com/pkg/errors" + + "github.com/ory/keto/internal/namespace" + "github.com/ory/keto/internal/relationtuple" + "github.com/ory/keto/ketoapi" +) + +type ( + Traverser struct { + conn *pop.Connection + d dependencies + nid uuid.UUID + p *Persister + } + + subjectExpandedRelationTupleRow struct { + RelationTuple + + Found bool `db:"found"` + } + + rewriteRelationTupleRow struct { + RelationTuple + Traversal relationtuple.Traversal `db:"traversal"` + } +) + +func whereSubject(sub relationtuple.Subject) (sqlFragment string, args []any, err error) { + switch s := sub.(type) { + case *relationtuple.SubjectID: + sqlFragment = "subject_id = ? AND subject_set_namespace IS NULL AND subject_set_object IS NULL AND subject_set_relation IS NULL" + args = []any{s.ID} + + case *relationtuple.SubjectSet: + sqlFragment = "subject_id IS NULL AND subject_set_namespace = ? AND subject_set_object = ? AND subject_set_relation = ?" + args = []any{s.Namespace, s.Object, s.Relation} + + case nil: + return "", nil, errors.WithStack(ketoapi.ErrNilSubject) + } + return sqlFragment, args, nil +} + +// TraverseSubjectSetExpansion gets all subject sets for the object#relation. +// It also checks whether the requested subject is a member of each of the returned subject sets. +func (t *Traverser) TraverseSubjectSetExpansion(ctx context.Context, start *relationtuple.RelationTuple) (res []*relationtuple.TraversalResult, err error) { + ctx, span := t.d.Tracer(ctx).Tracer().Start(ctx, "persistence.sql.TraverseSubjectSetExpansion") + defer otelx.End(span, &err) + + targetSubjectSQL, targetSubjectArgs, err := whereSubject(start.Subject) + if err != nil { + return nil, err + } + + shardID := uuid.Nil + for { + var ( + rows []*subjectExpandedRelationTupleRow + limit = 1000 + ) + err = t.conn.RawQuery(fmt.Sprintf(` +SELECT current.shard_id AS shard_id, + current.subject_set_namespace AS namespace, + current.subject_set_object AS object, + current.subject_set_relation AS relation, + EXISTS( + SELECT 1 FROM keto_relation_tuples + WHERE nid = current.nid AND + namespace = current.subject_set_namespace AND + object = current.subject_set_object AND + relation = current.subject_set_relation AND + %s -- subject where clause + ) AS found +FROM keto_relation_tuples AS current +WHERE current.nid = ? AND + current.shard_id > ? AND + current.namespace = ? AND + current.object = ? AND + current.relation = ? AND + current.subject_id IS NULL +ORDER BY current.nid, current.shard_id +LIMIT ? +`, targetSubjectSQL), + append(targetSubjectArgs, t.p.NetworkID(ctx), shardID, start.Namespace, start.Object, start.Relation, limit)..., + ).All(&rows) + if err != nil { + return nil, sqlcon.HandleError(err) + } + + for _, r := range rows { + to, err := r.RelationTuple.ToInternal() + if err != nil { + return nil, errors.WithStack(err) + } + to.Subject = start.Subject + res = append(res, &relationtuple.TraversalResult{ + From: start, + To: to, + Via: relationtuple.TraversalSubjectSetExpand, + Found: r.Found, + }) + if r.Found { + return res, nil + } + } + if len(rows) == limit { + shardID = rows[limit-1].ID + } else { + break + } + } + + return res, nil +} + +func (t *Traverser) TraverseSubjectSetRewrite(ctx context.Context, start *relationtuple.RelationTuple, computedSubjectSets []string) (res []*relationtuple.TraversalResult, err error) { + + namespaceManager, err := t.d.Config(ctx).NamespaceManager() + if err != nil { + return nil, err + } + + var relations []string + for _, relation := range computedSubjectSets { + astRel, _ := namespace.ASTRelationFor(ctx, namespaceManager, start.Namespace, relation) + // In strict mode, we can skip querying for those relations that have userset rewrites defined, + // because we can already apply those rewrites in memory. + if t.d.Config(ctx).StrictMode() && astRel != nil && astRel.SubjectSetRewrite != nil { + continue + } + relations = append(relations, relation) + } + + if len(relations) > 0 { + _, span := t.d.Tracer(ctx).Tracer().Start(ctx, "persistence.sql.TraverseSubjectSetRewrite") + defer otelx.End(span, &err) + + var rows relationTuples + + query := t.p.queryWithNetwork(ctx) + if err := t.p.whereQuery(ctx, query, &relationtuple.RelationQuery{ + Namespace: &start.Namespace, + Object: &start.Object, + Subject: start.Subject, + }); err != nil { + return nil, err + } + err = query.Where("relation IN (?)", relations).Limit(1).All(&rows) + if err != nil { + return nil, sqlcon.HandleError(err) + } + + // If we got any rows back, success! + if len(rows) > 0 { + r := rows[0] + to, err := r.ToInternal() + if err != nil { + return nil, errors.WithStack(err) + } + return []*relationtuple.TraversalResult{{ + From: start, + To: to, + Via: relationtuple.TraversalComputedUserset, + Found: true, + }}, nil + } + } + + // Otherwise, the next candidates are those tuples with relations from the rewrite + for _, relation := range computedSubjectSets { + res = append(res, &relationtuple.TraversalResult{ + From: start, + To: &relationtuple.RelationTuple{ + Namespace: start.Namespace, + Object: start.Object, + Relation: relation, + Subject: start.Subject, + }, + Via: relationtuple.TraversalComputedUserset, + Found: false, + }) + } + + return res, nil +} + +func NewTraverser(p *Persister) *Traverser { + return &Traverser{ + conn: p.conn, + d: p.d, + nid: p.nid, + p: p, + } +} diff --git a/internal/relationtuple/definitions.go b/internal/relationtuple/definitions.go index e499cc6d7..8f99ba34f 100644 --- a/internal/relationtuple/definitions.go +++ b/internal/relationtuple/definitions.go @@ -19,8 +19,13 @@ type ( ManagerProvider interface { RelationTupleManager() Manager } + Traverser interface { + TraverseSubjectSetExpansion(ctx context.Context, tuple *RelationTuple) ([]*TraversalResult, error) + TraverseSubjectSetRewrite(ctx context.Context, tuple *RelationTuple, computedSubjectSets []string) ([]*TraversalResult, error) + } Manager interface { GetRelationTuples(ctx context.Context, query *RelationQuery, options ...x.PaginationOptionSetter) ([]*RelationTuple, string, error) + ExistsRelationTuples(ctx context.Context, query *RelationQuery) (bool, error) WriteRelationTuples(ctx context.Context, rs ...*RelationTuple) error DeleteRelationTuples(ctx context.Context, rs ...*RelationTuple) error DeleteAllRelationTuples(ctx context.Context, query *RelationQuery) error @@ -65,6 +70,22 @@ type ( Subject Subject `json:"subject"` Children []*Tree `json:"children,omitempty"` } + + TraversalResult struct { + From *RelationTuple + To *RelationTuple + Via Traversal + Found bool + } + + Traversal string +) + +const ( + TraversalUnknown Traversal = "unknown" + TraversalSubjectSetExpand Traversal = "subject set expand" + TraversalComputedUserset Traversal = "computed userset" + TraversalTupleToUserset Traversal = "tuple to userset" ) var ( @@ -151,6 +172,10 @@ func (t *ManagerWrapper) GetRelationTuples(ctx context.Context, query *RelationQ return t.Reg.RelationTupleManager().GetRelationTuples(ctx, query, append(t.PageOpts, options...)...) } +func (t *ManagerWrapper) ExistsRelationTuples(ctx context.Context, query *RelationQuery) (bool, error) { + return t.Reg.RelationTupleManager().ExistsRelationTuples(ctx, query) +} + func (t *ManagerWrapper) WriteRelationTuples(ctx context.Context, rs ...*RelationTuple) error { return t.Reg.RelationTupleManager().WriteRelationTuples(ctx, rs...) } diff --git a/package-lock.json b/package-lock.json index f74d57b80..1e8317fac 100644 --- a/package-lock.json +++ b/package-lock.json @@ -14,6 +14,7 @@ "grpc-tools": "^1.11.3" }, "devDependencies": { + "@ory/keto-namespace-types": "file:contrib/namespace-type-lib", "doctoc": "^2.0.1", "license-checker": "^25.0.1", "opencollective": "^1.0.3", @@ -22,6 +23,14 @@ "prettier-plugin-packagejson": "^2.2.18" } }, + "contrib/namespace-type-lib": { + "version": "0.9.0-alpha.0", + "dev": true, + "devDependencies": { + "@ory/keto-namespace-types": "file:./", + "typescript": "^4.7.4" + } + }, "node_modules/@mapbox/node-pre-gyp": { "version": "1.0.10", "resolved": "https://registry.npmjs.org/@mapbox/node-pre-gyp/-/node-pre-gyp-1.0.10.tgz", @@ -207,6 +216,10 @@ "url": "https://opencollective.com/openapi_generator" } }, + "node_modules/@ory/keto-namespace-types": { + "resolved": "contrib/namespace-type-lib", + "link": true + }, "node_modules/@textlint/ast-node-types": { "version": "12.1.1", "resolved": "https://registry.npmjs.org/@textlint/ast-node-types/-/ast-node-types-12.1.1.tgz", @@ -926,7 +939,7 @@ "version": "0.1.13", "resolved": "https://registry.npmjs.org/encoding/-/encoding-0.1.13.tgz", "integrity": "sha512-ETBauow1T35Y/WZMkio9jiM0Z5xjHHmJ4XmjZOq1l/dXz3lr2sRn87nJy20RupqSh1F2m3HHPSp8ShIPQJrJ3A==", - "dev": true, + "devOptional": true, "dependencies": { "iconv-lite": "^0.6.2" } @@ -935,7 +948,7 @@ "version": "0.6.3", "resolved": "https://registry.npmjs.org/iconv-lite/-/iconv-lite-0.6.3.tgz", "integrity": "sha512-4fCk79wshMdzMp2rH06qWrJE4iolqLhCUH+OiuIgU++RB0+94NlDL81atO7GX55uUKueo0txHNtvEyI6D7WdMw==", - "dev": true, + "devOptional": true, "dependencies": { "safer-buffer": ">= 2.1.2 < 3.0.0" }, @@ -3320,6 +3333,19 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/typescript": { + "version": "4.9.4", + "resolved": "https://registry.npmjs.org/typescript/-/typescript-4.9.4.tgz", + "integrity": "sha512-Uz+dTXYzxXXbsFpM86Wh3dKCxrQqUcVMxwU54orwlJjOpO3ao8L7j5lH+dWfTwgCwIuM9GQ2kvVotzYJMXTBZg==", + "dev": true, + "bin": { + "tsc": "bin/tsc", + "tsserver": "bin/tsserver" + }, + "engines": { + "node": ">=4.2.0" + } + }, "node_modules/uglify-js": { "version": "3.16.1", "resolved": "https://registry.npmjs.org/uglify-js/-/uglify-js-3.16.1.tgz", @@ -3685,6 +3711,13 @@ "tslib": "2.0.3" } }, + "@ory/keto-namespace-types": { + "version": "file:contrib/namespace-type-lib", + "requires": { + "@ory/keto-namespace-types": "file:", + "typescript": "^4.7.4" + } + }, "@textlint/ast-node-types": { "version": "12.1.1", "resolved": "https://registry.npmjs.org/@textlint/ast-node-types/-/ast-node-types-12.1.1.tgz", @@ -4225,7 +4258,7 @@ "version": "0.1.13", "resolved": "https://registry.npmjs.org/encoding/-/encoding-0.1.13.tgz", "integrity": "sha512-ETBauow1T35Y/WZMkio9jiM0Z5xjHHmJ4XmjZOq1l/dXz3lr2sRn87nJy20RupqSh1F2m3HHPSp8ShIPQJrJ3A==", - "dev": true, + "devOptional": true, "requires": { "iconv-lite": "^0.6.2" }, @@ -4234,7 +4267,7 @@ "version": "0.6.3", "resolved": "https://registry.npmjs.org/iconv-lite/-/iconv-lite-0.6.3.tgz", "integrity": "sha512-4fCk79wshMdzMp2rH06qWrJE4iolqLhCUH+OiuIgU++RB0+94NlDL81atO7GX55uUKueo0txHNtvEyI6D7WdMw==", - "dev": true, + "devOptional": true, "requires": { "safer-buffer": ">= 2.1.2 < 3.0.0" } @@ -5983,6 +6016,12 @@ "resolved": "https://registry.npmjs.org/type-fest/-/type-fest-0.21.3.tgz", "integrity": "sha512-t0rzBq87m3fVcduHDUFhKmyyX+9eo6WQjZvf51Ea/M0Q7+T374Jp1aUiyUl0GKxp8M/OETVHSDvmkyPgvX+X2w==" }, + "typescript": { + "version": "4.9.4", + "resolved": "https://registry.npmjs.org/typescript/-/typescript-4.9.4.tgz", + "integrity": "sha512-Uz+dTXYzxXXbsFpM86Wh3dKCxrQqUcVMxwU54orwlJjOpO3ao8L7j5lH+dWfTwgCwIuM9GQ2kvVotzYJMXTBZg==", + "dev": true + }, "uglify-js": { "version": "3.16.1", "resolved": "https://registry.npmjs.org/uglify-js/-/uglify-js-3.16.1.tgz", diff --git a/package.json b/package.json index 953111df1..b6ad3c224 100644 --- a/package.json +++ b/package.json @@ -17,6 +17,7 @@ "grpc_tools_node_protoc_ts": "^5.3.2" }, "devDependencies": { + "@ory/keto-namespace-types": "file:contrib/namespace-type-lib", "doctoc": "^2.0.1", "license-checker": "^25.0.1", "opencollective": "^1.0.3",