Skip to content
This repository has been archived by the owner on Apr 22, 2024. It is now read-only.

Implement logic for TriggerRules config #3

Merged
merged 3 commits into from
Feb 12, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
5 changes: 4 additions & 1 deletion e2e/mock/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
10 changes: 10 additions & 0 deletions e2e/mock/authz-config.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,5 +41,15 @@
}
]
}
],
"triggerRules": [
{
"excludedPaths": [
{ "prefix": "/excluded" }
],
"includedPaths": [
{ "prefix": "/included" }
]
}
]
}
26 changes: 22 additions & 4 deletions e2e/mock/mock_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand All @@ -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")

Expand All @@ -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")

Expand All @@ -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
}
79 changes: 79 additions & 0 deletions internal/server/authz.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package server
import (
"context"
"fmt"
"regexp"
"strings"

envoy "github.com/envoyproxy/go-control-plane/envoy/service/auth/v3"
Expand Down Expand Up @@ -72,6 +73,17 @@ 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.
trLog := e.log.Context(ctx)
Copy link
Member

Choose a reason for hiding this comment

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

In line 90 we're already getting the log from the context. let's just create the log variable here and reuse it later?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

totally, didn't notice that.

if !mustTriggerCheck(trLog, e.cfg.TriggerRules, req) {
trLog.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)

Expand Down Expand Up @@ -147,3 +159,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
}
}
189 changes: 189 additions & 0 deletions internal/server/authz_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,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"
Expand Down Expand Up @@ -161,6 +162,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},
Copy link
Member

Choose a reason for hiding this comment

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

Add a test for the regexp error as well.

{"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{
Expand Down Expand Up @@ -202,3 +359,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,
},
}
}
Loading