diff --git a/src/main/java/it/inaf/oats/vospace/CreateNodeController.java b/src/main/java/it/inaf/oats/vospace/CreateNodeController.java index 62ce3cf0eb8b8760f70a626cc1632a068544eb8c..81e4c29fe3cc2d6d689203b871a944355fa699f5 100644 --- a/src/main/java/it/inaf/oats/vospace/CreateNodeController.java +++ b/src/main/java/it/inaf/oats/vospace/CreateNodeController.java @@ -20,7 +20,7 @@ import net.ivoa.xml.vospace.v2.Property; public class CreateNodeController extends BaseNodeController { private static final Logger LOG = LoggerFactory.getLogger(CreateNodeController.class); - + @Autowired private NodeDAO nodeDao; @@ -33,7 +33,7 @@ public class CreateNodeController extends BaseNodeController { public Node createNode(@RequestBody Node node, User principal) { String path = getPath(); - + LOG.debug("createNode called for path {}", path); // Validate payload node URI @@ -64,30 +64,59 @@ public class CreateNodeController extends BaseNodeController { } else { throw new ContainerNotFoundException(getParentPath(path)); } - } - - if(!NodeUtils.checkIfWritable(parentNode, principal.getName(), principal.getGroups())) { + } + + if (!NodeUtils.checkIfWritable(parentNode, principal.getName(), principal.getGroups())) { throw new PermissionDeniedException(path); } - + // Check if node creator property is set. If not set it according to // token. In case of creator mistmatch between node and token throw // exception - String creator = NodeProperties.getNodePropertyByURI( node, NodeProperties.CREATOR_URI); - - if(creator == null) - { + + if (creator == null) { Property creatorProperty = new Property(); creatorProperty.setUri(NodeProperties.CREATOR_URI); creatorProperty.setValue(principal.getName()); node.getProperties().add(creatorProperty); } else { - if(!creator.equals(principal.getName())) - // maybe a more specific exception would be more appropriate? + if (!creator.equals(principal.getName())) // maybe a more specific exception would be more appropriate? + { throw new PermissionDeniedException(path); - } + } + } + + // If payload has no read groups specified, inherit from parent node + String payloadReadGroups = NodeProperties.getNodePropertyByURI( + node, NodeProperties.GROUP_READ_URI); + + if (payloadReadGroups == null) { + String parentNodeReadGroups = NodeProperties.getNodePropertyByURI( + parentNode, NodeProperties.GROUP_READ_URI); + if (parentNodeReadGroups != null) { + Property readGroups = new Property(); + readGroups.setUri(NodeProperties.GROUP_READ_URI); + readGroups.setValue(parentNodeReadGroups); + node.getProperties().add(readGroups); + } + } + + // If payload has no write groups specified, inherit from parent node + String payloadWriteGroups = NodeProperties.getNodePropertyByURI( + node, NodeProperties.GROUP_WRITE_URI); + + if (payloadWriteGroups == null) { + String parentNodeWriteGroups = NodeProperties.getNodePropertyByURI( + parentNode, NodeProperties.GROUP_WRITE_URI); + if (parentNodeWriteGroups != null) { + Property writeGroups = new Property(); + writeGroups.setUri(NodeProperties.GROUP_WRITE_URI); + writeGroups.setValue(parentNodeWriteGroups); + node.getProperties().add(writeGroups); + } + } nodeDao.createNode(node); diff --git a/src/main/java/it/inaf/oats/vospace/ListNodeController.java b/src/main/java/it/inaf/oats/vospace/ListNodeController.java index 185f68eb8d754d88209de6832dd14450bb1ad2a3..7dd303f42c02a101c3334d6bf8604c8798434326 100644 --- a/src/main/java/it/inaf/oats/vospace/ListNodeController.java +++ b/src/main/java/it/inaf/oats/vospace/ListNodeController.java @@ -7,7 +7,6 @@ import org.springframework.web.bind.annotation.GetMapping; import org.springframework.http.ResponseEntity; import net.ivoa.xml.vospace.v2.Node; -import net.ivoa.xml.vospace.v2.ContainerNode; import it.inaf.oats.vospace.persistence.NodeDAO; import javax.servlet.http.HttpServletRequest; @@ -18,8 +17,6 @@ import it.inaf.ia2.aa.data.User; import it.inaf.oats.vospace.datamodel.NodeUtils; import java.util.Optional; import it.inaf.oats.vospace.exception.PermissionDeniedException; -import java.util.stream.Collectors; -import java.util.List; @RestController public class ListNodeController extends BaseNodeController { @@ -45,21 +42,6 @@ public class ListNodeController extends BaseNodeController { throw new PermissionDeniedException(path); } } - - Node node = optNode.get(); - - if(node instanceof ContainerNode) - { - ContainerNode cnd = (ContainerNode) node; - List<Node> children = - cnd.getNodes().stream().filter( - (n)->NodeUtils.checkIfReadable( - n, principal.getName(), - principal.getGroups())) - .collect(Collectors.toList()); - cnd.setNodes(children); - optNode = Optional.of(cnd); - } return ResponseEntity.ok(optNode.get()); } diff --git a/src/test/java/it/inaf/oats/vospace/CreateNodeControllerTest.java b/src/test/java/it/inaf/oats/vospace/CreateNodeControllerTest.java index baeef5d3ab6eea039951c222448ff44e8e9f4c7f..f7de3b34135a9d220404222d338ebe64f29c16be 100644 --- a/src/test/java/it/inaf/oats/vospace/CreateNodeControllerTest.java +++ b/src/test/java/it/inaf/oats/vospace/CreateNodeControllerTest.java @@ -39,17 +39,17 @@ import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMock @TestPropertySource(properties = "spring.main.allow-bean-definition-overriding=true") @AutoConfigureMockMvc public class CreateNodeControllerTest { - + @MockBean private NodeDAO nodeDao; - + @SpyBean @Autowired private CreateNodeController controller; - + @Autowired private MockMvc mockMvc; - + private ContainerNode getContainerParentNode(String path) { ContainerNode parentNode = new ContainerNode(); // Set parent node address at / @@ -61,7 +61,7 @@ public class CreateNodeControllerTest { parentNode.setProperties(List.of(groups)); return parentNode; } - + private ContainerNode getContainerParentNodeWithCreator(String path) { ContainerNode parentNode = new ContainerNode(); // Set parent node address at / @@ -72,7 +72,7 @@ public class CreateNodeControllerTest { parentNode.setProperties(List.of(creator)); return parentNode; } - + private LinkNode getLinkParentNode(String path) { LinkNode parentNode = new LinkNode(); // Set parent node address at / @@ -84,15 +84,15 @@ public class CreateNodeControllerTest { parentNode.setProperties(List.of(groups)); return parentNode; } - + @Test public void testFromJsonToXml() throws Exception { String requestBody = getResourceFileContent("create-unstructured-data-node.json"); - + when(nodeDao.listNode(eq("/"))) .thenReturn(Optional.of(getContainerParentNode("/"))); - + mockMvc.perform(put("/nodes/mydata1") .header("Authorization", "Bearer user2_token") .content(requestBody) @@ -100,17 +100,17 @@ public class CreateNodeControllerTest { .accept(MediaType.APPLICATION_XML)) .andDo(print()) .andExpect(status().isOk()); - + verifyArguments(); } - + @Test public void testFromXmlToJson() throws Exception { String requestBody = getResourceFileContent("create-unstructured-data-node.xml"); - + when(nodeDao.listNode(eq("/"))) .thenReturn(Optional.of(getContainerParentNode("/"))); - + mockMvc.perform(put("/nodes/mydata1") .header("Authorization", "Bearer user2_token") .content(requestBody) @@ -118,18 +118,18 @@ public class CreateNodeControllerTest { .accept(MediaType.APPLICATION_JSON)) .andDo(print()) .andExpect(status().isOk()); - + verifyArguments(); verify(nodeDao, times(1)).createNode(any()); } - + @Test public void testFromXmlToXml() throws Exception { String requestBody = getResourceFileContent("create-unstructured-data-node.xml"); - + when(nodeDao.listNode(eq("/"))) .thenReturn(Optional.of(getContainerParentNode("/"))); - + mockMvc.perform(put("/nodes/mydata1") .header("Authorization", "Bearer user2_token") .content(requestBody) @@ -137,18 +137,18 @@ public class CreateNodeControllerTest { .accept(MediaType.APPLICATION_XML)) .andDo(print()) .andExpect(status().isOk()); - + verifyArguments(); verify(nodeDao, times(1)).createNode(any()); } - + @Test public void testFromJsonToJson() throws Exception { String requestBody = getResourceFileContent("create-unstructured-data-node.json"); - + when(nodeDao.listNode(eq("/"))) .thenReturn(Optional.of(getContainerParentNode("/"))); - + mockMvc.perform(put("/nodes/mydata1") .header("Authorization", "Bearer user2_token") .content(requestBody) @@ -156,18 +156,18 @@ public class CreateNodeControllerTest { .accept(MediaType.APPLICATION_JSON)) .andDo(print()) .andExpect(status().isOk()); - + verifyArguments(); verify(nodeDao, times(1)).createNode(any()); } - + @Test public void testURIConsistence() throws Exception { String requestBody = getResourceFileContent("create-unstructured-data-node.xml"); - + when(nodeDao.listNode(eq("/"))) .thenReturn(Optional.of(getContainerParentNode("/"))); - + mockMvc.perform(put("/nodes/mydata2") .header("Authorization", "Bearer user2_token") .content(requestBody) @@ -175,20 +175,20 @@ public class CreateNodeControllerTest { .accept(MediaType.APPLICATION_XML)) .andDo(print()) .andExpect(status().is4xxClientError()); - + verifyArguments(); - + } - + @Test public void testNodeAlreadyExisting() throws Exception { String requestBody = getResourceFileContent("create-unstructured-data-node.xml"); - + when(nodeDao.listNode(eq("/"))) .thenReturn(Optional.of(getContainerParentNode("/"))); when(nodeDao.listNode(eq("/mydata1"))) .thenReturn(Optional.of(getContainerParentNode("/mydata1"))); - + mockMvc.perform(put("/nodes/mydata1") .header("Authorization", "Bearer user2_token") .content(requestBody) @@ -196,17 +196,17 @@ public class CreateNodeControllerTest { .accept(MediaType.APPLICATION_XML)) .andDo(print()) .andExpect(status().is4xxClientError()); - + verifyArguments(); } - + @Test public void testContainerNotFound() throws Exception { String requestBody = getResourceFileContent("create-unstructured-data-node.xml"); - + when(nodeDao.listNode(eq("/"))) .thenReturn(Optional.ofNullable(null)); - + mockMvc.perform(put("/nodes/mydata1") .header("Authorization", "Bearer user2_token") .content(requestBody) @@ -214,17 +214,17 @@ public class CreateNodeControllerTest { .accept(MediaType.APPLICATION_XML)) .andDo(print()) .andExpect(status().is4xxClientError()); - + verifyArguments(); } - + @Test public void testLinkNodeFound() throws Exception { String requestBody = getResourceFileContent("create-unstructured-data-node.xml"); - + when(nodeDao.listNode(eq("/"))) .thenReturn(Optional.of(getLinkParentNode("/"))); - + mockMvc.perform(put("/nodes/mydata1") .header("Authorization", "Bearer user2_token") .content(requestBody) @@ -232,17 +232,17 @@ public class CreateNodeControllerTest { .accept(MediaType.APPLICATION_XML)) .andDo(print()) .andExpect(status().is4xxClientError()); - + verifyArguments(); } - + @Test public void testPermissionDenied() throws Exception { String requestBody = getResourceFileContent("create-unstructured-data-node.xml"); - + when(nodeDao.listNode(eq("/"))) .thenReturn(Optional.of(getContainerParentNode("/"))); - + mockMvc.perform(put("/nodes/mydata1") .header("Authorization", "Bearer user1_token") .content(requestBody) @@ -250,17 +250,17 @@ public class CreateNodeControllerTest { .accept(MediaType.APPLICATION_XML)) .andDo(print()) .andExpect(status().is4xxClientError()); - + verifyArguments(); } - + @Test public void testWriteWithOnlyOwnership() throws Exception { String requestBody = getResourceFileContent("create-unstructured-data-node.xml"); - + when(nodeDao.listNode(eq("/"))) .thenReturn(Optional.of(getContainerParentNodeWithCreator("/"))); - + mockMvc.perform(put("/nodes/mydata1") .header("Authorization", "Bearer user2_token") .content(requestBody) @@ -268,68 +268,98 @@ public class CreateNodeControllerTest { .accept(MediaType.APPLICATION_XML)) .andDo(print()) .andExpect(status().is2xxSuccessful()); - + verifyArguments(); verify(nodeDao, times(1)).createNode(any()); } @Test public void testWriteOwnerAbsent() throws Exception { - String requestBody = - getResourceFileContent("create-unstructured-data-node.xml"); - - when(nodeDao.listNode(eq("/"))) + String requestBody + = getResourceFileContent("create-unstructured-data-node.xml"); + + when(nodeDao.listNode(eq("/"))) .thenReturn(Optional.of(getContainerParentNodeWithCreator("/"))); - - // no node creator specified in xml file - - mockMvc.perform(put("/nodes/mydata1") + + // no node creator specified in xml file + mockMvc.perform(put("/nodes/mydata1") .header("Authorization", "Bearer user2_token") .content(requestBody) .contentType(MediaType.APPLICATION_XML) .accept(MediaType.APPLICATION_XML)) .andDo(print()) .andExpect(status().is2xxSuccessful()); - + // assert creator properties now matches user2 - verify(nodeDao, times(1)).createNode(argThat(node->{ - UnstructuredDataNode udn = (UnstructuredDataNode) node; - String creator = NodeProperties.getNodePropertyByURI( - udn, NodeProperties.CREATOR_URI); - return (creator != null && creator.equals("user2")); - } - )); + verify(nodeDao, times(1)).createNode(argThat(node -> { + UnstructuredDataNode udn = (UnstructuredDataNode) node; + String creator = NodeProperties.getNodePropertyByURI( + udn, NodeProperties.CREATOR_URI); + return (creator != null && creator.equals("user2")); + } + )); + + } + + @Test + public void testGroupPropertiesAbsent() throws Exception { + String requestBody + = getResourceFileContent("create-unstructured-data-node.xml"); + + ContainerNode cdn = getContainerParentNode("/"); + + when(nodeDao.listNode(eq("/"))) + .thenReturn(Optional.of(cdn)); + + // no node creator specified in xml file + mockMvc.perform(put("/nodes/mydata1") + .header("Authorization", "Bearer user2_token") + .content(requestBody) + .contentType(MediaType.APPLICATION_XML) + .accept(MediaType.APPLICATION_XML)) + .andDo(print()) + .andExpect(status().is2xxSuccessful()); + + // assert creator properties now matches user2 + verify(nodeDao, times(1)).createNode(argThat(node -> { + UnstructuredDataNode udn = (UnstructuredDataNode) node; + String groupRead = NodeProperties.getNodePropertyByURI( + udn, NodeProperties.GROUP_READ_URI); + String groupWrite = NodeProperties.getNodePropertyByURI( + udn, NodeProperties.GROUP_WRITE_URI); + return (groupRead == null && groupWrite.equals("group1 group2")); + } + )); } @Test public void testWriteOwnerMismatch() throws Exception { - String requestBody = - getResourceFileContent("create-unstructured-data-node-user1.xml"); - - when(nodeDao.listNode(eq("/"))) + String requestBody + = getResourceFileContent("create-unstructured-data-node-user1.xml"); + + when(nodeDao.listNode(eq("/"))) .thenReturn(Optional.of(getContainerParentNodeWithCreator("/"))); - - // no node creator specified in xml file - - mockMvc.perform(put("/nodes/mydata1") + + // no node creator specified in xml file + mockMvc.perform(put("/nodes/mydata1") .header("Authorization", "Bearer user2_token") .content(requestBody) .contentType(MediaType.APPLICATION_XML) .accept(MediaType.APPLICATION_XML)) .andDo(print()) .andExpect(status().is4xxClientError()); - + // assert createNode is not called - verify(nodeDao, times(0)).createNode(any()); + verify(nodeDao, times(0)).createNode(any()); } - + @Test public void testSubPath() throws Exception { - + String requestBody = getResourceFileContent("create-unstructured-data-node.xml") .replace("/mydata1", "/mydata1/anothernode"); - + mockMvc.perform(put("/nodes/mydata1/anothernode") .header("Authorization", "Bearer user2_token") .content(requestBody) @@ -340,19 +370,19 @@ public class CreateNodeControllerTest { // Using ArgumentCaptor for verifying multiple method invocations ArgumentCaptor<String> argCaptor = ArgumentCaptor.forClass(String.class); - + verify(nodeDao, times(2)).listNode(argCaptor.capture()); - + assertEquals("/mydata1/anothernode", argCaptor.getAllValues().get(0)); assertEquals("/mydata1", argCaptor.getAllValues().get(1)); } @Test public void testIllegalChars() throws Exception { - + String requestBody = getResourceFileContent("create-unstructured-data-node.xml") .replace("/mydata1", "/mydata1/?anothernode"); - + String message = mockMvc.perform(put(new URI("/nodes/mydata1/%3Fanothernode")) .header("Authorization", "Bearer user2_token") .content(requestBody) @@ -361,10 +391,10 @@ public class CreateNodeControllerTest { .andDo(print()) .andExpect(status().isBadRequest()) .andReturn().getResolvedException().getMessage(); - + assertTrue(message.contains("contains an illegal character")); } - + private void verifyArguments() { verify(controller).createNode( argThat(node -> { @@ -375,7 +405,7 @@ public class CreateNodeControllerTest { && "ivo://ivoa.net/vospace/core#description".equals(property.getUri()); }), any()); } - + protected static String getResourceFileContent(String fileName) throws Exception { try (InputStream in = CreateNodeControllerTest.class.getClassLoader().getResourceAsStream(fileName)) { return new String(in.readAllBytes(), StandardCharsets.UTF_8); diff --git a/src/test/java/it/inaf/oats/vospace/ListNodeControllerTest.java b/src/test/java/it/inaf/oats/vospace/ListNodeControllerTest.java index 9d37ae60880438b47c0ef25454fa51de3e33f095..415e2b403099fa6f8f234d7baad4ccfd6458706c 100644 --- a/src/test/java/it/inaf/oats/vospace/ListNodeControllerTest.java +++ b/src/test/java/it/inaf/oats/vospace/ListNodeControllerTest.java @@ -26,7 +26,6 @@ import static org.springframework.test.web.servlet.request.MockMvcRequestBuilder import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.print; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; import org.w3c.dom.Document; -import org.w3c.dom.NodeList; @SpringBootTest @AutoConfigureMockMvc @@ -40,7 +39,7 @@ public class ListNodeControllerTest { private NodeDAO dao; @Autowired - private MockMvc mockMvc; + private MockMvc mockMvc; @Test public void testRootXml() throws Exception { @@ -84,71 +83,29 @@ public class ListNodeControllerTest { .accept(MediaType.APPLICATION_XML)) .andExpect(status().isNotFound()); } - + @Test public void testPermissionDeniedUser() throws Exception { - Node node = getDataNodeByOwnership("user2", "group1"); - + Node node = getDataNodeByOwnership("user2","group1"); + when(dao.listNode(eq("/mynode"))).thenReturn(Optional.of(node)); - + mockMvc.perform(get("/nodes/mynode") .header("Authorization", "Bearer user1_token") .accept(MediaType.APPLICATION_XML)) - .andExpect(status().is4xxClientError()); + .andExpect(status().is4xxClientError()); } - + @Test public void testGrantedByGroup() throws Exception { - Node node = getDataNodeByOwnership("user1", "group1"); - + Node node = getDataNodeByOwnership("user1","group1"); + when(dao.listNode(eq("/mynode"))).thenReturn(Optional.of(node)); - + mockMvc.perform(get("/nodes/mynode") .header("Authorization", "Bearer user2_token") .accept(MediaType.APPLICATION_XML)) - .andExpect(status().is2xxSuccessful()); - } - - @Test - public void testRemoveUnreadable() throws Exception { - // Create container node - ContainerNode root = (ContainerNode) getRootNode().get(); - - Node node1 = getDataNodeByOwnership("user1", "group10"); - node1.setUri(URI_PREFIX + "/mynode1"); - root.getNodes().add(node1); - - Node node2 = getDataNodeByOwnership("user1", "group10"); - node2.setUri(URI_PREFIX + "/mynode2"); - root.getNodes().add(node2); - - Node node3 = getDataNodeByOwnership("user2", "group10"); - node3.setUri(URI_PREFIX + "/mynode3"); - root.getNodes().add(node3); - - Node node4 = getDataNodeByOwnership("user3", "group10"); - node4.setUri(URI_PREFIX + "/mynode4"); - root.getNodes().add(node4); - - when(dao.listNode(eq("/"))).thenReturn(Optional.of(root)); - - String xml = mockMvc.perform(get("/nodes/") - .header("Authorization", "Bearer user2_token") - .accept(MediaType.APPLICATION_XML)) - .andExpect(status().is2xxSuccessful()) - .andDo(print()) - .andReturn().getResponse().getContentAsString(); - - Document doc = loadDocument(xml); - assertEquals("vos:node", doc.getDocumentElement().getNodeName()); - assertEquals("vos:ContainerNode", doc.getDocumentElement().getAttribute("xsi:type")); - NodeList nl = doc.getDocumentElement().getElementsByTagName("vos:nodes"); - - assertEquals(1, nl.getLength()); - NodeList children = nl.item(0).getChildNodes(); - assertEquals(2, children.getLength()); - verify(dao, times(1)).listNode(eq("/")); - + .andExpect(status().is2xxSuccessful()); } private Optional<Node> getRootNode() { @@ -157,8 +114,8 @@ public class ListNodeControllerTest { Property publicProperty = new Property(); publicProperty.setUri(NodeProperties.PUBLIC_READ_URI); publicProperty.setValue("true"); - root.getProperties().add(publicProperty); - + root.getProperties().add(publicProperty); + root.getNodes().add(getDataNode()); return Optional.of(root); } @@ -173,8 +130,9 @@ public class ListNodeControllerTest { return node; } - - private Node getDataNodeByOwnership(String ownerID, String group) { + + private Node getDataNodeByOwnership(String ownerID, String group) + { DataNode node = new DataNode(); node.setUri(URI_PREFIX + "/mynode"); // Set owner @@ -187,7 +145,7 @@ public class ListNodeControllerTest { readGroupProperty.setUri(NodeProperties.GROUP_READ_URI); readGroupProperty.setValue(group); node.getProperties().add(readGroupProperty); - - return node; + + return node; } }