From a999270cf3b4dc808fd22307bcab182965ba030e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mariano=20Vall=C3=A9s?= Date: Tue, 7 Jul 2020 10:55:38 +0200 Subject: [PATCH] Add req.host to targetURL when redirecting to https (#668) * Add req.host to targetURL when redirecting to https The req.URL.Host might not be present when redirecting to https if the given req.URL is something like "/". In such scenario, the req.Host is still present and valid. This commit adds the original req.Host to the targetURL before returning the 308 status, to avoid having a `Location: https:///` in the response. * Bring back empty line * Wrap the setting of targetURL.Host in a condition * Add a comment to the test explaining why the redirectURL includes example.com * Add changelog entry --- CHANGELOG.md | 1 + pkg/middleware/redirect_to_https.go | 5 +++++ pkg/middleware/redirect_to_https_test.go | 10 ++++++++++ 3 files changed, 16 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index bca36842..8ba2989e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ ## Changes since v6.0.0 +- [#668](https://github.com/oauth2-proxy/oauth2-proxy/pull/668) Use req.Host in --force-https when req.URL.Host is empty (@zucaritask) - [#660](https://github.com/oauth2-proxy/oauth2-proxy/pull/660) Use builder pattern to simplify requests to external endpoints (@JoelSpeed) - [#591](https://github.com/oauth2-proxy/oauth2-proxy/pull/591) Introduce upstream package with new reverse proxy implementation (@JoelSpeed) - [#576](https://github.com/oauth2-proxy/oauth2-proxy/pull/576) Separate Cookie validation out of main options validation (@JoelSpeed) diff --git a/pkg/middleware/redirect_to_https.go b/pkg/middleware/redirect_to_https.go index 141d076a..27384561 100644 --- a/pkg/middleware/redirect_to_https.go +++ b/pkg/middleware/redirect_to_https.go @@ -38,6 +38,11 @@ func redirectToHTTPS(httpsPort string, next http.Handler) http.Handler { // Set the scheme to HTTPS targetURL.Scheme = httpsScheme + // Set the req.Host when the targetURL still does not have one + if targetURL.Host == "" { + targetURL.Host = req.Host + } + // Overwrite the port if the original request was to a non-standard port if targetURL.Port() != "" { // If Port was not empty, this should be fine to ignore the error diff --git a/pkg/middleware/redirect_to_https_test.go b/pkg/middleware/redirect_to_https_test.go index c4b631b5..238c3b81 100644 --- a/pkg/middleware/redirect_to_https_test.go +++ b/pkg/middleware/redirect_to_https_test.go @@ -154,5 +154,15 @@ var _ = Describe("RedirectToHTTPS suite", func() { expectedStatus: 200, expectedBody: "test", }), + // By using newRequest from httptest we get example.com as a Host + // when the target is just a path. + // For details: https://golang.org/pkg/net/http/httptest/#NewRequest + Entry("without TLS with a path as request", &requestTableInput{ + requestString: "/", + useTLS: false, + expectedStatus: 308, + expectedBody: permanentRedirectBody("https://example.com/"), + expectedLocation: "https://example.com/", + }), ) })