Add tests for Secured ResourceInfo objects.

Signed-off-by: Erik Merkle <emerkle@boundlessgeo.com>

src/main/src/main/java/org/geoserver/security/SecureCatalogImpl.java

@@ -36,9 +36,7 @@
 import org.geoserver.catalog.StoreInfo;
 import org.geoserver.catalog.StyleInfo;
 import org.geoserver.catalog.ValidationResult;
-import org.geoserver.catalog.WMSLayerInfo;
 import org.geoserver.catalog.WMSStoreInfo;
-import org.geoserver.catalog.WMTSLayerInfo;
 import org.geoserver.catalog.WMTSStoreInfo;
 import org.geoserver.catalog.WorkspaceInfo;
 import org.geoserver.catalog.event.CatalogListener;
@@ -49,7 +47,6 @@
 import org.geoserver.platform.GeoServerResourceLoader;
 import org.geoserver.security.decorators.DecoratingCatalogFactory;
 import org.geoserver.security.decorators.SecuredCoverageInfo;
-import org.geoserver.security.decorators.SecuredCoverageStoreInfo;
 import org.geoserver.security.decorators.SecuredDataStoreInfo;
 import org.geoserver.security.decorators.SecuredFeatureTypeInfo;
 import org.geoserver.security.decorators.SecuredLayerGroupInfo;
@@ -71,6 +68,10 @@
 import com.google.common.base.Function;
 import com.google.common.collect.ImmutableList;
 
+import org.geoserver.security.decorators.SecuredCoverageStoreInfo;
+import org.geoserver.security.decorators.SecuredWMSStoreInfo;
+import org.geoserver.security.decorators.SecuredWMTSStoreInfo;
+
 /**
  * Wraps the catalog and applies the security directives provided by a {@link ResourceAccessManager}
  * or a {@link DataAccessManager} registered in the Spring application context
@@ -1107,6 +1108,12 @@ static ResourceInfo unwrap(ResourceInfo info) {
     static StoreInfo unwrap(StoreInfo info) {
         if(info instanceof SecuredDataStoreInfo)
             return ((SecuredDataStoreInfo) info).unwrap(StoreInfo.class);
+        if (info instanceof SecuredCoverageStoreInfo)
+            return ((SecuredCoverageStoreInfo) info).unwrap(StoreInfo.class);
+        if (info instanceof SecuredWMSStoreInfo)
+            return ((SecuredWMSStoreInfo) info).unwrap(StoreInfo.class);
+        if (info instanceof SecuredWMTSStoreInfo)
+            return ((SecuredWMTSStoreInfo) info).unwrap(StoreInfo.class);
         return info;
     }
 

src/main/src/main/java/org/geoserver/security/decorators/DefaultSecureCatalogFactory.java

@@ -45,20 +45,22 @@ public Object secure(Object object, WrapperPolicy policy) {
             return null;
 
         Class clazz = object.getClass();
+        // for each supported Info type, log a warning if the object to be secured is already secured. If this happens,
+        // it could lead to a StackOverflowError if the object is re-wrapped, over time, over and over agian.
         if (CoverageInfo.class.isAssignableFrom(clazz))
-            return new SecuredCoverageInfo(unwrap((CoverageInfo) object), policy);
+            return new SecuredCoverageInfo(logIfSecured((CoverageInfo) object), policy);
         else if (CoverageStoreInfo.class.isAssignableFrom(clazz))
-            return new SecuredCoverageStoreInfo(unwrap((CoverageStoreInfo) object), policy);
+            return new SecuredCoverageStoreInfo(logIfSecured((CoverageStoreInfo) object), policy);
         else if (DataStoreInfo.class.isAssignableFrom(clazz))
-            return new SecuredDataStoreInfo(unwrap((DataStoreInfo) object), policy);
+            return new SecuredDataStoreInfo(logIfSecured((DataStoreInfo) object), policy);
         else if (FeatureTypeInfo.class.isAssignableFrom(clazz))
-            return new SecuredFeatureTypeInfo(unwrap((FeatureTypeInfo) object), policy);
+            return new SecuredFeatureTypeInfo(logIfSecured((FeatureTypeInfo) object), policy);
         else if (LayerInfo.class.isAssignableFrom(clazz))
-            return new SecuredLayerInfo(unwrap((LayerInfo) object), policy);
+            return new SecuredLayerInfo(logIfSecured((LayerInfo) object), policy);
         else if (WMSLayerInfo.class.isAssignableFrom(clazz))
-            return new SecuredWMSLayerInfo(unwrap((WMSLayerInfo) object), policy);
+            return new SecuredWMSLayerInfo(logIfSecured((WMSLayerInfo) object), policy);
         else if (WMTSLayerInfo.class.isAssignableFrom(clazz))
-            return new SecuredWMTSLayerInfo(unwrap((WMTSLayerInfo) object), policy);
+            return new SecuredWMTSLayerInfo(logIfSecured((WMTSLayerInfo) object), policy);
         else
             throw new IllegalArgumentException("Don't know how to wrap " + object);
     }
@@ -77,84 +79,114 @@ private void logDoubleWrap(Object unwrapped, Object orig) {
         LOGGER.warning(msg);
     }
 
-    private void logDoubleWrap(final DataStoreInfo store) {
-        int proxies = 0;
-        int secured = 0;
-        DataStoreInfo object = store;
-        while (true) {
-            DataStoreInfo unwrapped;
-            if (object instanceof SecuredDataStoreInfo) {
-                secured++;
-                unwrapped = ((SecuredDataStoreInfo) object).unwrap(DataStoreInfo.class);
-            } else {
-                unwrapped = ModificationProxy.unwrap(object);
-                boolean isProxy = object != unwrapped;
-                if (isProxy) {
-                    proxies++;
-                } else {
-                    break;
-                }
-            }
-            object = unwrapped;
-        }
-
-        String msg = String.format("Double securing %s: proxies: %,d, SecuredDataStoreInfos: %,d",
-                store.getName(), proxies, secured);
-        LOGGER.warning(msg);
-    }
-
-    private WMTSLayerInfo unwrap(WMTSLayerInfo object) {
+    /**
+     * Generates a warning log if the Info object is already wrapped with a Secured decorator. This method is only
+     * intended to log a situation where a Catalog Info object is being secured, but is already secured. Repeated calls
+     * to this will keep adding additional wrapper layers and may eventually cause a StackOverflowError. The log
+     * generated is merely to aid in finding the real issue, as opposed to masking it here.
+     * @param object {@link WMTSLayerInfo} to check.
+     * @return The original object to be checked.
+     */
+    private WMTSLayerInfo logIfSecured(WMTSLayerInfo object) {
         WMTSLayerInfo unwrapped = ModificationProxy.unwrap(object);
         if(unwrapped instanceof SecuredWMTSLayerInfo) {
             logDoubleWrap(unwrapped, object);
         }
         return object;
     }
 
