diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/ClientAdapter.java b/model/jpa/src/main/java/org/keycloak/models/jpa/ClientAdapter.java index cb635c1516..389904341a 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/ClientAdapter.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/ClientAdapter.java @@ -261,8 +261,10 @@ public class ClientAdapter implements ClientModel, JpaModel { @Override public void setProtocol(String protocol) { - entity.setProtocol(protocol); - session.getKeycloakSessionFactory().publish((ClientModel.ClientProtocolUpdatedEvent) () -> ClientAdapter.this); + if (!Objects.equals(entity.getProtocol(), protocol)) { + entity.setProtocol(protocol); + session.getKeycloakSessionFactory().publish((ClientModel.ClientProtocolUpdatedEvent) () -> ClientAdapter.this); + } } @Override diff --git a/model/map/src/main/java/org/keycloak/models/map/client/MapClientAdapter.java b/model/map/src/main/java/org/keycloak/models/map/client/MapClientAdapter.java index 253763b81d..44768a72b9 100644 --- a/model/map/src/main/java/org/keycloak/models/map/client/MapClientAdapter.java +++ b/model/map/src/main/java/org/keycloak/models/map/client/MapClientAdapter.java @@ -231,8 +231,10 @@ public abstract class MapClientAdapter extends AbstractClientModel MapClientAdapter.this); + if (!Objects.equals(entity.getProtocol(), protocol)) { + entity.setProtocol(protocol); + session.getKeycloakSessionFactory().publish((ClientModel.ClientProtocolUpdatedEvent) () -> MapClientAdapter.this); + } } @Override diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/ClientScopeTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/ClientScopeTest.java index a029834c61..cd27d053c1 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/ClientScopeTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/ClientScopeTest.java @@ -19,6 +19,7 @@ package org.keycloak.testsuite.admin.client; import org.junit.Assert; import org.junit.Test; +import org.keycloak.admin.client.resource.ClientResource; import org.keycloak.admin.client.resource.ClientScopesResource; import org.keycloak.admin.client.resource.ProtocolMappersResource; import org.keycloak.admin.client.resource.RoleMappingResource; @@ -54,7 +55,9 @@ import java.util.UUID; import java.util.stream.Collectors; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; /** * @author Marek Posolda @@ -103,7 +106,7 @@ public class ClientScopeTest extends AbstractClientTest { String scope1Id = createClientScope(scopeRep); List clientScopes = clientScopes().findAll(); - Assert.assertTrue(getClientScopeNames(clientScopes).contains("scope1")); + assertTrue(getClientScopeNames(clientScopes).contains("scope1")); // Create scope2 scopeRep = new ClientScopeRepresentation(); @@ -111,14 +114,14 @@ public class ClientScopeTest extends AbstractClientTest { String scope2Id = createClientScope(scopeRep); clientScopes = clientScopes().findAll(); - Assert.assertTrue(getClientScopeNames(clientScopes).contains("scope2")); + assertTrue(getClientScopeNames(clientScopes).contains("scope2")); // Remove scope1 removeClientScope(scope1Id); clientScopes = clientScopes().findAll(); Assert.assertFalse(getClientScopeNames(clientScopes).contains("scope1")); - Assert.assertTrue(getClientScopeNames(clientScopes).contains("scope2")); + assertTrue(getClientScopeNames(clientScopes).contains("scope2")); // Remove scope2 @@ -149,7 +152,7 @@ public class ClientScopeTest extends AbstractClientTest { Assert.assertEquals("scope1", scopeRep.getName()); Assert.assertEquals("scope1-desc", scopeRep.getDescription()); Assert.assertEquals("someAttrValue", scopeRep.getAttributes().get("someAttr")); - Assert.assertTrue(ObjectUtil.isBlank(scopeRep.getAttributes().get("emptyAttr"))); + assertTrue(ObjectUtil.isBlank(scopeRep.getAttributes().get("emptyAttr"))); Assert.assertEquals(OIDCLoginProtocol.LOGIN_PROTOCOL, scopeRep.getProtocol()); @@ -269,7 +272,7 @@ public class ClientScopeTest extends AbstractClientTest { } private void assertRolesPresent(List roles, String... expectedRoleNames) { - List expectedList = Arrays.asList(expectedRoleNames); + String[] expectedList = expectedRoleNames; Set presentRoles = new HashSet<>(); for (RoleRepresentation roleRep : roles) { @@ -379,10 +382,10 @@ public class ClientScopeTest extends AbstractClientTest { // Ensure defaults and optional scopes are here List realmDefaultScopes = getClientScopeNames(testRealmResource().getDefaultDefaultClientScopes()); List realmOptionalScopes = getClientScopeNames(testRealmResource().getDefaultOptionalClientScopes()); - Assert.assertTrue(realmDefaultScopes.contains("scope-def")); + assertTrue(realmDefaultScopes.contains("scope-def")); Assert.assertFalse(realmOptionalScopes .contains("scope-def")); Assert.assertFalse(realmDefaultScopes.contains("scope-opt")); - Assert.assertTrue(realmOptionalScopes .contains("scope-opt")); + assertTrue(realmOptionalScopes .contains("scope-opt")); // create client. Ensure that it has scope-def and scope-opt scopes assigned ClientRepresentation clientRep = new ClientRepresentation(); @@ -393,10 +396,10 @@ public class ClientScopeTest extends AbstractClientTest { List clientDefaultScopes = getClientScopeNames(testRealmResource().clients().get(clientUuid).getDefaultClientScopes()); List clientOptionalScopes = getClientScopeNames(testRealmResource().clients().get(clientUuid).getOptionalClientScopes()); - Assert.assertTrue(clientDefaultScopes.contains("scope-def")); + assertTrue(clientDefaultScopes.contains("scope-def")); Assert.assertFalse(clientOptionalScopes .contains("scope-def")); Assert.assertFalse(clientDefaultScopes.contains("scope-opt")); - Assert.assertTrue(clientOptionalScopes .contains("scope-opt")); + assertTrue(clientOptionalScopes .contains("scope-opt")); // Unassign scope-def and scope-opt from realm testRealmResource().removeDefaultDefaultClientScope(scopeDefId); @@ -442,7 +445,7 @@ public class ClientScopeTest extends AbstractClientTest { // Ensure that scope is optional List realmOptionalScopes = getClientScopeNames(testRealmResource().getDefaultOptionalClientScopes()); - Assert.assertTrue(realmOptionalScopes.contains("optional-client-scope")); + assertTrue(realmOptionalScopes.contains("optional-client-scope")); // Create client ClientRepresentation client = new ClientRepresentation(); @@ -453,17 +456,67 @@ public class ClientScopeTest extends AbstractClientTest { // Ensure that default optional client scope is a default scope of the client List clientDefaultScopes = getClientScopeNames(testRealmResource().clients().get(clientUuid).getDefaultClientScopes()); - Assert.assertTrue(clientDefaultScopes.contains("optional-client-scope")); + assertTrue(clientDefaultScopes.contains("optional-client-scope")); // Ensure that no optional scopes are assigned to the client, even if there are default optional scopes! List clientOptionalScopes = getClientScopeNames(testRealmResource().clients().get(clientUuid).getOptionalClientScopes()); - Assert.assertTrue(clientOptionalScopes.isEmpty()); + assertTrue(clientOptionalScopes.isEmpty()); // Unassign optional client scope from realm for cleanup testRealmResource().removeDefaultOptionalClientScope(optionalClientScopeId); assertAdminEvents.assertEvent(getRealmId(), OperationType.DELETE, AdminEventPaths.defaultOptionalClientScopePath(optionalClientScopeId), ResourceType.CLIENT_SCOPE); } + // KEYCLOAK-18332 + @Test + public void scopesRemainAfterClientUpdate() { + // Create a bunch of scopes + ClientScopeRepresentation scopeRep = new ClientScopeRepresentation(); + scopeRep.setName("scope-def"); + scopeRep.setProtocol("openid-connect"); + String scopeDefId = createClientScope(scopeRep); + getCleanup().addClientScopeId(scopeDefId); + + scopeRep = new ClientScopeRepresentation(); + scopeRep.setName("scope-opt"); + scopeRep.setProtocol("openid-connect"); + String scopeOptId = createClientScope(scopeRep); + getCleanup().addClientScopeId(scopeOptId); + + // Add scope-def as default and scope-opt as optional client scope + testRealmResource().addDefaultDefaultClientScope(scopeDefId); + assertAdminEvents.assertEvent(getRealmId(), OperationType.CREATE, AdminEventPaths.defaultDefaultClientScopePath(scopeDefId), ResourceType.CLIENT_SCOPE); + testRealmResource().addDefaultOptionalClientScope(scopeOptId); + assertAdminEvents.assertEvent(getRealmId(), OperationType.CREATE, AdminEventPaths.defaultOptionalClientScopePath(scopeOptId), ResourceType.CLIENT_SCOPE); + + // Create a client + ClientRepresentation clientRep = new ClientRepresentation(); + clientRep.setClientId("bar-client"); + clientRep.setProtocol("openid-connect"); + String clientUuid = createClient(clientRep); + ClientResource client = testRealmResource().clients().get(clientUuid); + getCleanup().addClientUuid(clientUuid); + assertTrue(getClientScopeNames(client.getDefaultClientScopes()).contains("scope-def")); + assertTrue(getClientScopeNames(client.getOptionalClientScopes()).contains("scope-opt")); + + // Remove the scopes from client + client.removeDefaultClientScope(scopeDefId); + client.removeOptionalClientScope(scopeOptId); + List expectedDefScopes = getClientScopeNames(client.getDefaultClientScopes()); + List expectedOptScopes = getClientScopeNames(client.getOptionalClientScopes()); + assertFalse(expectedDefScopes.contains("scope-def")); + assertFalse(expectedOptScopes.contains("scope-opt")); + + // Update the client + clientRep = client.toRepresentation(); + clientRep.setDescription("desc"); // Make a small change + client.update(clientRep); + + // Assert scopes are intact + assertEquals(expectedDefScopes, getClientScopeNames(client.getDefaultClientScopes())); + assertEquals(expectedOptScopes, getClientScopeNames(client.getOptionalClientScopes())); + } + // KEYCLOAK-5863 @Test public void testUpdateProtocolMappers() {