Repository: oauth2-proxy/oauth2-proxy
Pull request #1060 (2021-02-22 16:08:32)

Implement RewriteTarget to allow requests to be rewritten before proxying to upstream servers
<!--- Provide a general summary of your changes in the Title above -->

## Description

<!--- Describe your changes in detail -->
Adds the `rewriteTarget` field to the upstream structure available in the alpha config.
This will allow users to rewrite their upstream paths so that the backend path need not match the user facing request path.

Eg, to strip an unwanted prefix

```
upstream:
- path: /unwanted-prefix/(.*)
  rewriteTarget: /$1
```

## Motivation and Context

<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->
We have had several requests to allow users to be able to rewrite the path when their backends may not necessarily match up with the request paths

## How Has This Been Tested?

<!--- Please describe in detail how you tested your changes. -->
<!--- Include details of your testing environment, and the tests you ran to -->
<!--- see how your change affects other areas of the code, etc. -->
Unit testing and manual testing environment.

## Checklist:

<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->

- [x] My change requires a change to the documentation or CHANGELOG.
- [x] I have updated the documentation/CHANGELOG accordingly.
- [x] I have created a feature (non-master) branch for my PR.


UiP9AV6Y: `http.ServeMux` issues a redirect when a request is received without a trailing slash for a route which was initially registered *with* a trailing slash
> If a subtree has been registered and a request is received naming the subtree root without its trailing slash, ServeMux
> redirects that request to the subtree root (adding the trailing slash).

how would i need to configure this with the implementation of this PR?
would the following config cover this scenario? it would not redirect but the request route should still be matched, i think.

```yaml
upstreams:
- id: subtree
  path: /subtree/?(.*)
  rewriteTarget: /$1
  uri: http://example.com/
```

otherwise this approach is superior to #1058
JoelSpeed: > http.ServeMux issues a redirect when a request is received without a trailing slash for a route which was initially registered with a trailing slash

Perhaps we can just also implement this behaviour, would that be helpful or would it cause confusion? :thinking: 


JoelSpeed: @UiP9AV6Y Please check out the latest commit, it should handle the trailing slash as is done in the standard serveMux now
JoelSpeed: This is now ready for review! Added all the test etc.

The first commit in this PR is #1142, so we should get that PR merged first, that would make the review process here easier 
JoelSpeed: @NickMeves I think this is now ready to go

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 55841190..52139ef8 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -13,6 +13,7 @@
 
 ## Changes since v7.1.3
 
