[KEYCLOAK-3723] Fixed updated of protocol mappers within client updates in clients-registrations resource

server-spi-private/src/main/java/org/keycloak/models/utils/RepresentationToModel.java

@@ -1336,7 +1336,6 @@ public class RepresentationToModel {
             }
         }
 
-
         if (rep.getNotBefore() != null) {
             resource.setNotBefore(rep.getNotBefore());
         }
@@ -1365,6 +1364,39 @@ public class RepresentationToModel {
         resource.updateClient();
     }
 
+    public static void updateClientProtocolMappers(ClientRepresentation rep, ClientModel resource) {
+
+        if (rep.getProtocolMappers() != null) {
+            Map<String,ProtocolMapperModel> existingProtocolMappers = new HashMap<>();
+            for (ProtocolMapperModel existingProtocolMapper : resource.getProtocolMappers()) {
+                existingProtocolMappers.put(generateProtocolNameKey(existingProtocolMapper.getProtocol(), existingProtocolMapper.getName()), existingProtocolMapper);
+            }
+
+            for (ProtocolMapperRepresentation protocolMapperRepresentation : rep.getProtocolMappers()) {
+                String protocolNameKey = generateProtocolNameKey(protocolMapperRepresentation.getProtocol(), protocolMapperRepresentation.getName());
+                ProtocolMapperModel existingMapper = existingProtocolMappers.get(protocolNameKey);
+                    if (existingMapper != null) {
+                        ProtocolMapperModel updatedProtocolMapperModel = toModel(protocolMapperRepresentation);
+                        updatedProtocolMapperModel.setId(existingMapper.getId());
+                        resource.updateProtocolMapper(updatedProtocolMapperModel);
+
+                        existingProtocolMappers.remove(protocolNameKey);
+
+                } else {
+                    resource.addProtocolMapper(toModel(protocolMapperRepresentation));
+                }
+            }
+
+            for (Map.Entry<String, ProtocolMapperModel> entryToDelete : existingProtocolMappers.entrySet()) {
+                resource.removeProtocolMapper(entryToDelete.getValue());
+            }
+        }
+    }
+
+    private static String generateProtocolNameKey(String protocol, String name) {
+        return String.format("%s%%%s", protocol, name);
+    }
+
     // CLIENT SCOPES
 
     private static Map<String, ClientScopeModel> createClientScopes(KeycloakSession session, List<ClientScopeRepresentation> clientScopes, RealmModel realm) {

services/src/main/java/org/keycloak/services/clientregistration/AbstractClientRegistrationProvider.java

@@ -19,11 +19,7 @@ package org.keycloak.services.clientregistration;
 
 import org.keycloak.events.EventBuilder;
 import org.keycloak.events.EventType;
-import org.keycloak.models.ClientInitialAccessModel;
+import org.keycloak.models.*;
-import org.keycloak.models.ClientModel;
-import org.keycloak.models.KeycloakSession;
-import org.keycloak.models.ModelDuplicateException;
-import org.keycloak.models.RealmModel;
 import org.keycloak.models.utils.ModelToRepresentation;
 import org.keycloak.models.utils.RepresentationToModel;
 import org.keycloak.representations.idm.ClientRepresentation;
@@ -142,6 +138,8 @@ public abstract class AbstractClientRegistrationProvider implements ClientRegist
         }
 
         RepresentationToModel.updateClient(rep, client);
