Merge e80d6c8571 into 13f012fb81

connector/oauth/oauth.go

@@ -193,8 +193,12 @@ func (c *oauthConnector) HandleCallback(s connector.Scopes, _ []byte, r *http.Re
 	if s.Groups {
 		groups := map[string]struct{}{}
 
-		c.addGroupsFromMap(groups, userInfoResult)
+		if err := c.addGroupsFromMap(groups, userInfoResult); err != nil {
-		c.addGroupsFromToken(groups, token.AccessToken)
+			c.logger.Warn("OAuth Connector: failed to add groups from userinfo", "error", err)
+		}
+		if err := c.addGroupsFromToken(groups, token.AccessToken); err != nil {
+			c.logger.Warn("OAuth Connector: failed to add groups from token", "error", err)
+		}
 
 		for groupName := range groups {
 			identity.Groups = append(identity.Groups, groupName)
@@ -216,7 +220,15 @@ func (c *oauthConnector) HandleCallback(s connector.Scopes, _ []byte, r *http.Re
 func (c *oauthConnector) addGroupsFromMap(groups map[string]struct{}, result map[string]interface{}) error {
 	groupsClaim, ok := result[c.groupsKey].([]interface{})
 	if !ok {
-		return errors.New("cannot convert to slice")
+		// sometimes the groups claim is a slice encoded as a JSON string
+		groupsStr, ok := result[c.groupsKey].(string)
+		if !ok {
+			return fmt.Errorf("%T claim is not a list or JSON-encoded list", result[c.groupsKey])
+		}
+		err := json.Unmarshal([]byte(groupsStr), &groupsClaim)
+		if err != nil {
+			return fmt.Errorf("failed to decode groups claim: %v", err)
+		}
 	}
 
 	for _, group := range groupsClaim {

connector/oauth/oauth_test.go

@@ -100,6 +100,39 @@ func TestHandleCallBackForGroupsInUserInfo(t *testing.T) {
 	assert.Equal(t, identity.EmailVerified, false)
 }
 
+func TestHandleCallBackForGroupsInUserInfoIsString(t *testing.T) {
+	tokenClaims := map[string]interface{}{}
+
+	userInfoClaims := map[string]interface{}{
+		"name":               "test-name",
+		"user_id_key":        "test-user-id",
+		"user_name_key":      "test-username",
+		"preferred_username": "test-preferred-username",
+		"mail":               "mod_mail",
+		"has_verified_email": false,
+		"groups_key":         `["admin-group", "user-group"]`,
+	}
+
+	testServer := testSetup(t, tokenClaims, userInfoClaims)
+	defer testServer.Close()
+
+	conn := newConnector(t, testServer.URL)
+	req := newRequestWithAuthCode(t, testServer.URL, "TestHandleCallBackForGroupsInUserInfo")
+
+	identity, err := conn.HandleCallback(connector.Scopes{Groups: true}, req)
+	assert.Equal(t, err, nil)
+
+	sort.Strings(identity.Groups)
+	assert.Equal(t, len(identity.Groups), 2)
+	assert.Equal(t, identity.Groups[0], "admin-group")
+	assert.Equal(t, identity.Groups[1], "user-group")
+	assert.Equal(t, identity.UserID, "test-user-id")
+	assert.Equal(t, identity.Username, "test-username")
+	assert.Equal(t, identity.PreferredUsername, "test-preferred-username")
+	assert.Equal(t, identity.Email, "mod_mail")
+	assert.Equal(t, identity.EmailVerified, false)
+}
+
 func TestHandleCallBackForGroupMapsInUserInfo(t *testing.T) {
 	tokenClaims := map[string]interface{}{}