From 41207ba2656ad336eb476a88bca315ca98d59576 Mon Sep 17 00:00:00 2001 From: Rui Yang Date: Tue, 11 Aug 2020 16:25:21 -0400 Subject: [PATCH] Combine #1691 and #1776 to unify OIDC provider claim mapping add tests for groups key mapping Signed-off-by: Rui Yang --- Documentation/connectors/oidc.md | 47 ++++++------ README.md | 2 +- connector/oidc/oidc.go | 118 +++++++++++++++++++------------ connector/oidc/oidc_test.go | 54 ++++++++++++-- 4 files changed, 145 insertions(+), 76 deletions(-) diff --git a/Documentation/connectors/oidc.md b/Documentation/connectors/oidc.md index c6fbf2a3..2ff2a0ba 100644 --- a/Documentation/connectors/oidc.md +++ b/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 diff --git a/README.md b/README.md index 412aa84e..432c2fa1 100644 --- a/README.md +++ b/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 | | diff --git a/connector/oidc/oidc.go b/connector/oidc/oidc.go index fd7396df..f26a390c 100644 --- a/connector/oidc/oidc.go +++ b/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 - if c.GroupsClaimMapping == "" { - c.GroupsClaimMapping = "groups" + // Backward compatibility + userIDKey := c.ClaimMapping.UserIDKey + 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 - promptType string + emailKey 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) - if !found { - preferredUsername, _ = claims[c.preferredUsernameKey].(string) + var groups []string + if c.insecureEnableGroups { + groupsKey := "groups" + vs, found := claims[groupsKey].([]interface{}) + if !found { + 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 } diff --git a/connector/oidc/oidc_test.go b/connector/oidc/oidc_test.go index de66db93..9d9bf751 100644 --- a/connector/oidc/oidc_test.go +++ b/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) }) } }