From 4940a33fbe49f128c18ea637c17f269655c7e5df Mon Sep 17 00:00:00 2001 From: Randy Fay Date: Mon, 1 Oct 2018 11:06:50 -0600 Subject: [PATCH] Add rewrites for ssl so apache behaves properly (#1124) * Add rewrites for ssl so apache behaves properly * Refactor testcommon.GetLocalHTTPResponse() to return resp for extra checking * Add TestHttpsRedirection() --- .../etc/apache2/apache-site-default.conf | 10 +++ pkg/ddevapp/ddevapp_test.go | 82 +++++++++++++++++++ .../testdata/TestHttpsRedirection/.htaccess | 9 ++ .../testdata/TestHttpsRedirection/index.php | 5 ++ .../testdata/TestHttpsRedirection/landed.php | 14 ++++ .../TestHttpsRedirection/redir_abs.php | 5 ++ .../TestHttpsRedirection/redir_relative.php | 3 + .../TestHttpsRedirection/subdir/index.html | 2 + pkg/testcommon/testcommon.go | 24 ++++-- pkg/testcommon/testcommon_test.go | 2 +- pkg/version/version.go | 2 +- 11 files changed, 148 insertions(+), 10 deletions(-) create mode 100644 pkg/ddevapp/testdata/TestHttpsRedirection/.htaccess create mode 100644 pkg/ddevapp/testdata/TestHttpsRedirection/index.php create mode 100644 pkg/ddevapp/testdata/TestHttpsRedirection/landed.php create mode 100644 pkg/ddevapp/testdata/TestHttpsRedirection/redir_abs.php create mode 100644 pkg/ddevapp/testdata/TestHttpsRedirection/redir_relative.php create mode 100644 pkg/ddevapp/testdata/TestHttpsRedirection/subdir/index.html diff --git a/containers/ddev-webserver/files/etc/apache2/apache-site-default.conf b/containers/ddev-webserver/files/etc/apache2/apache-site-default.conf index b254f670392..6e53f0006e9 100644 --- a/containers/ddev-webserver/files/etc/apache2/apache-site-default.conf +++ b/containers/ddev-webserver/files/etc/apache2/apache-site-default.conf @@ -8,6 +8,16 @@ # However, you must set it for any further virtual host explicitly. #ServerName www.example.com + # Workaround from https://mail-archives.apache.org/mod_mbox/httpd-users/201403.mbox/%3C49404A24C7FAD94BB7B45E86A9305F6214D04652@MSGEXSV21103.ent.wfb.bank.corp%3E + # See also https://gist.github.com/nurtext/b6ac07ac7d8c372bc8eb + + RewriteEngine On + RewriteCond %{HTTP:X-Forwarded-Proto} =https + RewriteCond %{DOCUMENT_ROOT}%{REQUEST_FILENAME} -d + RewriteRule ^(.+[^/])$ https://%{HTTP_HOST}$1/ [redirect,last] + + SetEnvIf X-Forwarded-Proto "https" HTTPS=on + ServerAdmin webmaster@localhost DocumentRoot $WEBSERVER_DOCROOT diff --git a/pkg/ddevapp/ddevapp_test.go b/pkg/ddevapp/ddevapp_test.go index cb663b35ca0..07fd4951434 100644 --- a/pkg/ddevapp/ddevapp_test.go +++ b/pkg/ddevapp/ddevapp_test.go @@ -1327,6 +1327,88 @@ func TestListWithoutDir(t *testing.T) { assert.NoError(err) } +type URLRedirectExpectations struct { + scheme string + uri string + expectedRedirectURI string +} + +// TestHttpsRedirection tests to make sure that webserver and php redirect to correct +// scheme (http or https). +func TestHttpsRedirection(t *testing.T) { + // Set up tests and give ourselves a working directory. + assert := asrt.New(t) + testcommon.ClearDockerEnv() + packageDir, _ := os.Getwd() + + testDir := testcommon.CreateTmpDir("TestHttpsRedirection") + defer testcommon.CleanupDir(testDir) + appDir := filepath.Join(testDir, "proj") + err := fileutil.CopyDir(filepath.Join(packageDir, "testdata", "TestHttpsRedirection"), appDir) + assert.NoError(err) + err = os.Chdir(appDir) + assert.NoError(err) + + app, err := ddevapp.NewApp(appDir, ddevapp.DefaultProviderName) + assert.NoError(err) + app.Name = "proj" + app.Type = "php" + + expectations := []URLRedirectExpectations{ + {"https", "/subdir", "/subdir/"}, + {"https", "/redir_abs.php", "/landed.php"}, + {"https", "/redir_relative.php", "/landed.php"}, + {"http", "/subdir", "/subdir/"}, + {"http", "/redir_abs.php", "/landed.php"}, + {"http", "/redir_relative.php", "/landed.php"}, + } + + for _, webserverType := range []string{"nginx-fpm", "apache-fpm", "apache-cgi"} { + app.WebserverType = webserverType + err = app.WriteConfig() + assert.NoError(err) + + // Do a start on the configured site. + app, err = ddevapp.GetActiveApp("") + assert.NoError(err) + err = app.Start() + assert.NoError(err) + + // Test for directory redirects under https and http + for _, parts := range expectations { + + reqURL := parts.scheme + "://" + app.GetHostname() + parts.uri + // nolint: vetshadow + _, resp, err := testcommon.GetLocalHTTPResponse(t, reqURL) + assert.Error(err) + assert.NotNil(resp, "resp was nil for webserver_type=%s url=%s", webserverType, reqURL) + if resp != nil { + locHeader := resp.Header.Get("Location") + + expectedRedirect := parts.expectedRedirectURI + // However, if we're hitting redir_abs.php (or apache hitting directory), the redirect will be the whole url. + if strings.Contains(parts.uri, "redir_abs.php") || webserverType != "nginx-fpm" { + expectedRedirect = parts.scheme + "://" + app.GetHostname() + parts.expectedRedirectURI + } + // Except the php relative redirect is always relative. + if strings.Contains(parts.uri, "redir_relative.php") { + expectedRedirect = parts.expectedRedirectURI + } + + assert.EqualValues(locHeader, expectedRedirect, "For webserver_type %s url %s expected redirect %s != actual %s", webserverType, reqURL, expectedRedirect, locHeader) + } + } + } + + err = app.Down(true, false) + assert.NoError(err) + + // Change back to package dir. Lots of things will have to be cleaned up + // in defers, and for windows we have to not be sitting in them. + err = os.Chdir(packageDir) + assert.NoError(err) +} + // TestMultipleComposeFiles checks to see if a set of docker-compose files gets // properly loaded in the right order, with docker-compose.yaml first and // with docker-compose.override.yaml last. diff --git a/pkg/ddevapp/testdata/TestHttpsRedirection/.htaccess b/pkg/ddevapp/testdata/TestHttpsRedirection/.htaccess new file mode 100644 index 00000000000..2e406c42bd1 --- /dev/null +++ b/pkg/ddevapp/testdata/TestHttpsRedirection/.htaccess @@ -0,0 +1,9 @@ +# DirectorySlash off +# DirectoryIndex index.php +# DirectorySlash off +# DirectoryIndexRedirect on +# Redirect to HTTPS before Apache mod_dir DirectorySlash redirect to HTTP +RewriteCond %{HTTP:X-Forwarded-Proto} =https +RewriteCond %{LA-U:REQUEST_FILENAME} -d +RewriteRule ^/(.*[^/])$ https://%{HTTP_HOST}/$1/ [R=301,L,QSA] + diff --git a/pkg/ddevapp/testdata/TestHttpsRedirection/index.php b/pkg/ddevapp/testdata/TestHttpsRedirection/index.php new file mode 100644 index 00000000000..432eaef2042 --- /dev/null +++ b/pkg/ddevapp/testdata/TestHttpsRedirection/index.php @@ -0,0 +1,5 @@ +"; + +echo 'Here are links to landed.php and subdir
'; \ No newline at end of file diff --git a/pkg/ddevapp/testdata/TestHttpsRedirection/landed.php b/pkg/ddevapp/testdata/TestHttpsRedirection/landed.php new file mode 100644 index 00000000000..b13a370a88c --- /dev/null +++ b/pkg/ddevapp/testdata/TestHttpsRedirection/landed.php @@ -0,0 +1,14 @@ +"; +echo "HTTPS is {$_SERVER['HTTPS']}
"; + +echo 'You can go to redir_abs.php or to redir_relative.php
'; + + +if (!empty($_SERVER['HTTPS']) && $_SERVER["HTTPS"] == "on") { + echo "You can switch from https to http
"; +} else { + echo "You can switch from http to https
"; +} diff --git a/pkg/ddevapp/testdata/TestHttpsRedirection/redir_abs.php b/pkg/ddevapp/testdata/TestHttpsRedirection/redir_abs.php new file mode 100644 index 00000000000..7db9550a914 --- /dev/null +++ b/pkg/ddevapp/testdata/TestHttpsRedirection/redir_abs.php @@ -0,0 +1,5 @@ + +Go back to the top. \ No newline at end of file diff --git a/pkg/testcommon/testcommon.go b/pkg/testcommon/testcommon.go index fcfbfa13e40..1d1c1f1d785 100644 --- a/pkg/testcommon/testcommon.go +++ b/pkg/testcommon/testcommon.go @@ -2,6 +2,7 @@ package testcommon import ( "bytes" + "crypto/tls" "io" "io/ioutil" "os" @@ -343,7 +344,7 @@ func GetCachedArchive(siteName string, prefixString string, internalExtractionPa // GetLocalHTTPResponse takes a URL, hits the local docker for it, returns result // Returns error (with the body) if not 200 status code. -func GetLocalHTTPResponse(t *testing.T, rawurl string) (string, error) { +func GetLocalHTTPResponse(t *testing.T, rawurl string) (string, *http.Response, error) { assert := asrt.New(t) u, err := url.Parse(rawurl) @@ -359,44 +360,51 @@ func GetLocalHTTPResponse(t *testing.T, rawurl string) (string, error) { localAddress := u.String() timeout := time.Duration(10 * time.Second) + + // Ignore https cert failure, since we are in testing environment. + insecureTransport := &http.Transport{ + TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, + } + // Do not follow redirects, https://stackoverflow.com/a/38150816/215713 client := &http.Client{ CheckRedirect: func(req *http.Request, via []*http.Request) error { return http.ErrUseLastResponse }, - Timeout: timeout, + Transport: insecureTransport, + Timeout: timeout, } req, err := http.NewRequest("GET", localAddress, nil) if err != nil { - return "", fmt.Errorf("Failed to NewRequest GET %s: %v", localAddress, err) + return "", nil, fmt.Errorf("Failed to NewRequest GET %s: %v", localAddress, err) } req.Host = fakeHost resp, err := client.Do(req) if err != nil { - return "", err + return "", resp, err } //nolint: errcheck defer resp.Body.Close() bodyBytes, err := ioutil.ReadAll(resp.Body) if err != nil { - return "", fmt.Errorf("unable to ReadAll resp.body: %v", err) + return "", resp, fmt.Errorf("unable to ReadAll resp.body: %v", err) } bodyString := string(bodyBytes) if resp.StatusCode != 200 { - return bodyString, fmt.Errorf("http status code was %d, not 200", resp.StatusCode) + return bodyString, resp, fmt.Errorf("http status code was %d, not 200", resp.StatusCode) } - return bodyString, nil + return bodyString, resp, nil } // EnsureLocalHTTPContent will verify a URL responds with a 200 and expected content string func EnsureLocalHTTPContent(t *testing.T, rawurl string, expectedContent string) { assert := asrt.New(t) - body, err := GetLocalHTTPResponse(t, rawurl) + body, _, err := GetLocalHTTPResponse(t, rawurl) assert.NoError(err, "GetLocalHTTPResponse returned err on rawurl %s: %v", rawurl, err) assert.Contains(body, expectedContent) } diff --git a/pkg/testcommon/testcommon_test.go b/pkg/testcommon/testcommon_test.go index a0c8cf7db5f..362bda63365 100644 --- a/pkg/testcommon/testcommon_test.go +++ b/pkg/testcommon/testcommon_test.go @@ -170,7 +170,7 @@ func TestGetLocalHTTPResponse(t *testing.T) { assert.NoError(err) safeURL := app.GetHTTPURL() + site.Safe200URL - out, err := GetLocalHTTPResponse(t, safeURL) + out, _, err := GetLocalHTTPResponse(t, safeURL) assert.NoError(err) assert.Contains(out, "Famous 5-minute install") diff --git a/pkg/version/version.go b/pkg/version/version.go index a7a393cfd20..9aa19887280 100644 --- a/pkg/version/version.go +++ b/pkg/version/version.go @@ -25,7 +25,7 @@ var DockerComposeFileFormatVersion = "3.6" var WebImg = "drud/ddev-webserver" // WebTag defines the default web image tag for drud dev -var WebTag = "20180922_upgrade_debian_stretch" // Note that this can be overridden by make +var WebTag = "20180922_apache_https" // Note that this can be overridden by make // DBImg defines the default db image used for applications. var DBImg = "drud/ddev-dbserver"