Combine #1691 and #1776 to unify OIDC provider claim mapping

add tests for groups key mapping

Signed-off-by: Rui Yang <ruiya@vmware.com>

Documentation/connectors/oidc.md

@@ -8,8 +8,6 @@ Prominent examples of OpenID Connect providers include Google Accounts, Salesfor
 
 ## Caveats
 
-This connector does not support the "groups" claim. Progress for this is tracked in [issue #1065][issue-1065].
-
 When using refresh tokens, changes to the upstream claims aren't propagated to the id_token returned by dex. If a user's email changes, the "email" claim returned by dex won't change unless the user logs in again. Progress for this is tracked in [issue #863][issue-863].
 
 ## Configuration
@@ -56,11 +54,6 @@ connectors:
     #  - email
     #  - groups
 
-    # Some providers return no standard email claim key (ex: 'mail')
-    # Override email claim key
-    # Default is "email"
-    # emailClaim: email
-
     # Some providers return claims without "email_verified", when they had no usage of emails verification in enrollment process
     # or if they are acting as a proxy for another IDP etc AWS Cognito with an upstream SAML IDP
     # This can be overridden with the below option
@@ -73,33 +66,43 @@ connectors:
     # This can be overridden with the below option
     # insecureEnableGroups: true
 
-    # If an OIDC provider uses a different claim name than the standard "groups" claim to provide group information
-    # the claim to use can be specified
-    # groupsClaimMapping: "cognito:groups"
-
     # When enabled, the OpenID Connector will query the UserInfo endpoint for additional claims. UserInfo claims
     # take priority over claims returned by the IDToken. This option should be used when the IDToken doesn't contain
     # all the claims requested.
     # https://openid.net/specs/openid-connect-core-1_0.html#UserInfo
     # getUserInfo: true
 
-    # The set claim is used as user id.
-    # Default: sub
-    # Claims list at https://openid.net/specs/openid-connect-core-1_0.html#Claims
-    #
-    # userIDKey: nickname
-    
-    # The set claim is used as user name.
-    # Default: name
-    # userNameKey: nickname
-
     # For offline_access, the prompt parameter is set by default to "prompt=consent". 
     # However this is not supported by all OIDC providers, some of them support different
     # value for prompt, like "prompt=login" or "prompt=none"
     # promptType: consent
+
+
+    # Some providers return no standard claim that is different to 
+    # claims list at https://openid.net/specs/openid-connect-core-1_0.html#Claims
+    # Use claimMapping to specify custom claim names
+    claimMapping:
+      # The set claim is used as user id.
+      # Default: sub
+      # user_id: nickname
+
+      # The set claim is used as user name.
+      # Default: name
+      # user_name: nickname
+
+      # The set claim is used as preferred username.
+      # Default: preferred_username
+      # preferred_username: other_user_name
+
+      # The set claim is used as email.
+      # Default: "email"
+      # email: mail
+
+      # The set claim is used as groups.
+      # Default: "groups"
+      # groups: "cognito:groups"
 ```
 
 [oidc-doc]: openid-connect.md
 [issue-863]: https://github.com/dexidp/dex/issues/863
-[issue-1065]: https://github.com/dexidp/dex/issues/1065
 [azure-ad-v1]: https://github.com/coreos/go-oidc/issues/133

README.md

@@ -69,7 +69,7 @@ Dex implements the following connectors:
 | [GitHub](Documentation/connectors/github.md) | yes | yes | yes | stable | |
 | [SAML 2.0](Documentation/connectors/saml.md) | no | yes | no | stable |
 | [GitLab](Documentation/connectors/gitlab.md) | yes | yes | yes | beta | |
-| [OpenID Connect](Documentation/connectors/oidc.md) | yes | no ([#1065][issue-1065]) | no | beta | Includes Salesforce, Azure, etc. |
+| [OpenID Connect](Documentation/connectors/oidc.md) | yes | yes | yes | beta | Includes Salesforce, Azure, etc. |
 | [Google](Documentation/connectors/google.md) | yes | yes | yes | alpha | |
 | [LinkedIn](Documentation/connectors/linkedin.md) | yes | no | no | beta | |
 | [Microsoft](Documentation/connectors/microsoft.md) | yes | yes | no | beta | |

connector/oidc/oidc.go

@@ -44,28 +44,36 @@ type Config struct {
 	// InsecureEnableGroups enables groups claims. This is disabled by default until https://github.com/dexidp/dex/issues/1065 is resolved
 	InsecureEnableGroups bool `json:"insecureEnableGroups"`
 
-	// GroupsClaimMapping sets the name of the claim which contains the users groups. InsecureEnableGroups must be enabled to use this setting
-	GroupsClaimMapping string `json:"groupsClaimMapping"` // defaults to "groups"
-
 	// GetUserInfo uses the userinfo endpoint to get additional claims for
 	// the token. This is especially useful where upstreams return "thin"
 	// id tokens
 	GetUserInfo bool `json:"getUserInfo"`
 
-	// Configurable key which contains the user id claim
+	// Deprecated: use UserIDKey in claimMapping instead
 	UserIDKey string `json:"userIDKey"`
 
-	// Configurable key which contains the user name claim
+	// Deprecated: use UserNameKey in claimMapping instead
 	UserNameKey string `json:"userNameKey"`
 
-	// Configurable key which contains the preferred username claims
-	PreferredUsernameKey string `json:"preferredUsernameKey"`
-
-	// EmailClaim override email claim key. Defaults to "email"
-	EmailClaim string `json:"emailClaim"`
-
 	// PromptType will be used fot the prompt parameter (when offline_access, by default prompt=consent)
 	PromptType string `json:"promptType"`
+
+	ClaimMapping struct {
+		// Configurable key which contains the user id claim
+		UserIDKey string `json:"user_id"` // defaults to "sub"
+
+		// Configurable key which contains the username claim
+		UserNameKey string `json:"user_name"` // defaults to "name"
+
+		// Configurable key which contains the preferred username claims
+		PreferredUsernameKey string `json:"preferred_username"` // defaults to "preferred_username"
+
+		// Configurable key which contains the email claims
+		EmailKey string `json:"email"` // defaults to "email"
+
+		// Configurable key which contains the groups claims
+		GroupsKey string `json:"groups"` // defaults to "groups"
+	} `json:"claimMapping"`
 }
 
 // Domains that don't support basic auth. golang.org/x/oauth2 has an internal
@@ -118,11 +126,6 @@ func (c *Config) Open(id string, logger log.Logger) (conn connector.Connector, e
 		endpoint.AuthStyle = oauth2.AuthStyleInParams
 	}
 
-	emailClaim := "email"
-	if len(c.EmailClaim) > 0 {
-		emailClaim = c.EmailClaim
-	}
-
 	scopes := []string{oidc.ScopeOpenID}
 	if len(c.Scopes) > 0 {
 		scopes = append(scopes, c.Scopes...)
@@ -135,9 +138,16 @@ func (c *Config) Open(id string, logger log.Logger) (conn connector.Connector, e
 		c.PromptType = "consent"
 	}
 
-	// GroupsClaimMapping should be "groups" by default, if not set
+	// Backward compatibility
-	if c.GroupsClaimMapping == "" {
+	userIDKey := c.ClaimMapping.UserIDKey
-		c.GroupsClaimMapping = "groups"
+	if userIDKey == "" {
+		userIDKey = c.UserIDKey
+	}
+
+	// Backward compatibility
+	userNameKey := c.ClaimMapping.UserNameKey
+	if userNameKey == "" {
+		userNameKey = c.UserNameKey
 	}
 
 	clientID := c.ClientID
@@ -159,13 +169,13 @@ func (c *Config) Open(id string, logger log.Logger) (conn connector.Connector, e
 		hostedDomains:             c.HostedDomains,
 		insecureSkipEmailVerified: c.InsecureSkipEmailVerified,
 		insecureEnableGroups:      c.InsecureEnableGroups,
-		groupsClaimMapping:        c.GroupsClaimMapping,
 		getUserInfo:               c.GetUserInfo,
-		userIDKey:                 c.UserIDKey,
-		userNameKey:               c.UserNameKey,
-		preferredUsernameKey:      c.PreferredUsernameKey,
-		emailClaim:                emailClaim,
 		promptType:                c.PromptType,
+		userIDKey:                 userIDKey,
+		userNameKey:               userNameKey,
+		preferredUsernameKey:      c.ClaimMapping.PreferredUsernameKey,
+		emailKey:                  c.ClaimMapping.EmailKey,
+		groupsKey:                 c.ClaimMapping.GroupsKey,
 	}, nil
 }
 
@@ -184,13 +194,13 @@ type oidcConnector struct {
 	hostedDomains             []string
 	insecureSkipEmailVerified bool
 	insecureEnableGroups      bool
-	groupsClaimMapping        string
 	getUserInfo               bool
+	promptType                string
 	userIDKey                 string
 	userNameKey               string
 	preferredUsernameKey      string
-	emailClaim                string
+	emailKey                  string
-	promptType                string
+	groupsKey                 string
 }
 
 func (c *oidcConnector) Close() error {
@@ -298,6 +308,11 @@ func (c *oidcConnector) createIdentity(ctx context.Context, identity connector.I
 		return identity, fmt.Errorf("missing \"%s\" claim", userNameKey)
 	}
 
+	preferredUsername, found := claims["preferred_username"].(string)
+	if !found {
+		preferredUsername, _ = claims[c.preferredUsernameKey].(string)
+	}
+
 	hasEmailScope := false
 	for _, s := range c.oauth2Config.Scopes {
 		if s == "email" {
@@ -306,9 +321,16 @@ func (c *oidcConnector) createIdentity(ctx context.Context, identity connector.I
 		}
 	}
 
-	email, found := claims[c.emailClaim].(string)
+	var email string
+	emailKey := "email"
+	email, found = claims[emailKey].(string)
+	if !found && c.emailKey != "" {
+		emailKey = c.emailKey
+		email, found = claims[emailKey].(string)
+	}
+
 	if !found && hasEmailScope {
-		return identity, fmt.Errorf("missing \"%s\" claim", c.emailClaim)
+		return identity, fmt.Errorf("missing \"%s\" claim", emailKey)
 	}
 
 	emailVerified, found := claims["email_verified"].(bool)
@@ -319,13 +341,28 @@ func (c *oidcConnector) createIdentity(ctx context.Context, identity connector.I
 			return identity, errors.New("missing \"email_verified\" claim")
 		}
 	}
-	hostedDomain, _ := claims["hd"].(string)
 
-	preferredUsername, found := claims["preferred_username"].(string)
+	var groups []string
+	if c.insecureEnableGroups {
+		groupsKey := "groups"
+		vs, found := claims[groupsKey].([]interface{})
 		if !found {
-		preferredUsername, _ = claims[c.preferredUsernameKey].(string)
+			groupsKey = c.groupsKey
+			vs, found = claims[groupsKey].([]interface{})
 		}
 
+		if found {
+			for _, v := range vs {
+				if s, ok := v.(string); ok {
+					groups = append(groups, s)
+				} else {
+					return identity, fmt.Errorf("malformed \"%v\" claim", groupsKey)
+				}
+			}
+		}
+	}
+
+	hostedDomain, _ := claims["hd"].(string)
 	if len(c.hostedDomains) > 0 {
 		found := false
 		for _, domain := range c.hostedDomains {
@@ -355,6 +392,7 @@ func (c *oidcConnector) createIdentity(ctx context.Context, identity connector.I
 		PreferredUsername: preferredUsername,
 		Email:             email,
 		EmailVerified:     emailVerified,
+		Groups:            groups,
 		ConnectorData:     connData,
 	}
 
@@ -366,19 +404,5 @@ func (c *oidcConnector) createIdentity(ctx context.Context, identity connector.I
 		identity.UserID = userID
 	}
 
-	if c.insecureEnableGroups {
-
-		vs, ok := claims[c.groupsClaimMapping].([]interface{})
-		if ok {
-			for _, v := range vs {
-				if s, ok := v.(string); ok {
-					identity.Groups = append(identity.Groups, s)
-				} else {
-					return identity, fmt.Errorf("malformed \"%v\" claim", c.groupsClaimMapping)
-				}
-			}
-		}
-	}
-
 	return identity, nil
 }

connector/oidc/oidc_test.go

@@ -50,11 +50,13 @@ func TestHandleCallback(t *testing.T) {
 		userIDKey                 string
 		userNameKey               string
 		preferredUsernameKey      string
+		emailKey                  string
+		groupsKey                 string
 		insecureSkipEmailVerified bool
 		scopes                    []string
-		emailClaim                string
 		expectUserID              string
 		expectUserName            string
+		expectGroups              []string
 		expectPreferredUsername   string
 		expectedEmailField        string
 		token                     map[string]interface{}
@@ -65,10 +67,12 @@ func TestHandleCallback(t *testing.T) {
 			userNameKey:        "", // not configured
 			expectUserID:       "subvalue",
 			expectUserName:     "namevalue",
+			expectGroups:       []string{"group1", "group2"},
 			expectedEmailField: "emailvalue",
 			token: map[string]interface{}{
 				"sub":            "subvalue",
 				"name":           "namevalue",
+				"groups":         []string{"group1", "group2"},
 				"email":          "emailvalue",
 				"email_verified": true,
 			},
@@ -77,7 +81,7 @@ func TestHandleCallback(t *testing.T) {
 			name:               "customEmailClaim",
 			userIDKey:          "", // not configured
 			userNameKey:        "", // not configured
-			emailClaim:         "mail",
+			emailKey:           "mail",
 			expectUserID:       "subvalue",
 			expectUserName:     "namevalue",
 			expectedEmailField: "emailvalue",
@@ -195,6 +199,41 @@ func TestHandleCallback(t *testing.T) {
 				"email":     "emailvalue",
 			},
 		},
+		{
+			name:                      "customGroupsKey",
+			groupsKey:                 "cognito:groups",
+			expectUserID:              "subvalue",
+			expectUserName:            "namevalue",
+			expectedEmailField:        "emailvalue",
+			expectGroups:              []string{"group3", "group4"},
+			scopes:                    []string{"groups"},
+			insecureSkipEmailVerified: true,
+			token: map[string]interface{}{
+				"sub":            "subvalue",
+				"name":           "namevalue",
+				"user_name":      "username",
+				"email":          "emailvalue",
+				"cognito:groups": []string{"group3", "group4"},
+			},
+		},
+		{
+			name:                      "customGroupsKeyButGroupsProvided",
+			groupsKey:                 "cognito:groups",
+			expectUserID:              "subvalue",
+			expectUserName:            "namevalue",
+			expectedEmailField:        "emailvalue",
+			expectGroups:              []string{"group1", "group2"},
+			scopes:                    []string{"groups"},
+			insecureSkipEmailVerified: true,
+			token: map[string]interface{}{
+				"sub":            "subvalue",
+				"name":           "namevalue",
+				"user_name":      "username",
+				"email":          "emailvalue",
+				"groups":         []string{"group1", "group2"},
+				"cognito:groups": []string{"group3", "group4"},
+			},
+		},
 	}
 
 	for _, tc := range tests {
@@ -219,13 +258,15 @@ func TestHandleCallback(t *testing.T) {
 				ClientSecret:              "clientSecret",
 				Scopes:                    scopes,
 				RedirectURI:               fmt.Sprintf("%s/callback", serverURL),
-				UserIDKey:                 tc.userIDKey,
-				UserNameKey:               tc.userNameKey,
-				PreferredUsernameKey:      tc.preferredUsernameKey,
-				EmailClaim:                tc.emailClaim,
 				InsecureSkipEmailVerified: tc.insecureSkipEmailVerified,
+				InsecureEnableGroups:      true,
 				BasicAuthUnsupported:      &basicAuth,
 			}
+			config.ClaimMapping.UserIDKey = tc.userIDKey
+			config.ClaimMapping.UserNameKey = tc.userNameKey
+			config.ClaimMapping.PreferredUsernameKey = tc.preferredUsernameKey
+			config.ClaimMapping.EmailKey = tc.emailKey
+			config.ClaimMapping.GroupsKey = tc.groupsKey
 
 			conn, err := newConnector(config)
 			if err != nil {
@@ -247,6 +288,7 @@ func TestHandleCallback(t *testing.T) {
 			expectEquals(t, identity.PreferredUsername, tc.expectPreferredUsername)
 			expectEquals(t, identity.Email, tc.expectedEmailField)
 			expectEquals(t, identity.EmailVerified, true)
+			expectEquals(t, identity.Groups, tc.expectGroups)
 		})
 	}
 }