From 4cd43ef39733c9301e286891896d91a868abff1f Mon Sep 17 00:00:00 2001 From: Jordan Crawford Date: Wed, 4 Mar 2020 11:27:43 +1300 Subject: [PATCH] Support the PreferEmailToUser option on PassUserHeaders 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. --- CHANGELOG.md | 1 + docs/configuration/configuration.md | 2 +- main.go | 2 +- oauthproxy.go | 14 +++++++---- oauthproxy_test.go | 36 +++++++++++++++++++++++------ options.go | 4 ++-- 6 files changed, 44 insertions(+), 15 deletions(-) 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:// 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/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 { 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 {