[GEOS-11271] Upgrade spring-security to 5.8

GeoCat · Aug 5, 2024 · b1f82b2 · b1f82b2
1 parent 36e78a7
commit b1f82b2
Show file tree

Hide file tree

Showing 6 changed files with 284 additions and 105 deletions.
diff --git a/src/main/src/main/java/org/geoserver/security/RESTfulDefinitionSource.java b/src/main/src/main/java/org/geoserver/security/RESTfulDefinitionSource.java
@@ -15,12 +15,13 @@
 import java.util.List;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import javax.servlet.http.HttpServletRequest;
 import org.geoserver.security.impl.RESTAccessRuleDAO;
 import org.geotools.util.logging.Logging;
 import org.springframework.security.access.ConfigAttribute;
 import org.springframework.security.access.SecurityConfig;
-import org.springframework.security.web.FilterInvocation;
 import org.springframework.security.web.access.intercept.FilterInvocationSecurityMetadataSource;
+import org.springframework.security.web.util.UrlUtils;
 import org.springframework.util.StringUtils;
 
 /** @author Chris Berry http://opensource.atlassian.com/projects/spring/browse/SEC-531 */
@@ -31,7 +32,7 @@ public class RESTfulDefinitionSource implements FilterInvocationSecurityMetadata
     private static final String[] validMethodNames = {"GET", "PUT", "DELETE", "POST"};
 
     /** Underlying SecurityMetedataSource object */
-    private RESTfulPathBasedFilterInvocationDefinitionMap delegate = null;
+    private RESTfulDefinitionSourceDelegateMap delegate = null;
     /** rest access rules dao */
     private RESTAccessRuleDAO dao;
 
@@ -41,11 +42,12 @@ public Collection<ConfigAttribute> getAttributes(Object object)
             throws IllegalArgumentException {
 
         if ((object == null) || !this.supports(object.getClass())) {
-            throw new IllegalArgumentException("Object must be a FilterInvocation");
+            throw new IllegalArgumentException("Object must be a HTTPServletRequest");
         }
 
-        String url = ((FilterInvocation) object).getRequestUrl();
-        String method = ((FilterInvocation) object).getHttpRequest().getMethod();
+        HttpServletRequest request = (HttpServletRequest) object;
+        String url = UrlUtils.buildRequestUrl(request);
+        String method = request.getMethod();
 
         return delegate().lookupAttributes(cleanURL(url), method);
     }
