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

Commit

Permalink
Implement logic for TriggerRules config (#3)
Browse files Browse the repository at this point in the history 
* Implement logic for TriggerRules config

* Addressing review comments

* review
  • Loading branch information
@sergicastro
sergicastro authored Feb 12, 2024
1 parent 871d5a8 commit a7ac38b
Show file tree
Hide file tree
Showing 5 changed files with 305 additions and 6 deletions.
5 changes: 4 additions & 1 deletion e2e/mock/Makefile
View file
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
View file
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
View file
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
}
81 changes: 80 additions & 1 deletion internal/server/authz.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package server
import (
"context"
"fmt"
"regexp"
"strings"
"time"

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
}
189 changes: 189 additions & 0 deletions internal/server/authz_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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,
},
}
}

0 comments on commit a7ac38b

Please sign in to comment.