libre.sh/keycloak-scim
3
0
Fork 0
Code Issues 15 Pull requests Releases 1 Packages Activity Actions

KEYCLOAK-18332 Client Scopes are reset to realm's default when Client is updated

Browse source
This commit is contained in:
Václav Muzikář 2021-06-10 16:30:54 +02:00 committed by Hynek Mlnařík
parent 4d1576b96a
commit 9854f21ace
3 changed files with 73 additions and 16 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
model/jpa/src/main/java/org/keycloak/models/jpa/ClientAdapter.java
View file

@ -261,8 +261,10 @@ public class ClientAdapter implements ClientModel, JpaModel<ClientEntity> {
@Override @Override
public void setProtocol(String protocol) { public void setProtocol(String protocol) {
entity.setProtocol(protocol); if (!Objects.equals(entity.getProtocol(), protocol)) {
session.getKeycloakSessionFactory().publish((ClientModel.ClientProtocolUpdatedEvent) () -> ClientAdapter.this); entity.setProtocol(protocol);
session.getKeycloakSessionFactory().publish((ClientModel.ClientProtocolUpdatedEvent) () -> ClientAdapter.this);
}
} }
@Override @Override

6
model/map/src/main/java/org/keycloak/models/map/client/MapClientAdapter.java
View file

@ -231,8 +231,10 @@ public abstract class MapClientAdapter<K> extends AbstractClientModel<MapClientE
@Override @Override
public void setProtocol(String protocol) { public void setProtocol(String protocol) {
entity.setProtocol(protocol); if (!Objects.equals(entity.getProtocol(), protocol)) {
session.getKeycloakSessionFactory().publish((ClientModel.ClientProtocolUpdatedEvent) () -> MapClientAdapter.this); entity.setProtocol(protocol);
session.getKeycloakSessionFactory().publish((ClientModel.ClientProtocolUpdatedEvent) () -> MapClientAdapter.this);
}
} }
@Override @Override

77
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/client/ClientScopeTest.java
View file

@ -19,6 +19,7 @@ package org.keycloak.testsuite.admin.client;
import org.junit.Assert; import org.junit.Assert;
import org.junit.Test; import org.junit.Test;
import org.keycloak.admin.client.resource.ClientResource;
import org.keycloak.admin.client.resource.ClientScopesResource; import org.keycloak.admin.client.resource.ClientScopesResource;
import org.keycloak.admin.client.resource.ProtocolMappersResource; import org.keycloak.admin.client.resource.ProtocolMappersResource;
import org.keycloak.admin.client.resource.RoleMappingResource; import org.keycloak.admin.client.resource.RoleMappingResource;
@ -54,7 +55,9 @@ import java.util.UUID;
import java.util.stream.Collectors; import java.util.stream.Collectors;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertThat; import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
/** /**
* @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a> * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
@ -103,7 +106,7 @@ public class ClientScopeTest extends AbstractClientTest {
String scope1Id = createClientScope(scopeRep); String scope1Id = createClientScope(scopeRep);
List<ClientScopeRepresentation> clientScopes = clientScopes().findAll(); List<ClientScopeRepresentation> clientScopes = clientScopes().findAll();
Assert.assertTrue(getClientScopeNames(clientScopes).contains("scope1")); assertTrue(getClientScopeNames(clientScopes).contains("scope1"));
// Create scope2 // Create scope2
scopeRep = new ClientScopeRepresentation(); scopeRep = new ClientScopeRepresentation();
@ -111,14 +114,14 @@ public class ClientScopeTest extends AbstractClientTest {
String scope2Id = createClientScope(scopeRep); String scope2Id = createClientScope(scopeRep);
clientScopes = clientScopes().findAll(); clientScopes = clientScopes().findAll();
Assert.assertTrue(getClientScopeNames(clientScopes).contains("scope2")); assertTrue(getClientScopeNames(clientScopes).contains("scope2"));
// Remove scope1 // Remove scope1
removeClientScope(scope1Id); removeClientScope(scope1Id);
clientScopes = clientScopes().findAll(); clientScopes = clientScopes().findAll();
Assert.assertFalse(getClientScopeNames(clientScopes).contains("scope1")); Assert.assertFalse(getClientScopeNames(clientScopes).contains("scope1"));
Assert.assertTrue(getClientScopeNames(clientScopes).contains("scope2")); assertTrue(getClientScopeNames(clientScopes).contains("scope2"));
// Remove scope2 // Remove scope2
@ -149,7 +152,7 @@ public class ClientScopeTest extends AbstractClientTest {
Assert.assertEquals("scope1", scopeRep.getName()); Assert.assertEquals("scope1", scopeRep.getName());
Assert.assertEquals("scope1-desc", scopeRep.getDescription()); Assert.assertEquals("scope1-desc", scopeRep.getDescription());
Assert.assertEquals("someAttrValue", scopeRep.getAttributes().get("someAttr")); 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()); Assert.assertEquals(OIDCLoginProtocol.LOGIN_PROTOCOL, scopeRep.getProtocol());
@ -269,7 +272,7 @@ public class ClientScopeTest extends AbstractClientTest {
} }
private void assertRolesPresent(List<RoleRepresentation> roles, String... expectedRoleNames) { private void assertRolesPresent(List<RoleRepresentation> roles, String... expectedRoleNames) {
List<String> expectedList = Arrays.asList(expectedRoleNames); String[] expectedList = expectedRoleNames;
Set<String> presentRoles = new HashSet<>(); Set<String> presentRoles = new HashSet<>();
for (RoleRepresentation roleRep : roles) { for (RoleRepresentation roleRep : roles) {
@ -379,10 +382,10 @@ public class ClientScopeTest extends AbstractClientTest {
// Ensure defaults and optional scopes are here // Ensure defaults and optional scopes are here
List<String> realmDefaultScopes = getClientScopeNames(testRealmResource().getDefaultDefaultClientScopes()); List<String> realmDefaultScopes = getClientScopeNames(testRealmResource().getDefaultDefaultClientScopes());
List<String> realmOptionalScopes = getClientScopeNames(testRealmResource().getDefaultOptionalClientScopes()); List<String> realmOptionalScopes = getClientScopeNames(testRealmResource().getDefaultOptionalClientScopes());
Assert.assertTrue(realmDefaultScopes.contains("scope-def")); assertTrue(realmDefaultScopes.contains("scope-def"));
Assert.assertFalse(realmOptionalScopes .contains("scope-def")); Assert.assertFalse(realmOptionalScopes .contains("scope-def"));
Assert.assertFalse(realmDefaultScopes.contains("scope-opt")); 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 // create client. Ensure that it has scope-def and scope-opt scopes assigned
ClientRepresentation clientRep = new ClientRepresentation(); ClientRepresentation clientRep = new ClientRepresentation();
@ -393,10 +396,10 @@ public class ClientScopeTest extends AbstractClientTest {
List<String> clientDefaultScopes = getClientScopeNames(testRealmResource().clients().get(clientUuid).getDefaultClientScopes()); List<String> clientDefaultScopes = getClientScopeNames(testRealmResource().clients().get(clientUuid).getDefaultClientScopes());
List<String> clientOptionalScopes = getClientScopeNames(testRealmResource().clients().get(clientUuid).getOptionalClientScopes()); List<String> 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(clientOptionalScopes .contains("scope-def"));
Assert.assertFalse(clientDefaultScopes.contains("scope-opt")); 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 // Unassign scope-def and scope-opt from realm
testRealmResource().removeDefaultDefaultClientScope(scopeDefId); testRealmResource().removeDefaultDefaultClientScope(scopeDefId);
@ -442,7 +445,7 @@ public class ClientScopeTest extends AbstractClientTest {
// Ensure that scope is optional // Ensure that scope is optional
List<String> realmOptionalScopes = getClientScopeNames(testRealmResource().getDefaultOptionalClientScopes()); List<String> realmOptionalScopes = getClientScopeNames(testRealmResource().getDefaultOptionalClientScopes());
Assert.assertTrue(realmOptionalScopes.contains("optional-client-scope")); assertTrue(realmOptionalScopes.contains("optional-client-scope"));
// Create client // Create client
ClientRepresentation client = new ClientRepresentation(); 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 // Ensure that default optional client scope is a default scope of the client
List<String> clientDefaultScopes = getClientScopeNames(testRealmResource().clients().get(clientUuid).getDefaultClientScopes()); List<String> 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! // Ensure that no optional scopes are assigned to the client, even if there are default optional scopes!
List<String> clientOptionalScopes = getClientScopeNames(testRealmResource().clients().get(clientUuid).getOptionalClientScopes()); List<String> clientOptionalScopes = getClientScopeNames(testRealmResource().clients().get(clientUuid).getOptionalClientScopes());
Assert.assertTrue(clientOptionalScopes.isEmpty()); assertTrue(clientOptionalScopes.isEmpty());
// Unassign optional client scope from realm for cleanup // Unassign optional client scope from realm for cleanup
testRealmResource().removeDefaultOptionalClientScope(optionalClientScopeId); testRealmResource().removeDefaultOptionalClientScope(optionalClientScopeId);
assertAdminEvents.assertEvent(getRealmId(), OperationType.DELETE, AdminEventPaths.defaultOptionalClientScopePath(optionalClientScopeId), ResourceType.CLIENT_SCOPE); 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<String> expectedDefScopes = getClientScopeNames(client.getDefaultClientScopes());
List<String> 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 // KEYCLOAK-5863
@Test @Test
public void testUpdateProtocolMappers() { public void testUpdateProtocolMappers() {