internal/envoy: support AuthPolicy on direct response routes (#6426)

Previously, auth policies on direct response routes were ignored. Signed-off-by: Steve Kriss <stephen.kriss@gmail.com> Co-authored-by: shadi-altarsha <shadi.altarsha@reddit.com>
projectcontour · May 8, 2024 · c07a0ba · c07a0ba
1 parent a485abb
commit c07a0ba
Show file tree

Hide file tree

Showing 4 changed files with 133 additions and 0 deletions.
diff --git a/changelogs/unreleased/6426-shadialtarsha-small.md b/changelogs/unreleased/6426-shadialtarsha-small.md
@@ -0,0 +1 @@
+Fixes bug where external authorization policy was ignored on HTTPProxy direct response routes.
diff --git 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
@@ -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
@@ -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)
 	})
 }