Merge pull request #1877 from stianst/KEYCLOAK-2129

KEYCLOAK-2129

services/src/main/java/org/keycloak/authentication/AuthenticationProcessor.java

@@ -754,7 +754,7 @@ public class AuthenticationProcessor {
         if (!code.isValidAction(action)) {
             throw new AuthenticationFlowException(AuthenticationFlowError.INVALID_CLIENT_SESSION);
         }
-        if (!code.isActionActive(action)) {
+        if (!code.isActionActive(ClientSessionCode.ActionType.LOGIN)) {
             throw new AuthenticationFlowException(AuthenticationFlowError.EXPIRED_CODE);
         }
         clientSession.setTimestamp(Time.currentTime());

services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java

@@ -202,7 +202,7 @@ public class TokenEndpoint {
 
         ClientSessionModel clientSession = accessCode.getClientSession();
         event.detail(Details.CODE_ID, clientSession.getId());
-        if (!accessCode.isValid(ClientSessionModel.Action.CODE_TO_TOKEN.name())) {
+        if (!accessCode.isValid(ClientSessionModel.Action.CODE_TO_TOKEN.name(), ClientSessionCode.ActionType.CLIENT)) {
             event.error(Errors.INVALID_CODE);
             throw new ErrorResponseException("invalid_grant", "Code is expired", Response.Status.BAD_REQUEST);
         }

services/src/main/java/org/keycloak/services/managers/ClientSessionCode.java

@@ -27,6 +27,12 @@ public class ClientSessionCode {
     private final RealmModel realm;
     private final ClientSessionModel clientSession;
 
+    public enum ActionType {
+        CLIENT,
+        LOGIN,
+        USER
+    }
+
     public ClientSessionCode(RealmModel realm, ClientSessionModel clientSession) {
         this.realm = realm;
         this.clientSession = clientSession;
@@ -128,23 +134,29 @@ public class ClientSessionCode {
         return clientSession;
     }
 
-    public boolean isValid(String requestedAction) {
+    public boolean isValid(String requestedAction, ActionType actionType) {
         if (!isValidAction(requestedAction)) return false;
-        return isActionActive(requestedAction);
+        return isActionActive(actionType);
     }
 
-    public boolean isActionActive(String requestedAction) {
+    public boolean isActionActive(ActionType actionType) {
         int timestamp = clientSession.getTimestamp();
 
         int lifespan;
-        if (requestedAction.equals(ClientSessionModel.Action.CODE_TO_TOKEN.name())) {
+        switch (actionType) {
+            case CLIENT:
                 lifespan = realm.getAccessCodeLifespan();
-
+                break;
-        } else if (requestedAction.equals(ClientSessionModel.Action.AUTHENTICATE.name())) {
+            case LOGIN:
                 lifespan = realm.getAccessCodeLifespanLogin() > 0 ? realm.getAccessCodeLifespanLogin() : realm.getAccessCodeLifespanUserAction();
-        } else {
+                break;
+            case USER:
                 lifespan = realm.getAccessCodeLifespanUserAction();
+                break;
+            default:
+                throw new IllegalArgumentException();
         }
+
         return timestamp + lifespan > Time.currentTime();
     }
 

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

@@ -446,7 +446,7 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal
     @Override
     public Response cancelled(String code) {
         ClientSessionCode clientCode = ClientSessionCode.parse(code, this.session, this.realmModel);
-        if (clientCode.getClientSession() == null || !clientCode.isValid(AUTHENTICATE.name())) {
+        if (clientCode.getClientSession() == null || !clientCode.isValid(AUTHENTICATE.name(), ClientSessionCode.ActionType.LOGIN)) {
             return redirectToErrorPage(Messages.INVALID_CODE);
         }
 
@@ -456,7 +456,7 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal
     @Override
     public Response error(String code, String message) {
         ClientSessionCode clientCode = ClientSessionCode.parse(code, this.session, this.realmModel);
-        if (clientCode.getClientSession() == null || !clientCode.isValid(AUTHENTICATE.name())) {
+        if (clientCode.getClientSession() == null || !clientCode.isValid(AUTHENTICATE.name(), ClientSessionCode.ActionType.LOGIN)) {
             return redirectToErrorPage(Messages.INVALID_CODE);
         }
         return browserAuthentication(clientCode.getClientSession(), message);
@@ -522,7 +522,7 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal
     private ClientSessionCode parseClientSessionCode(String code) {
         ClientSessionCode clientCode = ClientSessionCode.parse(code, this.session, this.realmModel);
 
-        if (clientCode != null && clientCode.isValid(AUTHENTICATE.name())) {
+        if (clientCode != null && clientCode.isValid(AUTHENTICATE.name(), ClientSessionCode.ActionType.LOGIN)) {
             ClientSessionModel clientSession = clientCode.getClientSession();
 
             if (clientSession != null) {

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

@@ -165,25 +165,25 @@ public class LoginActionsService {
         ClientSessionCode clientCode;
         Response response;
 
-        boolean verifyCode(String code, String requiredAction) {
+        boolean verifyCode(String code, String requiredAction, ClientSessionCode.ActionType actionType) {
             if (!verifyCode(code)) {
                 return false;
             }
-            if (!verifyAction(requiredAction)) {
+            if (!verifyAction(requiredAction, actionType)) {
                 return false;
             } else {
                 return true;
             }
         }
 
-        public boolean verifyAction(String requiredAction) {
+        public boolean verifyAction(String requiredAction, ClientSessionCode.ActionType actionType) {
             if (!clientCode.isValidAction(requiredAction)) {
                 event.client(clientCode.getClientSession().getClient());
                 event.error(Errors.INVALID_CODE);
                 response = ErrorPage.error(session, Messages.INVALID_CODE);
                 return false;
             }
-            if (!clientCode.isActionActive(requiredAction)) {
+            if (!clientCode.isActionActive(actionType)) {
                 event.client(clientCode.getClientSession().getClient());
                 event.clone().error(Errors.EXPIRED_CODE);
                 if (clientCode.getClientSession().getAction().equals(ClientSessionModel.Action.AUTHENTICATE.name())) {
@@ -264,7 +264,7 @@ public class LoginActionsService {
                                  @QueryParam("execution") String execution) {
         event.event(EventType.LOGIN);
         Checks checks = new Checks();
-        if (!checks.verifyCode(code, ClientSessionModel.Action.AUTHENTICATE.name())) {
+        if (!checks.verifyCode(code, ClientSessionModel.Action.AUTHENTICATE.name(), ClientSessionCode.ActionType.LOGIN)) {
             return checks.response;
         }
         event.detail(Details.CODE_ID, code);
@@ -315,7 +315,7 @@ public class LoginActionsService {
                                      @QueryParam("execution") String execution) {
         event.event(EventType.LOGIN);
         Checks checks = new Checks();
-        if (!checks.verifyCode(code, ClientSessionModel.Action.AUTHENTICATE.name())) {
+        if (!checks.verifyCode(code, ClientSessionModel.Action.AUTHENTICATE.name(), ClientSessionCode.ActionType.LOGIN)) {
             return checks.response;
         }
         final ClientSessionCode clientCode = checks.clientCode;
@@ -374,7 +374,7 @@ public class LoginActionsService {
     protected Response resetCredentials(String code, String execution) {
         event.event(EventType.RESET_PASSWORD);
         Checks checks = new Checks();
-        if (!checks.verifyCode(code, ClientSessionModel.Action.AUTHENTICATE.name())) {
+        if (!checks.verifyCode(code, ClientSessionModel.Action.AUTHENTICATE.name(), ClientSessionCode.ActionType.USER)) {
             return checks.response;
         }
         final ClientSessionCode clientCode = checks.clientCode;
@@ -438,7 +438,7 @@ public class LoginActionsService {
         }
 
         Checks checks = new Checks();
-        if (!checks.verifyCode(code, ClientSessionModel.Action.AUTHENTICATE.name())) {
+        if (!checks.verifyCode(code, ClientSessionModel.Action.AUTHENTICATE.name(), ClientSessionCode.ActionType.LOGIN)) {
             return checks.response;
         }
         event.detail(Details.CODE_ID, code);
@@ -468,7 +468,7 @@ public class LoginActionsService {
             return ErrorPage.error(session, Messages.REGISTRATION_NOT_ALLOWED);
         }
         Checks checks = new Checks();
-        if (!checks.verifyCode(code, ClientSessionModel.Action.AUTHENTICATE.name())) {
+        if (!checks.verifyCode(code, ClientSessionModel.Action.AUTHENTICATE.name(), ClientSessionCode.ActionType.LOGIN)) {
             return checks.response;
         }
 
@@ -496,7 +496,7 @@ public class LoginActionsService {
         event.event(EventType.IDENTITY_PROVIDER_FIRST_LOGIN);
 
         Checks checks = new Checks();
-        if (!checks.verifyCode(code, ClientSessionModel.Action.AUTHENTICATE.name())) {
+        if (!checks.verifyCode(code, ClientSessionModel.Action.AUTHENTICATE.name(), ClientSessionCode.ActionType.LOGIN)) {
             return checks.response;
         }
         event.detail(Details.CODE_ID, code);
@@ -556,7 +556,7 @@ public class LoginActionsService {
         String code = formData.getFirst("code");
 
         ClientSessionCode accessCode = ClientSessionCode.parse(code, session, realm);
-        if (accessCode == null || !accessCode.isValid(ClientSessionModel.Action.OAUTH_GRANT.name())) {
+        if (accessCode == null || !accessCode.isValid(ClientSessionModel.Action.OAUTH_GRANT.name(), ClientSessionCode.ActionType.LOGIN)) {
             event.error(Errors.INVALID_CODE);
             return ErrorPage.error(session, Messages.INVALID_ACCESS_CODE);
         }
@@ -622,7 +622,7 @@ public class LoginActionsService {
         event.event(EventType.VERIFY_EMAIL);
         if (key != null) {
             Checks checks = new Checks();
-            if (!checks.verifyCode(code, ClientSessionModel.Action.REQUIRED_ACTIONS.name())) {
+            if (!checks.verifyCode(code, ClientSessionModel.Action.REQUIRED_ACTIONS.name(), ClientSessionCode.ActionType.USER)) {
                 return checks.response;
             }
             ClientSessionCode accessCode = checks.clientCode;
@@ -665,7 +665,7 @@ public class LoginActionsService {
             return AuthenticationProcessor.createRequiredActionRedirect(realm, clientSession, uriInfo);
         } else {
             Checks checks = new Checks();
-            if (!checks.verifyCode(code, ClientSessionModel.Action.REQUIRED_ACTIONS.name())) {
+            if (!checks.verifyCode(code, ClientSessionModel.Action.REQUIRED_ACTIONS.name(), ClientSessionCode.ActionType.USER)) {
                 return checks.response;
             }
             ClientSessionCode accessCode = checks.clientCode;
@@ -697,7 +697,7 @@ public class LoginActionsService {
         event.event(EventType.EXECUTE_ACTIONS);
         if (key != null) {
             Checks checks = new Checks();
-            if (!checks.verifyCode(key, ClientSessionModel.Action.EXECUTE_ACTIONS.name())) {
+            if (!checks.verifyCode(key, ClientSessionModel.Action.EXECUTE_ACTIONS.name(), ClientSessionCode.ActionType.USER)) {
                 return checks.response;
             }
             ClientSessionModel clientSession = checks.clientCode.getClientSession();
@@ -767,7 +767,7 @@ public class LoginActionsService {
         event.event(EventType.CUSTOM_REQUIRED_ACTION);
         event.detail(Details.CUSTOM_REQUIRED_ACTION, action);
         Checks checks = new Checks();
-        if (!checks.verifyCode(code, ClientSessionModel.Action.REQUIRED_ACTIONS.name())) {
+        if (!checks.verifyCode(code, ClientSessionModel.Action.REQUIRED_ACTIONS.name(), ClientSessionCode.ActionType.USER)) {
             return checks.response;
         }
         final ClientSessionCode clientCode = checks.clientCode;

social/twitter/src/main/java/org/keycloak/social/twitter/TwitterIdentityProvider.java

@@ -166,7 +166,7 @@ public class TwitterIdentityProvider extends AbstractIdentityProvider<OAuth2Iden
         private ClientSessionCode parseClientSessionCode(String code) {
             ClientSessionCode clientCode = ClientSessionCode.parse(code, this.session, this.realm);
 
-            if (clientCode != null && clientCode.isValid(AUTHENTICATE.name())) {
+            if (clientCode != null && clientCode.isValid(AUTHENTICATE.name(), ClientSessionCode.ActionType.LOGIN)) {
                 ClientSessionModel clientSession = clientCode.getClientSession();
 
                 if (clientSession != null) {

testsuite/integration/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java

@@ -21,10 +21,7 @@
  */
 package org.keycloak.testsuite.forms;
 
-import org.junit.Before;
+import org.junit.*;
-import org.junit.ClassRule;
-import org.junit.Rule;
-import org.junit.Test;
 import org.keycloak.events.Details;
 import org.keycloak.events.Errors;
 import org.keycloak.events.Event;
@@ -39,13 +36,8 @@ import org.keycloak.testsuite.AssertEvents;
 import org.keycloak.testsuite.Constants;
 import org.keycloak.testsuite.MailUtil;
 import org.keycloak.testsuite.OAuthClient;
-import org.keycloak.testsuite.pages.AppPage;
+import org.keycloak.testsuite.pages.*;
 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.LoginPasswordResetPage;
-import org.keycloak.testsuite.pages.LoginPasswordUpdatePage;
 import org.keycloak.testsuite.rule.GreenMailRule;
 import org.keycloak.testsuite.rule.KeycloakRule;
 import org.keycloak.testsuite.rule.WebResource;
@@ -59,6 +51,7 @@ import javax.mail.internet.MimeMessage;
 
 import java.io.IOException;
 import java.util.Collections;
+import java.util.concurrent.atomic.AtomicInteger;
 
 import static org.junit.Assert.*;
 
@@ -114,6 +107,9 @@ public class ResetPasswordTest {
     @WebResource
     protected InfoPage infoPage;
 
+    @WebResource
+    protected VerifyEmailPage verifyEmailPage;
+
     @WebResource
     protected LoginPasswordResetPage resetPasswordPage;
 
@@ -422,6 +418,59 @@ public class ResetPasswordTest {
         }
     }
 
+    @Test
+    public void resetPasswordExpiredCodeShort() throws IOException, MessagingException, InterruptedException {
+        final AtomicInteger originalValue = new AtomicInteger();
+        keycloakRule.configure(new KeycloakRule.KeycloakSetup() {
+            @Override
+            public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) {
+                originalValue.set(appRealm.getAccessCodeLifespan());
+                appRealm.setAccessCodeLifespanUserAction(60);
+            }
+        });
+
+        try {
+            loginPage.open();
+            loginPage.resetPassword();
+
+            resetPasswordPage.assertCurrent();
+
+            resetPasswordPage.changePassword("login-test");
+
+            loginPage.assertCurrent();
+            assertEquals("You should receive an email shortly with further instructions.", loginPage.getSuccessMessage());
+
+            events.expectRequiredAction(EventType.SEND_RESET_PASSWORD)
+                    .session((String)null)
+                    .user(userId).detail(Details.USERNAME, "login-test").detail(Details.EMAIL, "login@test.com").assertEvent();
+
+            assertEquals(1, greenMail.getReceivedMessages().length);
+
+            MimeMessage message = greenMail.getReceivedMessages()[0];
+
+            String changePasswordUrl = getPasswordResetEmailLink(message);
+
+            Time.setOffset(70);
+
+            driver.navigate().to(changePasswordUrl.trim());
+
+            loginPage.assertCurrent();
+
+            assertEquals("You took too long to login. Login process starting from beginning.", loginPage.getError());
+
+            events.expectRequiredAction(EventType.RESET_PASSWORD).error("expired_code").client("test-app").user((String) null).session((String) null).clearDetails().assertEvent();
+        } finally {
+            Time.setOffset(0);
+
+            keycloakRule.configure(new KeycloakRule.KeycloakSetup() {
+                @Override
+                public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) {
+                    appRealm.setAccessCodeLifespanUserAction(originalValue.get());
+                }
+            });
+        }
+    }
+
     @Test
     public void resetPasswordDisabledUser() throws IOException, MessagingException, InterruptedException {
         keycloakRule.configure(new KeycloakRule.KeycloakSetup() {