Skip to content

Commit

Permalink
Merged in task/main-cris/DSC-2139 (pull request DSpace#3284)
Browse files Browse the repository at this point in the history
Task/main cris/DSC-2139

Approved-by: Vincenzo Mecca
  • Loading branch information
AdamF42 authored and vins01-4science committed Jan 10, 2025
2 parents 3342c8c + aea2867 commit bafb2b5
Show file tree
Hide file tree
Showing 3 changed files with 288 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,10 @@
*/
public class ImportFileUtil {

private static final Logger log = LogManager.getLogger(ImportFileUtil.class);

public static final String REMOTE = "REMOTE";

private static final String LOCAL = "LOCAL";

public static final String FTP = "FTP";

private static final Logger log = LogManager.getLogger(ImportFileUtil.class);
private static final String LOCAL = "LOCAL";
private static final String HTTP_PREFIX = "http:";

private static final String HTTPS_PREFIX = "https:";
Expand All @@ -47,6 +43,14 @@ public class ImportFileUtil {
private static final String FTP_PREFIX = "ftp:";

private static final String UNKNOWN = "UNKNOWN";
protected DSpaceRunnableHandler handler;

public ImportFileUtil() {
}

public ImportFileUtil(DSpaceRunnableHandler handler) {
this.handler = handler;
}

/**
* This method check if the given {@param possibleChild} is contained in the specified
Expand All @@ -73,14 +77,6 @@ private static String[] getAllowedIps() {
.getArrayProperty("allowed.ips.import");
}

protected DSpaceRunnableHandler handler;

public ImportFileUtil() {}

public ImportFileUtil(DSpaceRunnableHandler handler) {
this.handler = handler;
}

public Optional<InputStream> getInputStream(String path) {
String fileLocationType = getFileLocationTypeByPath(path);

Expand All @@ -99,6 +95,7 @@ protected void logWarning(String message) {
}
}


private String getFileLocationTypeByPath(String path) {
if (StringUtils.isNotBlank(path)) {
if (path.startsWith(HTTP_PREFIX) || path.startsWith(HTTPS_PREFIX)) {
Expand Down Expand Up @@ -160,9 +157,18 @@ private Optional<InputStream> getInputStreamOfLocalFile(String path) throws IOEx
}

private Optional<InputStream> getInputStreamOfRemoteFile(String path) throws IOException {
return Optional.of(new URL(path))
.filter(url -> Set.of(getAllowedIps()).contains(url.getHost()))
.map(this::openStream);
URL url = getUrl(path);
Set<String> allowedIps = Set.of(getAllowedIps());
String host = url.getHost();

if (allowedIps.isEmpty() || allowedIps.contains(host)) {
return Optional.ofNullable(openStream(url));
}

// Log the rejected domain
logWarning(String.format("Domain '%s' is not in the allowed list. Path: %s", host, path));

return Optional.empty();
}

protected InputStream openStream(URL url) {
Expand All @@ -173,9 +179,19 @@ protected InputStream openStream(URL url) {
}
}

protected URL getUrl(String path) throws IOException {
return new URL(path);
}

private Optional<InputStream> getInputStreamOfFtpFile(String url) throws IOException {
URL urlObject = new URL(url);
URL urlObject = getUrl(url);
URLConnection urlConnection = urlObject.openConnection();
return Optional.of(urlConnection.getInputStream());
InputStream inputStream = urlConnection.getInputStream();
if (inputStream == null) {
logError(
String.format("Failed to obtain InputStream for FTP URL: %s. getInputStream() returned null", url));
return Optional.empty();
}
return Optional.of(inputStream);
}
}
61 changes: 59 additions & 2 deletions dspace-api/src/test/java/org/dspace/app/bulkedit/BulkImportIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -1577,6 +1577,7 @@ public void testUploadBitstreamWithRemoteFilePathNotFromAllowedIps() throws Exce
assertThat(handler.getErrorMessages(), contains(
"Cannot create bitstream from file at path http://127.0.1.1"));
assertThat(handler.getWarningMessages(), contains(
containsString("Domain '127.0.1.1' is not in the allowed list. Path: http://127.0.1.1"),
containsString("Row 2 - Invalid item left in workspace"),
containsString("Row 3 - Invalid item left in workspace")));
assertThat(handler.getInfoMessages(), contains(
Expand All @@ -1586,8 +1587,8 @@ public void testUploadBitstreamWithRemoteFilePathNotFromAllowedIps() throws Exce
is("Found 1 bitstreams to process"),
is("Found 2 items to process")));

Item item = getItemFromMessage(handler.getWarningMessages().get(0));
Item item2 = getItemFromMessage(handler.getWarningMessages().get(1));
Item item = getItemFromMessage(handler.getWarningMessages().get(1));
Item item2 = getItemFromMessage(handler.getWarningMessages().get(2));

assertThat(getItemBitstreamsByBundle(item, "TEST-BUNDLE"), empty());
assertThat(getItemBitstreamsByBundle(item, "TEST-BUNDLE2"), empty());
Expand Down Expand Up @@ -1659,6 +1660,62 @@ public void testUploadBitstreamWithRemoteFilePathFromAllowedIps() throws Excepti
}
}

@Test
public void testUploadBitstreamWithRemoteFilePathAndEmptyAllowedIps() throws Exception {

InputStream mockInputStream = new ByteArrayInputStream("mocked content".getBytes());

context.turnOffAuthorisationSystem();
Collection publication = createCollection(context, community)
.withSubmissionDefinition("publication")
.withAdminGroup(eperson)
.build();
context.commit();
context.restoreAuthSystemState();

String fileLocation = getXlsFilePath("add-bitstream-with-http-url-to-item.xls");

String[] args = new String[] { "bulk-import", "-c", publication.getID().toString(), "-f", fileLocation,
"-e", eperson.getEmail()};
TestDSpaceRunnableHandler handler = new TestDSpaceRunnableHandler();

ImportFileUtilMockClass importFileUtilSpy = spy(new ImportFileUtilMockClass(handler));
doReturn(mockInputStream).when(importFileUtilSpy).openStream(any(URL.class));

ScriptService scriptService = ScriptServiceFactory.getInstance().getScriptService();
ScriptConfiguration scriptConfiguration = scriptService.getScriptConfiguration(args[0]);

BulkImport script = null;
if (scriptConfiguration != null) {
script = (BulkImport) scriptService.createDSpaceRunnableForScriptConfiguration(scriptConfiguration);
script.setImportFileUtil(importFileUtilSpy);
}
if (script != null) {
if (DSpaceRunnable.StepResult.Continue.equals(script.initialize(args, handler, eperson))) {
script.run();
}
}

assertThat(handler.getErrorMessages(), empty());
assertThat(handler.getWarningMessages(), contains(
containsString("Row 2 - Invalid item left in workspace"),
containsString("Row 3 - Invalid item left in workspace")));
assertThat(handler.getInfoMessages(), contains(
is("Start reading all the metadata group rows"),
is("Found 4 metadata groups to process"),
is("Start reading all the bitstream rows"),
is("Found 1 bitstreams to process"),
is("Found 2 items to process"),
containsString("Sheet bitstream-metadata - Row 2 - Bitstream created successfully")));

Item item = getItemFromMessage(handler.getWarningMessages().get(0));

assertThat(getItemBitstreamsByBundle(item, "TEST-BUNDLE"), contains(
bitstreamWith("Test title", "test file description",
"mocked content")));

}

@Test
public void testUploadMultipleBitstreamWithCorrectLocalPath() throws Exception {

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,194 @@
/**
* 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.bulkimport.util;

import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.mockStatic;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

import java.io.IOException;
import java.io.InputStream;
import java.net.URL;
import java.net.URLConnection;
import java.util.Optional;

import org.dspace.AbstractDSpaceTest;
import org.dspace.services.ConfigurationService;
import org.dspace.services.factory.DSpaceServicesFactory;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.mockito.MockedStatic;

public class ImportFileUtilTest extends AbstractDSpaceTest {

private ImportFileUtil importFileUtil;

private ConfigurationService configurationService;
private MockedStatic<DSpaceServicesFactory> dSpaceServicesFactoryMockedStatic;
private URLConnection urlConnection;

@Before
public void setUp() {
// Initialize the main class under test
importFileUtil = spy(new ImportFileUtil());

// Setup common mocks
dSpaceServicesFactoryMockedStatic = mockStatic(DSpaceServicesFactory.class);
DSpaceServicesFactory dSpaceServicesFactory = mock(DSpaceServicesFactory.class);
configurationService = mock(ConfigurationService.class);
urlConnection = mock(URLConnection.class);


// Setup common mock behavior
dSpaceServicesFactoryMockedStatic.when(DSpaceServicesFactory::getInstance)
.thenReturn(dSpaceServicesFactory);
when(dSpaceServicesFactory.getConfigurationService()).thenReturn(configurationService);
}

@After
public void tearDown() {
if (dSpaceServicesFactoryMockedStatic != null) {
dSpaceServicesFactoryMockedStatic.close();
}
}

@Test
public void getInputStream_shouldReturnStream_whenRemoteHostIsInAllowedList() throws Exception {
// Given
String allowedHost = "example.com";
when(configurationService.getArrayProperty("allowed.ips.import"))
.thenReturn(new String[] {allowedHost});

String path = "http://example.com/file.txt";
URL url = mock(URL.class);
InputStream mockStream = mock(InputStream.class);

doReturn(url).when(importFileUtil).getUrl(path);
when(url.getHost()).thenReturn(allowedHost);
doReturn(mockStream).when(importFileUtil).openStream(url);

// When
Optional<InputStream> result = importFileUtil.getInputStream(path);

// Then
assertTrue(result.isPresent());
verify(importFileUtil).openStream(url);
}

@Test
public void getInputStream_shouldReturnStream_whenNoIpRestrictionConfigured() throws Exception {
// Given
when(configurationService.getArrayProperty("allowed.ips.import"))
.thenReturn(new String[] {});

String path = "http://example.com/file.txt";
URL url = mock(URL.class);
InputStream mockStream = mock(InputStream.class);

doReturn(url).when(importFileUtil).getUrl(path);
doReturn(mockStream).when(importFileUtil).openStream(url);

// When
Optional<InputStream> result = importFileUtil.getInputStream(path);

// Then
assertTrue(result.isPresent());
verify(importFileUtil).openStream(url);
}

@Test
public void getInputStream_shouldReturnEmpty_whenRemoteHostIsNotAllowed() throws IOException {
// Given
when(configurationService.getArrayProperty("allowed.ips.import"))
.thenReturn(new String[] {"otherdomain.com"});

String path = "http://example.com/file.txt";
URL url = mock(URL.class);
doReturn(url).when(importFileUtil).getUrl(path);
when(url.getHost()).thenReturn("example.com");

// When
Optional<InputStream> result = importFileUtil.getInputStream(path);

// Then
assertFalse(result.isPresent());
}


@Test
public void getInputStream_shouldReturnEmpty_whenProtocolIsUnknown() {
// Given
String path = "unknown://file.txt";

// When
Optional<InputStream> result = importFileUtil.getInputStream(path);

// Then
assertFalse(result.isPresent());
}

@Test
public void getInputStream_shouldReturnEmpty_whenLocalFileDoesNotExist() {
// Given
when(configurationService.getProperty("uploads.local-folder"))
.thenReturn("/local/uploads");

String path = "file://file.txt";

// When
Optional<InputStream> result = importFileUtil.getInputStream(path);

// Then
assertFalse(result.isPresent());
}

@Test
public void getInputStream_shouldReturnStream_whenFtpConnectionSucceeds() throws Exception {
// Given
String ftpPath = "ftp://example.com/file.txt";
URL mockUrl = mock(URL.class);
InputStream mockInputStream = mock(InputStream.class);

doReturn(mockUrl).when(importFileUtil).getUrl(ftpPath);
when(mockUrl.openConnection()).thenReturn(urlConnection);
when(urlConnection.getInputStream()).thenReturn(mockInputStream);

// When
Optional<InputStream> result = importFileUtil.getInputStream(ftpPath);

// Then
assertTrue(result.isPresent());
verify(urlConnection).getInputStream();
}

@Test
public void getInputStream_shouldReturnEmpty_whenFtpStreamIsNull() throws Exception {
// Given
String ftpPath = "ftp://example.com/file.txt";
URL mockUrl = mock(URL.class);

doReturn(mockUrl).when(importFileUtil).getUrl(ftpPath);
when(mockUrl.openConnection()).thenReturn(urlConnection);
when(urlConnection.getInputStream()).thenReturn(null);

// When
Optional<InputStream> result = importFileUtil.getInputStream(ftpPath);

// Then
assertFalse(result.isPresent());
verify(urlConnection).getInputStream();
}

}

0 comments on commit bafb2b5

Please sign in to comment.