From b618ed7150ce84e7a9811b4304fe665650e49729 Mon Sep 17 00:00:00 2001 From: Ian Roberts Date: Thu, 21 Sep 2023 14:55:05 +0100 Subject: [PATCH 1/4] Test for a file:/// upstream combined with regex path rewrite --- pkg/upstream/proxy_test.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/pkg/upstream/proxy_test.go b/pkg/upstream/proxy_test.go index b685eae0..516d1a83 100644 --- a/pkg/upstream/proxy_test.go +++ b/pkg/upstream/proxy_test.go @@ -51,6 +51,12 @@ var _ = Describe("Proxy Suite", func() { Path: "/files/", URI: fmt.Sprintf("file:///%s", filesDir), }, + { + ID: "rewrite-file-backend", + Path: "^/rewrite-files/.*/(.*)$", + RewriteTarget: "/$1", + URI: fmt.Sprintf("file:///%s", filesDir), + }, { ID: "static-backend", Path: "/static/", @@ -174,6 +180,17 @@ var _ = Describe("Proxy Suite", func() { }, upstream: "file-backend", }), + Entry("with a request to the File backend with rewrite", &proxyTableInput{ + target: "http://example.localhost/rewrite-files/anything-at-all/foo", + response: testHTTPResponse{ + code: 200, + header: map[string][]string{ + contentType: {textPlainUTF8}, + }, + raw: "foo", + }, + upstream: "rewrite-file-backend", + }), Entry("with a request to the Static backend", &proxyTableInput{ target: "http://example.localhost/static/bar", response: testHTTPResponse{ From cb53401c3af540942a2dc4b13744d28ccf9e88b6 Mon Sep 17 00:00:00 2001 From: Ian Roberts Date: Thu, 21 Sep 2023 17:34:51 +0100 Subject: [PATCH 2/4] Don't use http.StripPrefix when a file: upstream has rewriteTarget A regular (non-regex) file: upstream needs to strip the prefix so that it is equivalent to "mounting" the specified directory under the configured path in the URL space, but with regex rewriting the target path is determined by the rewriteTarget. Fixes oauth2-proxy/oauth2-proxy#2242 --- pkg/upstream/file.go | 44 ++++++++++++++++++++++++++++++++++----- pkg/upstream/file_test.go | 8 ++++++- pkg/upstream/proxy.go | 2 +- 3 files changed, 47 insertions(+), 7 deletions(-) diff --git a/pkg/upstream/file.go b/pkg/upstream/file.go index 26b1c0b9..b88a855d 100644 --- a/pkg/upstream/file.go +++ b/pkg/upstream/file.go @@ -2,9 +2,12 @@ package upstream import ( "net/http" + "net/url" "runtime" "strings" + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options" + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware" ) @@ -12,22 +15,53 @@ const fileScheme = "file" // newFileServer creates a new fileServer that can serve requests // to a file system location. -func newFileServer(id, path, fileSystemPath string) http.Handler { +func newFileServer(upstream options.Upstream, fileSystemPath string) http.Handler { + handler := newFileServerForPath(fileSystemPath) + + if upstream.RewriteTarget == "" { + // if the upstream does not have a rewrite target, strip off the Path prefix + // (so e.g. a request for /static/some-file.html looks for some-file.html + // relative to the fileSystemPath rather than static/some-file.html). + handler = http.StripPrefix(upstream.Path, handler) + } else { + // if the upstream *does* have a rewrite target then that means the target + // path relative to the fileSystemPath will be the one in the (rewritten) + // RequestURI. + handler = requestURIToURL(handler) + } return &fileServer{ - upstream: id, - handler: newFileServerForPath(path, fileSystemPath), + upstream: upstream.ID, + handler: handler, } } // newFileServerForPath creates a http.Handler to serve files from the filesystem -func newFileServerForPath(path string, filesystemPath string) http.Handler { +func newFileServerForPath(filesystemPath string) http.Handler { // Windows fileSSystemPath will be be prefixed with `/`, eg`/C:/..., // if they were parsed by url.Parse` if runtime.GOOS == "windows" { filesystemPath = strings.TrimPrefix(filesystemPath, "/") } - return http.StripPrefix(path, http.FileServer(http.Dir(filesystemPath))) + return http.FileServer(http.Dir(filesystemPath)) +} + +// requestURIToURL returns a Handler that replaces the URL in its request with +// the result of parsing req.RequestURI. This is necessary for file handlers +// that have a rewrite target, since http.FileServer uses req.URL.Path when +// looking for the target file, but the rewrite handler only updates the +// RequestURI, leaving the original path in the URL. +func requestURIToURL(handler http.Handler) http.Handler { + return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + reqURL, err := url.ParseRequestURI(req.RequestURI) + if err != nil { + http.Error(rw, "500 Internal Server Error", http.StatusInternalServerError) + return + } + + req.URL = reqURL + handler.ServeHTTP(rw, req) + }) } // fileServer represents a single filesystem upstream proxy diff --git a/pkg/upstream/file_test.go b/pkg/upstream/file_test.go index 307ee24b..9763f6c6 100644 --- a/pkg/upstream/file_test.go +++ b/pkg/upstream/file_test.go @@ -7,6 +7,8 @@ import ( "net/http/httptest" "os" + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options" + middlewareapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -31,8 +33,12 @@ var _ = Describe("File Server Suite", func() { _, err := io.ReadFull(rand.Reader, idBytes) Expect(err).ToNot(HaveOccurred()) id = string(idBytes) + upstream := options.Upstream{ + ID: id, + Path: "/files", + } - handler = newFileServer(id, "/files", filesDir) + handler = newFileServer(upstream, filesDir) }) AfterEach(func() { diff --git a/pkg/upstream/proxy.go b/pkg/upstream/proxy.go index 8eaefc1a..74b0d02d 100644 --- a/pkg/upstream/proxy.go +++ b/pkg/upstream/proxy.go @@ -81,7 +81,7 @@ func (m *multiUpstreamProxy) registerStaticResponseHandler(upstream options.Upst // registerFileServer registers a new fileServer based on the configuration given. 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) - return m.registerHandler(upstream, newFileServer(upstream.ID, upstream.Path, u.Path), writer) + return m.registerHandler(upstream, newFileServer(upstream, u.Path), writer) } // registerHTTPUpstreamProxy registers a new httpUpstreamProxy based on the configuration given. From 16f032bce9e3cb54cf9bad8593dff2bec9ccda03 Mon Sep 17 00:00:00 2001 From: Ian Roberts Date: Thu, 21 Sep 2023 17:45:29 +0100 Subject: [PATCH 3/4] Clarify what rewriteTarget means for a file: upstream --- docs/docs/configuration/alpha_config.md | 2 +- pkg/apis/options/upstreams.go | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/docs/docs/configuration/alpha_config.md b/docs/docs/configuration/alpha_config.md index 5bbf891d..fc7c384a 100644 --- a/docs/docs/configuration/alpha_config.md +++ b/docs/docs/configuration/alpha_config.md @@ -524,7 +524,7 @@ Requests will be proxied to this upstream if the path matches the request path. | ----- | ---- | ----------- | | `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 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. | +| `rewriteTarget` | _string_ | RewriteTarget allows users to rewrite the request path before it is sent to
the upstream server (for an HTTP/HTTPS upstream) or mapped to the filesystem
(for a `file:` upstream).
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. Or if the upstream were `file:///app`, a request for
`/baz/info.html` would return the contents of the file `/app/foo/info.html`. | | `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
between OAuth2 Proxy and the upstream 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/pkg/apis/options/upstreams.go b/pkg/apis/options/upstreams.go index dab4f62b..971d151e 100644 --- a/pkg/apis/options/upstreams.go +++ b/pkg/apis/options/upstreams.go @@ -39,11 +39,13 @@ type Upstream struct { Path string `json:"path,omitempty"` // RewriteTarget allows users to rewrite the request path before it is sent to - // the upstream server. + // the upstream server (for an HTTP/HTTPS upstream) or mapped to the filesystem + // (for a `file:` upstream). // 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. + // upstream server. Or if the upstream were `file:///app`, a request for + // `/baz/info.html` would return the contents of the file `/app/foo/info.html`. RewriteTarget string `json:"rewriteTarget,omitempty"` // The URI of the upstream server. This may be an HTTP(S) server of a File From 51c65c9e79cd01ac13c575d388a34c4482518d6d Mon Sep 17 00:00:00 2001 From: Ian Roberts Date: Mon, 12 Aug 2024 18:31:21 +0100 Subject: [PATCH 4/4] docs: added changelog entry for file upstream rewriteTarget --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 36993d20..ca8b57c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ - [#2459](https://github.com/oauth2-proxy/oauth2-proxy/pull/2459) chore(deps): Updated to ginkgo v2 (@kvanzuijlen, @tuunit) - [#2112](https://github.com/oauth2-proxy/oauth2-proxy/pull/2112) docs: update list of providers which support refresh tokens (@mikefab-msf) - [#2734](https://github.com/oauth2-proxy/oauth2-proxy/pull/2734) Added s390x architecture option support (@priby05) +- [#2589](https://github.com/oauth2-proxy/oauth2-proxy/pull/2589) Added support for regex path matching and rewriting when using a static `file:` upstream (@ianroberts) # V7.6.0