From d2d62bb45293fc5690355e80d55b841c041427bf Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Mon, 22 Feb 2021 15:55:08 +0000 Subject: [PATCH 1/5] Replace standard serve mux with gorilla mux --- go.mod | 2 +- pkg/upstream/proxy.go | 22 +++++++++++++++++----- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index df007c82..c5da2c7d 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 diff --git a/pkg/upstream/proxy.go b/pkg/upstream/proxy.go index 2b0ab70e..e4b22bff 100644 --- a/pkg/upstream/proxy.go +++ b/pkg/upstream/proxy.go @@ -4,7 +4,9 @@ import ( "fmt" "net/http" "net/url" + "strings" + "github.com/gorilla/mux" "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,7 +20,7 @@ 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 { @@ -46,7 +48,7 @@ func NewProxy(upstreams options.Upstreams, sigData *options.SignatureData, write // 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. @@ -57,17 +59,27 @@ 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) { logger.Printf("mapping path %q => static response %d", upstream.Path, derefStaticCode(upstream.StaticCode)) - m.serveMux.Handle(upstream.Path, newStaticResponseHandler(upstream.ID, upstream.StaticCode)) + m.registerSimpleHandler(upstream.Path, newStaticResponseHandler(upstream.ID, upstream.StaticCode)) } // registerFileServer registers a new fileServer based on the configuration given. func (m *multiUpstreamProxy) registerFileServer(upstream options.Upstream, u *url.URL) { logger.Printf("mapping path %q => file system %q", upstream.Path, u.Path) - m.serveMux.Handle(upstream.Path, newFileServer(upstream.ID, upstream.Path, u.Path)) + m.registerSimpleHandler(upstream.Path, newFileServer(upstream.ID, upstream.Path, u.Path)) } // 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) { logger.Printf("mapping path %q => upstream %q", upstream.Path, upstream.URI) - m.serveMux.Handle(upstream.Path, newHTTPUpstreamProxy(upstream, u, sigData, writer.ProxyErrorHandler)) + m.registerSimpleHandler(upstream.Path, newHTTPUpstreamProxy(upstream, u, sigData, writer.ProxyErrorHandler)) +} + +// 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) + } } From 6c62b25bf13f245fb42db465ae1cd872e8f8cb38 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Mon, 22 Feb 2021 15:56:37 +0000 Subject: [PATCH 2/5] Allow request paths to be rewritten before proxying to upstream server --- docs/docs/configuration/alpha_config.md | 3 +- go.mod | 4 +- go.sum | 6 +- pkg/apis/options/upstreams.go | 14 +++++ pkg/upstream/proxy.go | 55 ++++++++++++++--- pkg/upstream/proxy_test.go | 48 +++++++++++++++ pkg/upstream/rewrite.go | 80 +++++++++++++++++++++++++ pkg/upstream/rewrite_test.go | 60 +++++++++++++++++++ 8 files changed, 256 insertions(+), 14 deletions(-) create mode 100644 pkg/upstream/rewrite.go create mode 100644 pkg/upstream/rewrite_test.go 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.
This value is required for all upstreams. | -| `path` | _string_ | Path is used to map requests to the upstream server.
The closest match will take precedence and all Paths must be unique. | +| `path` | _string_ | 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 | +| `rewriteTarget` | _string_ | 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. | | `uri` | _string_ | 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.
Eg:
- http://localhost:8080
- https://service.localhost
- https://service.localhost/path
- file://host/path
If the URI's path is "/base" and the incoming request was for "/dir",
the upstream request will be for "/base/dir". | | `insecureSkipTLSVerify` | _bool_ | InsecureSkipTLSVerify will skip TLS verification of upstream HTTPS hosts.
This option is insecure and will allow potential Man-In-The-Middle attacks
betweem OAuth2 Proxy and the usptream server.
Defaults to false. | | `static` | _bool_ | Static will make all requests to this upstream have a static response.
The response will have a body of "Authenticated" and a response code
matching StaticCode.
If StaticCode is not set, the response will return a 200 response. | diff --git a/go.mod b/go.mod index c5da2c7d..b3f328e0 100644 --- a/go.mod +++ b/go.mod @@ -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 e4b22bff..9b6246c8 100644 --- a/pkg/upstream/proxy.go +++ b/pkg/upstream/proxy.go @@ -4,9 +4,11 @@ import ( "fmt" "net/http" "net/url" + "regexp" "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" @@ -25,7 +27,9 @@ func NewProxy(upstreams options.Upstreams, sigData *options.SignatureData, write for _, upstream := range 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 } @@ -35,9 +39,13 @@ 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) } @@ -57,21 +65,31 @@ 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.registerSimpleHandler(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.registerSimpleHandler(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.registerSimpleHandler(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 @@ -83,3 +101,22 @@ func (m *multiUpstreamProxy) registerSimpleHandler(path string, handler http.Han 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 +} diff --git a/pkg/upstream/proxy_test.go b/pkg/upstream/proxy_test.go index 96b7a0dd..56570b14 100644 --- a/pkg/upstream/proxy_test.go +++ b/pkg/upstream/proxy_test.go @@ -58,6 +58,12 @@ var _ = Describe("Proxy Suite", func() { Static: true, StaticCode: &ok, }, + { + ID: "backend-with-rewrite-prefix", + Path: "^/rewrite-prefix/(.*)", + RewriteTarget: "/different/backend/path/$1", + URI: serverAddr, + }, } var err error @@ -187,5 +193,47 @@ var _ = Describe("Proxy Suite", func() { 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", + }), ) }) 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 +} 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", + }), + ) +}) From 8a06779d41e6cd23b6c5944739428e43984ba954 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Sun, 7 Mar 2021 12:42:10 +0000 Subject: [PATCH 3/5] Redirect request if it would match with an appended trailing slash --- pkg/upstream/proxy.go | 25 +++++++++++++++++++++++++ pkg/upstream/proxy_test.go | 27 +++++++++++++++++++++++++++ pkg/upstream/upstream_suite_test.go | 1 + 3 files changed, 53 insertions(+) diff --git a/pkg/upstream/proxy.go b/pkg/upstream/proxy.go index 9b6246c8..3907eb0a 100644 --- a/pkg/upstream/proxy.go +++ b/pkg/upstream/proxy.go @@ -1,6 +1,7 @@ package upstream import ( + "context" "fmt" "net/http" "net/url" @@ -50,6 +51,8 @@ func NewProxy(upstreams options.Upstreams, sigData *options.SignatureData, write return nil, fmt.Errorf("unknown scheme for upstream %q: %q", upstream.ID, u.Scheme) } } + + registerTrailingSlashHandler(m.serveMux) return m, nil } @@ -120,3 +123,25 @@ func (m *multiUpstreamProxy) registerRewriteHandler(upstream options.Upstream, 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) + })) +} diff --git a/pkg/upstream/proxy_test.go b/pkg/upstream/proxy_test.go index 56570b14..5ac9842c 100644 --- a/pkg/upstream/proxy_test.go +++ b/pkg/upstream/proxy_test.go @@ -29,6 +29,7 @@ var _ = Describe("Proxy Suite", func() { } ok := http.StatusOK + accepted := http.StatusAccepted upstreams := options.Upstreams{ { @@ -47,6 +48,12 @@ var _ = Describe("Proxy Suite", func() { Static: true, StaticCode: &ok, }, + { + ID: "static-backend-no-trailing-slash", + Path: "/static", + Static: true, + StaticCode: &accepted, + }, { ID: "bad-http-backend", Path: "/bad-http/", @@ -235,5 +242,25 @@ var _ = Describe("Proxy Suite", func() { }, 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: "Moved Permanently.\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", + }), ) }) 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" ) From 075cb9c3a0411ad56110766ee869e68e4b67d6b5 Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Sun, 7 Mar 2021 14:51:38 +0000 Subject: [PATCH 4/5] Ensure upstreams are sorted by longest first --- pkg/upstream/proxy.go | 33 ++- pkg/upstream/proxy_test.go | 588 ++++++++++++++++++++++--------------- 2 files changed, 386 insertions(+), 235 deletions(-) diff --git a/pkg/upstream/proxy.go b/pkg/upstream/proxy.go index 3907eb0a..594824e2 100644 --- a/pkg/upstream/proxy.go +++ b/pkg/upstream/proxy.go @@ -6,6 +6,7 @@ import ( "net/http" "net/url" "regexp" + "sort" "strings" "github.com/gorilla/mux" @@ -26,7 +27,7 @@ func NewProxy(upstreams options.Upstreams, sigData *options.SignatureData, write serveMux: mux.NewRouter(), } - for _, upstream := range upstreams { + for _, upstream := range sortByPathLongest(upstreams) { if upstream.Static { if err := m.registerStaticResponseHandler(upstream, writer); err != nil { return nil, fmt.Errorf("could not register static upstream %q: %v", upstream.ID, err) @@ -145,3 +146,33 @@ func registerTrailingSlashHandler(serveMux *mux.Router) { 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/proxy_test.go b/pkg/upstream/proxy_test.go index 5ac9842c..597f5422 100644 --- a/pkg/upstream/proxy_test.go +++ b/pkg/upstream/proxy_test.go @@ -18,249 +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")) - }, - } - - 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: "static-backend-no-trailing-slash", - Path: "/static", - 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, - }, - } - - var err error - upstreamServer, err = NewProxy(upstreams, sigData, writer) - Expect(err).ToNot(HaveOccurred()) - }) - - 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 + writer := &pagewriter.WriterFuncs{ + ProxyErrorFunc: func(rw http.ResponseWriter, _ *http.Request, _ error) { + rw.WriteHeader(502) + rw.Write([]byte("Proxy Error")) + }, } - // 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}, + ok := http.StatusOK + accepted := http.StatusAccepted + + upstreams := options.Upstreams{ + { + ID: "http-backend", + Path: "/http/", + URI: serverAddr, }, - request: testHTTPRequest{ - Method: "GET", - URL: "http://example.localhost/http/1234", - Header: map[string][]string{ - "Gap-Auth": {""}, - "Gap-Signature": {"sha256 ofB1u6+FhEUbFLc3/uGbJVkl7GaN4egFqVvyO3+2I1w="}, + { + 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 + } + + 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 + } + + // 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}, }, - 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="}, + 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/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="}, + 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}, }, - Body: []byte{}, - Host: "example.localhost", - RequestURI: "http://example.localhost/different/backend/path/1234/abc", + raw: "foo", }, - }, - 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/"}, + 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", }, - raw: "Moved Permanently.\n\n", + 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: "Moved Permanently.\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)) }, - }), - 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("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}, + }), + ) + }) }) From 9ce962be08846659d0d8b7cb0c393391769e38aa Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Fri, 2 Apr 2021 12:06:07 +0100 Subject: [PATCH 5/5] Add changelog entry for new rewrite target feature --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) 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)