From 011eed40725cafbb38fa16f8285282d9f39b534d Mon Sep 17 00:00:00 2001 From: Mykhaylo Date: Mon, 3 May 2021 10:43:54 +0200 Subject: [PATCH 01/11] respond with 401 instead than 403 to invalid or expired JWT tokens --- .../exception/DSpaceApiExceptionControllerAdvice.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/exception/DSpaceApiExceptionControllerAdvice.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/exception/DSpaceApiExceptionControllerAdvice.java index 6e48ec5478..2df37e2206 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/exception/DSpaceApiExceptionControllerAdvice.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/exception/DSpaceApiExceptionControllerAdvice.java @@ -11,18 +11,17 @@ import static org.springframework.web.servlet.DispatcherServlet.EXCEPTION_ATTRIB import java.io.IOException; import java.sql.SQLException; +import java.util.Objects; import java.util.Set; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.dspace.app.rest.security.RestAuthenticationService; import org.dspace.app.rest.utils.ContextUtil; import org.dspace.authorize.AuthorizeException; import org.dspace.core.Context; import org.springframework.beans.TypeMismatchException; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.core.annotation.AnnotationUtils; import org.springframework.data.repository.support.QueryMethodParameterConversionException; import org.springframework.http.HttpHeaders; @@ -59,13 +58,11 @@ public class DSpaceApiExceptionControllerAdvice extends ResponseEntityExceptionH */ private static final Set LOG_AS_ERROR = Set.of(422); - @Autowired - private RestAuthenticationService restAuthenticationService; - @ExceptionHandler({AuthorizeException.class, RESTAuthorizationException.class, AccessDeniedException.class}) protected void handleAuthorizeException(HttpServletRequest request, HttpServletResponse response, Exception ex) throws IOException { - if (restAuthenticationService.hasAuthenticationData(request)) { + Context context = ContextUtil.obtainContext(request); + if (Objects.nonNull(context.getCurrentUser())) { sendErrorResponse(request, response, ex, "Access is denied", HttpServletResponse.SC_FORBIDDEN); } else { sendErrorResponse(request, response, ex, "Authentication is required", HttpServletResponse.SC_UNAUTHORIZED); From fc066d9c139ef4438da4e26718e01485ee2da314 Mon Sep 17 00:00:00 2001 From: Mykhaylo Date: Mon, 3 May 2021 11:56:15 +0200 Subject: [PATCH 02/11] fix tests --- .../dspace/app/rest/AuthenticationRestControllerIT.java | 4 ++-- .../org/dspace/app/rest/StatisticsRestRepositoryIT.java | 4 ++-- .../dspace/app/rest/WorkflowActionRestRepositoryIT.java | 4 ++-- .../app/rest/WorkflowDefinitionRestRepositoryIT.java | 8 ++++---- .../org/dspace/app/rest/WorkflowStepRestRepositoryIT.java | 2 +- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/AuthenticationRestControllerIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/AuthenticationRestControllerIT.java index 46041c22ab..27efd9ff46 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/AuthenticationRestControllerIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/AuthenticationRestControllerIT.java @@ -1193,7 +1193,7 @@ public class AuthenticationRestControllerIT extends AbstractControllerIntegratio String loginToken = getAuthToken(eperson.getEmail(), password); getClient().perform(get("/api/core/bitstreams/" + bitstream.getID() + "/content?authentication-token=" + loginToken)) - .andExpect(status().isForbidden()); + .andExpect(status().isUnauthorized()); } @Test @@ -1204,7 +1204,7 @@ public class AuthenticationRestControllerIT extends AbstractControllerIntegratio Thread.sleep(1); getClient().perform(get("/api/core/bitstreams/" + bitstream.getID() + "/content?authentication-token=" + shortLivedToken)) - .andExpect(status().isForbidden()); + .andExpect(status().isUnauthorized()); } @Test diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/StatisticsRestRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/StatisticsRestRepositoryIT.java index 08303e57f2..a721a53687 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/StatisticsRestRepositoryIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/StatisticsRestRepositoryIT.java @@ -179,7 +179,7 @@ public class StatisticsRestRepositoryIT extends AbstractControllerIntegrationTes getClient("unvalidToken").perform( get("/api/statistics/usagereports/" + itemNotVisitedWithBitstreams.getID() + "_" + TOTAL_VISITS_REPORT_ID)) // ** THEN ** - .andExpect(status().isForbidden()); + .andExpect(status().isUnauthorized()); } @Test @@ -829,7 +829,7 @@ public class StatisticsRestRepositoryIT extends AbstractControllerIntegrationTes .perform(get("/api/statistics/usagereports/search/object?uri=http://localhost:8080/server/api/core" + "/items/" + itemNotVisitedWithBitstreams.getID())) // ** THEN ** - .andExpect(status().isForbidden()); + .andExpect(status().isUnauthorized()); } @Test diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowActionRestRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowActionRestRepositoryIT.java index 884fc6cfa5..3fafa4dc8b 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowActionRestRepositoryIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowActionRestRepositoryIT.java @@ -50,7 +50,7 @@ public class WorkflowActionRestRepositoryIT extends AbstractControllerIntegratio //When we call this facets endpoint getClient(token).perform(get(WORKFLOW_ACTIONS_ENDPOINT)) //We expect a 403 Forbidden status - .andExpect(status().isForbidden()); + .andExpect(status().isUnauthorized()); } @Test @@ -113,7 +113,7 @@ public class WorkflowActionRestRepositoryIT extends AbstractControllerIntegratio //When we call this facets endpoint getClient(token).perform(get(WORKFLOW_ACTIONS_ENDPOINT + "/" + nameActionWithOptions)) //We expect a 403 Forbidden status - .andExpect(status().isForbidden()); + .andExpect(status().isUnauthorized()); } @Test diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowDefinitionRestRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowDefinitionRestRepositoryIT.java index 3f7ae74000..bdb2559805 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowDefinitionRestRepositoryIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowDefinitionRestRepositoryIT.java @@ -122,7 +122,7 @@ public class WorkflowDefinitionRestRepositoryIT extends AbstractControllerIntegr //When we call this facets endpoint getClient(token).perform(get(WORKFLOW_DEFINITIONS_ENDPOINT)) //We expect a 403 Forbidden status - .andExpect(status().isForbidden()); + .andExpect(status().isUnauthorized()); } @Test @@ -193,7 +193,7 @@ public class WorkflowDefinitionRestRepositoryIT extends AbstractControllerIntegr //When we call this facets endpoint getClient(token).perform(get(WORKFLOW_DEFINITIONS_ENDPOINT + "/" + workflowName)) //We expect a 403 Forbidden status - .andExpect(status().isForbidden()); + .andExpect(status().isUnauthorized()); } @Test @@ -403,7 +403,7 @@ public class WorkflowDefinitionRestRepositoryIT extends AbstractControllerIntegr getClient(token).perform(get(WORKFLOW_DEFINITIONS_ENDPOINT + "/" + defaultWorkflow.getID() + "/collections")) //We expect a 403 Forbidden status - .andExpect(status().isForbidden()); + .andExpect(status().isUnauthorized()); } @Test @@ -442,7 +442,7 @@ public class WorkflowDefinitionRestRepositoryIT extends AbstractControllerIntegr getClient(token).perform(get(WORKFLOW_DEFINITIONS_ENDPOINT + "/" + defaultWorkflow.getID() + "/steps")) //We expect a 403 Forbidden status - .andExpect(status().isForbidden()); + .andExpect(status().isUnauthorized()); } @Test diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowStepRestRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowStepRestRepositoryIT.java index a9a5b12d94..2de66317ee 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowStepRestRepositoryIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowStepRestRepositoryIT.java @@ -48,7 +48,7 @@ public class WorkflowStepRestRepositoryIT extends AbstractControllerIntegrationT //When we call this facets endpoint getClient(token).perform(get(WORKFLOW_ACTIONS_ENDPOINT)) //We expect a 403 Forbidden status - .andExpect(status().isForbidden()); + .andExpect(status().isUnauthorized()); } @Test From 1719024796c5af0bd3285c132f7e8631cde7de0c Mon Sep 17 00:00:00 2001 From: Antoine Snyers Date: Fri, 7 May 2021 10:57:25 +0200 Subject: [PATCH 03/11] Add test for Private/Withdrawn "equals" filters in Administrative Search Related to https://github.com/DSpace/dspace-angular/issues/1115 --- .../app/rest/DiscoveryRestControllerIT.java | 131 ++++++++++++++++++ 1 file changed, 131 insertions(+) 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 55b4f81e0b..5ef9f91d34 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 @@ -5238,6 +5238,137 @@ public class DiscoveryRestControllerIT extends AbstractControllerIntegrationTest .andExpect(jsonPath("$._links.self.href", containsString("/api/discover/search/objects"))); } + @Test + public void discoverSearchObjectsTestForAdministrativeViewWithFiltersEquals() throws Exception { + + context.turnOffAuthorisationSystem(); + + 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(); + + ItemBuilder.createItem(context, col1) + .withTitle("Public Test Item") + .withIssueDate("2010-10-17") + .withAuthor("Smith, Donald") + .withSubject("ExtraEntry") + .build(); + + ItemBuilder.createItem(context, col2) + .withTitle("Withdrawn Test Item") + .withIssueDate("1990-02-13") + .withAuthor("Smith, Maria") + .withAuthor("Doe, Jane") + .withSubject("ExtraEntry") + .withdrawn() + .build(); + + ItemBuilder.createItem(context, col2) + .withTitle("Private Test Item") + .withIssueDate("2010-02-13") + .withAuthor("Smith, Maria") + .withAuthor("Doe, Jane") + .withSubject("AnotherTest") + .withSubject("ExtraEntry") + .makeUnDiscoverable() + .build(); + + context.restoreAuthSystemState(); + + String adminToken = getAuthToken(admin.getEmail(), password); + + getClient(adminToken) + .perform(get("/api/discover/search/objects") + .param("configuration", "administrativeView") + .param("query", "Test") + .param("f.withdrawn", "true,equals") + ) + + .andExpect(status().isOk()) + .andExpect(jsonPath("$.type", is("discover"))) + .andExpect(jsonPath("$._embedded.searchResult.page", is( + PageMatcher.pageEntryWithTotalPagesAndElements(0, 20, 1, 1) + ))) + .andExpect(jsonPath("$._embedded.searchResult._embedded.objects", + Matchers.contains( + SearchResultMatcher.matchOnItemName("item", "items", "Withdrawn Test Item") + ) + )) + .andExpect(jsonPath("$._links.self.href", containsString("/api/discover/search/objects"))); + + getClient(adminToken) + .perform(get("/api/discover/search/objects") + .param("configuration", "administrativeView") + .param("query", "Test") + .param("f.withdrawn", "false,equals") + ) + + .andExpect(status().isOk()) + .andExpect(jsonPath("$.type", is("discover"))) + .andExpect(jsonPath("$._embedded.searchResult.page", is( + PageMatcher.pageEntryWithTotalPagesAndElements(0, 20, 1, 2) + ))) + .andExpect(jsonPath("$._embedded.searchResult._embedded.objects", + Matchers.containsInAnyOrder( + SearchResultMatcher.matchOnItemName("item", "items", "Public Test Item"), + SearchResultMatcher.matchOnItemName("item", "items", "Private Test Item") + ) + )) + .andExpect(jsonPath("$._links.self.href", containsString("/api/discover/search/objects"))); + + getClient(adminToken) + .perform(get("/api/discover/search/objects") + .param("configuration", "administrativeView") + .param("query", "Test") + .param("f.discoverable", "true,equals") + ) + + .andExpect(status().isOk()) + .andExpect(jsonPath("$.type", is("discover"))) + .andExpect(jsonPath("$._embedded.searchResult.page", is( + PageMatcher.pageEntryWithTotalPagesAndElements(0, 20, 1, 2) + ))) + .andExpect(jsonPath("$._embedded.searchResult._embedded.objects", + Matchers.containsInAnyOrder( + SearchResultMatcher.matchOnItemName("item", "items", "Public Test Item"), + SearchResultMatcher.matchOnItemName("item", "items", "Withdrawn Test Item") + ) + )) + .andExpect(jsonPath("$._links.self.href", containsString("/api/discover/search/objects"))); + + getClient(adminToken) + .perform(get("/api/discover/search/objects") + .param("configuration", "administrativeView") + .param("query", "Test") + .param("f.discoverable", "false,equals") + ) + + .andExpect(status().isOk()) + .andExpect(jsonPath("$.type", is("discover"))) + .andExpect(jsonPath("$._embedded.searchResult.page", is( + PageMatcher.pageEntryWithTotalPagesAndElements(0, 20, 1, 1) + ))) + .andExpect(jsonPath("$._embedded.searchResult._embedded.objects", + Matchers.contains( + SearchResultMatcher.matchOnItemName("item", "items", "Private Test Item") + ) + )) + .andExpect(jsonPath("$._links.self.href", containsString("/api/discover/search/objects"))); + } + @Test public void discoverSearchPoolTaskObjectsTest() throws Exception { context.turnOffAuthorisationSystem(); From edf4214e440e7fb1fa98699ab356e53a92dc978c Mon Sep 17 00:00:00 2001 From: Antoine Snyers Date: Fri, 7 May 2021 11:49:31 +0200 Subject: [PATCH 04/11] Filter on standard fields without appending _keyword Related to https://github.com/DSpace/dspace-angular/issues/1115 --- .../main/java/org/dspace/discovery/SearchService.java | 5 ++++- .../java/org/dspace/discovery/SolrServiceImpl.java | 10 +++++++--- .../rest/repository/MetadataFieldRestRepository.java | 8 ++++---- .../dspace/app/rest/utils/DiscoverQueryBuilder.java | 3 ++- .../app/rest/utils/DiscoverQueryBuilderTest.java | 4 ++-- 5 files changed, 19 insertions(+), 11 deletions(-) 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 c6ad547b69..cefa6c0f94 100644 --- a/dspace-api/src/main/java/org/dspace/discovery/SearchService.java +++ b/dspace-api/src/main/java/org/dspace/discovery/SearchService.java @@ -12,6 +12,7 @@ import java.util.List; import org.dspace.content.Item; import org.dspace.core.Context; +import org.dspace.discovery.configuration.DiscoveryConfiguration; import org.dspace.discovery.configuration.DiscoveryMoreLikeThisConfiguration; import org.dspace.discovery.configuration.DiscoverySearchFilterFacet; @@ -62,11 +63,13 @@ public interface SearchService { * @param field the field of the filter query * @param operator equals/notequals/notcontains/authority/notauthority * @param value the filter query value + * @param config the discovery configuration * @return a filter query * @throws SQLException if database error * An exception that provides information on a database access error or other errors. */ - DiscoverFilterQuery toFilterQuery(Context context, String field, String operator, String value) throws SQLException; + DiscoverFilterQuery toFilterQuery(Context context, String field, String operator, String value, + DiscoveryConfiguration config) throws SQLException; List getRelatedItems(Context context, Item item, DiscoveryMoreLikeThisConfiguration moreLikeThisConfiguration); 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 fc73009644..fdeb45b881 100644 --- a/dspace-api/src/main/java/org/dspace/discovery/SolrServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/discovery/SolrServiceImpl.java @@ -60,6 +60,7 @@ import org.dspace.core.Context; import org.dspace.core.Email; import org.dspace.core.I18nUtil; import org.dspace.core.LogManager; +import org.dspace.discovery.configuration.DiscoveryConfiguration; import org.dspace.discovery.configuration.DiscoveryConfigurationParameters; import org.dspace.discovery.configuration.DiscoveryMoreLikeThisConfiguration; import org.dspace.discovery.configuration.DiscoverySearchFilterFacet; @@ -1069,9 +1070,9 @@ public class SolrServiceImpl implements SearchService, IndexingService { return new ArrayList<>(0); } } - @Override - public DiscoverFilterQuery toFilterQuery(Context context, String field, String operator, String value) + public DiscoverFilterQuery toFilterQuery(Context context, String field, String operator, String value, + DiscoveryConfiguration config) throws SQLException { DiscoverFilterQuery result = new DiscoverFilterQuery(); @@ -1081,7 +1082,10 @@ public class SolrServiceImpl implements SearchService, IndexingService { if (operator.endsWith("equals")) { - filterQuery.append("_keyword"); + final DiscoverySearchFilterFacet facet = config.getSidebarFacet(field); + if (facet == null || !facet.getType().equals(DiscoveryConfigurationParameters.TYPE_STANDARD)) { + filterQuery.append("_keyword"); + } } else if (operator.endsWith("authority")) { filterQuery.append("_authority"); } diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/MetadataFieldRestRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/MetadataFieldRestRepository.java index 59e547d449..177d149fc7 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/MetadataFieldRestRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/MetadataFieldRestRepository.java @@ -190,22 +190,22 @@ public class MetadataFieldRestRepository extends DSpaceRestRepository new FacetYearRange((DiscoverySearchFilterFacet) invocation.getArguments()[2])); - when(searchService.toFilterQuery(any(Context.class), any(String.class), any(String.class), any(String.class))) + when(searchService.toFilterQuery(any(Context.class), any(String.class), any(String.class), any(String.class), any(DiscoveryConfiguration.class))) .then(invocation -> new DiscoverFilterQuery((String) invocation.getArguments()[1], invocation.getArguments()[1] + ":\"" + invocation.getArguments()[3] + "\"", (String) invocation.getArguments()[3])); @@ -291,7 +291,7 @@ public class DiscoverQueryBuilderTest { @Test(expected = DSpaceBadRequestException.class) public void testInvalidSearchFilter2() throws Exception { - when(searchService.toFilterQuery(any(Context.class), any(String.class), any(String.class), any(String.class))) + when(searchService.toFilterQuery(any(Context.class), any(String.class), any(String.class), any(String.class), any(DiscoveryConfiguration.class))) .thenThrow(SQLException.class); queryBuilder From f33bf28dd441d93d89e098f4392f71a294795cd6 Mon Sep 17 00:00:00 2001 From: Antoine Snyers Date: Fri, 7 May 2021 13:04:18 +0200 Subject: [PATCH 05/11] Handle null value for config --- .../main/java/org/dspace/discovery/SolrServiceImpl.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) 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 fdeb45b881..dd6dd0d755 100644 --- a/dspace-api/src/main/java/org/dspace/discovery/SolrServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/discovery/SolrServiceImpl.java @@ -1082,8 +1082,12 @@ public class SolrServiceImpl implements SearchService, IndexingService { if (operator.endsWith("equals")) { - final DiscoverySearchFilterFacet facet = config.getSidebarFacet(field); - if (facet == null || !facet.getType().equals(DiscoveryConfigurationParameters.TYPE_STANDARD)) { + final boolean isStandardField + = Optional.ofNullable(config) + .flatMap(c -> Optional.ofNullable(c.getSidebarFacet(field))) + .map(facet -> facet.getType().equals(DiscoveryConfigurationParameters.TYPE_STANDARD)) + .orElse(false); + if (!isStandardField) { filterQuery.append("_keyword"); } } else if (operator.endsWith("authority")) { From 3ad67676c50180764ed7acc3a15528b22332e4d5 Mon Sep 17 00:00:00 2001 From: Antoine Snyers Date: Fri, 7 May 2021 13:31:11 +0200 Subject: [PATCH 06/11] Fix LineLength errors --- .../org/dspace/app/rest/utils/DiscoverQueryBuilderTest.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/utils/DiscoverQueryBuilderTest.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/utils/DiscoverQueryBuilderTest.java index 8d5994f292..9a8f07e76a 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/utils/DiscoverQueryBuilderTest.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/utils/DiscoverQueryBuilderTest.java @@ -113,7 +113,8 @@ public class DiscoverQueryBuilderTest { any(), any(DiscoverQuery.class))) .then(invocation -> new FacetYearRange((DiscoverySearchFilterFacet) invocation.getArguments()[2])); - when(searchService.toFilterQuery(any(Context.class), any(String.class), any(String.class), any(String.class), any(DiscoveryConfiguration.class))) + when(searchService.toFilterQuery(any(Context.class), any(String.class), any(String.class), any(String.class), + any(DiscoveryConfiguration.class))) .then(invocation -> new DiscoverFilterQuery((String) invocation.getArguments()[1], invocation.getArguments()[1] + ":\"" + invocation.getArguments()[3] + "\"", (String) invocation.getArguments()[3])); @@ -291,7 +292,8 @@ public class DiscoverQueryBuilderTest { @Test(expected = DSpaceBadRequestException.class) public void testInvalidSearchFilter2() throws Exception { - when(searchService.toFilterQuery(any(Context.class), any(String.class), any(String.class), any(String.class), any(DiscoveryConfiguration.class))) + when(searchService.toFilterQuery(any(Context.class), any(String.class), any(String.class), any(String.class), + any(DiscoveryConfiguration.class))) .thenThrow(SQLException.class); queryBuilder From 9436f1c426104f85c954ffd1be2ecc4d347ff244 Mon Sep 17 00:00:00 2001 From: Marie Verdonck Date: Fri, 7 May 2021 16:02:03 +0200 Subject: [PATCH 07/11] javadoc for new param -> nullable --- .../src/main/java/org/dspace/discovery/SearchService.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 cefa6c0f94..9b6ac0109d 100644 --- a/dspace-api/src/main/java/org/dspace/discovery/SearchService.java +++ b/dspace-api/src/main/java/org/dspace/discovery/SearchService.java @@ -63,7 +63,8 @@ public interface SearchService { * @param field the field of the filter query * @param operator equals/notequals/notcontains/authority/notauthority * @param value the filter query value - * @param config the discovery configuration + * @param config (nullable) the discovery configuration (if not null, field's corresponding facet.type checked to + * be standard so suffix is not added for equals operator) * @return a filter query * @throws SQLException if database error * An exception that provides information on a database access error or other errors. From 3a3cdb0f612bd4346c89dc8afff89a6f3e9cd53b Mon Sep 17 00:00:00 2001 From: Tim Donohue Date: Mon, 10 May 2021 10:10:51 -0500 Subject: [PATCH 08/11] Ensure we check for CSRF changes after login failure. --- dspace-server-webapp/src/main/webapp/login.html | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dspace-server-webapp/src/main/webapp/login.html b/dspace-server-webapp/src/main/webapp/login.html index 5304d0053d..47752ded07 100644 --- a/dspace-server-webapp/src/main/webapp/login.html +++ b/dspace-server-webapp/src/main/webapp/login.html @@ -216,7 +216,9 @@ } }, success : successHandler, - error : function() { + error : function(xhr) { + // Check for an update to the CSRF Token & save to a MyHalBrowserCsrfToken cookie (if found) + checkForUpdatedCSRFTokenInResponse(xhr); toastr.error('The credentials you entered are invalid. Please try again.', 'Login Failed'); } }); From 9435f70a79860ec14470c0819fa542af84943195 Mon Sep 17 00:00:00 2001 From: Chris Wilper Date: Mon, 10 May 2021 19:53:44 -0400 Subject: [PATCH 09/11] #3269 Log detail when Service Manager can't find bean --- .../org/dspace/servicemanager/DSpaceServiceManager.java | 8 ++++---- dspace/config/log4j2.xml | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/dspace-services/src/main/java/org/dspace/servicemanager/DSpaceServiceManager.java b/dspace-services/src/main/java/org/dspace/servicemanager/DSpaceServiceManager.java index c17b31f68d..56cc1285c1 100644 --- a/dspace-services/src/main/java/org/dspace/servicemanager/DSpaceServiceManager.java +++ b/dspace-services/src/main/java/org/dspace/servicemanager/DSpaceServiceManager.java @@ -426,10 +426,10 @@ public final class DSpaceServiceManager implements ServiceManagerSystem { service = (T) applicationContext.getBean(name, type); } catch (BeansException e) { // no luck, try the fall back option - log.info( + log.warn( "Unable to locate bean by name or id={}." + " Will try to look up bean by type next." - + " BeansException: {}", name, e.getMessage()); + + " BeansException: {}", name, e); service = null; } } else { @@ -438,9 +438,9 @@ public final class DSpaceServiceManager implements ServiceManagerSystem { service = (T) applicationContext.getBean(type.getName(), type); } catch (BeansException e) { // no luck, try the fall back option - log.info("Unable to locate bean by name or id={}." + log.warn("Unable to locate bean by name or id={}." + " Will try to look up bean by type next." - + " BeansException: {}", type.getName(), e.getMessage()); + + " BeansException: {}", type.getName(), e); service = null; } } diff --git a/dspace/config/log4j2.xml b/dspace/config/log4j2.xml index 67c2d39e52..1ebdf5ab3e 100644 --- a/dspace/config/log4j2.xml +++ b/dspace/config/log4j2.xml @@ -82,7 +82,7 @@ + level='WARN'/> Date: Wed, 12 May 2021 13:03:47 -0400 Subject: [PATCH 10/11] #3269 Log exception details --- .../org/dspace/servicemanager/DSpaceServiceManager.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/dspace-services/src/main/java/org/dspace/servicemanager/DSpaceServiceManager.java b/dspace-services/src/main/java/org/dspace/servicemanager/DSpaceServiceManager.java index 56cc1285c1..afd1627f5e 100644 --- a/dspace-services/src/main/java/org/dspace/servicemanager/DSpaceServiceManager.java +++ b/dspace-services/src/main/java/org/dspace/servicemanager/DSpaceServiceManager.java @@ -428,8 +428,7 @@ public final class DSpaceServiceManager implements ServiceManagerSystem { // no luck, try the fall back option log.warn( "Unable to locate bean by name or id={}." - + " Will try to look up bean by type next." - + " BeansException: {}", name, e); + + " Will try to look up bean by type next.", name, e); service = null; } } else { @@ -439,8 +438,7 @@ public final class DSpaceServiceManager implements ServiceManagerSystem { } catch (BeansException e) { // no luck, try the fall back option log.warn("Unable to locate bean by name or id={}." - + " Will try to look up bean by type next." - + " BeansException: {}", type.getName(), e); + + " Will try to look up bean by type next.", type.getName(), e); service = null; } } From 46a5bc2cab13f5ca3f618faec5d22d50faaf87c6 Mon Sep 17 00:00:00 2001 From: Mykhaylo Date: Thu, 13 May 2021 12:50:22 +0200 Subject: [PATCH 11/11] fix comments --- .../dspace/app/rest/WorkflowActionRestRepositoryIT.java | 4 ++-- .../app/rest/WorkflowDefinitionRestRepositoryIT.java | 8 ++++---- .../org/dspace/app/rest/WorkflowStepRestRepositoryIT.java | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowActionRestRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowActionRestRepositoryIT.java index 3fafa4dc8b..de687ebd9d 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowActionRestRepositoryIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowActionRestRepositoryIT.java @@ -49,7 +49,7 @@ public class WorkflowActionRestRepositoryIT extends AbstractControllerIntegratio String token = "nonValidToken"; //When we call this facets endpoint getClient(token).perform(get(WORKFLOW_ACTIONS_ENDPOINT)) - //We expect a 403 Forbidden status + //We expect a 401 Unauthorized status .andExpect(status().isUnauthorized()); } @@ -112,7 +112,7 @@ public class WorkflowActionRestRepositoryIT extends AbstractControllerIntegratio WorkflowActionConfig existentWorkflow = xmlWorkflowFactory.getActionByName(nameActionWithOptions); //When we call this facets endpoint getClient(token).perform(get(WORKFLOW_ACTIONS_ENDPOINT + "/" + nameActionWithOptions)) - //We expect a 403 Forbidden status + //We expect a 401 Unauthorized status .andExpect(status().isUnauthorized()); } diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowDefinitionRestRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowDefinitionRestRepositoryIT.java index bdb2559805..445ce87abc 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowDefinitionRestRepositoryIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowDefinitionRestRepositoryIT.java @@ -121,7 +121,7 @@ public class WorkflowDefinitionRestRepositoryIT extends AbstractControllerIntegr String token = "NonValidToken"; //When we call this facets endpoint getClient(token).perform(get(WORKFLOW_DEFINITIONS_ENDPOINT)) - //We expect a 403 Forbidden status + //We expect a 401 Unauthorized status .andExpect(status().isUnauthorized()); } @@ -192,7 +192,7 @@ public class WorkflowDefinitionRestRepositoryIT extends AbstractControllerIntegr String workflowName = defaultWorkflow.getID(); //When we call this facets endpoint getClient(token).perform(get(WORKFLOW_DEFINITIONS_ENDPOINT + "/" + workflowName)) - //We expect a 403 Forbidden status + //We expect a 401 Unauthorized status .andExpect(status().isUnauthorized()); } @@ -402,7 +402,7 @@ public class WorkflowDefinitionRestRepositoryIT extends AbstractControllerIntegr //When we call this facets endpoint getClient(token).perform(get(WORKFLOW_DEFINITIONS_ENDPOINT + "/" + defaultWorkflow.getID() + "/collections")) - //We expect a 403 Forbidden status + //We expect a 401 Unauthorized status .andExpect(status().isUnauthorized()); } @@ -441,7 +441,7 @@ public class WorkflowDefinitionRestRepositoryIT extends AbstractControllerIntegr //When we call this facets endpoint getClient(token).perform(get(WORKFLOW_DEFINITIONS_ENDPOINT + "/" + defaultWorkflow.getID() + "/steps")) - //We expect a 403 Forbidden status + //We expect a 401 Unauthorized status .andExpect(status().isUnauthorized()); } diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowStepRestRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowStepRestRepositoryIT.java index 2de66317ee..e06ba08f69 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowStepRestRepositoryIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/WorkflowStepRestRepositoryIT.java @@ -47,7 +47,7 @@ public class WorkflowStepRestRepositoryIT extends AbstractControllerIntegrationT String token = "NonValidToken"; //When we call this facets endpoint getClient(token).perform(get(WORKFLOW_ACTIONS_ENDPOINT)) - //We expect a 403 Forbidden status + //We expect a 401 Unauthorized status .andExpect(status().isUnauthorized()); }