@@ -67,7 +69,7 @@ public Collection<ConfigAttribute> getAllConfigAttributes() {
 
     @Override
     public boolean supports(Class<?> clazz) {
-        return FilterInvocation.class.isAssignableFrom(clazz);
+        return HttpServletRequest.class.isAssignableFrom(clazz);
     }
 
     public RESTfulDefinitionSource(RESTAccessRuleDAO dao) {
@@ -81,10 +83,10 @@ public void reload() {
         delegate = null;
     }
 
-    RESTfulPathBasedFilterInvocationDefinitionMap delegate() {
+    RESTfulDefinitionSourceDelegateMap delegate() {
         if (delegate == null || dao.isModified()) {
             synchronized (this) {
-                delegate = new RESTfulPathBasedFilterInvocationDefinitionMap();
+                delegate = new RESTfulDefinitionSourceDelegateMap();
                 for (String rule : dao.getRules()) {
                     processPathList(rule);
                 }

diff --git a/...thBasedFilterInvocationDefinitionMap.java → ...y/RESTfulDefinitionSourceDelegateMap.java b/...thBasedFilterInvocationDefinitionMap.java → ...y/RESTfulDefinitionSourceDelegateMap.java
@@ -15,18 +15,14 @@
 import java.util.logging.Logger;
 import org.geotools.util.logging.Logging;
 import org.springframework.security.access.ConfigAttribute;
-import org.springframework.security.web.FilterInvocation;
-import org.springframework.security.web.access.intercept.FilterInvocationSecurityMetadataSource;
 import org.springframework.util.AntPathMatcher;
 import org.springframework.util.PathMatcher;
 import org.springframework.util.StringUtils;
 
 /** @author Chris Berry http://opensource.atlassian.com/projects/spring/browse/SEC-531 */
-public class RESTfulPathBasedFilterInvocationDefinitionMap
-        implements FilterInvocationSecurityMetadataSource {
+public class RESTfulDefinitionSourceDelegateMap {
 
-    private static Logger log =
-            Logging.getLogger(RESTfulPathBasedFilterInvocationDefinitionMap.class);
+    private static Logger log = Logging.getLogger(RESTfulDefinitionSourceDelegateMap.class);
 
     // ~ Instance fields
     // ================================================================================================
@@ -35,13 +31,6 @@ public class RESTfulPathBasedFilterInvocationDefinitionMap
     private PathMatcher pathMatcher = new AntPathMatcher();
     private boolean convertUrlToLowercaseBeforeComparison = false;
 
-    // ~ Methods
-    // ========================================================================================================
-    @Override
-    public boolean supports(Class<?> clazz) {
-        return FilterInvocation.class.isAssignableFrom(clazz);
-    }
-
     public void addSecureUrl(
             String antPath, String[] httpMethods, Collection<ConfigAttribute> attrs) {
         requestMap.add(new EntryHolder(antPath, httpMethods, attrs));
@@ -57,12 +46,6 @@ public void addSecureUrl(
         }
     }
 
-    public void addSecureUrl(String antPath, Collection<ConfigAttribute> attrs) {
-        throw new IllegalArgumentException(
-                "addSecureUrl(String, Collection<ConfigAttribute> ) is INVALID for RESTfulDefinitionSource");
-    }
-
-    @Override
     public Collection<ConfigAttribute> getAllConfigAttributes() {
         Set<ConfigAttribute> set = new HashSet<>();
 
@@ -74,10 +57,6 @@ public Collection<ConfigAttribute> getAllConfigAttributes() {
         // return set.iterator();
     }
 
-    public int getMapSize() {
-        return this.requestMap.size();
-    }
-
     public boolean isConvertUrlToLowercaseBeforeComparison() {
         return convertUrlToLowercaseBeforeComparison;
     }
@@ -87,24 +66,6 @@ public void setConvertUrlToLowercaseBeforeComparison(
         this.convertUrlToLowercaseBeforeComparison = convertUrlToLowercaseBeforeComparison;
     }
 
-    @Override
-    public Collection<ConfigAttribute> getAttributes(Object object)
-            throws IllegalArgumentException {
-        if ((object == null) || !this.supports(object.getClass())) {
-            throw new IllegalArgumentException("Object must be a FilterInvocation");
-        }
-
-        String url = ((FilterInvocation) object).getRequestUrl();
-        String method = ((FilterInvocation) object).getHttpRequest().getMethod();
-
-        return this.lookupAttributes(url, method);
-    }
-
-    public Collection<ConfigAttribute> lookupAttributes(String url) {
-        throw new IllegalArgumentException(
-                "lookupAttributes(String url) is INVALID for RESTfulDefinitionSource");
-    }
-
     public Collection<ConfigAttribute> lookupAttributes(String url, String httpMethod) {
         // Strip anything after a question mark symbol, as per SEC-161. See also SEC-321
         int firstQuestionMarkIndex = url.indexOf("?");
@@ -127,9 +88,9 @@ public Collection<ConfigAttribute> lookupAttributes(String url, String httpMetho
             }
         }
 
-        Iterator iter = requestMap.iterator();
+        Iterator<EntryHolder> iter = requestMap.iterator();
         while (iter.hasNext()) {
-            EntryHolder entryHolder = (EntryHolder) iter.next();
+            EntryHolder entryHolder = iter.next();
 
             String antPath = entryHolder.getAntPath();
             String[] methodList = entryHolder.getHttpMethodList();

diff --git a/...rc/main/java/org/geoserver/security/filter/GeoServerSecurityContextPersistenceFilter.java b/...rc/main/java/org/geoserver/security/filter/GeoServerSecurityContextPersistenceFilter.java
@@ -7,6 +7,7 @@
 package org.geoserver.security.filter;
 
 import java.io.IOException;
+import java.util.function.Supplier;
 import javax.servlet.FilterChain;
 import javax.servlet.ServletException;
 import javax.servlet.http.HttpServletRequest;
@@ -67,8 +68,9 @@ public void doFilterInternal(
                         }
                         request.setAttribute(FILTER_APPLIED, Boolean.TRUE);
                         try {
-                            SecurityContext securityContext = repo.loadContext(request).get();
-                            SecurityContextHolder.setContext(securityContext);
+                            Supplier<SecurityContext> securityContext =
+                                    repo.loadDeferredContext(request);
+                            SecurityContextHolder.setDeferredContext(securityContext);
                             chain.doFilter(request, response);
                         } finally {
                             SecurityContext contextAfterChainExecution =