From 1e33931f233d4d634ec566eb22892b82e54b1d63 Mon Sep 17 00:00:00 2001 From: mposolda Date: Wed, 8 Oct 2014 22:17:39 +0200 Subject: [PATCH] KEYCLOAK-741 Failure to refresh token should invalidate http session --- .../adapters/as7/KeycloakAuthenticatorValve.java | 10 +++++++--- .../tomcat7/KeycloakAuthenticatorValve.java | 10 +++++++--- .../undertow/ServletRequestAuthenticator.java | 10 ++++++---- .../undertow/UndertowRequestAuthenticator.java | 14 ++++++++------ 4 files changed, 28 insertions(+), 16 deletions(-) diff --git a/integration/as7-eap6/adapter/src/main/java/org/keycloak/adapters/as7/KeycloakAuthenticatorValve.java b/integration/as7-eap6/adapter/src/main/java/org/keycloak/adapters/as7/KeycloakAuthenticatorValve.java index 3874d1e83a..9bf2c42636 100755 --- a/integration/as7-eap6/adapter/src/main/java/org/keycloak/adapters/as7/KeycloakAuthenticatorValve.java +++ b/integration/as7-eap6/adapter/src/main/java/org/keycloak/adapters/as7/KeycloakAuthenticatorValve.java @@ -185,11 +185,15 @@ public class KeycloakAuthenticatorValve extends FormAuthenticator implements Lif boolean success = session.refreshExpiredToken(false); if (success && session.isActive()) return; - request.getSessionInternal().removeNote(KeycloakSecurityContext.class.getName()); + // Refresh failed, so user is already logged out from keycloak. Cleanup and expire our session + Session catalinaSession = request.getSessionInternal(); + log.debugf("Cleanup and expire session %s after failed refresh", catalinaSession.getId()); + catalinaSession.removeNote(KeycloakSecurityContext.class.getName()); request.setUserPrincipal(null); request.setAuthType(null); - request.getSessionInternal().setPrincipal(null); - request.getSessionInternal().setAuthType(null); + catalinaSession.setPrincipal(null); + catalinaSession.setAuthType(null); + catalinaSession.expire(); } public void keycloakSaveRequest(Request request) throws IOException { diff --git a/integration/tomcat7/adapter/src/main/java/org/keycloak/adapters/tomcat7/KeycloakAuthenticatorValve.java b/integration/tomcat7/adapter/src/main/java/org/keycloak/adapters/tomcat7/KeycloakAuthenticatorValve.java index f3b773f6b2..208882c447 100755 --- a/integration/tomcat7/adapter/src/main/java/org/keycloak/adapters/tomcat7/KeycloakAuthenticatorValve.java +++ b/integration/tomcat7/adapter/src/main/java/org/keycloak/adapters/tomcat7/KeycloakAuthenticatorValve.java @@ -187,11 +187,15 @@ public class KeycloakAuthenticatorValve extends FormAuthenticator implements Lif boolean success = session.refreshExpiredToken(false); if (success && session.isActive()) return; - request.getSessionInternal().removeNote(KeycloakSecurityContext.class.getName()); + // Refresh failed, so user is already logged out from keycloak. Cleanup and expire our session + Session catalinaSession = request.getSessionInternal(); + log.fine("Cleanup and expire session " + catalinaSession + " after failed refresh"); + catalinaSession.removeNote(KeycloakSecurityContext.class.getName()); request.setUserPrincipal(null); request.setAuthType(null); - request.getSessionInternal().setPrincipal(null); - request.getSessionInternal().setAuthType(null); + catalinaSession.setPrincipal(null); + catalinaSession.setAuthType(null); + catalinaSession.expire(); } public void keycloakSaveRequest(Request request) throws IOException { diff --git a/integration/undertow/src/main/java/org/keycloak/adapters/undertow/ServletRequestAuthenticator.java b/integration/undertow/src/main/java/org/keycloak/adapters/undertow/ServletRequestAuthenticator.java index eb0582fcf7..5416d3c29e 100755 --- a/integration/undertow/src/main/java/org/keycloak/adapters/undertow/ServletRequestAuthenticator.java +++ b/integration/undertow/src/main/java/org/keycloak/adapters/undertow/ServletRequestAuthenticator.java @@ -63,10 +63,12 @@ public class ServletRequestAuthenticator extends UndertowRequestAuthenticator { securityContext.authenticationComplete(account, "KEYCLOAK", false); propagateKeycloakContext( account); return true; + } else { + log.debug("Refresh failed. Account was not active. Returning null and invalidating Http session"); + session.setAttribute(KeycloakUndertowAccount.class.getName(), null); + session.invalidate(); + return false; } - log.debug("Account was not active, returning null"); - session.setAttribute(KeycloakUndertowAccount.class.getName(), null); - return false; } @Override @@ -100,6 +102,6 @@ public class ServletRequestAuthenticator extends UndertowRequestAuthenticator { protected HttpSession getSession(boolean create) { final ServletRequestContext servletRequestContext = exchange.getAttachment(ServletRequestContext.ATTACHMENT_KEY); HttpServletRequest req = (HttpServletRequest) servletRequestContext.getServletRequest(); - return req.getSession(true); + return req.getSession(create); } } diff --git a/integration/undertow/src/main/java/org/keycloak/adapters/undertow/UndertowRequestAuthenticator.java b/integration/undertow/src/main/java/org/keycloak/adapters/undertow/UndertowRequestAuthenticator.java index 44c4e86e7c..671f2e7c36 100755 --- a/integration/undertow/src/main/java/org/keycloak/adapters/undertow/UndertowRequestAuthenticator.java +++ b/integration/undertow/src/main/java/org/keycloak/adapters/undertow/UndertowRequestAuthenticator.java @@ -88,24 +88,26 @@ public abstract class UndertowRequestAuthenticator extends RequestAuthenticator protected boolean isCached() { Session session = Sessions.getSession(exchange); if (session == null) { - log.info("session was null, returning null"); + log.debug("session was null, returning null"); return false; } KeycloakUndertowAccount account = (KeycloakUndertowAccount)session.getAttribute(KeycloakUndertowAccount.class.getName()); if (account == null) { - log.info("Account was not in session, returning null"); + log.debug("Account was not in session, returning null"); return false; } account.setDeployment(deployment); if (account.isActive()) { - log.info("Cached account found"); + log.debug("Cached account found"); securityContext.authenticationComplete(account, "KEYCLOAK", false); propagateKeycloakContext( account); return true; + } else { + log.debug("Account was not active, returning false"); + session.removeAttribute(KeycloakUndertowAccount.class.getName()); + session.invalidate(exchange); + return false; } - log.info("Account was not active, returning false"); - session.removeAttribute(KeycloakUndertowAccount.class.getName()); - return false; } @Override