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 2e2641c009..7e0e11274b 100755 --- a/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java +++ b/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java @@ -693,6 +693,20 @@ public class LoginActionsService { public Response emailVerification(@QueryParam("code") String code, @QueryParam("key") String key) { event.event(EventType.VERIFY_EMAIL); if (key != null) { + ClientSessionModel clientSession = null; + String keyFromSession = null; + if (code != null) { + clientSession = ClientSessionCode.getClientSession(code, session, realm); + keyFromSession = clientSession.getNote(Constants.VERIFY_EMAIL_KEY); + } + + if (clientSession == null || !key.equals(keyFromSession)) { + ServicesLogger.LOGGER.invalidKeyForEmailVerification(); + event.error(Errors.INVALID_CODE); + throw new WebApplicationException(ErrorPage.error(session, Messages.STALE_VERIFY_EMAIL_LINK)); + } + clientSession.removeNote(Constants.VERIFY_EMAIL_KEY); + Checks checks = new Checks(); if (!checks.verifyCode(code, ClientSessionModel.Action.REQUIRED_ACTIONS.name(), ClientSessionCode.ActionType.USER)) { if (checks.clientCode == null && checks.result.isClientSessionNotFound() || checks.result.isIllegalHash()) { @@ -700,8 +714,9 @@ public class LoginActionsService { } return checks.response; } + ClientSessionCode accessCode = checks.clientCode; - ClientSessionModel clientSession = accessCode.getClientSession(); + clientSession = accessCode.getClientSession(); if (!ClientSessionModel.Action.VERIFY_EMAIL.name().equals(clientSession.getNote(AuthenticationManager.CURRENT_REQUIRED_ACTION))) { ServicesLogger.LOGGER.reqdActionDoesNotMatch(); event.error(Errors.INVALID_CODE); @@ -713,14 +728,6 @@ public class LoginActionsService { initEvent(clientSession); event.event(EventType.VERIFY_EMAIL).detail(Details.EMAIL, user.getEmail()); - String keyFromSession = clientSession.getNote(Constants.VERIFY_EMAIL_KEY); - clientSession.removeNote(Constants.VERIFY_EMAIL_KEY); - if (!key.equals(keyFromSession)) { - ServicesLogger.LOGGER.invalidKeyForEmailVerification(); - event.error(Errors.INVALID_USER_CREDENTIALS); - throw new WebApplicationException(ErrorPage.error(session, Messages.INVALID_CODE)); - } - user.setEmailVerified(true); user.removeRequiredAction(RequiredAction.VERIFY_EMAIL); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionEmailVerificationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionEmailVerificationTest.java index 54da2cb328..e68af584be 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionEmailVerificationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionEmailVerificationTest.java @@ -31,6 +31,7 @@ import org.keycloak.testsuite.AssertEvents; import org.keycloak.testsuite.TestRealmKeycloakTest; import org.keycloak.testsuite.pages.AppPage; import org.keycloak.testsuite.pages.AppPage.RequestType; +import org.keycloak.testsuite.pages.ErrorPage; import org.keycloak.testsuite.pages.InfoPage; import org.keycloak.testsuite.pages.LoginPage; import org.keycloak.testsuite.pages.RegisterPage; @@ -72,6 +73,9 @@ public class RequiredActionEmailVerificationTest extends TestRealmKeycloakTest { @Page protected InfoPage infoPage; + @Page + protected ErrorPage errorPage; + @Override public void configureTestRealm(RealmRepresentation testRealm) { testRealm.setVerifyEmail(Boolean.TRUE); @@ -176,6 +180,33 @@ public class RequiredActionEmailVerificationTest extends TestRealmKeycloakTest { events.expectLogin().session(sessionId).assertEvent(); } + @Test + public void verifyEmailResendFirstInvalidSecondStillValid() throws IOException, MessagingException { + loginPage.open(); + loginPage.login("test-user@localhost", "password"); + + verifyEmailPage.clickResendEmail(); + + Assert.assertEquals(2, greenMail.getReceivedMessages().length); + + MimeMessage message1 = greenMail.getReceivedMessages()[0]; + + String verificationUrl1 = getPasswordResetEmailLink(message1); + + driver.navigate().to(verificationUrl1.trim()); + + assertTrue(errorPage.isCurrent()); + assertEquals("The link you clicked is a old stale link and is no longer valid. Maybe you have already verified your email?", errorPage.getError()); + + MimeMessage message2 = greenMail.getReceivedMessages()[1]; + + String verificationUrl2 = getPasswordResetEmailLink(message2); + + driver.navigate().to(verificationUrl2.trim()); + + Assert.assertEquals(RequestType.AUTH_RESPONSE, appPage.getRequestType()); + } + @Test public void verifyEmailNewBrowserSession() throws IOException, MessagingException { loginPage.open(); @@ -221,10 +252,7 @@ public class RequiredActionEmailVerificationTest extends TestRealmKeycloakTest { String resendEmailLink = verifyEmailPage.getResendEmailLink(); String keyInsteadCodeURL = resendEmailLink.replace("code=", "key="); - AssertEvents.ExpectedEvent emailEvent = events.expectRequiredAction(EventType.SEND_VERIFY_EMAIL).detail("email", "test-user@localhost"); - EventRepresentation sendEvent = emailEvent.assertEvent(); - String sessionId = sendEvent.getSessionId(); - String mailCodeId = sendEvent.getDetails().get(Details.CODE_ID); + events.expectRequiredAction(EventType.SEND_VERIFY_EMAIL).detail("email", "test-user@localhost").assertEvent(); driver.navigate().to(keyInsteadCodeURL); @@ -240,10 +268,11 @@ public class RequiredActionEmailVerificationTest extends TestRealmKeycloakTest { driver.navigate().to(badKeyURL); events.expectRequiredAction(EventType.VERIFY_EMAIL_ERROR) - .error(Errors.INVALID_USER_CREDENTIALS) - .session(sessionId) - .detail("email", "test-user@localhost") - .detail(Details.CODE_ID, mailCodeId) + .error(Errors.INVALID_CODE) + .client((String)null) + .user((String)null) + .session((String)null) + .clearDetails() .assertEvent(); }