Repository: pusher/oauth2_proxy (go)

Issue:
Feature request: Redirect to HTTPS
## Current Behavior

We are running this excellent of oauth2_proxy on Heroku using this build-pack: https://github.com/cfra/heroku-buildpack-oauth2-proxy

oauth2_proxy runs as reverse proxy in front of an nginx which serves static files.

When I access the heroku application using plain HTTP, the "login with github" button will be shown and the cookie will be set, all without encryption.

After authenticating with GitHub, we get redirected to the oauth2 callback, but this time via HTTPS, because such is the configuration in GitHub. (And oauth2 spec recommends to only use HTTPS anyway)

This will lead to oauth2_proxy showing an error that a required cookie is missing, because the cookie set via HTTP is not taken into the HTTPS session.

After clicking "login" again and doing everything over HTTPS, it works fine.

## Possible Solution

As the operation of oauth2_proxy as a user-facing reverse proxy seems to be a possible mode of deployment, it would be useful if there was an option to redirect all traffic to HTTPS.

Pull request description:
Redirect to HTTPS
## Description

Implements a new option, flag, and environment variable called `force-https` that will redirect any HTTP request to the HTTPS endpoint.

## Motivation and Context

This PR closes #259.

## How Has This Been Tested?

I have added 2 additional tests that check for the expected behaviour. I also ran the test suite.

## Checklist:

<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->

