Merge pull request #3644 from stianst/KEYCLOAK-4071

KEYCLOAK-4071

server-spi-private/src/main/java/org/keycloak/services/managers/ClientSessionCode.java

@@ -126,9 +126,13 @@ public class ClientSessionCode {
     }
 
     public static ClientSessionModel getClientSession(String code, KeycloakSession session, RealmModel realm) {
-        String[] parts = code.split("\\.");
+        try {
-        String id = parts[1];
+            String[] parts = code.split("\\.");
-        return session.sessions().getClientSession(realm, id);
+            String id = parts[1];
+            return session.sessions().getClientSession(realm, id);
+        } catch (ArrayIndexOutOfBoundsException e) {
+            return null;
+        }
     }
 
     public ClientSessionModel getClientSession() {

services/src/main/java/org/keycloak/services/resources/LoginActionsService.java

@@ -715,14 +715,15 @@ public class LoginActionsService {
             String keyFromSession = null;
             if (code != null) {
                 clientSession = ClientSessionCode.getClientSession(code, session, realm);
-                keyFromSession = clientSession.getNote(Constants.VERIFY_EMAIL_KEY);
+                keyFromSession = clientSession != null ? clientSession.getNote(Constants.VERIFY_EMAIL_KEY) : null;
             }
 
-            if (clientSession == null || !key.equals(keyFromSession)) {
+            if (!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();

testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/actions/RequiredActionEmailVerificationTest.java

@@ -264,7 +264,7 @@ public class RequiredActionEmailVerificationTest extends TestRealmKeycloakTest {
                 .clearDetails()
                 .assertEvent();
 
-        String badKeyURL = KeycloakUriBuilder.fromUri(resendEmailLink).queryParam("key", "foo").build().toString();
+        String badKeyURL = KeycloakUriBuilder.fromUri(resendEmailLink).replaceQueryParam("key", "foo").build().toString();
         driver.navigate().to(badKeyURL);
 
         events.expectRequiredAction(EventType.VERIFY_EMAIL_ERROR)
@@ -276,6 +276,37 @@ public class RequiredActionEmailVerificationTest extends TestRealmKeycloakTest {
                 .assertEvent();
     }
 
+    @Test
+    public void verifyEmailBadCode() throws IOException, MessagingException {
+        loginPage.open();
+        loginPage.login("test-user@localhost", "password");
+
+        Assert.assertTrue(verifyEmailPage.isCurrent());
+
+        Assert.assertEquals(1, greenMail.getReceivedMessages().length);
+
+        MimeMessage message = greenMail.getReceivedMessages()[0];
+
+        String verificationUrl = getPasswordResetEmailLink(message);
+
+        verificationUrl = KeycloakUriBuilder.fromUri(verificationUrl).replaceQueryParam("code", "foo").build().toString();
+
+        events.poll();
+
+        driver.navigate().to(verificationUrl.trim());
+
+        assertEquals("The link you clicked is a old stale link and is no longer valid. Maybe you have already verified your email?", errorPage.getError());
+
+        events.expectRequiredAction(EventType.VERIFY_EMAIL_ERROR)
+                .error(Errors.INVALID_CODE)
+                .client((String)null)
+                .user((String)null)
+                .session((String)null)
+                .clearDetails()
+                .assertEvent();
+    }
+
+
     public static String getPasswordResetEmailLink(MimeMessage message) throws IOException, MessagingException {
     	Multipart multipart = (Multipart) message.getContent();