+- [#1060](https://github.com/oauth2-proxy/oauth2-proxy/pull/1060) Implement RewriteTarget to allow requests to be rewritten before proxying to upstream servers (@JoelSpeed)
 - [#1086](https://github.com/oauth2-proxy/oauth2-proxy/pull/1086) Refresh sessions before token expiration if configured (@NickMeves)
 - [#1226](https://github.com/oauth2-proxy/oauth2-proxy/pull/1226) Move app redirection logic to its own package (@JoelSpeed)
 - [#1128](https://github.com/oauth2-proxy/oauth2-proxy/pull/1128) Use gorilla mux for OAuth Proxy routing (@JoelSpeed)
diff --git a/docs/docs/configuration/alpha_config.md b/docs/docs/configuration/alpha_config.md
index ca490b34..cd8f0f3f 100644
--- a/docs/docs/configuration/alpha_config.md
+++ b/docs/docs/configuration/alpha_config.md
@@ -371,7 +371,8 @@ Requests will be proxied to this upstream if the path matches the request path.
 | Field | Type | Description |
 | ----- | ---- | ----------- |
 | `id` | _string_ | ID should be a unique identifier for the upstream.<br/>This value is required for all upstreams. |
-| `path` | _string_ | Path is used to map requests to the upstream server.<br/>The closest match will take precedence and all Paths must be unique. |
+| `path` | _string_ | Path is used to map requests to the upstream server.<br/>The closest match will take precedence and all Paths must be unique.<br/>Path can also take a pattern when used with RewriteTarget.<br/>Path segments can be captured and matched using regular experessions.<br/>Eg:<br/>- `^/foo$`: Match only the explicit path `/foo`<br/>- `^/bar/$`: Match any path prefixed with `/bar/`<br/>- `^/baz/(.*)$`: Match any path prefixed with `/baz` and capture the remaining path for use with RewriteTarget |
+| `rewriteTarget` | _string_ | RewriteTarget allows users to rewrite the request path before it is sent to<br/>the upstream server.<br/>Use the Path to capture segments for reuse within the rewrite target.<br/>Eg: With a Path of `^/baz/(.*)`, a RewriteTarget of `/foo/$1` would rewrite<br/>the request `/baz/abc/123` to `/foo/abc/123` before proxying to the<br/>upstream server. |
 | `uri` | _string_ | The URI of the upstream server. This may be an HTTP(S) server of a File<br/>based URL. It may include a path, in which case all requests will be served<br/>under that path.<br/>Eg:<br/>- http://localhost:8080<br/>- https://service.localhost<br/>- https://service.localhost/path<br/>- file://host/path<br/>If the URI's path is "/base" and the incoming request was for "/dir",<br/>the upstream request will be for "/base/dir". |
 | `insecureSkipTLSVerify` | _bool_ | InsecureSkipTLSVerify will skip TLS verification of upstream HTTPS hosts.<br/>This option is insecure and will allow potential Man-In-The-Middle attacks<br/>betweem OAuth2 Proxy and the usptream server.<br/>Defaults to false. |
 | `static` | _bool_ | Static will make all requests to this upstream have a static response.<br/>The response will have a body of "Authenticated" and a response code<br/>matching StaticCode.<br/>If StaticCode is not set, the response will return a 200 response. |
diff --git a/go.mod b/go.mod
index df007c82..b3f328e0 100644
--- a/go.mod
+++ b/go.mod
@@ -16,7 +16,7 @@ require (
 	github.com/ghodss/yaml v1.0.1-0.20190212211648-25d852aebe32
 	github.com/go-redis/redis/v8 v8.2.3
 	github.com/google/uuid v1.2.0
-	github.com/gorilla/mux v1.8.0 // indirect
+	github.com/gorilla/mux v1.8.0
 	github.com/justinas/alice v1.2.0
 	github.com/mbland/hmacauth v0.0.0-20170912233209-44256dfd4bfa
 	github.com/mitchellh/mapstructure v1.1.2
@@ -31,8 +31,8 @@ require (
 	github.com/stretchr/testify v1.6.1
 	github.com/vmihailenco/msgpack/v4 v4.3.11
 	github.com/yhat/wsutil v0.0.0-20170731153501-1d66fa95c997
-	golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9
-	golang.org/x/net v0.0.0-20200707034311-ab3426394381
+	golang.org/x/crypto v0.0.0-20200820211705-5c72a883971a
+	golang.org/x/net v0.0.0-20200822124328-c89045814202
 	golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d
 	golang.org/x/sync v0.0.0-20201207232520-09787c993a3a
 	google.golang.org/api v0.20.0
diff --git a/go.sum b/go.sum
index 5344792d..2b425081 100644
--- a/go.sum
+++ b/go.sum
@@ -460,8 +460,9 @@ golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACk
 golang.org/x/crypto v0.0.0-20190510104115-cbcb75029529/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
 golang.org/x/crypto v0.0.0-20190701094942-4def268fd1a4/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
 golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
-golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9 h1:psW17arqaxU48Z5kZ0CQnkZWQJsqcURM6tKiBApRjXI=
 golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
+golang.org/x/crypto v0.0.0-20200820211705-5c72a883971a h1:vclmkQCjlDX5OydZ9wv8rBCcS0QyQY66Mpf/7BZbInM=
+golang.org/x/crypto v0.0.0-20200820211705-5c72a883971a/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
 golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
 golang.org/x/exp v0.0.0-20190306152737-a1d7652674e8/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
 golang.org/x/exp v0.0.0-20200908183739-ae8ad444f925/go.mod h1:1phAWC201xIgDyaFpmDeZkgf70Q4Pd/CNqfRtVPtxNw=
@@ -503,8 +504,9 @@ golang.org/x/net v0.0.0-20200301022130-244492dfa37a/go.mod h1:z5CRVTTTmAJ677TzLL
 golang.org/x/net v0.0.0-20200324143707-d3edc9973b7e/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A=
 golang.org/x/net v0.0.0-20200520004742-59133d7f0dd7/go.mod h1:qpuaurCH72eLCgpAm/N6yyVIVM9cpaDIP3A8BGJEC5A=
 golang.org/x/net v0.0.0-20200625001655-4c5254603344/go.mod h1:/O7V0waA8r7cgGh81Ro3o1hOxt32SMVPicZroKQ2sZA=
-golang.org/x/net v0.0.0-20200707034311-ab3426394381 h1:VXak5I6aEWmAXeQjA+QSZzlgNrpq9mjcfDemuexIKsU=
 golang.org/x/net v0.0.0-20200707034311-ab3426394381/go.mod h1:/O7V0waA8r7cgGh81Ro3o1hOxt32SMVPicZroKQ2sZA=
+golang.org/x/net v0.0.0-20200822124328-c89045814202 h1:VvcQYSHwXgi7W+TpUR6A9g6Up98WAHf3f/ulnJ62IyA=
+golang.org/x/net v0.0.0-20200822124328-c89045814202/go.mod h1:/O7V0waA8r7cgGh81Ro3o1hOxt32SMVPicZroKQ2sZA=
 golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
 golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
 golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw=
diff --git a/pkg/apis/options/upstreams.go b/pkg/apis/options/upstreams.go
index 6ae87487..1977c8da 100644
--- a/pkg/apis/options/upstreams.go
+++ b/pkg/apis/options/upstreams.go
@@ -19,8 +19,22 @@ type Upstream struct {
 
 	// Path is used to map requests to the upstream server.
 	// The closest match will take precedence and all Paths must be unique.
+	// Path can also take a pattern when used with RewriteTarget.
+	// Path segments can be captured and matched using regular experessions.
+	// Eg:
+	// - `^/foo$`: Match only the explicit path `/foo`
+	// - `^/bar/$`: Match any path prefixed with `/bar/`
+	// - `^/baz/(.*)$`: Match any path prefixed with `/baz` and capture the remaining path for use with RewriteTarget
 	Path string `json:"path,omitempty"`
 
+	// RewriteTarget allows users to rewrite the request path before it is sent to
+	// the upstream server.
+	// Use the Path to capture segments for reuse within the rewrite target.
+	// Eg: With a Path of `^/baz/(.*)`, a RewriteTarget of `/foo/$1` would rewrite
+	// the request `/baz/abc/123` to `/foo/abc/123` before proxying to the
+	// upstream server.
+	RewriteTarget string `json:"rewriteTarget,omitempty"`
+
 	// The URI of the upstream server. This may be an HTTP(S) server of a File
 	// based URL. It may include a path, in which case all requests will be served
 	// under that path.
diff --git a/pkg/upstream/proxy.go b/pkg/upstream/proxy.go
index 2b0ab70e..594824e2 100644
--- a/pkg/upstream/proxy.go
+++ b/pkg/upstream/proxy.go
@@ -1,10 +1,16 @@
 package upstream
 
 import (
+	"context"
 	"fmt"
 	"net/http"
 	"net/url"
+	"regexp"
+	"sort"
+	"strings"
 
+	"github.com/gorilla/mux"
+	"github.com/justinas/alice"
 	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options"
 	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/app/pagewriter"
 	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"
@@ -18,12 +24,14 @@ type ProxyErrorHandler func(http.ResponseWriter, *http.Request, error)
 // multiple upstreams.
 func NewProxy(upstreams options.Upstreams, sigData *options.SignatureData, writer pagewriter.Writer) (http.Handler, error) {
 	m := &multiUpstreamProxy{
-		serveMux: http.NewServeMux(),
+		serveMux: mux.NewRouter(),
 	}
 
-	for _, upstream := range upstreams {
+	for _, upstream := range sortByPathLongest(upstreams) {
 		if upstream.Static {
-			m.registerStaticResponseHandler(upstream)
+			if err := m.registerStaticResponseHandler(upstream, writer); err != nil {
+				return nil, fmt.Errorf("could not register static upstream %q: %v", upstream.ID, err)
+			}
 			continue
 		}
 
@@ -33,20 +41,26 @@ func NewProxy(upstreams options.Upstreams, sigData *options.SignatureData, write
 		}
 		switch u.Scheme {
 		case fileScheme:
-			m.registerFileServer(upstream, u)
+			if err := m.registerFileServer(upstream, u, writer); err != nil {
+				return nil, fmt.Errorf("could not register file upstream %q: %v", upstream.ID, err)
+			}
 		case httpScheme, httpsScheme:
-			m.registerHTTPUpstreamProxy(upstream, u, sigData, writer)
+			if err := m.registerHTTPUpstreamProxy(upstream, u, sigData, writer); err != nil {
+				return nil, fmt.Errorf("could not register HTTP upstream %q: %v", upstream.ID, err)
+			}
 		default:
 			return nil, fmt.Errorf("unknown scheme for upstream %q: %q", upstream.ID, u.Scheme)
 		}
 	}
+
+	registerTrailingSlashHandler(m.serveMux)
 	return m, nil
 }
 
 // multiUpstreamProxy will serve requests directed to multiple upstream servers
 // registered in the serverMux.
 type multiUpstreamProxy struct {
-	serveMux *http.ServeMux
+	serveMux *mux.Router
 }
 
 // ServerHTTP handles HTTP requests.
@@ -55,19 +69,110 @@ func (m *multiUpstreamProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request
 }
 
 // registerStaticResponseHandler registers a static response handler with at the given path.
-func (m *multiUpstreamProxy) registerStaticResponseHandler(upstream options.Upstream) {
+func (m *multiUpstreamProxy) registerStaticResponseHandler(upstream options.Upstream, writer pagewriter.Writer) error {
 	logger.Printf("mapping path %q => static response %d", upstream.Path, derefStaticCode(upstream.StaticCode))
-	m.serveMux.Handle(upstream.Path, newStaticResponseHandler(upstream.ID, upstream.StaticCode))
+	return m.registerHandler(upstream, newStaticResponseHandler(upstream.ID, upstream.StaticCode), writer)
 }
 
 // registerFileServer registers a new fileServer based on the configuration given.
-func (m *multiUpstreamProxy) registerFileServer(upstream options.Upstream, u *url.URL) {
+func (m *multiUpstreamProxy) registerFileServer(upstream options.Upstream, u *url.URL, writer pagewriter.Writer) error {
 	logger.Printf("mapping path %q => file system %q", upstream.Path, u.Path)
-	m.serveMux.Handle(upstream.Path, newFileServer(upstream.ID, upstream.Path, u.Path))
+	return m.registerHandler(upstream, newFileServer(upstream.ID, upstream.Path, u.Path), writer)
 }
 
 // registerHTTPUpstreamProxy registers a new httpUpstreamProxy based on the configuration given.
-func (m *multiUpstreamProxy) registerHTTPUpstreamProxy(upstream options.Upstream, u *url.URL, sigData *options.SignatureData, writer pagewriter.Writer) {
+func (m *multiUpstreamProxy) registerHTTPUpstreamProxy(upstream options.Upstream, u *url.URL, sigData *options.SignatureData, writer pagewriter.Writer) error {
 	logger.Printf("mapping path %q => upstream %q", upstream.Path, upstream.URI)
-	m.serveMux.Handle(upstream.Path, newHTTPUpstreamProxy(upstream, u, sigData, writer.ProxyErrorHandler))
+	return m.registerHandler(upstream, newHTTPUpstreamProxy(upstream, u, sigData, writer.ProxyErrorHandler), writer)
+}
+
+// registerHandler ensures the given handler is regiestered with the serveMux.
+func (m *multiUpstreamProxy) registerHandler(upstream options.Upstream, handler http.Handler, writer pagewriter.Writer) error {
+	if upstream.RewriteTarget == "" {
+		m.registerSimpleHandler(upstream.Path, handler)
+		return nil
+	}
+
+	return m.registerRewriteHandler(upstream, handler, writer)
+}
+
+// registerSimpleHandler maintains the behaviour of the go standard serveMux
+// by ensuring any path with a trailing `/` matches all paths under that prefix.
+func (m *multiUpstreamProxy) registerSimpleHandler(path string, handler http.Handler) {
+	if strings.HasSuffix(path, "/") {
+		m.serveMux.PathPrefix(path).Handler(handler)
+	} else {
+		m.serveMux.Path(path).Handler(handler)
+	}
+}
+
+// registerRewriteHandler ensures the handler is registered for all paths
+// which match the regex defined in the Path.
+// Requests to the handler will have the request path rewritten before the
+// request is made to the next handler.
+func (m *multiUpstreamProxy) registerRewriteHandler(upstream options.Upstream, handler http.Handler, writer pagewriter.Writer) error {
+	rewriteRegExp, err := regexp.Compile(upstream.Path)
+	if err != nil {
+		return fmt.Errorf("invalid path %q for upstream: %v", upstream.Path, err)
+	}
+
+	rewrite := newRewritePath(rewriteRegExp, upstream.RewriteTarget, writer)
+	h := alice.New(rewrite).Then(handler)
+	m.serveMux.MatcherFunc(func(req *http.Request, match *mux.RouteMatch) bool {
+		return rewriteRegExp.MatchString(req.URL.Path)
+	}).Handler(h)
+
+	return nil
+}
+
+// registerTrailingSlashHandler creates a new matcher that will check if the
+// requested path would match if it had a trailing slash appended.
+// If the path matches with a trailing slash, we send back a redirect.
+// This allows us to be consistent with the built in go servemux implementation.
+func registerTrailingSlashHandler(serveMux *mux.Router) {
+	serveMux.MatcherFunc(func(req *http.Request, _ *mux.RouteMatch) bool {
+		if strings.HasSuffix(req.URL.Path, "/") {
+			return false
+		}
+
+		// Use a separate RouteMatch so that we can redirect to the path + /.
+		// If we pass through the match then the matched backed will be served
+		// instead of the redirect handler.
+		m := &mux.RouteMatch{}
+		slashReq := req.Clone(context.Background())
+		slashReq.URL.Path += "/"
+		return serveMux.Match(slashReq, m)
+	}).Handler(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
+		http.Redirect(rw, req, req.URL.String()+"/", http.StatusMovedPermanently)
+	}))
+}
+
+// sortByPathLongest ensures that the upstreams are sorted by longest path.
+// If rewrites are involved, a rewrite takes precedence over a non-rewrite.
+// When two upstreams define rewrites, whichever has the longest path will take
+// precedence (note this is the input to the rewrite logic).
+// This does not account for when a rewrite would actually make the path shorter.
+// This should maintain the sorting behaviour of the standard go serve mux.
+func sortByPathLongest(in options.Upstreams) options.Upstreams {
+	sort.Slice(in, func(i, j int) bool {
+		iRW := in[i].RewriteTarget
+		jRW := in[j].RewriteTarget
+
+		switch {
+		case iRW != "" && jRW != "":
+			// If both have a rewrite target, whichever has the longest pattern
+			// should go first
+			return len(in[i].Path) > len(in[j].Path)
+		case iRW != "" && jRW == "":
+			// Only one has rewrite, it goes first
+			return true
+		case iRW == "" && jRW != "":
+			// Only one has rewrite, it goes first
+			return false
+		default:
+			// Default to longest Path wins
+			return len(in[i].Path) > len(in[j].Path)
+		}
+	})
+	return in
 }
diff --git a/pkg/upstream/rewrite.go b/pkg/upstream/rewrite.go
new file mode 100644
index 00000000..a84e18c0
--- /dev/null
+++ b/pkg/upstream/rewrite.go
@@ -0,0 +1,80 @@
+package upstream
+
+import (
+	"fmt"
+	"net/http"
+	"net/url"
+	"regexp"
+	"strings"
+
+	"github.com/justinas/alice"
+	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware"
+	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/app/pagewriter"
+	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"
+)
+
+// newRewritePath creates a new middleware that will rewrite the request URI
+// path before handing the request to the next server.
+func newRewritePath(rewriteRegExp *regexp.Regexp, rewriteTarget string, writer pagewriter.Writer) alice.Constructor {
+	return func(next http.Handler) http.Handler {
+		return rewritePath(rewriteRegExp, rewriteTarget, writer, next)
+	}
+}
+
+// rewritePath uses the regexp to rewrite the request URI based on the provided
+// rewriteTarget.
+func rewritePath(rewriteRegExp *regexp.Regexp, rewriteTarget string, writer pagewriter.Writer, next http.Handler) http.Handler {
+	return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
+		reqURL, err := url.ParseRequestURI(req.RequestURI)
+		if err != nil {
+			logger.Errorf("could not parse request URI: %v", err)
+			writer.WriteErrorPage(rw, pagewriter.ErrorPageOpts{
+				Status:    http.StatusInternalServerError,
+				RequestID: middleware.GetRequestScope(req).RequestID,
+				AppError:  fmt.Sprintf("Could not parse request URI: %v", err),
+			})
+			return
+		}
+
+		// Use the regex to rewrite the request path before proxying to the upstream.
+		newURI := rewriteRegExp.ReplaceAllString(reqURL.Path, rewriteTarget)
+		reqURL.Path, reqURL.RawQuery, err = splitPathAndQuery(reqURL.Query(), newURI)
+		if err != nil {
+			logger.Errorf("could not parse rewrite URI: %v", err)
+			writer.WriteErrorPage(rw, pagewriter.ErrorPageOpts{
+				Status:    http.StatusInternalServerError,
+				RequestID: middleware.GetRequestScope(req).RequestID,
+				AppError:  fmt.Sprintf("Could not parse rewrite URI: %v", err),
+			})
+			return
+		}
+
+		req.RequestURI = reqURL.String()
+		next.ServeHTTP(rw, req)
+	})
+}
+
+// splitPathAndQuery splits the rewritten path into the URL Path and the URL
+// raw query. Any rewritten query values are appended to the original query
+// values.
+// This relies on the underlying URL library to encode the query string.
+// For duplicate values it appends each as a separate value, e.g. ?foo=bar&foo=baz.
+func splitPathAndQuery(originalQuery url.Values, raw string) (string, string, error) {
+	s := strings.SplitN(raw, "?", 2)
+	if len(s) == 1 {
+		return s[0], originalQuery.Encode(), nil
+	}
+
+	queryValues, err := url.ParseQuery(s[1])
+	if err != nil {
+		return "", "", nil
+	}
+
+	for key, values := range queryValues {
+		for _, value := range values {
+			originalQuery.Add(key, value)
+		}
+	}
+
+	return s[0], originalQuery.Encode(), nil
+}


Test patch:
diff --git a/pkg/upstream/proxy_test.go b/pkg/upstream/proxy_test.go
index 96b7a0dd..597f5422 100644
--- a/pkg/upstream/proxy_test.go
+++ b/pkg/upstream/proxy_test.go
@@ -18,174 +18,369 @@ import (
 var _ = Describe("Proxy Suite", func() {
 	var upstreamServer http.Handler
 
-	BeforeEach(func() {
-		sigData := &options.SignatureData{Hash: crypto.SHA256, Key: "secret"}
+	Context("multiUpstreamProxy", func() {
+		BeforeEach(func() {
+			sigData := &options.SignatureData{Hash: crypto.SHA256, Key: "secret"}
 
-		writer := &pagewriter.WriterFuncs{
-			ProxyErrorFunc: func(rw http.ResponseWriter, _ *http.Request, _ error) {
-				rw.WriteHeader(502)
-				rw.Write([]byte("Proxy Error"))
-			},
-		}
+			writer := &pagewriter.WriterFuncs{
+				ProxyErrorFunc: func(rw http.ResponseWriter, _ *http.Request, _ error) {
+					rw.WriteHeader(502)
+					rw.Write([]byte("Proxy Error"))
+				},
+			}
 
-		ok := http.StatusOK
+			ok := http.StatusOK
+			accepted := http.StatusAccepted
 
-		upstreams := options.Upstreams{
-			{
-				ID:   "http-backend",
-				Path: "/http/",
-				URI:  serverAddr,
-			},
-			{
-				ID:   "file-backend",
-				Path: "/files/",
-				URI:  fmt.Sprintf("file:///%s", filesDir),
-			},
-			{
-				ID:         "static-backend",
-				Path:       "/static/",
-				Static:     true,
-				StaticCode: &ok,
-			},
-			{
-				ID:   "bad-http-backend",
-				Path: "/bad-http/",
-				URI:  "http://::1",
-			},
-			{
-				ID:         "single-path-backend",
-				Path:       "/single-path",
-				Static:     true,
-				StaticCode: &ok,
-			},
+			upstreams := options.Upstreams{
+				{
+					ID:   "http-backend",
+					Path: "/http/",
+					URI:  serverAddr,
+				},
+				{
+					ID:   "file-backend",
+					Path: "/files/",
+					URI:  fmt.Sprintf("file:///%s", filesDir),
+				},
+				{
+					ID:         "static-backend",
+					Path:       "/static/",
+					Static:     true,
+					StaticCode: &ok,
+				},
+				{
+					ID:         "static-backend-no-trailing-slash",
+					Path:       "/static",
+					Static:     true,
+					StaticCode: &accepted,
+				},
+				{
+					ID:         "static-backend-long",
+					Path:       "/static/long",
+					Static:     true,
+					StaticCode: &accepted,
+				},
+				{
+					ID:   "bad-http-backend",
+					Path: "/bad-http/",
+					URI:  "http://::1",
+				},
+				{
+					ID:         "single-path-backend",
+					Path:       "/single-path",
+					Static:     true,
+					StaticCode: &ok,
+				},
+				{
+					ID:            "backend-with-rewrite-prefix",
+					Path:          "^/rewrite-prefix/(.*)",
+					RewriteTarget: "/different/backend/path/$1",
+					URI:           serverAddr,
+				},
+				{
+					ID:   "double-match-plain",
+					Path: "/double-match/",
+					URI:  serverAddr,
+				},
+				{
+					ID:            "double-match-rewrite",
+					Path:          "^/double-match/(.*)",
+					RewriteTarget: "/double-match/rewrite/$1",
+					URI:           serverAddr,
+				},
+			}
+
+			var err error
+			upstreamServer, err = NewProxy(upstreams, sigData, writer)
+			Expect(err).ToNot(HaveOccurred())
+		})
+
+		type proxyTableInput struct {
+			target   string
+			response testHTTPResponse
+			upstream string
 		}
 
-		var err error
-		upstreamServer, err = NewProxy(upstreams, sigData, writer)
-		Expect(err).ToNot(HaveOccurred())
-	})
+		DescribeTable("Proxy ServeHTTP",
+			func(in *proxyTableInput) {
+				req := middlewareapi.AddRequestScope(
+					httptest.NewRequest("", in.target, nil),
+					&middlewareapi.RequestScope{},
+				)
+				rw := httptest.NewRecorder()
+				// Don't mock the remote Address
+				req.RemoteAddr = ""
 
-	type proxyTableInput struct {
-		target   string
-		response testHTTPResponse
-		upstream string
-	}
-
-	DescribeTable("Proxy ServeHTTP",
-		func(in *proxyTableInput) {
-			req := middlewareapi.AddRequestScope(
-				httptest.NewRequest("", in.target, nil),
-				&middlewareapi.RequestScope{},
-			)
-			rw := httptest.NewRecorder()
-			// Don't mock the remote Address
-			req.RemoteAddr = ""
-
-			upstreamServer.ServeHTTP(rw, req)
-
-			scope := middlewareapi.GetRequestScope(req)
-			Expect(scope.Upstream).To(Equal(in.upstream))
-
-			Expect(rw.Code).To(Equal(in.response.code))
-
-			// Delete extra headers that aren't relevant to tests
-			testSanitizeResponseHeader(rw.Header())
-			Expect(rw.Header()).To(Equal(in.response.header))
-
-			body := rw.Body.Bytes()
-			// If the raw body is set, check that, else check the Request object
-			if in.response.raw != "" {
-				Expect(string(body)).To(Equal(in.response.raw))
-				return
-			}
+				upstreamServer.ServeHTTP(rw, req)
+
+				scope := middlewareapi.GetRequestScope(req)
+				Expect(scope.Upstream).To(Equal(in.upstream))
+
+				Expect(rw.Code).To(Equal(in.response.code))
+
+				// Delete extra headers that aren't relevant to tests
+				testSanitizeResponseHeader(rw.Header())
+				Expect(rw.Header()).To(Equal(in.response.header))
+
+				body := rw.Body.Bytes()
+				// If the raw body is set, check that, else check the Request object
+				if in.response.raw != "" {
+					Expect(string(body)).To(Equal(in.response.raw))
+					return
+				}
 
-			// Compare the reflected request to the upstream
-			request := testHTTPRequest{}
-			Expect(json.Unmarshal(body, &request)).To(Succeed())
-			testSanitizeRequestHeader(request.Header)
-			Expect(request).To(Equal(in.response.request))
-		},
-		Entry("with a request to the HTTP service", &proxyTableInput{
-			target: "http://example.localhost/http/1234",
-			response: testHTTPResponse{
-				code: 200,
-				header: map[string][]string{
-					contentType: {applicationJSON},
-				},
-				request: testHTTPRequest{
-					Method: "GET",
-					URL:    "http://example.localhost/http/1234",
-					Header: map[string][]string{
-						"Gap-Auth":      {""},
-						"Gap-Signature": {"sha256 ofB1u6+FhEUbFLc3/uGbJVkl7GaN4egFqVvyO3+2I1w="},
+				// Compare the reflected request to the upstream
+				request := testHTTPRequest{}
+				Expect(json.Unmarshal(body, &request)).To(Succeed())
+				testSanitizeRequestHeader(request.Header)
+				Expect(request).To(Equal(in.response.request))
+			},
+			Entry("with a request to the HTTP service", &proxyTableInput{
+				target: "http://example.localhost/http/1234",
+				response: testHTTPResponse{
+					code: 200,
+					header: map[string][]string{
+						contentType: {applicationJSON},
+					},
+					request: testHTTPRequest{
+						Method: "GET",
+						URL:    "http://example.localhost/http/1234",
+						Header: map[string][]string{
+							"Gap-Auth":      {""},
+							"Gap-Signature": {"sha256 ofB1u6+FhEUbFLc3/uGbJVkl7GaN4egFqVvyO3+2I1w="},
+						},
+						Body:       []byte{},
+						Host:       "example.localhost",
+						RequestURI: "http://example.localhost/http/1234",
 					},
-					Body:       []byte{},
-					Host:       "example.localhost",
-					RequestURI: "http://example.localhost/http/1234",
 				},
+				upstream: "http-backend",
+			}),
+			Entry("with a request to the File backend", &proxyTableInput{
+				target: "http://example.localhost/files/foo",
+				response: testHTTPResponse{
+					code: 200,
+					header: map[string][]string{
+						contentType: {textPlainUTF8},
+					},
+					raw: "foo",
+				},
+				upstream: "file-backend",
+			}),
+			Entry("with a request to the Static backend", &proxyTableInput{
+				target: "http://example.localhost/static/bar",
+				response: testHTTPResponse{
+					code:   200,
+					header: map[string][]string{},
+					raw:    "Authenticated",
+				},
+				upstream: "static-backend",
+			}),
+			Entry("with a request to the bad HTTP backend", &proxyTableInput{
+				target: "http://example.localhost/bad-http/bad",
+				response: testHTTPResponse{
+					code:   502,
+					header: map[string][]string{},
+					// This tests the error handler
+					raw: "Proxy Error",
+				},
+				upstream: "bad-http-backend",
+			}),
+			Entry("with a request to the to an unregistered path", &proxyTableInput{
+				target: "http://example.localhost/unregistered",
+				response: testHTTPResponse{
+					code: 404,
+					header: map[string][]string{
+						"X-Content-Type-Options": {"nosniff"},
+						contentType:              {textPlainUTF8},
+					},
+					raw: "404 page not found\n",
+				},
+			}),
+			Entry("with a request to the to backend registered to a single path", &proxyTableInput{
+				target: "http://example.localhost/single-path",
+				response: testHTTPResponse{
+					code:   200,
+					header: map[string][]string{},
+					raw:    "Authenticated",
+				},
+				upstream: "single-path-backend",
+			}),
+			Entry("with a request to the to a subpath of a backend registered to a single path", &proxyTableInput{
+				target: "http://example.localhost/single-path/unregistered",
+				response: testHTTPResponse{
+					code: 404,
+					header: map[string][]string{
+						"X-Content-Type-Options": {"nosniff"},
+						contentType:              {textPlainUTF8},
+					},
+					raw: "404 page not found\n",
+				},
+			}),
+			Entry("with a request to the rewrite prefix server", &proxyTableInput{
+				target: "http://example.localhost/rewrite-prefix/1234",
+				response: testHTTPResponse{
+					code: 200,
+					header: map[string][]string{
+						contentType: {applicationJSON},
+					},
+					request: testHTTPRequest{
+						Method: "GET",
+						URL:    "http://example.localhost/different/backend/path/1234",
+						Header: map[string][]string{
+							"Gap-Auth":      {""},
+							"Gap-Signature": {"sha256 jeAeM7wHSj2ab/l9YPvtTJ9l/8q1tpY2V/iwXF48bgw="},
+						},
+						Body:       []byte{},
+						Host:       "example.localhost",
+						RequestURI: "http://example.localhost/different/backend/path/1234",
+					},
+				},
+				upstream: "backend-with-rewrite-prefix",
+			}),
+			Entry("with a request to a subpath of the rewrite prefix server", &proxyTableInput{
+				target: "http://example.localhost/rewrite-prefix/1234/abc",
+				response: testHTTPResponse{
+					code: 200,
+					header: map[string][]string{
+						contentType: {applicationJSON},
+					},
+					request: testHTTPRequest{
+						Method: "GET",
+						URL:    "http://example.localhost/different/backend/path/1234/abc",
+						Header: map[string][]string{
+							"Gap-Auth":      {""},
+							"Gap-Signature": {"sha256 rAkAc9gp7EndoOppJuvbuPnYuBcqrTkBnQx6iPS8xTA="},
+						},
+						Body:       []byte{},
+						Host:       "example.localhost",
+						RequestURI: "http://example.localhost/different/backend/path/1234/abc",
+					},
+				},
+				upstream: "backend-with-rewrite-prefix",
+			}),
+			Entry("with a request to a path, missing the trailing slash", &proxyTableInput{
+				target: "http://example.localhost/http",
+				response: testHTTPResponse{
+					code: 301,
+					header: map[string][]string{
+						contentType: {textHTMLUTF8},
+						"Location":  {"http://example.localhost/http/"},
+					},
+					raw: "<a href=\"http://example.localhost/http/\">Moved Permanently</a>.\n\n",
+				},
+			}),
+			Entry("with a request to a path, missing the trailing slash, but registered separately", &proxyTableInput{
+				target: "http://example.localhost/static",
+				response: testHTTPResponse{
+					code:   202,
+					header: map[string][]string{},
+					raw:    "Authenticated",
+				},
+				upstream: "static-backend-no-trailing-slash",
+			}),
+			Entry("should match longest path first", &proxyTableInput{
+				target: "http://example.localhost/static/long",
+				response: testHTTPResponse{
+					code:   202,
+					header: map[string][]string{},
+					raw:    "Authenticated",
+				},
+				upstream: "static-backend-long",
+			}),
+			Entry("should match rewrite path first", &proxyTableInput{
+				target: "http://example.localhost/double-match/foo",
+				response: testHTTPResponse{
+					code: 200,
+					header: map[string][]string{
+						contentType: {applicationJSON},
+					},
+					request: testHTTPRequest{
+						Method: "GET",
+						URL:    "http://example.localhost/double-match/rewrite/foo",
+						Header: map[string][]string{
+							"Gap-Auth":      {""},
+							"Gap-Signature": {"sha256 eYyUNdsrTmnvFpavpP8AdHGUGzqJ39QEjqn0/3fQPHA="},
+						},
+						Body:       []byte{},
+						Host:       "example.localhost",
+						RequestURI: "http://example.localhost/double-match/rewrite/foo",
+					},
+				},
+				upstream: "double-match-rewrite",
+			}),
+		)
+	})
+
+	Context("sortByPathLongest", func() {
+		type sortByPathLongestTableInput struct {
+			input          options.Upstreams
+			expectedOutput options.Upstreams
+		}
+
+		var httpPath = options.Upstream{
+			Path: "/http/",
+		}
+
+		var httpSubPath = options.Upstream{
+			Path: "/http/subpath/",
+		}
+
+		var longerPath = options.Upstream{
+			Path: "/longer-than-http",
+		}
+
+		var shortPathWithRewrite = options.Upstream{
+			Path:          "^/h/(.*)",
+			RewriteTarget: "/$1",
+		}
+
+		var shortSubPathWithRewrite = options.Upstream{
+			Path:          "^/h/bar/(.*)",
+			RewriteTarget: "/$1",
+		}
+
+		DescribeTable("short sort into the correct order",
+			func(in sortByPathLongestTableInput) {
+				Expect(sortByPathLongest(in.input)).To(Equal(in.expectedOutput))
 			},
-			upstream: "http-backend",
-		}),
-		Entry("with a request to the File backend", &proxyTableInput{
-			target: "http://example.localhost/files/foo",
-			response: testHTTPResponse{
-				code: 200,
-				header: map[string][]string{
-					contentType: {textPlainUTF8},
-				},
-				raw: "foo",
-			},
-			upstream: "file-backend",
-		}),
-		Entry("with a request to the Static backend", &proxyTableInput{
-			target: "http://example.localhost/static/bar",
-			response: testHTTPResponse{
-				code:   200,
-				header: map[string][]string{},
-				raw:    "Authenticated",
-			},
-			upstream: "static-backend",
-		}),
-		Entry("with a request to the bad HTTP backend", &proxyTableInput{
-			target: "http://example.localhost/bad-http/bad",
-			response: testHTTPResponse{
-				code:   502,
-				header: map[string][]string{},
-				// This tests the error handler
-				raw: "Proxy Error",
-			},
-			upstream: "bad-http-backend",
-		}),
-		Entry("with a request to the to an unregistered path", &proxyTableInput{
-			target: "http://example.localhost/unregistered",
-			response: testHTTPResponse{
-				code: 404,
-				header: map[string][]string{
-					"X-Content-Type-Options": {"nosniff"},
-					contentType:              {textPlainUTF8},
-				},
-				raw: "404 page not found\n",
-			},
-		}),
-		Entry("with a request to the to backend registered to a single path", &proxyTableInput{
-			target: "http://example.localhost/single-path",
-			response: testHTTPResponse{
-				code:   200,
-				header: map[string][]string{},
-				raw:    "Authenticated",
-			},
-			upstream: "single-path-backend",
-		}),
-		Entry("with a request to the to a subpath of a backend registered to a single path", &proxyTableInput{
-			target: "http://example.localhost/single-path/unregistered",
-			response: testHTTPResponse{
-				code: 404,
-				header: map[string][]string{
-					"X-Content-Type-Options": {"nosniff"},
-					contentType:              {textPlainUTF8},
-				},
-				raw: "404 page not found\n",
-			},
-		}),
-	)
+			Entry("with a mix of paths registered", sortByPathLongestTableInput{
+				input:          options.Upstreams{httpPath, httpSubPath, shortSubPathWithRewrite, longerPath, shortPathWithRewrite},
+				expectedOutput: options.Upstreams{shortSubPathWithRewrite, shortPathWithRewrite, longerPath, httpSubPath, httpPath},
+			}),
+			Entry("when a subpath is registered (in order)", sortByPathLongestTableInput{
+				input:          options.Upstreams{httpSubPath, httpPath},
+				expectedOutput: options.Upstreams{httpSubPath, httpPath},
+			}),
+			Entry("when a subpath is registered (out of order)", sortByPathLongestTableInput{
+				input:          options.Upstreams{httpPath, httpSubPath},
+				expectedOutput: options.Upstreams{httpSubPath, httpPath},
+			}),
+			Entry("when longer paths are registered (in order)", sortByPathLongestTableInput{
+				input:          options.Upstreams{longerPath, httpPath},
+				expectedOutput: options.Upstreams{longerPath, httpPath},
+			}),
+			Entry("when longer paths are registered (out of order)", sortByPathLongestTableInput{
+				input:          options.Upstreams{httpPath, longerPath},
+				expectedOutput: options.Upstreams{longerPath, httpPath},
+			}),
+			Entry("when a rewrite target is registered (in order)", sortByPathLongestTableInput{
+				input:          options.Upstreams{shortPathWithRewrite, longerPath},
+				expectedOutput: options.Upstreams{shortPathWithRewrite, longerPath},
+			}),
+			Entry("when a rewrite target is registered (out of order)", sortByPathLongestTableInput{
+				input:          options.Upstreams{longerPath, shortPathWithRewrite},
+				expectedOutput: options.Upstreams{shortPathWithRewrite, longerPath},
+			}),
+			Entry("with multiple rewrite targets registered (in order)", sortByPathLongestTableInput{
+				input:          options.Upstreams{shortSubPathWithRewrite, shortPathWithRewrite},
+				expectedOutput: options.Upstreams{shortSubPathWithRewrite, shortPathWithRewrite},
+			}),
+			Entry("with multiple rewrite targets registered (out of order)", sortByPathLongestTableInput{
+				input:          options.Upstreams{shortPathWithRewrite, shortSubPathWithRewrite},
+				expectedOutput: options.Upstreams{shortSubPathWithRewrite, shortPathWithRewrite},
+			}),
+		)
+	})
 })
diff --git a/pkg/upstream/rewrite_test.go b/pkg/upstream/rewrite_test.go
new file mode 100644
index 00000000..57a0f244
--- /dev/null
+++ b/pkg/upstream/rewrite_test.go
@@ -0,0 +1,60 @@
+package upstream
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"regexp"
+
+	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/app/pagewriter"
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/ginkgo/extensions/table"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("Rewrite", func() {
+	type rewritePathTableInput struct {
+		rewriteRegex       *regexp.Regexp
+		rewriteTarget      string
+		requestTarget      string
+		expectedRequestURI string
+	}
+
+	DescribeTable("should rewrite the request path",
+		func(in rewritePathTableInput) {
+			req := httptest.NewRequest("", in.requestTarget, nil)
+			rw := httptest.NewRecorder()
+
+			var gotRequestURI string
+			handler := newRewritePath(in.rewriteRegex, in.rewriteTarget, &pagewriter.WriterFuncs{})(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				gotRequestURI = r.RequestURI
+			}))
+			handler.ServeHTTP(rw, req)
+
+			Expect(gotRequestURI).To(Equal(in.expectedRequestURI))
+		},
+		Entry("when the path matches the regexp", rewritePathTableInput{
+			rewriteRegex:       regexp.MustCompile("^/http/(.*)"),
+			rewriteTarget:      "/$1",
+			requestTarget:      "http://example.com/http/foo/bar",
+			expectedRequestURI: "http://example.com/foo/bar",
+		}),
+		Entry("when the path does not match the regexp", rewritePathTableInput{
+			rewriteRegex:       regexp.MustCompile("^/http/(.*)"),
+			rewriteTarget:      "/$1",
+			requestTarget:      "https://example.com/https/foo/bar",
+			expectedRequestURI: "https://example.com/https/foo/bar",
+		}),
+		Entry("when the regexp is not anchored", rewritePathTableInput{
+			rewriteRegex:       regexp.MustCompile("/http/(.*)"),
+			rewriteTarget:      "/$1",
+			requestTarget:      "http://example.com/bar/http/foo/bar",
+			expectedRequestURI: "http://example.com/bar/foo/bar",
+		}),
+		Entry("when the regexp is rewriting to a query", rewritePathTableInput{
+			rewriteRegex:       regexp.MustCompile(`/articles/([a-z0-9\-]*)`),
+			rewriteTarget:      "/article?id=$1",
+			requestTarget:      "http://example.com/articles/blog-2021-01-01",
+			expectedRequestURI: "http://example.com/article?id=blog-2021-01-01",
+		}),
+	)
+})
diff --git a/pkg/upstream/upstream_suite_test.go b/pkg/upstream/upstream_suite_test.go
index 5e9b9e32..e91da077 100644
--- a/pkg/upstream/upstream_suite_test.go
+++ b/pkg/upstream/upstream_suite_test.go
@@ -59,6 +59,7 @@ const (
 	acceptEncoding  = "Accept-Encoding"
 	applicationJSON = "application/json"
 	textPlainUTF8   = "text/plain; charset=utf-8"
+	textHTMLUTF8    = "text/html; charset=utf-8"
 	gapAuth         = "Gap-Auth"
 	gapSignature    = "Gap-Signature"
 )