- [x] My change requires a change to the documentation or CHANGELOG.
- [x] I have updated the documentation/CHANGELOG accordingly.
- [x] I have created a feature (non-master) branch for my PR.


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 782278e2..55548857 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,6 +3,7 @@
 ## Changes since v4.0.0
 
 - [#227](https://github.com/pusher/oauth2_proxy/pull/227) Add Keycloak provider (@Ofinka)
+- [#259](https://github.com/pusher/oauth2_proxy/pull/259) Redirect to HTTPS (@jmickey)
 - [#273](https://github.com/pusher/oauth2_proxy/pull/273) Support Go 1.13 (@dio)
 - [#275](https://github.com/pusher/oauth2_proxy/pull/275) docker: build from debian buster (@syscll)
 - [#258](https://github.com/pusher/oauth2_proxy/pull/258) Add IDToken for Azure provider
diff --git a/docs/configuration/configuration.md b/docs/configuration/configuration.md
index c076dc88..332c2238 100644
--- a/docs/configuration/configuration.md
+++ b/docs/configuration/configuration.md
@@ -44,6 +44,7 @@ An example [oauth2_proxy.cfg]({{ site.gitweb }}/contrib/oauth2_proxy.cfg.example
 | `-extra-jwt-issuers` | string | if `-skip-jwt-bearer-tokens` is set, a list of extra JWT `issuer=audience` pairs (where the issuer URL has a `.well-known/openid-configuration` or a `.well-known/jwks.json`) | |
 | `-exclude-logging-paths` | string | comma separated list of paths to exclude from logging, eg: `"/ping,/path2"` |`""` (no paths excluded) |
 | `-flush-interval` | duration | period between flushing response buffers when streaming responses | `"1s"` |
+| `-force-https` | bool | enforce https redirect | `false` |
 | `-banner` | string | custom banner string. Use `"-"` to disable default banner. | |
 | `-footer` | string | custom footer string. Use `"-"` to disable default footer. | |
 | `-gcp-healthchecks` | bool | will enable `/liveness_check`, `/readiness_check`, and `/` (with the proper user-agent) endpoints that will make it work well with GCP App Engine and GKE Ingresses | false |
diff --git a/http.go b/http.go
index 2cee227b..88280c44 100644
--- a/http.go
+++ b/http.go
@@ -152,3 +152,14 @@ func (ln tcpKeepAliveListener) Accept() (c net.Conn, err error) {
 	tc.SetKeepAlivePeriod(3 * time.Minute)
 	return tc, nil
 }
+
+func redirectToHTTPS(opts *Options, h http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		proto := r.Header.Get("X-Forwarded-Proto")
+		if opts.ForceHTTPS && (r.TLS == nil || (proto != "" && strings.ToLower(proto) != "https")) {
+			http.Redirect(w, r, opts.HTTPSAddress, http.StatusPermanentRedirect)
+		}
+
+		h.ServeHTTP(w, r)
+	})
+}
diff --git a/main.go b/main.go
index a4bf378e..e84a796e 100644
--- a/main.go
+++ b/main.go
@@ -32,6 +32,7 @@ func main() {
 
 	flagSet.String("http-address", "127.0.0.1:4180", "[http://]<addr>:<port> or unix://<path> to listen on for HTTP clients")
 	flagSet.String("https-address", ":443", "<addr>:<port> to listen on for HTTPS clients")
+	flagSet.Bool("force-https", false, "force HTTPS redirect for HTTP requests")
 	flagSet.String("tls-cert-file", "", "path to certificate file")
 	flagSet.String("tls-key-file", "", "path to private key file")
 	flagSet.String("redirect-url", "", "the OAuth Redirect URL. ie: \"https://internalapp.yourcompany.com/oauth2/callback\"")
@@ -185,9 +186,9 @@ func main() {
 
 	var handler http.Handler
 	if opts.GCPHealthChecks {
-		handler = gcpHealthcheck(LoggingHandler(oauthproxy))
+		handler = redirectToHTTPS(opts, gcpHealthcheck(LoggingHandler(oauthproxy)))
 	} else {
-		handler = LoggingHandler(oauthproxy)
+		handler = redirectToHTTPS(opts, LoggingHandler(oauthproxy))
 	}
 	s := &Server{
 		Handler: handler,
diff --git a/options.go b/options.go
index 37bbb0b9..ddc10cfd 100644
--- a/options.go
+++ b/options.go
@@ -34,6 +34,7 @@ type Options struct {
 	ProxyWebSockets bool   `flag:"proxy-websockets" cfg:"proxy_websockets" env:"OAUTH2_PROXY_PROXY_WEBSOCKETS"`
 	HTTPAddress     string `flag:"http-address" cfg:"http_address" env:"OAUTH2_PROXY_HTTP_ADDRESS"`
 	HTTPSAddress    string `flag:"https-address" cfg:"https_address" env:"OAUTH2_PROXY_HTTPS_ADDRESS"`
+	ForceHTTPS      bool   `flag:"force-https" cfg:"force_https" env:"OAUTH2_PROXY_FORCE_HTTPS"`
 	RedirectURL     string `flag:"redirect-url" cfg:"redirect_url" env:"OAUTH2_PROXY_REDIRECT_URL"`
 	ClientID        string `flag:"client-id" cfg:"client_id" env:"OAUTH2_PROXY_CLIENT_ID"`
 	ClientSecret    string `flag:"client-secret" cfg:"client_secret" env:"OAUTH2_PROXY_CLIENT_SECRET"`
@@ -145,6 +146,7 @@ func NewOptions() *Options {
 		ProxyWebSockets:     true,
 		HTTPAddress:         "127.0.0.1:4180",
 		HTTPSAddress:        ":443",
+		ForceHTTPS:          false,
 		DisplayHtpasswdForm: true,
 		CookieOptions: options.CookieOptions{
 			CookieName:     "_oauth2_proxy",


Test patch:
diff --git a/http_test.go b/http_test.go
index f5ee1421..400213a0 100644
--- a/http_test.go
+++ b/http_test.go
@@ -106,3 +106,53 @@ func TestGCPHealthcheckNotIngressPut(t *testing.T) {
 
 	assert.Equal(t, "test", rw.Body.String())
 }
+
+func TestRedirectToHTTPSTrue(t *testing.T) {
+	opts := NewOptions()
+	opts.ForceHTTPS = true
+	handler := func(w http.ResponseWriter, req *http.Request) {
+		w.Write([]byte("test"))
+	}
+
+	h := redirectToHTTPS(opts, http.HandlerFunc(handler))
+	rw := httptest.NewRecorder()
+	r, _ := http.NewRequest("GET", "/", nil)
+	h.ServeHTTP(rw, r)
+
+	assert.Equal(t, http.StatusPermanentRedirect, rw.Code, "status code should be %d, got: %d", http.StatusPermanentRedirect, rw.Code)
+}
+
+func TestRedirectToHTTPSFalse(t *testing.T) {
+	opts := NewOptions()
+	handler := func(w http.ResponseWriter, req *http.Request) {
+		w.Write([]byte("test"))
+	}
+
+	h := redirectToHTTPS(opts, http.HandlerFunc(handler))
+	rw := httptest.NewRecorder()
+	r, _ := http.NewRequest("GET", "/", nil)
+	h.ServeHTTP(rw, r)
+
+	assert.Equal(t, http.StatusOK, rw.Code, "status code should be %d, got: %d", http.StatusOK, rw.Code)
+}
+
+func TestRedirectNotWhenHTTPS(t *testing.T) {
+	opts := NewOptions()
+	opts.ForceHTTPS = true
+	handler := func(w http.ResponseWriter, req *http.Request) {
+		w.Write([]byte("test"))
+	}
+
+	h := redirectToHTTPS(opts, http.HandlerFunc(handler))
+	s := httptest.NewTLSServer(h)
+	defer s.Close()
+
+	opts.HTTPSAddress = s.URL
+	client := s.Client()
+	res, err := client.Get(s.URL)
+	if err != nil {
+		t.Fatalf("request to test server failed with error: %v", err)
+	}
+
+	assert.Equal(t, http.StatusOK, res.StatusCode, "status code should be %d, got: %d", http.StatusOK, res.StatusCode)
+}