Skip to content
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

Only show thumbnails if user has viewAccessCopies permissions #1630

Merged
merged 5 commits into from
Dec 5, 2023
Merged

Only show thumbnails if user has viewAccessCopies permissions #1630

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
038d6ca
Only show thumbnails if user has viewAccessCopies permissions
lfarrell Nov 29, 2023
ed497b4
Simplify thumbnail permissions check
lfarrell Nov 29, 2023
b78dbdd
Fix thumbnail IT tests
lfarrell Nov 29, 2023
94d50ec
Let the client handle displaying a thumbnail or not
lfarrell Dec 1, 2023
29123fa
Update test permission
lfarrell Dec 1, 2023
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
4 changes: 2 additions & 2 deletions auth-api/src/main/java/edu/unc/lib/boxc/auth/api/services/DatastreamPermissionUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ public class DatastreamPermissionUtil {
DS_PERMISSION_MAP.put(DatastreamType.ORIGINAL_FILE, Permission.viewOriginal);
DS_PERMISSION_MAP.put(DatastreamType.TECHNICAL_METADATA, Permission.viewHidden);
DS_PERMISSION_MAP.put(DatastreamType.TECHNICAL_METADATA_HISTORY, Permission.viewHidden);
DS_PERMISSION_MAP.put(DatastreamType.THUMBNAIL_SMALL, Permission.viewMetadata);
DS_PERMISSION_MAP.put(DatastreamType.THUMBNAIL_LARGE, Permission.viewMetadata);
DS_PERMISSION_MAP.put(DatastreamType.THUMBNAIL_SMALL, Permission.viewAccessCopies);
DS_PERMISSION_MAP.put(DatastreamType.THUMBNAIL_LARGE, Permission.viewAccessCopies);
}

