From 61fc935fc94c5d3f5a605f976cdb43b43763df7d Mon Sep 17 00:00:00 2001 From: Sharon Luong Date: Tue, 12 Dec 2023 15:04:15 -0500 Subject: [PATCH] BXC-4375 update access controls, URI string building, and make sure connections close --- .../processing/ImageServerProxyService.java | 30 ++++---- .../rest/ImageServerProxyController.java | 71 +++++++------------ .../rest/ImageServerProxyControllerTest.java | 16 +++-- 3 files changed, 50 insertions(+), 67 deletions(-) diff --git a/web-services-app/src/main/java/edu/unc/lib/boxc/web/services/processing/ImageServerProxyService.java b/web-services-app/src/main/java/edu/unc/lib/boxc/web/services/processing/ImageServerProxyService.java index 243aedab0c..3bb0d4127e 100644 --- a/web-services-app/src/main/java/edu/unc/lib/boxc/web/services/processing/ImageServerProxyService.java +++ b/web-services-app/src/main/java/edu/unc/lib/boxc/web/services/processing/ImageServerProxyService.java @@ -4,7 +4,6 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.node.ObjectNode; import edu.unc.lib.boxc.common.util.URIUtil; -import edu.unc.lib.boxc.web.services.utils.ImageServerUtil; import org.apache.http.HttpStatus; import org.apache.http.client.config.RequestConfig; import org.apache.http.client.methods.CloseableHttpResponse; @@ -23,6 +22,8 @@ import java.io.InputStream; import java.net.URL; +import static edu.unc.lib.boxc.web.services.utils.ImageServerUtil.getImageServerEncodedId; + /** * Generates request, connects to, and streams the output from the image Server Proxy. * @author bbpennel, snluong @@ -50,16 +51,19 @@ public void setHttpClientConnectionManager(HttpClientConnectionManager manager) * @param id ID of the requested object */ public JsonNode getMetadata(String id) throws IOException { - HttpGet method = new HttpGet(getImageServerProxyBasePath() + ImageServerUtil.getImageServerEncodedId(id) + "/info.json"); - CloseableHttpResponse httpResp = httpClient.execute(method); - var statusCode = httpResp.getStatusLine().getStatusCode(); - if (statusCode == HttpStatus.SC_OK) { - var mapper = new ObjectMapper(); - var respData = mapper.readTree(httpResp.getEntity().getContent()); - ((ObjectNode) respData).put("id", URIUtil.join(baseIiifv3Path, id)); - return respData; + var url = URIUtil.join(getImageServerProxyBasePath(), getImageServerEncodedId(id), "/info.json"); + HttpGet method = new HttpGet(url); + try (CloseableHttpResponse httpResp = httpClient.execute(method)) { + var statusCode = httpResp.getStatusLine().getStatusCode(); + if (statusCode == HttpStatus.SC_OK) { + var mapper = new ObjectMapper(); + var respData = mapper.readTree(httpResp.getEntity().getContent()); + ((ObjectNode) respData).put("id", URIUtil.join(baseIiifv3Path, id)); + return respData; + } + } finally { + method.releaseConnection(); } - method.releaseConnection(); LOG.error("Unexpected failure while getting image server proxy path {}", method); return null; @@ -75,12 +79,12 @@ public JsonNode getMetadata(String id) throws IOException { * @param quality quality of image * @param format format like png or jpg */ - public ResponseEntity streamJP2(String id, String region, String size, String rotation, String quality, String format) throws IOException { - String path = getImageServerProxyBasePath() + ImageServerUtil.getImageServerEncodedId(id) + - "/" + region + "/" + size + "/" + rotation + "/" + quality + "." + format; + String path = URIUtil.join(getImageServerProxyBasePath(), getImageServerEncodedId(id), + region, size, rotation, quality); + path += "." + format; InputStream input = new URL(path).openStream(); InputStreamResource resource = new InputStreamResource(input); diff --git a/web-services-app/src/main/java/edu/unc/lib/boxc/web/services/rest/ImageServerProxyController.java b/web-services-app/src/main/java/edu/unc/lib/boxc/web/services/rest/ImageServerProxyController.java index de452a258a..d5f0ac8638 100644 --- a/web-services-app/src/main/java/edu/unc/lib/boxc/web/services/rest/ImageServerProxyController.java +++ b/web-services-app/src/main/java/edu/unc/lib/boxc/web/services/rest/ImageServerProxyController.java @@ -1,11 +1,8 @@ package edu.unc.lib.boxc.web.services.rest; import edu.unc.lib.boxc.auth.api.Permission; -import edu.unc.lib.boxc.auth.api.models.AgentPrincipals; +import edu.unc.lib.boxc.auth.api.models.AccessGroupSet; import edu.unc.lib.boxc.auth.api.services.AccessControlService; -import edu.unc.lib.boxc.auth.api.services.DatastreamPermissionUtil; -import edu.unc.lib.boxc.auth.fcrepo.models.AgentPrincipalsImpl; -import edu.unc.lib.boxc.auth.fcrepo.services.GroupsThreadStore; import edu.unc.lib.boxc.model.api.ids.PID; import edu.unc.lib.boxc.model.fcrepo.ids.PIDs; import edu.unc.lib.boxc.web.services.processing.ImageServerProxyService; @@ -21,7 +18,7 @@ import org.springframework.web.bind.annotation.CrossOrigin; import java.io.IOException; -import static edu.unc.lib.boxc.model.api.DatastreamType.JP2_ACCESS_COPY; +import static edu.unc.lib.boxc.auth.fcrepo.services.GroupsThreadStore.getAgentPrincipals; import static org.springframework.http.MediaType.APPLICATION_JSON_VALUE; /** @@ -31,29 +28,11 @@ @Controller public class ImageServerProxyController { private static final Logger LOG = LoggerFactory.getLogger(ImageServerProxyController.class); - @Autowired private ImageServerProxyService imageServerProxyService; - @Autowired private AccessControlService accessControlService; - /** - * Determines if the user is allowed to access a JP2 datastream on the selected object. - * @param pid PID of the object - * @return true if user is allowed, false if not - */ - private boolean hasAccess(PID pid) { - var datastream = JP2_ACCESS_COPY.getId(); - - Permission permission = DatastreamPermissionUtil.getPermissionForDatastream(datastream); - - AgentPrincipals agent = AgentPrincipalsImpl.createFromThread(); - LOG.debug("Checking if user {} has access to {} belonging to object {}.", - agent.getUsername(), datastream, pid); - return accessControlService.hasAccess(pid, agent.getPrincipals(), permission); - } - /** * Handles requests for individual region tiles. * @param id @@ -71,19 +50,18 @@ public ResponseEntity getRegion(@PathVariable("id") String PID pid = PIDs.get(id); // Check if the user is allowed to view this object - if (this.hasAccess(pid)) { - try { - String[] qualityFormatArray = qualityFormat.split("\\."); - String quality = qualityFormatArray[0]; - String format = qualityFormatArray[1]; - return imageServerProxyService.streamJP2(id, region, size, rotation, quality, format); - } catch (IOException e) { - LOG.error("Error retrieving streaming JP2 content for {}", id, e); - return new ResponseEntity<>(HttpStatus.INTERNAL_SERVER_ERROR); - } - } else { - LOG.debug("Access was forbidden to {} for user {}", id, GroupsThreadStore.getUsername()); - return new ResponseEntity<>(HttpStatus.FORBIDDEN); + AccessGroupSet principals = getAgentPrincipals().getPrincipals(); + accessControlService.assertHasAccess("Insufficient permissions to metadata for " + id, + pid, principals, Permission.viewAccessCopies); + + try { + String[] qualityFormatArray = qualityFormat.split("\\."); + String quality = qualityFormatArray[0]; + String format = qualityFormatArray[1]; + return imageServerProxyService.streamJP2(id, region, size, rotation, quality, format); + } catch (IOException e) { + LOG.error("Error retrieving streaming JP2 content for {}", id, e); + return new ResponseEntity<>(HttpStatus.INTERNAL_SERVER_ERROR); } } @@ -96,17 +74,16 @@ public ResponseEntity getRegion(@PathVariable("id") String public ResponseEntity getMetadata(@PathVariable("id") String id) { PID pid = PIDs.get(id); // Check if the user is allowed to view this object - if (this.hasAccess(pid)) { - try { - var metadata = imageServerProxyService.getMetadata(id); - return new ResponseEntity<>(metadata, HttpStatus.OK); - } catch (IOException e) { - LOG.error("Error retrieving JP2 metadata content for {}", id, e); - return new ResponseEntity<>(HttpStatus.INTERNAL_SERVER_ERROR); - } - } else { - LOG.debug("Access was forbidden to {} for user {}", id, GroupsThreadStore.getUsername()); - return new ResponseEntity<>(HttpStatus.FORBIDDEN); + AccessGroupSet principals = getAgentPrincipals().getPrincipals(); + accessControlService.assertHasAccess("Insufficient permissions to metadata for " + id, + pid, principals, Permission.viewAccessCopies); + + try { + var metadata = imageServerProxyService.getMetadata(id); + return new ResponseEntity<>(metadata, HttpStatus.OK); + } catch (IOException e) { + LOG.error("Error retrieving JP2 metadata content for {}", id, e); + return new ResponseEntity<>(HttpStatus.INTERNAL_SERVER_ERROR); } } } diff --git a/web-services-app/src/test/java/edu/unc/lib/boxc/web/services/rest/ImageServerProxyControllerTest.java b/web-services-app/src/test/java/edu/unc/lib/boxc/web/services/rest/ImageServerProxyControllerTest.java index f0ec4a652b..34c6ddc67f 100644 --- a/web-services-app/src/test/java/edu/unc/lib/boxc/web/services/rest/ImageServerProxyControllerTest.java +++ b/web-services-app/src/test/java/edu/unc/lib/boxc/web/services/rest/ImageServerProxyControllerTest.java @@ -2,7 +2,9 @@ import com.github.tomakehurst.wiremock.client.WireMock; import com.github.tomakehurst.wiremock.junit5.WireMockTest; +import edu.unc.lib.boxc.auth.api.exceptions.AccessRestrictionException; import edu.unc.lib.boxc.auth.api.services.AccessControlService; +import edu.unc.lib.boxc.auth.fcrepo.models.AccessGroupSetImpl; import edu.unc.lib.boxc.web.services.processing.ImageServerProxyService; import edu.unc.lib.boxc.web.services.rest.modify.AbstractAPIIT; import edu.unc.lib.boxc.web.services.utils.ImageServerUtil; @@ -24,9 +26,11 @@ import static com.github.tomakehurst.wiremock.client.WireMock.aResponse; import static com.github.tomakehurst.wiremock.client.WireMock.stubFor; import static com.github.tomakehurst.wiremock.client.WireMock.urlMatching; +import static edu.unc.lib.boxc.auth.api.Permission.viewAccessCopies; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doThrow; -import static org.mockito.Mockito.when; import static org.mockito.MockitoAnnotations.openMocks; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @@ -60,7 +64,8 @@ void closeService() throws Exception { void testGetRegionNoAccess() throws Exception { var pid = makePid(); var pidString = pid.getId(); - when(accessControlService.hasAccess(any(), any(), any())).thenReturn(false); + doThrow(new AccessRestrictionException()).when(accessControlService) + .assertHasAccess(anyString(), eq(pid), any(AccessGroupSetImpl.class), eq(viewAccessCopies)); mvc.perform(get("/iiif/v3/" + pidString + "/full/max/0/default.jpg")) .andExpect(status().isForbidden()) @@ -73,7 +78,6 @@ void testGetRegionSuccess() throws Exception { var pidString = pid.getId(); var formattedBasePath = "/iiif/v3/" + ImageServerUtil.getImageServerEncodedId(pidString); var filename = "bunny.jpg"; - when(accessControlService.hasAccess(any(), any(), any())).thenReturn(true); stubFor(WireMock.get(urlMatching(formattedBasePath + "/full/max/0/default.jpg")) .willReturn(aResponse() .withStatus(HttpStatus.OK.value()) @@ -92,7 +96,6 @@ void testGetRegionSuccess() throws Exception { void testGetRegionIOException() throws Exception { var pid = makePid(); var pidString = pid.getId(); - when(accessControlService.hasAccess(any(), any(), any())).thenReturn(true); doThrow(new IOException()).when(imageServerProxyService) .streamJP2(pidString, "full", "max", "0", "default", "jpg"); @@ -105,7 +108,8 @@ void testGetRegionIOException() throws Exception { void testGetMetadataNoAccess() throws Exception { var pid = makePid(); var pidString = pid.getId(); - when(accessControlService.hasAccess(any(), any(), any())).thenReturn(false); + doThrow(new AccessRestrictionException()).when(accessControlService) + .assertHasAccess(anyString(), eq(pid), any(AccessGroupSetImpl.class), eq(viewAccessCopies)); mvc.perform(get("/iiif/v3/" + pidString + "/info.json")) .andExpect(status().isForbidden()) @@ -119,7 +123,6 @@ void testGetMetadataSuccess() throws Exception { var formattedBasePath = "/iiif/v3/" + ImageServerUtil.getImageServerEncodedId(pidString); var json = "{\"@context\":\"http://iiif.io/api/image/3/context.json\",\"id\":\"http://example.com/iiif/v3/" + pidString + "\",\"type\":\"ImageService3\",\"protocol\":\"http://iiif.io/api/image\"}"; - when(accessControlService.hasAccess(any(), any(), any())).thenReturn(true); stubFor(WireMock.get(urlMatching(formattedBasePath + "/info.json")) .willReturn(aResponse() .withStatus(HttpStatus.OK.value()) @@ -139,7 +142,6 @@ void testGetMetadataIOException() throws Exception { var pid = makePid(); var pidString = pid.getId(); var formattedBasePath = "/iiif/v3/" + ImageServerUtil.getImageServerEncodedId(pidString); - when(accessControlService.hasAccess(any(), any(), any())).thenReturn(true); doThrow(new IOException()).when(imageServerProxyService).getMetadata(pidString); stubFor(WireMock.get(urlMatching(formattedBasePath + "/info.json"))