From 4e8cbf0f61dd1434c9202c0d1832bf82001b300a Mon Sep 17 00:00:00 2001
From: Stephan Renatus <srenatus@chef.io>
Date: Tue, 28 May 2019 14:43:00 +0200
Subject: [PATCH] connectors/oidc: truely ignore "email_verified" claim if
 configured that way

Fixes #1455, I hope.

Signed-off-by: Stephan Renatus <srenatus@chef.io>
---
 connector/oidc/oidc.go      | 10 ++---
 connector/oidc/oidc_test.go | 77 +++++++++++++++++++++++++------------
 2 files changed, 57 insertions(+), 30 deletions(-)

diff --git a/connector/oidc/oidc.go b/connector/oidc/oidc.go
index 628bd3e0..88b4e29d 100644
--- a/connector/oidc/oidc.go
+++ b/connector/oidc/oidc.go
@@ -219,7 +219,11 @@ func (c *oidcConnector) HandleCallback(s connector.Scopes, r *http.Request) (ide
 	}
 	emailVerified, found := claims["email_verified"].(bool)
 	if !found {
-		return identity, errors.New("missing \"email_verified\" claim")
+		if c.insecureSkipEmailVerified {
+			emailVerified = true
+		} else {
+			return identity, errors.New("missing \"email_verified\" claim")
+		}
 	}
 	hostedDomain, _ := claims["hd"].(string)
 
@@ -237,10 +241,6 @@ func (c *oidcConnector) HandleCallback(s connector.Scopes, r *http.Request) (ide
 		}
 	}
 
-	if c.insecureSkipEmailVerified {
-		emailVerified = true
-	}
-
 	if c.getUserInfo {
 		userInfo, err := c.provider.UserInfo(r.Context(), oauth2.StaticTokenSource(token))
 		if err != nil {
diff --git a/connector/oidc/oidc_test.go b/connector/oidc/oidc_test.go
index e9d0889a..7d41a37c 100644
--- a/connector/oidc/oidc_test.go
+++ b/connector/oidc/oidc_test.go
@@ -45,29 +45,62 @@ func TestHandleCallback(t *testing.T) {
 	t.Helper()
 
 	tests := []struct {
-		name         string
-		userIDKey    string
-		expectUserID string
+		name                      string
+		userIDKey                 string
+		insecureSkipEmailVerified bool
+		expectUserID              string
+		token                     map[string]interface{}
 	}{
-		{"simpleCase", "", "sub"},
-		{"withUserIDKey", "name", "name"},
+		{
+			name:         "simpleCase",
+			userIDKey:    "", // not configured
+			expectUserID: "subvalue",
+			token: map[string]interface{}{
+				"sub":            "subvalue",
+				"name":           "namevalue",
+				"email":          "emailvalue",
+				"email_verified": true,
+			},
+		},
+		{
+			name:                      "email_verified not in claims, configured to be skipped",
+			insecureSkipEmailVerified: true,
+			expectUserID:              "subvalue",
+			token: map[string]interface{}{
+				"sub":   "subvalue",
+				"name":  "namevalue",
+				"email": "emailvalue",
+			},
+		},
+		{
+			name:         "withUserIDKey",
+			userIDKey:    "name",
+			expectUserID: "namevalue",
+			token: map[string]interface{}{
+				"sub":            "subvalue",
+				"name":           "namevalue",
+				"email":          "emailvalue",
+				"email_verified": true,
+			},
+		},
 	}
 
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			testServer, err := setupServer()
+			testServer, err := setupServer(tc.token)
 			if err != nil {
 				t.Fatal("failed to setup test server", err)
 			}
 			defer testServer.Close()
 			serverURL := testServer.URL
 			config := Config{
-				Issuer:       serverURL,
-				ClientID:     "clientID",
-				ClientSecret: "clientSecret",
-				Scopes:       []string{"groups"},
-				RedirectURI:  fmt.Sprintf("%s/callback", serverURL),
-				UserIDKey:    tc.userIDKey,
+				Issuer:                    serverURL,
+				ClientID:                  "clientID",
+				ClientSecret:              "clientSecret",
+				Scopes:                    []string{"groups"},
+				RedirectURI:               fmt.Sprintf("%s/callback", serverURL),
+				UserIDKey:                 tc.userIDKey,
+				InsecureSkipEmailVerified: tc.insecureSkipEmailVerified,
 			}
 
 			conn, err := newConnector(config)
@@ -86,14 +119,14 @@ func TestHandleCallback(t *testing.T) {
 			}
 
 			expectEquals(t, identity.UserID, tc.expectUserID)
-			expectEquals(t, identity.Username, "name")
-			expectEquals(t, identity.Email, "email")
+			expectEquals(t, identity.Username, "namevalue")
+			expectEquals(t, identity.Email, "emailvalue")
 			expectEquals(t, identity.EmailVerified, true)
 		})
 	}
 }
 
-func setupServer() (*httptest.Server, error) {
+func setupServer(tok map[string]interface{}) (*httptest.Server, error) {
 	key, err := rsa.GenerateKey(rand.Reader, 1024)
 	if err != nil {
 		return nil, fmt.Errorf("failed to generate rsa key: %v", err)
@@ -121,16 +154,10 @@ func setupServer() (*httptest.Server, error) {
 
 	mux.HandleFunc("/token", func(w http.ResponseWriter, r *http.Request) {
 		url := fmt.Sprintf("http://%s", r.Host)
-
-		token, err := newToken(&jwk, map[string]interface{}{
-			"iss":            url,
-			"aud":            "clientID",
-			"exp":            time.Now().Add(time.Hour).Unix(),
-			"sub":            "sub",
-			"name":           "name",
-			"email":          "email",
-			"email_verified": true,
-		})
+		tok["iss"] = url
+		tok["exp"] = time.Now().Add(time.Hour).Unix()
+		tok["aud"] = "clientID"
+		token, err := newToken(&jwk, tok)
 		if err != nil {
 			w.WriteHeader(http.StatusInternalServerError)
 		}