From e41a9616283cbe1422e361273c9ce5a5983829be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Barto=C5=A1?= Date: Mon, 15 Sep 2025 10:20:50 +0200 Subject: [PATCH] Manual execution of Jakarta validation (#42388) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Martin Bartoš --- .../v2/validation/CreateClientDefault.java | 9 ++++ pom.xml | 5 -- .../keycloak/admin/api/client/ClientsApi.java | 2 +- .../admin/api/client/DefaultClientsApi.java | 13 ++--- .../jakarta/JakartaValidatorProvider.java | 9 ++++ services/pom.xml | 2 +- .../services/client/DefaultClientService.java | 9 ++-- .../error/ValidationExceptionHandler.java | 29 +++++++++++ .../jakarta/HibernateValidatorProvider.java | 21 ++++++++ .../org/keycloak/tests/admin/AdminV2Test.java | 50 +++++++++++++++++++ 10 files changed, 131 insertions(+), 18 deletions(-) create mode 100644 core/src/main/java/org/keycloak/representations/admin/v2/validation/CreateClientDefault.java create mode 100644 services/src/main/java/org/keycloak/services/error/ValidationExceptionHandler.java diff --git a/core/src/main/java/org/keycloak/representations/admin/v2/validation/CreateClientDefault.java b/core/src/main/java/org/keycloak/representations/admin/v2/validation/CreateClientDefault.java new file mode 100644 index 00000000000..bf4a4ff18de --- /dev/null +++ b/core/src/main/java/org/keycloak/representations/admin/v2/validation/CreateClientDefault.java @@ -0,0 +1,9 @@ +package org.keycloak.representations.admin.v2.validation; + +import jakarta.validation.GroupSequence; +import jakarta.validation.groups.Default; + +@GroupSequence({CreateClient.class, Default.class}) +// Jakarta Validation Group - validation is done only when creating a client + default group included +public interface CreateClientDefault { +} diff --git a/pom.xml b/pom.xml index 1b089efdb64..6811fe397bb 100644 --- a/pom.xml +++ b/pom.xml @@ -591,11 +591,6 @@ hibernate-validator ${hibernate-validator.version} - - org.hibernate.validator - hibernate-validator-cdi - ${hibernate-validator.version} - org.glassfish.expressly expressly diff --git a/rest/admin-api/src/main/java/org/keycloak/admin/api/client/ClientsApi.java b/rest/admin-api/src/main/java/org/keycloak/admin/api/client/ClientsApi.java index ad88273b983..70b8525bee0 100644 --- a/rest/admin-api/src/main/java/org/keycloak/admin/api/client/ClientsApi.java +++ b/rest/admin-api/src/main/java/org/keycloak/admin/api/client/ClientsApi.java @@ -36,7 +36,7 @@ public interface ClientsApi extends Provider { @Consumes(MediaType.APPLICATION_JSON) @Produces(MediaType.APPLICATION_JSON) @Operation(summary = "Create a new client", description = "Creates a new client in the realm") - ClientRepresentation createClient(@Valid @ConvertGroup(to = CreateClient.class) ClientRepresentation client, + ClientRepresentation createClient(@Valid ClientRepresentation client, @QueryParam("fieldValidation") FieldValidation fieldValidation); @Path("{id}") diff --git a/rest/admin-api/src/main/java/org/keycloak/admin/api/client/DefaultClientsApi.java b/rest/admin-api/src/main/java/org/keycloak/admin/api/client/DefaultClientsApi.java index 5eeef86fca7..bf6ff4b5c37 100644 --- a/rest/admin-api/src/main/java/org/keycloak/admin/api/client/DefaultClientsApi.java +++ b/rest/admin-api/src/main/java/org/keycloak/admin/api/client/DefaultClientsApi.java @@ -5,45 +5,46 @@ import java.util.Optional; import java.util.stream.Stream; import jakarta.validation.Valid; -import jakarta.validation.groups.ConvertGroup; import jakarta.ws.rs.NotFoundException; +import jakarta.ws.rs.QueryParam; import org.keycloak.admin.api.FieldValidation; import org.keycloak.http.HttpResponse; import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.representations.admin.v2.ClientRepresentation; -import org.keycloak.representations.admin.v2.validation.CreateClient; +import org.keycloak.representations.admin.v2.validation.CreateClientDefault; import org.keycloak.services.ServiceException; import org.keycloak.services.client.ClientService; -import jakarta.ws.rs.GET; import jakarta.ws.rs.PathParam; import jakarta.ws.rs.WebApplicationException; import jakarta.ws.rs.core.Response; +import org.keycloak.validation.jakarta.JakartaValidatorProvider; public class DefaultClientsApi implements ClientsApi { private final KeycloakSession session; private final RealmModel realm; private final HttpResponse response; private final ClientService clientService; + private final JakartaValidatorProvider validator; public DefaultClientsApi(KeycloakSession session) { this.session = session; this.realm = Objects.requireNonNull(session.getContext().getRealm()); this.clientService = session.services().clients(); this.response = session.getContext().getHttpResponse(); + this.validator = session.getProvider(JakartaValidatorProvider.class); } @Override - @GET public Stream getClients() { return clientService.getClients(realm, null, null, null); } @Override - public ClientRepresentation createClient(@Valid @ConvertGroup(to = CreateClient.class) ClientRepresentation client, - FieldValidation fieldValidation) { + public ClientRepresentation createClient(@Valid ClientRepresentation client, FieldValidation fieldValidation) { try { + validator.validate(client, CreateClientDefault.class); response.setStatus(Response.Status.CREATED.getStatusCode()); return clientService.createOrUpdate(realm, client, false).representation(); } catch (ServiceException e) { diff --git a/server-spi-private/src/main/java/org/keycloak/validation/jakarta/JakartaValidatorProvider.java b/server-spi-private/src/main/java/org/keycloak/validation/jakarta/JakartaValidatorProvider.java index 792b9b2d463..2a7a4b3f032 100644 --- a/server-spi-private/src/main/java/org/keycloak/validation/jakarta/JakartaValidatorProvider.java +++ b/server-spi-private/src/main/java/org/keycloak/validation/jakarta/JakartaValidatorProvider.java @@ -1,9 +1,18 @@ package org.keycloak.validation.jakarta; +import jakarta.validation.ConstraintViolation; +import jakarta.validation.ConstraintViolationException; import jakarta.validation.Validator; import org.keycloak.provider.Provider; +import java.util.Set; +import java.util.function.Function; + public interface JakartaValidatorProvider extends Provider { + void validate(T object, Class... groups) throws ConstraintViolationException; + + void validate(Function>> validation) throws ConstraintViolationException; + Validator getValidator(); } diff --git a/services/pom.xml b/services/pom.xml index 1ab53e8a63f..400526d1f91 100755 --- a/services/pom.xml +++ b/services/pom.xml @@ -88,7 +88,7 @@ org.hibernate.validator - hibernate-validator-cdi + hibernate-validator ${hibernate-validator.version} diff --git a/services/src/main/java/org/keycloak/services/client/DefaultClientService.java b/services/src/main/java/org/keycloak/services/client/DefaultClientService.java index cd3c729d052..ac9d6001e21 100644 --- a/services/src/main/java/org/keycloak/services/client/DefaultClientService.java +++ b/services/src/main/java/org/keycloak/services/client/DefaultClientService.java @@ -1,6 +1,5 @@ package org.keycloak.services.client; -import jakarta.validation.Validator; import jakarta.ws.rs.core.Response; import org.keycloak.models.ClientModel; import org.keycloak.models.KeycloakSession; @@ -8,7 +7,7 @@ import org.keycloak.models.RealmModel; import org.keycloak.models.mapper.ClientModelMapper; import org.keycloak.models.mapper.ModelMapper; import org.keycloak.representations.admin.v2.ClientRepresentation; -import org.keycloak.representations.admin.v2.validation.CreateClient; +import org.keycloak.representations.admin.v2.validation.CreateClientDefault; import org.keycloak.services.ServiceException; import org.keycloak.validation.jakarta.JakartaValidatorProvider; @@ -19,12 +18,12 @@ import java.util.stream.Stream; public class DefaultClientService implements ClientService { private final KeycloakSession session; private final ClientModelMapper mapper; - private final Validator validator; + private final JakartaValidatorProvider validator; public DefaultClientService(KeycloakSession session) { this.session = session; this.mapper = session.getProvider(ModelMapper.class).clients(); - this.validator = session.getProvider(JakartaValidatorProvider.class).getValidator(); + this.validator = session.getProvider(JakartaValidatorProvider.class); } @Override @@ -49,7 +48,7 @@ public class DefaultClientService implements ClientService { throw new ServiceException("Client already exists", Response.Status.CONFLICT); } } else { - validator.validate(client, CreateClient.class); // TODO improve it to avoid second validation when we know it is create and not update + validator.validate(client, CreateClientDefault.class); // TODO improve it to avoid second validation when we know it is create and not update model = realm.addClient(client.getClientId()); created = true; } diff --git a/services/src/main/java/org/keycloak/services/error/ValidationExceptionHandler.java b/services/src/main/java/org/keycloak/services/error/ValidationExceptionHandler.java new file mode 100644 index 00000000000..45b30ee6a0e --- /dev/null +++ b/services/src/main/java/org/keycloak/services/error/ValidationExceptionHandler.java @@ -0,0 +1,29 @@ +package org.keycloak.services.error; + +import jakarta.validation.ConstraintViolationException; +import jakarta.ws.rs.core.MediaType; +import jakarta.ws.rs.core.Response; +import jakarta.ws.rs.ext.ExceptionMapper; +import jakarta.ws.rs.ext.Provider; + +import java.util.Set; +import java.util.stream.Collectors; + +@Provider +public class ValidationExceptionHandler implements ExceptionMapper { + + @Override + public Response toResponse(ConstraintViolationException exception) { + return Response.status(400) + .entity(new ViolationExceptionResponse("Provided data is invalid", + exception.getConstraintViolations() + .stream() + .map(f -> "%s: %s".formatted(f.getPropertyPath(), f.getMessage())) + .collect(Collectors.toSet()))) + .type(MediaType.APPLICATION_JSON_TYPE) + .build(); + } + + public record ViolationExceptionResponse(String error, Set violations) { + } +} diff --git a/services/src/main/java/org/keycloak/validation/jakarta/HibernateValidatorProvider.java b/services/src/main/java/org/keycloak/validation/jakarta/HibernateValidatorProvider.java index e7150d0cb6c..027ab764a16 100644 --- a/services/src/main/java/org/keycloak/validation/jakarta/HibernateValidatorProvider.java +++ b/services/src/main/java/org/keycloak/validation/jakarta/HibernateValidatorProvider.java @@ -1,7 +1,12 @@ package org.keycloak.validation.jakarta; +import jakarta.validation.ConstraintViolation; +import jakarta.validation.ConstraintViolationException; import jakarta.validation.Validator; +import java.util.Set; +import java.util.function.Function; + public class HibernateValidatorProvider implements JakartaValidatorProvider { private final Validator validator; @@ -9,6 +14,22 @@ public class HibernateValidatorProvider implements JakartaValidatorProvider { this.validator = validator; } + @Override + public void validate(T object, Class... groups) throws ConstraintViolationException { + var errors = validator.validate(object, groups); + if (!errors.isEmpty()) { + throw new ConstraintViolationException(errors); + } + } + + @Override + public void validate(Function>> validation) throws ConstraintViolationException { + var errors = validation.apply(getValidator()); + if (!errors.isEmpty()) { + throw new ConstraintViolationException(errors); + } + } + @Override public Validator getValidator() { return validator; diff --git a/tests/base/src/test/java/org/keycloak/tests/admin/AdminV2Test.java b/tests/base/src/test/java/org/keycloak/tests/admin/AdminV2Test.java index 92852e4a1a8..ce8cc25eb37 100644 --- a/tests/base/src/test/java/org/keycloak/tests/admin/AdminV2Test.java +++ b/tests/base/src/test/java/org/keycloak/tests/admin/AdminV2Test.java @@ -22,6 +22,7 @@ import jakarta.ws.rs.core.HttpHeaders; import jakarta.ws.rs.core.MediaType; import org.apache.http.client.methods.HttpGet; import org.apache.http.client.methods.HttpPatch; +import org.apache.http.client.methods.HttpPost; import org.apache.http.entity.StringEntity; import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.util.EntityUtils; @@ -29,9 +30,13 @@ import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import org.keycloak.admin.api.client.ClientApi; import org.keycloak.representations.admin.v2.ClientRepresentation; +import org.keycloak.services.error.ValidationExceptionHandler; import org.keycloak.testframework.annotations.InjectHttpClient; import org.keycloak.testframework.annotations.KeycloakIntegrationTest; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.Matchers.notNullValue; import static org.junit.jupiter.api.Assertions.assertEquals; @KeycloakIntegrationTest() @@ -98,4 +103,49 @@ public class AdminV2Test { assertEquals("I'm also a description", client.getDescription()); } } + + @Test + public void clientRepresentationValidation() throws Exception { + HttpPost request = new HttpPost(HOSTNAME_LOCAL_ADMIN + "/realms/master/clients"); + request.setHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON); + + request.setEntity(new StringEntity(""" + { + "displayName": "something", + "appUrl": "notUrl" + } + """)); + + try (var response = client.execute(request)) { + assertThat(response, notNullValue()); + assertThat(response.getStatusLine().getStatusCode(), is(400)); + + var body = mapper.createParser(response.getEntity().getContent()).readValueAs(ValidationExceptionHandler.ViolationExceptionResponse.class); + assertThat(body.error(), is("Provided data is invalid")); + var violations = body.violations(); + assertThat(violations.size(), is(1)); + assertThat(violations.iterator().next(), is("clientId: must not be blank")); + } + + request.setEntity(new StringEntity(""" + { + "clientId": "some-client", + "displayName": "something", + "appUrl": "notUrl", + "auth": { + "method":"missing-enabled" + } + } + """)); + + try (var response = client.execute(request)) { + assertThat(response, notNullValue()); + assertThat(response.getStatusLine().getStatusCode(), is(400)); + var body = mapper.createParser(response.getEntity().getContent()).readValueAs(ValidationExceptionHandler.ViolationExceptionResponse.class); + assertThat(body.error(), is("Provided data is invalid")); + var violations = body.violations(); + assertThat(violations.size(), is(1)); + assertThat(violations.iterator().next(), is("appUrl: must be a valid URL")); + } + } }