Merge pull request #1405 from patriot1burke/master

cleanup client sessions

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

@@ -29,6 +29,7 @@ public interface UserSessionProvider extends Provider {
     void removeUserSessions(RealmModel realm, UserModel user);
     void removeExpiredUserSessions(RealmModel realm);
     void removeUserSessions(RealmModel realm);
+    void removeClientSession(RealmModel realm, ClientSessionModel clientSession);
 
     UsernameLoginFailureModel getUserLoginFailure(RealmModel realm, String username);
     UsernameLoginFailureModel addUserLoginFailure(RealmModel realm, String username);

model/sessions-infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProvider.java

@@ -334,6 +334,21 @@ public class InfinispanUserSessionProvider implements UserSessionProvider {
         }
     }
 
+    @Override
+    public void removeClientSession(RealmModel realm, ClientSessionModel clientSession) {
+        UserSessionModel userSession = clientSession.getUserSession();
+        if (userSession != null)  {
+            UserSessionEntity entity = ((UserSessionAdapter) userSession).getEntity();
+            if (entity.getClientSessions() != null) {
+                entity.getClientSessions().remove(clientSession.getId());
+
+            }
+            tx.replace(sessionCache, entity.getId(), entity);
+        }
+        tx.remove(sessionCache, clientSession.getId());
+    }
+
+
     void dettachSession(UserSessionModel userSession, ClientSessionModel clientSession) {
         UserSessionEntity entity = ((UserSessionAdapter) userSession).getEntity();
         String clientSessionId = clientSession.getId();
@@ -359,6 +374,7 @@ public class InfinispanUserSessionProvider implements UserSessionProvider {
         }
     }
 
+
     InfinispanKeycloakTransaction getTx() {
         return tx;
     }

model/sessions-infinispan/src/main/java/org/keycloak/models/sessions/infinispan/entities/ClientSessionEntity.java

@@ -135,4 +135,6 @@ public class ClientSessionEntity extends SessionEntity {
     public void setUserSessionNotes(Map<String, String> userSessionNotes) {
         this.userSessionNotes = userSessionNotes;
     }
+
+
 }

model/sessions-infinispan/src/main/java/org/keycloak/models/sessions/infinispan/entities/SessionEntity.java

@@ -26,4 +26,21 @@ public class SessionEntity implements Serializable {
     public void setRealm(String realm) {
         this.realm = realm;
     }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (!(o instanceof SessionEntity)) return false;
+
+        SessionEntity that = (SessionEntity) o;
+
+        if (id != null ? !id.equals(that.id) : that.id != null) return false;
+
+        return true;
+    }
+
+    @Override
+    public int hashCode() {
+        return id != null ? id.hashCode() : 0;
+    }
 }

model/sessions-jpa/src/main/java/org/keycloak/models/sessions/jpa/ClientSessionAdapter.java

@@ -128,6 +128,10 @@ public class ClientSessionAdapter implements ClientSessionModel {
         return realm.getClientById(entity.getClientId());
     }
 
