Implement one-time-salt use and add comprehensive tests

app/src/main/java/org/apache/roller/weblogger/ui/core/filters/LoadSaltFilter.java

@@ -37,13 +37,14 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha
         throws IOException, ServletException {
 
         HttpServletRequest httpReq = (HttpServletRequest) request;
-        RollerSession rses = RollerSession.getRollerSession(httpReq);
-        String userId = rses != null && rses.getAuthenticatedUser() != null ? rses.getAuthenticatedUser().getId() : "";
-
-        SaltCache saltCache = SaltCache.getInstance();
-        String salt = RandomStringUtils.random(20, 0, 0, true, true, null, new SecureRandom());
-        saltCache.put(salt, userId);
-        httpReq.setAttribute("salt", salt);
+        RollerSession rollerSession = RollerSession.getRollerSession(httpReq);
+        if (rollerSession != null) {
+            String userId = rollerSession.getAuthenticatedUser() != null ? rollerSession.getAuthenticatedUser().getId() : "";
+            SaltCache saltCache = SaltCache.getInstance();
+            String salt = RandomStringUtils.random(20, 0, 0, true, true, null, new SecureRandom());
+            saltCache.put(salt, userId);
+            httpReq.setAttribute("salt", salt);
+        }
 
         chain.doFilter(request, response);
     }

app/src/main/java/org/apache/roller/weblogger/ui/core/filters/ValidateSaltFilter.java

