KEYCLOAK-3641 Clicking an invalid verification link due to re-send removes the email verification key from the session

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);

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");
+        events.expectRequiredAction(EventType.SEND_VERIFY_EMAIL).detail("email", "test-user@localhost").assertEvent();
-        EventRepresentation sendEvent = emailEvent.assertEvent();
-        String sessionId = sendEvent.getSessionId();
-        String mailCodeId = sendEvent.getDetails().get(Details.CODE_ID);
 
         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)
+                .error(Errors.INVALID_CODE)
-                .session(sessionId)
+                .client((String)null)
-                .detail("email", "test-user@localhost")
+                .user((String)null)
-                .detail(Details.CODE_ID, mailCodeId)
+                .session((String)null)
+                .clearDetails()
                 .assertEvent();
     }