From 506e2537acf33c216833735ac90fc2ffe299bcfe Mon Sep 17 00:00:00 2001 From: Marek Posolda Date: Fri, 8 Sep 2023 08:05:05 +0200 Subject: [PATCH] Registration flow fixed (#23064) Closes #21514 Co-authored-by: Vilmos Nagy Co-authored-by: Alexander Schwartz Co-authored-by: Marek Posolda --- .../keycloak/authentication/FormContext.java | 2 +- .../main/java/org/keycloak/events/Errors.java | 1 + .../forms/RegistrationUserCreation.java | 14 +++++- .../testsuite/forms/RegisterTest.java | 48 ++++++++++++++++++- 4 files changed, 61 insertions(+), 4 deletions(-) diff --git a/server-spi-private/src/main/java/org/keycloak/authentication/FormContext.java b/server-spi-private/src/main/java/org/keycloak/authentication/FormContext.java index fb340ca640..f901af0979 100755 --- a/server-spi-private/src/main/java/org/keycloak/authentication/FormContext.java +++ b/server-spi-private/src/main/java/org/keycloak/authentication/FormContext.java @@ -58,7 +58,7 @@ public interface FormContext { AuthenticationExecutionModel getExecution(); /** - * Current user attached to this flow. It can return null if no uesr has been identified yet + * Current user attached to this flow. It can return null if no user has been identified yet * * @return */ diff --git a/server-spi-private/src/main/java/org/keycloak/events/Errors.java b/server-spi-private/src/main/java/org/keycloak/events/Errors.java index 1c021cd416..797a2e05c0 100755 --- a/server-spi-private/src/main/java/org/keycloak/events/Errors.java +++ b/server-spi-private/src/main/java/org/keycloak/events/Errors.java @@ -38,6 +38,7 @@ public interface Errors { String USER_DISABLED = "user_disabled"; String USER_TEMPORARILY_DISABLED = "user_temporarily_disabled"; String INVALID_USER_CREDENTIALS = "invalid_user_credentials"; + String DIFFERENT_USER_AUTHENTICATING = "different_user_authenticating"; String DIFFERENT_USER_AUTHENTICATED = "different_user_authenticated"; String USER_DELETE_ERROR = "user_delete_error"; diff --git a/services/src/main/java/org/keycloak/authentication/forms/RegistrationUserCreation.java b/services/src/main/java/org/keycloak/authentication/forms/RegistrationUserCreation.java index 84b33afd42..e1b583592a 100755 --- a/services/src/main/java/org/keycloak/authentication/forms/RegistrationUserCreation.java +++ b/services/src/main/java/org/keycloak/authentication/forms/RegistrationUserCreation.java @@ -18,6 +18,8 @@ package org.keycloak.authentication.forms; import org.keycloak.Config; +import org.keycloak.authentication.AuthenticationFlowError; +import org.keycloak.authentication.AuthenticationFlowException; import org.keycloak.authentication.FormAction; import org.keycloak.authentication.FormActionFactory; import org.keycloak.authentication.FormContext; @@ -106,11 +108,13 @@ public class RegistrationUserCreation implements FormAction, FormActionFactory { @Override public void buildPage(FormContext context, LoginFormsProvider form) { - + checkNotOtherUserAuthenticating(context); } @Override public void success(FormContext context) { + checkNotOtherUserAuthenticating(context); + MultivaluedMap formData = context.getHttpRequest().getDecodedFormParameters(); String email = formData.getFirst(UserModel.EMAIL); @@ -148,6 +152,14 @@ public class RegistrationUserCreation implements FormAction, FormActionFactory { } } + private void checkNotOtherUserAuthenticating(FormContext context) { + if (context.getUser() != null) { + // the user probably did some back navigation in the browser, hitting this page in a strange state + context.getEvent().detail(Details.EXISTING_USER, context.getUser().getUsername()); + throw new AuthenticationFlowException(AuthenticationFlowError.GENERIC_AUTHENTICATION_ERROR, Errors.DIFFERENT_USER_AUTHENTICATING, Messages.EXPIRED_ACTION); + } + } + @Override public boolean requiresUser() { return false; diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/RegisterTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/RegisterTest.java index cff5c589f6..25115aad02 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/RegisterTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/RegisterTest.java @@ -30,17 +30,20 @@ import org.keycloak.authentication.forms.RegistrationRecaptcha; import org.keycloak.authentication.forms.RegistrationTermsAndConditions; import org.keycloak.authentication.forms.RegistrationUserCreation; import org.keycloak.events.Details; +import org.keycloak.events.Errors; import org.keycloak.events.EventType; import org.keycloak.models.AuthenticationExecutionModel; +import org.keycloak.models.utils.DefaultAuthenticationFlows; import org.keycloak.representations.idm.EventRepresentation; import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.representations.idm.UserRepresentation; import org.keycloak.testsuite.AbstractTestRealmKeycloakTest; import org.keycloak.testsuite.AssertEvents; -import org.keycloak.testsuite.AbstractTestRealmKeycloakTest; import org.keycloak.testsuite.pages.AppPage; import org.keycloak.testsuite.pages.AppPage.RequestType; +import org.keycloak.testsuite.pages.ErrorPage; import org.keycloak.testsuite.pages.LoginPage; +import org.keycloak.testsuite.pages.LoginPasswordResetPage; import org.keycloak.testsuite.pages.RegisterPage; import org.keycloak.testsuite.pages.VerifyEmailPage; import org.keycloak.testsuite.updaters.RealmAttributeUpdater; @@ -79,12 +82,18 @@ public class RegisterTest extends AbstractTestRealmKeycloakTest { @Page protected LoginPage loginPage; + @Page + protected ErrorPage errorPage; + @Page protected RegisterPage registerPage; @Page protected VerifyEmailPage verifyEmailPage; + @Page + protected LoginPasswordResetPage resetPasswordPage; + @Rule public GreenMailRule greenMail = new GreenMailRule(); @@ -669,7 +678,7 @@ public class RegisterTest extends AbstractTestRealmKeycloakTest { .removeDetail(Details.EMAIL) .error("invalid_registration").assertEvent(); } finally { - configureRegistrationFlowWithCustomRegistrationPageForm(UUID.randomUUID().toString()); + revertRegistrationFlow(); } } @@ -697,6 +706,34 @@ public class RegisterTest extends AbstractTestRealmKeycloakTest { } } + @Test + public void testRegisterShouldFailBeforeUserCreationWhenUserIsInContext() { + loginPage.open(); + loginPage.clickRegister(); + registerPage.clickBackToLogin(); + loginPage.assertCurrent(testRealm().toRepresentation().getRealm()); + + loginPage.resetPassword(); + resetPasswordPage.assertCurrent(); + resetPasswordPage.changePassword("test-user@localhost"); + + driver.navigate().back(); + driver.navigate().back(); + events.clear(); + driver.navigate().back(); + + errorPage.assertCurrent(); + Assert.assertEquals("Action expired. Please continue with login now.", errorPage.getError()); + + events.expectRegister("registerUserMissingTermsAcceptance", "registerUserMissingTermsAcceptance@email") + .removeDetail(Details.USERNAME) + .removeDetail(Details.EMAIL) + .removeDetail(Details.REGISTER_METHOD) + .detail(Details.EXISTING_USER, "test-user@localhost") + .detail(Details.AUTHENTICATION_ERROR_DETAIL, Errors.DIFFERENT_USER_AUTHENTICATING) + .error(Errors.GENERIC_AUTHENTICATION_ERROR).assertEvent(); + } + protected RealmAttributeUpdater configureRealmRegistrationEmailAsUsername(final boolean value) { return getRealmAttributeUpdater().setRegistrationEmailAsUsername(value); } @@ -784,4 +821,11 @@ public class RegisterTest extends AbstractTestRealmKeycloakTest { ); } + private void revertRegistrationFlow() { + testingClient.server("test").run(session -> FlowUtil.inCurrentRealm(session) + .selectFlow(DefaultAuthenticationFlows.REGISTRATION_FLOW) + .defineAsRegistrationFlow() + ); + } + }