accessing removing data overlays didn't work for normal users

CHANGELOG

@@ -4,6 +4,8 @@ minerva (14.0.0~beta.0) unstable; urgency=low
   * Bug fix: user without admin right can accept terms of service (#893)
   * Bug fix: user without admin or curator privileges shouldn't be able to 
     check logs (#894)
+  * Bug fix: user without admin or curator privileges had issues with accesing
+    and removing data overlays (#897, #898, #899, #903)
 
  -- Piotr Gawron <piotr.gawron@uni.lu>  Mon, 21 Aug 2019 21:00:00 +0200
 

rest-api/src/main/java/lcsb/mapviewer/api/projects/overlays/OverlayController.java

@@ -60,7 +60,7 @@ public class OverlayController extends BaseController {
   @PreAuthorize("hasAuthority('IS_ADMIN')" +
       " or hasAuthority('IS_CURATOR') and hasAuthority('READ_PROJECT:' + #projectId)" +
       " or hasAuthority('READ_PROJECT:' + #projectId) and " +
-      "   (@layoutService.getLayoutById(#overlayId)?.creator?.login == authentication.name or @layoutService.getLayoutById(#overlayId)?.publicLayout == true)")
+      "   (@layoutService.getOverlayCreator(#overlayId)?.login == authentication.name or @layoutService.getLayoutById(#overlayId)?.publicLayout == true)")
   @GetMapping(value = "/{overlayId}/models/{modelId}/bioEntities/")
   public List<Map<String, Object>> getOverlayElements(
       @PathVariable(value = "projectId") String projectId,
@@ -72,7 +72,7 @@ public class OverlayController extends BaseController {
   @PreAuthorize("hasAuthority('IS_ADMIN')" +
       " or hasAuthority('IS_CURATOR') and hasAuthority('READ_PROJECT:' + #projectId)" +
       " or hasAuthority('READ_PROJECT:' + #projectId) and " +
-      "   (@layoutService.getLayoutById(#overlayId)?.creator?.login == authentication.name or @layoutService.getLayoutById(#overlayId)?.publicLayout == true)")
+      "   (@layoutService.getOverlayCreator(#overlayId)?.login == authentication.name or @layoutService.getLayoutById(#overlayId)?.publicLayout == true)")
   @GetMapping(value = "/{overlayId}/models/{modelId}/bioEntities/reactions/{reactionId}/")
   public Map<String, Object> getFullReaction(
       @PathVariable(value = "projectId") String projectId,
@@ -88,7 +88,7 @@ public class OverlayController extends BaseController {
   @PreAuthorize("hasAuthority('IS_ADMIN')" +
       " or hasAuthority('IS_CURATOR') and hasAuthority('READ_PROJECT:' + #projectId)" +
       " or hasAuthority('READ_PROJECT:' + #projectId) and " +
-      "   (@layoutService.getLayoutById(#overlayId)?.creator?.login == authentication.name or @layoutService.getLayoutById(#overlayId)?.publicLayout == true)")
+      "   (@layoutService.getOverlayCreator(#overlayId)?.login == authentication.name or @layoutService.getLayoutById(#overlayId)?.publicLayout == true)")
   @GetMapping(value = "/{overlayId}/models/{modelId}/bioEntities/elements/{elementId}/")
   public Map<String, Object> getFullSpecies(
       @PathVariable(value = "projectId") String projectId,
@@ -122,7 +122,7 @@ public class OverlayController extends BaseController {
 
   @PreAuthorize("hasAuthority('IS_ADMIN')" +
       "or hasAuthority('IS_CURATOR') and hasAuthority('WRITE_PROJECT:' + #projectId)" +
-      "or hasAuthority('READ_PROJECT:' + #projectId) and @layoutService.getLayoutById(#overlayId)?.creator?.login == authentication.name")
+      "or hasAuthority('READ_PROJECT:' + #projectId) and @layoutService.getOverlayCreator(#overlayId)?.login == authentication.name")
   @DeleteMapping(value = "/{overlayId}")
   public Map<String, Object> removeOverlay(
       @PathVariable(value = "projectId") String projectId,
@@ -132,7 +132,7 @@ public class OverlayController extends BaseController {
 
   @PreAuthorize("hasAuthority('IS_ADMIN')" +
       " or hasAuthority('IS_CURATOR') and hasAuthority('WRITE_PROJECT:' + #projectId)" +
-      " or hasAuthority('READ_PROJECT:' + #projectId) and @layoutService.getLayoutById(#overlayId)?.creator?.login == authentication.name")
+      " or hasAuthority('READ_PROJECT:' + #projectId) and @layoutService.getOverlayCreator(#overlayId)?.login == authentication.name")
   @PatchMapping(value = "/{overlayId}")
   public Map<String, Object> updateOverlay(
       @RequestBody String body,
@@ -147,7 +147,7 @@ public class OverlayController extends BaseController {
   @PreAuthorize("hasAuthority('IS_ADMIN')" +
       " or hasAuthority('IS_CURATOR') and hasAuthority('READ_PROJECT:' + #projectId)" +
       " or hasAuthority('READ_PROJECT:' + #projectId) and " +
-      "   (@layoutService.getLayoutById(#overlayId)?.creator?.login == authentication.name or @layoutService.getLayoutById(#overlayId)?.publicLayout == true)")
+      "   (@layoutService.getOverlayCreator(#overlayId)?.login == authentication.name or @layoutService.getLayoutById(#overlayId)?.publicLayout == true)")
   @GetMapping(value = "/{overlayId}:downloadSource")
   public ResponseEntity<byte[]> getOverlaySource(
       @PathVariable(value = "projectId") String projectId,

service/src/main/java/lcsb/mapviewer/services/impl/LayoutService.java

@@ -9,6 +9,7 @@ import java.util.List;
 import javax.mail.MessagingException;
 
 import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang3.math.NumberUtils;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -805,4 +806,20 @@ public class LayoutService implements ILayoutService {
     Layout overlay = getLayoutById(overlayId);
     return reader.readColorSchema(getInputDataForLayout(overlayId), overlay.getColorSchemaType());
   }
+
+  @Override
+  public User getOverlayCreator(String overlayId) {
+    User result = null;
+    if (NumberUtils.isCreatable(overlayId)) {
+      Layout overlay = layoutDao.getById(Integer.valueOf(overlayId));
+      if (overlay != null) {
+        result = overlay.getCreator();
+      }
+    }
+    if (result != null) {
+      // fetch lazy initialized object
+      result.getLogin();
+    }
+    return result;
+  }
 }

service/src/main/java/lcsb/mapviewer/services/interfaces/ILayoutService.java

@@ -143,6 +143,8 @@ public interface ILayoutService {
   List<Layout> getLayoutsByProject(Project project);
 
   Layout getLayoutById(Integer overlayId);
+  
+  User getOverlayCreator(String overlayId);
 
   void setLayoutDao(LayoutDao layoutDao);
 

web/src/test/java/lcsb/mapviewer/web/ControllerIntegrationTest.java

@@ -31,6 +31,7 @@ import lcsb.mapviewer.model.Project;
 import lcsb.mapviewer.model.ProjectStatus;
 import lcsb.mapviewer.model.cache.UploadedFileEntry;
 import lcsb.mapviewer.model.graphics.PolylineData;
+import lcsb.mapviewer.model.map.layout.Layout;
 import lcsb.mapviewer.model.map.model.ModelData;
 import lcsb.mapviewer.model.map.reaction.*;
 import lcsb.mapviewer.model.map.reaction.type.TransportReaction;
@@ -41,6 +42,7 @@ import lcsb.mapviewer.model.user.User;
 import lcsb.mapviewer.persist.DbUtils;
 import lcsb.mapviewer.persist.dao.ProjectDao;
 import lcsb.mapviewer.persist.dao.cache.UploadedFileEntryDao;
+import lcsb.mapviewer.persist.dao.map.LayoutDao;
 import lcsb.mapviewer.persist.dao.user.UserDao;
 import lcsb.mapviewer.services.interfaces.IUserService;
 import lcsb.mapviewer.web.config.SpringWebConfig;
@@ -66,6 +68,10 @@ abstract public class ControllerIntegrationTest {
   private PasswordEncoder passwordEncoder;
   @Autowired
   private UploadedFileEntryDao fileDao;
+
+  @Autowired
+  private LayoutDao layoutDao;
+
   @Autowired
   private DbUtils dbUtils;
   private MinervaLoggerAppender appender;
@@ -264,4 +270,41 @@ abstract public class ControllerIntegrationTest {
     });
   }
 
+  protected User createUserInSeparateThread(String login, String password) throws Exception {
+    return callInSeparateThread(() -> {
+      return createUser(login, password);
+    });
+  }
+
+  protected Layout createOverlay(Project project, User admin) {
+    return createOverlay(project, admin, "elementIdentifier\tvalue\n\t-1");
+  }
+
+  protected Layout createOverlay(Project project, User admin, String content) {
+    UploadedFileEntry file = new UploadedFileEntry();
+    file.setFileContent(content.getBytes());
+    file.setLength(content.getBytes().length);
+
+    Layout overlay = new Layout();
+    overlay.setInputData(file);
+    overlay.setProject(project);
+    overlay.setCreator(admin);
+    layoutDao.add(overlay);
+    return overlay;
+  }
+
+  protected Layout createOverlayInSeparateThread(String projectId, User user) throws Exception {
+    return callInSeparateThread(() -> {
+      Project project = projectDao.getProjectByProjectId(projectId);
+      return createOverlay(project, user);
+    });
+  }
+
+  protected void removeOverlayInSeparateThread(Layout layout) throws Exception {
+    callInSeparateThread(() -> {
+      layoutDao.delete(layoutDao.getById(layout.getId()));
+      return null;
+    });
+  }
+
 }

web/src/test/java/lcsb/mapviewer/web/OverlayControllerIntegrationTest.java

@@ -423,23 +423,6 @@ public class OverlayControllerIntegrationTest extends ControllerIntegrationTest
         .getAsJsonArray().size());
   }
 
-  private Layout createOverlay(User admin, String content) {
-    UploadedFileEntry file = new UploadedFileEntry();
-    file.setFileContent(content.getBytes());
-    file.setLength(content.getBytes().length);
-
-    Layout overlay = new Layout();
-    overlay.setInputData(file);
-    overlay.setProject(project);
-    overlay.setCreator(admin);
-    layoutDao.add(overlay);
-    return overlay;
-  }
-
-  private Layout createOverlay(User admin) {
-    return createOverlay(admin, "elementIdentifier\tvalue\n\t-1");
-  }
-
   @Test
   public void testGetReactionsForOverlay() throws Exception {
     User admin = createAdmin(TEST_ADMIN_LOGIN, TEST_ADMIN_PASSWORD);
@@ -1058,4 +1041,13 @@ public class OverlayControllerIntegrationTest extends ControllerIntegrationTest
     assertEquals(2, deprecatedColumns);
   }
 
+  private Layout createOverlay(User user, String string) {
+    return createOverlay(project, user, string);
+  }
+
+  private Layout createOverlay(User user) {
+    return createOverlay(project, user);
+  }
+
+
 }

web/src/test/java/lcsb/mapviewer/web/OverlayControllerIntegrationTestWithoutTransaction.java

@@ -10,19 +10,20 @@ import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.http.MediaType;
-import org.springframework.test.annotation.Rollback;
 import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
 import org.springframework.test.web.servlet.RequestBuilder;
 
+import lcsb.mapviewer.model.map.layout.Layout;
 import lcsb.mapviewer.model.user.ConfigurationElementType;
+import lcsb.mapviewer.model.user.User;
 import lcsb.mapviewer.services.interfaces.IConfigurationService;
 
 @RunWith(SpringJUnit4ClassRunner.class)
-@Rollback
 public class OverlayControllerIntegrationTestWithoutTransaction extends ControllerIntegrationTest {
 
-  private static final String BUILD_IN_TEST_ADMIN_PASSWORD = "admin";
-  private static final String BUILD_IN_TEST_ADMIN_LOGIN = "admin";
+  private static final String TEST_LOGIN = "test_l";
+  private static final String TEST_PASSWORD = "test_p";
+
   Logger logger = LogManager.getLogger();
   @Autowired
   IConfigurationService configurationService;
@@ -91,4 +92,24 @@ public class OverlayControllerIntegrationTestWithoutTransaction extends Controll
     mockMvc.perform(request)
         .andExpect(status().is4xxClientError());
   }
+
+  @Test
+
+  public void testRemoveOwnOverlayAsSimpleUser() throws Exception {
+    User user = createUserInSeparateThread(TEST_LOGIN, TEST_PASSWORD);
+    Layout overlay = null;
+    try {
+      overlay = createOverlayInSeparateThread(defaultProjectId, user);
+      RequestBuilder request = delete("/projects/" + defaultProjectId + "/overlays/" + overlay.getId())
+          .contentType(MediaType.APPLICATION_FORM_URLENCODED);
+
+      mockMvc.perform(request)
+          .andExpect(status().is4xxClientError());
+    } finally {
+      if (overlay != null) {
+        removeOverlayInSeparateThread(overlay);
+      }
+      removeUserInSeparateThread(user);
+    }
+  }
 }

web/src/test/java/lcsb/mapviewer/web/ProjectControllerIntegrationTestForAsyncCalls.java

@@ -381,10 +381,4 @@ public class ProjectControllerIntegrationTestForAsyncCalls extends ControllerInt
     });
   }
 
-  private User createUserInSeparateThread(String login, String password) throws Exception {
-    return callInSeparateThread(() -> {
-      return createUser(login, password);
-    });
-  }
-
 }