From cb53401c3af540942a2dc4b13744d28ccf9e88b6 Mon Sep 17 00:00:00 2001
From: Ian Roberts <i.roberts@dcs.shef.ac.uk>
Date: Thu, 21 Sep 2023 17:34:51 +0100
Subject: [PATCH] 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.