authorize: honor X-Forwarded-Uri in forward auth mode

Some ingress like traefik set the X-Forwarded-Uri header instead
of passing the actual path in request, we should hornor and use
that header in forward auth mode.

While at it, refactoring the handleForwardAuth to return earlier instead
of nested condition, and add more tests to cover all cases.

authorize/grpc.go

@@ -177,25 +177,33 @@ func (a *Authorize) handleForwardAuth(req *envoy_service_auth_v2.CheckRequest) b
 	}
 
 	checkURL := getCheckRequestURL(req)
-	if urlutil.StripPort(checkURL.Host) == urlutil.StripPort(opts.GetForwardAuthURL().Host) {
+	if urlutil.StripPort(checkURL.Host) != urlutil.StripPort(opts.GetForwardAuthURL().Host) {
-		if (checkURL.Path == "/" || checkURL.Path == "/verify") && checkURL.Query().Get("uri") != "" {
+		return false
-			verifyURL, err := url.Parse(checkURL.Query().Get("uri"))
+	}
+
+	uriQuery := checkURL.Query().Get("uri")
+	if (checkURL.Path != "/" && checkURL.Path != "/verify") || uriQuery == "" {
+		return false
+	}
+	verifyURL, err := url.Parse(uriQuery)
 	if err != nil {
 		log.Warn().Str("uri", checkURL.Query().Get("uri")).Err(err).Msg("failed to parse uri for forward authentication")
 		return false
 	}
+
 	req.Attributes.Request.Http.Scheme = verifyURL.Scheme
 	req.Attributes.Request.Http.Host = verifyURL.Host
 	req.Attributes.Request.Http.Path = verifyURL.Path
+	if headers := req.GetAttributes().GetRequest().GetHttp().GetHeaders(); headers != nil {
+		if xfu := headers[http.CanonicalHeaderKey("x-forwarded-uri")]; xfu != "" {
+			req.Attributes.Request.Http.Path += xfu
+		}
+	}
 	// envoy sends the query string as part of the path
 	if verifyURL.RawQuery != "" {
 		req.Attributes.Request.Http.Path += "?" + verifyURL.RawQuery
 	}
 	return true
-		}
-	}
-
-	return false
 }
 
 func (a *Authorize) getEvaluatorRequestFromCheckRequest(in *envoy_service_auth_v2.CheckRequest, sessionState *sessions.State) *evaluator.Request {

authorize/grpc_test.go

@@ -78,7 +78,16 @@ func Test_getEvaluatorRequest(t *testing.T) {
 }
 
 func Test_handleForwardAuth(t *testing.T) {
-	checkReq := &envoy_service_auth_v2.CheckRequest{
+	tests := []struct {
+		name           string
+		checkReq       *envoy_service_auth_v2.CheckRequest
+		attrCtxHTTPReq *envoy_service_auth_v2.AttributeContext_HttpRequest
+		forwardAuthURL string
+		isForwardAuth  bool
+	}{
+		{
+			name: "enabled",
+			checkReq: &envoy_service_auth_v2.CheckRequest{
 				Attributes: &envoy_service_auth_v2.AttributeContext{
 					Source: &envoy_service_auth_v2.AttributeContext_Peer{
 						Certificate: url.QueryEscape(certPEM),
@@ -92,30 +101,152 @@ func Test_handleForwardAuth(t *testing.T) {
 						},
 					},
 				},
-	}
+			},
-
+			attrCtxHTTPReq: &envoy_service_auth_v2.AttributeContext_HttpRequest{
-	t.Run("enabled", func(t *testing.T) {
-		a := new(Authorize)
-		a.currentOptions.Store(config.Options{
-			ForwardAuthURL: mustParseURL("https://forward-auth.example.com"),
-		})
-		isForwardAuth := a.handleForwardAuth(checkReq)
-		assert.True(t, isForwardAuth)
-		assert.Equal(t, &envoy_service_auth_v2.AttributeContext_HttpRequest{
 				Method: "GET",
 				Path:   "/some/path?qs=1",
 				Host:   "example.com",
 				Scheme: "https",
-		}, checkReq.Attributes.Request.Http)
+			},
-	})
+			forwardAuthURL: "https://forward-auth.example.com",
-	t.Run("disabled", func(t *testing.T) {
+			isForwardAuth:  true,
+		},
+		{
+			name:           "disabled",
+			checkReq:       nil,
+			attrCtxHTTPReq: nil,
+			forwardAuthURL: "",
+			isForwardAuth:  false,
+		},
+		{
+			name: "honor x-forwarded-uri set",
+			checkReq: &envoy_service_auth_v2.CheckRequest{
+				Attributes: &envoy_service_auth_v2.AttributeContext{
+					Source: &envoy_service_auth_v2.AttributeContext_Peer{
+						Certificate: url.QueryEscape(certPEM),
+					},
+					Request: &envoy_service_auth_v2.AttributeContext_Request{
+						Http: &envoy_service_auth_v2.AttributeContext_HttpRequest{
+							Method:  "GET",
+							Path:    "/verify?uri=" + url.QueryEscape("https://example.com?q=foo"),
+							Host:    "forward-auth.example.com",
+							Scheme:  "https",
+							Headers: map[string]string{"X-Forwarded-Uri": "/foo/bar"},
+						},
+					},
+				},
+			},
+			attrCtxHTTPReq: &envoy_service_auth_v2.AttributeContext_HttpRequest{
+				Method:  "GET",
+				Path:    "/foo/bar?q=foo",
+				Host:    "example.com",
+				Scheme:  "https",
+				Headers: map[string]string{"X-Forwarded-Uri": "/foo/bar"},
+			},
+			forwardAuthURL: "https://forward-auth.example.com",
+			isForwardAuth:  true,
+		},
+		{
+			name: "request with invalid forward auth url",
+			checkReq: &envoy_service_auth_v2.CheckRequest{
+				Attributes: &envoy_service_auth_v2.AttributeContext{
+					Source: &envoy_service_auth_v2.AttributeContext_Peer{
+						Certificate: url.QueryEscape(certPEM),
+					},
+					Request: &envoy_service_auth_v2.AttributeContext_Request{
+						Http: &envoy_service_auth_v2.AttributeContext_HttpRequest{
+							Method: "GET",
+							Path:   "/verify?uri=" + url.QueryEscape("https://example.com?q=foo"),
+							Host:   "fake-forward-auth.example.com",
+							Scheme: "https",
+						},
+					},
+				},
+			},
+			attrCtxHTTPReq: nil,
+			forwardAuthURL: "https://forward-auth.example.com",
+			isForwardAuth:  false,
+		},
+		{
+			name: "request with invalid path",
+			checkReq: &envoy_service_auth_v2.CheckRequest{
+				Attributes: &envoy_service_auth_v2.AttributeContext{
+					Source: &envoy_service_auth_v2.AttributeContext_Peer{
+						Certificate: url.QueryEscape(certPEM),
+					},
+					Request: &envoy_service_auth_v2.AttributeContext_Request{
+						Http: &envoy_service_auth_v2.AttributeContext_HttpRequest{
+							Method: "GET",
+							Path:   "/foo?uri=" + url.QueryEscape("https://example.com?q=foo"),
+							Host:   "forward-auth.example.com",
+							Scheme: "https",
+						},
+					},
+				},
+			},
+			attrCtxHTTPReq: nil,
+			forwardAuthURL: "https://forward-auth.example.com",
+			isForwardAuth:  false,
+		},
+		{
+			name: "request with empty uri",
+			checkReq: &envoy_service_auth_v2.CheckRequest{
+				Attributes: &envoy_service_auth_v2.AttributeContext{
+					Source: &envoy_service_auth_v2.AttributeContext_Peer{
+						Certificate: url.QueryEscape(certPEM),
+					},
+					Request: &envoy_service_auth_v2.AttributeContext_Request{
+						Http: &envoy_service_auth_v2.AttributeContext_HttpRequest{
+							Method: "GET",
+							Path:   "/verify?uri=",
+							Host:   "forward-auth.example.com",
+							Scheme: "https",
+						},
+					},
+				},
+			},
+			attrCtxHTTPReq: nil,
+			forwardAuthURL: "https://forward-auth.example.com",
+			isForwardAuth:  false,
+		},
+		{
+			name: "request with invalid uri",
+			checkReq: &envoy_service_auth_v2.CheckRequest{
+				Attributes: &envoy_service_auth_v2.AttributeContext{
+					Source: &envoy_service_auth_v2.AttributeContext_Peer{
+						Certificate: url.QueryEscape(certPEM),
+					},
+					Request: &envoy_service_auth_v2.AttributeContext_Request{
+						Http: &envoy_service_auth_v2.AttributeContext_HttpRequest{
+							Method: "GET",
+							Path:   "/verify?uri= http://example.com/foo",
+							Host:   "forward-auth.example.com",
+							Scheme: "https",
+						},
+					},
+				},
+			},
+			attrCtxHTTPReq: nil,
+			forwardAuthURL: "https://forward-auth.example.com",
+			isForwardAuth:  false,
+		},
+	}
+
+	for _, tc := range tests {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
 			a := new(Authorize)
-		a.currentOptions.Store(config.Options{
+			fau := new(url.URL)
-			ForwardAuthURL: nil,
+			if tc.forwardAuthURL != "" {
-		})
+				fau = mustParseURL(tc.forwardAuthURL)
-		isForwardAuth := a.handleForwardAuth(checkReq)
+			}
-		assert.False(t, isForwardAuth)
+			a.currentOptions.Store(config.Options{ForwardAuthURL: fau})
+			assert.Equal(t, tc.isForwardAuth, a.handleForwardAuth(tc.checkReq))
+			if tc.attrCtxHTTPReq != nil {
+				assert.Equal(t, tc.attrCtxHTTPReq, tc.checkReq.Attributes.Request.Http)
+			}
 		})
+	}
 }
 
 func mustParseURL(str string) *url.URL {