Manual execution of Jakarta validation (#42388)

Signed-off-by: Martin Bartoš <mabartos@redhat.com>

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 {
+}

pom.xml

@@ -591,11 +591,6 @@
                 <artifactId>hibernate-validator</artifactId>
                 <version>${hibernate-validator.version}</version>
             </dependency>
-            <dependency>
-                <groupId>org.hibernate.validator</groupId>
-                <artifactId>hibernate-validator-cdi</artifactId>
-                <version>${hibernate-validator.version}</version>
-            </dependency>
             <dependency>
                 <groupId>org.glassfish.expressly</groupId>
                 <artifactId>expressly</artifactId>

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}")

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<ClientRepresentation> getClients() {
         return clientService.getClients(realm, null, null, null);
     }
 
     @Override
-    public ClientRepresentation createClient(@Valid @ConvertGroup(to = CreateClient.class) ClientRepresentation client,
+    public ClientRepresentation createClient(@Valid ClientRepresentation client, FieldValidation fieldValidation) {
-                                             FieldValidation fieldValidation) {
         try {
+            validator.validate(client, CreateClientDefault.class);
             response.setStatus(Response.Status.CREATED.getStatusCode());
             return clientService.createOrUpdate(realm, client, false).representation();
         } catch (ServiceException e) {

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 {
 
+    <T> void validate(T object, Class<?>... groups) throws ConstraintViolationException;
+
+    void validate(Function<Validator, Set<ConstraintViolation<?>>> validation) throws ConstraintViolationException;
+
     Validator getValidator();
 }

services/pom.xml

@@ -88,7 +88,7 @@
         </dependency>
         <dependency>
             <groupId>org.hibernate.validator</groupId>
-            <artifactId>hibernate-validator-cdi</artifactId>
+            <artifactId>hibernate-validator</artifactId>
             <version>${hibernate-validator.version}</version> <!--Not sure why we need to set it as it should be part of dependencyManagement-->
         </dependency>
         <dependency>

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;
         }

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<ConstraintViolationException> {
+
+    @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<String> violations) {
+    }
+}

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 <T> 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<Validator, Set<ConstraintViolation<?>>> validation) throws ConstraintViolationException {
+        var errors = validation.apply(getValidator());
+        if (!errors.isEmpty()) {
+            throw new ConstraintViolationException(errors);
+        }
+    }
+
     @Override
     public Validator getValidator() {
         return validator;

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"));
+        }
+    }
 }