private DatastreamPermissionUtil() {
Expand Down
2 changes: 1 addition & 1 deletion static/js/vue-cdr-access/src/components/full_record/fileList.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ export default {
render: (data, type, row) => {
let img;

if ('thumbnail_url' in row) {
if ('thumbnail_url' in row && this.hasPermission(row,'viewAccessCopies')) {
const thumbnail_title = this.$t('full_record.thumbnail_title', { title: row.title })
img = `<img class="data-thumb" loading="lazy" src="${row.thumbnail_url}"` +
` alt="${thumbnail_title}">`;
Expand Down
2 changes: 1 addition & 1 deletion static/js/vue-cdr-access/src/components/full_record/thumbnail.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ export default {
},

src() {
if (this.objectData.thumbnail_url !== undefined) {
if (this.objectData.thumbnail_url !== undefined && this.hasPermission(this.objectData, 'viewAccessCopies')) {
return this.objectData.thumbnail_url;
}

Expand Down
11 changes: 10 additions & 1 deletion static/js/vue-cdr-access/tests/unit/thumbnail.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,12 +128,21 @@ describe('thumbnail.vue', () => {
});
});

it('displays a thumbnail, if present', () => {
it('displays a thumbnail, if present and user has viewAccessCopies permissions', () => {
expect(wrapper.find('.thumbnail .thumbnail-viewer').exists()).toBe(true);
expect(wrapper.find('i.placeholder').exists()).toBe(false);
expect(wrapper.find('a').attributes('class'))
.toEqual('thumbnail thumbnail-size-large has_tooltip')
});

it('does not display a thumbnail if user does not have viewAccessCopies permissions', async () => {
let updatedRecordData = cloneDeep(recordData);
updatedRecordData.briefObject.permissions = ['viewMetadata'];
await wrapper.setProps({ thumbnailData: updatedRecordData });
expect(wrapper.find('i.placeholder').exists()).toBe(true);
expect(wrapper.find('.thumbnail .thumbnail-viewer').exists()).toBe(false);
});

it('displays a placeholder, if no thumbnail', async () => {
let updatedRecordData = cloneDeep(recordData);
updatedRecordData.briefObject.thumbnail_url = undefined;
Expand Down
5 changes: 5 additions & 0 deletions web-common/src/main/java/edu/unc/lib/boxc/web/common/services/AccessCopiesService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,12 +130,17 @@ public String getDownloadUrl(ContentObjectRecord contentObjectRecord, AccessGrou
*/
public String getThumbnailId(ContentObjectRecord contentObjectRecord, AccessGroupSet principals,
boolean checkChildren) {
if (!permissionsHelper.hasThumbnailAccess(principals, contentObjectRecord)) {
return null;
}

// Find thumbnail datastream recorded directly on the object, if present
var thumbId = DatastreamUtil.getThumbnailOwnerId(contentObjectRecord);
if (thumbId != null) {
log.debug("Found thumbnail object directly assigned to object {}", thumbId);
return thumbId;
}

// Don't need to check any further if object isn't a work or doesn't contain files with thumbnails
if (!ResourceType.Work.name().equals(contentObjectRecord.getResourceType())
|| contentObjectRecord.getFileFormatCategory() == null
Expand Down
14 changes: 13 additions & 1 deletion web-common/src/main/java/edu/unc/lib/boxc/web/common/services/PermissionsHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import static edu.unc.lib.boxc.model.api.DatastreamType.JP2_ACCESS_COPY;
import static edu.unc.lib.boxc.model.api.DatastreamType.MD_DESCRIPTIVE;
import static edu.unc.lib.boxc.model.api.DatastreamType.ORIGINAL_FILE;
import static edu.unc.lib.boxc.model.api.DatastreamType.THUMBNAIL_LARGE;
import static edu.unc.lib.boxc.model.api.DatastreamType.THUMBNAIL_SMALL;
import static org.springframework.util.Assert.notNull;

Expand Down Expand Up @@ -57,7 +58,8 @@ public boolean hasOriginalAccess(AccessGroupSet principals, ContentObjectRecord
* @return
*/
public boolean hasThumbnailAccess(AccessGroupSet principals, ContentObjectRecord metadata) {
return hasDatastreamAccess(principals, THUMBNAIL_SMALL, metadata);
return hasThumbnailPreviewAccess(principals, THUMBNAIL_SMALL, metadata) ||
hasThumbnailPreviewAccess(principals, THUMBNAIL_LARGE, metadata);
}

/**
Expand Down Expand Up @@ -107,6 +109,16 @@ public boolean hasDatastreamAccess(AccessGroupSet principals, DatastreamType dat
return accessControlService.hasAccess(metadata.getPid(), principals, permission);
}

public boolean hasThumbnailPreviewAccess(AccessGroupSet principals, DatastreamType datastream,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want to keep this as public, we should add javadoc. Its only used internal to the permissions helper though at the moment, so it'd also be reasonable to make it private.

So this is roughly the same as hasDatastreamAccess but without the check to see if the datastream exists, right? In that case, I don't think we need to check for both the LARGE and SMALL thumbnail in hasThumbnailAccess, since they have the same permission and it doesn't verify if it exists. We could probably also simplify things a bit and remove the datastream input parameter, since internally we can figure out what the thumbnail datastream(s) are.

ContentObjectRecord metadata) {
notNull(principals, "Requires agent principals");
notNull(datastream, "Requires datastream type");
notNull(metadata, "Requires metadata object");

Permission permission = getPermissionForDatastream(datastream);
return accessControlService.hasAccess(metadata.getPid(), principals, permission);
}

private static boolean containsDatastream(ContentObjectRecord metadata, String datastream) {
return metadata.getDatastreamObjects().stream()
.anyMatch(d -> d.getName().equals(datastream));
Expand Down
64 changes: 63 additions & 1 deletion web-common/src/test/java/edu/unc/lib/boxc/web/common/services/AccessCopiesServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import java.util.List;
import java.util.UUID;

import static edu.unc.lib.boxc.auth.api.Permission.viewAccessCopies;
import static edu.unc.lib.boxc.auth.api.Permission.viewOriginal;
import static edu.unc.lib.boxc.model.api.DatastreamType.ORIGINAL_FILE;
import static edu.unc.lib.boxc.model.api.DatastreamType.TECHNICAL_METADATA;
Expand Down Expand Up @@ -137,6 +138,7 @@ private ContentObjectSolrRecord createImgObject(ResourceType resourceType) {
List<String> imgDatastreams = Arrays.asList(
ORIGINAL_FILE.getId() + "|image/png|file.png|png|766|urn:sha1:checksum|",
DatastreamType.THUMBNAIL_LARGE.getId() + "|image/png|thumb|png|55||",
DatastreamType.THUMBNAIL_SMALL.getId() + "|image/png|thumb|png|15||",
DatastreamType.JP2_ACCESS_COPY.getId() + "|image/jp2|thumb|jp2|555||");
mdObjectImg.setFileFormatCategory(Collections.singletonList(ContentCategory.image.getDisplayName()));
mdObjectImg.setFileFormatType(Collections.singletonList("image/png"));
Expand Down Expand Up @@ -236,6 +238,14 @@ public void primaryObjThumbnail() {
assertEquals(mdObjectImg.getId(), accessCopiesService.getThumbnailId(mdObjectImg, principals, true));
}

@Test
public void primaryObjThumbnailMetadataOnlyPermissions() {
hasPermissions(mdObjectImg, true, false);

assertNull(accessCopiesService.getThumbnailId(mdObjectImg, principals, false));
assertNull(accessCopiesService.getThumbnailId(mdObjectImg, principals, true));
}

@Test
public void noPrimaryObjThumbnailMultipleFiles() {
hasPermissions(noOriginalFileObj, true);
Expand All @@ -253,6 +263,21 @@ public void noPrimaryObjThumbnailMultipleFiles() {
assertSortType("default");
}

@Test
public void noPrimaryObjThumbnailMultipleFilesMetadataOnlyPermissions() {
hasPermissions(noOriginalFileObj, true, false);
hasPermissions(mdObjectXml, false);
hasPermissions(mdObjectImg, false);
noOriginalFileObj.setFileFormatCategory(Collections.singletonList(ContentCategory.image.getDisplayName()));
noOriginalFileObj.setFileFormatType(Collections.singletonList("png"));
populateResultList(mdObjectImg);
when(searchResultResponse.getResultCount()).thenReturn(2l);

assertNull(accessCopiesService.getThumbnailId(noOriginalFileObj, principals, false));
// Gets the ID of the specific child with a thumbnail
assertNull(accessCopiesService.getThumbnailId(noOriginalFileObj, principals, true));
}

@Test
public void noPrimaryObjNoThumbnail() {
hasPermissions(noOriginalFileObj, true);
Expand All @@ -273,7 +298,8 @@ public void getThumbnailIdNoPrimaryMultipleImages() {
mdObjectImg2.setId(UUID.randomUUID().toString());
var imgDatastreams = Arrays.asList(
ORIGINAL_FILE.getId() + "|image/jpg|file2.png|png|555|urn:sha1:checksum|",
DatastreamType.THUMBNAIL_LARGE.getId() + "|image/png|thumb|png|55||");
DatastreamType.THUMBNAIL_LARGE.getId() + "|image/png|thumb|png|55||",
DatastreamType.THUMBNAIL_SMALL.getId() + "|image/png|thumb|png|15||");
mdObjectImg2.setFileFormatCategory(Collections.singletonList(ContentCategory.image.getDisplayName()));
mdObjectImg2.setFileFormatType(Collections.singletonList("png"));
mdObjectImg2.setDatastream(imgDatastreams);
Expand All @@ -294,6 +320,33 @@ public void getThumbnailIdNoPrimaryMultipleImages() {
assertSortType("default");
}

@Test
public void getThumbnailIdNoPrimaryMultipleImagesMetadataOnlyPermissions() {
var mdObjectImg2 = new ContentObjectSolrRecord();
mdObjectImg2.setResourceType(ResourceType.File.name());
mdObjectImg2.setId(UUID.randomUUID().toString());
var imgDatastreams = Arrays.asList(
ORIGINAL_FILE.getId() + "|image/jpg|file2.png|png|555|urn:sha1:checksum|",
DatastreamType.THUMBNAIL_LARGE.getId() + "|image/png|thumb|png|55||",
DatastreamType.THUMBNAIL_SMALL.getId() + "|image/png|thumb|png|15||");
mdObjectImg2.setFileFormatCategory(Collections.singletonList(ContentCategory.image.getDisplayName()));
mdObjectImg2.setFileFormatType(Collections.singletonList("png"));
mdObjectImg2.setDatastream(imgDatastreams);

hasPermissions(noOriginalFileObj, true, false);
hasPermissions(mdObjectImg2, true, false);
hasPermissions(mdObjectImg, true, false);
noOriginalFileObj.setFileFormatCategory(Collections.singletonList(ContentCategory.image.getDisplayName()));
noOriginalFileObj.setFileFormatType(Collections.singletonList("png"));
populateResultList(mdObjectImg2);
when(searchResultResponse.getResultCount()).thenReturn(2l);

assertNull(accessCopiesService.getThumbnailId(noOriginalFileObj, principals, false));

// Gets the ID of the specific child with a thumbnail
assertNull(accessCopiesService.getThumbnailId(noOriginalFileObj, principals, true));
}

private void assertRequestedDatastreamFilter(DatastreamType expectedType) {
var searchState = searchRequestCaptor.getValue().getSearchState();
var queryFilter = (NamedDatastreamFilter) searchState.getFilters().get(0);
Expand Down Expand Up @@ -366,6 +419,15 @@ public void hasViewableFilesImageWorkTest() {

private void hasPermissions(ContentObjectSolrRecord contentObject, boolean hasAccess) {
when(accessControlService.hasAccess(contentObject.getPid(), principals, viewOriginal)).thenReturn(hasAccess);
when(accessControlService.hasAccess(contentObject.getPid(), principals, viewAccessCopies)).thenReturn(hasAccess);
}

private void hasPermissions(ContentObjectSolrRecord contentObject, boolean hasAccessOriginal,
boolean hasAccessThumb) {
when(accessControlService.hasAccess(contentObject.getPid(), principals, viewOriginal))
.thenReturn(hasAccessOriginal);
when(accessControlService.hasAccess(contentObject.getPid(), principals, viewAccessCopies))
.thenReturn(hasAccessThumb);
}

private void populateResultList(ContentObjectRecord... objects) {
Expand Down
20 changes: 19 additions & 1 deletion web-common/src/test/java/edu/unc/lib/boxc/web/common/services/PermissionsHelperTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
import static edu.unc.lib.boxc.auth.api.Permission.viewOriginal;
import static edu.unc.lib.boxc.model.api.DatastreamType.JP2_ACCESS_COPY;
import static edu.unc.lib.boxc.model.api.DatastreamType.ORIGINAL_FILE;
import static edu.unc.lib.boxc.model.api.DatastreamType.THUMBNAIL_LARGE;
import static edu.unc.lib.boxc.model.api.DatastreamType.THUMBNAIL_SMALL;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Matchers.any;
Expand Down Expand Up @@ -61,7 +63,9 @@ public void init() {
mdObject.setRoleGroup(roleGroups);
List<String> datastreams = Arrays.asList(
ORIGINAL_FILE.getId() + "|application/pdf|file.pdf|pdf|766|urn:sha1:checksum|",
JP2_ACCESS_COPY.getId() + "|application/jp2|file.jp2|jp2|884||");
JP2_ACCESS_COPY.getId() + "|application/jp2|file.jp2|jp2|884||",
THUMBNAIL_LARGE.getId() + "|image/png|thumb|png|55||",
THUMBNAIL_SMALL.getId() + "|image/png|thumb|png|15||");
mdObject.setDatastream(datastreams);

principals = new AccessGroupSetImpl("group");
Expand Down Expand Up @@ -110,6 +114,20 @@ public void testPermitDerivativeAccess() {
assertTrue(helper.hasDatastreamAccess(principals, JP2_ACCESS_COPY, mdObject));
}

@Test
public void testAllowsThumbnailAccess() {
assignPermission(viewAccessCopies, true);

assertTrue(helper.hasThumbnailAccess(principals, mdObject), "Thumbnail should have full patron access");
}

@Test
public void testDisAllowsThumbnailAccess() {
assignPermission(viewMetadata, true);

assertFalse(helper.hasThumbnailAccess(principals, mdObject), "Thumbnail must not have full patron access");
}

@Test
public void testDenyOriginalAccess() {
assignPermission(viewOriginal, false);
Expand Down
Loading