[KEYCLOAK-2052] Allows independently set timeouts for e-mail verifica…

…tion link and rest e.g. forgot password link Co-authored-by: Hynek Mlnarik <hmlnarik@redhat.com>
filipeferraz · Nov 13, 2017 · 03d0488 · 03d0488
1 parent 925d5e1
commit 03d0488
Show file tree

Hide file tree

Showing 20 changed files with 695 additions and 59 deletions.
diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmAdapter.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmAdapter.java
@@ -454,6 +454,12 @@ public void setAccessCodeLifespanUserAction(int seconds) {
         updated.setAccessCodeLifespanUserAction(seconds);
     }
 
+    @Override
+    public Map<String, Integer> getUserActionTokenLifespans() {
+        if (isUpdated()) return updated.getUserActionTokenLifespans();
+        return cached.getUserActionTokenLifespans();
+    }
+
     @Override
     public int getAccessCodeLifespanLogin() {
         if (isUpdated()) return updated.getAccessCodeLifespanLogin();
@@ -490,6 +496,20 @@ public void setActionTokenGeneratedByUserLifespan(int seconds) {
         updated.setActionTokenGeneratedByUserLifespan(seconds);
     }
 
+    @Override
+    public int getActionTokenGeneratedByUserLifespan(String actionTokenId) {
+        if (isUpdated()) return updated.getActionTokenGeneratedByUserLifespan(actionTokenId);
+        return cached.getActionTokenGeneratedByUserLifespan(actionTokenId);
+    }
+
+    @Override
+    public void setActionTokenGeneratedByUserLifespan(String actionTokenId, Integer seconds) {
+        if (seconds != null) {
+            getDelegateForUpdate();
+            updated.setActionTokenGeneratedByUserLifespan(actionTokenId, seconds);
+        }
+    }
+
     @Override
     public List<RequiredCredentialModel> getRequiredCredentials() {
         if (isUpdated()) return updated.getRequiredCredentials();

diff --git a/...l/infinispan/src/main/java/org/keycloak/models/cache/infinispan/entities/CachedRealm.java b/...l/infinispan/src/main/java/org/keycloak/models/cache/infinispan/entities/CachedRealm.java
@@ -143,6 +143,8 @@ public Set<IdentityProviderMapperModel> getIdentityProviderMapperSet() {
 
     protected Map<String, String> attributes;
 
+    private Map<String, Integer> userActionTokenLifespans;
+
     public CachedRealm(Long revision, RealmModel model) {
         super(revision, model.getId());
         name = model.getName();
@@ -192,6 +194,7 @@ public CachedRealm(Long revision, RealmModel model) {
         emailTheme = model.getEmailTheme();
 
         requiredCredentials = model.getRequiredCredentials();
+        userActionTokenLifespans = Collections.unmodifiableMap(new HashMap<>(model.getUserActionTokenLifespans()));
 
         this.identityProviders = new ArrayList<>();
 
@@ -407,6 +410,11 @@ public int getAccessCodeLifespan() {
     public int getAccessCodeLifespanUserAction() {
         return accessCodeLifespanUserAction;
     }
+
+    public Map<String, Integer> getUserActionTokenLifespans() {
+        return userActionTokenLifespans;
+    }
+
     public int getAccessCodeLifespanLogin() {
         return accessCodeLifespanLogin;
     }
@@ -419,6 +427,18 @@ public int getActionTokenGeneratedByUserLifespan() {
         return actionTokenGeneratedByUserLifespan;
     }
 
+    /**
+     * This method is supposed to return user lifespan based on the action token ID
+     * provided. If nothing is provided, it will return the default lifespan.
+     * @param actionTokenId
+     * @return lifespan
+     */
+    public int getActionTokenGeneratedByUserLifespan(String actionTokenId) {
+        if (actionTokenId == null || this.userActionTokenLifespans.get(actionTokenId) == null)
+            return getActionTokenGeneratedByUserLifespan();
+        return this.userActionTokenLifespans.get(actionTokenId);
+    }
+
     public List<RequiredCredentialModel> getRequiredCredentials() {
         return requiredCredentials;
     }
@@ -609,5 +629,4 @@ public Boolean getAttribute(String name, Boolean defaultValue) {
     public Map<String, String> getAttributes() {
         return attributes;
     }
-
 }
diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java b/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java
@@ -473,6 +473,19 @@ public void setAccessCodeLifespanUserAction(int accessCodeLifespanUserAction) {
         em.flush();
     }
 
+    @Override
+    public Map<String, Integer> getUserActionTokenLifespans() {
+
+        Map<String, Integer> userActionTokens = new HashMap<>();
+
+        getAttributes().entrySet().stream()
+                .filter(Objects::nonNull)
+                .filter(entry -> entry.getKey().startsWith(RealmAttributes.ACTION_TOKEN_GENERATED_BY_USER_LIFESPAN + "."))
+                .forEach(entry -> userActionTokens.put(entry.getKey().substring(RealmAttributes.ACTION_TOKEN_GENERATED_BY_USER_LIFESPAN.length() + 1), Integer.valueOf(entry.getValue())));
+
+        return Collections.unmodifiableMap(userActionTokens);
+    }
+
     @Override
     public int getAccessCodeLifespanLogin() {
         return realm.getAccessCodeLifespanLogin();
@@ -504,6 +517,17 @@ public void setActionTokenGeneratedByUserLifespan(int actionTokenGeneratedByUser
         setAttribute(RealmAttributes.ACTION_TOKEN_GENERATED_BY_USER_LIFESPAN, actionTokenGeneratedByUserLifespan);
     }
 
+    @Override
+    public int getActionTokenGeneratedByUserLifespan(String actionTokenId) {
+        return getAttribute(RealmAttributes.ACTION_TOKEN_GENERATED_BY_USER_LIFESPAN + "." + actionTokenId, getAccessCodeLifespanUserAction());
+    }
+
+    @Override
+    public void setActionTokenGeneratedByUserLifespan(String actionTokenId, Integer actionTokenGeneratedByUserLifespan) {
+        if (actionTokenGeneratedByUserLifespan != null)
+            setAttribute(RealmAttributes.ACTION_TOKEN_GENERATED_BY_USER_LIFESPAN + "." + actionTokenId, actionTokenGeneratedByUserLifespan);
+    }
+
     protected RequiredCredentialModel initRequiredCredentialModel(String type) {
         RequiredCredentialModel model = RequiredCredentialModel.BUILT_IN.get(type);
         if (model == null) {
@@ -647,7 +671,7 @@ public void removeDefaultRoles(String... defaultRoles) {
             entities.remove(entity);
         }
         em.flush();
-     }
+    }
 
     @Override
     public List<GroupModel> getDefaultGroups() {
@@ -1802,7 +1826,7 @@ public ComponentModel addComponentModel(ComponentModel model) {
 
     /**
      * This just exists for testing purposes
-     *
+     * 
      */
     public static final String COMPONENT_PROVIDER_EXISTS_DISABLED = "component.provider.exists.disabled";
 
@@ -1954,4 +1978,5 @@ public ComponentModel getComponent(String id) {
         if (c == null) return null;
         return entityToModel(c);
     }
+
 }
diff --git a/server-spi/src/main/java/org/keycloak/models/RealmModel.java b/server-spi/src/main/java/org/keycloak/models/RealmModel.java
@@ -186,6 +186,13 @@ interface IdentityProviderRemovedEvent extends ProviderEvent {
 
     void setAccessCodeLifespanUserAction(int seconds);
 
+    /**
+     * This method will return a map with all the lifespans available
+     * or an empty map, but never null.
+     * @return map with user action token lifespans
+     */
+    Map<String, Integer> getUserActionTokenLifespans();
+
     int getAccessCodeLifespanLogin();
 
     void setAccessCodeLifespanLogin(int seconds);
@@ -196,6 +203,9 @@ interface IdentityProviderRemovedEvent extends ProviderEvent {
     int getActionTokenGeneratedByUserLifespan();
     void setActionTokenGeneratedByUserLifespan(int seconds);
 
+    int getActionTokenGeneratedByUserLifespan(String actionTokenType);
+    void setActionTokenGeneratedByUserLifespan(String actionTokenType, Integer seconds);
+
     List<RequiredCredentialModel> getRequiredCredentials();
 
     void addRequiredCredential(String cred);

diff --git a/.../org/keycloak/authentication/authenticators/broker/IdpEmailVerificationAuthenticator.java b/.../org/keycloak/authentication/authenticators/broker/IdpEmailVerificationAuthenticator.java
@@ -116,7 +116,7 @@ private void sendVerifyEmail(KeycloakSession session, AuthenticationFlowContext
         UriInfo uriInfo = session.getContext().getUri();
         AuthenticationSessionModel authSession = context.getAuthenticationSession();
 
-        int validityInSecs = realm.getActionTokenGeneratedByUserLifespan();
+        int validityInSecs = realm.getActionTokenGeneratedByUserLifespan(IdpVerifyAccountLinkActionToken.TOKEN_TYPE);
         int absoluteExpirationInSecs = Time.currentTime() + validityInSecs;
 
         EventBuilder event = context.getEvent().clone().event(EventType.SEND_IDENTITY_PROVIDER_LINK)

diff --git a/.../main/java/org/keycloak/authentication/authenticators/resetcred/ResetCredentialEmail.java b/.../main/java/org/keycloak/authentication/authenticators/resetcred/ResetCredentialEmail.java
@@ -85,7 +85,7 @@ public void authenticate(AuthenticationFlowContext context) {
             return;
         }
 
-        int validityInSecs = context.getRealm().getActionTokenGeneratedByUserLifespan();
+        int validityInSecs = context.getRealm().getActionTokenGeneratedByUserLifespan(ResetCredentialsActionToken.TOKEN_TYPE);
         int absoluteExpirationInSecs = Time.currentTime() + validityInSecs;
 
         // We send the secret in the email in a link as a query param.

diff --git a/services/src/main/java/org/keycloak/authentication/requiredactions/VerifyEmail.java b/services/src/main/java/org/keycloak/authentication/requiredactions/VerifyEmail.java
@@ -131,7 +131,7 @@ private Response sendVerifyEmail(KeycloakSession session, LoginFormsProvider for
         RealmModel realm = session.getContext().getRealm();
         UriInfo uriInfo = session.getContext().getUri();
 
-        int validityInSecs = realm.getActionTokenGeneratedByUserLifespan();
+        int validityInSecs = realm.getActionTokenGeneratedByUserLifespan(VerifyEmailActionToken.TOKEN_TYPE);
         int absoluteExpirationInSecs = Time.currentTime() + validityInSecs;
 
         VerifyEmailActionToken token = new VerifyEmailActionToken(user.getId(), absoluteExpirationInSecs, authSession.getId(), user.getEmail());

diff --git a/...ntegration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/MailUtils.java b/...ntegration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/MailUtils.java
@@ -56,7 +56,11 @@ public static String getPasswordResetEmailLink(MimeMessage message) throws IOExc
         assertEquals("text/html; charset=UTF-8", htmlContentType);
 
         final String htmlBody = (String) multipart.getBodyPart(1).getContent();
-        final String htmlChangePwdUrl = getLink(htmlBody);
+        // .replace() accounts for escaping the ampersand
+        // It's not escaped in the html version because html retrieved from a
+        // message bundle is considered safe and it must be unescaped to display
+        // properly.
+        final String htmlChangePwdUrl = MailUtils.getLink(htmlBody).replace("&", "&amp;");
 
         assertEquals(htmlChangePwdUrl, textChangePwdUrl);
 

diff --git a/...ase/src/test/java/org/keycloak/testsuite/actions/RequiredActionEmailVerificationTest.java b/...ase/src/test/java/org/keycloak/testsuite/actions/RequiredActionEmailVerificationTest.java
@@ -27,14 +27,12 @@
 import org.keycloak.events.Errors;
 import org.keycloak.events.EventType;
 import org.keycloak.models.Constants;
-import org.keycloak.models.UserModel;
 import org.keycloak.representations.idm.EventRepresentation;
 import org.keycloak.representations.idm.RealmRepresentation;
 import org.keycloak.representations.idm.UserRepresentation;
 import org.keycloak.testsuite.AssertEvents;
 import org.keycloak.testsuite.AbstractTestRealmKeycloakTest;
 import org.keycloak.testsuite.admin.ApiUtil;
-import org.keycloak.testsuite.auth.page.AuthRealm;
 import org.keycloak.testsuite.pages.AppPage;
 import org.keycloak.testsuite.pages.AppPage.RequestType;
 import org.keycloak.testsuite.pages.ProceedPage;
@@ -45,16 +43,19 @@
 import org.keycloak.testsuite.pages.VerifyEmailPage;
 import org.keycloak.testsuite.util.GreenMailRule;
 import org.keycloak.testsuite.util.MailUtils;
+import org.keycloak.testsuite.util.UserActionTokenBuilder;
 import org.keycloak.testsuite.util.UserBuilder;
 
 import javax.mail.MessagingException;
 import javax.mail.Multipart;
 import javax.mail.internet.MimeMessage;
 import java.io.IOException;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
 
 import org.hamcrest.Matchers;
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
 
 /**
  * @author <a href="mailto:sthorger@redhat.com">Stian Thorgersen</a>
@@ -448,6 +449,95 @@ public void verifyEmailExpiredCode() throws IOException, MessagingException {
         }
     }
 
+    @Test
+    public void verifyEmailExpiredCodedPerActionLifespan() throws IOException, MessagingException {
+        RealmRepresentation realmRep = testRealm().toRepresentation();
+        Map<String, String> originalAttributes = Collections.unmodifiableMap(new HashMap<>(realmRep.getAttributes()));
+
+        realmRep.setAttributes(UserActionTokenBuilder.create().verifyEmailLifespan(60).build());
+        testRealm().update(realmRep);
+
+        loginPage.open();
+        loginPage.login("test-user@localhost", "password");
+
+        verifyEmailPage.assertCurrent();
+
+        Assert.assertEquals(1, greenMail.getReceivedMessages().length);
+
+        MimeMessage message = greenMail.getLastReceivedMessage();
+
+        String verificationUrl = getPasswordResetEmailLink(message);
+
+        events.poll();
+
+        try {
+            setTimeOffset(70);
+
+            driver.navigate().to(verificationUrl.trim());
+
+            loginPage.assertCurrent();
+            assertEquals("Action expired. Please start again.", loginPage.getError());
+
+            events.expectRequiredAction(EventType.EXECUTE_ACTION_TOKEN_ERROR)
+                    .error(Errors.EXPIRED_CODE)
+                    .client((String)null)
+                    .user(testUserId)
+                    .session((String)null)
+                    .clearDetails()
+                    .detail(Details.ACTION, VerifyEmailActionToken.TOKEN_TYPE)
+                    .assertEvent();
+        } finally {
+            setTimeOffset(0);
+            realmRep.setAttributes(originalAttributes);
+            testRealm().update(realmRep);
+        }
+    }
+
+    @Test
+    public void verifyEmailExpiredCodedPerActionMultipleTimeouts() throws IOException, MessagingException {
+        RealmRepresentation realmRep = testRealm().toRepresentation();
+        Map<String, String> originalAttributes = Collections.unmodifiableMap(new HashMap<>(realmRep.getAttributes()));
+
+        //Make sure that one attribute settings won't affect the other
+        realmRep.setAttributes(UserActionTokenBuilder.create().verifyEmailLifespan(60).resetCredentialsLifespan(300).build());
+        testRealm().update(realmRep);
+
+        loginPage.open();
+        loginPage.login("test-user@localhost", "password");
+
+        verifyEmailPage.assertCurrent();
+
+        Assert.assertEquals(1, greenMail.getReceivedMessages().length);
+
+        MimeMessage message = greenMail.getLastReceivedMessage();
+
+        String verificationUrl = getPasswordResetEmailLink(message);
+
+        events.poll();
+
+        try {
+            setTimeOffset(70);
+
+            driver.navigate().to(verificationUrl.trim());
+
+            loginPage.assertCurrent();
+            assertEquals("Action expired. Please start again.", loginPage.getError());
+
+            events.expectRequiredAction(EventType.EXECUTE_ACTION_TOKEN_ERROR)
+                    .error(Errors.EXPIRED_CODE)
+                    .client((String)null)
+                    .user(testUserId)
+                    .session((String)null)
+                    .clearDetails()
+                    .detail(Details.ACTION, VerifyEmailActionToken.TOKEN_TYPE)
+                    .assertEvent();
+        } finally {
+            setTimeOffset(0);
+            realmRep.setAttributes(originalAttributes);
+            testRealm().update(realmRep);
+        }
+    }
+
     @Test
     public void verifyEmailExpiredCodeAndExpiredSession() throws IOException, MessagingException {
         loginPage.open();

diff --git a/...-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/BrowserButtonsTest.java b/...-arquillian/tests/base/src/test/java/org/keycloak/testsuite/forms/BrowserButtonsTest.java
@@ -47,6 +47,7 @@
 import org.keycloak.testsuite.pages.RegisterPage;
 import org.keycloak.testsuite.pages.VerifyEmailPage;
 import org.keycloak.testsuite.util.GreenMailRule;
+import org.keycloak.testsuite.util.MailUtils;
 import org.keycloak.testsuite.util.OAuthClient;
 import org.keycloak.testsuite.util.UserBuilder;
 
@@ -337,7 +338,7 @@ public void backButtonInResetPasswordFlow() throws Exception {
         // Receive email
         MimeMessage message = greenMail.getReceivedMessages()[greenMail.getReceivedMessages().length - 1];
 
-        String changePasswordUrl = ResetPasswordTest.getPasswordResetEmailLink(message);
+        String changePasswordUrl = MailUtils.getPasswordResetEmailLink(message);
 
         driver.navigate().to(changePasswordUrl.trim());