KEYCLOAK-2011

forms/login-api/src/main/java/org/keycloak/login/LoginFormsProvider.java

@@ -52,6 +52,8 @@ public interface LoginFormsProvider extends Provider {
 
     public LoginFormsProvider setClientSessionCode(String accessCode);
 
+    public LoginFormsProvider setClientSession(ClientSessionModel clientSession);
+
     public LoginFormsProvider setAccessRequest(List<RoleModel> realmRolesRequested, MultivaluedMap<String,RoleModel> resourceRolesRequested, List<ProtocolMapperModel> protocolMappers);
     public LoginFormsProvider setAccessRequest(String message);
 

forms/login-freemarker/src/main/java/org/keycloak/login/freemarker/FreeMarkerLoginFormsProvider.java

@@ -47,6 +47,7 @@ import org.keycloak.login.freemarker.model.TotpBean;
 import org.keycloak.login.freemarker.model.UrlBean;
 import org.keycloak.models.ClientModel;
 import org.keycloak.models.ClientSessionModel;
+import org.keycloak.models.Constants;
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.ProtocolMapperModel;
 import org.keycloak.models.RealmModel;
@@ -138,7 +139,8 @@ public class FreeMarkerLoginFormsProvider implements LoginFormsProvider {
             case VERIFY_EMAIL:
                 try {
                     UriBuilder builder = Urls.loginActionEmailVerificationBuilder(uriInfo.getBaseUri());
-                    builder.queryParam("key", accessCode);
+                    builder.queryParam(OAuth2Constants.CODE, accessCode);
+                    builder.queryParam("key", clientSession.getNote(Constants.VERIFY_EMAIL_KEY));
 
                     String link = builder.build(realm.getName()).toString();
                     long expiration = TimeUnit.SECONDS.toMinutes(realm.getAccessCodeLifespanUserAction());
@@ -531,6 +533,12 @@ public class FreeMarkerLoginFormsProvider implements LoginFormsProvider {
         return this;
     }
 
+    @Override
+    public LoginFormsProvider setClientSession(ClientSessionModel clientSession) {
+        this.clientSession = clientSession;
+        return this;
+    }
+
     @Override
     public LoginFormsProvider setAccessRequest(List<RoleModel> realmRolesRequested, MultivaluedMap<String, RoleModel> resourceRolesRequested, List<ProtocolMapperModel> protocolMappersRequested) {
         this.realmRolesRequested = realmRolesRequested;

model/api/src/main/java/org/keycloak/models/Constants.java

@@ -22,4 +22,6 @@ public interface Constants {
 
     // 30 days
     int DEFAULT_OFFLINE_SESSION_IDLE_TIMEOUT = 2592000;
+
+    public static final String VERIFY_EMAIL_KEY = "VERIFY_EMAIL_KEY";
 }

services/src/main/java/org/keycloak/authentication/requiredactions/VerifyEmail.java

@@ -8,9 +8,12 @@ import org.keycloak.authentication.RequiredActionProvider;
 import org.keycloak.events.Details;
 import org.keycloak.events.EventType;
 import org.keycloak.login.LoginFormsProvider;
+import org.keycloak.models.ClientSessionModel;
+import org.keycloak.models.Constants;
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.KeycloakSessionFactory;
 import org.keycloak.models.UserModel;
+import org.keycloak.models.utils.HmacOTP;
 import org.keycloak.services.resources.LoginActionsService;
 import org.keycloak.services.validation.Validation;
 
@@ -44,8 +47,11 @@ public class VerifyEmail implements RequiredActionProvider, RequiredActionFactor
         context.getEvent().clone().event(EventType.SEND_VERIFY_EMAIL).detail(Details.EMAIL, context.getUser().getEmail()).success();
         LoginActionsService.createActionCookie(context.getRealm(), context.getUriInfo(), context.getConnection(), context.getUserSession().getId());
 
+        setupKey(context.getClientSession());
+
         LoginFormsProvider loginFormsProvider = context.getSession().getProvider(LoginFormsProvider.class)
                 .setClientSessionCode(context.generateCode())
+                .setClientSession(context.getClientSession())
                 .setUser(context.getUser());
         Response challenge = loginFormsProvider.createResponse(UserModel.RequiredAction.VERIFY_EMAIL);
         context.challenge(challenge);
@@ -87,4 +93,9 @@ public class VerifyEmail implements RequiredActionProvider, RequiredActionFactor
     public String getId() {
         return UserModel.RequiredAction.VERIFY_EMAIL.name();
     }
+
+    public static void setupKey(ClientSessionModel clientSession) {
+        String secret = HmacOTP.generateSecret(10);
+        clientSession.setNote(Constants.VERIFY_EMAIL_KEY, secret);
+    }
 }

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

@@ -23,6 +23,8 @@ package org.keycloak.services.resources;
 
 import org.jboss.logging.Logger;
 import org.jboss.resteasy.spi.HttpRequest;
+import org.keycloak.authentication.AuthenticationFlowError;
+import org.keycloak.authentication.requiredactions.VerifyEmail;
 import org.keycloak.common.ClientConnection;
 import org.keycloak.OAuth2Constants;
 import org.keycloak.authentication.AuthenticationProcessor;
@@ -49,6 +51,7 @@ import org.keycloak.models.UserModel;
 import org.keycloak.models.UserModel.RequiredAction;
 import org.keycloak.models.UserSessionModel;
 import org.keycloak.models.utils.FormMessage;
+import org.keycloak.models.utils.HmacOTP;
 import org.keycloak.models.utils.KeycloakModelUtils;
 import org.keycloak.protocol.LoginProtocol;
 import org.keycloak.protocol.RestartLoginCookie;
@@ -533,7 +536,7 @@ public class LoginActionsService {
         event.event(EventType.VERIFY_EMAIL);
         if (key != null) {
             Checks checks = new Checks();
-            if (!checks.verifyCode(key, ClientSessionModel.Action.REQUIRED_ACTIONS.name())) {
+            if (!checks.verifyCode(code, ClientSessionModel.Action.REQUIRED_ACTIONS.name())) {
                 return checks.response;
             }
             ClientSessionCode accessCode = checks.clientCode;
@@ -547,11 +550,21 @@ public class LoginActionsService {
             UserSessionModel userSession = clientSession.getUserSession();
             UserModel user = userSession.getUser();
             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)) {
+                logger.error("Invalid key for email verification");
+                event.error(Errors.INVALID_USER_CREDENTIALS);
+                throw new WebApplicationException(ErrorPage.error(session, Messages.INVALID_CODE));
+            }
+
             user.setEmailVerified(true);
 
             user.removeRequiredAction(RequiredAction.VERIFY_EMAIL);
 
-            event.event(EventType.VERIFY_EMAIL).detail(Details.EMAIL, user.getEmail()).success();
+            event.success();
 
             String actionCookieValue = getActionCookie();
             if (actionCookieValue == null || !actionCookieValue.equals(userSession.getId())) {
@@ -576,8 +589,11 @@ public class LoginActionsService {
 
             createActionCookie(realm, uriInfo, clientConnection, userSession.getId());
 
+            VerifyEmail.setupKey(clientSession);
+
             return session.getProvider(LoginFormsProvider.class)
                     .setClientSessionCode(accessCode.getCode())
+                    .setClientSession(clientSession)
                     .setUser(userSession.getUser())
                     .createResponse(RequiredAction.VERIFY_EMAIL);
         }

testsuite/integration/src/test/java/org/keycloak/testsuite/actions/RequiredActionEmailVerificationTest.java

@@ -26,7 +26,9 @@ import org.junit.Before;
 import org.junit.ClassRule;
 import org.junit.Rule;
 import org.junit.Test;
+import org.keycloak.common.util.KeycloakUriBuilder;
 import org.keycloak.events.Details;
+import org.keycloak.events.Errors;
 import org.keycloak.events.Event;
 import org.keycloak.events.EventType;
 import org.keycloak.models.RealmModel;
@@ -130,7 +132,7 @@ public class RequiredActionEmailVerificationTest {
 
         String mailCodeId = sendEvent.getDetails().get(Details.CODE_ID);
 
-        Assert.assertEquals(mailCodeId, verificationUrl.split("key=")[1].split("\\.")[1]);
+        Assert.assertEquals(mailCodeId, verificationUrl.split("code=")[1].split("\\&")[0].split("\\.")[1]);
 
         driver.navigate().to(verificationUrl.trim());
 
@@ -223,7 +225,7 @@ public class RequiredActionEmailVerificationTest {
 
         String mailCodeId = sendEvent.getDetails().get(Details.CODE_ID);
 
-        Assert.assertEquals(mailCodeId, verificationUrl.split("key=")[1].split("\\.")[1]);
+        Assert.assertEquals(mailCodeId, verificationUrl.split("code=")[1].split("\\&")[0].split("\\.")[1]);
 
         driver.manage().deleteAllCookies();
 
@@ -239,6 +241,42 @@ public class RequiredActionEmailVerificationTest {
         assertTrue(loginPage.isCurrent());
     }
 
+
+    @Test
+    public void verifyInvalidKeyOrCode() throws IOException, MessagingException {
+        loginPage.open();
+        loginPage.login("test-user@localhost", "password");
+
+        Assert.assertTrue(verifyEmailPage.isCurrent());
+        String resendEmailLink = verifyEmailPage.getResendEmailLink();
+        String keyInsteadCodeURL = resendEmailLink.replace("code=", "key=");
+
+        AssertEvents.ExpectedEvent emailEvent = events.expectRequiredAction(EventType.SEND_VERIFY_EMAIL).detail("email", "test-user@localhost");
+        Event sendEvent = emailEvent.assertEvent();
+        String sessionId = sendEvent.getSessionId();
+        String mailCodeId = sendEvent.getDetails().get(Details.CODE_ID);
+
+        driver.navigate().to(keyInsteadCodeURL);
+
+        events.expectRequiredAction(EventType.VERIFY_EMAIL_ERROR)
+                .error(Errors.INVALID_CODE)
+                .client((String)null)
+                .user((String)null)
+                .session((String)null)
+                .clearDetails()
+                .assertEvent();
+
+        String badKeyURL = KeycloakUriBuilder.fromUri(resendEmailLink).queryParam("key", "foo").build().toString();
+        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)
+                .assertEvent();
+    }
+    
     private String getPasswordResetEmailLink(MimeMessage message) throws IOException, MessagingException {
     	Multipart multipart = (Multipart) message.getContent();
     	

testsuite/integration/src/test/java/org/keycloak/testsuite/pages/VerifyEmailPage.java

@@ -50,4 +50,8 @@ public class VerifyEmailPage extends AbstractPage {
         resendEmailLink.click();
     }
 
+    public String getResendEmailLink() {
+        return resendEmailLink.getAttribute("href");
+    }
+
 }