Merge pull request #1610 from flant/oidc-email-scope-check

Adding oidc email scope check
dexidp · Jan 6, 2020 · 3cbba11 · 3cbba11
2 parents 53897e8 + 383c2fe
commit 3cbba11
Show file tree

Hide file tree

Showing 2 changed files with 67 additions and 17 deletions.
diff --git 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
@@ -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,17 +75,19 @@ func TestHandleCallback(t *testing.T) {
 			insecureSkipEmailVerified: true,
 			expectUserID:              "subvalue",
 			expectUserName:            "namevalue",
+			expectedEmailField:        "emailvalue",
 			token: map[string]interface{}{
 				"sub":   "subvalue",
 				"name":  "namevalue",
 				"email": "emailvalue",
 			},
 		},
 		{
-			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,17 +96,45 @@ 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",
 				"email":          "emailvalue",
 				"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)
 		})
 	}