From 9a4e0fcd00dcfbac480bbdefcf244f0b336a5b32 Mon Sep 17 00:00:00 2001 From: Josh Winters Date: Wed, 27 Feb 2019 15:12:11 -0500 Subject: [PATCH 1/9] Make OIDC username key configurable Signed-off-by: Josh Winters Co-authored-by: Mark Huang Signed-off-by: Rui Yang --- connector/oidc/oidc.go | 21 ++++++++++++++++----- connector/oidc/oidc_test.go | 19 +++++++++++++++++++ 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/connector/oidc/oidc.go b/connector/oidc/oidc.go index 675e4b95..4c3dc6ce 100644 --- a/connector/oidc/oidc.go +++ b/connector/oidc/oidc.go @@ -55,6 +55,9 @@ type Config struct { // Configurable key which contains the user name claim UserNameKey string `json:"userNameKey"` + // Configurable key which contains the username claims + PreferredUsernameKey string `json:"preferredUsernameKey"` // defaults to "username" + // PromptType will be used fot the prompt parameter (when offline_access, by default prompt=consent) PromptType string `json:"promptType"` } @@ -143,6 +146,7 @@ func (c *Config) Open(id string, logger log.Logger) (conn connector.Connector, e getUserInfo: c.GetUserInfo, userIDKey: c.UserIDKey, userNameKey: c.UserNameKey, + preferredUsernameKey: c.PreferredUsernameKey, promptType: c.PromptType, }, nil } @@ -165,6 +169,7 @@ type oidcConnector struct { getUserInfo bool userIDKey string userNameKey string + preferredUsernameKey string promptType string } @@ -296,6 +301,11 @@ func (c *oidcConnector) createIdentity(ctx context.Context, identity connector.I } hostedDomain, _ := claims["hd"].(string) + if c.preferredUsernameKey == "" { + c.preferredUsernameKey = "username" + } + username, _ := claims[c.preferredUsernameKey].(string) + if len(c.hostedDomains) > 0 { found := false for _, domain := range c.hostedDomains { @@ -320,11 +330,12 @@ func (c *oidcConnector) createIdentity(ctx context.Context, identity connector.I } identity = connector.Identity{ - UserID: idToken.Subject, - Username: name, - Email: email, - EmailVerified: emailVerified, - ConnectorData: connData, + UserID: idToken.Subject, + Username: name, + PreferredUsername: username, + Email: email, + EmailVerified: emailVerified, + ConnectorData: connData, } if c.userIDKey != "" { diff --git a/connector/oidc/oidc_test.go b/connector/oidc/oidc_test.go index 52afa158..b4d939e1 100644 --- a/connector/oidc/oidc_test.go +++ b/connector/oidc/oidc_test.go @@ -49,10 +49,12 @@ func TestHandleCallback(t *testing.T) { name string userIDKey string userNameKey string + preferredUsernameKey string insecureSkipEmailVerified bool scopes []string expectUserID string expectUserName string + expectPreferredUsername string expectedEmailField string token map[string]interface{} }{ @@ -108,6 +110,21 @@ func TestHandleCallback(t *testing.T) { "email_verified": true, }, }, + { + name: "withPreferredUsernameKey", + preferredUsernameKey: "preferred_username", + expectUserID: "subvalue", + expectUserName: "namevalue", + expectPreferredUsername: "usernamevalue", + expectedEmailField: "emailvalue", + token: map[string]interface{}{ + "sub": "subvalue", + "name": "namevalue", + "preferred_username": "usernamevalue", + "email": "emailvalue", + "email_verified": true, + }, + }, { name: "emptyEmailScope", expectUserID: "subvalue", @@ -161,6 +178,7 @@ func TestHandleCallback(t *testing.T) { RedirectURI: fmt.Sprintf("%s/callback", serverURL), UserIDKey: tc.userIDKey, UserNameKey: tc.userNameKey, + PreferredUsernameKey: tc.preferredUsernameKey, InsecureSkipEmailVerified: tc.insecureSkipEmailVerified, BasicAuthUnsupported: &basicAuth, } @@ -182,6 +200,7 @@ func TestHandleCallback(t *testing.T) { expectEquals(t, identity.UserID, tc.expectUserID) expectEquals(t, identity.Username, tc.expectUserName) + expectEquals(t, identity.PreferredUsername, tc.expectPreferredUsername) expectEquals(t, identity.Email, tc.expectedEmailField) expectEquals(t, identity.EmailVerified, true) }) From d9afb7e59ce5efb62b7f537c30ce8236b5382edc Mon Sep 17 00:00:00 2001 From: Rui Yang Date: Wed, 22 Jan 2020 00:12:35 +0800 Subject: [PATCH 2/9] default to preferred_username claim Signed-off-by: Rui Yang --- connector/oidc/oidc.go | 10 +++++----- connector/oidc/oidc_test.go | 34 ++++++++++++++++++---------------- 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/connector/oidc/oidc.go b/connector/oidc/oidc.go index 4c3dc6ce..4ec86a05 100644 --- a/connector/oidc/oidc.go +++ b/connector/oidc/oidc.go @@ -55,8 +55,8 @@ type Config struct { // Configurable key which contains the user name claim UserNameKey string `json:"userNameKey"` - // Configurable key which contains the username claims - PreferredUsernameKey string `json:"preferredUsernameKey"` // defaults to "username" + // Configurable key which contains the preferred username claims + PreferredUsernameKey string `json:"preferredUsernameKey"` // PromptType will be used fot the prompt parameter (when offline_access, by default prompt=consent) PromptType string `json:"promptType"` @@ -302,9 +302,9 @@ func (c *oidcConnector) createIdentity(ctx context.Context, identity connector.I hostedDomain, _ := claims["hd"].(string) if c.preferredUsernameKey == "" { - c.preferredUsernameKey = "username" + c.preferredUsernameKey = "preferred_username" } - username, _ := claims[c.preferredUsernameKey].(string) + preferredUsername, _ := claims[c.preferredUsernameKey].(string) if len(c.hostedDomains) > 0 { found := false @@ -332,7 +332,7 @@ func (c *oidcConnector) createIdentity(ctx context.Context, identity connector.I identity = connector.Identity{ UserID: idToken.Subject, Username: name, - PreferredUsername: username, + PreferredUsername: preferredUsername, Email: email, EmailVerified: emailVerified, ConnectorData: connData, diff --git a/connector/oidc/oidc_test.go b/connector/oidc/oidc_test.go index b4d939e1..1515e0ad 100644 --- a/connector/oidc/oidc_test.go +++ b/connector/oidc/oidc_test.go @@ -85,16 +85,18 @@ func TestHandleCallback(t *testing.T) { }, }, { - name: "withUserIDKey", - userIDKey: "name", - expectUserID: "namevalue", - expectUserName: "namevalue", - expectedEmailField: "emailvalue", + name: "withUserIDKey", + userIDKey: "name", + expectUserID: "namevalue", + expectUserName: "namevalue", + expectPreferredUsername: "usernamevalue", + expectedEmailField: "emailvalue", token: map[string]interface{}{ - "sub": "subvalue", - "name": "namevalue", - "email": "emailvalue", - "email_verified": true, + "sub": "subvalue", + "name": "namevalue", + "preferred_username": "usernamevalue", + "email": "emailvalue", + "email_verified": true, }, }, { @@ -112,17 +114,17 @@ func TestHandleCallback(t *testing.T) { }, { name: "withPreferredUsernameKey", - preferredUsernameKey: "preferred_username", + preferredUsernameKey: "username_key", expectUserID: "subvalue", expectUserName: "namevalue", - expectPreferredUsername: "usernamevalue", + expectPreferredUsername: "username_value", expectedEmailField: "emailvalue", token: map[string]interface{}{ - "sub": "subvalue", - "name": "namevalue", - "preferred_username": "usernamevalue", - "email": "emailvalue", - "email_verified": true, + "sub": "subvalue", + "name": "namevalue", + "username_key": "username_value", + "email": "emailvalue", + "email_verified": true, }, }, { From 4812079647c358a18696d96220183a62605cd0c9 Mon Sep 17 00:00:00 2001 From: Rui Yang Date: Tue, 4 Aug 2020 15:39:03 -0400 Subject: [PATCH 3/9] add tests when preferred username key is not set Signed-off-by: Rui Yang --- connector/oidc/oidc_test.go | 47 ++++++++++++++++++++++++++++--------- 1 file changed, 36 insertions(+), 11 deletions(-) diff --git a/connector/oidc/oidc_test.go b/connector/oidc/oidc_test.go index 1515e0ad..b5c717b9 100644 --- a/connector/oidc/oidc_test.go +++ b/connector/oidc/oidc_test.go @@ -85,18 +85,16 @@ func TestHandleCallback(t *testing.T) { }, }, { - name: "withUserIDKey", - userIDKey: "name", - expectUserID: "namevalue", - expectUserName: "namevalue", - expectPreferredUsername: "usernamevalue", - expectedEmailField: "emailvalue", + name: "withUserIDKey", + userIDKey: "name", + expectUserID: "namevalue", + expectUserName: "namevalue", + expectedEmailField: "emailvalue", token: map[string]interface{}{ - "sub": "subvalue", - "name": "namevalue", - "preferred_username": "usernamevalue", - "email": "emailvalue", - "email_verified": true, + "sub": "subvalue", + "name": "namevalue", + "email": "emailvalue", + "email_verified": true, }, }, { @@ -127,6 +125,33 @@ func TestHandleCallback(t *testing.T) { "email_verified": true, }, }, + { + name: "withoutPreferredUsernameKeyAndBackendReturns", + expectUserID: "subvalue", + expectUserName: "namevalue", + expectPreferredUsername: "preferredusernamevalue", + expectedEmailField: "emailvalue", + token: map[string]interface{}{ + "sub": "subvalue", + "name": "namevalue", + "preferred_username": "preferredusernamevalue", + "email": "emailvalue", + "email_verified": true, + }, + }, + { + name: "withoutPreferredUsernameKeyAndBackendNotReturn", + expectUserID: "subvalue", + expectUserName: "namevalue", + expectPreferredUsername: "", + expectedEmailField: "emailvalue", + token: map[string]interface{}{ + "sub": "subvalue", + "name": "namevalue", + "email": "emailvalue", + "email_verified": true, + }, + }, { name: "emptyEmailScope", expectUserID: "subvalue", From 52c39fb130caa887bb658055c53e5d07d60e7c3b Mon Sep 17 00:00:00 2001 From: Rui Yang Date: Wed, 5 Aug 2020 21:50:33 -0400 Subject: [PATCH 4/9] check if upstream contains preferrend username claim first Signed-off-by: Rui Yang Signed-off-by: Rui Yang --- connector/oidc/oidc.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/connector/oidc/oidc.go b/connector/oidc/oidc.go index 4ec86a05..c8172811 100644 --- a/connector/oidc/oidc.go +++ b/connector/oidc/oidc.go @@ -301,10 +301,10 @@ func (c *oidcConnector) createIdentity(ctx context.Context, identity connector.I } hostedDomain, _ := claims["hd"].(string) - if c.preferredUsernameKey == "" { - c.preferredUsernameKey = "preferred_username" + preferredUsername, found := claims["preferred_username"].(string) + if !found { + preferredUsername, _ = claims[c.preferredUsernameKey].(string) } - preferredUsername, _ := claims[c.preferredUsernameKey].(string) if len(c.hostedDomains) > 0 { found := false From 61312e726e777b355379ca743f9965f88e0e80ef Mon Sep 17 00:00:00 2001 From: Cyrille Nofficial Date: Fri, 17 Apr 2020 10:01:52 +0200 Subject: [PATCH 5/9] Add parameter configuration to override email claim key Signed-off-by: Rui Yang --- Documentation/connectors/oidc.md | 5 +++++ connector/oidc/oidc.go | 14 ++++++++++++-- connector/oidc/oidc_test.go | 17 +++++++++++++++++ 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/Documentation/connectors/oidc.md b/Documentation/connectors/oidc.md index c472e303..4df28915 100644 --- a/Documentation/connectors/oidc.md +++ b/Documentation/connectors/oidc.md @@ -56,6 +56,11 @@ 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 diff --git a/connector/oidc/oidc.go b/connector/oidc/oidc.go index c8172811..d1faba7b 100644 --- a/connector/oidc/oidc.go +++ b/connector/oidc/oidc.go @@ -58,6 +58,9 @@ type Config struct { // 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"` } @@ -112,6 +115,11 @@ 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...) @@ -147,6 +155,7 @@ func (c *Config) Open(id string, logger log.Logger) (conn connector.Connector, e userIDKey: c.UserIDKey, userNameKey: c.UserNameKey, preferredUsernameKey: c.PreferredUsernameKey, + emailClaim: emailClaim, promptType: c.PromptType, }, nil } @@ -170,6 +179,7 @@ type oidcConnector struct { userIDKey string userNameKey string preferredUsernameKey string + emailClaim string promptType string } @@ -286,9 +296,9 @@ func (c *oidcConnector) createIdentity(ctx context.Context, identity connector.I } } - email, found := claims["email"].(string) + email, found := claims[c.emailClaim].(string) if !found && hasEmailScope { - return identity, errors.New("missing \"email\" claim") + return identity, fmt.Errorf("missing \"%s\" claim", c.emailClaim) } emailVerified, found := claims["email_verified"].(bool) diff --git a/connector/oidc/oidc_test.go b/connector/oidc/oidc_test.go index b5c717b9..de66db93 100644 --- a/connector/oidc/oidc_test.go +++ b/connector/oidc/oidc_test.go @@ -52,6 +52,7 @@ func TestHandleCallback(t *testing.T) { preferredUsernameKey string insecureSkipEmailVerified bool scopes []string + emailClaim string expectUserID string expectUserName string expectPreferredUsername string @@ -72,6 +73,21 @@ func TestHandleCallback(t *testing.T) { "email_verified": true, }, }, + { + name: "customEmailClaim", + userIDKey: "", // not configured + userNameKey: "", // not configured + emailClaim: "mail", + expectUserID: "subvalue", + expectUserName: "namevalue", + expectedEmailField: "emailvalue", + token: map[string]interface{}{ + "sub": "subvalue", + "name": "namevalue", + "mail": "emailvalue", + "email_verified": true, + }, + }, { name: "email_verified not in claims, configured to be skipped", insecureSkipEmailVerified: true, @@ -206,6 +222,7 @@ func TestHandleCallback(t *testing.T) { UserIDKey: tc.userIDKey, UserNameKey: tc.userNameKey, PreferredUsernameKey: tc.preferredUsernameKey, + EmailClaim: tc.emailClaim, InsecureSkipEmailVerified: tc.insecureSkipEmailVerified, BasicAuthUnsupported: &basicAuth, } From a783667c57cd6f6988e8fb94f3e9d9c2bdc3be61 Mon Sep 17 00:00:00 2001 From: Scott Lemmon Date: Fri, 7 Aug 2020 15:06:14 -0700 Subject: [PATCH 6/9] Add groupsClaimMapping to the OIDC connector The groupsClaimMapping setting allows one to specify which claim to pull group information from the OIDC provider. Previously it assumed group information was always in the "groups" claim, but that isn't the case for many OIDC providers (such as AWS Cognito using the "cognito:groups" claim instead) Signed-off-by: Scott Lemmon Signed-off-by: Rui Yang --- Documentation/connectors/oidc.md | 4 ++++ connector/oidc/oidc.go | 15 +++++++++++++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/Documentation/connectors/oidc.md b/Documentation/connectors/oidc.md index 4df28915..c6fbf2a3 100644 --- a/Documentation/connectors/oidc.md +++ b/Documentation/connectors/oidc.md @@ -73,6 +73,10 @@ 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. diff --git a/connector/oidc/oidc.go b/connector/oidc/oidc.go index d1faba7b..fd7396df 100644 --- a/connector/oidc/oidc.go +++ b/connector/oidc/oidc.go @@ -44,6 +44,9 @@ 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 @@ -132,6 +135,11 @@ 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" + } + clientID := c.ClientID return &oidcConnector{ provider: provider, @@ -151,6 +159,7 @@ 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, @@ -175,6 +184,7 @@ type oidcConnector struct { hostedDomains []string insecureSkipEmailVerified bool insecureEnableGroups bool + groupsClaimMapping string getUserInfo bool userIDKey string userNameKey string @@ -357,13 +367,14 @@ func (c *oidcConnector) createIdentity(ctx context.Context, identity connector.I } if c.insecureEnableGroups { - vs, ok := claims["groups"].([]interface{}) + + 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, errors.New("malformed \"groups\" claim") + return identity, fmt.Errorf("malformed \"%v\" claim", c.groupsClaimMapping) } } } From 41207ba2656ad336eb476a88bca315ca98d59576 Mon Sep 17 00:00:00 2001 From: Rui Yang Date: Tue, 11 Aug 2020 16:25:21 -0400 Subject: [PATCH 7/9] 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) }) } } From 0494993326ac4df41f2695a1bbcce7acb763370b Mon Sep 17 00:00:00 2001 From: Rui Yang Date: Tue, 8 Sep 2020 10:03:52 -0400 Subject: [PATCH 8/9] update oidc documentation and email claim err msg Signed-off-by: Rui Yang --- Documentation/connectors/oidc.md | 19 ++++++------------- connector/oidc/oidc.go | 2 +- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/Documentation/connectors/oidc.md b/Documentation/connectors/oidc.md index 2ff2a0ba..6fd19184 100644 --- a/Documentation/connectors/oidc.md +++ b/Documentation/connectors/oidc.md @@ -78,28 +78,21 @@ connectors: # 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 + # Some providers return non-standard claims (eg. mail). + # Use claimMapping to map those claims to standard claims: + # https://openid.net/specs/openid-connect-core-1_0.html#Claims + # claimMapping can only map a non-standard claim to a standard one if it's not returned in the id_token. 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" + # Default: email # email: mail # The set claim is used as groups. - # Default: "groups" + # Default: groups # groups: "cognito:groups" ``` diff --git a/connector/oidc/oidc.go b/connector/oidc/oidc.go index f26a390c..4cc44ddb 100644 --- a/connector/oidc/oidc.go +++ b/connector/oidc/oidc.go @@ -330,7 +330,7 @@ func (c *oidcConnector) createIdentity(ctx context.Context, identity connector.I } if !found && hasEmailScope { - return identity, fmt.Errorf("missing \"%s\" claim", emailKey) + return identity, fmt.Errorf("missing email claim, not found \"%s\" key", emailKey) } emailVerified, found := claims["email_verified"].(bool) From 058202d007331d947f3f9934ce5cc9937bbee532 Mon Sep 17 00:00:00 2001 From: Rui Yang Date: Tue, 8 Sep 2020 13:12:53 -0400 Subject: [PATCH 9/9] revert changes for user id and user name Signed-off-by: Rui Yang --- Documentation/connectors/oidc.md | 10 +++++++++- connector/oidc/oidc.go | 24 ++---------------------- connector/oidc/oidc_test.go | 4 ++-- 3 files changed, 13 insertions(+), 25 deletions(-) diff --git a/Documentation/connectors/oidc.md b/Documentation/connectors/oidc.md index 6fd19184..e64cd2de 100644 --- a/Documentation/connectors/oidc.md +++ b/Documentation/connectors/oidc.md @@ -72,12 +72,20 @@ connectors: # https://openid.net/specs/openid-connect-core-1_0.html#UserInfo # getUserInfo: true + # The set claim is used as user id. + # Claims list at https://openid.net/specs/openid-connect-core-1_0.html#Claims + # Default: sub + # 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 non-standard claims (eg. mail). # Use claimMapping to map those claims to standard claims: # https://openid.net/specs/openid-connect-core-1_0.html#Claims diff --git a/connector/oidc/oidc.go b/connector/oidc/oidc.go index 4cc44ddb..b8e543d4 100644 --- a/connector/oidc/oidc.go +++ b/connector/oidc/oidc.go @@ -49,22 +49,14 @@ type Config struct { // id tokens GetUserInfo bool `json:"getUserInfo"` - // Deprecated: use UserIDKey in claimMapping instead UserIDKey string `json:"userIDKey"` - // Deprecated: use UserNameKey in claimMapping instead UserNameKey string `json:"userNameKey"` // 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" @@ -138,18 +130,6 @@ func (c *Config) Open(id string, logger log.Logger) (conn connector.Connector, e c.PromptType = "consent" } - // 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 return &oidcConnector{ provider: provider, @@ -171,8 +151,8 @@ func (c *Config) Open(id string, logger log.Logger) (conn connector.Connector, e insecureEnableGroups: c.InsecureEnableGroups, getUserInfo: c.GetUserInfo, promptType: c.PromptType, - userIDKey: userIDKey, - userNameKey: userNameKey, + userIDKey: c.UserIDKey, + userNameKey: c.UserNameKey, preferredUsernameKey: c.ClaimMapping.PreferredUsernameKey, emailKey: c.ClaimMapping.EmailKey, groupsKey: c.ClaimMapping.GroupsKey, diff --git a/connector/oidc/oidc_test.go b/connector/oidc/oidc_test.go index 9d9bf751..ae92f70c 100644 --- a/connector/oidc/oidc_test.go +++ b/connector/oidc/oidc_test.go @@ -258,12 +258,12 @@ func TestHandleCallback(t *testing.T) { ClientSecret: "clientSecret", Scopes: scopes, RedirectURI: fmt.Sprintf("%s/callback", serverURL), + UserIDKey: tc.userIDKey, + UserNameKey: tc.userNameKey, 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