From dc628c96de2c01703a47edeece1ca002bbac0a71 Mon Sep 17 00:00:00 2001 From: milanmajchrak <90026355+milanmajchrak@users.noreply.github.com> Date: Wed, 15 Nov 2023 12:29:15 +0100 Subject: [PATCH 1/2] ufal/downloading-restricted-bitstreams-not-working-properly (#457) * The admin could download restricted bitstreams. * The user cannot download bitstream if he/she is not signed in. * Cannot obtain context with user. * The user is already fetched from the context. * Added docs. * Do not use endpoint for downloading the single file because it already exists - remove it. * Fixed AuthorizationRestControllerIT tests. --- .../AuthorizationBitstreamUtils.java | 24 ++- .../app/rest/AuthorizationRestController.java | 15 +- .../app/rest/MetadataBitstreamController.java | 146 +++--------------- .../rest/AuthorizationRestControllerIT.java | 53 +++++-- .../rest/MetadataBitstreamControllerIT.java | 36 +---- 5 files changed, 85 insertions(+), 189 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/authorize/AuthorizationBitstreamUtils.java b/dspace-api/src/main/java/org/dspace/authorize/AuthorizationBitstreamUtils.java index adada5ad7c28..1b98556ecad0 100644 --- a/dspace-api/src/main/java/org/dspace/authorize/AuthorizationBitstreamUtils.java +++ b/dspace-api/src/main/java/org/dspace/authorize/AuthorizationBitstreamUtils.java @@ -18,7 +18,6 @@ import org.dspace.content.Bitstream; import org.dspace.content.Item; import org.dspace.content.clarin.ClarinLicense; -import org.dspace.content.clarin.ClarinLicenseLabel; import org.dspace.content.clarin.ClarinLicenseResourceMapping; import org.dspace.content.service.BitstreamService; import org.dspace.content.service.clarin.ClarinLicenseResourceMappingService; @@ -93,15 +92,15 @@ public boolean authorizeBitstream(Context context, Bitstream bitstream) throws S } /** - * If the bitstream has RES or ACA license and the user is Anonymous do not authorize that user. - * The user will be redirected to the login. + * Do not allow download for anonymous users. Allow it only if the bitstream has Clarin License and the license has + * confirmation = 3 (allow anonymous). + * * @param context DSpace context object * @param bitstreamID downloading Bitstream UUID * @return if the current user is authorized */ public boolean authorizeLicenseWithUser(Context context, UUID bitstreamID) throws SQLException { - // If the current user is null that means that the user is not signed in and cannot download the bitstream - // with RES or ACA license + // If the current user is null that means that the user is not signed if (Objects.nonNull(context.getCurrentUser())) { // User is signed return true; @@ -118,16 +117,13 @@ public boolean authorizeLicenseWithUser(Context context, UUID bitstreamID) throw // Bitstream should have only one type of the Clarin license, so we could get first record ClarinLicense clarinLicense = Objects.requireNonNull(clarinLicenseResourceMappings.get(0)).getLicense(); - // Get License Labels from clarin license and check if one of them is ACA or RES - List clarinLicenseLabels = clarinLicense.getLicenseLabels(); - for (ClarinLicenseLabel clarinLicenseLabel : clarinLicenseLabels) { - if (StringUtils.equals(clarinLicenseLabel.getLabel(), "RES") || - StringUtils.equals(clarinLicenseLabel.getLabel(), "ACA")) { - return false; - } + // 3 - Allow download for anonymous users, but with license confirmation + // 0 - License confirmation is not required + if (Objects.equals(clarinLicense.getConfirmation(), 3) || + Objects.equals(clarinLicense.getConfirmation(), 0)) { + return true; } - - return true; + return false; } private boolean userIsSubmitter(Context context, Bitstream bitstream, EPerson currentUser, UUID userID) { diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/AuthorizationRestController.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/AuthorizationRestController.java index e91242bae75c..5b23d5cb893a 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/AuthorizationRestController.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/AuthorizationRestController.java @@ -27,6 +27,7 @@ import org.dspace.content.Bitstream; import org.dspace.content.service.BitstreamService; import org.dspace.content.service.clarin.ClarinLicenseResourceMappingService; +import org.dspace.core.Constants; import org.dspace.core.Context; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -84,28 +85,30 @@ public ResponseEntity authrn(@PathVariable String id, HttpServletResponse respon return null; } - // If the bitstream has RES or ACA license and the user is Anonymous return NotAuthorized exception + // Do not allow download for anonymous users. Allow it only if the bitstream has Clarin License and the license + // has confirmation = 3 (allow anonymous). if (!authorizationBitstreamUtils.authorizeLicenseWithUser(context, bitstream.getID())) { response.sendError(HttpStatus.UNAUTHORIZED.value(), - "Anonymous user cannot download bitstream with REC or ACA license"); + "Anonymous user cannot download this bitstream"); return null; } // Wrap exceptions to the AuthrnRest object. - String errorMessage = "User is not authorized to download the bitstream."; - boolean isAuthorized = false; + String errorMessage = ""; try { - isAuthorized = authorizationBitstreamUtils.authorizeBitstream(context, bitstream); + authorizeService.authorizeAction(context, bitstream, Constants.READ); } catch (AuthorizeException e) { if (e instanceof MissingLicenseAgreementException) { errorMessage = MissingLicenseAgreementException.NAME; } else if (e instanceof DownloadTokenExpiredException) { errorMessage = DownloadTokenExpiredException.NAME; + } else { + errorMessage = e.getMessage(); } } - if (!isAuthorized) { + if (StringUtils.isNotBlank(errorMessage)) { // If the user is not authorized return response with the error message response.sendError(HttpStatus.UNAUTHORIZED.value(), errorMessage); return null; diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/MetadataBitstreamController.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/MetadataBitstreamController.java index ac26a2c69551..5e6700306959 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/MetadataBitstreamController.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/MetadataBitstreamController.java @@ -7,16 +7,18 @@ */ package org.dspace.app.rest; +import static org.dspace.app.rest.utils.RegexUtils.REGEX_REQUESTMAPPING_IDENTIFIER_AS_UUID; + import java.io.IOException; import java.io.InputStream; import java.sql.SQLException; import java.util.List; import java.util.Objects; +import java.util.UUID; import java.util.zip.Deflater; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import org.apache.commons.collections4.CollectionUtils; import org.apache.commons.compress.archivers.zip.ZipArchiveEntry; import org.apache.commons.compress.archivers.zip.ZipArchiveOutputStream; import org.apache.commons.compress.utils.IOUtils; @@ -24,38 +26,34 @@ import org.apache.logging.log4j.Logger; import org.dspace.app.rest.exception.DSpaceBadRequestException; import org.dspace.app.rest.exception.UnprocessableEntityException; -import org.dspace.app.rest.model.BitstreamRest; +import org.dspace.app.rest.model.ItemRest; import org.dspace.app.rest.utils.ContextUtil; +import org.dspace.authorize.AuthorizationBitstreamUtils; import org.dspace.authorize.AuthorizeException; -import org.dspace.authorize.MissingLicenseAgreementException; import org.dspace.authorize.service.AuthorizeService; import org.dspace.content.Bitstream; -import org.dspace.content.BitstreamFormat; import org.dspace.content.Bundle; import org.dspace.content.DSpaceObject; import org.dspace.content.Item; import org.dspace.content.service.BitstreamService; -import org.dspace.core.Constants; import org.dspace.core.Context; import org.dspace.handle.service.HandleService; import org.dspace.services.ConfigurationService; +import org.dspace.services.RequestService; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.core.io.InputStreamResource; -import org.springframework.core.io.Resource; import org.springframework.http.HttpHeaders; -import org.springframework.http.MediaType; -import org.springframework.http.ResponseEntity; -import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RequestMethod; import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.RestController; -/** + /** * This CLARIN Controller download a single file or a ZIP file from the Item's bitstream. */ @RestController -@RequestMapping("/api/" + BitstreamRest.CATEGORY + "/" + BitstreamRest.PLURAL_NAME) +@RequestMapping("/api/" + ItemRest.CATEGORY + "/" + ItemRest.PLURAL_NAME + REGEX_REQUESTMAPPING_IDENTIFIER_AS_UUID) public class MetadataBitstreamController { private static Logger log = org.apache.logging.log4j.LogManager.getLogger(MetadataBitstreamController.class); @@ -69,90 +67,17 @@ public class MetadataBitstreamController { private AuthorizeService authorizeService; @Autowired private ConfigurationService configurationService; - - - @GetMapping("/handle/{id}/{subId}/{fileName}") - public ResponseEntity downloadSingleFile(@PathVariable("id") String id, - @PathVariable("subId") String subId, - @PathVariable("fileName") String fileName, - HttpServletRequest request, HttpServletResponse response) - throws IOException { - String handleID = id + "/" + subId; - if (StringUtils.isBlank(id) || StringUtils.isBlank(subId)) { - log.error("Handle cannot be null! PathVariable `id` or `subId` is null."); - throw new DSpaceBadRequestException("Handle cannot be null!"); - } - - Context context = ContextUtil.obtainContext(request); - if (Objects.isNull(context)) { - log.error("Cannot obtain the context from the request."); - throw new RuntimeException("Cannot obtain the context from the request."); - } - - DSpaceObject dso = null; - try { - dso = handleService.resolveToObject(context, handleID); - } catch (Exception e) { - log.error("Cannot resolve handle: " + handleID); - throw new RuntimeException("Cannot resolve handle: " + handleID); - } - - - if (Objects.isNull(dso)) { - log.error("DSO is null"); - return null; - } - - if (!(dso instanceof Item)) { - log.error("DSO is not instance of Item"); - return null; - } - - Item item = (Item) dso; - List bundles = item.getBundles(); - // Find bitstream and start downloading. - for (Bundle bundle: bundles) { - for (Bitstream bitstream: bundle.getBitstreams()) { - // Authorize the action - it will send response redirect if something gets wrong. - authorizeBitstreamAction(context, bitstream, response); - - String btName = bitstream.getName(); - if (!(btName.equalsIgnoreCase(fileName))) { - continue; - } - try { - BitstreamFormat bitstreamFormat = bitstream.getFormat(context); - // Check if the bitstream has some extensions e.g., `.txt, .jpg,..` - checkBitstreamExtensions(bitstreamFormat); - - // Get content of the bitstream - InputStream inputStream = bitstreamService.retrieve(context, bitstream); - InputStreamResource resource = new InputStreamResource(inputStream); - HttpHeaders header = new HttpHeaders(); - header.add(HttpHeaders.CONTENT_DISPOSITION, - "attachment; filename=" + fileName); - header.add("Cache-Control", "no-cache, no-store, must-revalidate"); - header.add("Pragma", "no-cache"); - header.add("Expires", "0"); - return ResponseEntity.ok() - .headers(header) - .contentLength(inputStream.available()) - .contentType(MediaType.APPLICATION_OCTET_STREAM) - .body(resource); - } catch (Exception e) { - throw new RuntimeException(e); - } - } - } - - return null; - } + @Autowired + AuthorizationBitstreamUtils authorizationBitstreamUtils; + @Autowired + private RequestService requestService; /** * Download all Item's bitstreams as single ZIP file. */ - @GetMapping("/allzip") - public void downloadFileZip(@RequestParam("handleId") String handleId, + @PreAuthorize("hasPermission(#uuid, 'ITEM', 'READ')") + @RequestMapping( method = {RequestMethod.GET, RequestMethod.HEAD}, value = "allzip") + public void downloadFileZip(@PathVariable UUID uuid, @RequestParam("handleId") String handleId, HttpServletResponse response, HttpServletRequest request) throws IOException, SQLException, AuthorizeException { if (StringUtils.isBlank(handleId)) { @@ -195,11 +120,11 @@ public void downloadFileZip(@RequestParam("handleId") String handleId, for (Bundle original : bundles) { List bss = original.getBitstreams(); for (Bitstream bitstream : bss) { - authorizeBitstreamAction(context, bitstream, response); - String filename = bitstream.getName(); ZipArchiveEntry ze = new ZipArchiveEntry(filename); zip.putArchiveEntry(ze); + // Get content of the bitstream + // Retrieve method authorize bitstream download action. InputStream is = bitstreamService.retrieve(context, bitstream); IOUtils.copy(is, zip); zip.closeArchiveEntry(); @@ -209,37 +134,4 @@ public void downloadFileZip(@RequestParam("handleId") String handleId, zip.close(); response.getOutputStream().flush(); } - - /** - * Could the user download that bitstream? - * @param context DSpace context object - * @param bitstream Bitstream to download - * @param response for possibility to redirect - */ - private void authorizeBitstreamAction(Context context, Bitstream bitstream, HttpServletResponse response) - throws IOException { - - String uiURL = configurationService.getProperty("dspace.ui.url"); - if (StringUtils.isBlank(uiURL)) { - log.error("Configuration property `dspace.ui.url` cannot be empty or null!"); - throw new RuntimeException("Configuration property `dspace.ui.url` cannot be empty or null!"); - } - try { - authorizeService.authorizeAction(context, bitstream, Constants.READ); - } catch (MissingLicenseAgreementException e) { - response.sendRedirect(uiURL + "/bitstream/" + bitstream.getID() + "/download"); - } catch (AuthorizeException | SQLException e) { - response.sendRedirect(uiURL + "/login"); - } - } - - /** - * Check if the bitstream has file extension. - */ - private void checkBitstreamExtensions(BitstreamFormat bitstreamFormat) { - if ( Objects.isNull(bitstreamFormat) || CollectionUtils.isEmpty(bitstreamFormat.getExtensions())) { - log.error("Bitstream Extensions cannot be empty for downloading/previewing bitstreams."); - throw new RuntimeException("Bitstream Extensions cannot be empty for downloading/previewing bitstreams."); - } - } } diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/AuthorizationRestControllerIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/AuthorizationRestControllerIT.java index 03a5ae2ecc11..82546b0dacff 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/AuthorizationRestControllerIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/AuthorizationRestControllerIT.java @@ -39,6 +39,8 @@ import org.dspace.builder.ClarinUserRegistrationBuilder; import org.dspace.builder.CollectionBuilder; import org.dspace.builder.CommunityBuilder; +import org.dspace.builder.EPersonBuilder; +import org.dspace.builder.ResourcePolicyBuilder; import org.dspace.builder.WorkspaceItemBuilder; import org.dspace.content.Bitstream; import org.dspace.content.Collection; @@ -52,6 +54,11 @@ import org.dspace.content.service.clarin.ClarinLicenseLabelService; import org.dspace.content.service.clarin.ClarinLicenseResourceMappingService; import org.dspace.content.service.clarin.ClarinLicenseService; +import org.dspace.core.Constants; +import org.dspace.eperson.EPerson; +import org.dspace.eperson.Group; +import org.dspace.eperson.factory.EPersonServiceFactory; +import org.dspace.eperson.service.GroupService; import org.hamcrest.Matchers; import org.junit.Before; import org.junit.Test; @@ -59,8 +66,8 @@ public class AuthorizationRestControllerIT extends AbstractControllerIntegrationTest { - private static final String CLARIN_LICENSE_NAME = "Test Clarin License"; - private static final String TEXT_PLAIN_UTF_8 = "text/plain;charset=UTF-8"; + public static final String CLARIN_LICENSE_NAME = "Test Clarin License"; + public static final String TEXT_PLAIN_UTF_8 = "text/plain;charset=UTF-8"; @Autowired ClarinLicenseService clarinLicenseService; @@ -72,6 +79,8 @@ public class AuthorizationRestControllerIT extends AbstractControllerIntegration Item item; WorkspaceItem witem; ClarinLicense clarinLicense; + EPerson ePerson2; + String eperson2Password = "secret"; @Before public void setup() throws Exception { @@ -89,6 +98,19 @@ public void setup() throws Exception { context.setCurrentUser(eperson); InputStream pdf = getClass().getResourceAsStream("simple-article.pdf"); + // Create another person to try downloading the bitstream as anonymous user. + ePerson2 = EPersonBuilder.createEPerson(context) + .withCanLogin(true) + .withEmail("test@dtq.com") + .withPassword("secret") + .withNameInMetadata("Test", "User") + .build(); + + // The new user is not added into anonymous group by default + GroupService groupService = EPersonServiceFactory.getInstance().getGroupService(); + Group anonymousGroup = groupService.findByName(context, Group.ANONYMOUS); + groupService.addMember(context, anonymousGroup, ePerson2); + witem = WorkspaceItemBuilder.createWorkspaceItem(context, col1) .withTitle("Test WorkspaceItem") .withIssueDate("2017-10-17") @@ -96,9 +118,16 @@ public void setup() throws Exception { .build(); item = witem.getItem(); + Bitstream bitstream = item.getBundles().get(0).getBitstreams().get(0); + + // Create resource policy to allow anonymous user download the bitstream + ResourcePolicyBuilder.createResourcePolicy(context).withUser(ePerson2) + .withAction(Constants.READ) + .withGroup(anonymousGroup) + .withDspaceObject(bitstream).build(); // Create clarin license with clarin license label - clarinLicense = createClarinLicense(CLARIN_LICENSE_NAME, "Test Def", "Test R Info", 1); + clarinLicense = createClarinLicense(CLARIN_LICENSE_NAME, "Test Def", "Test R Info", 3); context.restoreAuthSystemState(); } @@ -118,12 +147,12 @@ public void shouldAuthorizeUserAsSubmitter() throws Exception { // DownloadTokenExpiredException, 401 @Test public void shouldNotAuthorizeUserByWrongToken() throws Exception { - // Admin is not the submitter. - String authTokenAdmin = getAuthToken(admin.getEmail(), password); + // The user is not submitter. + String authTokenUser = getAuthToken(ePerson2.getEmail(), eperson2Password); // Load bitstream from the item. Bitstream bitstream = item.getBundles().get(0).getBitstreams().get(0); - getClient(authTokenAdmin).perform(get("/api/authrn/" + + getClient(authTokenUser).perform(get("/api/authrn/" + bitstream.getID().toString() + "?dtoken=wrongToken")) .andExpect(status().isUnauthorized()) .andExpect(status().reason(is(Matchers.is(DownloadTokenExpiredException.NAME)))); @@ -192,10 +221,10 @@ public void shouldNotAuthorizeByExpiredToken() throws Exception { context.restoreAuthSystemState(); Bitstream bitstream = witem.getItem().getBundles().get(0).getBitstreams().get(0); - // Admin is not the submitter - String authTokenAdmin = getAuthToken(admin.getEmail(), password); - // The admin should be authorized to download the bitstream with token - getClient(authTokenAdmin).perform(get("/api/authrn/" + + // The user is not submitter + String authTokenUser = getAuthToken(ePerson2.getEmail(), eperson2Password); + // The user should not be authorized to download the bitstream with expired token + getClient(authTokenUser).perform(get("/api/authrn/" + bitstream.getID().toString() + "?dtoken=" + token)) .andExpect(status().isUnauthorized()) .andExpect(status().reason(is(Matchers.is(DownloadTokenExpiredException.NAME)))); @@ -254,8 +283,8 @@ public void shouldNotAuthorizeWhenUserMetadataAreNotFilledIn() throws Exception Bitstream bitstream = witem.getItem().getBundles().get(0).getBitstreams().get(0); // Admin is not the submitter - String authTokenAdmin = getAuthToken(admin.getEmail(), password); - getClient(authTokenAdmin).perform(get("/api/authrn/" + bitstream.getID().toString())) + String eperson2Token = getAuthToken(ePerson2.getEmail(), eperson2Password); + getClient(eperson2Token).perform(get("/api/authrn/" + bitstream.getID().toString())) .andExpect(status().isUnauthorized()) .andExpect(status().reason(is(Matchers.is(MissingLicenseAgreementException.NAME)))); } diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/MetadataBitstreamControllerIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/MetadataBitstreamControllerIT.java index d397062e0200..63f7e62206f8 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/MetadataBitstreamControllerIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/MetadataBitstreamControllerIT.java @@ -19,6 +19,7 @@ import org.apache.commons.compress.archivers.zip.ZipArchiveEntry; import org.apache.commons.compress.archivers.zip.ZipArchiveOutputStream; import org.apache.commons.io.IOUtils; +import org.dspace.app.rest.model.ItemRest; import org.dspace.app.rest.test.AbstractControllerIntegrationTest; import org.dspace.authorize.service.AuthorizeService; import org.dspace.builder.BitstreamBuilder; @@ -33,9 +34,9 @@ import org.springframework.beans.factory.annotation.Autowired; public class MetadataBitstreamControllerIT extends AbstractControllerIntegrationTest { - private static final String METADATABITSTREAM_ENDPOINT = "/api/core/bitstreams/"; - private static final String METADATABITSTREAM_DOWNLOAD_SINGLE_ENDPOINT = METADATABITSTREAM_ENDPOINT + "/handle"; - private static final String METADATABITSTREAM_DOWNLOAD_ALL_ENDPOINT = METADATABITSTREAM_ENDPOINT + "/allzip"; + private static final String METADATABITSTREAM_ENDPOINT = "/api/" + ItemRest.CATEGORY + "/" + ItemRest.PLURAL_NAME; + private static final String ALL_ZIP_PATH = "allzip"; + private static final String HANDLE_PARAM = "handleId"; private static final String AUTHOR = "Test author name"; private Collection col; @@ -75,29 +76,6 @@ public void setUp() throws Exception { context.restoreAuthSystemState(); } - @Test - public void downloadSingleFileNullPathVariable() throws Exception { - getClient().perform(get(METADATABITSTREAM_DOWNLOAD_SINGLE_ENDPOINT)).andExpect(status().is4xxClientError()); - } - - @Test - public void downloadSingleFileWithAuthorize() throws Exception { - InputStream ip = bitstreamService.retrieve(context, bts); - String token = getAuthToken(admin.getEmail(), password); - getClient(token).perform(get(METADATABITSTREAM_DOWNLOAD_SINGLE_ENDPOINT + - "/" + publicItem.getHandle() + "/" + bts.getName())) - .andExpect(status().isOk()) - .andExpect(content().contentType("application/octet-stream;charset=UTF-8")) - .andExpect(content().bytes(IOUtils.toByteArray(ip))); - } - - @Test - public void downloadSingleFileWithNoAuthorize() throws Exception { - getClient().perform(get(METADATABITSTREAM_DOWNLOAD_SINGLE_ENDPOINT + - "/" + publicItem.getHandle() + "/" + bts.getName())) - .andExpect(status().isOk()); - } - @Test public void downloadAllZip() throws Exception { ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream(); @@ -113,12 +91,10 @@ public void downloadAllZip() throws Exception { zip.close(); String token = getAuthToken(admin.getEmail(), password); - getClient(token).perform(get(METADATABITSTREAM_DOWNLOAD_ALL_ENDPOINT ).param("handleId", - publicItem.getHandle())) + getClient(token).perform(get(METADATABITSTREAM_ENDPOINT + "/" + publicItem.getID() + + "/" + ALL_ZIP_PATH).param(HANDLE_PARAM, publicItem.getHandle())) .andExpect(status().isOk()) .andExpect(content().bytes(byteArrayOutputStream.toByteArray())); } - - } From 2f8a81d9095093f091f82633bec0152f68323408 Mon Sep 17 00:00:00 2001 From: milanmajchrak <90026355+milanmajchrak@users.noreply.github.com> Date: Wed, 15 Nov 2023 12:55:58 +0100 Subject: [PATCH 2/2] ufal/be-cannot-download-and-preview-files-after-migration (#454) * Find bitstream format using mimetype not ID. * Updated tests according to fix. --- .../rest/ClarinBitstreamImportController.java | 7 +++-- .../ClarinBitstreamImportControllerIT.java | 26 +++++++++---------- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/ClarinBitstreamImportController.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/ClarinBitstreamImportController.java index 97ac1494d160..63380a756c2f 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/ClarinBitstreamImportController.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/ClarinBitstreamImportController.java @@ -127,11 +127,10 @@ public BitstreamRest importBitstreamForExistingFile(HttpServletRequest request) log.info("SequenceId is null. Bitstream UUID: " + bitstream.getID()); } //add bitstream format - String bitstreamFormatIdString = request.getParameter("bitstreamFormat"); - Integer bitstreamFormatId = getIntegerFromString(bitstreamFormatIdString); + String bitstreamFormatMimeType = request.getParameter("bitstreamFormat"); BitstreamFormat bitstreamFormat = null; - if (!Objects.isNull(bitstreamFormatId)) { - bitstreamFormat = bitstreamFormatService.find(context, bitstreamFormatId); + if (StringUtils.isNotBlank(bitstreamFormatMimeType)) { + bitstreamFormat = bitstreamFormatService.findByMIMEType(context, bitstreamFormatMimeType); } bitstream.setFormat(context, bitstreamFormat); String deletedString = request.getParameter("deleted"); diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/ClarinBitstreamImportControllerIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/ClarinBitstreamImportControllerIT.java index e16260565dc4..9e0c0f339a7c 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/ClarinBitstreamImportControllerIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/ClarinBitstreamImportControllerIT.java @@ -95,7 +95,7 @@ public void setup() throws Exception { .withName("TESTINGBUNDLE") .build(); token = getAuthToken(admin.getEmail(), password); - bitstreamFormat = BitstreamFormatBuilder.createBitstreamFormat(context).build(); + bitstreamFormat = BitstreamFormatBuilder.createBitstreamFormat(context).withMimeType("application/pdf").build(); String input = "Hello, World!"; MockMultipartFile file = new MockMultipartFile("file", "hello.txt", MediaType.TEXT_PLAIN_VALUE, @@ -147,7 +147,7 @@ public void importBitstreamForExistingFileWithBundleTest() throws Exception { .contentType(contentType) .param("internal_id", internalId) .param("storeNumber", Integer.toString(storeNumber)) - .param("bitstreamFormat", Integer.toString(bitstreamFormat.getID())) + .param("bitstreamFormat", bitstreamFormat.getMIMEType()) .param("deleted", Boolean.toString(deleted)) .param("sequenceId", Integer.toString(sequence)) .param("primaryBundle_id", "") @@ -156,8 +156,8 @@ public void importBitstreamForExistingFileWithBundleTest() throws Exception { .andReturn().getResponse().getContentAsString(), "$.id")); - checkCreatedBitstream(uuid, internalId, storeNumber, bitstreamFormat.getID(), sequence, deleted, sizeBytes, - checkSum); + checkCreatedBitstream(uuid, internalId, storeNumber, bitstreamFormat.getMIMEType(), sequence, deleted, + sizeBytes, checkSum); //clean all context.turnOffAuthorisationSystem(); @@ -182,7 +182,7 @@ public void importBitstreamForExistingFileWithoutBundleTest() throws Exception { .contentType(contentType) .param("internal_id", internalId) .param("storeNumber", Integer.toString(storeNumber)) - .param("bitstreamFormat", Integer.toString(bitstreamFormat.getID())) + .param("bitstreamFormat", bitstreamFormat.getMIMEType()) .param("deleted", Boolean.toString(deleted)) .param("sequenceId", Integer.toString(sequence)) .param("primaryBundle_id", "") @@ -191,8 +191,8 @@ public void importBitstreamForExistingFileWithoutBundleTest() throws Exception { .andReturn().getResponse().getContentAsString(), "$.id")); - checkCreatedBitstream(uuid, internalId, storeNumber, bitstreamFormat.getID(), sequence, deleted, sizeBytes, - checkSum); + checkCreatedBitstream(uuid, internalId, storeNumber, bitstreamFormat.getMIMEType(), sequence, deleted, + sizeBytes, checkSum); //clean all context.turnOffAuthorisationSystem(); @@ -217,7 +217,7 @@ public void importBitstreamForExistingFileAsPrimaryBitstreamOfBundleTest() throw .contentType(contentType) .param("internal_id", internalId) .param("storeNumber", Integer.toString(storeNumber)) - .param("bitstreamFormat", Integer.toString(bitstreamFormat.getID())) + .param("bitstreamFormat", bitstreamFormat.getMIMEType()) .param("deleted", Boolean.toString(deleted)) .param("sequenceId", Integer.toString(sequence)) .param("primaryBundle_id", bundle.getID().toString()) @@ -226,8 +226,8 @@ public void importBitstreamForExistingFileAsPrimaryBitstreamOfBundleTest() throw .andReturn().getResponse().getContentAsString(), "$.id")); - checkCreatedBitstream(uuid, internalId, storeNumber, bitstreamFormat.getID(), sequence, deleted, sizeBytes, - checkSum); + checkCreatedBitstream(uuid, internalId, storeNumber, bitstreamFormat.getMIMEType(), sequence, deleted, + sizeBytes, checkSum); bundle = bundleService.find(context, bundle.getID()); assertEquals(bundle.getPrimaryBitstream().getID(), bitstream.getID()); @@ -257,7 +257,7 @@ public void importBitstreamForExistingFileValidationErrorTest() throws Exception .contentType(contentType) .param("internal_id", internalId) .param("storeNumber", Integer.toString(storeNumber)) - .param("bitstreamFormat", Integer.toString(bitstreamFormat.getID())) + .param("bitstreamFormat", bitstreamFormat.getMIMEType()) .param("deleted", Boolean.toString(deleted)) .param("sequenceId", Integer.toString(sequence)) .param("primaryBundle_id", "") @@ -271,12 +271,12 @@ public void importBitstreamForExistingFileValidationErrorTest() throws Exception } private void checkCreatedBitstream(UUID uuid, String internalId, int storeNumber, - Integer bitstreamFormat, int sequence, boolean deleted, long sizeBytes, + String bitstreamFormat, int sequence, boolean deleted, long sizeBytes, String checkSum) throws SQLException { bitstream = bitstreamService.find(context, uuid); assertEquals(bitstream.getChecksum(), checkSum); assertEquals(bitstream.getSizeBytes(), sizeBytes); - assertEquals(bitstream.getFormat(context).getID(), bitstreamFormat); + assertEquals(bitstream.getFormat(context).getMIMEType(), bitstreamFormat); assertEquals(bitstream.getInternalId(), internalId); assertEquals(bitstream.getStoreNumber(), storeNumber); assertEquals(bitstream.getSequenceID(), sequence);