diff --git a/e2e/mock/Makefile b/e2e/mock/Makefile index e8485b4..3d65cd5 100644 --- a/e2e/mock/Makefile +++ b/e2e/mock/Makefile @@ -18,8 +18,11 @@ E2E_TEST_OPTS ?= -count=1 .PHONY: e2e e2e: e2e-pre + @$(MAKE) e2e-test e2e-post + +.PHONY: e2e-test +e2e-test: @go test $(E2E_TEST_OPTS) ./... || ( $(MAKE) e2e-post-error; exit 1 ) - @$(MAKE) e2e-post .PHONY: e2e-pre e2e-pre: diff --git a/e2e/mock/authz-config.json b/e2e/mock/authz-config.json index 9754319..9fe51fc 100644 --- a/e2e/mock/authz-config.json +++ b/e2e/mock/authz-config.json @@ -41,5 +41,15 @@ } ] } + ], + "triggerRules": [ + { + "excludedPaths": [ + { "prefix": "/excluded" } + ], + "includedPaths": [ + { "prefix": "/included" } + ] + } ] } diff --git a/e2e/mock/mock_test.go b/e2e/mock/mock_test.go index ac8247a..f4a63bd 100644 --- a/e2e/mock/mock_test.go +++ b/e2e/mock/mock_test.go @@ -24,11 +24,14 @@ import ( const ( testEnvoyURL = "http://localhost:8080" testAuthzHeader = "X-Authz-Decision" + + excludedPath = "/excluded" + includedPath = "/included" ) func TestMock(t *testing.T) { t.Run("allow-equality", func(t *testing.T) { - req, err := http.NewRequest("GET", testEnvoyURL, nil) + req, err := http.NewRequest("GET", withPath(includedPath), nil) require.NoError(t, err) req.Header.Set(testAuthzHeader, "allow") @@ -38,7 +41,7 @@ func TestMock(t *testing.T) { }) t.Run("allow-prefix", func(t *testing.T) { - req, err := http.NewRequest("GET", testEnvoyURL, nil) + req, err := http.NewRequest("GET", withPath(includedPath), nil) require.NoError(t, err) req.Header.Set(testAuthzHeader, "ok-prefix") @@ -48,7 +51,7 @@ func TestMock(t *testing.T) { }) t.Run("deny-header", func(t *testing.T) { - req, err := http.NewRequest("GET", testEnvoyURL, nil) + req, err := http.NewRequest("GET", withPath(includedPath), nil) require.NoError(t, err) req.Header.Set(testAuthzHeader, "non-match") @@ -58,9 +61,24 @@ func TestMock(t *testing.T) { }) t.Run("deny", func(t *testing.T) { - res, err := http.Get(testEnvoyURL) + res, err := http.Get(withPath(includedPath)) require.NoError(t, err) require.Equal(t, 403, res.StatusCode) }) + + // excluded path should not be affected by the header since the auth service checks are not triggered. + t.Run("allow-non-matching-header-for-excluded-path", func(t *testing.T) { + req, err := http.NewRequest("GET", withPath(excludedPath), nil) + require.NoError(t, err) + req.Header.Set(testAuthzHeader, "non-match") + + res, err := http.DefaultClient.Do(req) + require.NoError(t, err) + require.Equal(t, 200, res.StatusCode) + }) +} + +func withPath(p string) string { + return testEnvoyURL + p } diff --git a/internal/server/authz.go b/internal/server/authz.go index dfcf741..f864016 100644 --- a/internal/server/authz.go +++ b/internal/server/authz.go @@ -17,6 +17,7 @@ package server import ( "context" "fmt" + "regexp" "strings" "time" @@ -73,10 +74,21 @@ func (e *ExtAuthZFilter) Register(server *grpc.Server) { // Check is the implementation of the Envoy AuthorizationServer interface. func (e *ExtAuthZFilter) Check(ctx context.Context, req *envoy.CheckRequest) (response *envoy.CheckResponse, err error) { + // If there are no trigger rules, allow the request with no check executions. + // TriggerRules are used to determine which request should be checked by the filter and which don't. + log := e.log.Context(ctx) + if !mustTriggerCheck(log, e.cfg.TriggerRules, req) { + log.Debug(fmt.Sprintf("no matching trigger rule, so allowing request to proceed without any authservice functionality %s://%s%s", + req.GetAttributes().GetRequest().GetHttp().GetScheme(), + req.GetAttributes().GetRequest().GetHttp().GetHost(), + req.GetAttributes().GetRequest().GetHttp().GetPath())) + return allow, nil + } + for _, c := range e.cfg.Chains { match := matches(c.Match, req) - log := e.log.Context(ctx).With("chain", c.Name) + log = log.With("chain", c.Name) log.Debug("evaluate filter chain", "match", match) if !match { @@ -150,3 +162,70 @@ func matches(m *configv1.Match, req *envoy.CheckRequest) bool { } return strings.HasPrefix(headerValue, m.GetPrefix()) } + +// mustTriggerCheck returns true if the request must be checked by the authservice filters. +// If any of the TriggerRules match the request path, then the request must be checked. +func mustTriggerCheck(log telemetry.Logger, rules []*configv1.TriggerRule, req *envoy.CheckRequest) bool { + // If there are no trigger rules, authservice checks should be triggered for all requests. + // If the request path is empty, (unlikely, but the piece used to match the rules) then trigger the checks. + if len(rules) == 0 || len(req.GetAttributes().GetRequest().GetHttp().GetPath()) == 0 { + return true + } + + for i, rule := range rules { + l := log.With("rule-index", i) + if matchTriggerRule(l, rule, req.GetAttributes().GetRequest().GetHttp().GetPath()) { + return true + } + } + return false +} + +func matchTriggerRule(log telemetry.Logger, rule *configv1.TriggerRule, path string) bool { + if rule == nil { + return false + } + + // if any of the excluded paths match, this rule doesn't match + for i, match := range rule.GetExcludedPaths() { + l := log.With("excluded-match-index", i) + if stringMatch(l, match, path) { + return false + } + } + + // if no excluded paths match, and there are no included paths, this rule matches + if len(rule.GetIncludedPaths()) == 0 { + return true + } + + for i, match := range rule.GetIncludedPaths() { + // if any of the included paths match, this rule matches + l := log.With("included-match-index", i) + if stringMatch(l, match, path) { + return true + } + } + + // if none of the included paths match, this rule doesn't match + return false +} + +func stringMatch(log telemetry.Logger, match *configv1.StringMatch, path string) bool { + switch m := match.GetMatchType().(type) { + case *configv1.StringMatch_Exact: + return m.Exact == path + case *configv1.StringMatch_Prefix: + return strings.HasPrefix(path, m.Prefix) + case *configv1.StringMatch_Suffix: + return strings.HasSuffix(path, m.Suffix) + case *configv1.StringMatch_Regex: + b, err := regexp.MatchString(m.Regex, path) + if err != nil { + log.Error("error matching regex", err, "regex", m.Regex, "match", false) + } + return b + default: + return false + } +} diff --git a/internal/server/authz_test.go b/internal/server/authz_test.go index c6f764a..fb8f325 100644 --- a/internal/server/authz_test.go +++ b/internal/server/authz_test.go @@ -20,6 +20,7 @@ import ( envoy "github.com/envoyproxy/go-control-plane/envoy/service/auth/v3" "github.com/stretchr/testify/require" + "github.com/tetratelabs/telemetry" "google.golang.org/grpc/codes" configv1 "github.com/tetrateio/authservice-go/config/gen/go/v1" @@ -134,6 +135,162 @@ func TestGrpcNoChainsMatched(t *testing.T) { require.Equal(t, int32(codes.PermissionDenied), ok.Status.Code) } +func TestStringMatch(t *testing.T) { + tests := []struct { + name string + match *configv1.StringMatch + path string + want bool + }{ + {"no-match", nil, "", false}, + {"equality-match", stringExact("test"), "test", true}, + {"equality-no-match", stringExact("test"), "no-match", false}, + {"prefix-match", stringPrefix("test"), "test-123", true}, + {"prefix-no-match", stringPrefix("test"), "no-match", false}, + {"suffix-match", stringSuffix("test"), "123-test", true}, + {"suffix-no-match", stringSuffix("test"), "no-match", false}, + {"regex-match", stringRegex(".*st"), "test", true}, + {"regex-no-match", stringRegex(".*st"), "no-match", false}, + {"regex-invalid", stringRegex("["), "no-match", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.want, stringMatch(telemetry.NoopLogger(), tt.match, tt.path)) + }) + } +} + +func TestMatchTriggerRule(t *testing.T) { + tests := []struct { + name string + rule *configv1.TriggerRule + path string + want bool + }{ + {"no-rule", nil, "/path", false}, + {"no-path", &configv1.TriggerRule{}, "", true}, + {"empty-rule", &configv1.TriggerRule{}, "/path", true}, + {"excluded-path-match", &configv1.TriggerRule{ExcludedPaths: []*configv1.StringMatch{stringExact("/path")}}, "/path", false}, + {"excluded-path-no-match", &configv1.TriggerRule{ExcludedPaths: []*configv1.StringMatch{stringExact("/path")}}, "/no-match", true}, + {"included-path-match", &configv1.TriggerRule{IncludedPaths: []*configv1.StringMatch{stringExact("/path")}}, "/path", true}, + {"included-path-no-match", &configv1.TriggerRule{IncludedPaths: []*configv1.StringMatch{stringExact("/path")}}, "/no-match", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.want, matchTriggerRule(telemetry.NoopLogger(), tt.rule, tt.path)) + }) + } + +} + +func TestMustTriggerCheck(t *testing.T) { + test := []struct { + name string + rules []*configv1.TriggerRule + path string + want bool + }{ + {"no-rules", nil, "/path", true}, + {"no-path", nil, "", true}, + {"empty-rules", []*configv1.TriggerRule{}, "/path", true}, + {"inclusions-match", []*configv1.TriggerRule{{IncludedPaths: []*configv1.StringMatch{stringExact("/path")}}}, "/path", true}, + {"inclusions-no-match", []*configv1.TriggerRule{{IncludedPaths: []*configv1.StringMatch{stringExact("/path")}}}, "/no-match", false}, + {"exclusions-match", []*configv1.TriggerRule{{ExcludedPaths: []*configv1.StringMatch{stringExact("/path")}}}, "/path", false}, + {"exclusions-no-match", []*configv1.TriggerRule{{ExcludedPaths: []*configv1.StringMatch{stringExact("/path")}}}, "/no-match", true}, + {"multiple-rules-no-match", []*configv1.TriggerRule{ + {ExcludedPaths: []*configv1.StringMatch{stringExact("/ex-path")}}, + {IncludedPaths: []*configv1.StringMatch{stringExact("/path")}}, + }, "/ex-path", false}, + {"multiple-rules-match", []*configv1.TriggerRule{ + {ExcludedPaths: []*configv1.StringMatch{stringExact("/no-path")}}, + {IncludedPaths: []*configv1.StringMatch{stringExact("/path")}}, + }, "/path", true}, + {"inverse-order-multiple-rules-no-match", []*configv1.TriggerRule{ + {IncludedPaths: []*configv1.StringMatch{stringExact("/path")}}, + {ExcludedPaths: []*configv1.StringMatch{stringExact("/ex-path")}}, + }, "/ex-path", false}, + {"inverse-order-multiple-rules-match", []*configv1.TriggerRule{ + {IncludedPaths: []*configv1.StringMatch{stringExact("/path")}}, + {ExcludedPaths: []*configv1.StringMatch{stringExact("/no-path")}}, + }, "/path", true}, + } + + for _, tt := range test { + t.Run(tt.name, func(t *testing.T) { + req := &envoy.CheckRequest{ + Attributes: &envoy.AttributeContext{ + Request: &envoy.AttributeContext_Request{ + Http: &envoy.AttributeContext_HttpRequest{ + Path: tt.path, + }, + }, + }, + } + require.Equal(t, tt.want, mustTriggerCheck(telemetry.NoopLogger(), tt.rules, req)) + }) + } +} + +func TestCheckTriggerRules(t *testing.T) { + tests := []struct { + name string + config *configv1.Config + path string + want codes.Code + }{ + { + "no-rules-triggers-check", + &configv1.Config{ + Chains: []*configv1.FilterChain{{Filters: []*configv1.Filter{mock(false)}}}, + }, + "/path", codes.PermissionDenied, + }, + { + "rules-inclusions-matching-triggers-check", + &configv1.Config{ + Chains: []*configv1.FilterChain{{Filters: []*configv1.Filter{mock(false)}}}, + TriggerRules: []*configv1.TriggerRule{ + { + IncludedPaths: []*configv1.StringMatch{stringExact("/path")}, + }, + }, + }, + "/path", codes.PermissionDenied}, + { + "rules-inclusions-no-match-does-not-trigger-check", + &configv1.Config{ + Chains: []*configv1.FilterChain{{Filters: []*configv1.Filter{mock(false)}}}, + TriggerRules: []*configv1.TriggerRule{ + { + IncludedPaths: []*configv1.StringMatch{stringExact("/path")}, + }, + }, + }, + "/no-path", codes.OK, // it does not reach mock(allow=false), so it returns OK + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + e := NewExtAuthZFilter(tt.config) + req := &envoy.CheckRequest{ + Attributes: &envoy.AttributeContext{ + Request: &envoy.AttributeContext_Request{ + Http: &envoy.AttributeContext_HttpRequest{ + Path: tt.path, + }, + }, + }, + } + got, err := e.Check(context.Background(), req) + require.NoError(t, err) + require.Equal(t, int32(tt.want), got.Status.Code) + }) + } +} + func mock(allow bool) *configv1.Filter { return &configv1.Filter{ Type: &configv1.Filter_Mock{ @@ -175,3 +332,35 @@ func header(value string) *envoy.CheckRequest { }, } } + +func stringExact(s string) *configv1.StringMatch { + return &configv1.StringMatch{ + MatchType: &configv1.StringMatch_Exact{ + Exact: s, + }, + } +} + +func stringPrefix(s string) *configv1.StringMatch { + return &configv1.StringMatch{ + MatchType: &configv1.StringMatch_Prefix{ + Prefix: s, + }, + } +} + +func stringSuffix(s string) *configv1.StringMatch { + return &configv1.StringMatch{ + MatchType: &configv1.StringMatch_Suffix{ + Suffix: s, + }, + } +} + +func stringRegex(s string) *configv1.StringMatch { + return &configv1.StringMatch{ + MatchType: &configv1.StringMatch_Regex{ + Regex: s, + }, + } +}