From c8a89eca0800ca64da09ab09f758d78b98bc974f Mon Sep 17 00:00:00 2001 From: leyshon Date: Thu, 29 Aug 2019 14:32:01 +0100 Subject: [PATCH 01/17] Adding the IDToken to the session for the Azure Provider. --- providers/azure.go | 65 +++++++++++++++++++++++++++++++++++++++++ providers/azure_test.go | 18 +++++++++++- 2 files changed, 82 insertions(+), 1 deletion(-) diff --git a/providers/azure.go b/providers/azure.go index 653090b0..6729e4f0 100644 --- a/providers/azure.go +++ b/providers/azure.go @@ -5,6 +5,10 @@ import ( "fmt" "net/http" "net/url" + "encoding/json" + "io/ioutil" + "bytes" + "time" "github.com/bitly/go-simplejson" "github.com/pusher/oauth2_proxy/pkg/apis/sessions" @@ -65,6 +69,67 @@ func (p *AzureProvider) Configure(tenant string) { } } +func (p *AzureProvider) Redeem(redirectURL, code string) (s *sessions.SessionState, err error) { + if code == "" { + err = errors.New("missing code") + return + } + + params := url.Values{} + params.Add("redirect_uri", redirectURL) + params.Add("client_id", p.ClientID) + params.Add("client_secret", p.ClientSecret) + params.Add("code", code) + params.Add("grant_type", "authorization_code") + if p.ProtectedResource != nil && p.ProtectedResource.String() != "" { + params.Add("resource", p.ProtectedResource.String()) + } + + var req *http.Request + req, err = http.NewRequest("POST", p.RedeemURL.String(), bytes.NewBufferString(params.Encode())) + if err != nil { + return + } + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + + var resp *http.Response + resp, err = http.DefaultClient.Do(req) + if err != nil { + return nil, err + } + var body []byte + body, err = ioutil.ReadAll(resp.Body) + resp.Body.Close() + if err != nil { + return + } + + if resp.StatusCode != 200 { + err = fmt.Errorf("got %d from %q %s", resp.StatusCode, p.RedeemURL.String(), body) + return + } + + var jsonResponse struct { + AccessToken string `json:"access_token"` + RefreshToken string `json:"refresh_token"` + ExpiresOn int64 `json:"expires_on,string"` + IDToken string `json:"id_token"` + } + err = json.Unmarshal(body, &jsonResponse) + if err != nil { + return + } + + s = &sessions.SessionState{ + AccessToken: jsonResponse.AccessToken, + IDToken: jsonResponse.IDToken, + CreatedAt: time.Now(), + ExpiresOn: time.Unix(jsonResponse.ExpiresOn, 0), + RefreshToken: jsonResponse.RefreshToken, + } + return +} + func getAzureHeader(accessToken string) http.Header { header := make(http.Header) header.Set("Authorization", fmt.Sprintf("Bearer %s", accessToken)) diff --git a/providers/azure_test.go b/providers/azure_test.go index 8d34bdc8..8ec59785 100644 --- a/providers/azure_test.go +++ b/providers/azure_test.go @@ -20,6 +20,7 @@ func testAzureProvider(hostname string) *AzureProvider { ValidateURL: &url.URL{}, ProtectedResource: &url.URL{}, Scope: ""}) + if hostname != "" { updateURL(p.Data().LoginURL, hostname) updateURL(p.Data().RedeemURL, hostname) @@ -111,8 +112,11 @@ func testAzureBackend(payload string) *httptest.Server { return httptest.NewServer(http.HandlerFunc( func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path != path || r.URL.RawQuery != query { + if (r.URL.Path != path || r.URL.RawQuery != query) && r.Method != "POST" { w.WriteHeader(404) + } else if r.Method == "POST" && r.Body != nil { + w.WriteHeader(200) + w.Write([]byte(payload)) } else if r.Header.Get("Authorization") != "Bearer imaginary_access_token" { w.WriteHeader(403) } else { @@ -199,3 +203,15 @@ func TestAzureProviderGetEmailAddressIncorrectOtherMails(t *testing.T) { assert.Equal(t, "type assertion to string failed", err.Error()) assert.Equal(t, "", email) } + +func TestAzureProviderRedeemReturnsIdToken(t *testing.T) { + b := testAzureBackend(`{ "id_token": "testtoken1234" }`) + defer b.Close() + + bURL, _ := url.Parse(b.URL) + p := testAzureProvider(bURL.Host) + p.Data().RedeemURL.Path = "/common/oauth2/token" + s, err := p.Redeem("https://localhost", "1234") + assert.Equal(t, nil, err) + assert.Equal(t, "testtoken1234", s.IDToken) +} \ No newline at end of file From 0c541f6f5e339021a75b5477e49dce18e20500a8 Mon Sep 17 00:00:00 2001 From: leyshon Date: Thu, 29 Aug 2019 15:01:15 +0100 Subject: [PATCH 02/17] Adding additional asserts to the TestAzureProviderREdeemReturnsIdToken to ensure that the refresh token and expires on date are both being set --- providers/azure_test.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/providers/azure_test.go b/providers/azure_test.go index 8ec59785..3f2b00dd 100644 --- a/providers/azure_test.go +++ b/providers/azure_test.go @@ -5,6 +5,7 @@ import ( "net/http/httptest" "net/url" "testing" + "time" "github.com/pusher/oauth2_proxy/pkg/apis/sessions" "github.com/stretchr/testify/assert" @@ -205,7 +206,8 @@ func TestAzureProviderGetEmailAddressIncorrectOtherMails(t *testing.T) { } func TestAzureProviderRedeemReturnsIdToken(t *testing.T) { - b := testAzureBackend(`{ "id_token": "testtoken1234" }`) + b := testAzureBackend(`{ "id_token": "testtoken1234", "expires_on": "1136239445", "refresh_token": "refresh1234" }`) + timestamp, err := time.Parse(time.RFC3339, "2006-01-02T22:04:05Z") defer b.Close() bURL, _ := url.Parse(b.URL) @@ -214,4 +216,6 @@ func TestAzureProviderRedeemReturnsIdToken(t *testing.T) { s, err := p.Redeem("https://localhost", "1234") assert.Equal(t, nil, err) assert.Equal(t, "testtoken1234", s.IDToken) -} \ No newline at end of file + assert.Equal(t, timestamp, s.ExpiresOn.UTC()) + assert.Equal(t, "refresh1234", s.RefreshToken) +} From 311f14c7ebac531c3c8aac7a0e56fe9874e45bfc Mon Sep 17 00:00:00 2001 From: leyshon Date: Thu, 29 Aug 2019 15:37:25 +0100 Subject: [PATCH 03/17] Fixing linting errors: Making sure err is checked in azure_test and gofmt has been run --- providers/azure.go | 8 ++++---- providers/azure_test.go | 3 ++- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/providers/azure.go b/providers/azure.go index 6729e4f0..48cea846 100644 --- a/providers/azure.go +++ b/providers/azure.go @@ -1,13 +1,13 @@ package providers import ( + "bytes" + "encoding/json" "errors" "fmt" + "io/ioutil" "net/http" "net/url" - "encoding/json" - "io/ioutil" - "bytes" "time" "github.com/bitly/go-simplejson" @@ -119,7 +119,7 @@ func (p *AzureProvider) Redeem(redirectURL, code string) (s *sessions.SessionSta if err != nil { return } - + s = &sessions.SessionState{ AccessToken: jsonResponse.AccessToken, IDToken: jsonResponse.IDToken, diff --git a/providers/azure_test.go b/providers/azure_test.go index 3f2b00dd..24745f3a 100644 --- a/providers/azure_test.go +++ b/providers/azure_test.go @@ -207,8 +207,9 @@ func TestAzureProviderGetEmailAddressIncorrectOtherMails(t *testing.T) { func TestAzureProviderRedeemReturnsIdToken(t *testing.T) { b := testAzureBackend(`{ "id_token": "testtoken1234", "expires_on": "1136239445", "refresh_token": "refresh1234" }`) - timestamp, err := time.Parse(time.RFC3339, "2006-01-02T22:04:05Z") defer b.Close() + timestamp, err := time.Parse(time.RFC3339, "2006-01-02T22:04:05Z") + assert.Equal(t, nil, err) bURL, _ := url.Parse(b.URL) p := testAzureProvider(bURL.Host) From 41ed9f742910ad66502f3d83fbf90adf4eca8717 Mon Sep 17 00:00:00 2001 From: leyshon Date: Mon, 2 Sep 2019 14:56:20 +0100 Subject: [PATCH 04/17] Updating the changelog to include details of the change --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 44cd0cf6..3b515580 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## Changes since v4.0.0 +[#258](https://github.com/pusher/oauth2_proxy/pull/258) Add IDToken for Azure provider + - This PR adds the IDToken into the session for the Azure provider allowing requests to a backend to be identified as a specific user. As a consequence, if you are using a cookie to store the session the cookie will now exceed the 4kb size limit and be split into multiple cookies. This can cause problems when using nginx as a proxy, resulting in no cookie being passed at all. Either increase the proxy_buffer_size in nginx or implement the redis session storage (see https://pusher.github.io/oauth2_proxy/configuration#redis-storage) + # v4.0.0 ## Release Highlights From 21aba50ea5262cd29656778721f9da336c5c3ce5 Mon Sep 17 00:00:00 2001 From: leyshon Date: Mon, 2 Sep 2019 16:00:28 +0100 Subject: [PATCH 05/17] Adding a note to the Azure provider documentation to mention issues with the size of the cookie session storage --- docs/2_auth.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/2_auth.md b/docs/2_auth.md index e6c5cc6f..cbccae7c 100644 --- a/docs/2_auth.md +++ b/docs/2_auth.md @@ -80,6 +80,8 @@ Note: The user is checked against the group members list on initial authenticati --client-secret= ``` +Note: When using the Azure Auth provider with nginx and the cookie session store you may find the cookie is too large and doesn't get passed through correctly.. Increasing the proxy_buffer_size in nginx or implementing the redis session storage (see https://pusher.github.io/oauth2_proxy/configuration#redis-storage) should resolve this. + ### Facebook Auth Provider 1. Create a new FB App from From 1aad87d7ca4c166babca2098d07b3a22bf872bc5 Mon Sep 17 00:00:00 2001 From: leyshon Date: Mon, 2 Sep 2019 16:03:48 +0100 Subject: [PATCH 06/17] Fixing a small typo in the docs --- docs/2_auth.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/2_auth.md b/docs/2_auth.md index cbccae7c..884ab1d7 100644 --- a/docs/2_auth.md +++ b/docs/2_auth.md @@ -80,7 +80,7 @@ Note: The user is checked against the group members list on initial authenticati --client-secret= ``` -Note: When using the Azure Auth provider with nginx and the cookie session store you may find the cookie is too large and doesn't get passed through correctly.. Increasing the proxy_buffer_size in nginx or implementing the redis session storage (see https://pusher.github.io/oauth2_proxy/configuration#redis-storage) should resolve this. +Note: When using the Azure Auth provider with nginx and the cookie session store you may find the cookie is too large and doesn't get passed through correctly. Increasing the proxy_buffer_size in nginx or implementing the redis session storage (see https://pusher.github.io/oauth2_proxy/configuration#redis-storage) should resolve this. ### Facebook Auth Provider From 0b2eb91fa43cc04a4ac324f9bd9b1eeb6f2c5d70 Mon Sep 17 00:00:00 2001 From: leyshon Date: Thu, 3 Oct 2019 11:46:04 +0100 Subject: [PATCH 07/17] Update docs/2_auth.md Co-Authored-By: Joel Speed --- docs/2_auth.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/2_auth.md b/docs/2_auth.md index acef43b0..991bbae2 100644 --- a/docs/2_auth.md +++ b/docs/2_auth.md @@ -81,7 +81,7 @@ Note: The user is checked against the group members list on initial authenticati --client-secret= ``` -Note: When using the Azure Auth provider with nginx and the cookie session store you may find the cookie is too large and doesn't get passed through correctly. Increasing the proxy_buffer_size in nginx or implementing the redis session storage (see https://pusher.github.io/oauth2_proxy/configuration#redis-storage) should resolve this. +Note: When using the Azure Auth provider with nginx and the cookie session store you may find the cookie is too large and doesn't get passed through correctly. Increasing the proxy_buffer_size in nginx or implementing the [redis session storage](configuration#redis-storage) should resolve this. ### Facebook Auth Provider From e04411a789cde2b6819b31759fd9c3123518fb72 Mon Sep 17 00:00:00 2001 From: Alex Date: Mon, 14 Oct 2019 02:33:18 +0400 Subject: [PATCH 08/17] Update README - add more badges (#281) --- README.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index ad88331c..09fde41e 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,10 @@ # oauth2_proxy +[![Build Status](https://secure.travis-ci.org/pusher/oauth2_proxy.svg?branch=master)](http://travis-ci.org/pusher/oauth2_proxy) +[![Go Report Card](https://goreportcard.com/badge/github.com/pusher/oauth2_proxy)](https://goreportcard.com/report/github.com/pusher/oauth2_proxy) +[![GoDoc](https://godoc.org/github.com/pusher/oauth2_proxy?status.svg)](https://godoc.org/github.com/pusher/oauth2_proxy) +[![MIT licensed](https://img.shields.io/badge/license-MIT-blue.svg)](./LICENSE) + A reverse proxy and static file server that provides authentication using Providers (Google, GitHub, and others) to validate accounts by email, domain or group. @@ -7,8 +12,6 @@ to validate accounts by email, domain or group. Versions v3.0.0 and up are from this fork and will have diverged from any changes in the original fork. A list of changes can be seen in the [CHANGELOG](CHANGELOG.md). -[![Build Status](https://secure.travis-ci.org/pusher/oauth2_proxy.svg?branch=master)](http://travis-ci.org/pusher/oauth2_proxy) - ![Sign In Page](https://cloud.githubusercontent.com/assets/45028/4970624/7feb7dd8-6886-11e4-93e0-c9904af44ea8.png) ## Installation From e24e4ef8808e0d886d82f265ccf8ec2910eba764 Mon Sep 17 00:00:00 2001 From: Josh Michielsen Date: Thu, 17 Oct 2019 16:30:18 +0100 Subject: [PATCH 09/17] Add force-https option and flag Signed-off-by: Josh Michielsen --- main.go | 1 + options.go | 2 ++ 2 files changed, 3 insertions(+) diff --git a/main.go b/main.go index a4bf378e..5baa8cd2 100644 --- a/main.go +++ b/main.go @@ -32,6 +32,7 @@ func main() { flagSet.String("http-address", "127.0.0.1:4180", "[http://]: or unix:// to listen on for HTTP clients") flagSet.String("https-address", ":443", ": to listen on for HTTPS clients") + flagSet.Bool("force-https", false, "force HTTPS redirect") 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\"") 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", From aae91b0ad6cddae6d7ea17085f4713567aba248a Mon Sep 17 00:00:00 2001 From: Josh Michielsen Date: Thu, 17 Oct 2019 16:30:48 +0100 Subject: [PATCH 10/17] Add new handler to redirect to HTTPS if flag is set Signed-off-by: Josh Michielsen --- http.go | 10 ++++++++++ main.go | 4 ++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/http.go b/http.go index 2cee227b..0c84fb4b 100644 --- a/http.go +++ b/http.go @@ -152,3 +152,13 @@ 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) { + if opts.ForceHTTPS { + http.Redirect(w, r, opts.HTTPSAddress, http.StatusPermanentRedirect) + } + + h.ServeHTTP(w, r) + }) +} diff --git a/main.go b/main.go index 5baa8cd2..fb859064 100644 --- a/main.go +++ b/main.go @@ -186,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, From 271efe776e3b01b5526f1b284e48732e1131b5c0 Mon Sep 17 00:00:00 2001 From: Josh Michielsen Date: Thu, 17 Oct 2019 16:37:36 +0100 Subject: [PATCH 11/17] Added tests Signed-off-by: Josh Michielsen --- http_test.go | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/http_test.go b/http_test.go index f5ee1421..bfb9bb24 100644 --- a/http_test.go +++ b/http_test.go @@ -106,3 +106,32 @@ 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) +} From bed0336608ca139cd5617be1949ae05aa6f6dc0e Mon Sep 17 00:00:00 2001 From: Josh Michielsen Date: Thu, 17 Oct 2019 22:04:24 +0100 Subject: [PATCH 12/17] Add SSL check and test no redirect when HTTPS Signed-off-by: Josh Michielsen --- http.go | 2 +- http_test.go | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/http.go b/http.go index 0c84fb4b..9ef0499c 100644 --- a/http.go +++ b/http.go @@ -155,7 +155,7 @@ func (ln tcpKeepAliveListener) Accept() (c net.Conn, err error) { func redirectToHTTPS(opts *Options, h http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if opts.ForceHTTPS { + if opts.ForceHTTPS && r.TLS == nil { http.Redirect(w, r, opts.HTTPSAddress, http.StatusPermanentRedirect) } diff --git a/http_test.go b/http_test.go index bfb9bb24..400213a0 100644 --- a/http_test.go +++ b/http_test.go @@ -135,3 +135,24 @@ func TestRedirectToHTTPSFalse(t *testing.T) { 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) +} From 56d195a433d1c15d860e22fd47b619ac52765be5 Mon Sep 17 00:00:00 2001 From: Josh Michielsen Date: Thu, 17 Oct 2019 22:20:15 +0100 Subject: [PATCH 13/17] Docs and changelog Signed-off-by: Josh Michielsen --- CHANGELOG.md | 1 + docs/configuration/configuration.md | 1 + 2 files changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d76e8958..d3c1e4dc 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) 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 | From 9d0a0c74265ec190bb87d1825ac4618da8ee8910 Mon Sep 17 00:00:00 2001 From: Dan Bond Date: Fri, 18 Oct 2019 08:49:33 -0700 Subject: [PATCH 14/17] remove unnecessary validator tests (#288) * remove unnecessary validator tests * fix WriteString error --- validator_test.go | 75 +++++++++++++++++++----- validator_watcher_copy_test.go | 48 ---------------- validator_watcher_test.go | 102 --------------------------------- 3 files changed, 62 insertions(+), 163 deletions(-) delete mode 100644 validator_watcher_copy_test.go delete mode 100644 validator_watcher_test.go diff --git a/validator_test.go b/validator_test.go index 6e72cdbe..58253e8e 100644 --- a/validator_test.go +++ b/validator_test.go @@ -8,30 +8,34 @@ import ( ) type ValidatorTest struct { - authEmailFile *os.File - done chan bool - updateSeen bool + authEmailFileName string + done chan bool + updateSeen bool } func NewValidatorTest(t *testing.T) *ValidatorTest { vt := &ValidatorTest{} var err error - vt.authEmailFile, err = ioutil.TempFile("", "test_auth_emails_") + f, err := ioutil.TempFile("", "test_auth_emails_") if err != nil { - t.Fatal("failed to create temp file: " + err.Error()) + t.Fatalf("failed to create temp file: %v", err) } + if err := f.Close(); err != nil { + t.Fatalf("failed to close temp file: %v", err) + } + vt.authEmailFileName = f.Name() vt.done = make(chan bool, 1) return vt } func (vt *ValidatorTest) TearDown() { vt.done <- true - os.Remove(vt.authEmailFile.Name()) + os.Remove(vt.authEmailFileName) } func (vt *ValidatorTest) NewValidator(domains []string, updated chan<- bool) func(string) bool { - return newValidatorImpl(domains, vt.authEmailFile.Name(), + return newValidatorImpl(domains, vt.authEmailFileName, vt.done, func() { if vt.updateSeen == false { updated <- true @@ -40,13 +44,18 @@ func (vt *ValidatorTest) NewValidator(domains []string, }) } -// This will close vt.authEmailFile. func (vt *ValidatorTest) WriteEmails(t *testing.T, emails []string) { - defer vt.authEmailFile.Close() - vt.authEmailFile.WriteString(strings.Join(emails, "\n")) - if err := vt.authEmailFile.Close(); err != nil { - t.Fatal("failed to close temp file " + - vt.authEmailFile.Name() + ": " + err.Error()) + f, err := os.OpenFile(vt.authEmailFileName, os.O_WRONLY, 0600) + if err != nil { + t.Fatalf("failed to open auth email file: %v", err) + } + + if _, err := f.WriteString(strings.Join(emails, "\n")); err != nil { + t.Fatalf("failed to write emails to auth email file: %v", err) + } + + if err := f.Close(); err != nil { + t.Fatalf("failed to close auth email file: %v", err) } } @@ -160,3 +169,43 @@ func TestValidatorIgnoreSpacesInAuthEmails(t *testing.T) { t.Error("email should validate") } } + +func TestValidatorOverwriteEmailListDirectly(t *testing.T) { + vt := NewValidatorTest(t) + defer vt.TearDown() + + vt.WriteEmails(t, []string{ + "xyzzy@example.com", + "plugh@example.com", + }) + domains := []string(nil) + updated := make(chan bool) + validator := vt.NewValidator(domains, updated) + + if !validator("xyzzy@example.com") { + t.Error("first email in list should validate") + } + if !validator("plugh@example.com") { + t.Error("second email in list should validate") + } + if validator("xyzzy.plugh@example.com") { + t.Error("email not in list that matches no domains " + + "should not validate") + } + + vt.WriteEmails(t, []string{ + "xyzzy.plugh@example.com", + "plugh@example.com", + }) + <-updated + + if validator("xyzzy@example.com") { + t.Error("email removed from list should not validate") + } + if !validator("plugh@example.com") { + t.Error("email retained in list should validate") + } + if !validator("xyzzy.plugh@example.com") { + t.Error("email added to list should validate") + } +} diff --git a/validator_watcher_copy_test.go b/validator_watcher_copy_test.go deleted file mode 100644 index 15ed6faf..00000000 --- a/validator_watcher_copy_test.go +++ /dev/null @@ -1,48 +0,0 @@ -// +build go1.3,!plan9,!solaris,!windows - -// Turns out you can't copy over an existing file on Windows. - -package main - -import ( - "io/ioutil" - "os" - "testing" -) - -func (vt *ValidatorTest) UpdateEmailFileViaCopyingOver( - t *testing.T, emails []string) { - origFile := vt.authEmailFile - var err error - vt.authEmailFile, err = ioutil.TempFile("", "test_auth_emails_") - if err != nil { - t.Fatal("failed to create temp file for copy: " + err.Error()) - } - vt.WriteEmails(t, emails) - err = os.Rename(vt.authEmailFile.Name(), origFile.Name()) - if err != nil { - t.Fatal("failed to copy over temp file: " + err.Error()) - } - vt.authEmailFile = origFile -} - -func TestValidatorOverwriteEmailListViaCopyingOver(t *testing.T) { - vt := NewValidatorTest(t) - defer vt.TearDown() - - vt.WriteEmails(t, []string{"xyzzy@example.com"}) - domains := []string(nil) - updated := make(chan bool) - validator := vt.NewValidator(domains, updated) - - if !validator("xyzzy@example.com") { - t.Error("email in list should validate") - } - - vt.UpdateEmailFileViaCopyingOver(t, []string{"plugh@example.com"}) - <-updated - - if validator("xyzzy@example.com") { - t.Error("email removed from list should not validate") - } -} diff --git a/validator_watcher_test.go b/validator_watcher_test.go deleted file mode 100644 index b022d68f..00000000 --- a/validator_watcher_test.go +++ /dev/null @@ -1,102 +0,0 @@ -// +build go1.3,!plan9,!solaris - -package main - -import ( - "io/ioutil" - "os" - "testing" -) - -func (vt *ValidatorTest) UpdateEmailFile(t *testing.T, emails []string) { - var err error - vt.authEmailFile, err = os.OpenFile( - vt.authEmailFile.Name(), os.O_WRONLY|os.O_CREATE, 0600) - if err != nil { - t.Fatal("failed to re-open temp file for updates") - } - vt.WriteEmails(t, emails) -} - -func (vt *ValidatorTest) UpdateEmailFileViaRenameAndReplace( - t *testing.T, emails []string) { - origFile := vt.authEmailFile - var err error - vt.authEmailFile, err = ioutil.TempFile("", "test_auth_emails_") - if err != nil { - t.Fatal("failed to create temp file for rename and replace: " + - err.Error()) - } - vt.WriteEmails(t, emails) - - movedName := origFile.Name() + "-moved" - err = os.Rename(origFile.Name(), movedName) - err = os.Rename(vt.authEmailFile.Name(), origFile.Name()) - if err != nil { - t.Fatal("failed to rename and replace temp file: " + - err.Error()) - } - vt.authEmailFile = origFile - os.Remove(movedName) -} - -func TestValidatorOverwriteEmailListDirectly(t *testing.T) { - vt := NewValidatorTest(t) - defer vt.TearDown() - - vt.WriteEmails(t, []string{ - "xyzzy@example.com", - "plugh@example.com", - }) - domains := []string(nil) - updated := make(chan bool) - validator := vt.NewValidator(domains, updated) - - if !validator("xyzzy@example.com") { - t.Error("first email in list should validate") - } - if !validator("plugh@example.com") { - t.Error("second email in list should validate") - } - if validator("xyzzy.plugh@example.com") { - t.Error("email not in list that matches no domains " + - "should not validate") - } - - vt.UpdateEmailFile(t, []string{ - "xyzzy.plugh@example.com", - "plugh@example.com", - }) - <-updated - - if validator("xyzzy@example.com") { - t.Error("email removed from list should not validate") - } - if !validator("plugh@example.com") { - t.Error("email retained in list should validate") - } - if !validator("xyzzy.plugh@example.com") { - t.Error("email added to list should validate") - } -} - -func TestValidatorOverwriteEmailListViaRenameAndReplace(t *testing.T) { - vt := NewValidatorTest(t) - defer vt.TearDown() - - vt.WriteEmails(t, []string{"xyzzy@example.com"}) - domains := []string(nil) - updated := make(chan bool, 1) - validator := vt.NewValidator(domains, updated) - - if !validator("xyzzy@example.com") { - t.Error("email in list should validate") - } - - vt.UpdateEmailFileViaRenameAndReplace(t, []string{"plugh@example.com"}) - <-updated - - if validator("xyzzy@example.com") { - t.Error("email removed from list should not validate") - } -} From dcc430f6f141f2dc4071952ae2f6c3e7a1e15979 Mon Sep 17 00:00:00 2001 From: Josh Michielsen Date: Mon, 21 Oct 2019 23:21:35 +0100 Subject: [PATCH 15/17] Check `X-Forwared-Proto` for https (via another reverse proxy) Signed-off-by: Josh Michielsen --- http.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/http.go b/http.go index 9ef0499c..b7ee9598 100644 --- a/http.go +++ b/http.go @@ -155,7 +155,8 @@ func (ln tcpKeepAliveListener) Accept() (c net.Conn, err error) { func redirectToHTTPS(opts *Options, h http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if opts.ForceHTTPS && r.TLS == nil { + proto := r.Header.Get("X-Forwarded-Proto") + if opts.ForceHTTPS && r.TLS == nil && strings.ToLower(proto) != "https" { http.Redirect(w, r, opts.HTTPSAddress, http.StatusPermanentRedirect) } From fe9efba0c524c4356ef8e8020f20052954e04aa3 Mon Sep 17 00:00:00 2001 From: Josh Michielsen Date: Tue, 22 Oct 2019 14:19:39 +0100 Subject: [PATCH 16/17] Documentation change Co-Authored-By: Joel Speed --- main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/main.go b/main.go index fb859064..e84a796e 100644 --- a/main.go +++ b/main.go @@ -32,7 +32,7 @@ func main() { flagSet.String("http-address", "127.0.0.1:4180", "[http://]: or unix:// to listen on for HTTP clients") flagSet.String("https-address", ":443", ": to listen on for HTTPS clients") - flagSet.Bool("force-https", false, "force HTTPS redirect") + 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\"") From c0bfe0357a06339831f50a997a6708ea18ac745b Mon Sep 17 00:00:00 2001 From: Josh Michielsen Date: Tue, 22 Oct 2019 14:21:06 +0100 Subject: [PATCH 17/17] Confirm that the proto is not empty, and change condition to OR Co-Authored-By: Joel Speed --- http.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/http.go b/http.go index b7ee9598..88280c44 100644 --- a/http.go +++ b/http.go @@ -156,7 +156,7 @@ func (ln tcpKeepAliveListener) Accept() (c net.Conn, err error) { 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 && strings.ToLower(proto) != "https" { + if opts.ForceHTTPS && (r.TLS == nil || (proto != "" && strings.ToLower(proto) != "https")) { http.Redirect(w, r, opts.HTTPSAddress, http.StatusPermanentRedirect) }