Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add support for header in rule matching #967

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

feat: Add support for header in rule matching #967

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
4b4144e
feat: Add support for header in rule matching
m-guesnon-pvotal May 18, 2022
14eab8b
Force ory-prettier-styles version to 1.2.0 to match CI
m-guesnon-pvotal May 18, 2022
a88a7c5
Trigger rebuild
m-guesnon-pvotal May 18, 2022
ca4d80f
Revert formatting
m-guesnon-pvotal May 31, 2022
29138bb
Merge branch 'master' into feat/matching-rule-header-support
m-guesnon-pvotal May 31, 2022
c56898a
Merge branch 'master' into feat/matching-rule-header-support
m-guesnon-pvotal Jul 12, 2022
d52a51b
Merge branch 'master' into feat/matching-rule-header-support
m-guesnon-pvotal May 3, 2023
8c51cd5
fix: Unit tests. Run format
m-guesnon-pvotal May 3, 2023
2290ce1
fix: Rule wording. Full header match for consistency
m-guesnon-pvotal May 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion api/decision.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func (h *DecisionHandler) decisions(w http.ResponseWriter, r *http.Request) {
fields["subject"] = sess.Subject
}

rl, err := h.r.RuleMatcher().Match(r.Context(), r.Method, r.URL)
rl, err := h.r.RuleMatcher().Match(r.Context(), r.Method, r.URL, r.Header)
if err != nil {
h.r.Logger().WithError(err).
WithFields(fields).
Expand Down
2 changes: 1 addition & 1 deletion api/decision_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ func (*decisionHandlerRegistryMock) Logger() *logrusx.Logger {
return logrusx.New("", "")
}

func (m *decisionHandlerRegistryMock) Match(ctx context.Context, method string, u *url.URL) (*rule.Rule, error) {
func (m *decisionHandlerRegistryMock) Match(ctx context.Context, method string, u *url.URL, header http.Header) (*rule.Rule, error) {
args := m.Called(ctx, method, u)
return args.Get(0).(*rule.Rule), args.Error(1)
}
Expand Down
2 changes: 1 addition & 1 deletion proxy/proxy.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ func (d *Proxy) RoundTrip(r *http.Request) (*http.Response, error) {

func (d *Proxy) Director(r *http.Request) {
EnrichRequestedURL(r)
rl, err := d.r.RuleMatcher().Match(r.Context(), r.Method, r.URL)
rl, err := d.r.RuleMatcher().Match(r.Context(), r.Method, r.URL, r.Header)
if err != nil {
*r = *r.WithContext(context.WithValue(r.Context(), director, err))
return
Expand Down
3 changes: 2 additions & 1 deletion rule/matcher.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@ package rule

import (
"context"
"net/http"
"net/url"
)

type Matcher interface {
Match(ctx context.Context, method string, u *url.URL) (*Rule, error)
Match(ctx context.Context, method string, u *url.URL, headers http.Header) (*Rule, error)
}
69 changes: 46 additions & 23 deletions rule/matcher_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ package rule
import (
"context"
"fmt"
"net/http"
"net/url"
"testing"

Expand Down Expand Up @@ -66,6 +67,15 @@ var testRules = []Rule{
Mutators: []Handler{{Handler: "id_token"}},
Upstream: Upstream{URL: "http://localhost:3333/", StripPath: "/foo", PreserveHost: false},
},
{
ID: "foo4",
Match: &Match{URL: "https://localhost:343/<baz|bar>", Methods: []string{"PATCH"}, Headers: map[string]string{"Content-Type": "application/some-app.v2+json"}},
Description: "Patch users rule for version 2",
Authorizer: Handler{Handler: "deny"},
Authenticators: []Handler{{Handler: "oauth2_introspection"}},
Mutators: []Handler{{Handler: "id_token"}},
Upstream: Upstream{URL: "http://localhost:3333/", StripPath: "/foo", PreserveHost: false},
},
}

var testRulesGlob = []Rule{
Expand Down Expand Up @@ -96,6 +106,15 @@ var testRulesGlob = []Rule{
Mutators: []Handler{{Handler: "id_token"}},
Upstream: Upstream{URL: "http://localhost:3333/", StripPath: "/foo", PreserveHost: false},
},
{
ID: "foo4",
Match: &Match{URL: "https://localhost:343/<{baz*,bar*}>", Methods: []string{"PATCH"}, Headers: map[string]string{"Content-Type": "application/some-app.v2+json"}},
Description: "Patch users rule with version 2",
Authorizer: Handler{Handler: "deny"},
Authenticators: []Handler{{Handler: "oauth2_introspection"}},
Mutators: []Handler{{Handler: "id_token"}},
Upstream: Upstream{URL: "http://localhost:3333/", StripPath: "/foo", PreserveHost: false},
},
}

func TestMatcher(t *testing.T) {
Expand All @@ -104,8 +123,8 @@ func TestMatcher(t *testing.T) {
Repository
}

var testMatcher = func(t *testing.T, matcher Matcher, method string, url string, expectErr bool, expect *Rule) {
r, err := matcher.Match(context.Background(), method, mustParseURL(t, url))
var testMatcher = func(t *testing.T, matcher Matcher, method string, url string, headers http.Header, expectErr bool, expect *Rule) {
r, err := matcher.Match(context.Background(), method, mustParseURL(t, url), headers)
if expectErr {
require.Error(t, err)
} else {
Expand All @@ -119,59 +138,61 @@ func TestMatcher(t *testing.T) {
} {
t.Run(fmt.Sprintf("regexp matcher=%s", name), func(t *testing.T) {
t.Run("case=empty", func(t *testing.T) {
testMatcher(t, matcher, "GET", "https://localhost:34/baz", true, nil)
testMatcher(t, matcher, "POST", "https://localhost:1234/foo", true, nil)
testMatcher(t, matcher, "DELETE", "https://localhost:1234/foo", true, nil)
testMatcher(t, matcher, "GET", "https://localhost:34/baz", http.Header{}, true, nil)
testMatcher(t, matcher, "POST", "https://localhost:1234/foo", http.Header{}, true, nil)
testMatcher(t, matcher, "DELETE", "https://localhost:1234/foo", http.Header{}, true, nil)
})

require.NoError(t, matcher.Set(context.Background(), testRules))

t.Run("case=created", func(t *testing.T) {
testMatcher(t, matcher, "GET", "https://localhost:34/baz", false, &testRules[1])
testMatcher(t, matcher, "POST", "https://localhost:1234/foo", false, &testRules[0])
testMatcher(t, matcher, "DELETE", "https://localhost:1234/foo", true, nil)
testMatcher(t, matcher, "GET", "https://localhost:34/baz", http.Header{}, false, &testRules[1])
testMatcher(t, matcher, "POST", "https://localhost:1234/foo", http.Header{}, false, &testRules[0])
testMatcher(t, matcher, "DELETE", "https://localhost:1234/foo", http.Header{}, true, nil)
})

t.Run("case=cache", func(t *testing.T) {
r, err := matcher.Match(context.Background(), "GET", mustParseURL(t, "https://localhost:34/baz"))
r, err := matcher.Match(context.Background(), "GET", mustParseURL(t, "https://localhost:34/baz"), http.Header{})
require.NoError(t, err)
got, err := matcher.Get(context.Background(), r.ID)
require.NoError(t, err)
assert.NotEmpty(t, got.matchingEngine.Checksum())
})

t.Run("case=nil url", func(t *testing.T) {
_, err := matcher.Match(context.Background(), "GET", nil)
_, err := matcher.Match(context.Background(), "GET", nil, http.Header{})
require.Error(t, err)
})

require.NoError(t, matcher.Set(context.Background(), testRules[1:]))

t.Run("case=updated", func(t *testing.T) {
testMatcher(t, matcher, "GET", "https://localhost:34/baz", false, &testRules[1])
testMatcher(t, matcher, "POST", "https://localhost:1234/foo", true, nil)
testMatcher(t, matcher, "DELETE", "https://localhost:1234/foo", true, nil)
testMatcher(t, matcher, "GET", "https://localhost:34/baz", http.Header{}, false, &testRules[1])
testMatcher(t, matcher, "POST", "https://localhost:1234/foo", http.Header{}, true, nil)
testMatcher(t, matcher, "DELETE", "https://localhost:1234/foo", http.Header{}, true, nil)
testMatcher(t, matcher, "PATCH", "https://localhost:343/bar", http.Header{"Content-Type": []string{"application/some-app.v1+json"}}, true, nil)
testMatcher(t, matcher, "PATCH", "https://localhost:343/bar", http.Header{"Content-Type": []string{"application/some-app.v2+json"}}, false, &testRules[3])
})
})
t.Run(fmt.Sprintf("glob matcher=%s", name), func(t *testing.T) {
require.NoError(t, matcher.SetMatchingStrategy(context.Background(), configuration.Glob))
require.NoError(t, matcher.Set(context.Background(), []Rule{}))
t.Run("case=empty", func(t *testing.T) {
testMatcher(t, matcher, "GET", "https://localhost:34/baz", true, nil)
testMatcher(t, matcher, "POST", "https://localhost:1234/foo", true, nil)
testMatcher(t, matcher, "DELETE", "https://localhost:1234/foo", true, nil)
testMatcher(t, matcher, "GET", "https://localhost:34/baz", http.Header{}, true, nil)
testMatcher(t, matcher, "POST", "https://localhost:1234/foo", http.Header{}, true, nil)
testMatcher(t, matcher, "DELETE", "https://localhost:1234/foo", http.Header{}, true, nil)
})

require.NoError(t, matcher.Set(context.Background(), testRulesGlob))

t.Run("case=created", func(t *testing.T) {
testMatcher(t, matcher, "GET", "https://localhost:34/baz", false, &testRulesGlob[1])
testMatcher(t, matcher, "POST", "https://localhost:1234/foo", false, &testRulesGlob[0])
testMatcher(t, matcher, "DELETE", "https://localhost:1234/foo", true, nil)
testMatcher(t, matcher, "GET", "https://localhost:34/baz", http.Header{}, false, &testRulesGlob[1])
testMatcher(t, matcher, "POST", "https://localhost:1234/foo", http.Header{}, false, &testRulesGlob[0])
testMatcher(t, matcher, "DELETE", "https://localhost:1234/foo", http.Header{}, true, nil)
})

t.Run("case=cache", func(t *testing.T) {
r, err := matcher.Match(context.Background(), "GET", mustParseURL(t, "https://localhost:34/baz"))
r, err := matcher.Match(context.Background(), "GET", mustParseURL(t, "https://localhost:34/baz"), http.Header{})
require.NoError(t, err)
got, err := matcher.Get(context.Background(), r.ID)
require.NoError(t, err)
Expand All @@ -181,9 +202,11 @@ func TestMatcher(t *testing.T) {
require.NoError(t, matcher.Set(context.Background(), testRulesGlob[1:]))

t.Run("case=updated", func(t *testing.T) {
testMatcher(t, matcher, "GET", "https://localhost:34/baz", false, &testRulesGlob[1])
testMatcher(t, matcher, "POST", "https://localhost:1234/foo", true, nil)
testMatcher(t, matcher, "DELETE", "https://localhost:1234/foo", true, nil)
testMatcher(t, matcher, "GET", "https://localhost:34/baz", http.Header{}, false, &testRulesGlob[1])
testMatcher(t, matcher, "POST", "https://localhost:1234/foo", http.Header{}, true, nil)
testMatcher(t, matcher, "DELETE", "https://localhost:1234/foo", http.Header{}, true, nil)
testMatcher(t, matcher, "PATCH", "https://localhost:343/bar", http.Header{"Content-Type": []string{"application/some-app.v1+json"}}, true, nil)
testMatcher(t, matcher, "PATCH", "https://localhost:343/bar", http.Header{"Content-Type": []string{"application/some-app.v2+json"}}, false, &testRulesGlob[3])
})
})
}
Expand Down
5 changes: 3 additions & 2 deletions rule/repository_memory.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ package rule

import (
"context"
"net/http"
"net/url"
"sync"

Expand Down Expand Up @@ -125,7 +126,7 @@ func (m *RepositoryMemory) Set(ctx context.Context, rules []Rule) error {
return nil
}

func (m *RepositoryMemory) Match(_ context.Context, method string, u *url.URL) (*Rule, error) {
func (m *RepositoryMemory) Match(_ context.Context, method string, u *url.URL, headers http.Header) (*Rule, error) {
if u == nil {
return nil, errors.WithStack(errors.New("nil URL provided"))
}
Expand All @@ -136,7 +137,7 @@ func (m *RepositoryMemory) Match(_ context.Context, method string, u *url.URL) (
var rules []Rule
for k := range m.rules {
r := &m.rules[k]
if matched, err := r.IsMatching(m.matchingStrategy, method, u); err != nil {
if matched, err := r.IsMatching(m.matchingStrategy, method, u, headers); err != nil {
return nil, errors.WithStack(err)
} else if matched {
rules = append(rules, *r)
Expand Down
44 changes: 41 additions & 3 deletions rule/rule.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ package rule
import (
"encoding/json"
"fmt"
"net/http"
"net/url"
"strings"

Expand All @@ -35,7 +36,7 @@ type Match struct {
// An array of HTTP methods (e.g. GET, POST, PUT, DELETE, ...). When ORY Oathkeeper searches for rules
// to decide what to do with an incoming request to the proxy server, it compares the HTTP method of the incoming
// request with the HTTP methods of each rules. If a match is found, the rule is considered a partial match.
// If the matchesUrl field is satisfied as well, the rule is considered a full match.
// If the matchesUrl and matchesHeaders fields are satisfied as well, the rule is considered a full match.
Methods []string `json:"methods"`

// This field represents the URL pattern this rule matches. When ORY Oathkeeper searches for rules
Expand All @@ -50,6 +51,14 @@ type Match struct {
// The following regexp example matches all paths of the domain `mydomain.com`: `https://mydomain.com/<.*>`.
// The glob equivalent of the above regexp example is `https://mydomain.com/<*>`.
URL string `json:"url"`

// A map of HTTP headers. When ORY Oathkeeper searches for rules
// to decide what to do with an incoming request to the proxy server, it compares the HTTP headers of the incoming
// request with the HTTP headers of each rules. If a match is found, the rule is considered a partial match.
// For headers with values in array format (e.g. User-Agent headers), the rule header value must match at least one
// of the request header values.
// If the matchesUrl and matchesMethods fields are satisfied as well, the rule is considered a full match.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wha is the difference between a partial and a full match?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is wording have taken from the comments on other fields.
My interpretation is that a partial match means you satisfied to one aspect of the rule. The incoming request has to be a (partial) match to all aspects of this rule in order to become a full match.

Headers map[string]string `json:"headers"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I understand, this is an AND match. So if all requirements are met from the rule, it will apply. Is that desired? Are there use cases for OR? And if so, how do we handle those? We can also only support AND for now but will nede to support OR most likely at some point, without breaking the config.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this point, the implementation only supports AND.
You have to match all headers specified in the rule. IMHO, an OR implementation would be overcomplicating things. It should just be a different rule. Headers are seen just like URLs are (although, it is true that URLs support wildcards).

Copy link

@a-manraj-pvotal a-manraj-pvotal Jul 13, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that using using OR is definitely a breaking change as it would potentially override normal oathkeeper rule matching and documentation with an optional routing header that was aimed at obtaining rule unicity per route with header (header : service aV1 to match a certain rule and header: service aV2 to match another rule and for both to live at the same time). Therefore i would recommend supporting only AND for now as it is meant only as an advanced configuration parameter to address multi service version routing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, that makes sense!

}

type Handler struct {
Expand Down Expand Up @@ -170,15 +179,21 @@ func (r *Rule) GetID() string {
return r.ID
}

// IsMatching checks whether the provided url and method match the rule.
// IsMatching checks whether the provided url, method and headers match the rule.
// An error will be returned if a regexp matching strategy is selected and regexp timeout occurs.
func (r *Rule) IsMatching(strategy configuration.MatchingStrategy, method string, u *url.URL) (bool, error) {
func (r *Rule) IsMatching(strategy configuration.MatchingStrategy, method string, u *url.URL, headers http.Header) (bool, error) {
if !stringInSlice(method, r.Match.Methods) {
return false, nil
}

if !matchHeaders(headers, r.Match) {
return false, nil
}

if err := ensureMatchingEngine(r, strategy); err != nil {
return false, err
}

matchAgainst := fmt.Sprintf("%s://%s%s", u.Scheme, u.Host, u.Path)
return r.matchingEngine.IsMatching(r.Match.URL, matchAgainst)
}
Expand Down Expand Up @@ -218,6 +233,29 @@ func ensureMatchingEngine(rule *Rule, strategy configuration.MatchingStrategy) e
return errors.Wrap(ErrUnknownMatchingStrategy, string(strategy))
}

func matchHeaders(requestHeaders http.Header, ruleMatch *Match) bool {
for matcherHeaderKey, matcherHeaderValue := range ruleMatch.Headers {
foundMatch := false
for requestHeaderKey, requestHeaderValues := range requestHeaders {
// Break if we find the matching key
if strings.EqualFold(matcherHeaderKey, requestHeaderKey) {
// Match only with any of the header value
for _, requestHeaderValue := range requestHeaderValues {
if strings.EqualFold(matcherHeaderValue, requestHeaderValue) {
foundMatch = true
break
}
}
break
}
}
if !foundMatch {
return false
}
}
return true
}

// ExtractRegexGroups returns the values matching the rule pattern
func (r *Rule) ExtractRegexGroups(strategy configuration.MatchingStrategy, u *url.URL) ([]string, error) {
if err := ensureMatchingEngine(r, strategy); err != nil {
Expand Down
Loading