From d1d1d6984099919dea168a215dce64a9c415a76c Mon Sep 17 00:00:00 2001 From: Steven Hawkins Date: Mon, 8 Jan 2024 13:19:40 -0500 Subject: [PATCH] fix: adds a general error message and descriptions for some exceptions (#25806) closes: #25746 Signed-off-by: Steve Hawkins --- .../org/keycloak/services/error/KeycloakErrorHandler.java | 7 ++++--- .../admin/ClientAttributeCertificateResource.java | 4 +++- .../services/resources/admin/ComponentResource.java | 4 ++-- .../services/resources/admin/RoleContainerResource.java | 2 +- .../keycloak/testsuite/error/UncaughtErrorPageTest.java | 8 +++++--- 5 files changed, 15 insertions(+), 10 deletions(-) diff --git a/services/src/main/java/org/keycloak/services/error/KeycloakErrorHandler.java b/services/src/main/java/org/keycloak/services/error/KeycloakErrorHandler.java index f9d9de9d10..975f26da3f 100644 --- a/services/src/main/java/org/keycloak/services/error/KeycloakErrorHandler.java +++ b/services/src/main/java/org/keycloak/services/error/KeycloakErrorHandler.java @@ -85,7 +85,8 @@ public class KeycloakErrorHandler implements ExceptionMapper { OAuth2ErrorRepresentation error = new OAuth2ErrorRepresentation(); error.setError(getErrorCode(throwable)); - + error.setErrorDescription("For more on this error consult the server log at the debug level."); + return Response.status(statusCode) .header(HttpHeaders.CONTENT_TYPE, jakarta.ws.rs.core.MediaType.APPLICATION_JSON_TYPE.toString()) .entity(error) @@ -125,11 +126,11 @@ public class KeycloakErrorHandler implements ExceptionMapper { if (throwable instanceof JsonProcessingException) { status = Response.Status.BAD_REQUEST.getStatusCode(); } - + if (throwable instanceof ModelDuplicateException) { status = Response.Status.CONFLICT.getStatusCode(); } - + return status; } diff --git a/services/src/main/java/org/keycloak/services/resources/admin/ClientAttributeCertificateResource.java b/services/src/main/java/org/keycloak/services/resources/admin/ClientAttributeCertificateResource.java index 118a375f80..f3c8fb1565 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/ClientAttributeCertificateResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/ClientAttributeCertificateResource.java @@ -197,7 +197,9 @@ public class ClientAttributeCertificateResource { CertificateRepresentation info = new CertificateRepresentation(); MultivaluedMap uploadForm = session.getContext().getHttpRequest().getMultiPartFormParameters(); FormPartValue keystoreFormatPart = uploadForm.getFirst("keystoreFormat"); - if (keystoreFormatPart == null) throw new BadRequestException(); + if (keystoreFormatPart == null) { + throw new BadRequestException("keystoreFormat cannot be null"); + } String keystoreFormat = keystoreFormatPart.asString(); FormPartValue inputParts = uploadForm.getFirst("file"); if (keystoreFormat.equals(CERTIFICATE_PEM)) { diff --git a/services/src/main/java/org/keycloak/services/resources/admin/ComponentResource.java b/services/src/main/java/org/keycloak/services/resources/admin/ComponentResource.java index 3d0c411830..c43e87ff2d 100644 --- a/services/src/main/java/org/keycloak/services/resources/admin/ComponentResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/ComponentResource.java @@ -145,7 +145,7 @@ public class ComponentResource { } catch (ComponentValidationException e) { return localizedErrorResponse(e); } catch (IllegalArgumentException e) { - throw new BadRequestException(e); + throw new BadRequestException("Invalid provider type or no such provider", e); } } @@ -184,7 +184,7 @@ public class ComponentResource { } catch (ComponentValidationException e) { return localizedErrorResponse(e); } catch (IllegalArgumentException e) { - throw new BadRequestException(); + throw new BadRequestException("Invalid provider type or no such provider", e); } } @DELETE diff --git a/services/src/main/java/org/keycloak/services/resources/admin/RoleContainerResource.java b/services/src/main/java/org/keycloak/services/resources/admin/RoleContainerResource.java index c161f2b92e..4c1b1c3a18 100755 --- a/services/src/main/java/org/keycloak/services/resources/admin/RoleContainerResource.java +++ b/services/src/main/java/org/keycloak/services/resources/admin/RoleContainerResource.java @@ -143,7 +143,7 @@ public class RoleContainerResource extends RoleResource { auth.roles().requireManage(roleContainer); if (rep.getName() == null) { - throw new BadRequestException(); + throw new BadRequestException("role has no name"); } try { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/error/UncaughtErrorPageTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/error/UncaughtErrorPageTest.java index 2ff919bf57..8c2a50757e 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/error/UncaughtErrorPageTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/error/UncaughtErrorPageTest.java @@ -26,7 +26,6 @@ import org.keycloak.util.JsonSerialization; import org.keycloak.utils.MediaType; import org.openqa.selenium.By; -import jakarta.ws.rs.core.Response; import java.io.IOException; import java.io.InputStream; import java.net.MalformedURLException; @@ -38,10 +37,13 @@ import java.util.HashSet; import java.util.List; import java.util.Objects; +import jakarta.ws.rs.core.Response; + import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.core.Is.is; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.keycloak.utils.MediaType.APPLICATION_JSON; @@ -95,7 +97,7 @@ public class UncaughtErrorPageTest extends AbstractKeycloakTest { OAuth2ErrorRepresentation error = JsonSerialization.readValue(response.getEntity().getContent(), OAuth2ErrorRepresentation.class); assertEquals(OAuthErrorException.INVALID_REQUEST, error.getError()); - assertNull(error.getErrorDescription()); + assertNotNull(error.getErrorDescription()); } } @@ -115,7 +117,7 @@ public class UncaughtErrorPageTest extends AbstractKeycloakTest { OAuth2ErrorRepresentation error = JsonSerialization.readValue(response.getEntity().getContent(), OAuth2ErrorRepresentation.class); assertEquals(OAuthErrorException.INVALID_REQUEST, error.getError()); - assertNull(error.getErrorDescription()); + assertNotNull(error.getErrorDescription()); } }