From e7a792f922bc9baa23d90932f8f6d0d35397ce7f Mon Sep 17 00:00:00 2001 From: Stian Thorgersen Date: Wed, 21 Jan 2015 10:33:33 +0100 Subject: [PATCH] KEYCLOAK-983 Fix login after reset-password --- .../infinispan/ClientSessionAdapter.java | 23 +++++++++----- .../sessions/jpa/ClientSessionAdapter.java | 22 ++++++++++--- .../sessions/jpa/JpaUserSessionProvider.java | 3 ++ .../jpa/entities/UserSessionEntity.java | 2 +- .../sessions/mem/ClientSessionAdapter.java | 15 ++++++--- .../sessions/mongo/ClientSessionAdapter.java | 26 +++++++++++----- .../keycloak/protocol/oidc/TokenManager.java | 16 ++++++++-- .../resources/LoginActionsService.java | 7 +++-- .../testsuite/forms/ResetPasswordTest.java | 31 +++++++++++++++++++ 9 files changed, 117 insertions(+), 28 deletions(-) diff --git a/model/sessions-infinispan/src/main/java/org/keycloak/models/sessions/infinispan/ClientSessionAdapter.java b/model/sessions-infinispan/src/main/java/org/keycloak/models/sessions/infinispan/ClientSessionAdapter.java index 2c6addc2d7..645a76f47a 100644 --- a/model/sessions-infinispan/src/main/java/org/keycloak/models/sessions/infinispan/ClientSessionAdapter.java +++ b/model/sessions-infinispan/src/main/java/org/keycloak/models/sessions/infinispan/ClientSessionAdapter.java @@ -53,17 +53,24 @@ public class ClientSessionAdapter implements ClientSessionModel { @Override public void setUserSession(UserSessionModel userSession) { - if (entity.getUserSession() != null) { - if (entity.getUserSession().equals(userSession.getId())) { - return; - } else { - provider.dettachSession(userSession, this); + if (userSession == null) { + if (entity.getUserSession() != null) { + provider.dettachSession(getUserSession(), this); } + entity.setUserSession(null); } else { - provider.attachSession(userSession, this); - } + if (entity.getUserSession() != null) { + if (entity.getUserSession().equals(userSession.getId())) { + return; + } else { + provider.dettachSession(userSession, this); + } + } else { + provider.attachSession(userSession, this); + } - entity.setUserSession(userSession.getId()); + entity.setUserSession(userSession.getId()); + } update(); } diff --git a/model/sessions-jpa/src/main/java/org/keycloak/models/sessions/jpa/ClientSessionAdapter.java b/model/sessions-jpa/src/main/java/org/keycloak/models/sessions/jpa/ClientSessionAdapter.java index e05130c851..18b0332ef1 100755 --- a/model/sessions-jpa/src/main/java/org/keycloak/models/sessions/jpa/ClientSessionAdapter.java +++ b/model/sessions-jpa/src/main/java/org/keycloak/models/sessions/jpa/ClientSessionAdapter.java @@ -87,10 +87,17 @@ public class ClientSessionAdapter implements ClientSessionModel { @Override public void setUserSession(UserSessionModel userSession) { - UserSessionAdapter adapter = (UserSessionAdapter)userSession; - UserSessionEntity userSessionEntity = adapter.getEntity(); - entity.setSession(userSessionEntity); - userSessionEntity.getClientSessions().add(entity); + if (userSession == null) { + if (entity.getSession() != null) { + entity.getSession().getClientSessions().remove(entity); + } + entity.setSession(null); + } else { + UserSessionAdapter adapter = (UserSessionAdapter) userSession; + UserSessionEntity userSessionEntity = adapter.getEntity(); + entity.setSession(userSessionEntity); + userSessionEntity.getClientSessions().add(entity); + } } @Override @@ -109,6 +116,13 @@ public class ClientSessionAdapter implements ClientSessionModel { entity.getRoles().add(roleEntity); } + } else { + if (entity.getRoles() != null) { + for (ClientSessionRoleEntity r : entity.getRoles()) { + em.remove(r); + } + entity.getRoles().clear(); + } } } diff --git a/model/sessions-jpa/src/main/java/org/keycloak/models/sessions/jpa/JpaUserSessionProvider.java b/model/sessions-jpa/src/main/java/org/keycloak/models/sessions/jpa/JpaUserSessionProvider.java index 094d6adf1a..169a92d567 100755 --- a/model/sessions-jpa/src/main/java/org/keycloak/models/sessions/jpa/JpaUserSessionProvider.java +++ b/model/sessions-jpa/src/main/java/org/keycloak/models/sessions/jpa/JpaUserSessionProvider.java @@ -155,6 +155,9 @@ public class JpaUserSessionProvider implements UserSessionProvider { public void removeUserSession(RealmModel realm, UserSessionModel session) { UserSessionEntity entity = em.find(UserSessionEntity.class, session.getId()); if (entity != null) { + for (ClientSessionEntity c : entity.getClientSessions()) { + em.remove(c); + } em.remove(entity); } } diff --git a/model/sessions-jpa/src/main/java/org/keycloak/models/sessions/jpa/entities/UserSessionEntity.java b/model/sessions-jpa/src/main/java/org/keycloak/models/sessions/jpa/entities/UserSessionEntity.java index 73f9b605ae..e822e759fb 100755 --- a/model/sessions-jpa/src/main/java/org/keycloak/models/sessions/jpa/entities/UserSessionEntity.java +++ b/model/sessions-jpa/src/main/java/org/keycloak/models/sessions/jpa/entities/UserSessionEntity.java @@ -59,7 +59,7 @@ public class UserSessionEntity { @Column(name="USER_SESSION_STATE") protected UserSessionModel.State state; - @OneToMany(cascade = CascadeType.REMOVE, orphanRemoval = true, mappedBy="session") + @OneToMany(mappedBy="session") protected Collection clientSessions = new ArrayList(); @OneToMany(cascade = CascadeType.REMOVE, orphanRemoval = true, mappedBy="userSession") diff --git a/model/sessions-mem/src/main/java/org/keycloak/models/sessions/mem/ClientSessionAdapter.java b/model/sessions-mem/src/main/java/org/keycloak/models/sessions/mem/ClientSessionAdapter.java index 1328171b9a..9981fb7634 100755 --- a/model/sessions-mem/src/main/java/org/keycloak/models/sessions/mem/ClientSessionAdapter.java +++ b/model/sessions-mem/src/main/java/org/keycloak/models/sessions/mem/ClientSessionAdapter.java @@ -52,10 +52,17 @@ public class ClientSessionAdapter implements ClientSessionModel { @Override public void setUserSession(UserSessionModel userSession) { - UserSessionAdapter adapter = (UserSessionAdapter)userSession; - UserSessionEntity userSessionEntity = adapter.getEntity(); - entity.setSession(userSessionEntity); - userSessionEntity.getClientSessions().add(entity); + if (userSession == null) { + if (entity.getSession() != null) { + entity.getSession().getClientSessions().remove(entity); + } + entity.setSession(null); + } else { + UserSessionAdapter adapter = (UserSessionAdapter) userSession; + UserSessionEntity userSessionEntity = adapter.getEntity(); + entity.setSession(userSessionEntity); + userSessionEntity.getClientSessions().add(entity); + } } @Override diff --git a/model/sessions-mongo/src/main/java/org/keycloak/models/sessions/mongo/ClientSessionAdapter.java b/model/sessions-mongo/src/main/java/org/keycloak/models/sessions/mongo/ClientSessionAdapter.java index 92773f856d..2ab8630bc4 100755 --- a/model/sessions-mongo/src/main/java/org/keycloak/models/sessions/mongo/ClientSessionAdapter.java +++ b/model/sessions-mongo/src/main/java/org/keycloak/models/sessions/mongo/ClientSessionAdapter.java @@ -55,11 +55,19 @@ public class ClientSessionAdapter extends AbstractMongoAdapter roles) { - List list = new LinkedList(); - list.addAll(roles); - entity.setRoles(list); + if (roles == null) { + entity.setRoles(null); + } else { + List list = new LinkedList(); + list.addAll(roles); + entity.setRoles(list); + } updateMongoEntity(); } diff --git a/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java b/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java index a72aa1c53d..22041031c7 100755 --- a/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java +++ b/services/src/main/java/org/keycloak/protocol/oidc/TokenManager.java @@ -17,6 +17,7 @@ import org.keycloak.models.RealmModel; import org.keycloak.models.RoleModel; import org.keycloak.models.UserModel; import org.keycloak.models.UserSessionModel; +import org.keycloak.models.UserSessionProvider; import org.keycloak.models.utils.KeycloakModelUtils; import org.keycloak.representations.AccessToken; import org.keycloak.representations.AccessTokenResponse; @@ -137,10 +138,21 @@ public class TokenManager { requestedRoles.add(r.getId()); } clientSession.setRoles(requestedRoles); - - } + public static void dettachClientSession(UserSessionProvider sessions, RealmModel realm, ClientSessionModel clientSession) { + UserSessionModel userSession = clientSession.getUserSession(); + if (userSession == null) { + return; + } + + clientSession.setUserSession(null); + clientSession.setRoles(null); + + if (userSession.getClientSessions().isEmpty()) { + sessions.removeUserSession(realm, userSession); + } + } public static Set getAccess(String scopeParam, ClientModel client, UserModel user) { // todo scopeParam is ignored until we figure out a scheme that fits with openid connect 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 84f7cdb965..33988da021 100755 --- a/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java +++ b/services/src/main/java/org/keycloak/services/resources/LoginActionsService.java @@ -200,7 +200,10 @@ public class LoginActionsService { ClientSessionCode clientSessionCode = checks.clientCode; ClientSessionModel clientSession = clientSessionCode.getClientSession(); - + if (clientSession.getAction().equals(ClientSessionModel.Action.RECOVER_PASSWORD)) { + TokenManager.dettachClientSession(session.sessions(), realm, clientSession); + clientSession.setAction(ClientSessionModel.Action.AUTHENTICATE); + } LoginFormsProvider forms = Flows.forms(session, realm, clientSession.getClient(), uriInfo) .setClientSessionCode(clientSessionCode.getCode()); @@ -267,7 +270,7 @@ public class LoginActionsService { return Flows.forwardToSecurityFailurePage(session, realm, uriInfo, "Unknown code, please login again through your application."); } ClientSessionModel clientSession = clientCode.getClientSession(); - if (!(clientCode.isValid(ClientSessionModel.Action.AUTHENTICATE) || clientCode.isValid(ClientSessionModel.Action.RECOVER_PASSWORD))) { + if (!clientCode.isValid(ClientSessionModel.Action.AUTHENTICATE) || clientSession.getUserSession() != null) { clientCode.setAction(ClientSessionModel.Action.AUTHENTICATE); event.client(clientSession.getClient()).error(Errors.INVALID_CODE); return Flows.forms(this.session, realm, clientSession.getClient(), uriInfo).setError(Messages.INVALID_USER) diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java index 07b24d763a..6ac7860a8b 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java @@ -27,6 +27,7 @@ import org.junit.Rule; import org.junit.Test; import org.keycloak.events.Details; import org.keycloak.events.Errors; +import org.keycloak.events.Event; import org.keycloak.events.EventType; import org.keycloak.models.PasswordPolicy; import org.keycloak.models.RealmModel; @@ -151,6 +152,36 @@ public class ResetPasswordTest { Assert.assertEquals("Unknown code, please login again through your application.", errorPage.getError()); } + @Test + public void resetPasswordCancelChangeUser() throws IOException, MessagingException { + loginPage.open(); + loginPage.resetPassword(); + + resetPasswordPage.assertCurrent(); + + resetPasswordPage.changePassword("test-user@localhost"); + + resetPasswordPage.assertCurrent(); + + events.expectRequiredAction(EventType.SEND_RESET_PASSWORD).detail(Details.USERNAME, "test-user@localhost").detail(Details.EMAIL, "test-user@localhost").assertEvent().getSessionId(); + + resetPasswordPage.backToLogin(); + + Assert.assertTrue(loginPage.isCurrent()); + + loginPage.login("login@test.com", "password"); + + Event loginEvent = events.expectLogin().user(userId).detail(Details.USERNAME, "login@test.com").assertEvent(); + + String code = oauth.getCurrentQuery().get("code"); + OAuthClient.AccessTokenResponse tokenResponse = oauth.doAccessTokenRequest(code, "password"); + + Assert.assertEquals(200, tokenResponse.getStatusCode()); + Assert.assertEquals(userId, oauth.verifyToken(tokenResponse.getAccessToken()).getSubject()); + + events.expectCodeToToken(loginEvent.getDetails().get(Details.CODE_ID), loginEvent.getSessionId()).user(userId).assertEvent(); + } + @Test public void resetPasswordByEmail() throws IOException, MessagingException { resetPassword("login@test.com");