@@ -51,17 +51,31 @@ public void doFilter(ServletRequest request, ServletResponse response,
             FilterChain chain) throws IOException, ServletException {
         HttpServletRequest httpReq = (HttpServletRequest) request;
 
-        if ("POST".equals(httpReq.getMethod()) && !isIgnoredURL(httpReq.getServletPath())) {
-            RollerSession rses = RollerSession.getRollerSession(httpReq);
-            String userId = rses != null && rses.getAuthenticatedUser() != null ? rses.getAuthenticatedUser().getId() : "";
+        String requestURL = httpReq.getRequestURL().toString();
+        String queryString = httpReq.getQueryString();
+        if (queryString != null) {
+            requestURL += "?" + queryString;
+        }
+
+        if ("POST".equals(httpReq.getMethod()) && !isIgnoredURL(requestURL)) {
+            RollerSession rollerSession = RollerSession.getRollerSession(httpReq);
+            if (rollerSession != null) {
+                String userId = rollerSession.getAuthenticatedUser() != null ? rollerSession.getAuthenticatedUser().getId() : "";
+
+                String salt = httpReq.getParameter("salt");
+                SaltCache saltCache = SaltCache.getInstance();
+                if (salt == null || !Objects.equals(saltCache.get(salt), userId)) {
+                    if (log.isDebugEnabled()) {
+                        log.debug("Valid salt value not found on POST to URL : " + httpReq.getServletPath());
+                    }
+                    throw new ServletException("Security Violation");
+                }
 
-            String salt = httpReq.getParameter("salt");
-            SaltCache saltCache = SaltCache.getInstance();
-            if (salt == null || !Objects.equals(saltCache.get(salt), userId)) {
+                // Remove salt from cache after successful validation
+                saltCache.remove(salt);
                 if (log.isDebugEnabled()) {
-                    log.debug("Valid salt value not found on POST to URL : " + httpReq.getServletPath());
+                    log.debug("Salt used and invalidated: " + salt);
                 }
-                throw new ServletException("Security Violation");
             }
         }
 
@@ -70,8 +84,6 @@ public void doFilter(ServletRequest request, ServletResponse response,
 
     @Override
     public void init(FilterConfig filterConfig) throws ServletException {
-
-        // Construct our list of ignored urls
         String urls = WebloggerConfig.getProperty("salt.ignored.urls");
         ignored = Set.of(StringUtils.stripAll(StringUtils.split(urls, ",")));
     }
@@ -82,16 +94,10 @@ public void destroy() {
 
     /**
      * Checks if this is an ignored url defined in the salt.ignored.urls property
-     * @param theUrl the the url
+     * @param theUrl the url
      * @return true, if is ignored resource
      */
     private boolean isIgnoredURL(String theUrl) {
-        int i = theUrl.lastIndexOf('/');
-
-        // If it's not a resource then don't ignore it
-        if (i <= 0 || i == theUrl.length() - 1) {
-            return false;
-        }
-        return ignored.contains(theUrl.substring(i + 1));
+        return ignored.contains(theUrl);
     }
 }

app/src/test/java/org/apache/roller/weblogger/ui/core/filters/LoadSaltFilterTest.java

@@ -0,0 +1,88 @@
+package org.apache.roller.weblogger.ui.core.filters;
+
+import org.apache.roller.weblogger.pojos.User;
+import org.apache.roller.weblogger.ui.core.RollerSession;
+import org.apache.roller.weblogger.ui.rendering.util.cache.SaltCache;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.mockito.Mock;
+import org.mockito.MockedStatic;
+import org.mockito.MockitoAnnotations;
+
+import javax.servlet.FilterChain;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import static org.mockito.Mockito.*;
+
+public class LoadSaltFilterTest {
+
+    private LoadSaltFilter filter;
+
+    @Mock
+    private HttpServletRequest request;
+
+    @Mock
+    private HttpServletResponse response;
+
+    @Mock
+    private FilterChain chain;
+
+    @Mock
+    private RollerSession rollerSession;
+
+    @Mock
+    private SaltCache saltCache;
+
+    @BeforeEach
+    public void setUp() {
+        MockitoAnnotations.initMocks(this);
+        filter = new LoadSaltFilter();
+    }
+
+    @Test
+    public void testDoFilterGeneratesSalt() throws Exception {
+        try (MockedStatic<RollerSession> mockedRollerSession = mockStatic(RollerSession.class);
+             MockedStatic<SaltCache> mockedSaltCache = mockStatic(SaltCache.class)) {
+
+            mockedRollerSession.when(() -> RollerSession.getRollerSession(request)).thenReturn(rollerSession);
+            mockedSaltCache.when(SaltCache::getInstance).thenReturn(saltCache);
+
+            when(rollerSession.getAuthenticatedUser()).thenReturn(new TestUser("userId"));
+
+            filter.doFilter(request, response, chain);
+
+            verify(request).setAttribute(eq("salt"), anyString());
+            verify(saltCache).put(anyString(), eq("userId"));
+            verify(chain).doFilter(request, response);
+        }
+    }
+
+    @Test
+    public void testDoFilterWithNullRollerSession() throws Exception {
+        try (MockedStatic<RollerSession> mockedRollerSession = mockStatic(RollerSession.class);
+             MockedStatic<SaltCache> mockedSaltCache = mockStatic(SaltCache.class)) {
+
+            mockedRollerSession.when(() -> RollerSession.getRollerSession(request)).thenReturn(null);
+            mockedSaltCache.when(SaltCache::getInstance).thenReturn(saltCache);
+
+            filter.doFilter(request, response, chain);
+
+            verify(request, never()).setAttribute(eq("salt"), anyString());
+            verify(saltCache, never()).put(anyString(), anyString());
+            verify(chain).doFilter(request, response);
+        }
+    }
+
+    private static class TestUser extends User {
+        private final String id;
+
+        TestUser(String id) {
+            this.id = id;
+        }
+
+        public String getId() {
+            return id;
+        }
+    }
+}

app/src/test/java/org/apache/roller/weblogger/ui/core/filters/ValidateSaltFilterTest.java

@@ -0,0 +1,179 @@
+package org.apache.roller.weblogger.ui.core.filters;
+
+import org.apache.roller.weblogger.config.WebloggerConfig;
+import org.apache.roller.weblogger.pojos.User;
+import org.apache.roller.weblogger.ui.core.RollerSession;
+import org.apache.roller.weblogger.ui.rendering.util.cache.SaltCache;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.mockito.Mock;
+import org.mockito.MockedStatic;
+import org.mockito.MockitoAnnotations;
+
+import javax.servlet.FilterChain;
+import javax.servlet.FilterConfig;
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.Mockito.*;
+
+public class ValidateSaltFilterTest {
+
+    private ValidateSaltFilter filter;
+
+    @Mock
+    private HttpServletRequest request;
+
+    @Mock
+    private HttpServletResponse response;
+
+    @Mock
+    private FilterChain chain;
+
+    @Mock
+    private RollerSession rollerSession;
+
+    @Mock
+    private SaltCache saltCache;
+
+    @BeforeEach
+    public void setUp() {
+        MockitoAnnotations.openMocks(this);
+        filter = new ValidateSaltFilter();
+    }
+
+    @Test
+    public void testDoFilterWithGetMethod() throws Exception {
+        when(request.getMethod()).thenReturn("GET");
+        StringBuffer requestURL = new StringBuffer("https://example.com/app/ignoredurl");
+        when(request.getRequestURL()).thenReturn(requestURL);
+
+        filter.doFilter(request, response, chain);
+
+        verify(chain).doFilter(request, response);
+    }
+
+    @Test
+    public void testDoFilterWithPostMethodAndValidSalt() throws Exception {
+        try (MockedStatic<RollerSession> mockedRollerSession = mockStatic(RollerSession.class);
+             MockedStatic<SaltCache> mockedSaltCache = mockStatic(SaltCache.class)) {
+
+            mockedRollerSession.when(() -> RollerSession.getRollerSession(request)).thenReturn(rollerSession);
+            mockedSaltCache.when(SaltCache::getInstance).thenReturn(saltCache);
+
+            when(request.getMethod()).thenReturn("POST");
+            when(request.getParameter("salt")).thenReturn("validSalt");
+            when(saltCache.get("validSalt")).thenReturn("userId");
+            when(rollerSession.getAuthenticatedUser()).thenReturn(new TestUser("userId"));
+            StringBuffer requestURL = new StringBuffer("https://example.com/app/ignoredurl");
+            when(request.getRequestURL()).thenReturn(requestURL);
+
+            filter.doFilter(request, response, chain);
+
+            verify(chain).doFilter(request, response);
+            verify(saltCache).remove("validSalt");
+        }
+    }
+
+    @Test
+    public void testDoFilterWithPostMethodAndInvalidSalt() throws Exception {
+        try (MockedStatic<RollerSession> mockedRollerSession = mockStatic(RollerSession.class);
+             MockedStatic<SaltCache> mockedSaltCache = mockStatic(SaltCache.class)) {
+
+            mockedRollerSession.when(() -> RollerSession.getRollerSession(request)).thenReturn(rollerSession);
+            mockedSaltCache.when(SaltCache::getInstance).thenReturn(saltCache);
+
+            when(request.getMethod()).thenReturn("POST");
+            when(request.getParameter("salt")).thenReturn("invalidSalt");
+            when(saltCache.get("invalidSalt")).thenReturn(null);
+            StringBuffer requestURL = new StringBuffer("https://example.com/app/ignoredurl");
+            when(request.getRequestURL()).thenReturn(requestURL);
+
+            assertThrows(ServletException.class, () -> {
+                filter.doFilter(request, response, chain);
+            });
+        }
+    }
+
+    @Test
+    public void testDoFilterWithPostMethodAndMismatchedUserId() throws Exception {
+        try (MockedStatic<RollerSession> mockedRollerSession = mockStatic(RollerSession.class);
+             MockedStatic<SaltCache> mockedSaltCache = mockStatic(SaltCache.class)) {
+
+            mockedRollerSession.when(() -> RollerSession.getRollerSession(request)).thenReturn(rollerSession);
+            mockedSaltCache.when(SaltCache::getInstance).thenReturn(saltCache);
+
+            when(request.getMethod()).thenReturn("POST");
+            when(request.getParameter("salt")).thenReturn("validSalt");
+            when(saltCache.get("validSalt")).thenReturn("differentUserId");
+            when(rollerSession.getAuthenticatedUser()).thenReturn(new TestUser("userId"));
+            StringBuffer requestURL = new StringBuffer("https://example.com/app/ignoredurl");
+            when(request.getRequestURL()).thenReturn(requestURL);
+
+            assertThrows(ServletException.class, () -> {
+                filter.doFilter(request, response, chain);
+            });
+        }
+    }
+
+    @Test
+    public void testDoFilterWithPostMethodAndNullRollerSession() throws Exception {
+        try (MockedStatic<RollerSession> mockedRollerSession = mockStatic(RollerSession.class);
+             MockedStatic<SaltCache> mockedSaltCache = mockStatic(SaltCache.class)) {
+
+            mockedRollerSession.when(() -> RollerSession.getRollerSession(request)).thenReturn(null);
+            mockedSaltCache.when(SaltCache::getInstance).thenReturn(saltCache);
+
+            when(request.getMethod()).thenReturn("POST");
+            when(request.getParameter("salt")).thenReturn("validSalt");
+            when(saltCache.get("validSalt")).thenReturn("");
+            StringBuffer requestURL = new StringBuffer("https://example.com/app/ignoredurl");
+            when(request.getRequestURL()).thenReturn(requestURL);
+
+            filter.doFilter(request, response, chain);
+
+            verify(saltCache, never()).remove("validSalt");
+        }
+    }
+
+    @Test
+    public void testDoFilterWithIgnoredURL() throws Exception {
+        try (MockedStatic<RollerSession> mockedRollerSession = mockStatic(RollerSession.class);
+             MockedStatic<SaltCache> mockedSaltCache = mockStatic(SaltCache.class);
+             MockedStatic<WebloggerConfig> mockedWebloggerConfig = mockStatic(WebloggerConfig.class)) {
+
+            mockedRollerSession.when(() -> RollerSession.getRollerSession(request)).thenReturn(rollerSession);
+            mockedSaltCache.when(SaltCache::getInstance).thenReturn(saltCache);
+            mockedWebloggerConfig.when(() -> WebloggerConfig.getProperty("salt.ignored.urls"))
+                    .thenReturn("https://example.com/app/ignoredurl?param1=value1&m2=value2");
+
+            when(request.getMethod()).thenReturn("POST");
+            StringBuffer requestURL = new StringBuffer("https://example.com/app/ignoredurl");
+            when(request.getRequestURL()).thenReturn(requestURL);
+            when(request.getQueryString()).thenReturn("param1=value1&m2=value2");
+            when(request.getParameter("salt")).thenReturn(null);  // No salt provided
+
+            filter.init(mock(FilterConfig.class));
+            filter.doFilter(request, response, chain);
+
+            verify(chain).doFilter(request, response);
+            verify(saltCache, never()).get(anyString());
+            verify(saltCache, never()).remove(anyString());
+        }
+    }
+
+    private static class TestUser extends User {
+        private final String id;
+
+        TestUser(String id) {
+            this.id = id;
+        }
+
+        @Override
+        public String getId() {
+            return id;
+        }
+    }
+}