From 4e3dd09cf29fee601562ec3fc2c4d6d92841283d Mon Sep 17 00:00:00 2001
From: Joel Speed <Joel.speed@hotmail.co.uk>
Date: Tue, 12 May 2020 16:04:51 +0100
Subject: [PATCH] Drop fallback to email when user is empty (#537)

---
 CHANGELOG.md                            | 8 ++++++++
 oauthproxy.go                           | 8 ++++++--
 oauthproxy_test.go                      | 2 +-
 pkg/apis/sessions/session_state.go      | 3 ---
 pkg/apis/sessions/session_state_test.go | 6 +++---
 5 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 3511bb6..d0a2670 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -25,9 +25,17 @@
 - [#535](https://github.com/oauth2-proxy/oauth2-proxy/pull/535) Drop support for pre v3.1 cookies
   - The encoding for session cookies was changed starting in v3.1.0, support for the previous encoding is now dropped
   - If you are upgrading from a version earlier than this, please upgrade via a version between v3.1.0 and v5.1.1
+- [#537](https://github.com/oauth2-proxy/oauth2-proxy/pull/537) Drop Fallback to Email if User not set
+  - Previously, when a session was loaded, if the User was not set, it would be replaced by the Email.
+    This behaviour was inconsistent as it required the session to be stored and then loaded to function properly.
+  - This behaviour has now been removed and the User field will remain empty if it was not set when the session was saved.
+  - In some scenarios `X-Forwarded-User` will now be empty. Use `X-Forwarded-Email` instead.
+  - In some scenarios, this may break setting Basic Auth on upstream or responses.
+    Use `--prefer-email-to-user` to restore falling back to the Email in these cases.
 
 ## Changes since v5.1.1
 
+- [#537](https://github.com/oauth2-proxy/oauth2-proxy/pull/537) Drop Fallback to Email if User not set (@JoelSpeed)
 - [#535](https://github.com/oauth2-proxy/oauth2-proxy/pull/535) Drop support for pre v3.1 cookies (@JoelSpeed)
 - [#533](https://github.com/oauth2-proxy/oauth2-proxy/pull/487) Set up code coverage within Travis for Code Climate (@JoelSpeed)
 - [#514](https://github.com/oauth2-proxy/oauth2-proxy/pull/514) Add basic string functions to templates
diff --git a/oauthproxy.go b/oauthproxy.go
index 86d33c5..4293423 100644
--- a/oauthproxy.go
+++ b/oauthproxy.go
@@ -1048,10 +1048,14 @@ func (p *OAuthProxy) addHeadersForProxying(rw http.ResponseWriter, req *http.Req
 		}
 	}
 	if p.SetBasicAuth {
-		if session.User != "" {
+		switch {
+		case p.PreferEmailToUser && session.Email != "":
+			authVal := b64.StdEncoding.EncodeToString([]byte(session.Email + ":" + p.BasicAuthPassword))
+			rw.Header().Set("Authorization", "Basic "+authVal)
+		case session.User != "":
 			authVal := b64.StdEncoding.EncodeToString([]byte(session.User + ":" + p.BasicAuthPassword))
 			rw.Header().Set("Authorization", "Basic "+authVal)
-		} else {
+		default:
 			rw.Header().Del("Authorization")
 		}
 	}
diff --git a/oauthproxy_test.go b/oauthproxy_test.go
index 8957e18..33868f7 100644
--- a/oauthproxy_test.go
+++ b/oauthproxy_test.go
@@ -956,7 +956,7 @@ func TestLoadCookiedSession(t *testing.T) {
 	session, err := pcTest.LoadCookiedSession()
 	assert.Equal(t, nil, err)
 	assert.Equal(t, startSession.Email, session.Email)
-	assert.Equal(t, "john.doe@example.com", session.User)
+	assert.Equal(t, "", session.User)
 	assert.Equal(t, startSession.AccessToken, session.AccessToken)
 }
 
diff --git a/pkg/apis/sessions/session_state.go b/pkg/apis/sessions/session_state.go
index f970052..a09f01c 100644
--- a/pkg/apis/sessions/session_state.go
+++ b/pkg/apis/sessions/session_state.go
@@ -193,8 +193,5 @@ func DecodeSessionState(v string, c *encryption.Cipher) (*SessionState, error) {
 			}
 		}
 	}
-	if ss.User == "" {
-		ss.User = ss.Email
-	}
 	return ss, nil
 }
diff --git a/pkg/apis/sessions/session_state_test.go b/pkg/apis/sessions/session_state_test.go
index dbfea6e..94c624b 100644
--- a/pkg/apis/sessions/session_state_test.go
+++ b/pkg/apis/sessions/session_state_test.go
@@ -33,7 +33,7 @@ func TestSessionStateSerialization(t *testing.T) {
 	ss, err := sessions.DecodeSessionState(encoded, c)
 	t.Logf("%#v", ss)
 	assert.Equal(t, nil, err)
-	assert.Equal(t, "user@domain.com", ss.User)
+	assert.Equal(t, "", ss.User)
 	assert.Equal(t, s.Email, ss.Email)
 	assert.Equal(t, s.PreferredUsername, ss.PreferredUsername)
 	assert.Equal(t, s.AccessToken, ss.AccessToken)
@@ -112,7 +112,7 @@ func TestSessionStateSerializationNoCipher(t *testing.T) {
 	// only email should have been serialized
 	ss, err := sessions.DecodeSessionState(encoded, nil)
 	assert.Equal(t, nil, err)
-	assert.Equal(t, "user@domain.com", ss.User)
+	assert.Equal(t, "", ss.User)
 	assert.Equal(t, s.Email, ss.Email)
 	assert.Equal(t, s.PreferredUsername, ss.PreferredUsername)
 	assert.Equal(t, "", ss.AccessToken)
@@ -226,7 +226,7 @@ func TestDecodeSessionState(t *testing.T) {
 		{
 			SessionState: sessions.SessionState{
 				Email: "user@domain.com",
-				User:  "user@domain.com",
+				User:  "",
 			},
 			Encoded: `{"Email":"user@domain.com"}`,
 		},
-- 
GitLab