diff --git a/dspace-api/src/main/java/org/dspace/app/requestitem/RequestItemAuthorExtractor.java b/dspace-api/src/main/java/org/dspace/app/requestitem/RequestItemAuthorExtractor.java index 9b66030e90..e353637c79 100644 --- a/dspace-api/src/main/java/org/dspace/app/requestitem/RequestItemAuthorExtractor.java +++ b/dspace-api/src/main/java/org/dspace/app/requestitem/RequestItemAuthorExtractor.java @@ -14,14 +14,14 @@ import org.dspace.core.Context; /** * Interface to abstract the strategy for select the author to contact for - * request copy + * request copy. * * @author Andrea Bollini */ public interface RequestItemAuthorExtractor { /** - * Retrieve the auhtor to contact for a request copy of the give item. + * Retrieve the author to contact for requesting a copy of the given item. * * @param context DSpace context object * @param item item to request @@ -29,5 +29,15 @@ public interface RequestItemAuthorExtractor { * or null if no valid email address was found. * @throws SQLException if database error */ - public RequestItemAuthor getRequestItemAuthor(Context context, Item item) throws SQLException; + public RequestItemAuthor getRequestItemAuthor(Context context, Item item) + throws SQLException; + + /** + * + * @param context current DSpace session. + * @param item the requested Item. + * @return true if {@link eperson} is authorized to respond to requests for + * {@link item}. + */ + public boolean isAuthorized(Context context, Item item); } diff --git a/dspace-api/src/main/java/org/dspace/app/requestitem/RequestItemHelpdeskStrategy.java b/dspace-api/src/main/java/org/dspace/app/requestitem/RequestItemHelpdeskStrategy.java index 7b63d3ea8d..4a38dc6488 100644 --- a/dspace-api/src/main/java/org/dspace/app/requestitem/RequestItemHelpdeskStrategy.java +++ b/dspace-api/src/main/java/org/dspace/app/requestitem/RequestItemHelpdeskStrategy.java @@ -16,33 +16,40 @@ import org.dspace.core.I18nUtil; import org.dspace.eperson.EPerson; import org.dspace.eperson.service.EPersonService; import org.dspace.services.ConfigurationService; -import org.dspace.services.factory.DSpaceServicesFactory; import org.springframework.beans.factory.annotation.Autowired; /** - * RequestItem strategy to allow DSpace support team's helpdesk to receive requestItem request - * With this enabled, then the Item author/submitter doesn't receive the request, but the helpdesk instead does. + * RequestItem strategy to allow DSpace support team's help desk to receive + * requestItem requests. With this enabled, the Item author/submitter doesn't + * receive the request, but the help desk instead does. * - * Failover to the RequestItemSubmitterStrategy, which means the submitter would get the request if there is no - * specified helpdesk email. + *

Fails over to the {@link RequestItemSubmitterStrategy}, which means the + * submitter would get the request if there is no specified help desk email. * * @author Sam Ottenhoff * @author Peter Dietz */ -public class RequestItemHelpdeskStrategy extends RequestItemSubmitterStrategy { +public class RequestItemHelpdeskStrategy + extends RequestItemSubmitterStrategy { + static final String P_HELPDESK_OVERRIDE + = "request.item.helpdesk.override"; + static final String P_MAIL_HELPDESK = "mail.helpdesk"; + @Autowired(required = true) protected EPersonService ePersonService; + @Autowired(required = true) + protected ConfigurationService configurationService; + public RequestItemHelpdeskStrategy() { } @Override - public RequestItemAuthor getRequestItemAuthor(Context context, Item item) throws SQLException { - ConfigurationService configurationService - = DSpaceServicesFactory.getInstance().getConfigurationService(); + public RequestItemAuthor getRequestItemAuthor(Context context, Item item) + throws SQLException { boolean helpdeskOverridesSubmitter = configurationService - .getBooleanProperty("request.item.helpdesk.override", false); - String helpDeskEmail = configurationService.getProperty("mail.helpdesk"); + .getBooleanProperty(P_HELPDESK_OVERRIDE, false); + String helpDeskEmail = configurationService.getProperty(P_MAIL_HELPDESK); if (helpdeskOverridesSubmitter && StringUtils.isNotBlank(helpDeskEmail)) { return getHelpDeskPerson(context, helpDeskEmail); @@ -53,16 +60,18 @@ public class RequestItemHelpdeskStrategy extends RequestItemSubmitterStrategy { } /** - * Return a RequestItemAuthor object for the specified helpdesk email address. - * It makes an attempt to find if there is a matching eperson for the helpdesk address, to use the name, - * Otherwise it falls back to a helpdeskname key in the Messages.props. + * Return a RequestItemAuthor object for the specified help desk email address. + * It makes an attempt to find if there is a matching {@link EPerson} for + * the help desk address, to use its name. Otherwise it falls back to the + * {@code helpdeskname} key in {@code Messages.properties}. * * @param context context * @param helpDeskEmail email * @return RequestItemAuthor * @throws SQLException if database error */ - public RequestItemAuthor getHelpDeskPerson(Context context, String helpDeskEmail) throws SQLException { + public RequestItemAuthor getHelpDeskPerson(Context context, String helpDeskEmail) + throws SQLException { context.turnOffAuthorisationSystem(); EPerson helpdeskEPerson = ePersonService.findByEmail(context, helpDeskEmail); context.restoreAuthSystemState(); @@ -76,4 +85,9 @@ public class RequestItemHelpdeskStrategy extends RequestItemSubmitterStrategy { return new RequestItemAuthor(helpdeskName, helpDeskEmail); } } + + @Override + public boolean isAuthorized(Context context, Item item) { + return true; + } } diff --git a/dspace-api/src/main/java/org/dspace/app/requestitem/RequestItemSubmitterStrategy.java b/dspace-api/src/main/java/org/dspace/app/requestitem/RequestItemSubmitterStrategy.java index 2708c24ba9..a08d2f0723 100644 --- a/dspace-api/src/main/java/org/dspace/app/requestitem/RequestItemSubmitterStrategy.java +++ b/dspace-api/src/main/java/org/dspace/app/requestitem/RequestItemSubmitterStrategy.java @@ -9,6 +9,8 @@ package org.dspace.app.requestitem; import java.sql.SQLException; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.dspace.content.Item; import org.dspace.core.Context; import org.dspace.eperson.EPerson; @@ -19,6 +21,7 @@ import org.dspace.eperson.EPerson; * @author Andrea Bollini */ public class RequestItemSubmitterStrategy implements RequestItemAuthorExtractor { + private static final Logger LOG = LogManager.getLogger(); public RequestItemSubmitterStrategy() { } @@ -32,7 +35,7 @@ public class RequestItemSubmitterStrategy implements RequestItemAuthorExtractor */ @Override public RequestItemAuthor getRequestItemAuthor(Context context, Item item) - throws SQLException { + throws SQLException { EPerson submitter = item.getSubmitter(); RequestItemAuthor author = null; if (null != submitter) { @@ -41,4 +44,20 @@ public class RequestItemSubmitterStrategy implements RequestItemAuthorExtractor } return author; } + + @Override + public boolean isAuthorized(Context context, Item item) { + RequestItemAuthor authorizer; + try { + authorizer = getRequestItemAuthor(context, item); + } catch (SQLException ex) { + LOG.warn("Failed to find an authorizer: {}", ex::getMessage); + return false; + } + EPerson user = context.getCurrentUser(); + if (null == user) { + return false; + } + return authorizer.getEmail().equals(user.getEmail()); + } } diff --git a/dspace-api/src/test/java/org/dspace/app/requestitem/RequestItemHelpdeskStrategyTest.java b/dspace-api/src/test/java/org/dspace/app/requestitem/RequestItemHelpdeskStrategyTest.java new file mode 100644 index 0000000000..eaafcb58c2 --- /dev/null +++ b/dspace-api/src/test/java/org/dspace/app/requestitem/RequestItemHelpdeskStrategyTest.java @@ -0,0 +1,128 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ +package org.dspace.app.requestitem; + +import java.sql.SQLException; +import static org.junit.Assert.assertTrue; + +import org.dspace.AbstractUnitTest; +import org.dspace.builder.AbstractBuilder; +import org.dspace.builder.CollectionBuilder; +import org.dspace.builder.CommunityBuilder; +import org.dspace.builder.EPersonBuilder; +import org.dspace.builder.ItemBuilder; +import org.dspace.content.Collection; +import org.dspace.content.Community; +import org.dspace.content.Item; +import org.dspace.core.Context; +import org.dspace.eperson.EPerson; +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.junit.AfterClass; +import static org.junit.Assert.assertEquals; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Ignore; +import org.junit.Test; + +/** + * + * @author mwood + */ +public class RequestItemHelpdeskStrategyTest + extends AbstractUnitTest { + private static final String HELPDESK_ADDRESS = "helpdesk@example.com"; + private static final String AUTHOR_ADDRESS = "john.doe@example.com"; + + private static ConfigurationService configurationService; + private static EPersonService epersonService; + private static EPerson johnDoe; + + private Item item; + + @BeforeClass + public static void setUpClass() + throws SQLException { + AbstractBuilder.init(); // AbstractUnitTest doesn't do this for us. + + configurationService = DSpaceServicesFactory.getInstance().getConfigurationService(); + epersonService = EPersonServiceFactory.getInstance().getEPersonService(); + + Context ctx = new Context(); + ctx.turnOffAuthorisationSystem(); + johnDoe = EPersonBuilder.createEPerson(ctx) + .withEmail(AUTHOR_ADDRESS) + .withNameInMetadata("John", "Doe") + .build(); + ctx.restoreAuthSystemState(); + ctx.complete(); + } + + @AfterClass + public static void tearDownClass() { + AbstractBuilder.destroy(); // AbstractUnitTest doesn't do this for us. + } + + @Before + public void setUp() { + context = new Context(); + context.setCurrentUser(johnDoe); + context.turnOffAuthorisationSystem(); + Community community = CommunityBuilder.createCommunity(context).build(); + Collection collection = CollectionBuilder.createCollection(context, community).build(); + item = ItemBuilder.createItem(context, collection) + .build(); + context.restoreAuthSystemState(); + context.setCurrentUser(null); + } + + /** + * Test of getRequestItemAuthor method, of class RequestItemHelpdeskStrategy. + * @throws java.lang.Exception passed through. + */ + @Test + public void testGetRequestItemAuthor() + throws Exception { + RequestItemHelpdeskStrategy instance = new RequestItemHelpdeskStrategy(); + instance.configurationService = configurationService; + instance.ePersonService = epersonService; + + // Check with help desk enabled. + configurationService.setProperty(RequestItemHelpdeskStrategy.P_HELPDESK_OVERRIDE, "true"); + configurationService.setProperty(RequestItemHelpdeskStrategy.P_MAIL_HELPDESK, HELPDESK_ADDRESS); + RequestItemAuthor author = instance.getRequestItemAuthor(context, item); + assertEquals("Wrong author address", HELPDESK_ADDRESS, author.getEmail()); + + // Check with help desk disabled. + configurationService.setProperty(RequestItemHelpdeskStrategy.P_HELPDESK_OVERRIDE, "false"); + author = instance.getRequestItemAuthor(context, item); + assertEquals("Wrong author address", AUTHOR_ADDRESS, author.getEmail()); + } + + /** + * Test of getHelpDeskPerson method, of class RequestItemHelpdeskStrategy. + * @throws java.lang.Exception passed through. + */ + @Ignore + @Test + public void testGetHelpDeskPerson() throws Exception { + } + + /** + * Silly test of isAuthorized method. + */ + @Test + public void testIsAuthorized() { + RequestItemHelpdeskStrategy instance = new RequestItemHelpdeskStrategy(); + instance.configurationService = configurationService; + instance.ePersonService = epersonService; + assertTrue(instance.isAuthorized(context, item)); + } +} diff --git a/dspace-api/src/test/java/org/dspace/app/requestitem/RequestItemSubmitterStrategyTest.java b/dspace-api/src/test/java/org/dspace/app/requestitem/RequestItemSubmitterStrategyTest.java new file mode 100644 index 0000000000..1b1f91ac94 --- /dev/null +++ b/dspace-api/src/test/java/org/dspace/app/requestitem/RequestItemSubmitterStrategyTest.java @@ -0,0 +1,115 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ +package org.dspace.app.requestitem; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.sql.SQLException; + +import org.dspace.AbstractUnitTest; +import org.dspace.builder.AbstractBuilder; +import org.dspace.builder.CollectionBuilder; +import org.dspace.builder.CommunityBuilder; +import org.dspace.builder.EPersonBuilder; +import org.dspace.builder.ItemBuilder; +import org.dspace.content.Collection; +import org.dspace.content.Community; +import org.dspace.content.Item; +import org.dspace.core.Context; +import org.dspace.eperson.EPerson; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.Test; + +/** + * + * @author mwood + */ +public class RequestItemSubmitterStrategyTest + extends AbstractUnitTest { + private static final String AUTHOR_ADDRESS = "john.doe@example.com"; + private static final String NON_AUTHOR_ADDRESS = "richard.roe@example.com"; + + private static EPerson johnDoe; + private static EPerson richardRoe; + + private Item item; + + @BeforeClass + public static void setUpClass() + throws SQLException { + AbstractBuilder.init(); // AbstractUnitTest doesn't do this for us. + + Context ctx = new Context(); + ctx.turnOffAuthorisationSystem(); + johnDoe = EPersonBuilder.createEPerson(ctx) + .withEmail(AUTHOR_ADDRESS) + .withNameInMetadata("John", "Doe") + .build(); + richardRoe = EPersonBuilder.createEPerson(ctx) + .withEmail(NON_AUTHOR_ADDRESS) + .withNameInMetadata("Richard", "Roe") + .build(); + ctx.restoreAuthSystemState(); + ctx.complete(); + } + + @AfterClass + public static void tearDownClass() { + AbstractBuilder.destroy(); // AbstractUnitTest doesn't do this for us. + } + + @Before + public void setUp() { + context = new Context(); + context = new Context(); + context.setCurrentUser(johnDoe); + context.turnOffAuthorisationSystem(); + Community community = CommunityBuilder.createCommunity(context).build(); + Collection collection = CollectionBuilder.createCollection(context, community).build(); + item = ItemBuilder.createItem(context, collection) + .build(); + context.restoreAuthSystemState(); + context.setCurrentUser(null); + } + + /** + * Test of getRequestItemAuthor method, of class RequestItemSubmitterStrategy. + * @throws java.lang.Exception passed through. + */ + @Test + public void testGetRequestItemAuthor() + throws Exception { + RequestItemSubmitterStrategy instance = new RequestItemSubmitterStrategy(); + RequestItemAuthor author = instance.getRequestItemAuthor(context, item); + assertEquals("Wrong author address", AUTHOR_ADDRESS, author.getEmail()); + } + + /** + * Test of isAuthorized method, of class RequestItemSubmitterStrategy. + */ + @Test + public void testIsAuthorized() { + RequestItemSubmitterStrategy instance = new RequestItemSubmitterStrategy(); + + // Test with correct logged-in user. + context.setCurrentUser(johnDoe); + assertTrue("Should be authorized", instance.isAuthorized(context, item)); + + // Test with incorrect logged-in user. + context.setCurrentUser(richardRoe); + assertFalse("Should not be authorized", instance.isAuthorized(context, item)); + + // Test when not logged in. + context.setCurrentUser(null); + assertFalse("Should not be authorized", instance.isAuthorized(context, item)); + } +} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/RequestItemRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/RequestItemRepository.java index d013566a2c..33faca1a30 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/RequestItemRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/RequestItemRepository.java @@ -27,7 +27,6 @@ import org.apache.http.client.utils.URIBuilder; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.dspace.app.requestitem.RequestItem; -import org.dspace.app.requestitem.RequestItemAuthor; import org.dspace.app.requestitem.RequestItemAuthorExtractor; import org.dspace.app.requestitem.RequestItemEmailNotifier; import org.dspace.app.requestitem.service.RequestItemService; @@ -197,7 +196,7 @@ public class RequestItemRepository responseLink = getLinkTokenEmail(ri.getToken()); } catch (URISyntaxException | MalformedURLException e) { LOG.warn("Impossible URL error while composing email: {}", - e.getMessage()); + e::getMessage); throw new RuntimeException("Request not sent: " + e.getMessage()); } @@ -229,14 +228,7 @@ public class RequestItemRepository } // Check for authorized user - RequestItemAuthor authorizer; - try { - authorizer = requestItemAuthorExtractor.getRequestItemAuthor(context, ri.getItem()); - } catch (SQLException ex) { - LOG.warn("Failed to find an authorizer: {}", ex.getMessage()); - authorizer = new RequestItemAuthor("", ""); - } - if (!authorizer.getEmail().equals(context.getCurrentUser().getEmail())) { + if (!requestItemAuthorExtractor.isAuthorized(context, ri.getItem())) { throw new AuthorizeException("Not authorized to approve this request"); } @@ -267,7 +259,7 @@ public class RequestItemRepository try { RequestItemEmailNotifier.sendResponse(context, ri, subject, message); } catch (IOException ex) { - LOG.warn("Response not sent: {}", ex.getMessage()); + LOG.warn("Response not sent: {}", ex::getMessage); throw new RuntimeException("Response not sent", ex); } @@ -276,7 +268,7 @@ public class RequestItemRepository try { RequestItemEmailNotifier.requestOpenAccess(context, ri); } catch (IOException ex) { - LOG.warn("Open access request not sent: {}", ex.getMessage()); + LOG.warn("Open access request not sent: {}", ex::getMessage); throw new RuntimeException("Open access request not sent", ex); } }