From 45143c98b348749524c3a5092b95c738a97fd764 Mon Sep 17 00:00:00 2001 From: Happy2C0de <46957159+Happy2C0de@users.noreply.github.com> Date: Wed, 11 Aug 2021 12:20:46 +0200 Subject: [PATCH 1/7] Add claimMapping enforcement Signed-off-by: Happy2C0de <46957159+Happy2C0de@users.noreply.github.com> --- connector/oidc/oidc.go | 53 ++++++++++++++++---------- connector/oidc/oidc_test.go | 76 +++++++++++++++++++++++++++++-------- 2 files changed, 92 insertions(+), 37 deletions(-) diff --git a/connector/oidc/oidc.go b/connector/oidc/oidc.go index b752f9da..ca28a988 100644 --- a/connector/oidc/oidc.go +++ b/connector/oidc/oidc.go @@ -56,16 +56,23 @@ type Config struct { // 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 preferred username claims - PreferredUsernameKey string `json:"preferred_username"` // defaults to "preferred_username" + ClaimMapping ClaimMapping `json:"claimMapping"` +} - // Configurable key which contains the email claims - EmailKey string `json:"email"` // defaults to "email" +type ClaimMapping struct { + // Enforce the ClaimMapping. + // i.e. an 'email' claim will always be taken if available, + // irrelevant of the settings in EmailKey. This option will enforce the ClaimMapping options independent of the existing claims. + Enforce bool `json:"enforce"` // defaults to false - // Configurable key which contains the groups claims - GroupsKey string `json:"groups"` // defaults to "groups" - } `json:"claimMapping"` + // 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" } // Domains that don't support basic auth. golang.org/x/oauth2 has an internal @@ -153,9 +160,7 @@ func (c *Config) Open(id string, logger log.Logger) (conn connector.Connector, e promptType: c.PromptType, userIDKey: c.UserIDKey, userNameKey: c.UserNameKey, - preferredUsernameKey: c.ClaimMapping.PreferredUsernameKey, - emailKey: c.ClaimMapping.EmailKey, - groupsKey: c.ClaimMapping.GroupsKey, + claimMapping: c.ClaimMapping, }, nil } @@ -178,9 +183,7 @@ type oidcConnector struct { promptType string userIDKey string userNameKey string - preferredUsernameKey string - emailKey string - groupsKey string + claimMapping ClaimMapping } func (c *oidcConnector) Close() error { @@ -288,9 +291,14 @@ 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) + prefUsername := "preferred_username" + preferredUsername, found := claims[prefUsername].(string) + if (!found || c.claimMapping.Enforce) && c.claimMapping.PreferredUsernameKey != "" { + prefUsername = c.claimMapping.PreferredUsernameKey + preferredUsername, found = claims[prefUsername].(string) + if !found { + return identity, fmt.Errorf("missing \"%s\" claim", prefUsername) + } } hasEmailScope := false @@ -304,9 +312,12 @@ func (c *oidcConnector) createIdentity(ctx context.Context, identity connector.I var email string emailKey := "email" email, found = claims[emailKey].(string) - if !found && c.emailKey != "" { - emailKey = c.emailKey + if (!found || c.claimMapping.Enforce) && c.claimMapping.EmailKey != "" { + emailKey = c.claimMapping.EmailKey email, found = claims[emailKey].(string) + if !found { + return identity, fmt.Errorf("missing \"%s\" claim", emailKey) + } } if !found && hasEmailScope { @@ -326,8 +337,8 @@ func (c *oidcConnector) createIdentity(ctx context.Context, identity connector.I if c.insecureEnableGroups { groupsKey := "groups" vs, found := claims[groupsKey].([]interface{}) - if !found { - groupsKey = c.groupsKey + if (!found || c.claimMapping.Enforce) && c.claimMapping.GroupsKey != "" { + groupsKey = c.claimMapping.GroupsKey vs, found = claims[groupsKey].([]interface{}) } diff --git a/connector/oidc/oidc_test.go b/connector/oidc/oidc_test.go index ae92f70c..6bc89645 100644 --- a/connector/oidc/oidc_test.go +++ b/connector/oidc/oidc_test.go @@ -49,9 +49,7 @@ func TestHandleCallback(t *testing.T) { name string userIDKey string userNameKey string - preferredUsernameKey string - emailKey string - groupsKey string + claimMapping ClaimMapping insecureSkipEmailVerified bool scopes []string expectUserID string @@ -78,10 +76,12 @@ func TestHandleCallback(t *testing.T) { }, }, { - name: "customEmailClaim", - userIDKey: "", // not configured - userNameKey: "", // not configured - emailKey: "mail", + name: "customEmailClaim", + userIDKey: "", // not configured + userNameKey: "", // not configured + claimMapping: ClaimMapping{ + EmailKey: "mail", + }, expectUserID: "subvalue", expectUserName: "namevalue", expectedEmailField: "emailvalue", @@ -92,6 +92,25 @@ func TestHandleCallback(t *testing.T) { "email_verified": true, }, }, + { + name: "enforceCustomEmailClaim", + userIDKey: "", // not configured + userNameKey: "", // not configured + claimMapping: ClaimMapping{ + Enforce: true, + EmailKey: "custommail", + }, + expectUserID: "subvalue", + expectUserName: "namevalue", + expectedEmailField: "customemailvalue", + token: map[string]interface{}{ + "sub": "subvalue", + "name": "namevalue", + "mail": "emailvalue", + "custommail": "customemailvalue", + "email_verified": true, + }, + }, { name: "email_verified not in claims, configured to be skipped", insecureSkipEmailVerified: true, @@ -131,8 +150,10 @@ func TestHandleCallback(t *testing.T) { }, }, { - name: "withPreferredUsernameKey", - preferredUsernameKey: "username_key", + name: "withPreferredUsernameKey", + claimMapping: ClaimMapping{ + PreferredUsernameKey: "username_key", + }, expectUserID: "subvalue", expectUserName: "namevalue", expectPreferredUsername: "username_value", @@ -200,8 +221,10 @@ func TestHandleCallback(t *testing.T) { }, }, { - name: "customGroupsKey", - groupsKey: "cognito:groups", + name: "customGroupsKey", + claimMapping: ClaimMapping{ + GroupsKey: "cognito:groups", + }, expectUserID: "subvalue", expectUserName: "namevalue", expectedEmailField: "emailvalue", @@ -217,8 +240,10 @@ func TestHandleCallback(t *testing.T) { }, }, { - name: "customGroupsKeyButGroupsProvided", - groupsKey: "cognito:groups", + name: "customGroupsKeyButGroupsProvided", + claimMapping: ClaimMapping{ + GroupsKey: "cognito:groups", + }, expectUserID: "subvalue", expectUserName: "namevalue", expectedEmailField: "emailvalue", @@ -234,6 +259,27 @@ func TestHandleCallback(t *testing.T) { "cognito:groups": []string{"group3", "group4"}, }, }, + { + name: "customGroupsKeyButGroupsProvidedButEnforced", + claimMapping: ClaimMapping{ + Enforce: true, + 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", + "groups": []string{"group1", "group2"}, + "cognito:groups": []string{"group3", "group4"}, + }, + }, } for _, tc := range tests { @@ -264,9 +310,7 @@ func TestHandleCallback(t *testing.T) { InsecureEnableGroups: true, BasicAuthUnsupported: &basicAuth, } - config.ClaimMapping.PreferredUsernameKey = tc.preferredUsernameKey - config.ClaimMapping.EmailKey = tc.emailKey - config.ClaimMapping.GroupsKey = tc.groupsKey + config.ClaimMapping = tc.claimMapping conn, err := newConnector(config) if err != nil { From 14a0aecc8198523741ce580e4158760b8143546f Mon Sep 17 00:00:00 2001 From: Happy2C0de <46957159+Happy2C0de@users.noreply.github.com> Date: Fri, 13 Aug 2021 12:49:24 +0200 Subject: [PATCH 2/7] Move claimMapping.enforce to overrideClaimMapping Signed-off-by: Happy2C0de <46957159+Happy2C0de@users.noreply.github.com> --- connector/oidc/oidc.go | 17 ++++++++++------- connector/oidc/oidc_test.go | 14 ++++++++------ 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/connector/oidc/oidc.go b/connector/oidc/oidc.go index ca28a988..04df3715 100644 --- a/connector/oidc/oidc.go +++ b/connector/oidc/oidc.go @@ -56,14 +56,15 @@ type Config struct { // PromptType will be used fot the prompt parameter (when offline_access, by default prompt=consent) PromptType string `json:"promptType"` + // OverrideClaimMapping will be used to override the options defined in claimMappings. + // i.e. if there are 'email' and `preferred_email` claims available, by default Dex will always use the `email` claim independent of the ClaimMapping.EmailKey. + // This setting allows you to override the default behavior of Dex and enforce the mappings defined in `claimMapping`. + OverrideClaimMapping bool `json:"overrideClaimMapping"` // defaults to false + ClaimMapping ClaimMapping `json:"claimMapping"` } type ClaimMapping struct { - // Enforce the ClaimMapping. - // i.e. an 'email' claim will always be taken if available, - // irrelevant of the settings in EmailKey. This option will enforce the ClaimMapping options independent of the existing claims. - Enforce bool `json:"enforce"` // defaults to false // Configurable key which contains the preferred username claims PreferredUsernameKey string `json:"preferred_username"` // defaults to "preferred_username" @@ -160,6 +161,7 @@ func (c *Config) Open(id string, logger log.Logger) (conn connector.Connector, e promptType: c.PromptType, userIDKey: c.UserIDKey, userNameKey: c.UserNameKey, + overrideClaimMapping: c.OverrideClaimMapping, claimMapping: c.ClaimMapping, }, nil } @@ -183,6 +185,7 @@ type oidcConnector struct { promptType string userIDKey string userNameKey string + overrideClaimMapping bool claimMapping ClaimMapping } @@ -293,7 +296,7 @@ func (c *oidcConnector) createIdentity(ctx context.Context, identity connector.I prefUsername := "preferred_username" preferredUsername, found := claims[prefUsername].(string) - if (!found || c.claimMapping.Enforce) && c.claimMapping.PreferredUsernameKey != "" { + if (!found || c.overrideClaimMapping) && c.claimMapping.PreferredUsernameKey != "" { prefUsername = c.claimMapping.PreferredUsernameKey preferredUsername, found = claims[prefUsername].(string) if !found { @@ -312,7 +315,7 @@ func (c *oidcConnector) createIdentity(ctx context.Context, identity connector.I var email string emailKey := "email" email, found = claims[emailKey].(string) - if (!found || c.claimMapping.Enforce) && c.claimMapping.EmailKey != "" { + if (!found || c.overrideClaimMapping) && c.claimMapping.EmailKey != "" { emailKey = c.claimMapping.EmailKey email, found = claims[emailKey].(string) if !found { @@ -337,7 +340,7 @@ func (c *oidcConnector) createIdentity(ctx context.Context, identity connector.I if c.insecureEnableGroups { groupsKey := "groups" vs, found := claims[groupsKey].([]interface{}) - if (!found || c.claimMapping.Enforce) && c.claimMapping.GroupsKey != "" { + if (!found || c.overrideClaimMapping) && c.claimMapping.GroupsKey != "" { groupsKey = c.claimMapping.GroupsKey vs, found = claims[groupsKey].([]interface{}) } diff --git a/connector/oidc/oidc_test.go b/connector/oidc/oidc_test.go index 6bc89645..267c0fcf 100644 --- a/connector/oidc/oidc_test.go +++ b/connector/oidc/oidc_test.go @@ -49,6 +49,7 @@ func TestHandleCallback(t *testing.T) { name string userIDKey string userNameKey string + overrideClaimMapping bool claimMapping ClaimMapping insecureSkipEmailVerified bool scopes []string @@ -93,11 +94,11 @@ func TestHandleCallback(t *testing.T) { }, }, { - name: "enforceCustomEmailClaim", - userIDKey: "", // not configured - userNameKey: "", // not configured + name: "overrideWithCustomEmailClaim", + userIDKey: "", // not configured + userNameKey: "", // not configured + overrideClaimMapping: true, claimMapping: ClaimMapping{ - Enforce: true, EmailKey: "custommail", }, expectUserID: "subvalue", @@ -260,9 +261,9 @@ func TestHandleCallback(t *testing.T) { }, }, { - name: "customGroupsKeyButGroupsProvidedButEnforced", + name: "customGroupsKeyButGroupsProvidedButOverride", + overrideClaimMapping: true, claimMapping: ClaimMapping{ - Enforce: true, GroupsKey: "cognito:groups", }, expectUserID: "subvalue", @@ -309,6 +310,7 @@ func TestHandleCallback(t *testing.T) { InsecureSkipEmailVerified: tc.insecureSkipEmailVerified, InsecureEnableGroups: true, BasicAuthUnsupported: &basicAuth, + OverrideClaimMapping: tc.overrideClaimMapping, } config.ClaimMapping = tc.claimMapping From 2b6bb1997c44aa881fd341860e308d65e53d116e Mon Sep 17 00:00:00 2001 From: Happy2C0de <46957159+Happy2C0de@users.noreply.github.com> Date: Thu, 19 Aug 2021 10:02:55 +0200 Subject: [PATCH 3/7] Revert ClaimMapping struct Signed-off-by: Happy2C0de <46957159+Happy2C0de@users.noreply.github.com> --- connector/oidc/oidc.go | 39 +++++++++++++------------- connector/oidc/oidc_test.go | 56 ++++++++++++++++--------------------- 2 files changed, 44 insertions(+), 51 deletions(-) diff --git a/connector/oidc/oidc.go b/connector/oidc/oidc.go index 04df3715..29241dc6 100644 --- a/connector/oidc/oidc.go +++ b/connector/oidc/oidc.go @@ -61,19 +61,16 @@ type Config struct { // This setting allows you to override the default behavior of Dex and enforce the mappings defined in `claimMapping`. OverrideClaimMapping bool `json:"overrideClaimMapping"` // defaults to false - ClaimMapping ClaimMapping `json:"claimMapping"` -} + ClaimMapping struct { + // Configurable key which contains the preferred username claims + PreferredUsernameKey string `json:"preferred_username"` // defaults to "preferred_username" -type ClaimMapping struct { + // Configurable key which contains the email claims + EmailKey string `json:"email"` // defaults to "email" - // 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" + // 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 @@ -162,7 +159,9 @@ func (c *Config) Open(id string, logger log.Logger) (conn connector.Connector, e userIDKey: c.UserIDKey, userNameKey: c.UserNameKey, overrideClaimMapping: c.OverrideClaimMapping, - claimMapping: c.ClaimMapping, + preferredUsernameKey: c.ClaimMapping.PreferredUsernameKey, + emailKey: c.ClaimMapping.EmailKey, + groupsKey: c.ClaimMapping.GroupsKey, }, nil } @@ -186,7 +185,9 @@ type oidcConnector struct { userIDKey string userNameKey string overrideClaimMapping bool - claimMapping ClaimMapping + preferredUsernameKey string + emailKey string + groupsKey string } func (c *oidcConnector) Close() error { @@ -296,8 +297,8 @@ func (c *oidcConnector) createIdentity(ctx context.Context, identity connector.I prefUsername := "preferred_username" preferredUsername, found := claims[prefUsername].(string) - if (!found || c.overrideClaimMapping) && c.claimMapping.PreferredUsernameKey != "" { - prefUsername = c.claimMapping.PreferredUsernameKey + if (!found || c.overrideClaimMapping) && c.preferredUsernameKey != "" { + prefUsername = c.preferredUsernameKey preferredUsername, found = claims[prefUsername].(string) if !found { return identity, fmt.Errorf("missing \"%s\" claim", prefUsername) @@ -315,8 +316,8 @@ func (c *oidcConnector) createIdentity(ctx context.Context, identity connector.I var email string emailKey := "email" email, found = claims[emailKey].(string) - if (!found || c.overrideClaimMapping) && c.claimMapping.EmailKey != "" { - emailKey = c.claimMapping.EmailKey + if (!found || c.overrideClaimMapping) && c.emailKey != "" { + emailKey = c.emailKey email, found = claims[emailKey].(string) if !found { return identity, fmt.Errorf("missing \"%s\" claim", emailKey) @@ -340,8 +341,8 @@ func (c *oidcConnector) createIdentity(ctx context.Context, identity connector.I if c.insecureEnableGroups { groupsKey := "groups" vs, found := claims[groupsKey].([]interface{}) - if (!found || c.overrideClaimMapping) && c.claimMapping.GroupsKey != "" { - groupsKey = c.claimMapping.GroupsKey + if (!found || c.overrideClaimMapping) && c.groupsKey != "" { + groupsKey = c.groupsKey vs, found = claims[groupsKey].([]interface{}) } diff --git a/connector/oidc/oidc_test.go b/connector/oidc/oidc_test.go index 267c0fcf..9040cf5c 100644 --- a/connector/oidc/oidc_test.go +++ b/connector/oidc/oidc_test.go @@ -50,7 +50,9 @@ func TestHandleCallback(t *testing.T) { userIDKey string userNameKey string overrideClaimMapping bool - claimMapping ClaimMapping + preferredUsernameKey string + emailKey string + groupsKey string insecureSkipEmailVerified bool scopes []string expectUserID string @@ -77,12 +79,10 @@ func TestHandleCallback(t *testing.T) { }, }, { - name: "customEmailClaim", - userIDKey: "", // not configured - userNameKey: "", // not configured - claimMapping: ClaimMapping{ - EmailKey: "mail", - }, + name: "customEmailClaim", + userIDKey: "", // not configured + userNameKey: "", // not configured + emailKey: "mail", expectUserID: "subvalue", expectUserName: "namevalue", expectedEmailField: "emailvalue", @@ -98,16 +98,14 @@ func TestHandleCallback(t *testing.T) { userIDKey: "", // not configured userNameKey: "", // not configured overrideClaimMapping: true, - claimMapping: ClaimMapping{ - EmailKey: "custommail", - }, - expectUserID: "subvalue", - expectUserName: "namevalue", - expectedEmailField: "customemailvalue", + emailKey: "custommail", + expectUserID: "subvalue", + expectUserName: "namevalue", + expectedEmailField: "customemailvalue", token: map[string]interface{}{ "sub": "subvalue", "name": "namevalue", - "mail": "emailvalue", + "email": "emailvalue", "custommail": "customemailvalue", "email_verified": true, }, @@ -151,10 +149,8 @@ func TestHandleCallback(t *testing.T) { }, }, { - name: "withPreferredUsernameKey", - claimMapping: ClaimMapping{ - PreferredUsernameKey: "username_key", - }, + name: "withPreferredUsernameKey", + preferredUsernameKey: "username_key", expectUserID: "subvalue", expectUserName: "namevalue", expectPreferredUsername: "username_value", @@ -222,10 +218,8 @@ func TestHandleCallback(t *testing.T) { }, }, { - name: "customGroupsKey", - claimMapping: ClaimMapping{ - GroupsKey: "cognito:groups", - }, + name: "customGroupsKey", + groupsKey: "cognito:groups", expectUserID: "subvalue", expectUserName: "namevalue", expectedEmailField: "emailvalue", @@ -241,10 +235,8 @@ func TestHandleCallback(t *testing.T) { }, }, { - name: "customGroupsKeyButGroupsProvided", - claimMapping: ClaimMapping{ - GroupsKey: "cognito:groups", - }, + name: "customGroupsKeyButGroupsProvided", + groupsKey: "cognito:groups", expectUserID: "subvalue", expectUserName: "namevalue", expectedEmailField: "emailvalue", @@ -261,11 +253,9 @@ func TestHandleCallback(t *testing.T) { }, }, { - name: "customGroupsKeyButGroupsProvidedButOverride", - overrideClaimMapping: true, - claimMapping: ClaimMapping{ - GroupsKey: "cognito:groups", - }, + name: "customGroupsKeyButGroupsProvidedButOverride", + overrideClaimMapping: true, + groupsKey: "cognito:groups", expectUserID: "subvalue", expectUserName: "namevalue", expectedEmailField: "emailvalue", @@ -312,7 +302,9 @@ func TestHandleCallback(t *testing.T) { BasicAuthUnsupported: &basicAuth, OverrideClaimMapping: tc.overrideClaimMapping, } - config.ClaimMapping = tc.claimMapping + config.ClaimMapping.PreferredUsernameKey = tc.preferredUsernameKey + config.ClaimMapping.EmailKey = tc.emailKey + config.ClaimMapping.GroupsKey = tc.groupsKey conn, err := newConnector(config) if err != nil { From 1608b473ebee57b9d7290b6e464abb53bd2fcd8c Mon Sep 17 00:00:00 2001 From: Happy2C0de <46957159+Happy2C0de@users.noreply.github.com> Date: Thu, 19 Aug 2021 13:23:02 +0200 Subject: [PATCH 4/7] Remove false failed errors. Signed-off-by: Happy2C0de <46957159+Happy2C0de@users.noreply.github.com> --- connector/oidc/oidc.go | 8 +------- connector/oidc/oidc_test.go | 2 +- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/connector/oidc/oidc.go b/connector/oidc/oidc.go index 29241dc6..318bcf86 100644 --- a/connector/oidc/oidc.go +++ b/connector/oidc/oidc.go @@ -299,10 +299,7 @@ func (c *oidcConnector) createIdentity(ctx context.Context, identity connector.I preferredUsername, found := claims[prefUsername].(string) if (!found || c.overrideClaimMapping) && c.preferredUsernameKey != "" { prefUsername = c.preferredUsernameKey - preferredUsername, found = claims[prefUsername].(string) - if !found { - return identity, fmt.Errorf("missing \"%s\" claim", prefUsername) - } + preferredUsername, _ = claims[prefUsername].(string) } hasEmailScope := false @@ -319,9 +316,6 @@ func (c *oidcConnector) createIdentity(ctx context.Context, identity connector.I if (!found || c.overrideClaimMapping) && c.emailKey != "" { emailKey = c.emailKey email, found = claims[emailKey].(string) - if !found { - return identity, fmt.Errorf("missing \"%s\" claim", emailKey) - } } if !found && hasEmailScope { diff --git a/connector/oidc/oidc_test.go b/connector/oidc/oidc_test.go index 9040cf5c..3038cebc 100644 --- a/connector/oidc/oidc_test.go +++ b/connector/oidc/oidc_test.go @@ -253,7 +253,7 @@ func TestHandleCallback(t *testing.T) { }, }, { - name: "customGroupsKeyButGroupsProvidedButOverride", + name: "customGroupsKeyDespiteGroupsProvidedButOverride", overrideClaimMapping: true, groupsKey: "cognito:groups", expectUserID: "subvalue", From b28098dde8aacc1f3693972ed36f08ee6efe47fc Mon Sep 17 00:00:00 2001 From: Happy2C0de <46957159+Happy2C0de@users.noreply.github.com> Date: Thu, 19 Aug 2021 13:28:32 +0200 Subject: [PATCH 5/7] Revert querying preferrredUsernameKey Signed-off-by: Happy2C0de <46957159+Happy2C0de@users.noreply.github.com> --- connector/oidc/oidc.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/connector/oidc/oidc.go b/connector/oidc/oidc.go index 318bcf86..b0467330 100644 --- a/connector/oidc/oidc.go +++ b/connector/oidc/oidc.go @@ -295,11 +295,9 @@ func (c *oidcConnector) createIdentity(ctx context.Context, identity connector.I return identity, fmt.Errorf("missing \"%s\" claim", userNameKey) } - prefUsername := "preferred_username" - preferredUsername, found := claims[prefUsername].(string) + preferredUsername, found := claims["preferred_username"].(string) if (!found || c.overrideClaimMapping) && c.preferredUsernameKey != "" { - prefUsername = c.preferredUsernameKey - preferredUsername, _ = claims[prefUsername].(string) + preferredUsername, _ = claims[c.preferredUsernameKey].(string) } hasEmailScope := false From 55605751f500a6d9ef2e2873f5aad5aeab8458ad Mon Sep 17 00:00:00 2001 From: Happy2C0de <46957159+Happy2C0de@users.noreply.github.com> Date: Tue, 24 Aug 2021 07:13:34 +0200 Subject: [PATCH 6/7] Add overrideWithMissingCustomEmailClaim test Signed-off-by: Happy2C0de <46957159+Happy2C0de@users.noreply.github.com> --- connector/oidc/oidc.go | 4 ++++ connector/oidc/oidc_test.go | 17 +++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/connector/oidc/oidc.go b/connector/oidc/oidc.go index b0467330..f33b01f0 100644 --- a/connector/oidc/oidc.go +++ b/connector/oidc/oidc.go @@ -314,6 +314,10 @@ func (c *oidcConnector) createIdentity(ctx context.Context, identity connector.I if (!found || c.overrideClaimMapping) && c.emailKey != "" { emailKey = c.emailKey email, found = claims[emailKey].(string) + if !found && c.overrideClaimMapping { + // If override is enabled but claim was not found, empty string is preferred over fallback. + email, found = "", true + } } if !found && hasEmailScope { diff --git a/connector/oidc/oidc_test.go b/connector/oidc/oidc_test.go index 3038cebc..d92fdea5 100644 --- a/connector/oidc/oidc_test.go +++ b/connector/oidc/oidc_test.go @@ -110,6 +110,23 @@ func TestHandleCallback(t *testing.T) { "email_verified": true, }, }, + { + name: "overrideWithMissingCustomEmailClaim", + userIDKey: "", // not configured + userNameKey: "", // not configured + overrideClaimMapping: true, + emailKey: "custommail", + expectUserID: "subvalue", + expectUserName: "namevalue", + expectedEmailField: "", + token: map[string]interface{}{ + // no "custommail" claim + "sub": "subvalue", + "name": "namevalue", + "email": "emailvalue", + "email_verified": true, + }, + }, { name: "email_verified not in claims, configured to be skipped", insecureSkipEmailVerified: true, From 419db81c6769329432ebe74e905a71adbcd8d28b Mon Sep 17 00:00:00 2001 From: Happy2C0de <46957159+Happy2C0de@users.noreply.github.com> Date: Mon, 20 Sep 2021 07:43:16 +0200 Subject: [PATCH 7/7] Remove overrideWithMissingCustomEmailClaim Signed-off-by: Happy2C0de <46957159+Happy2C0de@users.noreply.github.com> --- connector/oidc/oidc.go | 4 ---- connector/oidc/oidc_test.go | 17 ----------------- 2 files changed, 21 deletions(-) diff --git a/connector/oidc/oidc.go b/connector/oidc/oidc.go index f33b01f0..b0467330 100644 --- a/connector/oidc/oidc.go +++ b/connector/oidc/oidc.go @@ -314,10 +314,6 @@ func (c *oidcConnector) createIdentity(ctx context.Context, identity connector.I if (!found || c.overrideClaimMapping) && c.emailKey != "" { emailKey = c.emailKey email, found = claims[emailKey].(string) - if !found && c.overrideClaimMapping { - // If override is enabled but claim was not found, empty string is preferred over fallback. - email, found = "", true - } } if !found && hasEmailScope { diff --git a/connector/oidc/oidc_test.go b/connector/oidc/oidc_test.go index d92fdea5..3038cebc 100644 --- a/connector/oidc/oidc_test.go +++ b/connector/oidc/oidc_test.go @@ -110,23 +110,6 @@ func TestHandleCallback(t *testing.T) { "email_verified": true, }, }, - { - name: "overrideWithMissingCustomEmailClaim", - userIDKey: "", // not configured - userNameKey: "", // not configured - overrideClaimMapping: true, - emailKey: "custommail", - expectUserID: "subvalue", - expectUserName: "namevalue", - expectedEmailField: "", - token: map[string]interface{}{ - // no "custommail" claim - "sub": "subvalue", - "name": "namevalue", - "email": "emailvalue", - "email_verified": true, - }, - }, { name: "email_verified not in claims, configured to be skipped", insecureSkipEmailVerified: true,