diff --git a/pom.xml b/pom.xml index b1f0b00a..7f3a0d8c 100644 --- a/pom.xml +++ b/pom.xml @@ -177,6 +177,13 @@ aws-java-sdk-core 1.12.129 + + + org.mockito + mockito-core + 4.1.0 + test + junit junit diff --git a/src/main/java/uk/org/llgc/annotation/store/controllers/AuthorisationController.java b/src/main/java/uk/org/llgc/annotation/store/controllers/AuthorisationController.java index 940b9c0e..f42b52dd 100644 --- a/src/main/java/uk/org/llgc/annotation/store/controllers/AuthorisationController.java +++ b/src/main/java/uk/org/llgc/annotation/store/controllers/AuthorisationController.java @@ -130,4 +130,12 @@ public boolean allowThrough(final HttpServletRequest pRequest) { public boolean deleteUser(final User pAdmin, final User pTarget) { return pAdmin.isAdmin(); } + + public boolean allowReadSomeoneElseAnnos(final User pAnnoOwner, final User pRequester) { + if (pAnnoOwner.getId().equals(pRequester.getId())) { + return true; + } else { + return pRequester.isAdmin(); + } + } } diff --git a/src/main/java/uk/org/llgc/annotation/store/controllers/StoreService.java b/src/main/java/uk/org/llgc/annotation/store/controllers/StoreService.java index 616021cd..de616ab2 100644 --- a/src/main/java/uk/org/llgc/annotation/store/controllers/StoreService.java +++ b/src/main/java/uk/org/llgc/annotation/store/controllers/StoreService.java @@ -13,6 +13,8 @@ import uk.org.llgc.annotation.store.data.users.User; import uk.org.llgc.annotation.store.adapters.StoreAdapter; import uk.org.llgc.annotation.store.StoreConfig; +import uk.org.llgc.annotation.store.exceptions.PermissionDenied; +import uk.org.llgc.annotation.store.controllers.AuthorisationController; import java.util.Base64; import java.util.zip.Deflater; @@ -59,26 +61,46 @@ public void init() { _store = StoreConfig.getConfig().getStore(); } - public List listAnnoPages(final String pURI) { - Manifest tManifest = new Manifest(); - tManifest.setURI(pURI); + protected User getCurrentUser() { + UserService tService = new UserService(); + return tService.getUser(); + } - try { - UserService tService = new UserService(); - return _store.listAnnoPages(tManifest, tService.getUser()); - } catch (IOException tExcpt) { - System.err.println("Failed to retrieve stats for " + pURI); - tExcpt.printStackTrace(); + protected AuthorisationController getAuth() { + return new AuthorisationController(); + } + + protected User getCurrentUser(final HttpServletRequest pRequest) { + UserService tService = new UserService(pRequest); + return tService.getUser(); + } + + public List listAnnoPages(final String pURI) throws PermissionDenied { + return listAnnoPages(pURI, this.getCurrentUser()); + } + + public List listAnnoPages(final String pURI, final User pUser) throws PermissionDenied { + if (this.getAuth().allowReadSomeoneElseAnnos(pUser, this.getCurrentUser())) { + Manifest tManifest = new Manifest(); + tManifest.setURI(pURI); + + try { + return _store.listAnnoPages(tManifest, pUser); + } catch (IOException tExcpt) { + System.err.println("Failed to retrieve stats for " + pURI); + tExcpt.printStackTrace(); + } + return new ArrayList(); + } else { + throw new PermissionDenied("User " + this.getCurrentUser().getName() + "(" + this.getCurrentUser().getId() + ") cannot access " + pUser.getName() + "(" + pUser.getId() + ") annotations as user is not an admin"); } - return new ArrayList(); } - public Map countAnnotations(final Manifest pManifest) { - UserService tService = new UserService(); - return countAnnotations(pManifest, tService.getUser()); + public Map countAnnotations(final Manifest pManifest) throws PermissionDenied { + return countAnnotations(pManifest, this.getCurrentUser()); } - public Map countAnnotations(final Manifest pManifest, final User pUser) { + public Map countAnnotations(final Manifest pManifest, final User pUser) throws PermissionDenied { String tKey = "stats_" + pManifest.getShortId(); HttpServletRequest tRequest = this.getRequest(); if (tRequest.getAttribute(tKey) != null) { @@ -88,22 +110,17 @@ public Map countAnnotations(final Manifest pManifest, final User Map tStats = new HashMap(); tStats.put("canvas_count", 0); tStats.put("total_annos", 0); - try { - List tCount = _store.listAnnoPages(pManifest, pUser); - tStats.put("canvas_count", tCount.size()); + List tCount = this.listAnnoPages(pManifest.getURI(), pUser); + tStats.put("canvas_count", tCount.size()); - int tTotalAnnos = 0; - for (PageAnnoCount tPageCount : tCount) { - tTotalAnnos += tPageCount.getCount(); - } - tStats.put("total_annos", tTotalAnnos); + int tTotalAnnos = 0; + for (PageAnnoCount tPageCount : tCount) { + tTotalAnnos += tPageCount.getCount(); + } + tStats.put("total_annos", tTotalAnnos); - if (tRequest.getAttribute(tKey) == null) { - tRequest.setAttribute(tKey, tStats); - } - } catch (IOException tExcpt) { - System.err.println("Failed to retrieve stats for " + pManifest.getURI()); - tExcpt.printStackTrace(); + if (tRequest.getAttribute(tKey) == null) { + tRequest.setAttribute(tKey, tStats); } return tStats; } @@ -119,8 +136,7 @@ protected HttpServletRequest getRequest() { public Manifest getEnhancedManifest(final String pManifestURI) throws IOException { Manifest tManifest = this.getManifestId(pManifestURI); - UserService tService = new UserService(); - return this.getEnhancedManifest(tService.getUser(), tManifest, false); + return this.getEnhancedManifest(this.getCurrentUser(), tManifest, false); } public Manifest getEnhancedManifest(final User pUser, final Manifest pManifest, final boolean regenerate) throws IOException { @@ -170,8 +186,7 @@ public List listAnnoPages(final Manifest pManifest) { } try { //new PageAnnoCount(final Canvas pCanvas, final int pCount, final Manifest pManifest) - UserService tService = new UserService(); - List tAnnosCount = _store.listAnnoPages(pManifest, tService.getUser()); + List tAnnosCount = _store.listAnnoPages(pManifest, this.getCurrentUser()); List tFullCanvasList = new ArrayList(); for (Canvas tCanvas : pManifest.getCanvases()) { PageAnnoCount tCanvasCount = new PageAnnoCount(tCanvas, 0, pManifest); @@ -227,8 +242,7 @@ public List getManifests() { public List getAnnoManifests() { try { - UserService tService = new UserService(); - return _store.getSkeletonManifests(tService.getUser()); + return _store.getSkeletonManifests(this.getCurrentUser()); } catch (IOException tExcpt) { return new ArrayList(); } @@ -244,8 +258,7 @@ public Manifest getManifestFromCanvas(final String pCanvasURI) { } public AnnotationList getAnnotations(final String pCanvasURI) { - UserService tUserService = new UserService(); - return this.getAnnotations(pCanvasURI, tUserService.getUser()); + return this.getAnnotations(pCanvasURI, this.getCurrentUser()); } public AnnotationList getAnnotations(final String pCanvasURI, final User pUser) { @@ -304,9 +317,8 @@ public Manifest getManifestId(final String pURI) { } public List getCollections(final HttpServletRequest pRequest) throws IOException { - UserService tService = new UserService(pRequest); - User tUser = tService.getUser(); - String tKey = "get_collections_from_request_" + tService.getUser().getShortId(); + User tUser = this.getCurrentUser(pRequest); + String tKey = "get_collections_from_request_" + tUser.getShortId(); if (this.isCached(tKey)) { return (List)this.getCacheObject(tKey); } @@ -352,8 +364,7 @@ public List getCollections(final User pUser) throws IOException { List tCollections = new ArrayList(); - UserService tService = new UserService(); - User tUser = tService.getUser(); + User tUser = this.getCurrentUser(); if (tUser.isAdmin()) { tCollections = _store.getCollections(pUser); @@ -366,8 +377,7 @@ public List getCollections(final User pUser) throws IOException { public Collection getCollection(final String pID, final HttpServletRequest pRequest) throws IOException { String tCollectionKey = "collection_"; if (pID == null || pID.length() == 0) { - UserService tService = new UserService(pRequest); - User tUser = tService.getUser(); + User tUser = this.getCurrentUser(pRequest); List tCollections = this.getCollections(pRequest); for (Collection tCollection : tCollections) { diff --git a/src/main/java/uk/org/llgc/annotation/store/exceptions/PermissionDenied.java b/src/main/java/uk/org/llgc/annotation/store/exceptions/PermissionDenied.java new file mode 100644 index 00000000..7ad79cd1 --- /dev/null +++ b/src/main/java/uk/org/llgc/annotation/store/exceptions/PermissionDenied.java @@ -0,0 +1,13 @@ +package uk.org.llgc.annotation.store.exceptions; + +public class PermissionDenied extends Exception { + public PermissionDenied() { + super(); + } + + public PermissionDenied(final String pMessage) { + super(pMessage); + } +} + + diff --git a/src/main/webapp/admin/canvases.xhtml b/src/main/webapp/admin/canvases.xhtml index 7d9b0e6b..d41ac53c 100644 --- a/src/main/webapp/admin/canvases.xhtml +++ b/src/main/webapp/admin/canvases.xhtml @@ -16,7 +16,7 @@

Annotated Canvases for user:

    - +
  • @@ -47,7 +47,7 @@ diff --git a/src/test/java/uk/org/llgc/annotation/store/test/TestUsers.java b/src/test/java/uk/org/llgc/annotation/store/test/TestUsers.java index cdb5e747..b956384e 100644 --- a/src/test/java/uk/org/llgc/annotation/store/test/TestUsers.java +++ b/src/test/java/uk/org/llgc/annotation/store/test/TestUsers.java @@ -22,14 +22,19 @@ import uk.org.llgc.annotation.store.StoreConfig; import uk.org.llgc.annotation.store.exceptions.IDConflictException; import uk.org.llgc.annotation.store.exceptions.MalformedAnnotation; +import uk.org.llgc.annotation.store.exceptions.PermissionDenied; import uk.org.llgc.annotation.store.data.Manifest; import uk.org.llgc.annotation.store.data.Annotation; import uk.org.llgc.annotation.store.data.AnnotationList; import uk.org.llgc.annotation.store.data.SearchQuery; import uk.org.llgc.annotation.store.data.Canvas; +import uk.org.llgc.annotation.store.data.PageAnnoCount; import uk.org.llgc.annotation.store.data.users.User; import uk.org.llgc.annotation.store.data.users.LocalUser; import uk.org.llgc.annotation.store.controllers.AuthorisationController; +import uk.org.llgc.annotation.store.controllers.StoreService; +import uk.org.llgc.annotation.store.test.mocks.ControllerMocks; + import com.github.jsonldjava.utils.JsonUtils; @@ -543,4 +548,56 @@ public void testDeleteUser() throws IOException, URISyntaxException { assertNull("Found user but expected it to be deleted.", tFoundUser); } + @Test + public void testAuthLevelListAnnos() throws IOException, IDConflictException, MalformedAnnotation, URISyntaxException, PermissionDenied { + // test listAnnoPages with non admin user but retrieving annos of someone else. + // Should be refused + User tUser = new User(); + tUser.setId("http://example.com/user1"); + tUser.setShortId("user1"); + tUser.setName("Glen"); + tUser.setEmail("glen@glen.com"); + tUser.setAuthenticationMethod("test"); + tUser.setAdmin(false); + tUser.setPicture("http://picture.net"); + _store.saveUser(tUser); + + Map tAnnotation = (Map)JsonUtils.fromInputStream(new FileInputStream(getClass().getResource("/jsonld/testManifestWithin.json").getFile())); + Annotation tAnno = new Annotation(tAnnotation); + tAnno.setCreator(tUser); + + Annotation tStoredAnno = _store.addAnnotation(tAnno); + + User tNormalUser = new User(); + tNormalUser.setId("http://example.com/normal"); + tNormalUser.setShortId("normal"); + tNormalUser.setAuthenticationMethod("test"); + tNormalUser.setAdmin(false); + _store.saveUser(tNormalUser); + + + StoreService storeService = ControllerMocks.getStoreServiceWithUser(tUser); + List tAnnos = storeService.listAnnoPages("http://example.com/manfiest/test/manifest.json", tUser); + + assertEquals("With same user there should be 1 result. ", 1, tAnnos.size()); + + Exception exception = assertThrows(PermissionDenied.class, () -> { + final StoreService tNormalUserService = ControllerMocks.getStoreServiceWithUser(tNormalUser); + tNormalUserService.listAnnoPages("http://example.com/manfiest/test/manifest.json", tUser); + }); + + User tAdmin = new User(); + tAdmin.setId("http://example.com/admin"); + tAdmin.setShortId("admin"); + tAdmin.setAuthenticationMethod("test"); + tAdmin.setAdmin(true); + _store.saveUser(tAdmin); + + storeService = ControllerMocks.getStoreServiceWithUser(tAdmin); + tAnnos = storeService.listAnnoPages("http://example.com/manfiest/test/manifest.json", tUser); + + assertEquals("Admin should be able to see the 1 result. ", 1, tAnnos.size()); + + } + } diff --git a/src/test/java/uk/org/llgc/annotation/store/test/mocks/ControllerMocks.java b/src/test/java/uk/org/llgc/annotation/store/test/mocks/ControllerMocks.java new file mode 100644 index 00000000..06724143 --- /dev/null +++ b/src/test/java/uk/org/llgc/annotation/store/test/mocks/ControllerMocks.java @@ -0,0 +1,18 @@ +package uk.org.llgc.annotation.store.test.mocks; + +import uk.org.llgc.annotation.store.data.users.User; +import uk.org.llgc.annotation.store.controllers.StoreService; + +public class ControllerMocks { + + public static StoreService getStoreServiceWithUser(final User pUser) { + StoreService tService = new StoreService() { + public User getCurrentUser() { + return pUser; + } + }; + tService.init(); + + return tService; + } +} diff --git a/src/test/java/uk/org/llgc/annotation/store/test/mocks/HttpServletMocks.java b/src/test/java/uk/org/llgc/annotation/store/test/mocks/HttpServletMocks.java new file mode 100644 index 00000000..4e77a237 --- /dev/null +++ b/src/test/java/uk/org/llgc/annotation/store/test/mocks/HttpServletMocks.java @@ -0,0 +1,142 @@ +package uk.org.llgc.annotation.store.test.mocks; + +import javax.servlet.http.HttpSession; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import static org.mockito.Mockito.*; +import org.mockito.stubbing.Answer; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.Mockito; + +import com.github.scribejava.core.model.OAuth2AccessToken; + +import java.util.Map; +import java.util.HashMap; + +import java.io.StringWriter; +import java.io.PrintWriter; +import java.io.IOException; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class HttpServletMocks { + static final Logger _logger = LoggerFactory.getLogger(HttpServletMocks.class); + + public static HttpSession createSession(final Map pItems) { + HttpSession tSession = mock(HttpSession.class); + + when(tSession.getAttribute(anyString())).thenAnswer(new Answer() { + /** + * @see org.mockito.stubbing.Answer#answer(org.mockito.invocation.InvocationOnMock) + */ + @Override + public Object answer(InvocationOnMock aInvocation) throws Throwable { + String key = (String) aInvocation.getArguments()[0]; + return pItems.get(key); + } + }); + Mockito.doAnswer(new Answer() { + /** + * @see org.mockito.stubbing.Answer#answer(org.mockito.invocation.InvocationOnMock) + */ + @Override + public Object answer(InvocationOnMock aInvocation) throws Throwable { + String key = (String) aInvocation.getArguments()[0]; + Object value = aInvocation.getArguments()[1]; + pItems.put(key, value); + return null; + } + }).when(tSession).setAttribute(anyString(), any()); + + return tSession; + } + + public static HttpServletRequest createRequest(final String pRequestURI, final String pMethod, final Map pParameters, final HttpSession pSession) { + HttpServletRequest tReq = mock(HttpServletRequest.class); + + _logger.debug("Request URI: {} ", pRequestURI); + + when(tReq.getParameter(anyString())).thenAnswer(new Answer() { + /** + * @see org.mockito.stubbing.Answer#answer(org.mockito.invocation.InvocationOnMock) + */ + @Override + public Object answer(InvocationOnMock aInvocation) throws Throwable { + String key = (String) aInvocation.getArguments()[0]; + return pParameters.get(key); + } + }); + + when(tReq.getMethod()).thenReturn(pMethod); + when(tReq.getRequestURI()).thenReturn(pRequestURI); + when(tReq.getSession()).thenReturn(pSession); + + return tReq; + } + + public static HttpServletResponse createResponse(final StringWriter pWriter) throws IOException { + HttpServletResponse tResponse = mock(HttpServletResponse.class); + + PrintWriter writer = new PrintWriter(pWriter); + when(tResponse.getWriter()).thenReturn(writer); + + Map tValues = new HashMap(); + when(tResponse.getStatus()).thenAnswer(new Answer() { + /** + * @see org.mockito.stubbing.Answer#answer(org.mockito.invocation.InvocationOnMock) + */ + @Override + public Object answer(InvocationOnMock aInvocation) throws Throwable { + return (int)tValues.get("status"); + } + }); + + Mockito.doAnswer(new Answer() { + @Override + public Object answer(InvocationOnMock aInvocation) throws Throwable { + Object value = aInvocation.getArguments()[0]; + tValues.put("status", value); + return null; + } + }).when(tResponse).setStatus(anyInt()); + when(tResponse.getContentType()).thenAnswer(new Answer() { + /** + * @see org.mockito.stubbing.Answer#answer(org.mockito.invocation.InvocationOnMock) + */ + @Override + public Object answer(InvocationOnMock aInvocation) throws Throwable { + return (String)tValues.get("mime"); + } + }); + Mockito.doAnswer(new Answer() { + @Override + public Object answer(InvocationOnMock aInvocation) throws Throwable { + String value = (String) aInvocation.getArguments()[0]; + tValues.put("mime", value); + return null; + } + }).when(tResponse).setContentType(anyString()); + + when(tResponse.getCharacterEncoding()).thenAnswer(new Answer() { + /** + * @see org.mockito.stubbing.Answer#answer(org.mockito.invocation.InvocationOnMock) + */ + @Override + public Object answer(InvocationOnMock aInvocation) throws Throwable { + return (String)tValues.get("encoding"); + } + }); + Mockito.doAnswer(new Answer() { + @Override + public Object answer(InvocationOnMock aInvocation) throws Throwable { + String value = (String) aInvocation.getArguments()[0]; + tValues.put("encoding", value); + return null; + } + }).when(tResponse).setCharacterEncoding(anyString()); + + return tResponse; + } +}