From e8d3ce1a2ee704f771c74baa88e3a26f8aff8777 Mon Sep 17 00:00:00 2001
From: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Date: Tue, 21 Jul 2020 00:58:14 +0700
Subject: [PATCH] authorize,proxy: allow traefik forward auth without uri query
 (#1103)

In #1030, the fix was done without aware of the context that traefik
forward auth mode did allow request without the "?uri=". Previosuly,
this is done in proxy, and by converting the forward auth request to
actual request. The fix is #1030 prevent this conversion, to makre
authorize service aware of which is forward auth request.

But that causes traefik forward auth without "?uri" stop working. Fixing
it by making the authorize service also honor the forwarded uri header,
too.

Fixes #1096
---
 authorize/grpc.go      | 14 +++++++++-----
 authorize/grpc_test.go | 29 +++++++++++++++++++----------
 proxy/forward_auth.go  |  6 +++---
 3 files changed, 31 insertions(+), 18 deletions(-)

diff --git a/authorize/grpc.go b/authorize/grpc.go
index 653e3680a..2655d6d94 100644
--- a/authorize/grpc.go
+++ b/authorize/grpc.go
@@ -192,6 +192,14 @@ func (a *Authorize) handleForwardAuth(req *envoy_service_auth_v2.CheckRequest) b
 	}
 
 	uriQuery := checkURL.Query().Get("uri")
+	if headers := req.GetAttributes().GetRequest().GetHttp().GetHeaders(); uriQuery == "" && headers != nil {
+		uriQuery = headers[http.CanonicalHeaderKey(httputil.HeaderForwardedProto)] + "://" +
+			headers[http.CanonicalHeaderKey(httputil.HeaderForwardedHost)]
+		if xfu := headers[http.CanonicalHeaderKey(httputil.HeaderForwardedURI)]; xfu != "/" {
+			uriQuery += xfu
+		}
+	}
+
 	if (checkURL.Path != "/" && checkURL.Path != "/verify") || uriQuery == "" {
 		return false
 	}
@@ -204,11 +212,7 @@ func (a *Authorize) handleForwardAuth(req *envoy_service_auth_v2.CheckRequest) b
 	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(httputil.HeaderForwardedURI)]; 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
diff --git a/authorize/grpc_test.go b/authorize/grpc_test.go
index 5d46cdd05..759731b4d 100644
--- a/authorize/grpc_test.go
+++ b/authorize/grpc_test.go
@@ -10,6 +10,7 @@ import (
 	"github.com/pomerium/pomerium/authorize/evaluator"
 	"github.com/pomerium/pomerium/config"
 	"github.com/pomerium/pomerium/internal/encoding/jws"
+	"github.com/pomerium/pomerium/internal/httputil"
 )
 
 const certPEM = `
@@ -127,21 +128,29 @@ func Test_handleForwardAuth(t *testing.T) {
 					},
 					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"},
+							Method: "GET",
+							Path:   "/",
+							Host:   "forward-auth.example.com",
+							Scheme: "https",
+							Headers: map[string]string{
+								httputil.HeaderForwardedURI:   "/foo/bar",
+								httputil.HeaderForwardedProto: "https",
+								httputil.HeaderForwardedHost:  "example.com",
+							},
 						},
 					},
 				},
 			},
 			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"},
+				Method: "GET",
+				Path:   "/foo/bar",
+				Host:   "example.com",
+				Scheme: "https",
+				Headers: map[string]string{
+					httputil.HeaderForwardedURI:   "/foo/bar",
+					httputil.HeaderForwardedProto: "https",
+					httputil.HeaderForwardedHost:  "example.com",
+				},
 			},
 			forwardAuthURL: "https://forward-auth.example.com",
 			isForwardAuth:  true,
diff --git a/proxy/forward_auth.go b/proxy/forward_auth.go
index c36a8a80b..f66854082 100644
--- a/proxy/forward_auth.go
+++ b/proxy/forward_auth.go
@@ -141,10 +141,10 @@ func (p *Proxy) Verify(verifyOnly bool) http.Handler {
 // forwardAuthRedirectToSignInWithURI redirects request to authenticate signin url,
 // with all necessary information extracted from given input uri.
 func (p *Proxy) forwardAuthRedirectToSignInWithURI(w http.ResponseWriter, r *http.Request, uri *url.URL) {
-	// Traefik set the uri in the header, we must add it to redirect uri if present. Otherwise, request like
+	// Traefik set the uri in the header, we must set it in redirect uri if present. Otherwise, request like
 	// https://example.com/foo will be redirected to https://example.com after authentication.
-	if xfu := r.Header.Get(httputil.HeaderForwardedURI); xfu != "" {
-		uri.Path += xfu
+	if xfu := r.Header.Get(httputil.HeaderForwardedURI); xfu != "/" {
+		uri.Path = xfu
 	}
 
 	// redirect to authenticate