Merge branch 'master' into kamal/whitelist-redirects-with-ports

CHANGELOG.md

@@ -3,8 +3,11 @@
 ## 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
+  - 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
 

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

docs/2_auth.md

@@ -81,6 +81,8 @@ Note: The user is checked against the group members list on initial authenticati
    --client-secret=<value from step 6>
 ```
 
+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
 
 1.  Create a new FB App from <https://developers.facebook.com/>

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 |

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)
+	})
+}

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)
+}

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,

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",

providers/azure.go

@@ -1,10 +1,14 @@
 package providers
 
 import (
+	"bytes"
+	"encoding/json"
 	"errors"
 	"fmt"
+	"io/ioutil"
 	"net/http"
 	"net/url"
+	"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))

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"
@@ -20,6 +21,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 +113,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 +204,19 @@ 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", "expires_on": "1136239445", "refresh_token": "refresh1234" }`)
+	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)
+	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)
+	assert.Equal(t, timestamp, s.ExpiresOn.UTC())
+	assert.Equal(t, "refresh1234", s.RefreshToken)
+}

validator_test.go

@@ -8,7 +8,7 @@ import (
 )
 
 type ValidatorTest struct {
-	authEmailFile *os.File
+	authEmailFileName string
 	done              chan bool
 	updateSeen        bool
 }
@@ -16,22 +16,26 @@ type ValidatorTest struct {
 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()
+	f, err := os.OpenFile(vt.authEmailFileName, os.O_WRONLY, 0600)
-	vt.authEmailFile.WriteString(strings.Join(emails, "\n"))
+	if err != nil {
-	if err := vt.authEmailFile.Close(); err != nil {
+		t.Fatalf("failed to open auth email file: %v", err)
-		t.Fatal("failed to close temp file " +
+	}
-			vt.authEmailFile.Name() + ": " + err.Error())
+
+	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")
+	}
+}

validator_watcher_copy_test.go

@@ -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")
-	}
-}

validator_watcher_test.go

@@ -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")
-	}
-}