Skip to content

[Port to dtq-dev] Issue ufal/clarin-dspace#1321: disable File preview for files where user has no Bitstream READ permission #1280

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kosarko wants to merge 1 commit into
base: dtq-dev
Choose a base branch
from
Open

[Port to dtq-dev] Issue ufal/clarin-dspace#1321: disable File preview for files where user has no Bitstream READ permission #1280

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 22 additions & 14 deletions dspace-api/src/main/java/org/dspace/content/PreviewContentServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,21 +155,26 @@ public List<PreviewContent> findAll(Context context) throws SQLException {
@Override
public boolean canPreview(Context context, Bitstream bitstream, boolean authorization)
throws SQLException, AuthorizeException {
try {
// Check it is allowed by configuration
boolean isAllowedByCfg = configurationService.getBooleanProperty("file.preview.enabled", true);
if (!isAllowedByCfg) {
return false;
}

// Check it is allowed by license
if (authorization) {
// Check it is allowed by configuration
boolean isAllowedByCfg = configurationService.getBooleanProperty("file.preview.enabled", true);
if (!isAllowedByCfg) {
return false;
}
if (authorization) {
// Verify that bitstream policy allows user to READ the bitstream.
// If not, the preview content is disabled.
try {
authorizeService.authorizeAction(context, bitstream, Constants.READ);
} catch (AuthorizeException e) {
// In case the license agreement(for bitstream downloading) is needed,
// the MissingLicenseAgreementException, that extends AuthorizeException, is thrown.
// For this case we also disable the content preview.
// Otherwise, user could see the content of some files without accepting the agreement,
// which could cause a security issue.
return false;
}
return true;
} catch (MissingLicenseAgreementException e) {
return false;
}
return true;
}

@Override
Expand All @@ -178,13 +183,16 @@ public List<FileInfo> getFilePreviewContent(Context context, Bitstream bitstream
File file = null;

try {
file = bitstreamService.retrieveFile(context, bitstream, false); // Retrieve the file
file = bitstreamService.retrieveFile(context, bitstream, true); // Retrieve the file

if (Objects.nonNull(file)) {
fileInfos = processFileToFilePreview(context, bitstream, file);
}
} catch (MissingLicenseAgreementException e) {
log.error("Missing license agreement: ", e);
log.warn("File Preview disabled: Missing license agreement!");
throw e;
} catch (AuthorizeException e) {
log.warn("File Preview disabled: Authorization error!");
throw e;
} catch (IOException e) {
log.error("IOException during file processing: ", e);
Expand Down
113 changes: 78 additions & 35 deletions dspace-api/src/test/java/org/dspace/scripts/filepreview/FilePreviewIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import static org.hamcrest.Matchers.hasItem;
import static org.hamcrest.Matchers.hasSize;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;

Expand Down Expand Up @@ -60,15 +61,16 @@ public class FilePreviewIT extends AbstractIntegrationTestWithDatabase {

Collection collection;
Item item;
EPerson eperson;
// avoid using eperson created in superclass
EPerson ePerson;
String PASSWORD = "test";

@Before
public void setup() throws SQLException, AuthorizeException {
InputStream previewZipIs = getClass().getResourceAsStream("preview-file-test.zip");

context.turnOffAuthorisationSystem();
eperson = EPersonBuilder.createEPerson(context)
ePerson = EPersonBuilder.createEPerson(context)
.withEmail("test@test.edu").withPassword(PASSWORD).build();
Community community = CommunityBuilder.createCommunity(context).withName("Com").build();
collection = CollectionBuilder.createCollection(context, community).withName("Col").build();
Expand Down Expand Up @@ -106,7 +108,7 @@ public void testUnauthorizedEmail() throws Exception {
public void testUnauthorizedPassword() throws Exception {
// Run the script
TestDSpaceRunnableHandler testDSpaceRunnableHandler = new TestDSpaceRunnableHandler();
String[] args = new String[] { "file-preview", "-e", eperson.getEmail()};
String[] args = new String[] { "file-preview", "-e", ePerson.getEmail()};
int run = ScriptLauncher.handleScript(args, ScriptLauncher.getConfig(kernelImpl),
testDSpaceRunnableHandler, kernelImpl);
assertEquals(1, run); // Since a ParseException was caught, expect return code 1
Expand All @@ -116,7 +118,7 @@ public void testUnauthorizedPassword() throws Exception {
public void testWhenNoFilesRun() throws Exception {
TestDSpaceRunnableHandler testDSpaceRunnableHandler = new TestDSpaceRunnableHandler();

String[] args = new String[] { "file-preview", "-e", eperson.getEmail(), "-p", PASSWORD };
String[] args = new String[] { "file-preview", "-e", ePerson.getEmail(), "-p", PASSWORD };
int run = ScriptLauncher.handleScript(args, ScriptLauncher.getConfig(kernelImpl),
testDSpaceRunnableHandler, kernelImpl);
assertEquals(0, run);
Expand All @@ -125,53 +127,68 @@ public void testWhenNoFilesRun() throws Exception {

@Test
public void testForSpecificItem() throws Exception {
Item item2 = createOtherWorkspaceItemWithBitstream(ePerson, 0);
// Run the script
runScriptForItemWithBitstreams(item);
runScriptForItemWithBitstreams(item2, ePerson, PASSWORD);

Bitstream b = bitstreamService.findAll(context).stream()
.filter(bitstream -> bitstream.getName().equals("logos.tgz"))
.findFirst().orElse(null);

assertNotNull(b);
assertEquals("logos.tgz", b.getName());

// the preview content was created since the item was created by the same user as the script was run
assertTrue("Expects preview content created.", previewContentService.hasPreview(context, b));
assertEquals(2, previewContentService.getPreview(context, b).size());
}

@Test
public void testPreviewWithSyncStorage() throws Exception {
configurationService.setProperty("sync.storage.service.enabled", true);
public void testWhenScriptCannotCreateFilePreview() throws Exception {
Item item2 = createOtherWorkspaceItemWithBitstream(eperson, 0);
// Run the script as another user, without admin rights
runScriptForItemWithBitstreams(item2, ePerson, PASSWORD);

context.turnOffAuthorisationSystem();
Bitstream b = bitstreamService.findAll(context).stream()
.filter(bitstream -> bitstream.getName().equals("logos.tgz"))
.findFirst().orElse(null);

WorkspaceItem wItem2;
try (InputStream tgzFile = getClass().getResourceAsStream("logos.tgz")) {
wItem2 = WorkspaceItemBuilder.createWorkspaceItem(context, collection)
.withBitstream("logos.tgz", "/local/path/logos.tgz", tgzFile, SYNC_STORE_NUMBER)
.build();
}
assertNotNull(b);
assertEquals("logos.tgz", b.getName());

context.restoreAuthSystemState();
// the preview content cannot be created since the item was created by another user (eperson)
// than the user (ePerson) who runs the script
assertFalse("Expects preview content not created.", previewContentService.hasPreview(context, b));

// Get the item and its bitstream
Item item2 = wItem2.getItem();
List<Bundle> bundles = item2.getBundles();
Bitstream bitstream2 = bundles.get(0).getBitstreams().get(0);
// Run the script as admin user
runScriptForItemWithBitstreams(item2, admin, password);

// Set the bitstream format to application/zip
BitstreamFormat bitstreamFormat = bitstreamFormatService.findByMIMEType(context, "application/x-gtar");
bitstream2.setFormat(context, bitstreamFormat);
bitstreamService.update(context, bitstream2);
context.commit();
context.reloadEntity(bitstream2);
context.reloadEntity(item2);
// now the preview content was created since the script was run by admin user
assertTrue("Expects preview content created.", previewContentService.hasPreview(context, b));
assertEquals(2, previewContentService.getPreview(context, b).size());
}

runScriptForItemWithBitstreams(item2);
@Test
public void testPreviewWithSyncStorage() throws Exception {
configurationService.setProperty("sync.storage.service.enabled", true);
Item item2 = createOtherWorkspaceItemWithBitstream(ePerson, SYNC_STORE_NUMBER);
// Run the script
runScriptForItemWithBitstreams(item2, ePerson, PASSWORD);

Bitstream b2 = bitstreamService.findAll(context).stream()
.filter(b -> b.getStoreNumber() == SYNC_STORE_NUMBER)
Bitstream b = bitstreamService.findAll(context).stream()
.filter(bitstream -> bitstream.getStoreNumber() == SYNC_STORE_NUMBER)
.findFirst().orElse(null);

assertNotNull(b2);
assertTrue("Expects preview content created and stored.", previewContentService.hasPreview(context, b2));
assertNotNull(b);
assertEquals("logos.tgz", b.getName());
assertTrue("Expects preview content created and stored.", previewContentService.hasPreview(context, b));
}

@Test
public void testForAllItem() throws Exception {
// Run the script
TestDSpaceRunnableHandler testDSpaceRunnableHandler = new TestDSpaceRunnableHandler();
String[] args = new String[] { "file-preview", "-e", eperson.getEmail(), "-p", PASSWORD};
String[] args = new String[] { "file-preview", "-e", ePerson.getEmail(), "-p", PASSWORD};
int run = ScriptLauncher.handleScript(args, ScriptLauncher.getConfig(kernelImpl),
testDSpaceRunnableHandler, kernelImpl);
assertEquals(0, run);
Expand All @@ -184,11 +201,11 @@ private void checkNoError(TestDSpaceRunnableHandler testDSpaceRunnableHandler) {
assertThat(testDSpaceRunnableHandler.getWarningMessages(), empty());
}

private void runScriptForItemWithBitstreams(Item item) throws Exception {
private void runScriptForItemWithBitstreams(Item item, EPerson user, String password) throws Exception {
// Run the script
TestDSpaceRunnableHandler testDSpaceRunnableHandler = new TestDSpaceRunnableHandler();
String[] args = new String[] { "file-preview", "-u", item.getID().toString(),
"-e", eperson.getEmail(), "-p", PASSWORD};
"-e", user.getEmail(), "-p", password};
int run = ScriptLauncher.handleScript(args, ScriptLauncher.getConfig(kernelImpl),
testDSpaceRunnableHandler, kernelImpl);
assertEquals(0, run);
Expand All @@ -201,6 +218,32 @@ private void runScriptForItemWithBitstreams(Item item) throws Exception {
assertThat(messages, hasItem(containsString("Generate the file previews for the specified item with " +
"the given UUID: " + item.getID())));
assertThat(messages,
hasItem(containsString("Authentication by user: " + eperson.getEmail())));
hasItem(containsString("Authentication by user: " + user.getEmail())));
}

private Item createOtherWorkspaceItemWithBitstream(EPerson user, int storageNumber) throws Exception {
context.turnOffAuthorisationSystem();
context.setCurrentUser(user);
WorkspaceItem wItem2;
try (InputStream tgzFile = getClass().getResourceAsStream("logos.tgz")) {
wItem2 = WorkspaceItemBuilder.createWorkspaceItem(context, collection)
.withBitstream("logos.tgz", "/local/path/logos.tgz", tgzFile, storageNumber)
.build();
}
context.restoreAuthSystemState();

Item item2 = wItem2.getItem();
List<Bundle> bundles = item2.getBundles();
Bitstream bitstream2 = bundles.get(0).getBitstreams().get(0);

// Set the bitstream format to application/zip
BitstreamFormat bitstreamFormat = bitstreamFormatService.findByMIMEType(context, "application/x-gtar");
bitstream2.setFormat(context, bitstreamFormat);
bitstreamService.update(context, bitstream2);
context.commit();
context.reloadEntity(bitstream2);
context.reloadEntity(item2);

return item2;
}
}
10 changes: 8 additions & 2 deletions ...-webapp/src/main/java/org/dspace/app/rest/repository/MetadataBitstreamRestRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.dspace.app.rest.exception.UnprocessableEntityException;
import org.dspace.app.rest.model.MetadataBitstreamWrapperRest;
import org.dspace.app.rest.model.wrapper.MetadataBitstreamWrapper;
import org.dspace.authorize.AuthorizeException;
import org.dspace.content.Bitstream;
import org.dspace.content.Bundle;
import org.dspace.content.DSpaceObject;
Expand Down Expand Up @@ -108,7 +109,7 @@ public Page<MetadataBitstreamWrapperRest> findByHandle(@Parameter(value = "handl
for (Bitstream bitstream : bitstreams) {
String url = previewContentService.composePreviewURL(context, item, bitstream, contextPath);
List<FileInfo> fileInfos = new ArrayList<>();
boolean canPreview = previewContentService.canPreview(context, bitstream, false);
boolean canPreview = previewContentService.canPreview(context, bitstream, true);
String mimeType = bitstream.getFormat(context).getMIMEType();
// HTML content could be longer than the limit, so we do not store it in the DB.
// It has to be generated even if property is false.
Expand All @@ -119,7 +120,12 @@ public Page<MetadataBitstreamWrapperRest> findByHandle(@Parameter(value = "handl
boolean allowComposePreviewContent = configurationService.getBooleanProperty
("create.file-preview.on-item-page-load", false);
if (allowComposePreviewContent) {
fileInfos.addAll(previewContentService.getFilePreviewContent(context, bitstream));
try {
fileInfos.addAll(previewContentService.getFilePreviewContent(context, bitstream));
} catch (AuthorizeException e) {
log.warn("Cannot create preview content for bitstream: {} because: {}",
bitstream.getID(), e.getMessage());
}
// Do not store HTML content in the database because it could be longer than the limit
// of the database column
if (!fileInfos.isEmpty() &&
Expand Down
Loading
Loading