From c07a0ba2d55f8448da0dff3bc17c8393fdbd6756 Mon Sep 17 00:00:00 2001 From: Steve Kriss Date: Wed, 8 May 2024 14:11:27 -0600 Subject: [PATCH] internal/envoy: support AuthPolicy on direct response routes (#6426) Previously, auth policies on direct response routes were ignored. Signed-off-by: Steve Kriss Co-authored-by: shadi-altarsha --- .../unreleased/6426-shadialtarsha-small.md | 1 + internal/envoy/v3/route.go | 9 +++ internal/envoy/v3/route_test.go | 75 +++++++++++++++++++ test/e2e/httpproxy/external_auth_test.go | 48 ++++++++++++ 4 files changed, 133 insertions(+) create mode 100644 changelogs/unreleased/6426-shadialtarsha-small.md diff --git a/changelogs/unreleased/6426-shadialtarsha-small.md b/changelogs/unreleased/6426-shadialtarsha-small.md new file mode 100644 index 00000000000..d6891ebc0c2 --- /dev/null +++ b/changelogs/unreleased/6426-shadialtarsha-small.md @@ -0,0 +1 @@ +Fixes bug where external authorization policy was ignored on HTTPProxy direct response routes. \ No newline at end of file diff --git a/internal/envoy/v3/route.go b/internal/envoy/v3/route.go index ecc2a4934fa..0dc875082c6 100644 --- a/internal/envoy/v3/route.go +++ b/internal/envoy/v3/route.go @@ -122,6 +122,15 @@ func buildRoute(dagRoute *dag.Route, vhostName string, secure bool) *envoy_confi // redirect routes to *both* the insecure and secure vhosts. route.Action = UpgradeHTTPS() case dagRoute.DirectResponse != nil: + route.TypedPerFilterConfig = map[string]*anypb.Any{} + + // Apply per-route authorization policy modifications. + if dagRoute.AuthDisabled { + route.TypedPerFilterConfig["envoy.filters.http.ext_authz"] = routeAuthzDisabled() + } else if len(dagRoute.AuthContext) > 0 { + route.TypedPerFilterConfig["envoy.filters.http.ext_authz"] = routeAuthzContext(dagRoute.AuthContext) + } + route.Action = routeDirectResponse(dagRoute.DirectResponse) case dagRoute.Redirect != nil: // TODO request/response headers? diff --git a/internal/envoy/v3/route_test.go b/internal/envoy/v3/route_test.go index 2f91680d7e4..152776134ae 100644 --- a/internal/envoy/v3/route_test.go +++ b/internal/envoy/v3/route_test.go @@ -993,6 +993,81 @@ func TestRouteDirectResponse(t *testing.T) { } } +func TestBuildRouteWithDirectResponse(t *testing.T) { + tests := map[string]struct { + dagRoute *dag.Route + vhostName string + secure bool + want *envoy_config_route_v3.Route + }{ + "direct-response-with-auth": { + dagRoute: &dag.Route{ + DirectResponse: &dag.DirectResponse{ + StatusCode: 500, + Body: "Internal Server Error", + }, + AuthContext: map[string]string{ + "PrincipalName": "user", + }, + PathMatchCondition: &dag.PrefixMatchCondition{ + Prefix: "/foo", + PrefixMatchType: dag.PrefixMatchString, + }, + }, + vhostName: "example", + secure: true, + want: &envoy_config_route_v3.Route{ + TypedPerFilterConfig: map[string]*anypb.Any{ + "envoy.filters.http.ext_authz": routeAuthzContext(map[string]string{ + "PrincipalName": "user", + }), + }, + Action: routeDirectResponse(&dag.DirectResponse{ + StatusCode: 500, + Body: "Internal Server Error", + }), + Match: &envoy_config_route_v3.RouteMatch{ + PathSpecifier: &envoy_config_route_v3.RouteMatch_Prefix{ + Prefix: "/foo", + }, + }, + }, + }, + "direct-response-auth-disabled": { + dagRoute: &dag.Route{ + DirectResponse: &dag.DirectResponse{ + StatusCode: 403, + }, + AuthDisabled: true, + PathMatchCondition: &dag.PrefixMatchCondition{ + Prefix: "/foo", + PrefixMatchType: dag.PrefixMatchString, + }, + }, + vhostName: "example", + secure: false, + want: &envoy_config_route_v3.Route{ + TypedPerFilterConfig: map[string]*anypb.Any{ + "envoy.filters.http.ext_authz": routeAuthzDisabled(), + }, + Action: routeDirectResponse(&dag.DirectResponse{StatusCode: 403}), + Match: &envoy_config_route_v3.RouteMatch{ + PathSpecifier: &envoy_config_route_v3.RouteMatch_Prefix{ + Prefix: "/foo", + }, + }, + }, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + got := buildRoute(tc.dagRoute, tc.vhostName, tc.secure) + protobuf.ExpectEqual(t, tc.want, got) + }) + } +} + func TestWeightedClusters(t *testing.T) { tests := map[string]struct { route *dag.Route diff --git a/test/e2e/httpproxy/external_auth_test.go b/test/e2e/httpproxy/external_auth_test.go index da651497486..89f678c382a 100644 --- a/test/e2e/httpproxy/external_auth_test.go +++ b/test/e2e/httpproxy/external_auth_test.go @@ -17,6 +17,7 @@ package httpproxy import ( "context" + "net/http" . "github.com/onsi/ginkgo/v2" "github.com/stretchr/testify/assert" @@ -206,6 +207,25 @@ func testExternalAuth(namespace string) { }, }, }, + { + Conditions: []contour_v1.MatchCondition{ + {Prefix: "/direct-response-auth-enabled"}, + }, + DirectResponsePolicy: &contour_v1.HTTPDirectResponsePolicy{ + StatusCode: http.StatusTeapot, + }, + }, + { + Conditions: []contour_v1.MatchCondition{ + {Prefix: "/direct-response-auth-disabled"}, + }, + DirectResponsePolicy: &contour_v1.HTTPDirectResponsePolicy{ + StatusCode: http.StatusTeapot, + }, + AuthPolicy: &contour_v1.AuthorizationPolicy{ + Disabled: true, + }, + }, { AuthPolicy: &contour_v1.AuthorizationPolicy{ @@ -283,5 +303,33 @@ func testExternalAuth(namespace string) { body = f.GetEchoResponseBody(res.Body) assert.Equal(t, "default", body.RequestHeaders.Get("Auth-Context-Target")) assert.Equal(t, "externalauth.projectcontour.io", body.RequestHeaders.Get("Auth-Context-Hostname")) + + // Direct response with external auth enabled should get a 401. + res, ok = f.HTTP.SecureRequestUntil(&e2e.HTTPSRequestOpts{ + Host: p.Spec.VirtualHost.Fqdn, + Path: "/direct-response-auth-enabled", + Condition: e2e.HasStatusCode(401), + }) + require.NotNil(t, res, "request never succeeded") + require.Truef(t, ok, "expected 401 response code, got %d", res.StatusCode) + + // Direct response with external auth enabled with "allow" in the path + // should succeed. + res, ok = f.HTTP.SecureRequestUntil(&e2e.HTTPSRequestOpts{ + Host: p.Spec.VirtualHost.Fqdn, + Path: "/direct-response-auth-enabled/allow", + Condition: e2e.HasStatusCode(http.StatusTeapot), + }) + require.NotNil(t, res, "request never succeeded") + require.Truef(t, ok, "expected 418 response code, got %d", res.StatusCode) + + // Direct response with external auth disabled should succeed. + res, ok = f.HTTP.SecureRequestUntil(&e2e.HTTPSRequestOpts{ + Host: p.Spec.VirtualHost.Fqdn, + Path: "/direct-response-auth-disabled", + Condition: e2e.HasStatusCode(http.StatusTeapot), + }) + require.NotNil(t, res, "request never succeeded") + require.Truef(t, ok, "expected 418 response code, got %d", res.StatusCode) }) }