+    public ClientSessionEntity getEntity() {
+        return entity;
+    }
+
     @Override
     public void setUserSession(UserSessionModel userSession) {
         if (userSession == null) {

model/sessions-jpa/src/main/java/org/keycloak/models/sessions/jpa/JpaUserSessionProvider.java

@@ -18,6 +18,7 @@ import org.keycloak.util.Time;
 
 import javax.persistence.EntityManager;
 import javax.persistence.TypedQuery;
+import java.util.Iterator;
 import java.util.LinkedList;
 import java.util.List;
 
@@ -47,6 +48,13 @@ public class JpaUserSessionProvider implements UserSessionProvider {
         return new ClientSessionAdapter(session, em, realm, entity);
     }
 
+    @Override
+    public void removeClientSession(RealmModel realm, ClientSessionModel clientSession) {
+        ClientSessionEntity clientSessionEntity = ((ClientSessionAdapter)clientSession).getEntity();
+        em.remove(clientSessionEntity);
+        em.flush();
+   }
+
     @Override
     public ClientSessionModel getClientSession(RealmModel realm, String id) {
         ClientSessionEntity clientSession = em.find(ClientSessionEntity.class, id);

model/sessions-jpa/src/main/java/org/keycloak/models/sessions/jpa/entities/ClientSessionEntity.java

@@ -186,4 +186,21 @@ public class ClientSessionEntity {
     public void setUserSessionNotes(Collection<ClientUserSessionNoteEntity> userSessionNotes) {
         this.userSessionNotes = userSessionNotes;
     }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (!(o instanceof ClientSessionEntity)) return false;
+
+        ClientSessionEntity that = (ClientSessionEntity) o;
+
+        if (id != null ? !id.equals(that.id) : that.id != null) return false;
+
+        return true;
+    }
+
+    @Override
+    public int hashCode() {
+        return id != null ? id.hashCode() : 0;
+    }
 }

model/sessions-mem/src/main/java/org/keycloak/models/sessions/mem/ClientSessionAdapter.java

@@ -39,7 +39,9 @@ public class ClientSessionAdapter implements ClientSessionModel {
         return session.realms().getRealm(entity.getRealmId());
     }
 
-
+    public ClientSessionEntity getEntity() {
+        return entity;
+    }
 
     @Override
     public ClientModel getClient() {

model/sessions-mem/src/main/java/org/keycloak/models/sessions/mem/MemUserSessionProvider.java

@@ -58,6 +58,17 @@ public class MemUserSessionProvider implements UserSessionProvider {
         return new ClientSessionAdapter(session, this, realm, entity);
     }
 
+    @Override
+    public void removeClientSession(RealmModel realm, ClientSessionModel clientSession) {
+        ClientSessionEntity entity = ((ClientSessionAdapter)clientSession).getEntity();
+        UserSessionModel userSession = clientSession.getUserSession();
+        if (userSession != null) {
+            UserSessionEntity userSessionEntity = ((UserSessionAdapter)userSession).getEntity();
+            userSessionEntity.getClientSessions().remove(entity);
+        }
+        clientSessions.remove(clientSession.getId());
+    }
+
     @Override
     public ClientSessionModel getClientSession(RealmModel realm, String id) {
         ClientSessionEntity entity = clientSessions.get(id);

model/sessions-mem/src/main/java/org/keycloak/models/sessions/mem/entities/ClientSessionEntity.java

@@ -132,4 +132,21 @@ public class ClientSessionEntity {
     public Map<String, String> getUserSessionNotes() {
         return userSessionNotes;
     }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) return true;
+        if (!(o instanceof ClientSessionEntity)) return false;
+
+        ClientSessionEntity that = (ClientSessionEntity) o;
+
+        if (id != null ? !id.equals(that.id) : that.id != null) return false;
+
+        return true;
+    }
+
+    @Override
+    public int hashCode() {
+        return id != null ? id.hashCode() : 0;
+    }
 }

model/sessions-mongo/src/main/java/org/keycloak/models/sessions/mongo/MongoUserSessionProvider.java

@@ -55,6 +55,19 @@ public class MongoUserSessionProvider implements UserSessionProvider {
         return new ClientSessionAdapter(session, this, realm, entity, invocationContext);
     }
 
+    @Override
+    public void removeClientSession(RealmModel realm, ClientSessionModel clientSession) {
+        MongoClientSessionEntity entity = ((ClientSessionAdapter)clientSession).getMongoEntity();
+        if (entity.getSessionId() != null) {
+            MongoUserSessionEntity userSessionEntity = getUserSessionEntity(realm, entity.getSessionId());
+            getMongoStore().pullItemFromList(userSessionEntity, "clientSessions", entity.getSessionId(), invocationContext);
+
+        }
+
+        mongoStore.removeEntity(entity, invocationContext);
+
+    }
+
     @Override
     public ClientSessionModel getClientSession(RealmModel realm, String id) {
         MongoClientSessionEntity entity = getClientSessionEntity(id);

saml/saml-protocol/src/main/java/org/keycloak/protocol/saml/SamlProtocol.java

@@ -134,12 +134,9 @@ public class SamlProtocol implements LoginProtocol {
 
     @Override
     public Response cancelLogin(ClientSessionModel clientSession) {
-        return getErrorResponse(clientSession, JBossSAMLURIConstants.STATUS_REQUEST_DENIED.get());
+        Response error = getErrorResponse(clientSession, JBossSAMLURIConstants.STATUS_REQUEST_DENIED.get());
-    }
+        session.sessions().removeClientSession(realm, clientSession);
-
+        return error;
-    @Override
-    public Response invalidSessionError(ClientSessionModel clientSession) {
-        return getErrorResponse(clientSession, JBossSAMLURIConstants.STATUS_AUTHNFAILED.get());
     }
 
     protected String getResponseIssuer(RealmModel realm) {

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

@@ -459,7 +459,8 @@ public class AuthenticationProcessor {
         return authenticationComplete();
     }
 
-    protected void resetFlow() {
+    public static  void resetFlow(ClientSessionModel clientSession) {
+        clientSession.setAuthenticatedUser(null);
         clientSession.clearExecutionStatus();
         clientSession.clearUserSessionNotes();
         clientSession.removeNote(CURRENT_AUTHENTICATION_EXECUTION);
@@ -471,14 +472,14 @@ public class AuthenticationProcessor {
         if (!execution.equals(current)) {
             logger.debug("Current execution does not equal executed execution.  Might be a page refresh");
             logFailure();
-            resetFlow();
+            resetFlow(clientSession);
             return authenticate();
         }
         AuthenticationExecutionModel model = realm.getAuthenticationExecutionById(execution);
         if (model == null) {
             logger.debug("Cannot find execution, reseting flow");
             logFailure();
-            resetFlow();
+            resetFlow(clientSession);
             return authenticate();
         }
         event.event(EventType.LOGIN);

services/src/main/java/org/keycloak/protocol/LoginProtocol.java

@@ -28,7 +28,6 @@ public interface LoginProtocol extends Provider {
     LoginProtocol setEventBuilder(EventBuilder event);
 
     Response cancelLogin(ClientSessionModel clientSession);
-    Response invalidSessionError(ClientSessionModel clientSession);
     Response authenticated(UserSessionModel userSession, ClientSessionCode accessCode);
     Response consentDenied(ClientSessionModel clientSession);
 

services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocol.java

@@ -122,6 +122,7 @@ public class OIDCLoginProtocol implements LoginProtocol {
         if (state != null) {
             redirectUri.queryParam(OAuth2Constants.STATE, state);
         }
+        session.sessions().removeClientSession(realm, clientSession);
         return Response.status(302).location(redirectUri.build()).build();
     }
 

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

@@ -182,7 +182,15 @@ public class LoginActionsService {
             } else if (!(clientCode.isActionActive(requiredAction) || clientCode.isActionActive(alternativeRequiredAction))) {
                 event.client(clientCode.getClientSession().getClient());
                 event.error(Errors.EXPIRED_CODE);
+                if (clientCode.getClientSession().getAction().equals(ClientSessionModel.Action.AUTHENTICATE.name())) {
+                    AuthenticationProcessor.resetFlow(clientCode.getClientSession());
+                    response = processAuthentication(null, clientCode.getClientSession());
+                } else {
+                    if (clientCode.getClientSession().getUserSession() == null) {
+                        session.sessions().removeClientSession(realm, clientCode.getClientSession());
+                    }
                     response = ErrorPage.error(session, Messages.EXPIRED_CODE);
+                }
                 return false;
             } else {
                 return true;
@@ -207,21 +215,26 @@ public class LoginActionsService {
                 return false;
             }
             ClientSessionModel clientSession = clientCode.getClientSession();
+            if (clientSession == null) {
+                event.error(Errors.INVALID_CODE);
+                response = ErrorPage.error(session, Messages.INVALID_CODE);
+                return false;
+            }
             event.detail(Details.CODE_ID, clientSession.getId());
             ClientModel client = clientSession.getClient();
             if (client == null) {
                 event.error(Errors.CLIENT_NOT_FOUND);
                 response = ErrorPage.error(session, Messages.UNKNOWN_LOGIN_REQUESTER);
+                session.sessions().removeClientSession(realm, clientSession);
                 return false;
             }
-            session.getContext().setClient(client);
-
             if (!client.isEnabled()) {
                 event.error(Errors.CLIENT_NOT_FOUND);
                 response = ErrorPage.error(session, Messages.LOGIN_REQUESTER_NOT_ENABLED);
+                session.sessions().removeClientSession(realm, clientSession);
                 return false;
             }
-            session.getContext().setClient(clientCode.getClientSession().getClient());
+            session.getContext().setClient(client);
             return true;
         }
     }
@@ -239,7 +252,7 @@ public class LoginActionsService {
                                  @QueryParam("execution") String execution) {
         event.event(EventType.LOGIN);
         Checks checks = new Checks();
-        if (!checks.check(code)) {
+        if (!checks.check(code, ClientSessionModel.Action.AUTHENTICATE.name(), ClientSessionModel.Action.RECOVER_PASSWORD.name())) {
             return checks.response;
         }
         event.detail(Details.CODE_ID, code);

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

@@ -137,6 +137,7 @@ public class ResetPasswordTest {
 
         events.expectRequiredAction(EventType.SEND_RESET_PASSWORD).user(userId).detail(Details.USERNAME, "login-test").detail(Details.EMAIL, "login@test.com").assertEvent().getSessionId();
 
+        String src = driver.getPageSource();
         resetPasswordPage.backToLogin();
 
         assertTrue(loginPage.isCurrent());