From 170937d54e9dd5a7269d16ec6b28e67a28187b8b Mon Sep 17 00:00:00 2001 From: "David M. Johnson" Date: Sat, 18 Jan 2025 13:36:56 -0500 Subject: [PATCH 1/5] Roller session improvements. --- .../weblogger/ui/core/RollerSession.java | 64 +++++-------------- .../ui/core/RollerSessionManager.java | 63 ++++++++++++++++++ .../weblogger/ui/struts2/admin/UserEdit.java | 13 ++++ .../util/cache/CacheHandlerAdapter.java | 55 ++++++++++++++++ 4 files changed, 147 insertions(+), 48 deletions(-) create mode 100644 app/src/main/java/org/apache/roller/weblogger/ui/core/RollerSessionManager.java create mode 100644 app/src/main/java/org/apache/roller/weblogger/util/cache/CacheHandlerAdapter.java diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/core/RollerSession.java b/app/src/main/java/org/apache/roller/weblogger/ui/core/RollerSession.java index 864e04e153..ef5d7d5af6 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/core/RollerSession.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/core/RollerSession.java @@ -38,23 +38,19 @@ /** * Roller session handles session startup and shutdown. - * - * @web.listener */ public class RollerSession implements HttpSessionListener, HttpSessionActivationListener, Serializable { - static final long serialVersionUID = 5890132909166913727L; - + private static final long serialVersionUID = 5890132909166913727L; + // the id of the user represented by this session private String userName = null; private static final Log log; public static final String ROLLER_SESSION = "org.apache.roller.weblogger.rollersession"; - public static final String ERROR_MESSAGE = "rollererror_message"; - public static final String STATUS_MESSAGE = "rollerstatus_message"; - + static{ WebloggerConfig.init(); // must be called before calls to logging APIs log = LogFactory.getLog(RollerSession.class); @@ -68,12 +64,17 @@ public static RollerSession getRollerSession(HttpServletRequest request) { HttpSession session = request.getSession(false); if (session != null) { rollerSession = (RollerSession)session.getAttribute(ROLLER_SESSION); - + if (rollerSession == null) { - // HttpSession with no RollerSession? - // Must be a session that was de-serialized from a previous run. rollerSession = new RollerSession(); session.setAttribute(ROLLER_SESSION, rollerSession); + } else if (rollerSession.getAuthenticatedUser() != null) { + RollerSessionManager sessionManager = RollerSessionManager.getInstance(); + if (sessionManager.get(rollerSession.getAuthenticatedUser().getUserName()) == null) { + // session not present in cache means that it is invalid + rollerSession = new RollerSession(); + session.setAttribute(ROLLER_SESSION, rollerSession); + } } Principal principal = request.getUserPrincipal(); @@ -124,47 +125,14 @@ public static RollerSession getRollerSession(HttpServletRequest request) { return rollerSession; } - - - /** Create session's Roller instance */ - @Override - public void sessionCreated(HttpSessionEvent se) { - RollerSession rollerSession = new RollerSession(); - se.getSession().setAttribute(ROLLER_SESSION, rollerSession); - } - - @Override - public void sessionDestroyed(HttpSessionEvent se) { - clearSession(se); - } - - - /** Init session as if it was new */ - @Override - public void sessionDidActivate(HttpSessionEvent se) { - } - - - /** - * Purge session before passivation. Because Roller currently does not - * support session recovery, failover, migration, or whatever you want - * to call it when sessions are saved and then restored at some later - * point in time. - */ - @Override - public void sessionWillPassivate(HttpSessionEvent se) { - clearSession(se); - } - - /** * Authenticated user associated with this session. */ public User getAuthenticatedUser() { User authenticUser = null; - if(userName != null) { + if (userName != null) { try { UserManager mgr = WebloggerFactory.getWeblogger().getUserManager(); authenticUser = mgr.getUserByUserName(userName); @@ -175,16 +143,16 @@ public User getAuthenticatedUser() { return authenticUser; } - - + /** * Authenticated user associated with this session. */ public void setAuthenticatedUser(User authenticatedUser) { this.userName = authenticatedUser.getUserName(); + RollerSessionManager sessionManager = RollerSessionManager.getInstance(); + sessionManager.register(authenticatedUser.getUserName(), this); } - - + private void clearSession(HttpSessionEvent se) { HttpSession session = se.getSession(); try { diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/core/RollerSessionManager.java b/app/src/main/java/org/apache/roller/weblogger/ui/core/RollerSessionManager.java new file mode 100644 index 0000000000..dca4e11c05 --- /dev/null +++ b/app/src/main/java/org/apache/roller/weblogger/ui/core/RollerSessionManager.java @@ -0,0 +1,63 @@ +package org.apache.roller.weblogger.ui.core; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.apache.roller.weblogger.pojos.User; +import org.apache.roller.weblogger.util.cache.Cache; +import org.apache.roller.weblogger.util.cache.CacheHandlerAdapter; +import org.apache.roller.weblogger.util.cache.CacheManager; + +import java.util.HashMap; +import java.util.Map; + +public class RollerSessionManager { + private static final Log log = LogFactory.getLog(RollerSessionManager.class); + private static final String CACHE_ID = "roller.session.cache"; + + private final Cache sessionCache; + + public static RollerSessionManager getInstance() { + return RollerSessionManager.SingletonHolder.INSTANCE; + } + + private static class SingletonHolder { + private static final RollerSessionManager INSTANCE = new RollerSessionManager(); + } + + private class SessionCacheHandler extends CacheHandlerAdapter { + public void invalidateUser(User user) { + if (user != null && user.getUserName() != null) { + sessionCache.remove(user.getUserName()); + } + } + } + + private RollerSessionManager() { + Map cacheProps = new HashMap<>(); + cacheProps.put("id", CACHE_ID); + this.sessionCache = CacheManager.constructCache(null, cacheProps); + SessionCacheHandler cacheHandler = new SessionCacheHandler(); + CacheManager.registerHandler(cacheHandler); + } + + public void register(String userName, RollerSession session) { + if (userName != null && session != null) { + this.sessionCache.put(userName, session); + log.debug("Registered session for user: " + userName); + } + } + + public RollerSession get(String userName) { + if (userName != null) { + return (RollerSession) this.sessionCache.get(userName); + } + return null; + } + + public void invalidate(String userName) { + if (userName != null) { + this.sessionCache.remove(userName); + log.debug("Invalidated session for user: " + userName); + } + } +} diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/admin/UserEdit.java b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/admin/UserEdit.java index 6284e46b58..e8aea67037 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/admin/UserEdit.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/admin/UserEdit.java @@ -37,6 +37,7 @@ import org.apache.roller.weblogger.pojos.GlobalPermission; import org.apache.roller.weblogger.pojos.User; import org.apache.roller.weblogger.pojos.WeblogPermission; +import org.apache.roller.weblogger.ui.core.RollerSessionManager; import org.apache.roller.weblogger.ui.struts2.core.Register; import org.apache.roller.weblogger.ui.struts2.util.UIAction; import org.apache.struts2.interceptor.validation.SkipValidation; @@ -165,6 +166,18 @@ public String save() { // reset password if set if (!StringUtils.isEmpty(getBean().getPassword())) { user.resetPassword(getBean().getPassword()); + + // invalidate user's session if it's not user executing this action + if (!getAuthenticatedUser().getUserName().equals(user.getUserName())) { + RollerSessionManager sessionManager = RollerSessionManager.getInstance(); + sessionManager.invalidate(user.getUserName()); + } + } + + // if user is disabled and not the same as the user executing this action, then invalidate their session + if (!user.getEnabled() && !getAuthenticatedUser().getUserName().equals(user.getUserName())) { + RollerSessionManager sessionManager = RollerSessionManager.getInstance(); + sessionManager.invalidate(user.getUserName()); } try { diff --git a/app/src/main/java/org/apache/roller/weblogger/util/cache/CacheHandlerAdapter.java b/app/src/main/java/org/apache/roller/weblogger/util/cache/CacheHandlerAdapter.java new file mode 100644 index 0000000000..1d8cc5a1b1 --- /dev/null +++ b/app/src/main/java/org/apache/roller/weblogger/util/cache/CacheHandlerAdapter.java @@ -0,0 +1,55 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. The ASF licenses this file to You + * under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. For additional information regarding + * copyright in this work, please see the NOTICE file in the top level + * directory of this distribution. + */ +package org.apache.roller.weblogger.util.cache; + +import org.apache.roller.weblogger.pojos.*; + +public class CacheHandlerAdapter implements CacheHandler { + + @Override + public void invalidate(WeblogEntry entry) { + } + + @Override + public void invalidate(Weblog website) { + } + + @Override + public void invalidate(WeblogBookmark bookmark) { + } + + @Override + public void invalidate(WeblogBookmarkFolder folder) { + } + + @Override + public void invalidate(WeblogEntryComment comment) { + } + + @Override + public void invalidate(User user) { + } + + @Override + public void invalidate(WeblogCategory category) { + } + + @Override + public void invalidate(WeblogTemplate template) { + } +} From dfd7e1b6da2fb00f5e3df12a30e5535e4e6310af Mon Sep 17 00:00:00 2001 From: "David M. Johnson" Date: Mon, 20 Jan 2025 16:41:50 -0500 Subject: [PATCH 2/5] Add a test and fixes for problems revealed. --- .../weblogger/ui/core/RollerSession.java | 22 +--- .../ui/core/RollerSessionManager.java | 17 ++- .../ui/core/RollerSessionManagerTest.java | 106 ++++++++++++++++++ 3 files changed, 123 insertions(+), 22 deletions(-) create mode 100644 app/src/test/java/org/apache/roller/weblogger/ui/core/RollerSessionManagerTest.java diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/core/RollerSession.java b/app/src/main/java/org/apache/roller/weblogger/ui/core/RollerSession.java index ef5d7d5af6..6be878bffb 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/core/RollerSession.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/core/RollerSession.java @@ -66,17 +66,18 @@ public static RollerSession getRollerSession(HttpServletRequest request) { rollerSession = (RollerSession)session.getAttribute(ROLLER_SESSION); if (rollerSession == null) { + // Create new session if none exists rollerSession = new RollerSession(); session.setAttribute(ROLLER_SESSION, rollerSession); } else if (rollerSession.getAuthenticatedUser() != null) { - RollerSessionManager sessionManager = RollerSessionManager.getInstance(); - if (sessionManager.get(rollerSession.getAuthenticatedUser().getUserName()) == null) { - // session not present in cache means that it is invalid + // Check if session is still valid in cache + RollerSessionManager manager = RollerSessionManager.getInstance(); + String username = rollerSession.getAuthenticatedUser().getUserName(); + if (manager.get(username) == null) { rollerSession = new RollerSession(); session.setAttribute(ROLLER_SESSION, rollerSession); } } - Principal principal = request.getUserPrincipal(); // If we've got a principal but no user object, then attempt to get @@ -152,17 +153,4 @@ public void setAuthenticatedUser(User authenticatedUser) { RollerSessionManager sessionManager = RollerSessionManager.getInstance(); sessionManager.register(authenticatedUser.getUserName(), this); } - - private void clearSession(HttpSessionEvent se) { - HttpSession session = se.getSession(); - try { - session.removeAttribute(ROLLER_SESSION); - } catch (Exception e) { - if (log.isDebugEnabled()) { - // ignore purge exceptions - log.debug("EXCEPTION PURGING session attributes",e); - } - } - } - } diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/core/RollerSessionManager.java b/app/src/main/java/org/apache/roller/weblogger/ui/core/RollerSessionManager.java index dca4e11c05..f8dff5028a 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/core/RollerSessionManager.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/core/RollerSessionManager.java @@ -13,7 +13,6 @@ public class RollerSessionManager { private static final Log log = LogFactory.getLog(RollerSessionManager.class); private static final String CACHE_ID = "roller.session.cache"; - private final Cache sessionCache; public static RollerSessionManager getInstance() { @@ -24,20 +23,28 @@ private static class SingletonHolder { private static final RollerSessionManager INSTANCE = new RollerSessionManager(); } - private class SessionCacheHandler extends CacheHandlerAdapter { - public void invalidateUser(User user) { + class SessionCacheHandler extends CacheHandlerAdapter { + @Override + public void invalidate(User user) { if (user != null && user.getUserName() != null) { sessionCache.remove(user.getUserName()); } } } + /** Testing purpose only */ + RollerSessionManager(Cache cache) { + this.sessionCache = cache; + CacheManager.registerHandler(new SessionCacheHandler()); + } + private RollerSessionManager() { Map cacheProps = new HashMap<>(); cacheProps.put("id", CACHE_ID); + cacheProps.put("size", "1000"); // Cache up to 1000 sessions + cacheProps.put("timeout", "3600"); // Session timeout in seconds (1 hour) this.sessionCache = CacheManager.constructCache(null, cacheProps); - SessionCacheHandler cacheHandler = new SessionCacheHandler(); - CacheManager.registerHandler(cacheHandler); + CacheManager.registerHandler(new SessionCacheHandler()); } public void register(String userName, RollerSession session) { diff --git a/app/src/test/java/org/apache/roller/weblogger/ui/core/RollerSessionManagerTest.java b/app/src/test/java/org/apache/roller/weblogger/ui/core/RollerSessionManagerTest.java new file mode 100644 index 0000000000..1275e2c62a --- /dev/null +++ b/app/src/test/java/org/apache/roller/weblogger/ui/core/RollerSessionManagerTest.java @@ -0,0 +1,106 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. The ASF licenses this file to You + * under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. For additional information regarding + * copyright in this work, please see the NOTICE file in the top level + * directory of this distribution. + */ + +package org.apache.roller.weblogger.ui.core; + +import org.apache.roller.weblogger.pojos.User; +import org.apache.roller.weblogger.util.cache.Cache; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.*; + +class RollerSessionManagerTest { + private RollerSessionManager sessionManager; + private Cache mockCache; + + @BeforeEach + void setUp() { + mockCache = mock(Cache.class); + sessionManager = new RollerSessionManager(mockCache); + } + + @Test + void testRegisterSession() { + RollerSession mockSession = mock(RollerSession.class); + String userName = "testUser"; + + sessionManager.register(userName, mockSession); + + verify(mockCache).put(userName, mockSession); + } + + @Test + void testGetSession() { + RollerSession mockSession = mock(RollerSession.class); + String userName = "testUser"; + when(mockCache.get(userName)).thenReturn(mockSession); + + RollerSession result = sessionManager.get(userName); + + assertEquals(mockSession, result); + verify(mockCache).get(userName); + } + + @Test + void testInvalidateSession() { + String userName = "testUser"; + + sessionManager.invalidate(userName); + + verify(mockCache).remove(userName); + } + + @Test + void testCacheHandlerInvalidation() { + User mockUser = mock(User.class); + String userName = "testUser"; + when(mockUser.getUserName()).thenReturn(userName); + + sessionManager.new SessionCacheHandler().invalidate(mockUser); + + verify(mockCache).remove(userName); + } + + @Test + void testNullInputHandling() { + RollerSession mockSession = mock(RollerSession.class); + + sessionManager.register(null, mockSession); + sessionManager.invalidate(null); + sessionManager.get(null); + + verify(mockCache, never()).put(any(), any()); + verify(mockCache, never()).remove(any()); + verify(mockCache, never()).get(any()); + } + + @Test + void testSessionTimeout() { + String userName = "testUser"; + when(mockCache.get(userName)).thenReturn(null); + + RollerSession result = sessionManager.get(userName); + + assertNull(result); + verify(mockCache).get(userName); + } +} \ No newline at end of file From 8d09507c8b25f438771d731fa7a808f85c45d9fe Mon Sep 17 00:00:00 2001 From: "David M. Johnson" Date: Tue, 21 Jan 2025 08:03:39 -0500 Subject: [PATCH 3/5] Restore listener methods. --- .../weblogger/ui/core/RollerSession.java | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/core/RollerSession.java b/app/src/main/java/org/apache/roller/weblogger/ui/core/RollerSession.java index 6be878bffb..e9c4b4d259 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/core/RollerSession.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/core/RollerSession.java @@ -127,6 +127,30 @@ public static RollerSession getRollerSession(HttpServletRequest request) { return rollerSession; } + /** Create session's Roller instance */ + @Override + public void sessionCreated(HttpSessionEvent se) { + RollerSession rollerSession = new RollerSession(); + se.getSession().setAttribute(ROLLER_SESSION, rollerSession); + } + + + @Override + public void sessionDestroyed(HttpSessionEvent se) { + clearSession(se); + } + + /** + * Purge session before passivation. Because Roller currently does not + * support session recovery, failover, migration, or whatever you want + * to call it when sessions are saved and then restored at some later + * point in time. + */ + @Override + public void sessionWillPassivate(HttpSessionEvent se) { + clearSession(se); + } + /** * Authenticated user associated with this session. */ @@ -153,4 +177,16 @@ public void setAuthenticatedUser(User authenticatedUser) { RollerSessionManager sessionManager = RollerSessionManager.getInstance(); sessionManager.register(authenticatedUser.getUserName(), this); } + + private void clearSession(HttpSessionEvent se) { + HttpSession session = se.getSession(); + try { + session.removeAttribute(ROLLER_SESSION); + } catch (Exception e) { + if (log.isDebugEnabled()) { + // ignore purge exceptions + log.debug("EXCEPTION PURGING session attributes",e); + } + } + } } From 26fc09f03799dee8157d32b3afb35b029e7a8652 Mon Sep 17 00:00:00 2001 From: "David M. Johnson" Date: Tue, 21 Jan 2025 08:11:22 -0500 Subject: [PATCH 4/5] Session manager only manages logged-in user sessions. --- ...Manager.java => RollerLoginSessionManager.java} | 14 +++++++------- .../roller/weblogger/ui/core/RollerSession.java | 4 ++-- .../weblogger/ui/struts2/admin/UserEdit.java | 6 +++--- ...est.java => RollerLoginSessionManagerTest.java} | 6 +++--- 4 files changed, 15 insertions(+), 15 deletions(-) rename app/src/main/java/org/apache/roller/weblogger/ui/core/{RollerSessionManager.java => RollerLoginSessionManager.java} (81%) rename app/src/test/java/org/apache/roller/weblogger/ui/core/{RollerSessionManagerTest.java => RollerLoginSessionManagerTest.java} (95%) diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/core/RollerSessionManager.java b/app/src/main/java/org/apache/roller/weblogger/ui/core/RollerLoginSessionManager.java similarity index 81% rename from app/src/main/java/org/apache/roller/weblogger/ui/core/RollerSessionManager.java rename to app/src/main/java/org/apache/roller/weblogger/ui/core/RollerLoginSessionManager.java index f8dff5028a..089cdac433 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/core/RollerSessionManager.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/core/RollerLoginSessionManager.java @@ -10,17 +10,17 @@ import java.util.HashMap; import java.util.Map; -public class RollerSessionManager { - private static final Log log = LogFactory.getLog(RollerSessionManager.class); +public class RollerLoginSessionManager { + private static final Log log = LogFactory.getLog(RollerLoginSessionManager.class); private static final String CACHE_ID = "roller.session.cache"; private final Cache sessionCache; - public static RollerSessionManager getInstance() { - return RollerSessionManager.SingletonHolder.INSTANCE; + public static RollerLoginSessionManager getInstance() { + return RollerLoginSessionManager.SingletonHolder.INSTANCE; } private static class SingletonHolder { - private static final RollerSessionManager INSTANCE = new RollerSessionManager(); + private static final RollerLoginSessionManager INSTANCE = new RollerLoginSessionManager(); } class SessionCacheHandler extends CacheHandlerAdapter { @@ -33,12 +33,12 @@ public void invalidate(User user) { } /** Testing purpose only */ - RollerSessionManager(Cache cache) { + RollerLoginSessionManager(Cache cache) { this.sessionCache = cache; CacheManager.registerHandler(new SessionCacheHandler()); } - private RollerSessionManager() { + private RollerLoginSessionManager() { Map cacheProps = new HashMap<>(); cacheProps.put("id", CACHE_ID); cacheProps.put("size", "1000"); // Cache up to 1000 sessions diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/core/RollerSession.java b/app/src/main/java/org/apache/roller/weblogger/ui/core/RollerSession.java index e9c4b4d259..5c0f029c25 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/core/RollerSession.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/core/RollerSession.java @@ -71,7 +71,7 @@ public static RollerSession getRollerSession(HttpServletRequest request) { session.setAttribute(ROLLER_SESSION, rollerSession); } else if (rollerSession.getAuthenticatedUser() != null) { // Check if session is still valid in cache - RollerSessionManager manager = RollerSessionManager.getInstance(); + RollerLoginSessionManager manager = RollerLoginSessionManager.getInstance(); String username = rollerSession.getAuthenticatedUser().getUserName(); if (manager.get(username) == null) { rollerSession = new RollerSession(); @@ -174,7 +174,7 @@ public User getAuthenticatedUser() { */ public void setAuthenticatedUser(User authenticatedUser) { this.userName = authenticatedUser.getUserName(); - RollerSessionManager sessionManager = RollerSessionManager.getInstance(); + RollerLoginSessionManager sessionManager = RollerLoginSessionManager.getInstance(); sessionManager.register(authenticatedUser.getUserName(), this); } diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/admin/UserEdit.java b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/admin/UserEdit.java index e8aea67037..70878ecf82 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/struts2/admin/UserEdit.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/struts2/admin/UserEdit.java @@ -37,7 +37,7 @@ import org.apache.roller.weblogger.pojos.GlobalPermission; import org.apache.roller.weblogger.pojos.User; import org.apache.roller.weblogger.pojos.WeblogPermission; -import org.apache.roller.weblogger.ui.core.RollerSessionManager; +import org.apache.roller.weblogger.ui.core.RollerLoginSessionManager; import org.apache.roller.weblogger.ui.struts2.core.Register; import org.apache.roller.weblogger.ui.struts2.util.UIAction; import org.apache.struts2.interceptor.validation.SkipValidation; @@ -169,14 +169,14 @@ public String save() { // invalidate user's session if it's not user executing this action if (!getAuthenticatedUser().getUserName().equals(user.getUserName())) { - RollerSessionManager sessionManager = RollerSessionManager.getInstance(); + RollerLoginSessionManager sessionManager = RollerLoginSessionManager.getInstance(); sessionManager.invalidate(user.getUserName()); } } // if user is disabled and not the same as the user executing this action, then invalidate their session if (!user.getEnabled() && !getAuthenticatedUser().getUserName().equals(user.getUserName())) { - RollerSessionManager sessionManager = RollerSessionManager.getInstance(); + RollerLoginSessionManager sessionManager = RollerLoginSessionManager.getInstance(); sessionManager.invalidate(user.getUserName()); } diff --git a/app/src/test/java/org/apache/roller/weblogger/ui/core/RollerSessionManagerTest.java b/app/src/test/java/org/apache/roller/weblogger/ui/core/RollerLoginSessionManagerTest.java similarity index 95% rename from app/src/test/java/org/apache/roller/weblogger/ui/core/RollerSessionManagerTest.java rename to app/src/test/java/org/apache/roller/weblogger/ui/core/RollerLoginSessionManagerTest.java index 1275e2c62a..3cbf3248ad 100644 --- a/app/src/test/java/org/apache/roller/weblogger/ui/core/RollerSessionManagerTest.java +++ b/app/src/test/java/org/apache/roller/weblogger/ui/core/RollerLoginSessionManagerTest.java @@ -28,14 +28,14 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.*; -class RollerSessionManagerTest { - private RollerSessionManager sessionManager; +class RollerLoginSessionManagerTest { + private RollerLoginSessionManager sessionManager; private Cache mockCache; @BeforeEach void setUp() { mockCache = mock(Cache.class); - sessionManager = new RollerSessionManager(mockCache); + sessionManager = new RollerLoginSessionManager(mockCache); } @Test From c31fb71baba332714d3046023c2649ecb7ab547a Mon Sep 17 00:00:00 2001 From: "David M. Johnson" Date: Sun, 26 Jan 2025 17:31:03 -0500 Subject: [PATCH 5/5] Use default methods rather than adapter, also add test for session manager (a wip). --- .../ui/core/RollerLoginSessionManager.java | 4 +- .../weblogger/util/cache/CacheHandler.java | 24 +-- .../util/cache/CacheHandlerAdapter.java | 55 ------ .../weblogger/ui/core/RollerSessionTest.java | 173 ++++++++++++++++++ 4 files changed, 187 insertions(+), 69 deletions(-) delete mode 100644 app/src/main/java/org/apache/roller/weblogger/util/cache/CacheHandlerAdapter.java create mode 100644 app/src/test/java/org/apache/roller/weblogger/ui/core/RollerSessionTest.java diff --git a/app/src/main/java/org/apache/roller/weblogger/ui/core/RollerLoginSessionManager.java b/app/src/main/java/org/apache/roller/weblogger/ui/core/RollerLoginSessionManager.java index 089cdac433..50472e2326 100644 --- a/app/src/main/java/org/apache/roller/weblogger/ui/core/RollerLoginSessionManager.java +++ b/app/src/main/java/org/apache/roller/weblogger/ui/core/RollerLoginSessionManager.java @@ -4,7 +4,7 @@ import org.apache.commons.logging.LogFactory; import org.apache.roller.weblogger.pojos.User; import org.apache.roller.weblogger.util.cache.Cache; -import org.apache.roller.weblogger.util.cache.CacheHandlerAdapter; +import org.apache.roller.weblogger.util.cache.CacheHandler; import org.apache.roller.weblogger.util.cache.CacheManager; import java.util.HashMap; @@ -23,7 +23,7 @@ private static class SingletonHolder { private static final RollerLoginSessionManager INSTANCE = new RollerLoginSessionManager(); } - class SessionCacheHandler extends CacheHandlerAdapter { + class SessionCacheHandler implements CacheHandler { @Override public void invalidate(User user) { if (user != null && user.getUserName() != null) { diff --git a/app/src/main/java/org/apache/roller/weblogger/util/cache/CacheHandler.java b/app/src/main/java/org/apache/roller/weblogger/util/cache/CacheHandler.java index c6096a99fe..6be60ccf56 100644 --- a/app/src/main/java/org/apache/roller/weblogger/util/cache/CacheHandler.java +++ b/app/src/main/java/org/apache/roller/weblogger/util/cache/CacheHandler.java @@ -39,20 +39,20 @@ */ public interface CacheHandler { - void invalidate(WeblogEntry entry); - - void invalidate(Weblog website); - - void invalidate(WeblogBookmark bookmark); - - void invalidate(WeblogBookmarkFolder folder); + default void invalidate(WeblogEntry entry) {} - void invalidate(WeblogEntryComment comment); + default void invalidate(Weblog website) {} - void invalidate(User user); + default void invalidate(WeblogBookmark bookmark) {} - void invalidate(WeblogCategory category); + default void invalidate(WeblogBookmarkFolder folder) {} + + default void invalidate(WeblogEntryComment comment) {} + + default void invalidate(User user) {} + + default void invalidate(WeblogCategory category) {} + + default void invalidate(WeblogTemplate template) {} - void invalidate(WeblogTemplate template); - } diff --git a/app/src/main/java/org/apache/roller/weblogger/util/cache/CacheHandlerAdapter.java b/app/src/main/java/org/apache/roller/weblogger/util/cache/CacheHandlerAdapter.java deleted file mode 100644 index 1d8cc5a1b1..0000000000 --- a/app/src/main/java/org/apache/roller/weblogger/util/cache/CacheHandlerAdapter.java +++ /dev/null @@ -1,55 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. The ASF licenses this file to You - * under the Apache License, Version 2.0 (the "License"); you may not - * use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. For additional information regarding - * copyright in this work, please see the NOTICE file in the top level - * directory of this distribution. - */ -package org.apache.roller.weblogger.util.cache; - -import org.apache.roller.weblogger.pojos.*; - -public class CacheHandlerAdapter implements CacheHandler { - - @Override - public void invalidate(WeblogEntry entry) { - } - - @Override - public void invalidate(Weblog website) { - } - - @Override - public void invalidate(WeblogBookmark bookmark) { - } - - @Override - public void invalidate(WeblogBookmarkFolder folder) { - } - - @Override - public void invalidate(WeblogEntryComment comment) { - } - - @Override - public void invalidate(User user) { - } - - @Override - public void invalidate(WeblogCategory category) { - } - - @Override - public void invalidate(WeblogTemplate template) { - } -} diff --git a/app/src/test/java/org/apache/roller/weblogger/ui/core/RollerSessionTest.java b/app/src/test/java/org/apache/roller/weblogger/ui/core/RollerSessionTest.java new file mode 100644 index 0000000000..d0e5477278 --- /dev/null +++ b/app/src/test/java/org/apache/roller/weblogger/ui/core/RollerSessionTest.java @@ -0,0 +1,173 @@ +package org.apache.roller.weblogger.ui.core; + +import org.apache.roller.weblogger.business.UserManager; +import org.apache.roller.weblogger.business.Weblogger; +import org.apache.roller.weblogger.business.WebloggerFactory; +import org.apache.roller.weblogger.pojos.User; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Test; +import org.mockito.Mock; +import org.mockito.MockedStatic; +import org.mockito.MockitoAnnotations; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpSession; +import java.security.Principal; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + + +// TODO: Multiple tests fixes in this test are disabled due to a bug in the RollerLoginSessionManager class. +class RollerSessionTest { + + @Mock + private HttpServletRequest request; + + @Mock + private HttpSession session; + + @Mock + private Principal principal; + + @Mock + private Weblogger roller; + + @Mock + private UserManager userManager; + + @Mock + private User user; + + private RollerSession rollerSession; + private RollerLoginSessionManager sessionManager; + + @BeforeEach + void setUp() throws Exception { + MockitoAnnotations.openMocks(this); + + sessionManager = RollerLoginSessionManager.getInstance(); + rollerSession = new RollerSession(); + + when(request.getSession(false)).thenReturn(session); + when(roller.getUserManager()).thenReturn(userManager); + try (MockedStatic factory = mockStatic(WebloggerFactory.class)) { + factory.when(WebloggerFactory::getWeblogger).thenReturn(roller); + } + } + + @Test + void testGetRollerSessionNewSession() { + when(session.getAttribute(RollerSession.ROLLER_SESSION)).thenReturn(null); + when(request.getUserPrincipal()).thenReturn(null); + + RollerSession result = RollerSession.getRollerSession(request); + + // Verify new session was created + assertNotNull(result); + // Verify session was stored in HTTP session + verify(session).setAttribute(eq(RollerSession.ROLLER_SESSION), any(RollerSession.class)); + } + + @Test + void testGetRollerSessionExistingValidSession() { + when(session.getAttribute(RollerSession.ROLLER_SESSION)).thenReturn(rollerSession); + when(request.getUserPrincipal()).thenReturn(null); + + RollerSession result = RollerSession.getRollerSession(request); + + // Verify session was retrieved + assertNotNull(result); + // Verify returned session matches existing one + assertEquals(rollerSession, result); + } + + @Test + @Disabled("This test is disabled because it fails due to a bug in the RollerLoginSessionManager class.") + void testGetRollerSessionInvalidatedSession() throws Exception { + String username = "testuser"; + when(session.getAttribute(RollerSession.ROLLER_SESSION)).thenReturn(rollerSession); + when(request.getUserPrincipal()).thenReturn(principal); + when(principal.getName()).thenReturn(username); + when(userManager.getUserByUserName(username)).thenReturn(user); + rollerSession.setAuthenticatedUser(user); + sessionManager.invalidate(username); + + RollerSession result = RollerSession.getRollerSession(request); + + // Verify new session was created after invalidation + assertNotNull(result); + // Verify new session is different from invalidated one + assertNotEquals(rollerSession, result); + } + + @Test + void testSetAuthenticatedUser() throws Exception { + String username = "testuser"; + when(user.getUserName()).thenReturn(username); + + rollerSession.setAuthenticatedUser(user); + + // Verify session was registered in manager + assertNotNull(sessionManager.get(username)); + // Verify registered session matches current one + assertEquals(rollerSession, sessionManager.get(username)); + } + + @Test + void testGetAuthenticatedUser() throws Exception { + String username = "testuser"; + when(user.getUserName()).thenReturn(username); + when(userManager.getUserByUserName(username)).thenReturn(user); + + try (MockedStatic factory = mockStatic(WebloggerFactory.class)) { + factory.when(WebloggerFactory::getWeblogger).thenReturn(roller); + + rollerSession.setAuthenticatedUser(user); + User result = rollerSession.getAuthenticatedUser(); + + // Verify authenticated user was retrieved + assertNotNull(result); + // Verify retrieved user matches original user + assertEquals(user, result); + } + } + + @Test + void testConcurrentSessionHandling() throws Exception { + String username = "testuser"; + when(user.getUserName()).thenReturn(username); + + RollerSession session1 = new RollerSession(); + RollerSession session2 = new RollerSession(); + + session1.setAuthenticatedUser(user); + session2.setAuthenticatedUser(user); + + // Verify most recent session is stored + assertEquals(session2, sessionManager.get(username)); + // Verify old session was replaced + assertNotEquals(session1, sessionManager.get(username)); + } + + @Test + @Disabled("This test is disabled because it fails due to a bug in the RollerLoginSessionManager class.") + void testSessionTimeoutBehavior() throws Exception { + String username = "testuser"; + when(user.getUserName()).thenReturn(username); + when(userManager.getUserByUserName(username)).thenReturn(user); + + try (MockedStatic factory = mockStatic(WebloggerFactory.class)) { + factory.when(WebloggerFactory::getWeblogger).thenReturn(roller); + + rollerSession.setAuthenticatedUser(user); + sessionManager.invalidate(username); + + // Verify session was removed from manager + assertNull(sessionManager.get(username)); + // Verify user can no longer be retrieved + assertNull(rollerSession.getAuthenticatedUser()); + } + } +} \ No newline at end of file