diff --git a/dspace-api/src/main/java/org/dspace/browse/ItemCountDAOSolr.java b/dspace-api/src/main/java/org/dspace/browse/ItemCountDAOSolr.java index 4b741847a8..57c3b83ee7 100644 --- a/dspace-api/src/main/java/org/dspace/browse/ItemCountDAOSolr.java +++ b/dspace-api/src/main/java/org/dspace/browse/ItemCountDAOSolr.java @@ -128,7 +128,7 @@ public class ItemCountDAOSolr implements ItemCountDAO { DiscoverResult sResponse = null; try { - sResponse = searcher.search(context, query, false); + sResponse = searcher.search(context, query); List commCount = sResponse.getFacetResult("location.comm"); List collCount = sResponse.getFacetResult("location.coll"); for (FacetResult c : commCount) { diff --git a/dspace-api/src/main/java/org/dspace/browse/SolrBrowseDAO.java b/dspace-api/src/main/java/org/dspace/browse/SolrBrowseDAO.java index ae491ad935..087ec42774 100644 --- a/dspace-api/src/main/java/org/dspace/browse/SolrBrowseDAO.java +++ b/dspace-api/src/main/java/org/dspace/browse/SolrBrowseDAO.java @@ -169,9 +169,6 @@ public class SolrBrowseDAO implements BrowseDAO { private DiscoverResult sResponse = null; - private boolean itemsWithdrawn = false; - private boolean itemsDiscoverable = true; - private boolean showFrequencies; private DiscoverResult getSolrResponse() throws BrowseException { @@ -217,8 +214,7 @@ public class SolrBrowseDAO implements BrowseDAO { } } try { - sResponse = searcher.search(context, query, itemsWithdrawn - || !itemsDiscoverable); + sResponse = searcher.search(context, query); } catch (SearchServiceException e) { throw new BrowseException(e); } @@ -227,21 +223,14 @@ public class SolrBrowseDAO implements BrowseDAO { } private void addStatusFilter(DiscoverQuery query) { - if (itemsWithdrawn) { - query.addFilterQueries("withdrawn:true"); - } else if (!itemsDiscoverable) { - query.addFilterQueries("discoverable:false"); - // TODO - - try { - if (!authorizeService.isAdmin(context) - && (authorizeService.isCommunityAdmin(context) - || authorizeService.isCollectionAdmin(context))) { - query.addFilterQueries(searcher.createLocationQueryForAdministrableItems(context)); - } - } catch (SQLException ex) { - log.error(ex); + try { + if (!authorizeService.isAdmin(context) + && (authorizeService.isCommunityAdmin(context) + || authorizeService.isCollectionAdmin(context))) { + query.addFilterQueries(searcher.createLocationQueryForAdministrableItems(context)); } + } catch (SQLException ex) { + log.error("Error looking up authorization rights of current user", ex); } } @@ -363,10 +352,9 @@ public class SolrBrowseDAO implements BrowseDAO { query.setQuery("bi_" + column + "_sort" + ": {\"" + value + "\" TO *]"); query.addFilterQueries("-(bi_" + column + "_sort" + ":" + value + "*)"); } - boolean includeUnDiscoverable = itemsWithdrawn || !itemsDiscoverable; DiscoverResult resp = null; try { - resp = searcher.search(context, query, includeUnDiscoverable); + resp = searcher.search(context, query); } catch (SearchServiceException e) { throw new BrowseException(e); } @@ -693,11 +681,6 @@ public class SolrBrowseDAO implements BrowseDAO { */ @Override public void setTable(String table) { - if (table.equals(BrowseIndex.getWithdrawnBrowseIndex().getTableName())) { - itemsWithdrawn = true; - } else if (table.equals(BrowseIndex.getPrivateBrowseIndex().getTableName())) { - itemsDiscoverable = false; - } facetField = table; } diff --git a/dspace-api/src/main/java/org/dspace/discovery/SearchService.java b/dspace-api/src/main/java/org/dspace/discovery/SearchService.java index 596e784bf7..31b114fb33 100644 --- a/dspace-api/src/main/java/org/dspace/discovery/SearchService.java +++ b/dspace-api/src/main/java/org/dspace/discovery/SearchService.java @@ -51,29 +51,6 @@ public interface SearchService { DiscoverResult search(Context context, IndexableObject dso, DiscoverQuery query) throws SearchServiceException; - /** - * @param context DSpace Context object. - * @param query the discovery query object. - * @param includeWithdrawn use true to include in the results also withdrawn - * items that match the query. - * @return discovery search result object - * @throws SearchServiceException if search error - */ - DiscoverResult search(Context context, DiscoverQuery query, - boolean includeWithdrawn) throws SearchServiceException; - - /** - * @param context DSpace Context object - * @param dso a DSpace Object to use as scope of the search (only results - * within this object) - * @param query the discovery query object - * @param includeWithdrawn use true to include in the results also withdrawn - * items that match the query - * @return discovery search result object - * @throws SearchServiceException if search error - */ - DiscoverResult search(Context context, IndexableObject dso, DiscoverQuery query, boolean includeWithdrawn) - throws SearchServiceException; List search(Context context, String query, String orderfield, boolean ascending, int offset, int max, String... filterquery); diff --git a/dspace-api/src/main/java/org/dspace/discovery/SolrServiceImpl.java b/dspace-api/src/main/java/org/dspace/discovery/SolrServiceImpl.java index 00865f919c..6af95070d6 100644 --- a/dspace-api/src/main/java/org/dspace/discovery/SolrServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/discovery/SolrServiceImpl.java @@ -1855,21 +1855,10 @@ public class SolrServiceImpl implements SearchService, IndexingService { } //========== SearchService implementation - @Override - public DiscoverResult search(Context context, DiscoverQuery query) throws SearchServiceException { - return search(context, query, false); - } @Override - public DiscoverResult search(Context context, IndexableObject dso, - DiscoverQuery query) + public DiscoverResult search(Context context, IndexableObject dso, DiscoverQuery discoveryQuery) throws SearchServiceException { - return search(context, dso, query, false); - } - - @Override - public DiscoverResult search(Context context, IndexableObject dso, DiscoverQuery discoveryQuery, - boolean includeUnDiscoverable) throws SearchServiceException { if (dso != null) { if (dso instanceof Community) { discoveryQuery.addFilterQueries("location:m" + dso.getID()); @@ -1879,19 +1868,19 @@ public class SolrServiceImpl implements SearchService, IndexingService { discoveryQuery.addFilterQueries(HANDLE_FIELD + ":" + ((Item) dso).getHandle()); } } - return search(context, discoveryQuery, includeUnDiscoverable); + return search(context, discoveryQuery); } @Override - public DiscoverResult search(Context context, DiscoverQuery discoveryQuery, boolean includeUnDiscoverable) + public DiscoverResult search(Context context, DiscoverQuery discoveryQuery ) throws SearchServiceException { try { if (getSolr() == null) { return new DiscoverResult(); } - SolrQuery solrQuery = resolveToSolrQuery(context, discoveryQuery, includeUnDiscoverable); + SolrQuery solrQuery = resolveToSolrQuery(context, discoveryQuery); QueryResponse queryResponse = getSolr().query(solrQuery, SolrRequest.METHOD.POST); @@ -1902,8 +1891,8 @@ public class SolrServiceImpl implements SearchService, IndexingService { } } - protected SolrQuery resolveToSolrQuery(Context context, DiscoverQuery discoveryQuery, - boolean includeUnDiscoverable) throws SearchServiceException { + protected SolrQuery resolveToSolrQuery(Context context, DiscoverQuery discoveryQuery) + throws SearchServiceException { SolrQuery solrQuery = new SolrQuery(); String query = "*:*"; @@ -1929,11 +1918,6 @@ public class SolrServiceImpl implements SearchService, IndexingService { solrQuery.setParam("spellcheck", Boolean.TRUE); } - if (!includeUnDiscoverable) { - solrQuery.addFilterQuery("NOT(withdrawn:true)"); - solrQuery.addFilterQuery("NOT(discoverable:false)"); - } - for (int i = 0; i < discoveryQuery.getFilterQueries().size(); i++) { String filterQuery = discoveryQuery.getFilterQueries().get(i); solrQuery.addFilterQuery(filterQuery); @@ -2401,8 +2385,8 @@ public class SolrServiceImpl implements SearchService, IndexingService { } catch (Exception e) { log.error( - LogManager.getHeader(context, "Error while retrieving related items", "Handle: " + item.getHandle()), - e); + LogManager.getHeader(context, "Error while retrieving related items", "Handle: " + + item.getHandle()), e); } return results; } diff --git a/dspace-api/src/main/java/org/dspace/discovery/SolrServicePrivateItemPlugin.java b/dspace-api/src/main/java/org/dspace/discovery/SolrServicePrivateItemPlugin.java new file mode 100644 index 0000000000..47dea08fc8 --- /dev/null +++ b/dspace-api/src/main/java/org/dspace/discovery/SolrServicePrivateItemPlugin.java @@ -0,0 +1,49 @@ +/** + * 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.discovery; + +import static org.apache.logging.log4j.LogManager.getLogger; + +import java.sql.SQLException; + +import org.apache.logging.log4j.Logger; +import org.apache.solr.client.solrj.SolrQuery; +import org.dspace.authorize.service.AuthorizeService; +import org.dspace.core.Context; +import org.dspace.core.LogManager; +import org.springframework.beans.factory.annotation.Autowired; + +/** + * This plugin prevents discovery of private items by non-administrators. + */ +public class SolrServicePrivateItemPlugin implements SolrServiceSearchPlugin { + + private static final Logger log = getLogger(SolrServicePrivateItemPlugin.class.getSimpleName()); + + @Autowired(required = true) + protected AuthorizeService authorizeService; + + + @Autowired(required = true) + protected SearchService searchService; + + @Override + public void additionalSearchParameters(Context context, DiscoverQuery discoveryQuery, SolrQuery solrQuery) { + try { + // Prevents access if user has no administrative rights on the community or collection. + // NOTE: the resource restriction plugin adds location filters for community and collection admins. + if ( !authorizeService.isAdmin(context) && !authorizeService.isCommunityAdmin(context) + && !authorizeService.isCollectionAdmin(context)) { + solrQuery.addFilterQuery("NOT(discoverable:false)"); + } + } catch (SQLException ex) { + log.error(LogManager.getHeader(context, "Error looking up authorization rights of current user", + ""), ex); + } + } +} \ No newline at end of file diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/DiscoveryRestControllerIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/DiscoveryRestControllerIT.java index cdee407282..1212704c92 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/DiscoveryRestControllerIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/DiscoveryRestControllerIT.java @@ -3582,4 +3582,139 @@ public class DiscoveryRestControllerIT extends AbstractControllerIntegrationTest } + @Test + public void discoverSearchObjectsTestForWithdrawnOrPrivateItemsNonAdmin() throws Exception { + //We turn off the authorization system in order to create the structure as defined below + context.turnOffAuthorisationSystem(); + + //** GIVEN ** + //1. A community-collection structure with one parent community with sub-community and two collections. + parentCommunity = CommunityBuilder.createCommunity(context) + .withName("Parent Community") + .build(); + Community child1 = CommunityBuilder.createSubCommunity(context, parentCommunity) + .withName("Sub Community") + .build(); + Collection col1 = CollectionBuilder.createCollection(context, child1).withName("Collection 1").build(); + Collection col2 = CollectionBuilder.createCollection(context, child1).withName("Collection 2").build(); + //2. One public item, one private, one withdrawn. + Item publicItem1 = ItemBuilder.createItem(context, col1) + .withTitle("Test") + .withIssueDate("2010-10-17") + .withAuthor("Smith, Donald") + .withSubject("ExtraEntry") + .build(); + + Item publicItem2 = ItemBuilder.createItem(context, col2) + .withTitle("WithdrawnTest 2") + .withIssueDate("1990-02-13") + .withAuthor("Smith, Maria").withAuthor("Doe, Jane") + .withSubject("ExtraEntry") + .withdrawn() + .build(); + + Item publicItem3 = ItemBuilder.createItem(context, col2) + .withTitle("Private Test item 2") + .withIssueDate("2010-02-13") + .withAuthor("Smith, Maria").withAuthor("Doe, Jane") + .withSubject("AnotherTest").withSubject("ExtraEntry") + .makeUnDiscoverable() + .build(); + + + String query = "Test"; + //** WHEN ** + //A non-admin user browses this endpoint to find the withdrawn or private objects in the system + //With a query stating 'Test' + getClient().perform(get("/api/discover/search/objects") + .param("configuration", "undiscoverable") + .param("query", query)) + //** THEN ** + //The status has to be 200 OK + .andExpect(status().isOk()) + //The type has to be 'discover' + .andExpect(jsonPath("$.type", is("discover"))) + //The page object needs to look like this + .andExpect(jsonPath("$._embedded.searchResult.page", is( + PageMatcher.pageEntry(0, 20) + ))) + //The search results should be an empty list. + .andExpect(jsonPath("$._embedded.searchResult._embedded.objects", Matchers.empty())) + //There always needs to be a self link available + .andExpect(jsonPath("$._links.self.href", containsString("/api/discover/search/objects"))) + + ; + + } + + @Test + public void discoverSearchObjectsTestForWithdrawnOrPrivateItemsByAdminUser() throws Exception { + //We turn off the authorization system in order to create the structure as defined below + context.turnOffAuthorisationSystem(); + + //** GIVEN ** + //1. A community-collection structure with one parent community with sub-community and two collections. + parentCommunity = CommunityBuilder.createCommunity(context) + .withName("Parent Community") + .build(); + Community child1 = CommunityBuilder.createSubCommunity(context, parentCommunity) + .withName("Sub Community") + .build(); + Collection col1 = CollectionBuilder.createCollection(context, child1).withName("Collection 1").build(); + Collection col2 = CollectionBuilder.createCollection(context, child1).withName("Collection 2").build(); + //2. One public item, one private, one withdrawn. + Item publicItem1 = ItemBuilder.createItem(context, col1) + .withTitle("Test") + .withIssueDate("2010-10-17") + .withAuthor("Smith, Donald") + .withSubject("ExtraEntry") + .build(); + + Item publicItem2 = ItemBuilder.createItem(context, col2) + .withTitle("Withdrawn Test 2") + .withIssueDate("1990-02-13") + .withAuthor("Smith, Maria").withAuthor("Doe, Jane") + .withSubject("ExtraEntry") + .withdrawn() + .build(); + + Item publicItem3 = ItemBuilder.createItem(context, col2) + .withTitle("Private Test item 2") + .withIssueDate("2010-02-13") + .withAuthor("Smith, Maria").withAuthor("Doe, Jane") + .withSubject("AnotherTest").withSubject("ExtraEntry") + .makeUnDiscoverable() + .build(); + context.restoreAuthSystemState(); + + String query = "Test"; + String adminToken = getAuthToken(admin.getEmail(), password); + //** WHEN ** + // A system admin user browses this endpoint to find the withdrawn or private objects in the system + // With a query stating 'Test' + getClient(adminToken).perform(get("/api/discover/search/objects") + .param("configuration", "undiscoverable") + .param("query", query)) + //** THEN ** + //The status has to be 200 OK + .andExpect(status().isOk()) + //The type has to be 'discover' + .andExpect(jsonPath("$.type", is("discover"))) + //The page object needs to look like this + .andExpect(jsonPath("$._embedded.searchResult.page", is( + PageMatcher.pageEntry(0, 20) + ))) + //The search results should be an empty list. + .andExpect(jsonPath("$._embedded.searchResult._embedded.objects", + Matchers.containsInAnyOrder( + SearchResultMatcher.matchOnItemName("item", "items", "Private Test item 2"), + SearchResultMatcher.matchOnItemName("item", "items", "Withdrawn Test 2") + ))) + //There always needs to be a self link available + .andExpect(jsonPath("$._links.self.href", containsString("/api/discover/search/objects"))) + + ; + + } + } \ No newline at end of file diff --git a/dspace/config/spring/api/discovery.xml b/dspace/config/spring/api/discovery.xml index 3da454ba09..be03cd70d0 100644 --- a/dspace/config/spring/api/discovery.xml +++ b/dspace/config/spring/api/discovery.xml @@ -27,6 +27,7 @@ + @@ -51,9 +52,9 @@ - + - + @@ -249,6 +250,147 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + search.resourcetype:2 + + withdrawn:true OR discoverable:false + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + dc.title + dc.contributor.author + dc.creator + dc.subject + + + + + + + + + + + + +