Merge pull request kitodo#6047 from matthias-ronge/improve-handle-ses…

…sions Improve handle sessions
subhh · Aug 23, 2024 · 56954cd · 56954cd
2 parents d838f8e + 97695a1
commit 56954cd
Show file tree

Hide file tree

Showing 4 changed files with 70 additions and 32 deletions.
diff --git a/Kitodo/src/main/java/org/kitodo/production/helper/Helper.java b/Kitodo/src/main/java/org/kitodo/production/helper/Helper.java
@@ -550,4 +550,23 @@ public static String generateRandomString(int length) {
         }
         return sb.toString();
     }
+
+    /**
+     * Returns an object description for error messages. It consists of the
+     * class name and optionally the {@code toString()}, if {@code toString()}
+     * is overloaded.
+     * 
+     * @param object
+     *            object to be described
+     * @return object description
+     */
+    public static String getObjectDescription(Object object) {
+        if (Objects.isNull(object)) {
+            return "null";
+        }
+        String fullClassName = object.getClass().getName();
+        String objectToString = object.toString();
+        return objectToString.startsWith(fullClassName.concat("@")) ? fullClassName
+                : fullClassName + '(' + objectToString + ')';
+    }
 }
diff --git a/Kitodo/src/main/java/org/kitodo/production/security/CustomLogoutSuccessHandler.java b/Kitodo/src/main/java/org/kitodo/production/security/CustomLogoutSuccessHandler.java
@@ -12,11 +12,15 @@
 package org.kitodo.production.security;
 
 import java.io.IOException;
+import java.text.MessageFormat;
 import java.util.Objects;
 
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.kitodo.production.helper.Helper;
 import org.kitodo.production.services.ServiceManager;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.userdetails.UserDetails;
