From d853dcab7d9e076a9290f11c48d19ae6b177f1ba Mon Sep 17 00:00:00 2001 From: vramik Date: Thu, 31 Oct 2024 10:34:19 +0100 Subject: [PATCH] Use specific error message from required actions for SamlProtocol if available Closes #34514 Signed-off-by: vramik --- .../src/main/java/org/keycloak/protocol/LoginProtocol.java | 6 +----- .../keycloak/authentication/AuthenticationProcessor.java | 2 +- .../org/keycloak/protocol/AuthorizationEndpointBase.java | 4 ++-- .../org/keycloak/protocol/docker/DockerAuthV2Protocol.java | 2 +- .../java/org/keycloak/protocol/oidc/OIDCLoginProtocol.java | 5 ----- .../main/java/org/keycloak/protocol/saml/SamlProtocol.java | 4 ++-- .../keycloak/services/managers/AuthenticationManager.java | 2 +- .../keycloak/services/resources/IdentityBrokerService.java | 2 +- .../keycloak/services/resources/LoginActionsService.java | 4 ++-- 9 files changed, 11 insertions(+), 20 deletions(-) diff --git a/server-spi-private/src/main/java/org/keycloak/protocol/LoginProtocol.java b/server-spi-private/src/main/java/org/keycloak/protocol/LoginProtocol.java index 71f630cc04..9d5c452825 100755 --- a/server-spi-private/src/main/java/org/keycloak/protocol/LoginProtocol.java +++ b/server-spi-private/src/main/java/org/keycloak/protocol/LoginProtocol.java @@ -84,12 +84,8 @@ public interface LoginProtocol extends Provider { Response authenticated(AuthenticationSessionModel authSession, UserSessionModel userSession, ClientSessionContext clientSessionCtx); - Response sendError(AuthenticationSessionModel authSession, Error error); + Response sendError(AuthenticationSessionModel authSession, Error error, String errorMessage); - default Response sendError(AuthenticationSessionModel authSession, Error error, String errorMessage) { - return sendError(authSession, error); - } - /** * Returns client data, which will be wrapped in the "clientData" parameter sent within "authentication flow" requests. The purpose of clientData is to be able to send HTTP error * response back to the client if authentication fails due some error and authenticationSession is not available anymore (was either expired or removed). So clientData need to contain diff --git a/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java b/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java index ea9bb18ec5..4feb42f45e 100755 --- a/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java +++ b/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java @@ -648,7 +648,7 @@ public class AuthenticationProcessor { .setHttpHeaders(getHttpRequest().getHttpHeaders()) .setUriInfo(getUriInfo()) .setEventBuilder(event); - Response response = protocol.sendError(getAuthenticationSession(), Error.CANCELLED_BY_USER); + Response response = protocol.sendError(getAuthenticationSession(), Error.CANCELLED_BY_USER, null); forceChallenge(response); } diff --git a/services/src/main/java/org/keycloak/protocol/AuthorizationEndpointBase.java b/services/src/main/java/org/keycloak/protocol/AuthorizationEndpointBase.java index 7db504458e..07752d166c 100755 --- a/services/src/main/java/org/keycloak/protocol/AuthorizationEndpointBase.java +++ b/services/src/main/java/org/keycloak/protocol/AuthorizationEndpointBase.java @@ -124,14 +124,14 @@ public abstract class AuthorizationEndpointBase { return challenge; } else { - return protocol.sendError(authSession, Error.PASSIVE_LOGIN_REQUIRED); + return protocol.sendError(authSession, Error.PASSIVE_LOGIN_REQUIRED, null); } } AuthenticationManager.setClientScopesInSession(session, authSession); if (processor.nextRequiredAction() != null) { - return protocol.sendError(authSession, Error.PASSIVE_INTERACTION_REQUIRED); + return protocol.sendError(authSession, Error.PASSIVE_INTERACTION_REQUIRED, null); } } catch (Exception e) { diff --git a/services/src/main/java/org/keycloak/protocol/docker/DockerAuthV2Protocol.java b/services/src/main/java/org/keycloak/protocol/docker/DockerAuthV2Protocol.java index 0ddcc5fa60..ae8619f7e0 100644 --- a/services/src/main/java/org/keycloak/protocol/docker/DockerAuthV2Protocol.java +++ b/services/src/main/java/org/keycloak/protocol/docker/DockerAuthV2Protocol.java @@ -145,7 +145,7 @@ public class DockerAuthV2Protocol implements LoginProtocol { } @Override - public Response sendError(final AuthenticationSessionModel clientSession, final LoginProtocol.Error error) { + public Response sendError(final AuthenticationSessionModel clientSession, final LoginProtocol.Error error, String errorMessage) { return new ResponseBuilderImpl().status(Response.Status.INTERNAL_SERVER_ERROR).build(); } diff --git a/services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocol.java b/services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocol.java index 45ef54c794..07d6305f52 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocol.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocol.java @@ -340,11 +340,6 @@ public class OIDCLoginProtocol implements LoginProtocol { return Boolean.valueOf(Optional.ofNullable(client.getAttribute(OIDCConfigAttributes.ID_TOKEN_AS_DETACHED_SIGNATURE)).orElse(Boolean.FALSE.toString())).booleanValue(); } - @Override - public Response sendError(AuthenticationSessionModel authSession, Error error) { - return sendError(authSession, error, null); - } - @Override public Response sendError(AuthenticationSessionModel authSession, Error error, String errorMessage) { if (isOAuth2DeviceVerificationFlow(authSession)) { diff --git a/services/src/main/java/org/keycloak/protocol/saml/SamlProtocol.java b/services/src/main/java/org/keycloak/protocol/saml/SamlProtocol.java index 3733a5b6fb..9652ec58fa 100755 --- a/services/src/main/java/org/keycloak/protocol/saml/SamlProtocol.java +++ b/services/src/main/java/org/keycloak/protocol/saml/SamlProtocol.java @@ -219,7 +219,7 @@ public class SamlProtocol implements LoginProtocol { } @Override - public Response sendError(AuthenticationSessionModel authSession, Error error) { + public Response sendError(AuthenticationSessionModel authSession, Error error, String errorMessage) { try { ClientModel client = authSession.getClient(); @@ -233,7 +233,7 @@ public class SamlProtocol implements LoginProtocol { URI redirect = builder.buildFromMap(params); return Response.status(302).location(redirect).build(); } else { - return ErrorPage.error(session, authSession, Response.Status.BAD_REQUEST, translateErrorToIdpInitiatedErrorMessage(error)); + return ErrorPage.error(session, authSession, Response.Status.BAD_REQUEST, errorMessage != null ? errorMessage : translateErrorToIdpInitiatedErrorMessage(error)); } } else { return samlErrorMessage( diff --git a/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java b/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java index 67b8110d91..9bd107b7a8 100755 --- a/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java +++ b/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java @@ -1311,7 +1311,7 @@ public class AuthenticationManager { .setHttpHeaders(context.getHttpRequest().getHttpHeaders()) .setUriInfo(context.getUriInfo()) .setEventBuilder(event); - Response response = protocol.sendError(context.getAuthenticationSession(), Error.CONSENT_DENIED); + Response response = protocol.sendError(context.getAuthenticationSession(), Error.CONSENT_DENIED, null); event.error(Errors.REJECTED_BY_USER); return response; } 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 f33ef3e150..8eb68f0d84 100755 --- a/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java +++ b/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java @@ -1207,7 +1207,7 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal .setHttpHeaders(headers) .setUriInfo(session.getContext().getUri()) .setEventBuilder(event); - return protocol.sendError(authSession, error); + return protocol.sendError(authSession, error, null); } return null; } diff --git a/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java b/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java index 5747f16319..7cece59f8f 100755 --- a/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java +++ b/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java @@ -925,7 +925,7 @@ public class LoginActionsService { .setHttpHeaders(headers) .setUriInfo(session.getContext().getUri()) .setEventBuilder(event); - return protocol.sendError(authSession, Error.PASSIVE_INTERACTION_REQUIRED); + return protocol.sendError(authSession, Error.PASSIVE_INTERACTION_REQUIRED, null); } } return challenge; @@ -1014,7 +1014,7 @@ public class LoginActionsService { .setHttpHeaders(headers) .setUriInfo(session.getContext().getUri()) .setEventBuilder(event); - Response response = protocol.sendError(authSession, Error.CONSENT_DENIED); + Response response = protocol.sendError(authSession, Error.CONSENT_DENIED, null); event.error(Errors.REJECTED_BY_USER); return response; }