From 3a267c76c7498806a8d2dac75bc65969063faafc Mon Sep 17 00:00:00 2001 From: David van der Spek Date: Mon, 10 Jul 2023 13:47:27 +0200 Subject: [PATCH 1/8] feat: find rules with prefix matching using trie Signed-off-by: David van der Spek --- rule/repository_memory.go | 47 +++++++--- rule/trie.go | 176 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 210 insertions(+), 13 deletions(-) create mode 100644 rule/trie.go diff --git a/rule/repository_memory.go b/rule/repository_memory.go index 3dc90c1b03..eff40890ff 100644 --- a/rule/repository_memory.go +++ b/rule/repository_memory.go @@ -31,6 +31,7 @@ type RepositoryMemory struct { invalidRules []Rule matchingStrategy configuration.MatchingStrategy r repositoryMemoryRegistry + trie *Trie } // MatchingStrategy returns current MatchingStrategy. @@ -52,6 +53,7 @@ func NewRepositoryMemory(r repositoryMemoryRegistry) *RepositoryMemory { return &RepositoryMemory{ r: r, rules: make([]Rule, 0), + trie: NewTrie(), } } @@ -59,6 +61,9 @@ func NewRepositoryMemory(r repositoryMemoryRegistry) *RepositoryMemory { func (m *RepositoryMemory) WithRules(rules []Rule) { m.Lock() m.rules = rules + for _, rule := range rules { + m.trie.InsertRule(&rule) + } m.Unlock() } @@ -105,6 +110,7 @@ func (m *RepositoryMemory) Set(ctx context.Context, rules []Rule) error { } else { m.rules = append(m.rules, check) } + m.trie.InsertRule(&check) // TODO: this needs error handling } return nil @@ -119,20 +125,35 @@ func (m *RepositoryMemory) Match(ctx context.Context, method string, u *url.URL, defer m.Unlock() var rules []*Rule - for k := range m.rules { - r := &m.rules[k] - if matched, err := r.IsMatching(m.matchingStrategy, method, u, protocol); err != nil { - return nil, errors.WithStack(err) - } else if matched { - rules = append(rules, r) + + var rulesFromTrie []*Rule + if m.trie != nil { + rulesFromTrie = m.trie.Match(u) + + for _, r := range rulesFromTrie { + if matched, err := r.IsMatching(m.matchingStrategy, method, u, protocol); err != nil { + return nil, errors.WithStack(err) + } else if matched { + rules = append(rules, r) + } } - } - for k := range m.invalidRules { - r := &m.invalidRules[k] - if matched, err := r.IsMatching(m.matchingStrategy, method, u, protocol); err != nil { - return nil, errors.WithStack(err) - } else if matched { - rules = append(rules, r) + } else { + + for k := range m.rules { + r := &m.rules[k] + if matched, err := r.IsMatching(m.matchingStrategy, method, u, protocol); err != nil { + return nil, errors.WithStack(err) + } else if matched { + rules = append(rules, r) + } + } + for k := range m.invalidRules { + r := &m.invalidRules[k] + if matched, err := r.IsMatching(m.matchingStrategy, method, u, protocol); err != nil { + return nil, errors.WithStack(err) + } else if matched { + rules = append(rules, r) + } } } diff --git a/rule/trie.go b/rule/trie.go new file mode 100644 index 0000000000..4c9a69477d --- /dev/null +++ b/rule/trie.go @@ -0,0 +1,176 @@ +package rule + +import ( + "fmt" + "net/url" + "strings" +) + +// types for a trie root node +type TrieNode struct { + children map[string]*TrieNode + rules []*Rule + // isWord bool +} + +// types for a trie node +type Trie struct { + root *TrieNode +} + +// NewTrie creates a new trie +func NewTrie() *Trie { + return &Trie{ + root: &TrieNode{ + children: make(map[string]*TrieNode), + }, + } +} + +// Insert inserts a word into the trie +func (t *Trie) InsertString(word string) { + node := t.root + if _, ok := node.children[word]; !ok { + node.children[word] = &TrieNode{ + children: make(map[string]*TrieNode), + } + } + node = node.children[word] +} + +// Insert a url host and paths into the trie along with the rule +func (t *Trie) InsertURL(u *url.URL, r *Rule) { + // TODO: should this also handle scheme? + + node := t.root + // insert the host into the trie + if _, ok := node.children[u.Host]; !ok { + node.children[u.Host] = &TrieNode{ + children: make(map[string]*TrieNode), + } + } + node = node.children[u.Host] + + // remove the leading and trailing slash + trimmedPath := strings.Trim(u.Path, "/") + if len(trimmedPath) == 0 { + node.rules = append(node.rules, r) + } else { + // insert the paths into the trie + splitPaths := strings.Split(trimmedPath, "/") + i := 0 + for _, path := range splitPaths { + i++ + if _, ok := node.children[string(path)]; !ok { + node.children[string(path)] = &TrieNode{ + children: make(map[string]*TrieNode), + } + } + node = node.children[string(path)] + if i == len(splitPaths) { + node.rules = append(node.rules, r) + } + } + } +} + +// Insert a url host and paths into the trie +func (t *Trie) InsertRule(r *Rule) { + node := t.root + + // TODO: handle error properly + matchURL, err := url.Parse(r.Match.GetURL()) + if err != nil { + fmt.Println("error parsing url") + } + + // TODO: should this also handle scheme? + + // insert the host into the trie + if _, ok := node.children[matchURL.Host]; !ok { + node.children[matchURL.Host] = &TrieNode{ + children: make(map[string]*TrieNode), + } + } + node = node.children[matchURL.Host] + // remove the leading and trailing slash + trimmedPath := strings.Trim(matchURL.Path, "/") + + if len(trimmedPath) == 0 { + node.rules = append(node.rules, r) + } else { + + // insert the paths into the trie + splitPaths := strings.Split(trimmedPath, "/") + i := 0 + for _, path := range splitPaths { + i++ + if _, ok := node.children[string(path)]; !ok { + node.children[string(path)] = &TrieNode{ + children: make(map[string]*TrieNode), + } + } + node = node.children[string(path)] + // if this is the last path, append the rule + if i == len(splitPaths) { + node.rules = append(node.rules, r) + } + } + } +} + +// return the longest prefix of the url that is in the trie +func (t *Trie) LongestPrefix(u *url.URL) string { + node := t.root + var prefix string + // check the host + if _, ok := node.children[u.Host]; !ok { + return prefix + } + prefix += u.Host + node = node.children[u.Host] + // check the paths + // remove the leading and trailing slash + trimmedPath := strings.Trim(u.Path, "/") + + if len(trimmedPath) > 0 { + splitPaths := strings.Split(trimmedPath, "/") + for _, path := range splitPaths { + if _, ok := node.children[string(path)]; !ok { + break + } + prefix += "/" + string(path) + node = node.children[string(path)] + } + } + return prefix + +} + +// return the rules of the longest prefix of the url that is in the trie +func (t *Trie) Match(u *url.URL) []*Rule { + node := t.root + var rules []*Rule + // check the host + if _, ok := node.children[u.Host]; !ok { + return rules + } + node = node.children[u.Host] + // remove the leading and trailing slash + trimmedPath := strings.Trim(u.Path, "/") + if len(trimmedPath) == 0 { + rules = node.rules + return rules + } else { + // check the paths + splitPaths := strings.Split(trimmedPath, "/") + for _, path := range splitPaths { + if _, ok := node.children[string(path)]; !ok { + break + } + node = node.children[string(path)] + } + rules = node.rules + return rules + } +} From 82f93e33e43726b99675f96e480e111cf8b159a5 Mon Sep 17 00:00:00 2001 From: David van der Spek Date: Mon, 10 Jul 2023 13:47:32 +0200 Subject: [PATCH 2/8] make prefix matching configurable Signed-off-by: David van der Spek --- driver/configuration/provider.go | 1 + rule/repository_memory.go | 23 +++++----- rule/trie.go | 78 +++++++++++--------------------- 3 files changed, 38 insertions(+), 64 deletions(-) diff --git a/driver/configuration/provider.go b/driver/configuration/provider.go index 2b66517b97..89f952db23 100644 --- a/driver/configuration/provider.go +++ b/driver/configuration/provider.go @@ -28,6 +28,7 @@ type MatchingStrategy string const ( Regexp MatchingStrategy = "regexp" Glob MatchingStrategy = "glob" + Prefix MatchingStrategy = "prefix" DefaultMatchingStrategy = Regexp ) diff --git a/rule/repository_memory.go b/rule/repository_memory.go index eff40890ff..e30e6ed81c 100644 --- a/rule/repository_memory.go +++ b/rule/repository_memory.go @@ -109,8 +109,13 @@ func (m *RepositoryMemory) Set(ctx context.Context, rules []Rule) error { m.invalidRules = append(m.invalidRules, check) } else { m.rules = append(m.rules, check) + if m.matchingStrategy == configuration.Prefix { + if err := m.trie.InsertRule(&check); err != nil { + m.r.Logger().WithError(err).WithField("rule_id", check.ID). + Errorf("A Prefix Rule could not be loaded into the trie so all requests will be sent to the closest matching prefix. You should resolve this issue now.") + } + } } - m.trie.InsertRule(&check) // TODO: this needs error handling } return nil @@ -126,19 +131,13 @@ func (m *RepositoryMemory) Match(ctx context.Context, method string, u *url.URL, var rules []*Rule - var rulesFromTrie []*Rule - if m.trie != nil { - rulesFromTrie = m.trie.Match(u) - - for _, r := range rulesFromTrie { - if matched, err := r.IsMatching(m.matchingStrategy, method, u, protocol); err != nil { - return nil, errors.WithStack(err) - } else if matched { - rules = append(rules, r) - } + if m.matchingStrategy == configuration.Prefix { + if m.trie.root == nil { + return nil, errors.WithStack(errors.New("prefix trie is nil")) + } else { + rules = m.trie.Match(u) } } else { - for k := range m.rules { r := &m.rules[k] if matched, err := r.IsMatching(m.matchingStrategy, method, u, protocol); err != nil { diff --git a/rule/trie.go b/rule/trie.go index 4c9a69477d..d0c2e36d0e 100644 --- a/rule/trie.go +++ b/rule/trie.go @@ -1,7 +1,9 @@ +// Copyright © 2023 Ory Corp +// SPDX-License-Identifier: Apache-2.0 + package rule import ( - "fmt" "net/url" "strings" ) @@ -27,64 +29,22 @@ func NewTrie() *Trie { } } -// Insert inserts a word into the trie -func (t *Trie) InsertString(word string) { - node := t.root - if _, ok := node.children[word]; !ok { - node.children[word] = &TrieNode{ - children: make(map[string]*TrieNode), - } - } - node = node.children[word] -} - -// Insert a url host and paths into the trie along with the rule -func (t *Trie) InsertURL(u *url.URL, r *Rule) { - // TODO: should this also handle scheme? - - node := t.root - // insert the host into the trie - if _, ok := node.children[u.Host]; !ok { - node.children[u.Host] = &TrieNode{ - children: make(map[string]*TrieNode), - } - } - node = node.children[u.Host] - - // remove the leading and trailing slash - trimmedPath := strings.Trim(u.Path, "/") - if len(trimmedPath) == 0 { - node.rules = append(node.rules, r) - } else { - // insert the paths into the trie - splitPaths := strings.Split(trimmedPath, "/") - i := 0 - for _, path := range splitPaths { - i++ - if _, ok := node.children[string(path)]; !ok { - node.children[string(path)] = &TrieNode{ - children: make(map[string]*TrieNode), - } - } - node = node.children[string(path)] - if i == len(splitPaths) { - node.rules = append(node.rules, r) - } - } - } -} - // Insert a url host and paths into the trie -func (t *Trie) InsertRule(r *Rule) { +func (t *Trie) InsertRule(r *Rule) error { node := t.root - // TODO: handle error properly matchURL, err := url.Parse(r.Match.GetURL()) if err != nil { - fmt.Println("error parsing url") + return err } - // TODO: should this also handle scheme? + // insert the scheme into the trie + if _, ok := node.children[matchURL.Scheme]; !ok { + node.children[matchURL.Scheme] = &TrieNode{ + children: make(map[string]*TrieNode), + } + } + node = node.children[matchURL.Scheme] // insert the host into the trie if _, ok := node.children[matchURL.Host]; !ok { @@ -117,12 +77,20 @@ func (t *Trie) InsertRule(r *Rule) { } } } + return nil } // return the longest prefix of the url that is in the trie func (t *Trie) LongestPrefix(u *url.URL) string { node := t.root var prefix string + // check the scheme + if _, ok := node.children[u.Scheme]; !ok { + return prefix + } + prefix += u.Scheme + node = node.children[u.Scheme] + // check the host if _, ok := node.children[u.Host]; !ok { return prefix @@ -151,6 +119,12 @@ func (t *Trie) LongestPrefix(u *url.URL) string { func (t *Trie) Match(u *url.URL) []*Rule { node := t.root var rules []*Rule + // check the scheme + if _, ok := node.children[u.Scheme]; !ok { + return rules + } + node = node.children[u.Scheme] + // check the host if _, ok := node.children[u.Host]; !ok { return rules From 47094fd3d154e0a535f245882ef363ea4cab099f Mon Sep 17 00:00:00 2001 From: David van der Spek Date: Mon, 10 Jul 2023 13:47:36 +0200 Subject: [PATCH 3/8] init add some unit tests for prefix matching Signed-off-by: David van der Spek --- rule/matcher_test.go | 73 +++++++++++++++++++++++++++++ rule/repository_memory.go | 14 ++++-- rule/trie.go | 98 +++++++++++++++++++++++---------------- 3 files changed, 141 insertions(+), 44 deletions(-) diff --git a/rule/matcher_test.go b/rule/matcher_test.go index 28ee2d051e..d35a2d0547 100644 --- a/rule/matcher_test.go +++ b/rule/matcher_test.go @@ -99,6 +99,45 @@ var testRulesGlob = []Rule{ }, } +var testRulesPrefix = []Rule{ + { + ID: "foo1", + Match: &Match{URL: "https://localhost:1234/foo", Methods: []string{"POST"}}, + Description: "Create users rule", + Authorizer: Handler{Handler: "allow", Config: []byte(`{"type":"any"}`)}, + Authenticators: []Handler{{Handler: "anonymous", Config: []byte(`{"name":"anonymous1"}`)}}, + Mutators: []Handler{{Handler: "id_token", Config: []byte(`{"issuer":"anything"}`)}}, + Upstream: Upstream{URL: "http://localhost:1235/", StripPath: "/bar", PreserveHost: true}, + }, + { + ID: "foo2", + Match: &Match{URL: "https://localhost:34/baz", Methods: []string{"GET"}}, + Description: "Get users rule", + Authorizer: Handler{Handler: "deny", Config: []byte(`{"type":"any"}`)}, + Authenticators: []Handler{{Handler: "oauth2_introspection", Config: []byte(`{"name":"anonymous1"}`)}}, + Mutators: []Handler{{Handler: "id_token", Config: []byte(`{"issuer":"anything"}`)}}, + Upstream: Upstream{URL: "http://localhost:333/", StripPath: "/foo", PreserveHost: false}, + }, + { + ID: "foo3", + Match: &Match{URL: "https://localhost:343/baz", Methods: []string{"GET"}}, + Description: "Get users rule", + Authorizer: Handler{Handler: "deny"}, + Authenticators: []Handler{{Handler: "oauth2_introspection"}}, + Mutators: []Handler{{Handler: "id_token"}}, + Upstream: Upstream{URL: "http://localhost:3333/", StripPath: "/foo", PreserveHost: false}, + }, + { + ID: "grpc1", + Match: &MatchGRPC{Authority: "baz.example.com", FullMethod: "grpc.api/Call"}, + Description: "gRPC Rule", + Authorizer: Handler{Handler: "allow", Config: []byte(`{"type":"any"}`)}, + Authenticators: []Handler{{Handler: "anonymous", Config: []byte(`{"name":"anonymous1"}`)}}, + Mutators: []Handler{{Handler: "id_token", Config: []byte(`{"issuer":"anything"}`)}}, + Upstream: Upstream{URL: "http://bar.example.com/", PreserveHost: false}, + }, +} + func TestMatcher(t *testing.T) { type m interface { Matcher @@ -192,5 +231,39 @@ func TestMatcher(t *testing.T) { testMatcher(t, matcher, "DELETE", "https://localhost:1234/foo", ProtocolHTTP, true, nil) }) }) + t.Run(fmt.Sprintf("prefix matcher=%s", name), func(t *testing.T) { + require.NoError(t, matcher.SetMatchingStrategy(context.Background(), configuration.Prefix)) + require.NoError(t, matcher.Set(context.Background(), []Rule{})) + t.Run("case=empty", func(t *testing.T) { + testMatcher(t, matcher, "GET", "https://localhost:34/baz", ProtocolHTTP, true, nil) + testMatcher(t, matcher, "POST", "https://localhost:1234/foo", ProtocolHTTP, true, nil) + testMatcher(t, matcher, "DELETE", "https://localhost:1234/foo", ProtocolHTTP, true, nil) + }) + + require.NoError(t, matcher.Set(context.Background(), testRulesPrefix)) + + t.Run("case=created", func(t *testing.T) { + testMatcher(t, matcher, "POST", "https://localhost:1234/foo", ProtocolHTTP, false, &testRulesPrefix[0]) + testMatcher(t, matcher, "GET", "https://localhost:34/baz", ProtocolHTTP, false, &testRulesPrefix[1]) + testMatcher(t, matcher, "DELETE", "https://localhost:1234/foo", ProtocolHTTP, true, nil) + testMatcher(t, matcher, "POST", "grpc://bar.example.com/grpc.api/Call", ProtocolGRPC, false, &testRulesPrefix[3]) + }) + + t.Run("case=cache", func(t *testing.T) { + r, err := matcher.Match(context.Background(), "GET", mustParseURL(t, "https://localhost:34/baz"), ProtocolHTTP) + require.NoError(t, err) + _, err = matcher.Get(context.Background(), r.ID) + require.NoError(t, err) + // assert.NotEmpty(t, got.matchingEngine.Checksum()) + }) + + require.NoError(t, matcher.Set(context.Background(), testRulesPrefix[1:])) + + t.Run("case=updated", func(t *testing.T) { + testMatcher(t, matcher, "GET", "https://localhost:34/baz", ProtocolHTTP, false, &testRulesPrefix[1]) + testMatcher(t, matcher, "POST", "https://localhost:1234/foo", ProtocolHTTP, true, nil) + testMatcher(t, matcher, "DELETE", "https://localhost:1234/foo", ProtocolHTTP, true, nil) + }) + }) } } diff --git a/rule/repository_memory.go b/rule/repository_memory.go index e30e6ed81c..091230d844 100644 --- a/rule/repository_memory.go +++ b/rule/repository_memory.go @@ -62,7 +62,7 @@ func (m *RepositoryMemory) WithRules(rules []Rule) { m.Lock() m.rules = rules for _, rule := range rules { - m.trie.InsertRule(&rule) + m.trie.InsertRule(rule) } m.Unlock() } @@ -102,6 +102,11 @@ func (m *RepositoryMemory) Set(ctx context.Context, rules []Rule) error { m.rules = make([]Rule, 0, len(rules)) m.invalidRules = make([]Rule, 0) + // Reset the trie if we are using prefix matching and the rules have changed. + if m.matchingStrategy == configuration.Prefix { + m.trie = NewTrie() + } + for _, check := range rules { if err := m.r.RuleValidator().Validate(&check); err != nil { m.r.Logger().WithError(err).WithField("rule_id", check.ID). @@ -110,7 +115,7 @@ func (m *RepositoryMemory) Set(ctx context.Context, rules []Rule) error { } else { m.rules = append(m.rules, check) if m.matchingStrategy == configuration.Prefix { - if err := m.trie.InsertRule(&check); err != nil { + if err := m.trie.InsertRule(check); err != nil { m.r.Logger().WithError(err).WithField("rule_id", check.ID). Errorf("A Prefix Rule could not be loaded into the trie so all requests will be sent to the closest matching prefix. You should resolve this issue now.") } @@ -135,7 +140,10 @@ func (m *RepositoryMemory) Match(ctx context.Context, method string, u *url.URL, if m.trie.root == nil { return nil, errors.WithStack(errors.New("prefix trie is nil")) } else { - rules = m.trie.Match(u) + matchedRules := m.trie.Match(method, u) + for _, r := range matchedRules { + rules = append(rules, &r) + } } } else { for k := range m.rules { diff --git a/rule/trie.go b/rule/trie.go index d0c2e36d0e..dfcc59df73 100644 --- a/rule/trie.go +++ b/rule/trie.go @@ -11,7 +11,7 @@ import ( // types for a trie root node type TrieNode struct { children map[string]*TrieNode - rules []*Rule + rules []Rule // isWord bool } @@ -30,7 +30,7 @@ func NewTrie() *Trie { } // Insert a url host and paths into the trie -func (t *Trie) InsertRule(r *Rule) error { +func (t *Trie) InsertRule(r Rule) error { node := t.root matchURL, err := url.Parse(r.Match.GetURL()) @@ -38,42 +38,54 @@ func (t *Trie) InsertRule(r *Rule) error { return err } - // insert the scheme into the trie - if _, ok := node.children[matchURL.Scheme]; !ok { - node.children[matchURL.Scheme] = &TrieNode{ - children: make(map[string]*TrieNode), + // insert the methods into the trie + for _, method := range r.Match.GetMethods() { + // reset the node to the root + node = t.root + if _, ok := node.children[method]; !ok { + node.children[method] = &TrieNode{ + children: make(map[string]*TrieNode), + } } - } - node = node.children[matchURL.Scheme] + node = node.children[method] - // insert the host into the trie - if _, ok := node.children[matchURL.Host]; !ok { - node.children[matchURL.Host] = &TrieNode{ - children: make(map[string]*TrieNode), + // insert the scheme into the trie + if _, ok := node.children[matchURL.Scheme]; !ok { + node.children[matchURL.Scheme] = &TrieNode{ + children: make(map[string]*TrieNode), + } } - } - node = node.children[matchURL.Host] - // remove the leading and trailing slash - trimmedPath := strings.Trim(matchURL.Path, "/") + node = node.children[matchURL.Scheme] - if len(trimmedPath) == 0 { - node.rules = append(node.rules, r) - } else { - - // insert the paths into the trie - splitPaths := strings.Split(trimmedPath, "/") - i := 0 - for _, path := range splitPaths { - i++ - if _, ok := node.children[string(path)]; !ok { - node.children[string(path)] = &TrieNode{ - children: make(map[string]*TrieNode), - } + // insert the host into the trie + if _, ok := node.children[matchURL.Host]; !ok { + node.children[matchURL.Host] = &TrieNode{ + children: make(map[string]*TrieNode), } - node = node.children[string(path)] - // if this is the last path, append the rule - if i == len(splitPaths) { - node.rules = append(node.rules, r) + } + node = node.children[matchURL.Host] + // remove the leading and trailing slash + trimmedPath := strings.Trim(matchURL.Path, "/") + + if len(trimmedPath) == 0 { + node.rules = append(node.rules, r) + } else { + + // insert the paths into the trie + splitPaths := strings.Split(trimmedPath, "/") + i := 0 + for _, path := range splitPaths { + i++ + if _, ok := node.children[string(path)]; !ok { + node.children[string(path)] = &TrieNode{ + children: make(map[string]*TrieNode), + } + } + node = node.children[string(path)] + // if this is the last path, append the rule + if i == len(splitPaths) { + node.rules = append(node.rules, r) + } } } } @@ -116,25 +128,30 @@ func (t *Trie) LongestPrefix(u *url.URL) string { } // return the rules of the longest prefix of the url that is in the trie -func (t *Trie) Match(u *url.URL) []*Rule { +func (t *Trie) Match(method string, u *url.URL) []Rule { node := t.root - var rules []*Rule + + // check the method + if _, ok := node.children[method]; !ok { + return nil + } + node = node.children[method] + // check the scheme if _, ok := node.children[u.Scheme]; !ok { - return rules + return nil } node = node.children[u.Scheme] // check the host if _, ok := node.children[u.Host]; !ok { - return rules + return nil } node = node.children[u.Host] // remove the leading and trailing slash trimmedPath := strings.Trim(u.Path, "/") if len(trimmedPath) == 0 { - rules = node.rules - return rules + return node.rules } else { // check the paths splitPaths := strings.Split(trimmedPath, "/") @@ -144,7 +161,6 @@ func (t *Trie) Match(u *url.URL) []*Rule { } node = node.children[string(path)] } - rules = node.rules - return rules + return node.rules } } From df3363fdca42c7473428b07dbf99318c13dbd9d6 Mon Sep 17 00:00:00 2001 From: David van der Spek Date: Mon, 10 Jul 2023 13:47:40 +0200 Subject: [PATCH 4/8] fix unit test Signed-off-by: David van der Spek --- rule/matcher_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rule/matcher_test.go b/rule/matcher_test.go index d35a2d0547..b58af91207 100644 --- a/rule/matcher_test.go +++ b/rule/matcher_test.go @@ -129,7 +129,7 @@ var testRulesPrefix = []Rule{ }, { ID: "grpc1", - Match: &MatchGRPC{Authority: "baz.example.com", FullMethod: "grpc.api/Call"}, + Match: &MatchGRPC{Authority: "bar.example.com", FullMethod: "grpc.api/Call"}, Description: "gRPC Rule", Authorizer: Handler{Handler: "allow", Config: []byte(`{"type":"any"}`)}, Authenticators: []Handler{{Handler: "anonymous", Config: []byte(`{"name":"anonymous1"}`)}}, From cb81e5e1b98d200dc811fd0dee001b3e9b3927bb Mon Sep 17 00:00:00 2001 From: David van der Spek Date: Mon, 10 Jul 2023 13:47:45 +0200 Subject: [PATCH 5/8] add protocol to trie Signed-off-by: David van der Spek --- rule/matcher.go | 6 +++--- rule/repository_memory.go | 2 +- rule/trie.go | 16 +++++++++++++++- 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/rule/matcher.go b/rule/matcher.go index 69d7b2e75e..5eca6619d6 100644 --- a/rule/matcher.go +++ b/rule/matcher.go @@ -9,7 +9,7 @@ import ( ) type ( - Protocol int + Protocol string Matcher interface { Match(ctx context.Context, method string, u *url.URL, protocol Protocol) (*Rule, error) @@ -17,6 +17,6 @@ type ( ) const ( - ProtocolHTTP Protocol = iota - ProtocolGRPC + ProtocolHTTP Protocol = "http" + ProtocolGRPC Protocol = "grpc" ) diff --git a/rule/repository_memory.go b/rule/repository_memory.go index 091230d844..c9dd8dbc7f 100644 --- a/rule/repository_memory.go +++ b/rule/repository_memory.go @@ -140,7 +140,7 @@ func (m *RepositoryMemory) Match(ctx context.Context, method string, u *url.URL, if m.trie.root == nil { return nil, errors.WithStack(errors.New("prefix trie is nil")) } else { - matchedRules := m.trie.Match(method, u) + matchedRules := m.trie.Match(method, u, protocol) for _, r := range matchedRules { rules = append(rules, &r) } diff --git a/rule/trie.go b/rule/trie.go index dfcc59df73..cd8fe4f47f 100644 --- a/rule/trie.go +++ b/rule/trie.go @@ -38,6 +38,14 @@ func (t *Trie) InsertRule(r Rule) error { return err } + // insert the protocol into the trie + if _, ok := node.children[string(r.Match.Protocol())]; !ok { + node.children[string(r.Match.Protocol())] = &TrieNode{ + children: make(map[string]*TrieNode), + } + } + node = node.children[string(r.Match.Protocol())] + // insert the methods into the trie for _, method := range r.Match.GetMethods() { // reset the node to the root @@ -128,9 +136,15 @@ func (t *Trie) LongestPrefix(u *url.URL) string { } // return the rules of the longest prefix of the url that is in the trie -func (t *Trie) Match(method string, u *url.URL) []Rule { +func (t *Trie) Match(method string, u *url.URL, protocol Protocol) []Rule { node := t.root + // check the protocol + if _, ok := node.children[string(protocol)]; !ok { + return nil + } + node = node.children[string(protocol)] + // check the method if _, ok := node.children[method]; !ok { return nil From 12d52e572d9f07e82ddbe847ad4ca9e599d64d27 Mon Sep 17 00:00:00 2001 From: David van der Spek Date: Mon, 10 Jul 2023 13:47:49 +0200 Subject: [PATCH 6/8] fix tests Signed-off-by: David van der Spek --- rule/trie.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rule/trie.go b/rule/trie.go index cd8fe4f47f..eaf76bc95e 100644 --- a/rule/trie.go +++ b/rule/trie.go @@ -48,8 +48,9 @@ func (t *Trie) InsertRule(r Rule) error { // insert the methods into the trie for _, method := range r.Match.GetMethods() { - // reset the node to the root + // reset the node to the root, followed by the protocol node = t.root + node = node.children[string(r.Match.Protocol())] if _, ok := node.children[method]; !ok { node.children[method] = &TrieNode{ children: make(map[string]*TrieNode), From 00eb5ea583b378f2861e90c2d0dff78fb15d5a71 Mon Sep 17 00:00:00 2001 From: David van der Spek Date: Mon, 10 Jul 2023 13:47:53 +0200 Subject: [PATCH 7/8] add separate config + fallback to pattern matching Signed-off-by: David van der Spek --- driver/configuration/config_keys.go | 1 + driver/configuration/provider.go | 2 +- driver/configuration/provider_koanf.go | 5 +++ internal/config/.oathkeeper.yaml | 2 ++ rule/matcher_test.go | 42 ++++++++++++++++++++------ rule/repository.go | 2 ++ rule/repository_memory.go | 33 +++++++++++++++++--- rule/trie.go | 16 ++++++++-- 8 files changed, 86 insertions(+), 17 deletions(-) diff --git a/driver/configuration/config_keys.go b/driver/configuration/config_keys.go index 6fac519d5c..3a1b532c0c 100644 --- a/driver/configuration/config_keys.go +++ b/driver/configuration/config_keys.go @@ -24,6 +24,7 @@ const ( PrometheusServeCollapseRequestPaths Key = "serve.prometheus.collapse_request_paths" AccessRuleRepositories Key = "access_rules.repositories" AccessRuleMatchingStrategy Key = "access_rules.matching_strategy" + AcccessRulePrefixMatchingEnabled Key = "access_rules.prefix_matching_enabled" ) // Authorizers diff --git a/driver/configuration/provider.go b/driver/configuration/provider.go index 89f952db23..68f221bfa5 100644 --- a/driver/configuration/provider.go +++ b/driver/configuration/provider.go @@ -28,7 +28,6 @@ type MatchingStrategy string const ( Regexp MatchingStrategy = "regexp" Glob MatchingStrategy = "glob" - Prefix MatchingStrategy = "prefix" DefaultMatchingStrategy = Regexp ) @@ -59,6 +58,7 @@ type Provider interface { AccessRuleRepositories() []url.URL AccessRuleMatchingStrategy() MatchingStrategy + AcccessRulePrefixMatchingEnabled() bool ProxyServeAddress() string APIServeAddress() string diff --git a/driver/configuration/provider_koanf.go b/driver/configuration/provider_koanf.go index 2589b3a1ca..23674f267d 100644 --- a/driver/configuration/provider_koanf.go +++ b/driver/configuration/provider_koanf.go @@ -172,6 +172,11 @@ func (v *KoanfProvider) AccessRuleMatchingStrategy() MatchingStrategy { return MatchingStrategy(v.source.String(AccessRuleMatchingStrategy)) } +// AcccessRulePrefixMatching returns if prefix matching should be used. +func (v *KoanfProvider) AcccessRulePrefixMatchingEnabled() bool { + return v.source.Bool(AcccessRulePrefixMatchingEnabled) +} + func (v *KoanfProvider) CORSEnabled(iface string) bool { _, enabled := v.CORS(iface) return enabled diff --git a/internal/config/.oathkeeper.yaml b/internal/config/.oathkeeper.yaml index 39a02cb7fc..88426cb6a7 100644 --- a/internal/config/.oathkeeper.yaml +++ b/internal/config/.oathkeeper.yaml @@ -103,6 +103,8 @@ access_rules: - https://path-to-my-rules/rules.json # Optional fields describing matching strategy, defaults to "regexp". matching_strategy: glob + # Optional fields describing if rules should be matched using path prefixes, defaults to false. + prefix_matching_enabled: false errors: fallback: diff --git a/rule/matcher_test.go b/rule/matcher_test.go index b58af91207..2a1561dd8d 100644 --- a/rule/matcher_test.go +++ b/rule/matcher_test.go @@ -102,7 +102,7 @@ var testRulesGlob = []Rule{ var testRulesPrefix = []Rule{ { ID: "foo1", - Match: &Match{URL: "https://localhost:1234/foo", Methods: []string{"POST"}}, + Match: &Match{URL: "https://localhost:1234/", Methods: []string{"POST"}}, Description: "Create users rule", Authorizer: Handler{Handler: "allow", Config: []byte(`{"type":"any"}`)}, Authenticators: []Handler{{Handler: "anonymous", Config: []byte(`{"name":"anonymous1"}`)}}, @@ -111,7 +111,25 @@ var testRulesPrefix = []Rule{ }, { ID: "foo2", - Match: &Match{URL: "https://localhost:34/baz", Methods: []string{"GET"}}, + Match: &Match{URL: "https://localhost:1234/foo/", Methods: []string{"POST"}}, + Description: "Create users rule", + Authorizer: Handler{Handler: "allow", Config: []byte(`{"type":"any"}`)}, + Authenticators: []Handler{{Handler: "anonymous", Config: []byte(`{"name":"anonymous1"}`)}}, + Mutators: []Handler{{Handler: "id_token", Config: []byte(`{"issuer":"anything"}`)}}, + Upstream: Upstream{URL: "http://localhost:1235/", StripPath: "/bar", PreserveHost: true}, + }, + { + ID: "foo3", + Match: &Match{URL: "https://localhost:1234/foo/something/", Methods: []string{"POST"}}, + Description: "Create users rule", + Authorizer: Handler{Handler: "allow", Config: []byte(`{"type":"any"}`)}, + Authenticators: []Handler{{Handler: "anonymous", Config: []byte(`{"name":"anonymous1"}`)}}, + Mutators: []Handler{{Handler: "id_token", Config: []byte(`{"issuer":"anything"}`)}}, + Upstream: Upstream{URL: "http://localhost:1235/", StripPath: "/bar", PreserveHost: true}, + }, + { + ID: "foo4", + Match: &Match{URL: "https://localhost:34/", Methods: []string{"GET"}}, Description: "Get users rule", Authorizer: Handler{Handler: "deny", Config: []byte(`{"type":"any"}`)}, Authenticators: []Handler{{Handler: "oauth2_introspection", Config: []byte(`{"name":"anonymous1"}`)}}, @@ -119,8 +137,8 @@ var testRulesPrefix = []Rule{ Upstream: Upstream{URL: "http://localhost:333/", StripPath: "/foo", PreserveHost: false}, }, { - ID: "foo3", - Match: &Match{URL: "https://localhost:343/baz", Methods: []string{"GET"}}, + ID: "foo5", + Match: &Match{URL: "https://localhost:343/", Methods: []string{"GET"}}, Description: "Get users rule", Authorizer: Handler{Handler: "deny"}, Authenticators: []Handler{{Handler: "oauth2_introspection"}}, @@ -232,7 +250,8 @@ func TestMatcher(t *testing.T) { }) }) t.Run(fmt.Sprintf("prefix matcher=%s", name), func(t *testing.T) { - require.NoError(t, matcher.SetMatchingStrategy(context.Background(), configuration.Prefix)) + require.NoError(t, matcher.SetMatchingStrategy(context.Background(), configuration.Regexp)) + require.NoError(t, matcher.SetPrefixMatching(context.Background(), true)) require.NoError(t, matcher.Set(context.Background(), []Rule{})) t.Run("case=empty", func(t *testing.T) { testMatcher(t, matcher, "GET", "https://localhost:34/baz", ProtocolHTTP, true, nil) @@ -243,10 +262,13 @@ func TestMatcher(t *testing.T) { require.NoError(t, matcher.Set(context.Background(), testRulesPrefix)) t.Run("case=created", func(t *testing.T) { - testMatcher(t, matcher, "POST", "https://localhost:1234/foo", ProtocolHTTP, false, &testRulesPrefix[0]) - testMatcher(t, matcher, "GET", "https://localhost:34/baz", ProtocolHTTP, false, &testRulesPrefix[1]) + testMatcher(t, matcher, "POST", "https://localhost:1234/", ProtocolHTTP, false, &testRulesPrefix[0]) + testMatcher(t, matcher, "POST", "https://localhost:1234/foo", ProtocolHTTP, false, &testRulesPrefix[1]) + testMatcher(t, matcher, "POST", "https://localhost:1234/foo/something/very/long", ProtocolHTTP, false, &testRulesPrefix[2]) + testMatcher(t, matcher, "POST", "https://localhost:1234/foo/baz/something/very/long", ProtocolHTTP, false, &testRulesPrefix[1]) + testMatcher(t, matcher, "GET", "https://localhost:34/baz", ProtocolHTTP, false, &testRulesPrefix[3]) testMatcher(t, matcher, "DELETE", "https://localhost:1234/foo", ProtocolHTTP, true, nil) - testMatcher(t, matcher, "POST", "grpc://bar.example.com/grpc.api/Call", ProtocolGRPC, false, &testRulesPrefix[3]) + testMatcher(t, matcher, "POST", "grpc://bar.example.com/grpc.api/Call", ProtocolGRPC, false, &testRulesPrefix[5]) }) t.Run("case=cache", func(t *testing.T) { @@ -257,10 +279,10 @@ func TestMatcher(t *testing.T) { // assert.NotEmpty(t, got.matchingEngine.Checksum()) }) - require.NoError(t, matcher.Set(context.Background(), testRulesPrefix[1:])) + require.NoError(t, matcher.Set(context.Background(), testRulesPrefix[3:])) t.Run("case=updated", func(t *testing.T) { - testMatcher(t, matcher, "GET", "https://localhost:34/baz", ProtocolHTTP, false, &testRulesPrefix[1]) + testMatcher(t, matcher, "GET", "https://localhost:34/baz", ProtocolHTTP, false, &testRulesPrefix[3]) testMatcher(t, matcher, "POST", "https://localhost:1234/foo", ProtocolHTTP, true, nil) testMatcher(t, matcher, "DELETE", "https://localhost:1234/foo", ProtocolHTTP, true, nil) }) diff --git a/rule/repository.go b/rule/repository.go index de1462399f..9037ac9c50 100644 --- a/rule/repository.go +++ b/rule/repository.go @@ -17,5 +17,7 @@ type Repository interface { Count(context.Context) (int, error) MatchingStrategy(context.Context) (configuration.MatchingStrategy, error) SetMatchingStrategy(context.Context, configuration.MatchingStrategy) error + PrefixMatching(context.Context) (bool, error) + SetPrefixMatching(context.Context, bool) error ReadyChecker(*http.Request) error } diff --git a/rule/repository_memory.go b/rule/repository_memory.go index c9dd8dbc7f..a529a6bba2 100644 --- a/rule/repository_memory.go +++ b/rule/repository_memory.go @@ -30,6 +30,7 @@ type RepositoryMemory struct { rules []Rule invalidRules []Rule matchingStrategy configuration.MatchingStrategy + prefixMatching bool r repositoryMemoryRegistry trie *Trie } @@ -49,6 +50,21 @@ func (m *RepositoryMemory) SetMatchingStrategy(_ context.Context, ms configurati return nil } +// PrefixMatching returns current PrefixMatching. +func (m *RepositoryMemory) PrefixMatching(_ context.Context) (bool, error) { + m.RLock() + defer m.RUnlock() + return m.prefixMatching, nil +} + +// SetPrefixMatching updates PrefixMatching. +func (m *RepositoryMemory) SetPrefixMatching(_ context.Context, enabled bool) error { + m.Lock() + defer m.Unlock() + m.prefixMatching = enabled + return nil +} + func NewRepositoryMemory(r repositoryMemoryRegistry) *RepositoryMemory { return &RepositoryMemory{ r: r, @@ -103,7 +119,7 @@ func (m *RepositoryMemory) Set(ctx context.Context, rules []Rule) error { m.invalidRules = make([]Rule, 0) // Reset the trie if we are using prefix matching and the rules have changed. - if m.matchingStrategy == configuration.Prefix { + if m.prefixMatching { m.trie = NewTrie() } @@ -114,7 +130,7 @@ func (m *RepositoryMemory) Set(ctx context.Context, rules []Rule) error { m.invalidRules = append(m.invalidRules, check) } else { m.rules = append(m.rules, check) - if m.matchingStrategy == configuration.Prefix { + if m.prefixMatching { if err := m.trie.InsertRule(check); err != nil { m.r.Logger().WithError(err).WithField("rule_id", check.ID). Errorf("A Prefix Rule could not be loaded into the trie so all requests will be sent to the closest matching prefix. You should resolve this issue now.") @@ -136,13 +152,22 @@ func (m *RepositoryMemory) Match(ctx context.Context, method string, u *url.URL, var rules []*Rule - if m.matchingStrategy == configuration.Prefix { + if m.prefixMatching { if m.trie.root == nil { return nil, errors.WithStack(errors.New("prefix trie is nil")) } else { matchedRules := m.trie.Match(method, u, protocol) for _, r := range matchedRules { - rules = append(rules, &r) + // if there are multiple rules that match, we will procede to filter them using the matching strategy + if len(matchedRules) > 1 { + if matched, err := r.IsMatching(m.matchingStrategy, method, u, protocol); err != nil { + return nil, errors.WithStack(err) + } else if matched { + rules = append(rules, &r) + } + } else { + rules = append(rules, &r) + } } } } else { diff --git a/rule/trie.go b/rule/trie.go index eaf76bc95e..3e1f2fc408 100644 --- a/rule/trie.go +++ b/rule/trie.go @@ -6,6 +6,8 @@ package rule import ( "net/url" "strings" + + "github.com/dlclark/regexp2" ) // types for a trie root node @@ -17,7 +19,8 @@ type TrieNode struct { // types for a trie node type Trie struct { - root *TrieNode + root *TrieNode + pathCleaner *regexp2.Regexp } // NewTrie creates a new trie @@ -26,6 +29,8 @@ func NewTrie() *Trie { root: &TrieNode{ children: make(map[string]*TrieNode), }, + // if the path contains a regex, we don't need to insert it or anything after into the trie + pathCleaner: regexp2.MustCompile(`<.*>.*`, 0), } } @@ -73,8 +78,15 @@ func (t *Trie) InsertRule(r Rule) error { } } node = node.children[matchURL.Host] + + // remove any regex from the path + cleanPath, err := t.pathCleaner.Replace(matchURL.Path, "", 0, -1) + if err != nil { + return err + } + // remove the leading and trailing slash - trimmedPath := strings.Trim(matchURL.Path, "/") + trimmedPath := strings.Trim(cleanPath, "/") if len(trimmedPath) == 0 { node.rules = append(node.rules, r) From 5542209c4d97d96f444e9e2b9d08f2a8f170ba86 Mon Sep 17 00:00:00 2001 From: David van der Spek Date: Mon, 10 Jul 2023 13:47:57 +0200 Subject: [PATCH 8/8] add prefix_matching_enabled to config schema Signed-off-by: David van der Spek --- spec/config.schema.json | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/spec/config.schema.json b/spec/config.schema.json index 3a90a77a6b..45d84325ce 100644 --- a/spec/config.schema.json +++ b/spec/config.schema.json @@ -1267,6 +1267,12 @@ "default": "regexp", "enum": ["glob", "regexp"], "examples": ["glob"] + }, + "prefix_matching_enabled": { + "title": "Enable prefix matching", + "description": "This an optional field describing if rules should be matched using path prefixes, defaults to false.", + "type": "boolean", + "default": false } } },