diff --git a/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java b/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java index fe1d6b8587..b1d517341d 100755 --- a/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java +++ b/services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java @@ -54,11 +54,14 @@ import org.keycloak.services.util.CacheControlUtil; import org.keycloak.services.util.AuthenticationFlowURLHelper; import org.keycloak.sessions.AuthenticationSessionModel; import org.keycloak.sessions.CommonClientSessionModel; +import org.keycloak.util.JsonSerialization; import javax.ws.rs.core.MultivaluedHashMap; import javax.ws.rs.core.Response; import javax.ws.rs.core.UriBuilder; import javax.ws.rs.core.UriInfo; + +import java.io.IOException; import java.net.URI; import java.util.HashMap; import java.util.List; @@ -95,12 +98,12 @@ public class AuthenticationProcessor { /** * This could be an error message forwarded from another authenticator */ - protected FormMessage forwardedErrorMessage; + protected ForwardedFormMessageStore forwardedErrorMessageStore = new ForwardedFormMessageStore(ForwardedFormMessageType.ERROR); /** * This could be an success message forwarded from another authenticator */ - protected FormMessage forwardedSuccessMessage; + protected ForwardedFormMessageStore forwardedSuccessMessageStore = new ForwardedFormMessageStore(ForwardedFormMessageType.SUCCESS); // Used for client authentication protected ClientModel client; @@ -212,12 +215,20 @@ public class AuthenticationProcessor { } public AuthenticationProcessor setForwardedErrorMessage(FormMessage forwardedErrorMessage) { - this.forwardedErrorMessage = forwardedErrorMessage; + this.forwardedErrorMessageStore.setForwardedMessage(forwardedErrorMessage); return this; } + FormMessage getAndRemoveForwardedErrorMessage() { + FormMessage formMessage = this.forwardedErrorMessageStore.getForwardedMessage(); + if (formMessage != null) { + this.forwardedErrorMessageStore.removeForwardedMessage(); + } + return formMessage; + } + public AuthenticationProcessor setForwardedSuccessMessage(FormMessage forwardedSuccessMessage) { - this.forwardedSuccessMessage = forwardedSuccessMessage; + this.forwardedSuccessMessageStore.setForwardedMessage(forwardedSuccessMessage); return this; } @@ -480,7 +491,7 @@ public class AuthenticationProcessor { @Override public FormMessage getForwardedErrorMessage() { - return AuthenticationProcessor.this.forwardedErrorMessage; + return AuthenticationProcessor.this.forwardedErrorMessageStore.getForwardedMessage(); } @Override @@ -513,8 +524,10 @@ public class AuthenticationProcessor { .setClientSessionCode(accessCode); if (getForwardedErrorMessage() != null) { provider.addError(getForwardedErrorMessage()); + forwardedErrorMessageStore.removeForwardedMessage(); } else if (getForwardedSuccessMessage() != null) { provider.addSuccess(getForwardedSuccessMessage()); + forwardedSuccessMessageStore.removeForwardedMessage(); } return provider; } @@ -626,7 +639,7 @@ public class AuthenticationProcessor { @Override public FormMessage getForwardedSuccessMessage() { - return AuthenticationProcessor.this.forwardedSuccessMessage; + return AuthenticationProcessor.this.forwardedSuccessMessageStore.getForwardedMessage(); } public FormMessage getErrorMessage() { @@ -1087,6 +1100,51 @@ public class AuthenticationProcessor { } + // This takes care of CRUD of FormMessage to the authenticationSession, so that message can be displayed on the forms in different HTTP request + private class ForwardedFormMessageStore { + private final String messageKey; + + private ForwardedFormMessageStore(ForwardedFormMessageType messageType) { + this.messageKey = messageType.getKey(); + } + + private void setForwardedMessage(FormMessage message) { + try { + logger.tracef("Saving message %s to the authentication session under key %s", message, messageKey); + getAuthenticationSession().setAuthNote(messageKey, JsonSerialization.writeValueAsString(message)); + } catch (IOException ioe) { + throw new RuntimeException("Unexpected exception when serializing formMessage: " + message, ioe); + } + } + + private FormMessage getForwardedMessage() { + String note = getAuthenticationSession().getAuthNote(messageKey); + try { + return note == null ? null : JsonSerialization.readValue(note, FormMessage.class); + } catch (IOException ioe) { + throw new RuntimeException("Unexpected exception when deserializing formMessage JSON: " + note, ioe); + } + } + + private void removeForwardedMessage() { + logger.tracef("Removing message %s from the authentication session", messageKey); + getAuthenticationSession().removeAuthNote(messageKey); + } + } + + private enum ForwardedFormMessageType { + SUCCESS("fwMessageSuccess"), ERROR("fwMessageError"); + + private final String key; + + private ForwardedFormMessageType(String key) { + this.key = key; + } + + private String getKey() { + return key; + } + } } diff --git a/services/src/main/java/org/keycloak/authentication/FormAuthenticationFlow.java b/services/src/main/java/org/keycloak/authentication/FormAuthenticationFlow.java index 5c7c167a67..cdb3bc45d9 100755 --- a/services/src/main/java/org/keycloak/authentication/FormAuthenticationFlow.java +++ b/services/src/main/java/org/keycloak/authentication/FormAuthenticationFlow.java @@ -279,7 +279,8 @@ public class FormAuthenticationFlow implements AuthenticationFlow { public Response processFlow() { // KEYCLOAK-16143: Propagate forwarded error messages if present - List errors = processor.forwardedErrorMessage != null ? Collections.singletonList(processor.forwardedErrorMessage) : null; + FormMessage forwardedErrorMessage = processor.getAndRemoveForwardedErrorMessage(); + List errors = forwardedErrorMessage != null ? Collections.singletonList(forwardedErrorMessage) : null; return renderForm(null, errors); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/kerberos/AbstractKerberosTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/kerberos/AbstractKerberosTest.java index 3c5081346e..ea8c265a0b 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/kerberos/AbstractKerberosTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/kerberos/AbstractKerberosTest.java @@ -362,7 +362,11 @@ public abstract class AbstractKerberosTest extends AbstractAuthTest { protected AuthenticationExecutionModel.Requirement updateKerberosAuthExecutionRequirement(AuthenticationExecutionModel.Requirement requirement) { - Optional kerberosAuthExecutionOpt = testRealmResource() + return updateKerberosAuthExecutionRequirement(requirement, testRealmResource()); + } + + public static AuthenticationExecutionModel.Requirement updateKerberosAuthExecutionRequirement(AuthenticationExecutionModel.Requirement requirement, RealmResource realmResource) { + Optional kerberosAuthExecutionOpt = realmResource .flows() .getExecutions(DefaultAuthenticationFlows.BROWSER_FLOW) .stream() @@ -376,7 +380,7 @@ public abstract class AbstractKerberosTest extends AbstractAuthTest { AuthenticationExecutionModel.Requirement oldRequirement = AuthenticationExecutionModel.Requirement.valueOf(oldRequirementStr); kerberosAuthExecution.setRequirement(requirement.name()); - testRealmResource() + realmResource .flows() .updateExecutions(DefaultAuthenticationFlows.BROWSER_FLOW, kerberosAuthExecution); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java index 7880eef766..84d896509a 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java @@ -26,6 +26,7 @@ import org.keycloak.common.constants.ServiceAccountConstants; import org.keycloak.events.Details; import org.keycloak.events.Errors; import org.keycloak.events.EventType; +import org.keycloak.models.AuthenticationExecutionModel; import org.keycloak.models.Constants; import org.keycloak.models.utils.SystemClientUtil; import org.keycloak.protocol.oidc.utils.RedirectUtils; @@ -38,6 +39,7 @@ import org.keycloak.testsuite.AbstractTestRealmKeycloakTest; import org.keycloak.testsuite.admin.ApiUtil; import org.keycloak.testsuite.arquillian.annotation.AuthServerContainerExclude; import org.keycloak.testsuite.arquillian.annotation.DisableFeature; +import org.keycloak.testsuite.federation.kerberos.AbstractKerberosTest; import org.keycloak.testsuite.pages.AppPage; import org.keycloak.testsuite.pages.AppPage.RequestType; import org.keycloak.testsuite.pages.ErrorPage; @@ -1068,6 +1070,22 @@ public class ResetPasswordTest extends AbstractTestRealmKeycloakTest { assertThat(driver2.getPageSource(), Matchers.containsString("Your account has been updated.")); } + + // KEYCLOAK-15239 + @Test + public void resetPasswordWithSpnegoEnabled() throws IOException, MessagingException { + // Just switch SPNEGO authenticator requirement to alternative. No real usage of SPNEGO needed for this test + AuthenticationExecutionModel.Requirement origRequirement = AbstractKerberosTest.updateKerberosAuthExecutionRequirement(AuthenticationExecutionModel.Requirement.ALTERNATIVE, testRealm()); + + try { + resetPassword("login-test"); + } finally { + // Revert + AbstractKerberosTest.updateKerberosAuthExecutionRequirement(origRequirement, testRealm()); + } + } + + @Test public void failResetPasswordServiceAccount() { String username = ServiceAccountConstants.SERVICE_ACCOUNT_USER_PREFIX + "client-user";