Merge pull request #1920 from mdreem/do-not-remove-emails-claim

Fill empty UserIDClaim before assigning it to other values

CHANGELOG.md

@@ -14,6 +14,7 @@
 - [#1906](https://github.com/oauth2-proxy/oauth2-proxy/pull/1906) Fix PKCE code verifier generation to never use UTF-8 characters
 - [#1839](https://github.com/oauth2-proxy/oauth2-proxy/pull/1839) Add readiness checks for deeper health checks (@kobim)
 - [#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

providers/providers.go

@@ -145,6 +145,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 &&
@@ -159,9 +163,6 @@ func newProviderDataFromConfig(providerConfig options.Provider) (*ProviderData,
 			p.Scope += " groups"
 		}
 	}
-	if providerConfig.OIDCConfig.UserIDClaim == "" {
-		providerConfig.OIDCConfig.UserIDClaim = "email"
-	}
 
 	p.setAllowedGroups(providerConfig.AllowedGroups)
 

providers/providers_test.go

@@ -221,3 +221,52 @@ 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))
+		})
+	}
+}