Repository: pusher/oauth2_proxy
Pull request #434 (2020-03-03 22:44:20)

Support the PreferEmailToUser option on PassUserHeaders
## Description

Previously in #401, an option was added to support forwarding the email
address as the username to the upstream service when the PassBasicAuth
option is used.

The PassBasicAuth option is not appropriate for all users, with PassUserHeaders
allowing very similar functionality without specifying a basic auth headers.

The PreferEmailToUser option has been expanded to support the PassUserHeaders
option.

## Motivation and Context

For the application I run, I use the PassUserHeaders option but since upgrading to 5.0.0 the username header has been providing the user ID, as discussed in #369.

Now, with this change, I can consistently authenticate based on email address.

## How Has This Been Tested?

Unit tests have been added for this scenario and I've deployed a build with this change to my server using this new option and confirmed it works successfully.

## 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 74b2f624..a20e907b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -18,6 +18,7 @@
 - [#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)
+- [#434](https://github.com/pusher/oauth2_proxy/pull/434) Give the option to prefer email address in the username header when using the -pass-user-headers option (@jordancrawfordnz)
 
 # v5.0.0
 
diff --git a/docs/configuration/configuration.md b/docs/configuration/configuration.md
index e272944b..40efd01c 100644
--- a/docs/configuration/configuration.md
+++ b/docs/configuration/configuration.md
@@ -74,7 +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, X-Forwarded-Email and X-Forwarded-Preferred-Username 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 |
+| `-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. Used in conjunction with `-pass-basic-auth` and `-pass-user-headers` | false |
 | `-pass-host-header` | bool | pass the request Host Header to upstream | true |
 | `-pass-user-headers` | bool | pass X-Forwarded-User, X-Forwarded-Email and X-Forwarded-Preferred-Username information to upstream | true |
 | `-profile-url` | string | Profile access endpoint | |
diff --git a/main.go b/main.go
index 0c40e3a8..d2a4f306 100644
--- a/main.go
+++ b/main.go
@@ -41,7 +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("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. Used in conjunction with -pass-basic-auth and -pass-user-headers")
 	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 b3717e34..586c1e63 100644
--- a/oauthproxy.go
+++ b/oauthproxy.go
@@ -959,12 +959,18 @@ func (p *OAuthProxy) addHeadersForProxying(rw http.ResponseWriter, req *http.Req
 	}
 
 	if p.PassUserHeaders {
-		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.Header["X-Forwarded-User"] = []string{session.Email}
 			req.Header.Del("X-Forwarded-Email")
+		} else {
+			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")
+			}
 		}
+
 		if session.PreferredUsername != "" {
 			req.Header["X-Forwarded-Preferred-Username"] = []string{session.PreferredUsername}
 		} else {
diff --git a/options.go b/options.go
index 7b19ffcb..aed1ae54 100644
--- a/options.go
+++ b/options.go
@@ -281,8 +281,8 @@ func (o *Options) Validate() error {
 		}
 	}
 
-	if o.PreferEmailToUser == true && o.PassBasicAuth == false {
-		msgs = append(msgs, "PreferEmailToUser should only be used with PassBasicAuth")
+	if o.PreferEmailToUser == true && o.PassBasicAuth == false && o.PassUserHeaders == false {
+		msgs = append(msgs, "PreferEmailToUser should only be used with PassBasicAuth or PassUserHeaders")
 	}
 
 	if o.SkipJwtBearerTokens {


Test patch:
diff --git a/oauthproxy_test.go b/oauthproxy_test.go
index 2dd01151..1a576f97 100644
--- a/oauthproxy_test.go
+++ b/oauthproxy_test.go
@@ -511,23 +511,45 @@ func TestBasicAuthWithEmail(t *testing.T) {
 		assert.Equal(t, expectedEmailHeader, req.Header["Authorization"][0])
 		assert.Equal(t, emailAddress, req.Header["X-Forwarded-User"][0])
 	}
+}
 
+func TestPassUserHeadersWithEmail(t *testing.T) {
+	opts := NewOptions()
+	opts.PassBasicAuth = false
 	opts.PassUserHeaders = true
+	opts.PreferEmailToUser = false
+	opts.Validate()
+
+	const emailAddress = "john.doe@example.com"
+	const userName = "9fcab5c9b889a557"
+
+	session := &sessions.SessionState{
+		User:        userName,
+		Email:       emailAddress,
+		AccessToken: "oauth_token",
+		CreatedAt:   time.Now(),
+	}
 	{
-		// 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)
+		req, _ := http.NewRequest("GET", opts.ProxyPrefix+"/testCase0", 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])
 	}
+
+	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, emailAddress, req.Header["X-Forwarded-User"][0])
+	}
 }
 
 type PassAccessTokenTest struct {