From cccddebfd03e035b8eacef31943065ff1c0cd28b Mon Sep 17 00:00:00 2001 From: stianst Date: Wed, 6 Dec 2017 11:53:01 +0100 Subject: [PATCH] KEYCLOAK-5984 Fix error message in client initiated --- .../resources/IdentityBrokerService.java | 20 +++++++++---------- .../error/UncaughtErrorPageTest.java | 10 ++++++++++ 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java b/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java index 611f05d07b..7f62fb53e0 100755 --- a/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java +++ b/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java @@ -317,12 +317,12 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal return response; } } catch (IdentityBrokerException e) { - return redirectToErrorPage(authSession, Messages.COULD_NOT_SEND_AUTHENTICATION_REQUEST, e, providerId); + return redirectToErrorPage(authSession, Response.Status.INTERNAL_SERVER_ERROR, Messages.COULD_NOT_SEND_AUTHENTICATION_REQUEST, e, providerId); } catch (Exception e) { - return redirectToErrorPage(authSession, Messages.UNEXPECTED_ERROR_HANDLING_REQUEST, e, providerId); + return redirectToErrorPage(authSession, Response.Status.INTERNAL_SERVER_ERROR, Messages.UNEXPECTED_ERROR_HANDLING_REQUEST, e, providerId); } - return redirectToErrorPage(authSession, Messages.COULD_NOT_PROCEED_WITH_AUTHENTICATION_REQUEST); + return redirectToErrorPage(authSession, Response.Status.INTERNAL_SERVER_ERROR, Messages.COULD_NOT_PROCEED_WITH_AUTHENTICATION_REQUEST); } @@ -670,7 +670,7 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal return finishOrRedirectToPostBrokerLogin(authSession, context, true, clientSessionCode); } catch (Exception e) { - return redirectToErrorPage(authSession,Messages.IDENTITY_PROVIDER_UNEXPECTED_ERROR, e); + return redirectToErrorPage(authSession, Response.Status.INTERNAL_SERVER_ERROR, Messages.IDENTITY_PROVIDER_UNEXPECTED_ERROR, e); } } @@ -734,7 +734,7 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal return afterPostBrokerLoginFlowSuccess(authenticationSession, context, wasFirstBrokerLogin, parsedCode.clientSessionCode); } catch (IdentityBrokerException e) { - return redirectToErrorPage(authenticationSession, Messages.IDENTITY_PROVIDER_UNEXPECTED_ERROR, e); + return redirectToErrorPage(authenticationSession, Response.Status.INTERNAL_SERVER_ERROR, Messages.IDENTITY_PROVIDER_UNEXPECTED_ERROR, e); } } @@ -752,7 +752,7 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal UserModel linkingUser = AbstractIdpAuthenticator.getExistingUser(session, realmModel, authSession); if (!linkingUser.getId().equals(federatedUser.getId())) { - return redirectToErrorPage(authSession, Messages.IDENTITY_PROVIDER_DIFFERENT_USER_MESSAGE, federatedUser.getUsername(), linkingUser.getUsername()); + return redirectToErrorPage(authSession, Response.Status.BAD_REQUEST, Messages.IDENTITY_PROVIDER_DIFFERENT_USER_MESSAGE, federatedUser.getUsername(), linkingUser.getUsername()); } SerializedBrokeredIdentityContext serializedCtx = SerializedBrokeredIdentityContext.readFromAuthenticationSession(authSession, AbstractIdpAuthenticator.BROKERED_CONTEXT_NOTE); @@ -866,7 +866,7 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal } if (!authenticatedUser.hasRole(this.realmModel.getClientByClientId(Constants.ACCOUNT_MANAGEMENT_CLIENT_ID).getRole(AccountRoles.MANAGE_ACCOUNT))) { - return redirectToErrorPage(authSession, Messages.INSUFFICIENT_PERMISSION); + return redirectToErrorPage(authSession, Response.Status.FORBIDDEN, Messages.INSUFFICIENT_PERMISSION); } if (!authenticatedUser.isEnabled()) { @@ -919,7 +919,7 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal if (authSession.getClient() != null && authSession.getClient().getClientId().equals(Constants.ACCOUNT_MANAGEMENT_CLIENT_ID)) { return redirectToAccountErrorPage(authSession, message, parameters); } else { - return redirectToErrorPage(authSession, message, parameters); // Should rather redirect to app instead and display error here? + return redirectToErrorPage(authSession, Response.Status.BAD_REQUEST, message, parameters); // Should rather redirect to app instead and display error here? } } @@ -1057,8 +1057,8 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal return Urls.identityProviderAuthnResponse(this.uriInfo.getBaseUri(), providerId, this.realmModel.getName()).toString(); } - private Response redirectToErrorPage(AuthenticationSessionModel authSession,String message, Object ... parameters) { - return redirectToErrorPage(authSession, Response.Status.INTERNAL_SERVER_ERROR, message, null, parameters); + private Response redirectToErrorPage(AuthenticationSessionModel authSession, Response.Status status, String message, Object ... parameters) { + return redirectToErrorPage(authSession, status, message, null, parameters); } private Response redirectToErrorPage(Response.Status status, String message, Object ... parameters) { 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 94e3831979..8a57e26bba 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 @@ -55,6 +55,16 @@ public class UncaughtErrorPageTest extends AbstractKeycloakTest { assertEquals("An internal server error has occurred", errorPage.getError()); } + @Test + public void errorPageException() { + oauth.realm("master"); + oauth.clientId("nosuch"); + oauth.openLoginForm(); + + assertTrue(errorPage.isCurrent()); + assertEquals("Client not found.", errorPage.getError()); + } + private void checkPageNotFound(String path) throws MalformedURLException { URI uri = suiteContext.getAuthServerInfo().getUriBuilder().path(path).build(); driver.navigate().to(uri.toURL());