Repository: pusher/oauth2_proxy
Pull request #401 (2020-02-18 16:08:29)

Add option to prefer an Email address to a Username
## Description

Implement an option to prefer using email address as the username to upstream services.

## Motivation and Context

With some providers the Username is an upstream Unique ID, like fex. in the
case of Google, where you get a stable string of characters that are guaranteed to be unique for Google..

When matching this with downstream databases, it's sometimes preferred to use
the email address as the  known identifier.

However, when _mixing_ this with sometimes other sources, like htaccess, which
doesn't have a concept of an email address, it can turn difficult, as you can no longer only trust the "X-Forwarded-Email" header but must manage fallback handling, etc.

This change makes the headers _prefer_ to use the Email address, if such exists,
for the Username identifier when passing data to downstream services. 

Defaults to Off.

## How Has This Been Tested?

Unittests only

## 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.


Spindel: Now it's been live tested as well.

Spindel: Thanks for the review, I'll fix that up on Monday and re-push my branch

Spindel: Squashed the changes up, should be all clear.
Spindel: Since there's some conflicts now vs. upstream/master, how should I proceed with this?  Do you want me to rebase my branch, or merge master into this?



JoelSpeed: Rebase would be preferable if that's ok
Spindel: Rebased the changes.
Spindel: So, rebasing drops the reviews as stale? Annoying. 

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index a8bea95f..74b2f624 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -16,6 +16,7 @@
 - [#363](https://github.com/pusher/oauth2_proxy/pull/363) Extension of Redis Session Store to Support Redis Cluster (@yan-dblinf)
 - [#353](https://github.com/pusher/oauth2_proxy/pull/353) Fix login page fragment handling after soft reload on Firefox (@ffdybuster)
 - [#355](https://github.com/pusher/oauth2_proxy/pull/355) Add Client Secret File support for providers that rotate client secret via file system (@pasha-r)
+- [#401](https://github.com/pusher/oauth2_proxy/pull/401) Give the option to pass email address in the Basic auth header instead of upstream usernames. (@Spindel)
 - [#405](https://github.com/pusher/oauth2_proxy/pull/405) The `/sign_in` page now honors the `rd` query parameter, fixing the redirect after a successful authentication (@ti-mo)
 
 # v5.0.0
diff --git a/docs/configuration/configuration.md b/docs/configuration/configuration.md
index aa9164ce..b765b5a1 100644
--- a/docs/configuration/configuration.md
+++ b/docs/configuration/configuration.md
@@ -74,6 +74,7 @@ An example [oauth2_proxy.cfg]({{ site.gitweb }}/contrib/oauth2_proxy.cfg.example
 | `-pass-access-token` | bool | pass OAuth access_token to upstream via X-Forwarded-Access-Token header | false |
 | `-pass-authorization-header` | bool | pass OIDC IDToken to upstream via Authorization Bearer header | false |
 | `-pass-basic-auth` | bool | pass HTTP Basic Auth, X-Forwarded-User and X-Forwarded-Email information to upstream | true |
+| `-prefer-email-to-user` | bool | Prefer to use the Email address as the Username when passing information to upstream. Will only use Username if Email is unavailable, eg. htaccess authentication. | false |
 | `-pass-host-header` | bool | pass the request Host Header to upstream | true |
 | `-pass-user-headers` | bool | pass X-Forwarded-User and X-Forwarded-Email information to upstream | true |
 | `-profile-url` | string | Profile access endpoint | |
diff --git a/main.go b/main.go
index 0fb0d0f5..0c40e3a8 100644
--- a/main.go
+++ b/main.go
@@ -41,6 +41,7 @@ func main() {
 	flagSet.Bool("set-xauthrequest", false, "set X-Auth-Request-User and X-Auth-Request-Email response headers (useful in Nginx auth_request mode)")
 	flagSet.Var(&upstreams, "upstream", "the http url(s) of the upstream endpoint, file:// paths for static files or static://<status_code> for static response. Routing is based on the path")
 	flagSet.Bool("pass-basic-auth", true, "pass HTTP Basic Auth, X-Forwarded-User and X-Forwarded-Email information to upstream")
+	flagSet.Bool("prefer-email-to-user", false, "Prefer to use the Email address as the Username when passing information to upstream. Will only use Username if Email is unavailable, eg. htaccess authentication.")
 	flagSet.Bool("pass-user-headers", true, "pass X-Forwarded-User and X-Forwarded-Email information to upstream")
 	flagSet.String("basic-auth-password", "", "the password to set when passing the HTTP Basic Auth header")
 	flagSet.Bool("pass-access-token", false, "pass OAuth access_token to upstream via X-Forwarded-Access-Token header")
diff --git a/oauthproxy.go b/oauthproxy.go
index 76db15ad..cf4ca1f7 100644
--- a/oauthproxy.go
+++ b/oauthproxy.go
@@ -99,6 +99,7 @@ type OAuthProxy struct {
 	PassAccessToken      bool
 	SetAuthorization     bool
 	PassAuthorization    bool
+	PreferEmailToUser    bool
 	skipAuthRegex        []string
 	skipAuthPreflight    bool
 	skipJwtBearerTokens  bool
@@ -305,6 +306,7 @@ func NewOAuthProxy(opts *Options, validator func(string) bool) *OAuthProxy {
 		PassAccessToken:      opts.PassAccessToken,
 		SetAuthorization:     opts.SetAuthorization,
 		PassAuthorization:    opts.PassAuthorization,
+		PreferEmailToUser:    opts.PreferEmailToUser,
 		SkipProviderButton:   opts.SkipProviderButton,
 		templates:            loadTemplates(opts.CustomTemplatesDir),
 		Banner:               opts.Banner,
@@ -924,12 +926,18 @@ func (p *OAuthProxy) getAuthenticatedSession(rw http.ResponseWriter, req *http.R
 // addHeadersForProxying adds the appropriate headers the request / response for proxying
 func (p *OAuthProxy) addHeadersForProxying(rw http.ResponseWriter, req *http.Request, session *sessionsapi.SessionState) {
 	if p.PassBasicAuth {
-		req.SetBasicAuth(session.User, p.BasicAuthPassword)
-		req.Header["X-Forwarded-User"] = []string{session.User}
-		if session.Email != "" {
-			req.Header["X-Forwarded-Email"] = []string{session.Email}
-		} else {
+		if p.PreferEmailToUser && session.Email != "" {
+			req.SetBasicAuth(session.Email, p.BasicAuthPassword)
+			req.Header["X-Forwarded-User"] = []string{session.Email}
 			req.Header.Del("X-Forwarded-Email")
+		} else {
+			req.SetBasicAuth(session.User, p.BasicAuthPassword)
+			req.Header["X-Forwarded-User"] = []string{session.User}
+			if session.Email != "" {
+				req.Header["X-Forwarded-Email"] = []string{session.Email}
+			} else {
+				req.Header.Del("X-Forwarded-Email")
+			}
 		}
 	}
 
diff --git a/options.go b/options.go
index 97dfbf60..7b19ffcb 100644
--- a/options.go
+++ b/options.go
@@ -73,6 +73,7 @@ type Options struct {
 	SkipJwtBearerTokens           bool          `flag:"skip-jwt-bearer-tokens" cfg:"skip_jwt_bearer_tokens" env:"OAUTH2_PROXY_SKIP_JWT_BEARER_TOKENS"`
 	ExtraJwtIssuers               []string      `flag:"extra-jwt-issuers" cfg:"extra_jwt_issuers" env:"OAUTH2_PROXY_EXTRA_JWT_ISSUERS"`
 	PassBasicAuth                 bool          `flag:"pass-basic-auth" cfg:"pass_basic_auth" env:"OAUTH2_PROXY_PASS_BASIC_AUTH"`
+	PreferEmailToUser             bool          `flag:"prefer-email-to-user" cfg:"prefer_email_to_user" env:"OAUTH2_PROXY_PREFER_EMAIL_TO_USER"`
 	BasicAuthPassword             string        `flag:"basic-auth-password" cfg:"basic_auth_password" env:"OAUTH2_PROXY_BASIC_AUTH_PASSWORD"`
 	PassAccessToken               bool          `flag:"pass-access-token" cfg:"pass_access_token" env:"OAUTH2_PROXY_PASS_ACCESS_TOKEN"`
 	PassHostHeader                bool          `flag:"pass-host-header" cfg:"pass_host_header" env:"OAUTH2_PROXY_PASS_HOST_HEADER"`
@@ -169,6 +170,7 @@ func NewOptions() *Options {
 		PassHostHeader:                   true,
 		SetAuthorization:                 false,
 		PassAuthorization:                false,
+		PreferEmailToUser:                false,
 		ApprovalPrompt:                   "force",
 		InsecureOIDCAllowUnverifiedEmail: false,
 		SkipOIDCDiscovery:                false,
@@ -279,6 +281,10 @@ func (o *Options) Validate() error {
 		}
 	}
 
+	if o.PreferEmailToUser == true && o.PassBasicAuth == false {
+		msgs = append(msgs, "PreferEmailToUser should only be used with PassBasicAuth")
+	}
+
 	if o.SkipJwtBearerTokens {
 		// If we are using an oidc provider, go ahead and add that provider to the list
 		if o.oidcVerifier != nil {


Test patch:
diff --git a/oauthproxy_test.go b/oauthproxy_test.go
index abe449d9..2dd01151 100644
--- a/oauthproxy_test.go
+++ b/oauthproxy_test.go
@@ -412,6 +412,7 @@ func TestBasicAuthPassword(t *testing.T) {
 	opts.CookieSecure = false
 	opts.PassBasicAuth = true
 	opts.PassUserHeaders = true
+	opts.PreferEmailToUser = true
 	opts.BasicAuthPassword = "This is a secure password"
 	opts.Validate()
 
@@ -466,6 +467,69 @@ func TestBasicAuthPassword(t *testing.T) {
 	providerServer.Close()
 }
 
+func TestBasicAuthWithEmail(t *testing.T) {
+	opts := NewOptions()
+	opts.PassBasicAuth = true
+	opts.PassUserHeaders = false
+	opts.PreferEmailToUser = false
+	opts.BasicAuthPassword = "This is a secure password"
+	opts.Validate()
+
+	const emailAddress = "john.doe@example.com"
+	const userName = "9fcab5c9b889a557"
+
+	// The username in the basic auth credentials is expected to be equal to the email address from the
+	expectedEmailHeader := "Basic " + base64.StdEncoding.EncodeToString([]byte(emailAddress+":"+opts.BasicAuthPassword))
+	expectedUserHeader := "Basic " + base64.StdEncoding.EncodeToString([]byte(userName+":"+opts.BasicAuthPassword))
+
+	session := &sessions.SessionState{
+		User:        userName,
+		Email:       emailAddress,
+		AccessToken: "oauth_token",
+		CreatedAt:   time.Now(),
+	}
+	{
+		rw := httptest.NewRecorder()
+		req, _ := http.NewRequest("GET", opts.ProxyPrefix+"/testCase0", nil)
+		proxy := NewOAuthProxy(opts, func(email string) bool {
+			return email == emailAddress
+		})
+		proxy.addHeadersForProxying(rw, req, session)
+		assert.Equal(t, expectedUserHeader, req.Header["Authorization"][0])
+		assert.Equal(t, userName, req.Header["X-Forwarded-User"][0])
+	}
+
+	opts.PreferEmailToUser = true
+	{
+		rw := httptest.NewRecorder()
+		req, _ := http.NewRequest("GET", opts.ProxyPrefix+"/testCase1", nil)
+
+		proxy := NewOAuthProxy(opts, func(email string) bool {
+			return email == emailAddress
+		})
+		proxy.addHeadersForProxying(rw, req, session)
+		assert.Equal(t, expectedEmailHeader, req.Header["Authorization"][0])
+		assert.Equal(t, emailAddress, req.Header["X-Forwarded-User"][0])
+	}
+
+	opts.PassUserHeaders = true
+	{
+		// PassUserHeaders takes predecense over the headers added by
+		// PassBasicAuth, thus we expect them to contain something else.
+		rw := httptest.NewRecorder()
+		req, _ := http.NewRequest("GET", opts.ProxyPrefix+"/testCase2", nil)
+		proxy := NewOAuthProxy(opts, func(email string) bool {
+			return email == emailAddress
+		})
+
+		proxy.addHeadersForProxying(rw, req, session)
+		// The user address here should still be an email.
+		assert.Equal(t, expectedEmailHeader, req.Header["Authorization"][0])
+		assert.Equal(t, emailAddress, req.Header["X-Forwarded-Email"][0])
+		assert.Equal(t, userName, req.Header["X-Forwarded-User"][0])
+	}
+}
+
 type PassAccessTokenTest struct {
 	providerServer *httptest.Server
 	proxy          *OAuthProxy