From 71bd957326bc154293901b058d30077ec9025f6f Mon Sep 17 00:00:00 2001 From: Nicola Fulvio Calabria <nicola.calabria@inaf.it> Date: Mon, 11 Jan 2021 00:35:59 +0100 Subject: [PATCH] Task #3546 - Complete CreateNode endpoint implementation. Added tests: didn't use new Mock Filter for User injection, will include it later --- .../oats/vospace/CreateNodeController.java | 84 ++++++++++ .../exception/ContainerNotFoundException.java | 12 ++ .../exception/DuplicateNodeException.java | 12 ++ .../exception/InvalidURIException.java | 18 ++ .../vospace/exception/LinkFoundException.java | 12 ++ .../exception/PermissionDeniedException.java | 12 ++ .../vospace/CreateNodeControllerTest.java | 156 +++++++++++++++++- 7 files changed, 303 insertions(+), 3 deletions(-) create mode 100644 src/main/java/it/inaf/oats/vospace/exception/ContainerNotFoundException.java create mode 100644 src/main/java/it/inaf/oats/vospace/exception/DuplicateNodeException.java create mode 100644 src/main/java/it/inaf/oats/vospace/exception/InvalidURIException.java create mode 100644 src/main/java/it/inaf/oats/vospace/exception/LinkFoundException.java create mode 100644 src/main/java/it/inaf/oats/vospace/exception/PermissionDeniedException.java diff --git a/src/main/java/it/inaf/oats/vospace/CreateNodeController.java b/src/main/java/it/inaf/oats/vospace/CreateNodeController.java index 169f88b..1a0e791 100644 --- a/src/main/java/it/inaf/oats/vospace/CreateNodeController.java +++ b/src/main/java/it/inaf/oats/vospace/CreateNodeController.java @@ -8,7 +8,11 @@ import org.springframework.web.bind.annotation.RestController; import org.springframework.beans.factory.annotation.Autowired; import it.inaf.oats.vospace.persistence.NodeDAO; import java.util.List; +import org.springframework.beans.factory.annotation.Value; import org.springframework.web.bind.annotation.PutMapping; +import it.inaf.oats.vospace.exception.*; +import java.util.stream.Collectors; +import java.util.Arrays; @RestController public class CreateNodeController extends BaseNodeController { @@ -16,6 +20,9 @@ public class CreateNodeController extends BaseNodeController { @Autowired private NodeDAO nodeDao; + @Value("${vospace-authority}") + private String authority; + @PutMapping(value = {"/nodes", "/nodes/**"}, consumes = {MediaType.APPLICATION_XML_VALUE, MediaType.APPLICATION_JSON_VALUE}, produces = {MediaType.APPLICATION_XML_VALUE, MediaType.APPLICATION_JSON_VALUE}) @@ -25,7 +32,84 @@ public class CreateNodeController extends BaseNodeController { List<String> userGroups = principal.getGroups(); + if (!isValidURI(node.getUri())) + throw new InvalidURIException(node.getUri()); + + if(!isUrlConsistentWithPayloadURI(node.getUri(), path)) { + throw new InvalidURIException(node.getUri(), path); + } + + // This checks if the user is trying to insert the root node at "/" too + if (nodeDao.listNode(path).isPresent()) { + throw new DuplicateNodeException(path); + } + + // Retrieve parent node + Node parentNode = nodeDao.listNode(getParentPath(path)) + .orElseThrow(() -> new ContainerNotFoundException(getParentPath(path))); + + List<String> groupWritePropValues = parentNode.getProperties().stream() + .filter((i) -> i.getUri() + .equals("ivo://ivoa.net/vospace/core#groupwrite")) + .map((i) -> i.getValue()) + .collect(Collectors.toList()); + + if (groupWritePropValues.isEmpty()) { + throw new PermissionDeniedException(path); + } + + List<String> nodeGroups + = Arrays.asList(groupWritePropValues.get(0).split(",",-1)); + + if (!nodeGroups.stream().anyMatch((i) -> userGroups.contains(i))) { + throw new PermissionDeniedException(path); + } + + // Check if parent node is a LinkNode and if so throw exception + if (parentNode.getType().equals("vos:LinkNode")) { + throw new LinkFoundException(getParentPath(path)); + } + nodeDao.createNode(node); return node; } + + // Assuming that this service implementation uses only ! as a separator + // in the authority part of the URI + private boolean isValidURI(String nodeURI) + { + String parsedAuthority; + if(!nodeURI.startsWith("vos://")) + { + return false; + } else { + parsedAuthority = nodeURI.replaceAll("vos://", "").split("/",-1)[0]; + } + + if(parsedAuthority.isEmpty() || + !parsedAuthority.replace("~","!").equals(authority)) + return false; + + return true; + } + + private boolean isUrlConsistentWithPayloadURI(String nodeURI, String path) { + // It's assumed that nodeURI has been validated to be in the expected + // form vos://authority[!~]somepath/mynode..." + + return nodeURI.replaceAll("vos://[^/]+", "").equals(path); + + } + + private String getParentPath(String path) { + String[] parsedPath = path.split("/"); + + StringBuilder sb = new StringBuilder(); + + for (int i = 0; i < parsedPath.length - 1; i++) { + sb.append("/").append(parsedPath[i]); + } + + return sb.toString(); + } } diff --git a/src/main/java/it/inaf/oats/vospace/exception/ContainerNotFoundException.java b/src/main/java/it/inaf/oats/vospace/exception/ContainerNotFoundException.java new file mode 100644 index 0000000..7480c29 --- /dev/null +++ b/src/main/java/it/inaf/oats/vospace/exception/ContainerNotFoundException.java @@ -0,0 +1,12 @@ +package it.inaf.oats.vospace.exception; + +import org.springframework.http.HttpStatus; +import org.springframework.web.bind.annotation.ResponseStatus; + +@ResponseStatus(value = HttpStatus.NOT_FOUND) +public class ContainerNotFoundException extends VoSpaceException { + + public ContainerNotFoundException(String path) { + super("Container Not Found at path: " + path); + } +} diff --git a/src/main/java/it/inaf/oats/vospace/exception/DuplicateNodeException.java b/src/main/java/it/inaf/oats/vospace/exception/DuplicateNodeException.java new file mode 100644 index 0000000..a64223e --- /dev/null +++ b/src/main/java/it/inaf/oats/vospace/exception/DuplicateNodeException.java @@ -0,0 +1,12 @@ +package it.inaf.oats.vospace.exception; + +import org.springframework.http.HttpStatus; +import org.springframework.web.bind.annotation.ResponseStatus; + +@ResponseStatus(value = HttpStatus.CONFLICT) +public class DuplicateNodeException extends VoSpaceException { + + public DuplicateNodeException(String path) { + super("Duplicate Node at path: " + path); + } +} diff --git a/src/main/java/it/inaf/oats/vospace/exception/InvalidURIException.java b/src/main/java/it/inaf/oats/vospace/exception/InvalidURIException.java new file mode 100644 index 0000000..c303711 --- /dev/null +++ b/src/main/java/it/inaf/oats/vospace/exception/InvalidURIException.java @@ -0,0 +1,18 @@ +package it.inaf.oats.vospace.exception; + +import org.springframework.http.HttpStatus; +import org.springframework.web.bind.annotation.ResponseStatus; + +@ResponseStatus(value = HttpStatus.BAD_REQUEST) +public class InvalidURIException extends VoSpaceException { + + public InvalidURIException(String URI, String path) { + super("InvalidURI. Payload node URI: " + URI + + " is not consistent with request path: " + path); + } + + public InvalidURIException(String URI) + { + super("InvalidURI. URI: "+URI+ " is not in a valid format"); + } +} diff --git a/src/main/java/it/inaf/oats/vospace/exception/LinkFoundException.java b/src/main/java/it/inaf/oats/vospace/exception/LinkFoundException.java new file mode 100644 index 0000000..0f316c1 --- /dev/null +++ b/src/main/java/it/inaf/oats/vospace/exception/LinkFoundException.java @@ -0,0 +1,12 @@ +package it.inaf.oats.vospace.exception; + +import org.springframework.http.HttpStatus; +import org.springframework.web.bind.annotation.ResponseStatus; + +@ResponseStatus(value = HttpStatus.BAD_REQUEST) +public class LinkFoundException extends VoSpaceException { + + public LinkFoundException(String path) { + super("Link Found at path: " + path); + } +} diff --git a/src/main/java/it/inaf/oats/vospace/exception/PermissionDeniedException.java b/src/main/java/it/inaf/oats/vospace/exception/PermissionDeniedException.java new file mode 100644 index 0000000..20a1991 --- /dev/null +++ b/src/main/java/it/inaf/oats/vospace/exception/PermissionDeniedException.java @@ -0,0 +1,12 @@ +package it.inaf.oats.vospace.exception; + +import org.springframework.http.HttpStatus; +import org.springframework.web.bind.annotation.ResponseStatus; + +@ResponseStatus(value = HttpStatus.FORBIDDEN) +public class PermissionDeniedException extends VoSpaceException { + + public PermissionDeniedException(String path) { + super("Permission Denied at path: " + path); + } +} diff --git a/src/test/java/it/inaf/oats/vospace/CreateNodeControllerTest.java b/src/test/java/it/inaf/oats/vospace/CreateNodeControllerTest.java index e2e3331..dc807a1 100644 --- a/src/test/java/it/inaf/oats/vospace/CreateNodeControllerTest.java +++ b/src/test/java/it/inaf/oats/vospace/CreateNodeControllerTest.java @@ -7,7 +7,9 @@ import net.ivoa.xml.vospace.v2.Property; import net.ivoa.xml.vospace.v2.UnstructuredDataNode; import org.junit.jupiter.api.Test; import static org.mockito.ArgumentMatchers.argThat; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import org.springframework.http.MediaType; import org.springframework.test.web.servlet.MockMvc; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put; @@ -19,9 +21,14 @@ import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMock import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.mock.mockito.MockBean; import org.springframework.boot.test.mock.mockito.SpyBean; +import net.ivoa.xml.vospace.v2.ContainerNode; +import net.ivoa.xml.vospace.v2.LinkNode; +import java.util.List; +import it.inaf.ia2.aa.data.User; +import java.util.Optional; @SpringBootTest -@AutoConfigureMockMvc +@AutoConfigureMockMvc(addFilters = false) public class CreateNodeControllerTest { @MockBean @@ -34,11 +41,46 @@ public class CreateNodeControllerTest { @Autowired private MockMvc mockMvc; + private ContainerNode getContainerParentNode(String path) { + ContainerNode parentNode = new ContainerNode(); + // Set parent node address at / + parentNode.setUri("vos://example.com!vospace" + path); + // Set groupwrite property + Property groups = new Property(); + groups.setUri("ivo://ivoa.net/vospace/core#groupwrite"); + groups.setValue("test1,test2"); + parentNode.setProperties(List.of(groups)); + return parentNode; + } + + private LinkNode getLinkParentNode(String path) { + LinkNode parentNode = new LinkNode(); + // Set parent node address at / + parentNode.setUri("vos://example.com!vospace" + path); + // Set groupwrite property + Property groups = new Property(); + groups.setUri("ivo://ivoa.net/vospace/core#groupwrite"); + groups.setValue("test1,test2"); + parentNode.setProperties(List.of(groups)); + return parentNode; + } + + private User getUser() { + User user = new User(); + user.setGroups(List.of("test1")); + return user; + } + @Test public void testFromJsonToXml() throws Exception { - String requestBody = getResourceFileContent("create-unstructured-data-node.json"); + String requestBody + = getResourceFileContent("create-unstructured-data-node.json"); + + when(nodeDao.listNode(eq("/"))) + .thenReturn(Optional.of(getContainerParentNode("/"))); mockMvc.perform(put("/nodes/mydata1") + .principal(getUser()) .content(requestBody) .contentType(MediaType.APPLICATION_JSON) .accept(MediaType.APPLICATION_XML)) @@ -52,7 +94,11 @@ public class CreateNodeControllerTest { 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") + .principal(getUser()) .content(requestBody) .contentType(MediaType.APPLICATION_XML) .accept(MediaType.APPLICATION_JSON)) @@ -66,7 +112,11 @@ public class CreateNodeControllerTest { 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") + .principal(getUser()) .content(requestBody) .contentType(MediaType.APPLICATION_XML) .accept(MediaType.APPLICATION_XML)) @@ -80,7 +130,11 @@ public class CreateNodeControllerTest { 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") + .principal(getUser()) .content(requestBody) .contentType(MediaType.APPLICATION_JSON) .accept(MediaType.APPLICATION_JSON)) @@ -90,6 +144,102 @@ public class CreateNodeControllerTest { verifyArguments(); } + @Test + public void testURIConsistence() throws Exception { + String requestBody = getResourceFileContent("create-unstructured-data-node.xml"); + + when(nodeDao.listNode(eq("/"))) + .thenReturn(Optional.of(getContainerParentNode("/"))); + + User user = new User(); + user.setGroups(List.of("test3", "test4")); + + mockMvc.perform(put("/nodes/mydata2") + .principal(user) + .content(requestBody) + .contentType(MediaType.APPLICATION_XML) + .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") + .principal(getUser()) + .content(requestBody) + .contentType(MediaType.APPLICATION_XML) + .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") + .principal(getUser()) + .content(requestBody) + .contentType(MediaType.APPLICATION_XML) + .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") + .principal(getUser()) + .content(requestBody) + .contentType(MediaType.APPLICATION_XML) + .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(getLinkParentNode("/"))); + + mockMvc.perform(put("/nodes/mydata1") + .principal(getUser()) + .content(requestBody) + .contentType(MediaType.APPLICATION_XML) + .accept(MediaType.APPLICATION_XML)) + .andDo(print()) + .andExpect(status().is4xxClientError()); + + verifyArguments(); + } + private void verifyArguments() { verify(controller).createNode( argThat(node -> { @@ -102,7 +252,7 @@ public class CreateNodeControllerTest { } protected static String getResourceFileContent(String fileName) throws Exception { - try ( InputStream in = CreateNodeControllerTest.class.getClassLoader().getResourceAsStream(fileName)) { + try (InputStream in = CreateNodeControllerTest.class.getClassLoader().getResourceAsStream(fileName)) { return new String(in.readAllBytes(), StandardCharsets.UTF_8); } } -- GitLab