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

Updating validation logic to match our expectations on what applicable should mean.

Browse source 
Signed-off-by: Patrick Jennings <pajennin@redhat.com>
This commit is contained in:
Patrick Jennings 2024-04-05 16:28:00 -04:00 committed by Marek Posolda
parent 03db2e8b56
commit 551a3db987
2 changed files with 28 additions and 8 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

20
services/src/main/java/org/keycloak/services/clienttype/impl/DefaultClientType.java
View file

@ -122,11 +122,21 @@ public class DefaultClientType implements ClientType {
ClientRepresentation newClient,
String propertyName,
ClientTypeRepresentation.PropertyConfig propertyConfig) {
// Validate that read-only client properties were not changed.
return propertyConfig.getApplicable() &&
propertyConfig.getReadOnly() &&
!Objects.isNull(getClientProperty(newClient, propertyName)) &&
!Objects.equals(getClientProperty(oldClient, propertyName), getClientProperty(newClient, propertyName));
Object newClientProperty = getClientProperty(newClient, propertyName);
Object oldClientProperty = getClientProperty(oldClient, propertyName);
return (
// Validate that non-applicable client properties were not changed.
!propertyConfig.getApplicable() &&
!Objects.isNull(newClientProperty) &&
!Objects.equals(oldClientProperty, newClientProperty)
) || (
// Validate that applicable read-only client properties were not changed.
propertyConfig.getApplicable() &&
propertyConfig.getReadOnly() &&
!Objects.isNull(newClientProperty) &&
!Objects.equals(oldClientProperty, newClientProperty)
);
}
private void setClientProperty(ClientRepresentation client,

16
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/client/ClientTypesTest.java
View file

@ -121,13 +121,19 @@ public class ClientTypesTest extends AbstractTestRealmKeycloakTest {
testRealm().clients().get(clientRep.getId()).update(clientRep);
Assert.fail("Not expected to update client");
} catch (BadRequestException bre) {
// Expected
assertErrorResponseContainsParams(bre.getResponse(), "serviceAccountsEnabled");
}
clientRep.setServiceAccountsEnabled(true);
// Adding non-applicable attribute should not fail
clientRep.getAttributes().put(ClientModel.LOGO_URI, "https://foo");
try {
testRealm().clients().get(clientRep.getId()).update(clientRep);
Assert.fail("Not expected to update client");
} catch (BadRequestException bre) {
assertErrorResponseContainsParams(bre.getResponse(), "logoUri");
}
// Update of supported attribute should be successful
clientRep.getAttributes().remove(ClientModel.LOGO_URI);
@ -138,18 +144,22 @@ public class ClientTypesTest extends AbstractTestRealmKeycloakTest {
@Test
public void testCreateClientFailsWithMultipleInvalidClientTypeOverrides() {
ClientRepresentation clientRep = ClientBuilder.create()
.clientId("invalid-client-type-fields-set")
.clientId("service-account-client-type-required-to-be-confidential-and-service-accounts-enabled")
.type(ClientTypeManager.SERVICE_ACCOUNT)
.serviceAccountsEnabled(false)
.publicClient()
.build();
Response response = testRealm().clients().create(clientRep);
assertErrorResponseContainsParams(response, "publicClient", "serviceAccountsEnabled");
}
private void assertErrorResponseContainsParams(Response response, String... items) {
assertEquals(Response.Status.BAD_REQUEST, response.getStatusInfo());
ErrorRepresentation errorRepresentation = response.readEntity(ErrorRepresentation.class);
assertThat(
List.of(errorRepresentation.getParams()),
containsInAnyOrder("publicClient", "serviceAccountsEnabled"));
containsInAnyOrder(items));
}
@Test