Fix SWORDv2 classes that use comma-separated configs (and ensure they all use ConfigurationService). Correct insecure default value for "on-behalf-of.enable".

dspace-swordv2/src/main/java/org/dspace/sword2/GenericStatementDisseminator.java

@@ -10,7 +10,6 @@ package org.dspace.sword2;
 import org.dspace.content.Bitstream;
 import org.dspace.content.Bundle;
 import org.dspace.content.Item;
-import org.dspace.core.ConfigurationManager;
 import org.dspace.core.Context;
 import org.swordapp.server.OriginalDeposit;
 import org.swordapp.server.ResourcePart;
@@ -22,6 +21,7 @@ import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import org.dspace.services.factory.DSpaceServicesFactory;
 
 public abstract class GenericStatementDisseminator
         implements SwordStatementDisseminator
@@ -177,15 +177,14 @@ public abstract class GenericStatementDisseminator
 
     private List<String> getIncludeBundles()
     {
-        String cfg = ConfigurationManager
+        String[] bundles = DSpaceServicesFactory.getInstance().getConfigurationService()
-                .getProperty("swordv2-server", "statement.bundles");
+                .getArrayProperty("swordv2-server.statement.bundles");
-        if (cfg == null || "".equals(cfg))
+        if (bundles == null || bundles.length==0)
         {
-            cfg = "ORIGINAL, SWORD";
+            bundles = new String[] {"ORIGINAL", "SWORD"};
         }
-        String[] bits = cfg.split(",");
         List<String> include = new ArrayList<String>();
-        for (String bit : bits)
+        for (String bit : bundles)
         {
             String bundleName = bit.trim().toUpperCase();
             if (!include.contains(bundleName))
@@ -198,8 +197,8 @@ public abstract class GenericStatementDisseminator
 
     private String getOriginalDepositsBundle()
     {
-        String swordBundle = ConfigurationManager
+        String swordBundle = DSpaceServicesFactory.getInstance().getConfigurationService()
-                .getProperty("swordv2-server", "bundle.name");
+                .getProperty("swordv2-server.bundle.name");
         if (swordBundle == null)
         {
             swordBundle = "SWORD";

dspace-swordv2/src/main/java/org/dspace/sword2/SimpleDCEntryIngester.java

@@ -12,15 +12,17 @@ import org.dspace.authorize.AuthorizeException;
 import org.dspace.content.*;
 import org.dspace.content.factory.ContentServiceFactory;
 import org.dspace.content.service.WorkspaceItemService;
-import org.dspace.core.ConfigurationManager;
 import org.dspace.core.Context;
+import org.dspace.services.ConfigurationService;
+import org.dspace.services.factory.DSpaceServicesFactory;
 import org.swordapp.server.*;
 
-import java.io.IOException;
 import java.sql.SQLException;
 import java.util.Date;
 import java.util.List;
 import java.util.Map;
+import org.apache.commons.lang3.StringUtils;
+
 
 public class SimpleDCEntryIngester extends AbstractSimpleDC
         implements SwordEntryIngester
@@ -31,6 +33,9 @@ public class SimpleDCEntryIngester extends AbstractSimpleDC
     protected WorkspaceItemService workspaceItemService = ContentServiceFactory
             .getInstance().getWorkspaceItemService();
     
+    protected ConfigurationService configurationService = DSpaceServicesFactory
+            .getInstance().getConfigurationService();
+
     public SimpleDCEntryIngester()
     {
         this.loadMetadataMaps();
@@ -115,10 +120,9 @@ public class SimpleDCEntryIngester extends AbstractSimpleDC
     private void removeMetadata(Context context, Item item)
             throws DSpaceSwordException
     {
-        String raw = ConfigurationManager
+        String[] replaceableMetadata = configurationService
-                .getProperty("swordv2-server", "metadata.replaceable");
+                .getArrayProperty("swordv2-server.metadata.replaceable");
-        String[] parts = raw.split(",");
+        for (String part : replaceableMetadata)
-        for (String part : parts)
         {
             MetadataValueInfo info = this
                     .makeMetadataValueInfo(part.trim(), null);
@@ -340,12 +344,12 @@ public class SimpleDCEntryIngester extends AbstractSimpleDC
             VerboseDescription verboseDescription)
             throws DSpaceSwordException
     {
-        String field = ConfigurationManager
+        String field = configurationService
-                .getProperty("swordv2-server", "updated.field");
+                .getProperty("swordv2-server.updated.field");
-        if (field == null || "".equals(field))
+        if (StringUtils.isBlank(field))
         {
             throw new DSpaceSwordException(
-                    "No configuration, or configuration is invalid for: sword.updated.field");
+                    "No configuration, or configuration is invalid for: swordv2-server.updated.field");
         }
 
         MetadataValueInfo info = this.makeMetadataValueInfo(field, null);
@@ -389,12 +393,12 @@ public class SimpleDCEntryIngester extends AbstractSimpleDC
             return;
         }
 
-        String field = ConfigurationManager
+        String field = configurationService
-                .getProperty("swordv2-server", "slug.field");
+                .getProperty("swordv2-server.slug.field");
-        if (field == null || "".equals(field))
+        if (StringUtils.isBlank(field))
         {
             throw new DSpaceSwordException(
-                    "No configuration, or configuration is invalid for: sword.slug.field");
+                    "No configuration, or configuration is invalid for: swordv2-server.slug.field");
         }
 
         MetadataValueInfo info = this.makeMetadataValueInfo(field, null);

dspace-swordv2/src/main/java/org/dspace/sword2/SwordAuthenticator.java

@@ -7,10 +7,8 @@
  */
 package org.dspace.sword2;
 
-import org.dspace.authenticate.AuthenticationServiceImpl;
 import org.dspace.authenticate.factory.AuthenticateServiceFactory;
 import org.dspace.authenticate.service.AuthenticationService;
-import org.dspace.authorize.AuthorizeServiceImpl;
 import org.dspace.authorize.factory.AuthorizeServiceFactory;
 import org.dspace.authorize.service.AuthorizeService;
 import org.dspace.content.factory.ContentServiceFactory;
@@ -18,17 +16,17 @@ import org.dspace.content.service.CollectionService;
 import org.dspace.content.service.CommunityService;
 import org.dspace.content.service.ItemService;
 import org.dspace.core.Context;
-import org.dspace.core.ConfigurationManager;
 import org.dspace.core.LogManager;
 import org.dspace.core.Constants;
 import org.dspace.authenticate.AuthenticationMethod;
 import org.dspace.eperson.EPerson;
 import org.dspace.eperson.Group;
-import org.dspace.authorize.AuthorizeException;
 import org.dspace.content.*;
 import org.apache.log4j.Logger;
 import org.dspace.eperson.factory.EPersonServiceFactory;
 import org.dspace.eperson.service.EPersonService;
+import org.dspace.services.ConfigurationService;
+import org.dspace.services.factory.DSpaceServicesFactory;
 import org.swordapp.server.AuthCredentials;
 import org.swordapp.server.SwordAuthException;
 import org.swordapp.server.SwordError;
@@ -38,6 +36,7 @@ import java.sql.SQLException;
 import java.util.Iterator;
 import java.util.List;
 import java.util.ArrayList;
+import org.apache.commons.lang.StringUtils;
 
 /**
  * This class offers a thin wrapper for the default DSpace
@@ -69,6 +68,9 @@ public class SwordAuthenticator
     protected ItemService itemService = ContentServiceFactory.getInstance()
             .getItemService();
     
+    protected ConfigurationService configurationService = DSpaceServicesFactory
+            .getInstance().getConfigurationService();
+
     /**
      * Does the given username and password authenticate for the
      * given DSpace Context?
@@ -155,14 +157,14 @@ public class SwordAuthenticator
 
         // smooth out the OnBehalfOf request, so that empty strings are
         // treated as null
-        if ("".equals(obo))
+        if (StringUtils.isBlank(obo))
         {
             obo = null;
         }
 
         // first find out if we support on-behalf-of deposit
-        boolean mediated = ConfigurationManager
+        boolean mediated = configurationService
-                .getBooleanProperty("swordv2-server", "on-behalf-of.enable");
+                .getBooleanProperty("swordv2-server.on-behalf-of.enable", false);
         if (!mediated && obo != null)
         {
             // user is trying to do a mediated deposit on a repository which does not support it
@@ -1002,9 +1004,9 @@ public class SwordAuthenticator
     private boolean allowedToMediate(Context context)
     {
         // get the configuration
-        String mediatorCfg = ConfigurationManager
+        String[] mediators = configurationService
-                .getProperty("swordv2-server", "on-behalf-of.update.mediators");
+                .getArrayProperty("swordv2-server.on-behalf-of.update.mediators");
-        if (mediatorCfg == null)
+        if (mediators == null || mediators.length==0)
         {
             // if there's no explicit list of mediators, then anyone can mediate
             return true;
@@ -1019,7 +1021,6 @@ public class SwordAuthenticator
         String email = eperson.getEmail();
         String netid = eperson.getNetid();
 
-        String[] mediators = mediatorCfg.split(",");
         for (String mediator : mediators)
         {
             String m = mediator.trim();

dspace-swordv2/src/main/java/org/dspace/sword2/SwordConfigurationDSpace.java

@@ -14,18 +14,16 @@ import org.dspace.content.DSpaceObject;
 import org.dspace.content.Item;
 import org.dspace.content.factory.ContentServiceFactory;
 import org.dspace.content.service.BitstreamFormatService;
-import org.dspace.core.ConfigurationManager;
 import org.dspace.core.Context;
-import org.dspace.core.LegacyPluginServiceImpl;
-import org.jaxen.function.FalseFunction;
 import org.swordapp.server.SwordConfiguration;
 import org.swordapp.server.SwordError;
 
 import java.sql.SQLException;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.Properties;
+import org.apache.commons.lang.StringUtils;
-import java.util.Set;
+import org.dspace.services.ConfigurationService;
+import org.dspace.services.factory.DSpaceServicesFactory;
 
 public class SwordConfigurationDSpace implements SwordConfiguration
 {
@@ -36,6 +34,9 @@ public class SwordConfigurationDSpace implements SwordConfiguration
     protected BitstreamFormatService bitstreamFormatService = ContentServiceFactory
             .getInstance().getBitstreamFormatService();
     
+    protected ConfigurationService configurationService = DSpaceServicesFactory
+            .getInstance().getConfigurationService();
+
     /** whether we can be verbose */
     private boolean verbose = true;
 
@@ -71,59 +72,58 @@ public class SwordConfigurationDSpace implements SwordConfiguration
     public SwordConfigurationDSpace()
     {
         // set the max upload size
-        int mus = ConfigurationManager
+        int mus = configurationService
-                .getIntProperty("swordv2-server", "max-upload-size");
+                .getIntProperty("swordv2-server.max-upload-size");
         if (mus > 0)
         {
             this.maxUploadSize = mus;
         }
 
         // set the mediation value
-        this.mediated = ConfigurationManager
+        this.mediated = configurationService
-                .getBooleanProperty("swordv2-server", "on-behalf-of.enable");
+                .getBooleanProperty("swordv2-server.on-behalf-of.enable", false);
 
         // find out if we keep the original as bitstream
-        this.keepOriginal = ConfigurationManager
+        this.keepOriginal = configurationService
-                .getBooleanProperty("swordv2-server", "keep-original-package");
+                .getBooleanProperty("swordv2-server.keep-original-package");
 
         // get the sword bundle
-        String bundle = ConfigurationManager
+        String bundle = configurationService
-                .getProperty("swordv2-server", "bundle.name");
+                .getProperty("swordv2-server.bundle.name");
-        if (bundle != null && "".equals(bundle))
+        if (StringUtils.isBlank(bundle))
         {
             this.swordBundle = bundle;
         }
 
         // find out if we keep the package as a file in specified directory
-        this.keepPackageOnFailedIngest = ConfigurationManager
+        this.keepPackageOnFailedIngest = configurationService
-                .getBooleanProperty("swordv2-server", "keep-package-on-fail",
+                .getBooleanProperty("swordv2-server.keep-package-on-fail",
                         false);
 
         // get directory path and name
-        this.failedPackageDir = ConfigurationManager
+        this.failedPackageDir = configurationService
-                .getProperty("swordv2-server", "failed-package.dir");
+                .getProperty("swordv2-server.failed-package.dir");
 
         // Get the accepted formats
-        String acceptsProperty = ConfigurationManager
+        String[] acceptsFormats = configurationService
-                .getProperty("swordv2-server", "accepts");
+                .getArrayProperty("swordv2-server.accepts");
         swordaccepts = new ArrayList<String>();
-        if (acceptsProperty == null)
+        if (acceptsFormats == null)
         {
-            acceptsProperty = "application/zip";
+            acceptsFormats = new String[]{"application/zip"};
         }
-        for (String element : acceptsProperty.split(","))
+        for (String element : acceptsFormats)
         {
             swordaccepts.add(element.trim());
         }
 
         // find out if community deposit is allowed
-        this.allowCommunityDeposit = ConfigurationManager
+        this.allowCommunityDeposit = configurationService
-                .getBooleanProperty("swordv2-server",
+                .getBooleanProperty("swordv2-server.allow-community-deposit");
-                        "allow-community-deposit");
 
         // find out if we keep the package as a file in specified directory
-        this.entryFirst = ConfigurationManager
+        this.entryFirst = configurationService
-                .getBooleanProperty("swordv2-server", "multipart.entry-first",
+                .getBooleanProperty("swordv2-server.multipart.entry-first",
                         false);
 
     }
@@ -132,19 +132,11 @@ public class SwordConfigurationDSpace implements SwordConfiguration
     // Utilities
     ///////////////////////////////////////////////////////////////////////////////////
 
-    public String getStringProperty(String module, String propName,
+    public String getStringProperty(String propName,
             String defaultValue, String[] allowedValues)
     {
-        String cfg;
+        String cfg = configurationService.getProperty(propName);   
-        if (module == null)
+        if (StringUtils.isBlank(cfg))
-        {
-            cfg = ConfigurationManager.getProperty(propName);
-        }
-        else
-        {
-            cfg = ConfigurationManager.getProperty(module, propName);
-        }
-        if (cfg == null || "".equals(cfg))
         {
             return defaultValue;
         }
@@ -170,10 +162,10 @@ public class SwordConfigurationDSpace implements SwordConfiguration
         return defaultValue;
     }
 
-    public String getStringProperty(String module, String propName,
+    public String getStringProperty(String propName,
             String defaultValue)
     {
-        return this.getStringProperty(module, propName, defaultValue, null);
+        return this.getStringProperty(propName, defaultValue, null);
     }
 
     ///////////////////////////////////////////////////////////////////////////////////
@@ -187,8 +179,7 @@ public class SwordConfigurationDSpace implements SwordConfiguration
 
     public boolean returnStackTraceInError()
     {
-        return ConfigurationManager.getBooleanProperty("swordv2-server",
+        return configurationService.getBooleanProperty("swordv2-server.verbose-description.error.enable");
-                "verbose-description.error.enable");
     }
 
     public boolean returnErrorBody()
@@ -198,24 +189,24 @@ public class SwordConfigurationDSpace implements SwordConfiguration
 
     public String generator()
     {
-        return this.getStringProperty("swordv2-server", "generator.url",
+        return this.getStringProperty("swordv2-server.generator.url",
                 DSpaceUriRegistry.DSPACE_SWORD_NS);
     }
 
     public String generatorVersion()
     {
-        return this.getStringProperty("swordv2-server", "generator.version",
+        return this.getStringProperty("swordv2-server.generator.version",
                 "2.0");
     }
 
     public String administratorEmail()
     {
-        return this.getStringProperty(null, "mail.admin", null);
+        return this.getStringProperty("mail.admin", null);
     }
 
     public String getAuthType()
     {
-        return this.getStringProperty("swordv2-server", "auth-type", "Basic",
+        return this.getStringProperty("swordv2-server.auth-type", "Basic",
                 new String[] { "Basic", "None" });
     }
 
@@ -226,19 +217,19 @@ public class SwordConfigurationDSpace implements SwordConfiguration
 
     public String getTempDirectory()
     {
-        return this.getStringProperty("swordv2-server", "upload.tempdir", null);
+        return this.getStringProperty("swordv2-server.upload.tempdir", null);
     }
 
     public String getAlternateUrl()
     {
-        return ConfigurationManager
+        return configurationService
-                .getProperty("swordv2-server", "error.alternate.url");
+                .getProperty("swordv2-server.error.alternate.url");
     }
 
     public String getAlternateUrlContentType()
     {
-        return ConfigurationManager
+        return configurationService
-                .getProperty("swordv2-server", "error.alternate.content-type");
+                .getProperty("swordv2-server.error.alternate.content-type");
     }
 
     ///////////////////////////////////////////////////////////////////////////////////
@@ -255,25 +246,10 @@ public class SwordConfigurationDSpace implements SwordConfiguration
             throws DSpaceSwordException, SwordError
     {
         List<String> dps = new ArrayList<String>();
-        Properties props = ConfigurationManager.getProperties("swordv2-server");
+        List<String> packagingFormats = configurationService.getPropertyKeys("swordv2-server.disseminate-packaging");
-        Set keyset = props.keySet();
+        for (String key : packagingFormats)
-        for (Object keyObj : keyset)
         {
-            // start by getting anything that starts with sword.disseminate-packging.
+            String value = configurationService.getProperty(key);
-            String sw = "disseminate-packaging.";
-
-            if (!(keyObj instanceof String))
-            {
-                continue;
-            }
-            String key = (String) keyObj;
-
-            if (!key.startsWith(sw))
-            {
-                continue;
-            }
-
-            String value = props.getProperty((key));
 
             // now we want to ensure that the packaging format we offer has a disseminator
             // associated with it
@@ -517,28 +493,14 @@ public class SwordConfigurationDSpace implements SwordConfiguration
         List<String> aps = new ArrayList<String>();
 
         // build the holding maps of identifiers
-        Properties props = ConfigurationManager.getProperties("swordv2-server");
+        String acceptPackagingPrefix = "swordv2-server.accept-packaging.collection";
-        Set keyset = props.keySet();
+        List<String> acceptFormats = configurationService.getPropertyKeys(acceptPackagingPrefix);
-        for (Object keyObj : keyset)
+        for (String key : acceptFormats)
         {
-            // start by getting anything that starts with sword.accept-packaging.collection.
-            String sw = "accept-packaging.collection.";
-
-            if (!(keyObj instanceof String))
-            {
-                continue;
-            }
-            String key = (String) keyObj;
-
-            if (!key.startsWith(sw))
-            {
-                continue;
-            }
-
             // extract the configuration into the holding Maps
 
             // the suffix will be [typeid] or [handle].[typeid]
-            String suffix = key.substring(sw.length());
+            String suffix = key.substring(acceptPackagingPrefix.length()+1);
 
             // is there a handle which represents this collection?
             boolean withHandle = false;
@@ -557,7 +519,7 @@ public class SwordConfigurationDSpace implements SwordConfiguration
 
             if (withHandle || general)
             {
-                String value = props.getProperty((key));
+                String value = configurationService.getProperty(key);
                 aps.add(value);
             }
         }
@@ -570,26 +532,12 @@ public class SwordConfigurationDSpace implements SwordConfiguration
         List<String> aps = new ArrayList<String>();
 
         // build the holding maps of identifiers
-        Properties props = ConfigurationManager.getProperties("swordv2-server");
+        String acceptPackagingPrefix = "swordv2-server.accept-packaging.item";
-        Set keyset = props.keySet();
+        List<String> acceptFormats = configurationService.getPropertyKeys(acceptPackagingPrefix);
-        for (Object keyObj : keyset)
+        for (String key : acceptFormats)
         {
-            // start by getting anything that starts with sword.accept-packging.collection.
-            String sw = "accept-packaging.item.";
-
-            if (!(keyObj instanceof String))
-            {
-                continue;
-            }
-            String key = (String) keyObj;
-
-            if (!key.startsWith(sw))
-            {
-                continue;
-            }
-
             // extract the configuration into the holding Maps
-            String value = props.getProperty((key));
+            String value = configurationService.getProperty(key);
             aps.add(value);
         }
 
@@ -696,14 +644,14 @@ public class SwordConfigurationDSpace implements SwordConfiguration
 
     public String getStateUri(String state)
     {
-        return ConfigurationManager
+        return configurationService
-                .getProperty("swordv2-server", "state." + state + ".uri");
+                .getProperty("swordv2-server.state." + state + ".uri");
     }
 
     public String getStateDescription(String state)
     {
-        return ConfigurationManager.getProperty("swordv2-server",
+        return configurationService
-                "state." + state + ".description");
+                .getProperty("swordv2-server.state." + state + ".description");
     }
 
     public boolean allowUnauthenticatedMediaAccess()

dspace/config/modules/swordv2-server.cfg

@@ -135,7 +135,7 @@ swordv2-server.keep-original-package = true
 # See the SWORD specification for a detailed explanation of deposit
 # On-Behalf-Of another user
 #
-swordv2-server.on-behalf-of.enable = true
+swordv2-server.on-behalf-of.enable = false
 #
 # Which user accounts are allowed to do updates on items which already
 # exist in DSpace, on-behalf-of other users?
@@ -190,7 +190,8 @@ swordv2-server.error.alternate.content-type = text/html
 swordv2-server.generator.version = 2.0
 
 # The form of authentication to use
-# This is normally set to 'basic' for HTTP Basic
+# This is normally set to 'Basic' for HTTP Basic
+# Other valid values: 'None'
 swordv2-server.auth-type = Basic
 
 # The location where uploaded files and packages are