KEYCLOAK-14869: Fix nullpointer exception in FullNameLDAPStorageMapper

Setting an attribute should be possible with a list
containing no elements or a null list

This can happen e.g. when creating users via idps
using a UserAttributeStatementMapper.

Fix this unprotected access in other classes too

federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/FullNameLDAPStorageMapper.java

@@ -141,14 +141,15 @@ public class FullNameLDAPStorageMapper extends AbstractLDAPStorageMapper {
 
                 @Override
                 public void setAttribute(String name, List<String> values) {
+                    String valueToSet = (values != null && values.size() > 0) ? values.get(0) : null;
                     if (UserModel.FIRST_NAME.equals(name)) {
-                        this.firstName = values.get(0);
+                        this.firstName = valueToSet;
                         setFullNameToLDAPObject();
                     } else if (UserModel.LAST_NAME.equals(name)) {
-                        this.lastName = values.get(0);
+                        this.lastName = valueToSet;
                         setFullNameToLDAPObject();
                     }
-                    super.setSingleAttribute(name, values.get(0));
+                    super.setSingleAttribute(name, valueToSet);
                 }
 
                 @Override

federation/ldap/src/main/java/org/keycloak/storage/ldap/mappers/UserAttributeLDAPStorageMapper.java

@@ -203,10 +203,11 @@ public class UserAttributeLDAPStorageMapper extends AbstractLDAPStorageMapper {
 
                 @Override
                 public void setAttribute(String name, List<String> values) {
+                    String valueToSet = (values != null && values.size() > 0) ? values.get(0) : null;
                     if (UserModel.USERNAME.equals(name)) {
-                        checkDuplicateUsername(userModelAttrName, values.get(0), realm, ldapProvider.getSession(), this);
+                        checkDuplicateUsername(userModelAttrName, valueToSet, realm, ldapProvider.getSession(), this);
                     } else if (UserModel.EMAIL.equals(name)) {
-                        checkDuplicateEmail(userModelAttrName, values.get(0), realm, ldapProvider.getSession(), this);
+                        checkDuplicateEmail(userModelAttrName, valueToSet, realm, ldapProvider.getSession(), this);
                     }
                     if (setLDAPAttribute(name, values)) {
                         super.setAttribute(name, values);

model/jpa/src/main/java/org/keycloak/models/jpa/UserAdapter.java

@@ -164,17 +164,18 @@ public class UserAdapter implements UserModel, JpaModel<UserEntity> {
 
     @Override
     public void setAttribute(String name, List<String> values) {
+        String valueToSet = (values != null && values.size() > 0) ? values.get(0) : null;
         if (UserModel.FIRST_NAME.equals(name)) {
-            user.setFirstName(values.get(0));
+            user.setFirstName(valueToSet);
             return;
         } else if (UserModel.LAST_NAME.equals(name)) {
-            user.setLastName(values.get(0));
+            user.setLastName(valueToSet);
             return;
         } else if (UserModel.EMAIL.equals(name)) {
-            setEmail(values.get(0));
+            setEmail(valueToSet);
             return;
         } else if (UserModel.USERNAME.equals(name)) {
-            setUsername(values.get(0));
+            setUsername(valueToSet);
             return;
         }
         // Remove all existing

server-spi/src/main/java/org/keycloak/storage/adapter/AbstractUserAdapterFederatedStorage.java

@@ -354,7 +354,7 @@ public abstract class AbstractUserAdapterFederatedStorage extends UserModelDefau
     @Override
     public void setAttribute(String name, List<String> values) {
         if (UserModel.USERNAME.equals(name)) {
-            setUsername(values.get(0));
+            setUsername((values != null && values.size() > 0) ? values.get(0) : null);
         }
         getFederatedStorage().setAttribute(realm, this.getId(), mapAttribute(name), values);
 

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPProvidersFullNameMapperTest.java

@@ -0,0 +1,166 @@
+/*
+ * Copyright 2017 Red Hat, Inc. and/or its affiliates
+ * and other contributors as indicated by the @author tags.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.keycloak.testsuite.federation.ldap;
+
+import org.junit.Assert;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.keycloak.component.ComponentModel;
+import org.keycloak.models.RealmModel;
+import org.keycloak.models.UserModel;
+import org.keycloak.models.utils.KeycloakModelUtils;
+import org.keycloak.storage.ldap.LDAPStorageProvider;
+import org.keycloak.storage.ldap.mappers.FullNameLDAPStorageMapper;
+import org.keycloak.storage.ldap.mappers.FullNameLDAPStorageMapperFactory;
+import org.keycloak.storage.ldap.mappers.LDAPStorageMapper;
+import org.keycloak.storage.ldap.mappers.UserAttributeLDAPStorageMapper;
+import org.keycloak.testsuite.util.LDAPRule;
+import org.keycloak.testsuite.util.LDAPTestUtils;
+
+import java.util.ArrayList;
+import java.util.Collections;
+
+import static org.hamcrest.Matchers.contains;
+import static org.hamcrest.Matchers.empty;
+import static org.hamcrest.Matchers.is;
+import static org.junit.Assert.assertThat;
+
+/**
+ * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
+ * @author <a href="mailto:external.Martin.Idel@bosch.io">Martin Idel</a>
+ */
+public class LDAPProvidersFullNameMapperTest extends AbstractLDAPTest {
+
+    @ClassRule
+    public static LDAPRule ldapRule = new LDAPRule();
+
+    @Override
+    protected LDAPRule getLDAPRule() {
+        return ldapRule;
+    }
+
+    @Override
+    protected void afterImportTestRealm() {
+        testingClient.server().run(session -> {
+            LDAPTestContext ctx = LDAPTestContext.init(session);
+            RealmModel appRealm = ctx.getRealm();
+
+            LDAPTestUtils.addZipCodeLDAPMapper(appRealm, ctx.getLdapModel());
+            LDAPTestUtils.removeAllLDAPUsers(ctx.getLdapProvider(), appRealm);
+
+            appRealm.getClientByClientId("test-app").setDirectAccessGrantsEnabled(true);
+
+            // assert that user "fullnameUser" is not in local DB
+            Assert.assertNull(session.users().getUserByUsername("fullname", appRealm));
+
+            // Add the user with some fullName into LDAP directly. Ensure that fullName is saved into "cn" attribute in LDAP (currently mapped to model firstName)
+            ComponentModel ldapModel = LDAPTestUtils.getLdapProviderModel(session, appRealm);
+
+            // add fullname mapper to the provider and remove "firstNameMapper". For this test, we will simply map full name to the LDAP attribute, which was before firstName ( "givenName" on active directory, "cn" on other LDAP servers)
+            ComponentModel firstNameMapper = LDAPTestUtils.getSubcomponentByName(appRealm, ldapModel, "first name");
+            String ldapFirstNameAttributeName = firstNameMapper.getConfig().getFirst(UserAttributeLDAPStorageMapper.LDAP_ATTRIBUTE);
+            appRealm.removeComponent(firstNameMapper);
+
+            ComponentModel fullNameMapperModel = KeycloakModelUtils.createComponentModel("full name", ldapModel.getId(), FullNameLDAPStorageMapperFactory.PROVIDER_ID, LDAPStorageMapper.class.getName(),
+                    FullNameLDAPStorageMapper.LDAP_FULL_NAME_ATTRIBUTE, ldapFirstNameAttributeName,
+                    FullNameLDAPStorageMapper.READ_ONLY, "false");
+            appRealm.addComponentModel(fullNameMapperModel);
+        });
+    }
+
+    @Test
+    public void testUpdatingFirstNameAndLastNamePropagatesToFullnameMapper() {
+
+        testingClient.server().run(session -> {
+            LDAPTestContext ctx = LDAPTestContext.init(session);
+            RealmModel appRealm = ctx.getRealm();
+
+            ComponentModel ldapModel = LDAPTestUtils.getLdapProviderModel(session, appRealm);
+            LDAPStorageProvider ldapFedProvider = LDAPTestUtils.getLdapProvider(session, ldapModel);
+            LDAPTestUtils.addLDAPUser(ldapFedProvider, appRealm, "fullname", "James", "Dee", "fullname@email.org", null, "4578");
+
+            // Assert user is successfully imported in Keycloak DB now with correct firstName and lastName
+            LDAPTestAsserts.assertUserImported(session.users(), appRealm, "fullname", "James", "Dee", "fullname@email.org", "4578");
+        });
+
+        // Assert user will be changed in LDAP too
+        testingClient.server().run(session -> {
+            LDAPTestContext ctx = LDAPTestContext.init(session);
+            RealmModel appRealm = ctx.getRealm();
+
+            UserModel fullnameUser = session.users().getUserByUsername("fullname", appRealm);
+            fullnameUser.setFirstName("James2");
+            fullnameUser.setLastName("Dee2");
+        });
+
+        // Assert changed user available in Keycloak
+        testingClient.server().run(session -> {
+            LDAPTestContext ctx = LDAPTestContext.init(session);
+            RealmModel appRealm = ctx.getRealm();
+
+            // Assert user is successfully imported in Keycloak DB now with correct firstName and lastName
+            LDAPTestAsserts.assertUserImported(session.users(), appRealm, "fullname", "James2", "Dee2", "fullname@email.org", "4578");
+
+            // Remove "fullnameUser" to assert he is removed from LDAP.
+            UserModel fullnameUser = session.users().getUserByUsername("fullname", appRealm);
+            session.users().removeUser(appRealm, fullnameUser);
+        });
+    }
+
+    @Test
+    public void testUpdatingAttributesWorksEvenWithEmptyAttributes() {
+
+        testingClient.server().run(session -> {
+            LDAPTestContext ctx = LDAPTestContext.init(session);
+            RealmModel appRealm = ctx.getRealm();
+
+            ComponentModel ldapModel = LDAPTestUtils.getLdapProviderModel(session, appRealm);
+            LDAPStorageProvider ldapFedProvider = LDAPTestUtils.getLdapProvider(session, ldapModel);
+            LDAPTestUtils.addLDAPUser(ldapFedProvider, appRealm, "fullname", "James", "Dee", "fullname@email.org", null, "4578");
+
+            LDAPTestAsserts.assertUserImported(session.users(), appRealm, "fullname", "James", "Dee", "fullname@email.org", "4578");
+        });
+
+        testingClient.server().run(session -> {
+            LDAPTestContext ctx = LDAPTestContext.init(session);
+            RealmModel appRealm = ctx.getRealm();
+
+            UserModel fullnameUser = session.users().getUserByUsername("fullname", appRealm);
+            fullnameUser.setAttribute("myAttribute", Collections.singletonList("test"));
+            fullnameUser.setAttribute("myEmptyAttribute", new ArrayList<>());
+            fullnameUser.setAttribute("myNullAttribute", null);
+        });
+
+        // Assert changed user available in Keycloak
+        testingClient.server().run(session -> {
+            LDAPTestContext ctx = LDAPTestContext.init(session);
+            RealmModel appRealm = ctx.getRealm();
+
+            // Assert user is successfully imported in Keycloak DB now with correct firstName and lastName
+            LDAPTestAsserts.assertUserImported(session.users(), appRealm, "fullname", "James", "Dee", "fullname@email.org", "4578");
+
+            UserModel fullnameUser = session.users().getUserByUsername("fullname", appRealm);
+            assertThat(fullnameUser.getAttribute("myAttribute"), contains("test"));
+            assertThat(fullnameUser.getAttribute("myEmptyAttribute"), is(empty()));
+            assertThat(fullnameUser.getAttribute("myNullAttribute"), is(empty()));
+
+            // Remove "fullnameUser" to assert he is removed from LDAP.
+            session.users().removeUser(appRealm, fullnameUser);
+        });
+    }
+}

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPProvidersIntegrationTest.java

@@ -36,7 +36,6 @@ import org.keycloak.models.UserModel;
 import org.keycloak.models.cache.CachedUserModel;
 import org.keycloak.models.credential.PasswordCredentialModel;
 import org.keycloak.models.utils.KeycloakModelUtils;
-import org.keycloak.models.utils.ModelToRepresentation;
 import org.keycloak.representations.AccessToken;
 import org.keycloak.representations.idm.ComponentRepresentation;
 import org.keycloak.representations.idm.CredentialRepresentation;
@@ -49,8 +48,6 @@ import org.keycloak.storage.UserStorageProviderModel;
 import org.keycloak.storage.ldap.LDAPConfig;
 import org.keycloak.storage.ldap.LDAPStorageProvider;
 import org.keycloak.storage.ldap.idm.model.LDAPObject;
-import org.keycloak.storage.ldap.mappers.FullNameLDAPStorageMapper;
-import org.keycloak.storage.ldap.mappers.FullNameLDAPStorageMapperFactory;
 import org.keycloak.storage.ldap.mappers.HardcodedLDAPAttributeMapper;
 import org.keycloak.storage.ldap.mappers.HardcodedLDAPAttributeMapperFactory;
 import org.keycloak.storage.ldap.mappers.HardcodedLDAPGroupStorageMapper;
@@ -68,7 +65,6 @@ import org.keycloak.testsuite.util.OAuthClient;
 
 import javax.naming.AuthenticationException;
 import javax.ws.rs.core.Response;
-
 import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
@@ -554,80 +550,6 @@ public class LDAPProvidersIntegrationTest extends AbstractLDAPTest {
     }
 
 
-    // TODO: Rather separate test class for fullNameMapper to better test all the possibilities
-    @Test
-    public void testFullNameMapper() {
-
-        ComponentRepresentation firstNameMapperRep = testingClient.server().fetch(session -> {
-            LDAPTestContext ctx = LDAPTestContext.init(session);
-            RealmModel appRealm = ctx.getRealm();
-
-            // assert that user "fullnameUser" is not in local DB
-            Assert.assertNull(session.users().getUserByUsername("fullname", appRealm));
-
-            // Add the user with some fullName into LDAP directly. Ensure that fullName is saved into "cn" attribute in LDAP (currently mapped to model firstName)
-            ComponentModel ldapModel = LDAPTestUtils.getLdapProviderModel(session, appRealm);
-            LDAPStorageProvider ldapFedProvider = LDAPTestUtils.getLdapProvider(session, ldapModel);
-            LDAPTestUtils.addLDAPUser(ldapFedProvider, appRealm, "fullname", "James Dee", "Dee", "fullname@email.org", null, "4578");
-
-            // add fullname mapper to the provider and remove "firstNameMapper". For this test, we will simply map full name to the LDAP attribute, which was before firstName ( "givenName" on active directory, "cn" on other LDAP servers)
-            ComponentModel firstNameMapper =  LDAPTestUtils.getSubcomponentByName(appRealm, ldapModel, "first name");
-            String ldapFirstNameAttributeName = firstNameMapper.getConfig().getFirst(UserAttributeLDAPStorageMapper.LDAP_ATTRIBUTE);
-            appRealm.removeComponent(firstNameMapper);
-
-            ComponentRepresentation firstNameMapperRepp = ModelToRepresentation.toRepresentation(session, firstNameMapper, true);
-
-            ComponentModel fullNameMapperModel = KeycloakModelUtils.createComponentModel("full name", ldapModel.getId(), FullNameLDAPStorageMapperFactory.PROVIDER_ID, LDAPStorageMapper.class.getName(),
-                    FullNameLDAPStorageMapper.LDAP_FULL_NAME_ATTRIBUTE, ldapFirstNameAttributeName,
-                    FullNameLDAPStorageMapper.READ_ONLY, "false");
-            appRealm.addComponentModel(fullNameMapperModel);
-
-            return firstNameMapperRepp;
-        }, ComponentRepresentation.class);
-
-        testingClient.server().run(session -> {
-            LDAPTestContext ctx = LDAPTestContext.init(session);
-            RealmModel appRealm = ctx.getRealm();
-
-            // Assert user is successfully imported in Keycloak DB now with correct firstName and lastName
-            LDAPTestAsserts.assertUserImported(session.users(), appRealm, "fullname", "James", "Dee", "fullname@email.org", "4578");
-        });
-
-        // Assert user will be changed in LDAP too
-        testingClient.server().run(session -> {
-            LDAPTestContext ctx = LDAPTestContext.init(session);
-            RealmModel appRealm = ctx.getRealm();
-
-            UserModel fullnameUser = session.users().getUserByUsername("fullname", appRealm);
-            fullnameUser.setFirstName("James2");
-            fullnameUser.setLastName("Dee2");
-        });
-
-
-        // Assert changed user available in Keycloak
-        testingClient.server().run(session -> {
-            LDAPTestContext ctx = LDAPTestContext.init(session);
-            RealmModel appRealm = ctx.getRealm();
-
-            // Assert user is successfully imported in Keycloak DB now with correct firstName and lastName
-            LDAPTestAsserts.assertUserImported(session.users(), appRealm, "fullname", "James2", "Dee2", "fullname@email.org", "4578");
-
-            // Remove "fullnameUser" to assert he is removed from LDAP. Revert mappers to previous state
-            UserModel fullnameUser = session.users().getUserByUsername("fullname", appRealm);
-            session.users().removeUser(appRealm, fullnameUser);
-
-            // Revert mappers
-            ComponentModel fullNameMapperModel = LDAPTestUtils.getSubcomponentByName(appRealm, ctx.getLdapModel(), "full name");
-            appRealm.removeComponent(fullNameMapperModel);
-        });
-
-        firstNameMapperRep.setId(null);
-        Response response = testRealm().components().add(firstNameMapperRep);
-        Assert.assertEquals(201, response.getStatus());
-        response.close();
-    }
-
-
     @Test
     public void testHardcodedAttributeMapperTest() throws Exception {
         // Create hardcoded mapper for "description"