+        RepresentationToModel.updateClientProtocolMappers(rep, client);
+
         rep = ModelToRepresentation.toRepresentation(client, session);
 
         if (auth.isRegistrationAccessToken()) {

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientRegistrationTest.java

@@ -24,21 +24,19 @@ import org.keycloak.client.registration.Auth;
 import org.keycloak.client.registration.ClientRegistration;
 import org.keycloak.client.registration.ClientRegistrationException;
 import org.keycloak.client.registration.HttpErrorException;
-import org.keycloak.models.ClientModel;
 import org.keycloak.models.Constants;
-import org.keycloak.models.UserModel;
 import org.keycloak.representations.idm.ClientRepresentation;
+import org.keycloak.representations.idm.ProtocolMapperRepresentation;
 import org.keycloak.representations.idm.UserRepresentation;
 import org.keycloak.testsuite.runonserver.RunOnServerDeployment;
-import org.keycloak.testsuite.runonserver.RunOnServerTest;
 
 import javax.ws.rs.NotFoundException;
+import java.util.ArrayList;
 import java.util.Collections;
 
-import static org.junit.Assert.assertEquals;
+import static org.hamcrest.Matchers.nullValue;
-import static org.junit.Assert.assertNotNull;
+import static org.hamcrest.core.Is.is;
-import static org.junit.Assert.assertNull;
+import static org.junit.Assert.*;
-import static org.junit.Assert.fail;
 
 /**
  * @author <a href="mailto:sthorger@redhat.com">Stian Thorgersen</a>
@@ -245,6 +243,68 @@ public class ClientRegistrationTest extends AbstractClientRegistrationTest {
         assertEquals("mysecret", updatedClient.getSecret());
     }
 
+    @Test
+    public void addClientProtcolMappers() throws ClientRegistrationException {
+        authManageClients();
+
+        ClientRepresentation initialClient = buildClient();
+
+        registerClient(initialClient);
+        ClientRepresentation client = reg.get(CLIENT_ID);
+
+        addProtocolMapper(client, "mapperA");
+        reg.update(client);
+
+        ClientRepresentation updatedClient = reg.get(CLIENT_ID);
+        assertThat("Adding protocolMapper failed", updatedClient.getProtocolMappers().size(), is(1));
+    }
+
+    @Test
+    public void removeClientProtcolMappers() throws ClientRegistrationException {
+        authManageClients();
+
+        ClientRepresentation initialClient = buildClient();
+        addProtocolMapper(initialClient, "mapperA");
+        registerClient(initialClient);
+        ClientRepresentation client = reg.get(CLIENT_ID);
+        client.setProtocolMappers(new ArrayList<>());
+        reg.update(client);
+
+        ClientRepresentation updatedClient = reg.get(CLIENT_ID);
+        assertThat("Removing protocolMapper failed", updatedClient.getProtocolMappers(), nullValue());
+    }
+
+    @Test
+    public void updateClientProtcolMappers() throws ClientRegistrationException {
+        authManageClients();
+
+        ClientRepresentation initialClient = buildClient();
+        addProtocolMapper(initialClient, "mapperA");
+        registerClient(initialClient);
+        ClientRepresentation client = reg.get(CLIENT_ID);
+        client.getProtocolMappers().get(0).getConfig().put("claim.name", "updatedClaimName");
+        reg.update(client);
+
+        ClientRepresentation updatedClient = reg.get(CLIENT_ID);
+        assertThat("Updating protocolMapper failed", updatedClient.getProtocolMappers().get(0).getConfig().get("claim.name"), is("updatedClaimName"));
+    }
+
+    private void addProtocolMapper(ClientRepresentation client, String mapperName) {
+        ProtocolMapperRepresentation mapper = new ProtocolMapperRepresentation();
+        mapper.setName(mapperName);
+        mapper.setProtocol("openid-connect");
+        mapper.setProtocolMapper("oidc-usermodel-attribute-mapper");
+        mapper.getConfig().put("userinfo.token.claim", "true");
+        mapper.getConfig().put("user.attribute", "someAttribute");
+        mapper.getConfig().put("id.token.claim", "true");
+        mapper.getConfig().put("access.token.claim", "true");
+        mapper.getConfig().put("claim.name", "someClaimName");
+        mapper.getConfig().put("jsonType.label", "long");
+
+        client.setProtocolMappers(new ArrayList<>());
+        client.getProtocolMappers().add(mapper);
+    }
+
     @Test
     public void updateClientAsAdminWithCreateOnly() throws ClientRegistrationException {
         authCreateClients();