keycloak-aplcache

KEYCLOAK-2011

10/23/2015 6:05:27 PM

Details

diff --git a/forms/login-api/src/main/java/org/keycloak/login/LoginFormsProvider.java b/forms/login-api/src/main/java/org/keycloak/login/LoginFormsProvider.java
index cb5f2be..fccfce1 100755
--- a/forms/login-api/src/main/java/org/keycloak/login/LoginFormsProvider.java
+++ b/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);
 
diff --git a/forms/login-freemarker/src/main/java/org/keycloak/login/freemarker/FreeMarkerLoginFormsProvider.java b/forms/login-freemarker/src/main/java/org/keycloak/login/freemarker/FreeMarkerLoginFormsProvider.java
index 7fc1bcd..6125d0b 100755
--- a/forms/login-freemarker/src/main/java/org/keycloak/login/freemarker/FreeMarkerLoginFormsProvider.java
+++ b/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());
@@ -532,6 +534,12 @@ public class FreeMarkerLoginFormsProvider implements LoginFormsProvider {
     }
 
     @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;
         this.resourceRolesRequested = resourceRolesRequested;
diff --git a/model/api/src/main/java/org/keycloak/models/Constants.java b/model/api/src/main/java/org/keycloak/models/Constants.java
index 43bdc7d..8977def 100755
--- a/model/api/src/main/java/org/keycloak/models/Constants.java
+++ b/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";
 }
diff --git a/services/src/main/java/org/keycloak/authentication/requiredactions/VerifyEmail.java b/services/src/main/java/org/keycloak/authentication/requiredactions/VerifyEmail.java
index 01ddcff..7fc77a8 100755
--- a/services/src/main/java/org/keycloak/authentication/requiredactions/VerifyEmail.java
+++ b/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);
+    }
 }
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 46bc553..20de78b 100755
--- a/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java
+++ b/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);
         }
diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/actions/RequiredActionEmailVerificationTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/actions/RequiredActionEmailVerificationTest.java
index a9c0c58..c7f075f 100755
--- a/testsuite/integration/src/test/java/org/keycloak/testsuite/actions/RequiredActionEmailVerificationTest.java
+++ b/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();
 
@@ -238,6 +240,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();
diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/pages/VerifyEmailPage.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/pages/VerifyEmailPage.java
index cfcfbb4..9968ce1 100644
--- a/testsuite/integration/src/test/java/org/keycloak/testsuite/pages/VerifyEmailPage.java
+++ b/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");
+    }
+
 }