-    private WMSLayerInfo unwrap(WMSLayerInfo object) {
+    /**
+     * Generates a warning log if the Info object is already wrapped with a Secured decorator. This method is only
+     * intended to log a situation where a Catalog Info object is being secured, but is already secured. Repeated calls
+     * to this will keep adding additional wrapper layers and may eventually cause a StackOverflowError. The log
+     * generated is merely to aid in finding the real issue, as opposed to masking it here.
+     * @param object {@link WMSLayerInfo} to check.
+     * @return The original object to be checked.
+     */
+    private WMSLayerInfo logIfSecured(WMSLayerInfo object) {
         WMSLayerInfo unwrapped = ModificationProxy.unwrap(object);
         if(unwrapped instanceof SecuredWMSLayerInfo) {
             logDoubleWrap(unwrapped, object);
         }
         return object;
     }
 
-    private LayerInfo unwrap(LayerInfo object) {
+    /**
+     * Generates a warning log if the Info object is already wrapped with a Secured decorator. This method is only
+     * intended to log a situation where a Catalog Info object is being secured, but is already secured. Repeated calls
+     * to this will keep adding additional wrapper layers and may eventually cause a StackOverflowError. The log
+     * generated is merely to aid in finding the real issue, as opposed to masking it here.
+     * @param object {@link LayerInfo} to check.
+     * @return The original object to be checked.
+     */
+    private LayerInfo logIfSecured(LayerInfo object) {
         LayerInfo unwrapped = ModificationProxy.unwrap(object);
         if(unwrapped instanceof SecuredLayerInfo) {
             logDoubleWrap(unwrapped, object);
         }
         return object;
     }
 
-    private FeatureTypeInfo unwrap(FeatureTypeInfo object) {
+    /**
+     * Generates a warning log if the Info object is already wrapped with a Secured decorator. This method is only
+     * intended to log a situation where a Catalog Info object is being secured, but is already secured. Repeated calls
+     * to this will keep adding additional wrapper layers and may eventually cause a StackOverflowError. The log
+     * generated is merely to aid in finding the real issue, as opposed to masking it here.
+     * @param object {@link FeatureTypeInfo} to check.
+     * @return The original object to be checked.
+     */
+    private FeatureTypeInfo logIfSecured(FeatureTypeInfo object) {
         FeatureTypeInfo unwrapped = ModificationProxy.unwrap(object);
         if(unwrapped instanceof SecuredFeatureTypeInfo) {
             logDoubleWrap(unwrapped, object);
         }
         return object;
     }
 
-    private CoverageStoreInfo unwrap(CoverageStoreInfo object) {
+    /**
+     * Generates a warning log if the Info object is already wrapped with a Secured decorator. This method is only
+     * intended to log a situation where a Catalog Info object is being secured, but is already secured. Repeated calls
+     * to this will keep adding additional wrapper layers and may eventually cause a StackOverflowError. The log
+     * generated is merely to aid in finding the real issue, as opposed to masking it here.
+     * @param object {@link CoverageStoreInfo} to check.
+     * @return The original object to be checked.
+     */
+    private CoverageStoreInfo logIfSecured(CoverageStoreInfo object) {
         CoverageStoreInfo unwrapped = ModificationProxy.unwrap(object);
         if(unwrapped instanceof SecuredCoverageStoreInfo) {
             logDoubleWrap(unwrapped, object);
         }
         return object;
     }
 
-    private CoverageInfo unwrap(CoverageInfo object) {
+    /**
+     * Generates a warning log if the Info object is already wrapped with a Secured decorator. This method is only
+     * intended to log a situation where a Catalog Info object is being secured, but is already secured. Repeated calls
+     * to this will keep adding additional wrapper layers and may eventually cause a StackOverflowError. The log
+     * generated is merely to aid in finding the real issue, as opposed to masking it here.
+     * @param object {@link CoverageInfo} to check.
+     * @return The original object to be checked.
+     */
+    private CoverageInfo logIfSecured(CoverageInfo object) {
         CoverageInfo unwrapped = ModificationProxy.unwrap(object);
         if(unwrapped instanceof SecuredDataStoreInfo) {
             logDoubleWrap(unwrapped, object);
         }
         return object;
     }
 
-    private DataStoreInfo unwrap(DataStoreInfo object) {
+    /**
+     * Generates a warning log if the Info object is already wrapped with a Secured decorator. This method is only
+     * intended to log a situation where a Catalog Info object is being secured, but is already secured. Repeated calls
+     * to this will keep adding additional wrapper layers and may eventually cause a StackOverflowError. The log
+     * generated is merely to aid in finding the real issue, as opposed to masking it here.
+     * @param object {@link DataStoreInfo} to check.
+     * @return The original object to be checked.
+     */
+    private DataStoreInfo logIfSecured(DataStoreInfo object) {
         DataStoreInfo unwrapped = ModificationProxy.unwrap(object);
         if(unwrapped instanceof SecuredDataStoreInfo) {
-            logDoubleWrap(object);
+            logDoubleWrap(unwrapped, object);
         }
         return object;
     }

src/main/src/test/java/org/geoserver/security/decorators/SecuredCoverageInfoTest.java

@@ -0,0 +1,49 @@
+/* (c) 2018 Open Source Geospatial Foundation - all rights reserved
+ * This code is licensed under the GPL 2.0 license, available at the root
+ * application directory.
+ */
+package org.geoserver.security.decorators;
+
+
+import org.geoserver.catalog.CoverageInfo;
+import org.geoserver.catalog.CoverageStoreInfo;
+import org.geoserver.catalog.impl.CoverageInfoImpl;
+import org.geoserver.catalog.impl.CoverageStoreInfoImpl;
+
+public class SecuredCoverageInfoTest extends SecuredResourceInfoTest<CoverageInfo, SecuredCoverageInfo> {
+
+    @Override
+    CoverageInfo createDelegate()  {
+        final CoverageInfo info = new CoverageInfoImpl(getCatalog());
+        final CoverageStoreInfo storeInfo = new CoverageStoreInfoImpl(getCatalog());
+        info.setStore(storeInfo);
+        return info;
+    }
+
+    @Override
+    SecuredCoverageInfo createSecuredDecorator(CoverageInfo delegate) {
+        return new SecuredCoverageInfo(delegate, policy);
+    }
+
+    @Override
+    Class getDelegateClass() {
+        return CoverageInfo.class;
+    }
+
+    @Override
+    Class getSecuredDecoratorClass() {
+        return SecuredCoverageInfo.class;
+    }
+
+    @Override
+    Class getSecuredStoreInfoClass() {
+        return SecuredCoverageStoreInfo.class;
+    }
+
+    @Override
+    int getStackOverflowCount() {
+        return 500;
+    }
+
+
+}

src/main/src/test/java/org/geoserver/security/decorators/SecuredFeatureTypeInfoTest.java

@@ -0,0 +1,45 @@
+/* (c) 2018 Open Source Geospatial Foundation - all rights reserved
+ * This code is licensed under the GPL 2.0 license, available at the root
+ * application directory.
+ */
+package org.geoserver.security.decorators;
+
+import org.geoserver.catalog.FeatureTypeInfo;
+import org.geoserver.catalog.impl.DataStoreInfoImpl;
+import org.geoserver.catalog.impl.FeatureTypeInfoImpl;
+
+public class SecuredFeatureTypeInfoTest extends SecuredResourceInfoTest<FeatureTypeInfo, SecuredFeatureTypeInfo> {
+
+    @Override
+    FeatureTypeInfo createDelegate() {
+        FeatureTypeInfo info = new FeatureTypeInfoImpl(getCatalog());
+        info.setStore(new DataStoreInfoImpl(getCatalog()));
+        return info;
+    }
+
+    @Override
+    SecuredFeatureTypeInfo createSecuredDecorator(FeatureTypeInfo delegate) {
+        return new SecuredFeatureTypeInfo(delegate, policy);
+    }
+
+    @Override
+    Class getDelegateClass() {
+        return FeatureTypeInfo.class;
+    }
+
+    @Override
+    Class getSecuredDecoratorClass() {
+        return SecuredFeatureTypeInfo.class;
+    }
+
+    @Override
+    Class getSecuredStoreInfoClass() {
+        return SecuredDataStoreInfo.class;
+    }
+
+    @Override
+    int getStackOverflowCount() {
+        return 500;
+    }
+
+}