From cbd4ce654e2dc9aa350aefb44ad3abdad911a1c6 Mon Sep 17 00:00:00 2001
From: Nick Meves <nick.meves@greenhouse.io>
Date: Tue, 22 Jun 2021 19:54:06 -0700
Subject: [PATCH] Don't squash profileURL session fields during refresh

---
 CHANGELOG.md           |  2 ++
 providers/oidc.go      | 19 +++++++++++++++++--
 providers/oidc_test.go |  4 ++--
 3 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 94b02409..9931b52d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,6 +12,8 @@
 ## Breaking Changes
 
 ## Changes since v7.1.3
+
+- [#1251](https://github.com/oauth2-proxy/oauth2-proxy/pull/1251) Add safety checks to OIDC session fields during refresh (@NickMeves)
 - [#1233](https://github.com/oauth2-proxy/oauth2-proxy/pull/1233) Extend email-domain validation with sub-domain capability (@morarucostel)
 - [#1060](https://github.com/oauth2-proxy/oauth2-proxy/pull/1060) Implement RewriteTarget to allow requests to be rewritten before proxying to upstream servers (@JoelSpeed)
 - [#1086](https://github.com/oauth2-proxy/oauth2-proxy/pull/1086) Refresh sessions before token expiration if configured (@NickMeves)
diff --git a/providers/oidc.go b/providers/oidc.go
index b1711d54..28e15cb2 100644
--- a/providers/oidc.go
+++ b/providers/oidc.go
@@ -186,14 +186,31 @@ func (p *OIDCProvider) redeemRefreshToken(ctx context.Context, s *sessions.Sessi
 		return fmt.Errorf("unable create new session state from response: %v", err)
 	}
 
+	replaceSession(s, newSession)
+	return nil
+}
+
+func replaceSession(s *sessions.SessionState, newSession *sessions.SessionState) {
 	// It's possible that if the refresh token isn't in the token response the
 	// session will not contain an id token.
 	// If it doesn't it's probably better to retain the old one
 	if newSession.IDToken != "" {
 		s.IDToken = newSession.IDToken
+	}
+
+	// Only copy over fields if they are present. Otherwise they might've
+	// originally been set via a ProfileURL call in `EnrichSession` but
+	// are empty in IDToken claims.
+	if newSession.Email != "" {
 		s.Email = newSession.Email
+	}
+	if newSession.User != "" {
 		s.User = newSession.User
+	}
+	if newSession.Groups != nil {
 		s.Groups = newSession.Groups
+	}
+	if newSession.PreferredUsername != "" {
 		s.PreferredUsername = newSession.PreferredUsername
 	}
 
@@ -201,8 +218,6 @@ func (p *OIDCProvider) redeemRefreshToken(ctx context.Context, s *sessions.Sessi
 	s.RefreshToken = newSession.RefreshToken
 	s.CreatedAt = newSession.CreatedAt
 	s.ExpiresOn = newSession.ExpiresOn
-
-	return nil
 }
 
 // CreateSessionFromToken converts Bearer IDTokens into sessions
diff --git a/providers/oidc_test.go b/providers/oidc_test.go
index 9879678d..1c0f6dcf 100644
--- a/providers/oidc_test.go
+++ b/providers/oidc_test.go
@@ -464,7 +464,7 @@ func TestOIDCProvider_EnrichSession(t *testing.T) {
 	}
 }
 
-func TestOIDCProviderRefreshSessionIfNeededWithoutIdToken(t *testing.T) {
+func TestOIDCProviderRefreshSessionWithoutIdToken(t *testing.T) {
 
 	idToken, _ := newSignedTestIDToken(defaultIDToken)
 	body, _ := json.Marshal(redeemTokenResponse{
@@ -497,7 +497,7 @@ func TestOIDCProviderRefreshSessionIfNeededWithoutIdToken(t *testing.T) {
 	assert.Equal(t, "11223344", existingSession.User)
 }
 
-func TestOIDCProviderRefreshSessionIfNeededWithIdToken(t *testing.T) {
+func TestOIDCProviderRefreshSessionWithIdToken(t *testing.T) {
 
 	idToken, _ := newSignedTestIDToken(defaultIDToken)
 	body, _ := json.Marshal(redeemTokenResponse{