@@ -31,6 +35,7 @@
  */
 public class CustomLogoutSuccessHandler extends SimpleUrlLogoutSuccessHandler implements LogoutSuccessHandler {
 
+    private static final Logger logger = LogManager.getLogger(CustomLogoutSuccessHandler.class);
     private final String onSuccessUrl;
     private final RedirectStrategy redirectStrategy = new DefaultRedirectStrategy();
 
@@ -41,12 +46,21 @@ public CustomLogoutSuccessHandler(String onSuccessUrl) {
     @Override
     public void onLogoutSuccess(HttpServletRequest request, HttpServletResponse response, Authentication authentication)
             throws IOException {
-        if (Objects.nonNull(authentication) && Objects.nonNull(authentication.getDetails())) {
-            Object principal = authentication.getPrincipal();
-            if (principal instanceof UserDetails) {
-                UserDetails user = (UserDetails) principal;
-                ServiceManager.getSessionService().expireSessionsOfUser(user);
+        if (Objects.nonNull(authentication)) {
+            if (Objects.nonNull(authentication.getDetails())) {
+                Object principal = authentication.getPrincipal();
+                if (principal instanceof UserDetails) {
+                    UserDetails user = (UserDetails) principal;
+                    ServiceManager.getSessionService().expireSessionsOfUser(user);
+                } else {
+                    logger.warn(MessageFormat.format("Cannot expire session: {0} is not an instance of UserDetails",
+                        Helper.getObjectDescription(principal)));
+                }
+            } else {
+                logger.warn("Cannot expire session: authentication.getDetails() is null");
             }
+        } else {
+            logger.warn("Cannot expire session: authentication is null");
         }
         redirectStrategy.sendRedirect(request, response, onSuccessUrl);
     }

diff --git a/Kitodo/src/main/java/org/kitodo/production/services/security/SessionService.java b/Kitodo/src/main/java/org/kitodo/production/services/security/SessionService.java
@@ -11,21 +11,30 @@
 
 package org.kitodo.production.services.security;
 
+import java.text.MessageFormat;
 import java.time.ZoneId;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Objects;
 
+import javax.servlet.http.HttpSessionEvent;
+import javax.servlet.http.HttpSessionListener;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.kitodo.production.helper.Helper;
 import org.kitodo.production.metadata.MetadataLock;
 import org.kitodo.production.security.SecurityConfig;
 import org.kitodo.production.security.SecuritySession;
 import org.kitodo.production.security.SecurityUserDetails;
+import org.springframework.security.core.context.SecurityContextImpl;
 import org.springframework.security.core.session.SessionInformation;
 import org.springframework.security.core.session.SessionRegistry;
 import org.springframework.security.core.userdetails.UserDetails;
 
-public class SessionService {
+public class SessionService implements HttpSessionListener {
 
+    private static final Logger logger = LogManager.getLogger(SessionService.class);
     private static volatile SessionService instance = null;
     private final SessionRegistry sessionRegistry;
 
@@ -37,6 +46,27 @@ private SessionService() {
         this.sessionRegistry = securityConfig.getSessionRegistry();
     }
 
+    /*
+     * This function is called when the session from the servlet container expires.
+     */
+    @Override
+    public void sessionDestroyed(HttpSessionEvent se) {
+        Object securityContextObject = se.getSession().getAttribute("SPRING_SECURITY_CONTEXT");
+        if (securityContextObject instanceof SecurityContextImpl) {
+            SecurityContextImpl securityContext = (SecurityContextImpl) securityContextObject;
+            Object principal = securityContext.getAuthentication().getPrincipal();
+            if (principal instanceof SecurityUserDetails) {
+                expireSessionsOfUser((SecurityUserDetails) principal);
+            } else {
+                logger.warn(MessageFormat.format("Cannot expire session: {0} is not an instance of SecurityUserDetails",
+                    Helper.getObjectDescription(principal)));
+            }
+        } else {
+            logger.warn(MessageFormat.format("Cannot expire session: {0} is not an instance of SecurityContextImpl",
+                Helper.getObjectDescription(securityContextObject)));
+        }
+    }
+
     /**
      * Expires all active sessions of a spring security UserDetails object.
      *

diff --git a/Kitodo/src/main/java/org/kitodo/production/version/KitodoVersionListener.java b/Kitodo/src/main/java/org/kitodo/production/version/KitodoVersionListener.java
@@ -20,21 +20,13 @@
 import javax.servlet.ServletContextEvent;
 import javax.servlet.ServletContextListener;
 import javax.servlet.annotation.WebListener;
-import javax.servlet.http.HttpSessionAttributeListener;
-import javax.servlet.http.HttpSessionBindingEvent;
-import javax.servlet.http.HttpSessionEvent;
-import javax.servlet.http.HttpSessionListener;
-
-import org.kitodo.production.security.SecurityUserDetails;
-import org.kitodo.production.services.ServiceManager;
-import org.springframework.security.core.context.SecurityContextImpl;
 
 /**
  * Listener to set up Kitodo versioning information from Manifest on application
  * startup.
  */
 @WebListener
-public class KitodoVersionListener implements ServletContextListener, HttpSessionListener {
+public class KitodoVersionListener implements ServletContextListener {
 
     @Override
     public void contextInitialized(ServletContextEvent sce) {
@@ -56,21 +48,4 @@ public void contextInitialized(ServletContextEvent sce) {
     public void contextDestroyed(ServletContextEvent sce) {
         // nothing is done here
     }
-
-    @Override
-    public void sessionCreated(HttpSessionEvent se) {
-        // nothing is done here
-    }
-
-    @Override
-    public void sessionDestroyed(HttpSessionEvent se) {
-        Object securityContextObject = se.getSession().getAttribute("SPRING_SECURITY_CONTEXT");
-        if (securityContextObject instanceof SecurityContextImpl) {
-            SecurityContextImpl securityContext = (SecurityContextImpl) securityContextObject;
-            Object principal = securityContext.getAuthentication().getPrincipal();
-            if (principal instanceof SecurityUserDetails) {
-                ServiceManager.getSessionService().expireSessionsOfUser((SecurityUserDetails) principal);
-            }
-        }
-    }
 }