From e5548827302530310ecadef393c5ef9ca6c51ba0 Mon Sep 17 00:00:00 2001
From: Stan Hu <stanhu@gmail.com>
Date: Mon, 27 Aug 2018 11:30:14 -0700
Subject: [PATCH] Disable automatic HTTP -> HTTPS redirection if no HTTPS
 server defined

Right now the logic for determing when HTTPS redirection should happen
is split in both the GitLab Rails and Pages processes. This means that
the Pages configuration in Omnibus has to be specified correctly
for Sidekiq for the HTTPS-only feature to work properly. We can
simplify this by consolidating alll the HTTPS redirection logic
here and removing it from Rails.

Closes https://gitlab.com/gitlab-org/gitlab-pages/issues/163
---
 acceptance_test.go | 89 ++++++++++++++++++++++++++++++++++------------
 app.go             | 15 +++++++-
 2 files changed, 81 insertions(+), 23 deletions(-)

diff --git a/acceptance_test.go b/acceptance_test.go
index 361cba68e..b195c7ea8 100644
--- a/acceptance_test.go
+++ b/acceptance_test.go
@@ -197,18 +197,35 @@ func TestKnownHostWithPortReturns200(t *testing.T) {
 
 func TestHttpToHttpsRedirectDisabled(t *testing.T) {
 	skipUnlessEnabled(t)
-	teardown := RunPagesProcess(t, *pagesBinary, listeners, "")
-	defer teardown()
 
-	rsp, err := GetRedirectPage(t, httpListener, "group.gitlab-example.com", "project/")
-	require.NoError(t, err)
-	defer rsp.Body.Close()
-	assert.Equal(t, http.StatusOK, rsp.StatusCode)
+	cases := []struct {
+		Listeners []ListenSpec
+		Args      string
+	}{
+		{
+			Listeners: listeners,
+			Args:      "",
+		},
+		{
+			Listeners: []ListenSpec{httpListener},
+			Args:      "-redirect-http=true",
+		},
+	}
 
-	rsp, err = GetPageFromListener(t, httpsListener, "group.gitlab-example.com", "project/")
-	require.NoError(t, err)
-	defer rsp.Body.Close()
-	assert.Equal(t, http.StatusOK, rsp.StatusCode)
+	for _, c := range cases {
+		teardown := RunPagesProcess(t, *pagesBinary, c.Listeners, "", c.Args)
+		defer teardown()
+
+		rsp, err := GetRedirectPage(t, httpListener, "group.gitlab-example.com", "project/")
+		require.NoError(t, err)
+		defer rsp.Body.Close()
+		assert.Equal(t, http.StatusOK, rsp.StatusCode)
+
+		rsp, err = GetPageFromListener(t, httpsListener, "group.gitlab-example.com", "project/")
+		require.NoError(t, err)
+		defer rsp.Body.Close()
+		assert.Equal(t, http.StatusOK, rsp.StatusCode)
+	}
 }
 
 func TestHttpToHttpsRedirectEnabled(t *testing.T) {
@@ -242,13 +259,27 @@ func TestHttpsOnlyGroupEnabled(t *testing.T) {
 
 func TestHttpsOnlyGroupDisabled(t *testing.T) {
 	skipUnlessEnabled(t)
-	teardown := RunPagesProcess(t, *pagesBinary, listeners, "")
-	defer teardown()
 
-	rsp, err := GetPageFromListener(t, httpListener, "group.https-only.gitlab-example.com", "project2/")
-	require.NoError(t, err)
-	defer rsp.Body.Close()
-	assert.Equal(t, http.StatusOK, rsp.StatusCode)
+	cases := []struct {
+		Listeners []ListenSpec
+	}{
+		{
+			Listeners: listeners,
+		},
+		{
+			Listeners: []ListenSpec{httpListener},
+		},
+	}
+
+	for _, c := range cases {
+		teardown := RunPagesProcess(t, *pagesBinary, c.Listeners, "")
+		defer teardown()
+
+		rsp, err := GetPageFromListener(t, httpListener, "group.https-only.gitlab-example.com", "project2/")
+		require.NoError(t, err)
+		defer rsp.Body.Close()
+		assert.Equal(t, http.StatusOK, rsp.StatusCode)
+	}
 }
 
 func TestHttpsOnlyProjectEnabled(t *testing.T) {
@@ -264,13 +295,27 @@ func TestHttpsOnlyProjectEnabled(t *testing.T) {
 
 func TestHttpsOnlyProjectDisabled(t *testing.T) {
 	skipUnlessEnabled(t)
-	teardown := RunPagesProcess(t, *pagesBinary, listeners, "")
-	defer teardown()
 
-	rsp, err := GetPageFromListener(t, httpListener, "test2.my-domain.com", "/")
-	require.NoError(t, err)
-	defer rsp.Body.Close()
-	assert.Equal(t, http.StatusOK, rsp.StatusCode)
+	cases := []struct {
+		Listeners []ListenSpec
+	}{
+		{
+			Listeners: listeners,
+		},
+		{
+			Listeners: []ListenSpec{httpListener},
+		},
+	}
+
+	for _, c := range cases {
+		teardown := RunPagesProcess(t, *pagesBinary, c.Listeners, "")
+		defer teardown()
+
+		rsp, err := GetPageFromListener(t, httpListener, "test2.my-domain.com", "/")
+		require.NoError(t, err)
+		defer rsp.Body.Close()
+		assert.Equal(t, http.StatusOK, rsp.StatusCode)
+	}
 }
 
 func TestHttpsOnlyDomainDisabled(t *testing.T) {
diff --git a/app.go b/app.go
index 0a7a82685..741caa9aa 100644
--- a/app.go
+++ b/app.go
@@ -92,6 +92,10 @@ func (a *theApp) getHostAndDomain(r *http.Request) (host string, domain *domain.
 	return host, a.domain(host)
 }
 
+func (a *theApp) shouldRedirectDomain(domain *domain.D, r *http.Request) bool {
+	return a.IsHTTPSEnabled() && domain.IsHTTPSOnly(r)
+}
+
 func (a *theApp) tryAuxiliaryHandlers(w http.ResponseWriter, r *http.Request, https bool, host string, domain *domain.D) bool {
 	// short circuit content serving to check for a status page
 	if r.RequestURI == a.appConfig.StatusPath {
@@ -121,7 +125,7 @@ func (a *theApp) tryAuxiliaryHandlers(w http.ResponseWriter, r *http.Request, ht
 		return true
 	}
 
-	if !https && domain.IsHTTPSOnly(r) {
+	if !https && a.shouldRedirectDomain(domain, r) {
 		a.redirectToHTTPS(w, r, http.StatusMovedPermanently)
 		return true
 	}
@@ -284,6 +288,10 @@ func (a *theApp) listenAdminHTTPS(wg *sync.WaitGroup) {
 	}()
 }
 
+func (a *theApp) IsHTTPSEnabled() bool {
+	return len(a.appConfig.ListenHTTPS) > 0
+}
+
 func runApp(config appConfig) {
 	a := theApp{appConfig: config}
 
@@ -293,6 +301,11 @@ func runApp(config appConfig) {
 
 	configureLogging(config.LogFormat, config.LogVerbose)
 
+	if config.RedirectHTTP && len(config.ListenHTTPS) == 0 {
+		log.Warn("No HTTPS listener defined, disabling automatic redirect to HTTPS")
+		config.RedirectHTTP = false
+	}
+
 	if err := mimedb.LoadTypes(); err != nil {
 		log.WithError(err).Warn("Loading extended MIME database failed")
 	}
-- 
GitLab