Server: Use canonical host for AutoTLS HTTP redirects

Signed-off-by: Michael Mayer <michael@photoprism.app>

internal/server/start.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"net"
 	"net/http"
+	"net/url"
 	"os"
 	"time"
 
@@ -279,7 +280,9 @@ func StartAutoTLS(s *http.Server, m *autocert.Manager, conf *config.Config) {
 	var g errgroup.Group
 
 	g.Go(func() error {
-		redirectSrv := newHTTPServer(m.HTTPHandler(http.HandlerFunc(redirect)), conf)
+		redirectSrv := newHTTPServer(m.HTTPHandler(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
+			redirect(w, req, conf)
+		})), conf)
 		redirectSrv.Addr = fmt.Sprintf("%s:%d", conf.HttpHost(), conf.HttpPort())
 
 		return redirectSrv.ListenAndServe()
@@ -298,12 +301,48 @@ func StartAutoTLS(s *http.Server, m *autocert.Manager, conf *config.Config) {
 	}
 }
 
-func redirect(w http.ResponseWriter, req *http.Request) {
-	target := "https://" + req.Host + req.RequestURI
+// redirect sends HTTP requests to the configured HTTPS site host.
+func redirect(w http.ResponseWriter, req *http.Request, conf *config.Config) {
+	target := canonicalRedirectTarget(req, conf)
 
 	http.Redirect(w, req, target, httpsRedirect)
 }
 
+// canonicalRedirectTarget returns the HTTPS redirect target using the configured public site host.
+func canonicalRedirectTarget(req *http.Request, conf *config.Config) string {
+	target := &url.URL{
+		Scheme:   "https",
+		Host:     "",
+		Path:     "/",
+		RawPath:  "",
+		RawQuery: "",
+	}
+
+	if req != nil {
+		target.Host = req.Host
+	}
+
+	if conf != nil {
+		if host := conf.SiteHost(); host != "" {
+			target.Host = host
+		}
+	}
+
+	if req != nil && req.URL != nil {
+		target.Path = req.URL.Path
+		target.RawPath = req.URL.RawPath
+		target.RawQuery = req.URL.RawQuery
+	} else if req != nil && req.RequestURI != "" {
+		if u, err := url.ParseRequestURI(req.RequestURI); err == nil {
+			target.Path = u.Path
+			target.RawPath = u.RawPath
+			target.RawQuery = u.RawQuery
+		}
+	}
+
+	return target.String()
+}
+
 // newHTTPServer creates an HTTP server with hardened header and idle settings.
 func newHTTPServer(handler http.Handler, conf *config.Config) *http.Server {
 	headerTimeout := config.DefaultHttpHeaderTimeout

internal/server/start_test.go

@@ -6,6 +6,8 @@ import (
 	"testing"
 	"time"
 
+	"golang.org/x/crypto/acme/autocert"
+
 	"github.com/gin-gonic/gin"
 	"github.com/stretchr/testify/assert"
 
@@ -104,3 +106,69 @@ func TestNewHTTPServer(t *testing.T) {
 		assert.Equal(t, config.DefaultHttpHeaderBytes, server.MaxHeaderBytes)
 	})
 }
+
+func TestCanonicalRedirectTarget(t *testing.T) {
+	t.Run("UsesConfiguredSiteHostInsteadOfRequestHost", func(t *testing.T) {
+		conf := config.NewConfig(config.CliTestContext())
+		conf.Options().SiteUrl = "https://photos.example.com:7443/library/"
+
+		req := httptest.NewRequest(http.MethodGet, "http://evil.example.test/library/login?next=%2Flibrary", nil)
+		req.Host = "evil.example.test"
+
+		target := canonicalRedirectTarget(req, conf)
+
+		assert.Equal(t, "https://photos.example.com:7443/library/login?next=%2Flibrary", target)
+	})
+	t.Run("FallsBackToRequestHostWithoutConfig", func(t *testing.T) {
+		req := httptest.NewRequest(http.MethodGet, "http://localhost:2342/library", nil)
+		req.Host = "localhost:2342"
+
+		target := canonicalRedirectTarget(req, nil)
+
+		assert.Equal(t, "https://localhost:2342/library", target)
+	})
+}
+
+func TestAutoTLSHTTPHandler(t *testing.T) {
+	t.Run("UsesCanonicalHostForFallbackRedirect", func(t *testing.T) {
+		conf := config.NewConfig(config.CliTestContext())
+		conf.Options().SiteUrl = "https://photos.example.com:7443/library/"
+
+		handler := (&autocert.Manager{
+			HostPolicy: autocert.HostWhitelist(conf.SiteDomain()),
+		}).HTTPHandler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			redirect(w, r, conf)
+		}))
+
+		req := httptest.NewRequest(http.MethodGet, "http://evil.example.test/library/login?next=%2Flibrary", nil)
+		req.Host = "evil.example.test"
+
+		w := httptest.NewRecorder()
+		handler.ServeHTTP(w, req)
+
+		assert.Equal(t, httpsRedirect, w.Code)
+		assert.Equal(t, "https://photos.example.com:7443/library/login?next=%2Flibrary", w.Header().Get(header.Location))
+	})
+	t.Run("HandlesAcmeChallengeWithoutFallbackRedirect", func(t *testing.T) {
+		conf := config.NewConfig(config.CliTestContext())
+		conf.Options().SiteUrl = "https://photos.example.com/"
+
+		fallbackCalled := false
+		handler := (&autocert.Manager{
+			HostPolicy: autocert.HostWhitelist(conf.SiteDomain()),
+		}).HTTPHandler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			fallbackCalled = true
+			redirect(w, r, conf)
+		}))
+
+		req := httptest.NewRequest(http.MethodGet, "http://photos.example.com/.well-known/acme-challenge/test-token", nil)
+		req.Host = conf.SiteDomain()
+
+		w := httptest.NewRecorder()
+		handler.ServeHTTP(w, req)
+
+		assert.False(t, fallbackCalled)
+		assert.Equal(t, http.StatusNotFound, w.Code)
+		assert.Empty(t, w.Header().Get(header.Location))
+	})
+}