From 1a99488733feb36b2751e6b3ed9b89c0cc26ca1f Mon Sep 17 00:00:00 2001 From: Terry Brady Date: Wed, 3 Feb 2016 10:03:45 -0800 Subject: [PATCH 1/5] remove semicolon from config prop --- dspace/config/modules/authentication-shibboleth.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dspace/config/modules/authentication-shibboleth.cfg b/dspace/config/modules/authentication-shibboleth.cfg index a42c4d0b74..979716a114 100644 --- a/dspace/config/modules/authentication-shibboleth.cfg +++ b/dspace/config/modules/authentication-shibboleth.cfg @@ -136,7 +136,7 @@ authentication-shibboleth.lastname-header = SHIB-SURNAME # SHIB-cn => cn # If the eperson metadata field is not found, should it be automatically created? -authentication-shibboleth.eperson.metadata.autocreate = true; +authentication-shibboleth.eperson.metadata.autocreate = true # Shibboleth attributes are by default UTF-8 encoded. Some servlet container # automatically converts the attributes from ISO-8859-1 (latin-1) to UTF-8. From 492d474fb7fe9c722a14286fd9f1f5d33bababcc Mon Sep 17 00:00:00 2001 From: Terry Brady Date: Wed, 3 Feb 2016 14:06:13 -0800 Subject: [PATCH 2/5] auth service allocation --- .../java/org/dspace/authenticate/ShibAuthentication.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/authenticate/ShibAuthentication.java b/dspace-api/src/main/java/org/dspace/authenticate/ShibAuthentication.java index 4e43fd1eb2..4ccf367464 100644 --- a/dspace-api/src/main/java/org/dspace/authenticate/ShibAuthentication.java +++ b/dspace-api/src/main/java/org/dspace/authenticate/ShibAuthentication.java @@ -77,7 +77,6 @@ public class ShibAuthentication implements AuthenticationMethod /** Maximum length for eperson additional metadata fields **/ protected final int METADATA_MAX_SIZE = 1024; - protected AuthenticationService authenticationService = AuthenticateServiceFactory.getInstance().getAuthenticationService(); protected EPersonService ePersonService = EPersonServiceFactory.getInstance().getEPersonService(); protected GroupService groupService = EPersonServiceFactory.getInstance().getGroupService(); protected MetadataFieldService metadataFieldService = ContentServiceFactory.getInstance().getMetadataFieldService(); @@ -226,7 +225,7 @@ public class ShibAuthentication implements AuthenticationMethod // Step 4: Log the user in. context.setCurrentUser(eperson); request.getSession().setAttribute("shib.authenticated", true); - authenticationService.initEPerson(context, request, eperson); + AuthenticateServiceFactory.getInstance().getAuthenticationService().initEPerson(context, request, eperson); log.info(eperson.getEmail()+" has been authenticated via shibboleth."); return AuthenticationMethod.SUCCESS; @@ -730,7 +729,7 @@ public class ShibAuthentication implements AuthenticationMethod eperson.setCanLogIn(true); // Commit the new eperson - authenticationService.initEPerson(context, request, eperson); + AuthenticateServiceFactory.getInstance().getAuthenticationService().initEPerson(context, request, eperson); ePersonService.update(context, eperson); context.dispatchEvents(); @@ -880,7 +879,7 @@ public class ShibAuthentication implements AuthenticationMethod else if (ePersonService.checkPassword(context, eperson, password)) { // Password matched - authenticationService.initEPerson(context, request, eperson); + AuthenticateServiceFactory.getInstance().getAuthenticationService().initEPerson(context, request, eperson); context.setCurrentUser(eperson); log.info(eperson.getEmail()+" has been authenticated via shibboleth using password-based sword compatibility mode."); return SUCCESS; From 7f5e44404788413e9ebc387285029402edf2bb50 Mon Sep 17 00:00:00 2001 From: Terry Brady Date: Wed, 3 Feb 2016 14:29:36 -0800 Subject: [PATCH 3/5] use config service for props --- .../authenticate/ShibAuthentication.java | 74 +++++++++---------- 1 file changed, 36 insertions(+), 38 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/authenticate/ShibAuthentication.java b/dspace-api/src/main/java/org/dspace/authenticate/ShibAuthentication.java index 4ccf367464..41d6bb40f1 100644 --- a/dspace-api/src/main/java/org/dspace/authenticate/ShibAuthentication.java +++ b/dspace-api/src/main/java/org/dspace/authenticate/ShibAuthentication.java @@ -21,7 +21,6 @@ import org.apache.commons.collections.ListUtils; import org.apache.commons.lang.StringUtils; import org.apache.log4j.Logger; import org.dspace.authenticate.factory.AuthenticateServiceFactory; -import org.dspace.authenticate.service.AuthenticationService; import org.dspace.authorize.AuthorizeException; import org.dspace.content.MetadataField; @@ -31,12 +30,13 @@ import org.dspace.content.factory.ContentServiceFactory; import org.dspace.content.service.MetadataFieldService; import org.dspace.content.service.MetadataSchemaService; import org.dspace.core.Context; -import org.dspace.core.ConfigurationManager; import org.dspace.eperson.EPerson; import org.dspace.eperson.Group; import org.dspace.eperson.factory.EPersonServiceFactory; import org.dspace.eperson.service.EPersonService; import org.dspace.eperson.service.GroupService; +import org.dspace.services.ConfigurationService; +import org.dspace.services.factory.DSpaceServicesFactory; /** * Shibboleth authentication for DSpace @@ -81,6 +81,7 @@ public class ShibAuthentication implements AuthenticationMethod protected GroupService groupService = EPersonServiceFactory.getInstance().getGroupService(); protected MetadataFieldService metadataFieldService = ContentServiceFactory.getInstance().getMetadataFieldService(); protected MetadataSchemaService metadataSchemaService = ContentServiceFactory.getInstance().getMetadataSchemaService(); + protected ConfigurationService configurationService = DSpaceServicesFactory.getInstance().getConfigurationService(); /** @@ -169,7 +170,7 @@ public class ShibAuthentication implements AuthenticationMethod // sword. This allows this compatibility without installing the password-based // authentication method which has side effects such as allowing users to login // with a username and password from the webui. - boolean swordCompatibility = ConfigurationManager.getBooleanProperty("authentication-shibboleth","sword.compatibility", true); + boolean swordCompatibility = configurationService.getBooleanProperty("authentication-shibboleth.sword.compatibility", true); if ( swordCompatibility && username != null && username.length() > 0 && password != null && password.length() > 0 ) { @@ -205,7 +206,7 @@ public class ShibAuthentication implements AuthenticationMethod } // Should we auto register new users. - boolean autoRegister = ConfigurationManager.getBooleanProperty("authentication-shibboleth","autoregister", true); + boolean autoRegister = configurationService.getBooleanProperty("authentication-shibboleth.autoregister", true); // Four steps to authenticate a user try { @@ -302,10 +303,10 @@ public class ShibAuthentication implements AuthenticationMethod } log.debug("Starting to determine special groups"); - String defaultRoles = ConfigurationManager.getProperty("authentication-shibboleth","default-roles"); - String roleHeader = ConfigurationManager.getProperty("authentication-shibboleth","role-header"); - boolean ignoreScope = ConfigurationManager.getBooleanProperty("authentication-shibboleth","role-header.ignore-scope", true); - boolean ignoreValue = ConfigurationManager.getBooleanProperty("authentication-shibboleth","role-header.ignore-value", false); + String defaultRoles = configurationService.getProperty("authentication-shibboleth.default-roles"); + String roleHeader = configurationService.getProperty("authentication-shibboleth.role-header"); + boolean ignoreScope = configurationService.getBooleanProperty("authentication-shibboleth.role-header.ignore-scope", true); + boolean ignoreValue = configurationService.getBooleanProperty("authentication-shibboleth.role-header.ignore-value", false); if (ignoreScope && ignoreValue) { throw new IllegalStateException("Both config parameters for ignoring an roll attributes scope and value are turned on, this is not a permissable configuration. (Note: ignore-scope defaults to true) The configuration parameters are: 'authentication.shib.role-header.ignore-scope' and 'authentication.shib.role-header.ignore-value'"); @@ -341,9 +342,9 @@ public class ShibAuthentication implements AuthenticationMethod } // Get the group names - String groupNames = ConfigurationManager.getProperty("authentication-shibboleth","role." + affiliation); - if (groupNames == null || groupNames.trim().length() == 0) { - groupNames = ConfigurationManager.getProperty("authentication-shibboleth","role." + affiliation.toLowerCase()); + String[] groupNames = configurationService.getArrayProperty("authentication-shibboleth.role." + affiliation); + if (groupNames == null || groupNames.length == 0) { + groupNames = configurationService.getArrayProperty("authentication-shibboleth.role." + affiliation.toLowerCase()); } if (groupNames == null) { @@ -354,16 +355,15 @@ public class ShibAuthentication implements AuthenticationMethod } // Add each group to the list. - String[] names = groupNames.split(","); - for (int i = 0; i < names.length; i++) { + for (int i = 0; i < groupNames.length; i++) { try { - Group group = groupService.findByName(context,names[i].trim()); + Group group = groupService.findByName(context,groupNames[i].trim()); if (group != null) groups.add(group); else - log.debug("Unable to find group: '"+names[i].trim()+"'"); + log.debug("Unable to find group: '"+groupNames[i].trim()+"'"); } catch (SQLException sqle) { - log.error("Exception thrown while trying to lookup affiliation role for group name: '"+names[i].trim()+"'",sqle); + log.error("Exception thrown while trying to lookup affiliation role for group name: '"+groupNames[i].trim()+"'",sqle); } } // for each groupNames } // foreach affiliations @@ -488,11 +488,11 @@ public class ShibAuthentication implements AuthenticationMethod // If this server is configured for lazy sessions then use this to // login, otherwise default to the protected shibboleth url. - boolean lazySession = ConfigurationManager.getBooleanProperty("authentication-shibboleth","lazysession", false); + boolean lazySession = configurationService.getBooleanProperty("authentication-shibboleth.lazysession", false); if ( lazySession ) { - String shibURL = ConfigurationManager.getProperty("authentication-shibboleth","lazysession.loginurl"); - boolean forceHTTPS = ConfigurationManager.getBooleanProperty("authentication-shibboleth","lazysession.secure",true); + String shibURL = configurationService.getProperty("authentication-shibboleth.lazysession.loginurl"); + boolean forceHTTPS = configurationService.getBooleanProperty("authentication-shibboleth.lazysession.secure",true); // Shibboleth authentication initiator if (shibURL == null || shibURL.length() == 0) @@ -581,9 +581,9 @@ public class ShibAuthentication implements AuthenticationMethod */ protected EPerson findEPerson(Context context, HttpServletRequest request) throws SQLException, AuthorizeException { - boolean isUsingTomcatUser = ConfigurationManager.getBooleanProperty("authentication-shibboleth","email-use-tomcat-remote-user"); - String netidHeader = ConfigurationManager.getProperty("authentication-shibboleth","netid-header"); - String emailHeader = ConfigurationManager.getProperty("authentication-shibboleth","email-header"); + boolean isUsingTomcatUser = configurationService.getBooleanProperty("authentication-shibboleth.email-use-tomcat-remote-user"); + String netidHeader = configurationService.getProperty("authentication-shibboleth.netid-header"); + String emailHeader = configurationService.getProperty("authentication-shibboleth.email-header"); EPerson eperson = null; boolean foundNetID = false; @@ -680,10 +680,10 @@ public class ShibAuthentication implements AuthenticationMethod protected EPerson registerNewEPerson(Context context, HttpServletRequest request) throws SQLException, AuthorizeException { // Header names - String netidHeader = ConfigurationManager.getProperty("authentication-shibboleth","netid-header"); - String emailHeader = ConfigurationManager.getProperty("authentication-shibboleth","email-header"); - String fnameHeader = ConfigurationManager.getProperty("authentication-shibboleth","firstname-header"); - String lnameHeader = ConfigurationManager.getProperty("authentication-shibboleth","lastname-header"); + String netidHeader = configurationService.getProperty("authentication-shibboleth.netid-header"); + String emailHeader = configurationService.getProperty("authentication-shibboleth.email-header"); + String fnameHeader = configurationService.getProperty("authentication-shibboleth.firstname-header"); + String lnameHeader = configurationService.getProperty("authentication-shibboleth.lastname-header"); // Header values String netid = findSingleAttribute(request,netidHeader); @@ -769,10 +769,10 @@ public class ShibAuthentication implements AuthenticationMethod protected void updateEPerson(Context context, HttpServletRequest request, EPerson eperson) throws SQLException, AuthorizeException { // Header names & values - String netidHeader = ConfigurationManager.getProperty("authentication-shibboleth","netid-header"); - String emailHeader = ConfigurationManager.getProperty("authentication-shibboleth","email-header"); - String fnameHeader = ConfigurationManager.getProperty("authentication-shibboleth","firstname-header"); - String lnameHeader = ConfigurationManager.getProperty("authentication-shibboleth","lastname-header"); + String netidHeader = configurationService.getProperty("authentication-shibboleth.netid-header"); + String emailHeader = configurationService.getProperty("authentication-shibboleth.email-header"); + String fnameHeader = configurationService.getProperty("authentication-shibboleth.firstname-header"); + String lnameHeader = configurationService.getProperty("authentication-shibboleth.lastname-header"); String netid = findSingleAttribute(request,netidHeader); String email = findSingleAttribute(request,emailHeader); @@ -912,11 +912,11 @@ public class ShibAuthentication implements AuthenticationMethod HashMap map = new HashMap(); - String mappingString = ConfigurationManager.getProperty("authentication-shibboleth","eperson.metadata"); - boolean autoCreate = ConfigurationManager.getBooleanProperty("authentication-shibboleth","eperson.metadata.autocreate", true); + String[] mappingString = configurationService.getArrayProperty("authentication-shibboleth.eperson.metadata"); + boolean autoCreate = configurationService.getBooleanProperty("authentication-shibboleth.eperson.metadata.autocreate", true); // Bail out if not set, returning an empty map. - if (mappingString == null || mappingString.trim().length() == 0) { + if (mappingString == null || mappingString.length == 0) { log.debug("No additional eperson metadata mapping found: authentication.shib.eperson.metadata"); metadataHeaderMap = map; @@ -926,8 +926,7 @@ public class ShibAuthentication implements AuthenticationMethod log.debug("Loading additional eperson metadata from: 'authentication.shib.eperson.metadata' = '"+mappingString+"'"); - String[] metadataStringList = mappingString.split(","); - for (String metadataString : metadataStringList) { + for (String metadataString : mappingString) { metadataString = metadataString.trim(); String[] metadataParts = metadataString.split("=>"); @@ -1052,9 +1051,8 @@ public class ShibAuthentication implements AuthenticationMethod value = request.getHeader(name.toUpperCase()); boolean reconvertAttributes = - ConfigurationManager.getBooleanProperty( - "authentication-shibboleth", - "reconvert.attributes", + configurationService.getBooleanProperty( + "authentication-shibboleth.reconvert.attributes", false); if (!StringUtils.isEmpty(value) && reconvertAttributes) From bcfe75dbefb66c3501081cf5aaeee5a89893f2fc Mon Sep 17 00:00:00 2001 From: Terry Brady Date: Wed, 3 Feb 2016 15:07:00 -0800 Subject: [PATCH 4/5] handle another array prop --- .../org/dspace/authenticate/ShibAuthentication.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/authenticate/ShibAuthentication.java b/dspace-api/src/main/java/org/dspace/authenticate/ShibAuthentication.java index 41d6bb40f1..6706d868b2 100644 --- a/dspace-api/src/main/java/org/dspace/authenticate/ShibAuthentication.java +++ b/dspace-api/src/main/java/org/dspace/authenticate/ShibAuthentication.java @@ -303,7 +303,7 @@ public class ShibAuthentication implements AuthenticationMethod } log.debug("Starting to determine special groups"); - String defaultRoles = configurationService.getProperty("authentication-shibboleth.default-roles"); + String[] defaultRoles = configurationService.getArrayProperty("authentication-shibboleth.default-roles"); String roleHeader = configurationService.getProperty("authentication-shibboleth.role-header"); boolean ignoreScope = configurationService.getBooleanProperty("authentication-shibboleth.role-header.ignore-scope", true); boolean ignoreValue = configurationService.getBooleanProperty("authentication-shibboleth.role-header.ignore-value", false); @@ -316,8 +316,8 @@ public class ShibAuthentication implements AuthenticationMethod List affiliations = findMultipleAttributes(request, roleHeader); if (affiliations == null) { if (defaultRoles != null) - affiliations = Arrays.asList(defaultRoles.split(",")); - log.debug("Failed to find Shibboleth role header, '"+roleHeader+"', falling back to the default roles: '"+defaultRoles+"'"); + affiliations = Arrays.asList(defaultRoles); + log.debug("Failed to find Shibboleth role header, '"+roleHeader+"', falling back to the default roles: '"+String.join(",", defaultRoles) +"'"); } else { log.debug("Found Shibboleth role header: '"+roleHeader+"' = '"+affiliations+"'"); } @@ -351,7 +351,7 @@ public class ShibAuthentication implements AuthenticationMethod log.debug("Unable to find role mapping for the value, '"+affiliation+"', there should be a mapping in config/modules/authentication-shibboleth.cfg: role."+affiliation+" = "); continue; } else { - log.debug("Mapping role affiliation to DSpace group: '"+groupNames+"'"); + log.debug("Mapping role affiliation to DSpace group: '"+String.join(",",groupNames)+"'"); } // Add each group to the list. @@ -923,7 +923,7 @@ public class ShibAuthentication implements AuthenticationMethod return; } - log.debug("Loading additional eperson metadata from: 'authentication.shib.eperson.metadata' = '"+mappingString+"'"); + log.debug("Loading additional eperson metadata from: 'authentication.shib.eperson.metadata' = '"+String.join(",", mappingString) +"'"); for (String metadataString : mappingString) { From 2ae5b47f2aa379dd40b2c369b0a3b96b9efc933f Mon Sep 17 00:00:00 2001 From: Terry Brady Date: Wed, 3 Feb 2016 15:15:26 -0800 Subject: [PATCH 5/5] StringUtils.join for log --- .../java/org/dspace/authenticate/ShibAuthentication.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/authenticate/ShibAuthentication.java b/dspace-api/src/main/java/org/dspace/authenticate/ShibAuthentication.java index 6706d868b2..31e5793e39 100644 --- a/dspace-api/src/main/java/org/dspace/authenticate/ShibAuthentication.java +++ b/dspace-api/src/main/java/org/dspace/authenticate/ShibAuthentication.java @@ -317,7 +317,7 @@ public class ShibAuthentication implements AuthenticationMethod if (affiliations == null) { if (defaultRoles != null) affiliations = Arrays.asList(defaultRoles); - log.debug("Failed to find Shibboleth role header, '"+roleHeader+"', falling back to the default roles: '"+String.join(",", defaultRoles) +"'"); + log.debug("Failed to find Shibboleth role header, '"+roleHeader+"', falling back to the default roles: '"+ StringUtils.join(defaultRoles, ",") + "'"); } else { log.debug("Found Shibboleth role header: '"+roleHeader+"' = '"+affiliations+"'"); } @@ -351,7 +351,7 @@ public class ShibAuthentication implements AuthenticationMethod log.debug("Unable to find role mapping for the value, '"+affiliation+"', there should be a mapping in config/modules/authentication-shibboleth.cfg: role."+affiliation+" = "); continue; } else { - log.debug("Mapping role affiliation to DSpace group: '"+String.join(",",groupNames)+"'"); + log.debug("Mapping role affiliation to DSpace group: '"+ StringUtils.join(groupNames, ",") +"'"); } // Add each group to the list. @@ -923,7 +923,7 @@ public class ShibAuthentication implements AuthenticationMethod return; } - log.debug("Loading additional eperson metadata from: 'authentication.shib.eperson.metadata' = '"+String.join(",", mappingString) +"'"); + log.debug("Loading additional eperson metadata from: 'authentication.shib.eperson.metadata' = '"+ StringUtils.join(mappingString, ",") +"'"); for (String metadataString : mappingString) {