diff --git a/connector/oidc/oidc.go b/connector/oidc/oidc.go index 2c199112..ee6b24a8 100644 --- a/connector/oidc/oidc.go +++ b/connector/oidc/oidc.go @@ -262,15 +262,25 @@ func (c *oidcConnector) createIdentity(ctx context.Context, identity connector.I if !found { return identity, fmt.Errorf("missing \"%s\" claim", userNameKey) } + + hasEmailScope := false + for _, s := range c.oauth2Config.Scopes { + if s == "email" { + hasEmailScope = true + break + } + } + email, found := claims["email"].(string) - if !found { + if !found && hasEmailScope { return identity, errors.New("missing \"email\" claim") } + emailVerified, found := claims["email_verified"].(bool) if !found { if c.insecureSkipEmailVerified { emailVerified = true - } else { + } else if hasEmailScope { return identity, errors.New("missing \"email_verified\" claim") } } diff --git a/connector/oidc/oidc_test.go b/connector/oidc/oidc_test.go index a6118b2f..52afa158 100644 --- a/connector/oidc/oidc_test.go +++ b/connector/oidc/oidc_test.go @@ -50,16 +50,19 @@ func TestHandleCallback(t *testing.T) { userIDKey string userNameKey string insecureSkipEmailVerified bool + scopes []string expectUserID string expectUserName string + expectedEmailField string token map[string]interface{} }{ { - name: "simpleCase", - userIDKey: "", // not configured - userNameKey: "", // not configured - expectUserID: "subvalue", - expectUserName: "namevalue", + name: "simpleCase", + userIDKey: "", // not configured + userNameKey: "", // not configured + expectUserID: "subvalue", + expectUserName: "namevalue", + expectedEmailField: "emailvalue", token: map[string]interface{}{ "sub": "subvalue", "name": "namevalue", @@ -72,6 +75,7 @@ func TestHandleCallback(t *testing.T) { insecureSkipEmailVerified: true, expectUserID: "subvalue", expectUserName: "namevalue", + expectedEmailField: "emailvalue", token: map[string]interface{}{ "sub": "subvalue", "name": "namevalue", @@ -79,10 +83,11 @@ func TestHandleCallback(t *testing.T) { }, }, { - name: "withUserIDKey", - userIDKey: "name", - expectUserID: "namevalue", - expectUserName: "namevalue", + name: "withUserIDKey", + userIDKey: "name", + expectUserID: "namevalue", + expectUserName: "namevalue", + expectedEmailField: "emailvalue", token: map[string]interface{}{ "sub": "subvalue", "name": "namevalue", @@ -91,10 +96,11 @@ func TestHandleCallback(t *testing.T) { }, }, { - name: "withUserNameKey", - userNameKey: "user_name", - expectUserID: "subvalue", - expectUserName: "username", + name: "withUserNameKey", + userNameKey: "user_name", + expectUserID: "subvalue", + expectUserName: "username", + expectedEmailField: "emailvalue", token: map[string]interface{}{ "sub": "subvalue", "user_name": "username", @@ -102,6 +108,33 @@ func TestHandleCallback(t *testing.T) { "email_verified": true, }, }, + { + name: "emptyEmailScope", + expectUserID: "subvalue", + expectUserName: "namevalue", + expectedEmailField: "", + scopes: []string{"groups"}, + insecureSkipEmailVerified: true, + token: map[string]interface{}{ + "sub": "subvalue", + "name": "namevalue", + "user_name": "username", + }, + }, + { + name: "emptyEmailScopeButEmailProvided", + expectUserID: "subvalue", + expectUserName: "namevalue", + expectedEmailField: "emailvalue", + scopes: []string{"groups"}, + insecureSkipEmailVerified: true, + token: map[string]interface{}{ + "sub": "subvalue", + "name": "namevalue", + "user_name": "username", + "email": "emailvalue", + }, + }, } for _, tc := range tests { @@ -111,13 +144,20 @@ func TestHandleCallback(t *testing.T) { t.Fatal("failed to setup test server", err) } defer testServer.Close() + + var scopes []string + if len(tc.scopes) > 0 { + scopes = tc.scopes + } else { + scopes = []string{"email", "groups"} + } serverURL := testServer.URL basicAuth := true config := Config{ Issuer: serverURL, ClientID: "clientID", ClientSecret: "clientSecret", - Scopes: []string{"groups"}, + Scopes: scopes, RedirectURI: fmt.Sprintf("%s/callback", serverURL), UserIDKey: tc.userIDKey, UserNameKey: tc.userNameKey, @@ -142,7 +182,7 @@ func TestHandleCallback(t *testing.T) { expectEquals(t, identity.UserID, tc.expectUserID) expectEquals(t, identity.Username, tc.expectUserName) - expectEquals(t, identity.Email, "emailvalue") + expectEquals(t, identity.Email, tc.expectedEmailField) expectEquals(t, identity.EmailVerified, true) }) }