From 4d0303eef8ff34a8110a50ccb05978fa56c14251 Mon Sep 17 00:00:00 2001
From: Leandro Lafin <leandro.lafin@philips.com>
Date: Thu, 11 Jul 2024 11:11:50 -0300
Subject: [PATCH] Fill empty UserIDClaim before assigning it to other values

---
 CHANGELOG.md                |  1 +
 providers/providers.go      |  7 +++---
 providers/providers_test.go | 48 +++++++++++++++++++++++++++++++++++++
 3 files changed, 53 insertions(+), 3 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 4ff5afe8..d8fbf64b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -14,6 +14,7 @@
 - [#1883](https://github.com/oauth2-proxy/oauth2-proxy/pull/1883) Ensure v8 manifest variant is set on docker images
 - [#1906](https://github.com/oauth2-proxy/oauth2-proxy/pull/1906) Fix PKCE code verifier generation to never use UTF-8 characters
 - [#1927](https://github.com/oauth2-proxy/oauth2-proxy/pull/1927) Fix default scope settings for none oidc providers
+- [#1920](https://github.com/oauth2-proxy/oauth2-proxy/pull/1920) Make sure emailClaim is not overriden if userIDClaim is not set
 
 # V7.4.0
 
diff --git a/providers/providers.go b/providers/providers.go
index 68755820..49db8fa6 100644
--- a/providers/providers.go
+++ b/providers/providers.go
@@ -147,6 +147,10 @@ func newProviderDataFromConfig(providerConfig options.Provider) (*ProviderData,
 		logger.Printf("Warning: Your provider supports PKCE methods %+q, but you have not enabled one with --code-challenge-method", p.SupportedCodeChallengeMethods)
 	}
 
+    if providerConfig.OIDCConfig.UserIDClaim == "" {
+		providerConfig.OIDCConfig.UserIDClaim = "email"
+	}
+
 	// TODO (@NickMeves) - Remove This
 	// Backwards Compatibility for Deprecated UserIDClaim option
 	if providerConfig.OIDCConfig.EmailClaim == options.OIDCEmailClaim &&
@@ -161,9 +165,6 @@ func newProviderDataFromConfig(providerConfig options.Provider) (*ProviderData,
 			p.Scope += " groups"
 		}
 	}
-	if providerConfig.OIDCConfig.UserIDClaim == "" {
-		providerConfig.OIDCConfig.UserIDClaim = "email"
-	}
 
 	p.setAllowedGroups(providerConfig.AllowedGroups)
 
diff --git a/providers/providers_test.go b/providers/providers_test.go
index d830a9ae..e445b0bd 100644
--- a/providers/providers_test.go
+++ b/providers/providers_test.go
@@ -221,3 +221,51 @@ func TestCanOverwriteS256(t *testing.T) {
 
 	g.Expect(method).To(Equal(CodeChallengeMethodPlain))
 }
+
+func TestEmailClaimCorrectlySet(t *testing.T) {
+	g := NewWithT(t)
+
+	testCases := []struct {
+		name               string
+		userIDClaim        string
+		emailClaim         string
+		expectedEmailClaim string
+	}{
+		{
+			name:               "do not override EmailClaim if UserIDClaim is empty",
+			userIDClaim:        "",
+			emailClaim:         "email",
+			expectedEmailClaim: "email",
+		},
+		{
+			name:               "set EmailClaim to UserIDClaim",
+			userIDClaim:        "user_id_claim",
+			emailClaim:         "email",
+			expectedEmailClaim: "user_id_claim",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			providerConfig := options.Provider{
+				ID:               providerID,
+				Type:             "oidc",
+				ClientID:         clientID,
+				ClientSecretFile: clientSecret,
+				LoginURL:         msAuthURL,
+				RedeemURL:        msTokenURL,
+				OIDCConfig: options.OIDCOptions{
+					IssuerURL:     msIssuerURL,
+					SkipDiscovery: true,
+					JwksURL:       msKeysURL,
+					UserIDClaim:   tc.userIDClaim,
+					EmailClaim:    tc.emailClaim,
+				},
+			}
+
+			pd, err := newProviderDataFromConfig(providerConfig)
+			g.Expect(err).ToNot(HaveOccurred())
+
+			g.Expect(pd.EmailClaim).To(Equal(tc.expectedEmailClaim))
+		})
+	}