Integration tests around client type parameter validation. Throw common ClientTypeException with invalid params requested during client creation/update requests. This gets translated into ErrorResponseException in the Resource handlers.

Signed-off-by: Patrick Jennings <pajennin@redhat.com>
This commit is contained in:
Patrick Jennings 2024-04-02 14:48:51 -04:00 committed by Marek Posolda
parent 9814733dd3
commit 03db2e8b56
4 changed files with 33 additions and 23 deletions

View file

@ -18,29 +18,23 @@
package org.keycloak.services.clienttype.impl; package org.keycloak.services.clienttype.impl;
import java.beans.PropertyDescriptor;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import jakarta.ws.rs.core.Response; import jakarta.ws.rs.core.Response;
import org.jboss.logging.Logger; import org.jboss.logging.Logger;
import org.keycloak.common.util.ObjectUtil; import org.keycloak.client.clienttype.ClientType;
import org.keycloak.client.clienttype.ClientTypeException;
import org.keycloak.models.ClientModel; import org.keycloak.models.ClientModel;
import org.keycloak.models.KeycloakSession; import org.keycloak.models.KeycloakSession;
import org.keycloak.models.utils.ModelToRepresentation; import org.keycloak.models.utils.ModelToRepresentation;
import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.idm.ClientRepresentation;
import org.keycloak.representations.idm.ClientTypeRepresentation; import org.keycloak.representations.idm.ClientTypeRepresentation;
import org.keycloak.client.clienttype.ClientType;
import org.keycloak.client.clienttype.ClientTypeException; import java.beans.PropertyDescriptor;
import org.keycloak.representations.idm.ErrorRepresentation; import java.lang.reflect.Method;
import org.keycloak.services.ErrorResponse; import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.stream.Collectors;
/** /**
* @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a> * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
@ -104,17 +98,16 @@ public class DefaultClientType implements ClientType {
validateClientRequest(newClient, currentRep); validateClientRequest(newClient, currentRep);
} }
protected void validateClientRequest(ClientRepresentation newClient, ClientRepresentation currentClient) { protected void validateClientRequest(ClientRepresentation newClient, ClientRepresentation currentClient) throws ClientTypeException {
List<String> validationErrors = clientType.getConfig().entrySet().stream() List<String> validationErrors = clientType.getConfig().entrySet().stream()
.filter(property -> clientPropertyHasInvalidChangeRequested(currentClient, newClient, property.getKey(), property.getValue())) .filter(property -> clientPropertyHasInvalidChangeRequested(currentClient, newClient, property.getKey(), property.getValue()))
.map(Map.Entry::getKey) .map(Map.Entry::getKey)
.collect(Collectors.toList()); .collect(Collectors.toList());
if (validationErrors.size() > 0) { if (validationErrors.size() > 0) {
throw ErrorResponse.error( throw new ClientTypeException(
"Cannot change property of client as it is not allowed by the specified client type.", "Cannot change property of client as it is not allowed by the specified client type.",
validationErrors.toArray(), validationErrors.toArray());
Response.Status.BAD_REQUEST);
} }
} }

View file

@ -186,7 +186,7 @@ public class ClientResource {
} catch (ModelDuplicateException e) { } catch (ModelDuplicateException e) {
throw ErrorResponse.exists("Client already exists"); throw ErrorResponse.exists("Client already exists");
} catch (ClientTypeException cte) { } catch (ClientTypeException cte) {
throw ErrorResponse.error(cte.getMessage(), Response.Status.BAD_REQUEST); throw ErrorResponse.error(cte.getMessage(), cte.getParameters(), Response.Status.BAD_REQUEST);
} catch (ClientPolicyException cpe) { } catch (ClientPolicyException cpe) {
throw new ErrorResponseException(cpe.getError(), cpe.getErrorDetail(), Response.Status.BAD_REQUEST); throw new ErrorResponseException(cpe.getError(), cpe.getErrorDetail(), Response.Status.BAD_REQUEST);
} }

View file

@ -229,7 +229,7 @@ public class ClientsResource {
throw new ErrorResponseException(cpe.getError(), cpe.getErrorDetail(), Response.Status.BAD_REQUEST); throw new ErrorResponseException(cpe.getError(), cpe.getErrorDetail(), Response.Status.BAD_REQUEST);
} }
catch (ClientTypeException cte) { catch (ClientTypeException cte) {
throw ErrorResponse.error(cte.getMessage(), Response.Status.BAD_REQUEST); throw ErrorResponse.error(cte.getMessage(), cte.getParameters(), Response.Status.BAD_REQUEST);
} }
} }

View file

@ -28,6 +28,7 @@ import org.keycloak.protocol.oidc.OIDCLoginProtocol;
import org.keycloak.representations.idm.ClientRepresentation; import org.keycloak.representations.idm.ClientRepresentation;
import org.keycloak.representations.idm.ClientTypeRepresentation; import org.keycloak.representations.idm.ClientTypeRepresentation;
import org.keycloak.representations.idm.ClientTypesRepresentation; import org.keycloak.representations.idm.ClientTypesRepresentation;
import org.keycloak.representations.idm.ErrorRepresentation;
import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.representations.idm.RealmRepresentation;
import org.keycloak.services.clienttype.impl.DefaultClientTypeProviderFactory; import org.keycloak.services.clienttype.impl.DefaultClientTypeProviderFactory;
import org.keycloak.testsuite.AbstractTestRealmKeycloakTest; import org.keycloak.testsuite.AbstractTestRealmKeycloakTest;
@ -44,6 +45,7 @@ import java.util.List;
import java.util.stream.Collectors; import java.util.stream.Collectors;
import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
import static org.junit.Assert.fail; import static org.junit.Assert.fail;
import static org.keycloak.common.Profile.Feature.CLIENT_TYPES; import static org.keycloak.common.Profile.Feature.CLIENT_TYPES;
@ -122,7 +124,6 @@ public class ClientTypesTest extends AbstractTestRealmKeycloakTest {
// Expected // Expected
} }
clientRep.setServiceAccountsEnabled(true); clientRep.setServiceAccountsEnabled(true);
// Adding non-applicable attribute should not fail // Adding non-applicable attribute should not fail
@ -134,6 +135,22 @@ public class ClientTypesTest extends AbstractTestRealmKeycloakTest {
testRealm().clients().get(clientRep.getId()).update(clientRep); testRealm().clients().get(clientRep.getId()).update(clientRep);
} }
@Test
public void testCreateClientFailsWithMultipleInvalidClientTypeOverrides() {
ClientRepresentation clientRep = ClientBuilder.create()
.clientId("invalid-client-type-fields-set")
.type(ClientTypeManager.SERVICE_ACCOUNT)
.serviceAccountsEnabled(false)
.publicClient()
.build();
Response response = testRealm().clients().create(clientRep);
assertEquals(Response.Status.BAD_REQUEST, response.getStatusInfo());
ErrorRepresentation errorRepresentation = response.readEntity(ErrorRepresentation.class);
assertThat(
List.of(errorRepresentation.getParams()),
containsInAnyOrder("publicClient", "serviceAccountsEnabled"));
}
@Test @Test
public void testClientTypesAdminRestAPI_globalTypes() { public void testClientTypesAdminRestAPI_globalTypes() {