From a551367614bb3f3ac063af65c7115a4dffc77aed Mon Sep 17 00:00:00 2001
From: Nicola Fulvio Calabria <nicola.calabria@inaf.it>
Date: Sun, 17 Jan 2021 22:45:51 +0100
Subject: [PATCH] BugFix #3613 - Fixed CreateNodeController: if userId matches
parent node creator property write is allowed.
---
.../oats/vospace/CreateNodeController.java | 80 +++++++++++++------
.../vospace/CreateNodeControllerTest.java | 6 +-
2 files changed, 55 insertions(+), 31 deletions(-)
diff --git a/src/main/java/it/inaf/oats/vospace/CreateNodeController.java b/src/main/java/it/inaf/oats/vospace/CreateNodeController.java
index 1979ee7..cab8451 100644
--- a/src/main/java/it/inaf/oats/vospace/CreateNodeController.java
+++ b/src/main/java/it/inaf/oats/vospace/CreateNodeController.java
@@ -12,7 +12,6 @@ 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 {
@@ -30,8 +29,6 @@ public class CreateNodeController extends BaseNodeController {
String path = getPath();
- List<String> userGroups = principal.getGroups();
-
// Validate payload node URI
if (!isValidURI(node.getUri())) {
throw new InvalidURIException(node.getUri());
@@ -52,29 +49,6 @@ public class CreateNodeController extends BaseNodeController {
Node parentNode = nodeDao.listNode(getParentPath(path))
.orElseThrow(() -> new ContainerNotFoundException(getParentPath(path)));
- // Check user write/ownership privilege against parent node
- List<String> groupWritePropValues
- = getNodePropertyByURI(parentNode, "ivo://ivoa.net/vospace/core#groupwrite");
-
- if (groupWritePropValues.isEmpty()) {
- throw new PermissionDeniedException(path);
- }
-
- List<String> nodeGroups
- = Arrays.asList(groupWritePropValues.get(0).split(" ", -1));
-
- if (userGroups == null
- || !nodeGroups.stream().anyMatch((i) -> userGroups.contains(i))) {
- // If groups don't match check ownership at least
- List<String> nodeOwner
- = getNodePropertyByURI(parentNode, "ivo://ivoa.net/vospace/core#creator");
-
- if (nodeOwner.isEmpty()
- || !nodeOwner.get(0).equals(principal.getName())) {
- throw new PermissionDeniedException(path);
- }
- }
-
// Check if parent node is not a Container node and in case throw
// appropriate exception
if (!parentNode.getType().equals("vos:ContainerNode")) {
@@ -85,6 +59,45 @@ public class CreateNodeController extends BaseNodeController {
}
}
+ // First check if parent node creator is == userid
+ List<String> nodeOwner
+ = getNodePropertyByURI(
+ parentNode, "ivo://ivoa.net/vospace/core#creator");
+
+ if (nodeOwner == null
+ || nodeOwner.isEmpty()
+ || !nodeOwner.get(0).equals(principal.getName())) {
+ // Node owner check has failed: let's check if user can write
+ // due to group privileges
+
+ List<String> userGroups = principal.getGroups();
+
+ // If the user doesn't belong to any groups throw exception
+ if (userGroups == null || userGroups.isEmpty()) {
+ throw new PermissionDeniedException(path);
+ }
+
+ List<String> groupWritePropValues
+ = getNodePropertyByURI(parentNode,
+ "ivo://ivoa.net/vospace/core#groupwrite");
+
+ // If groupwrite property is absent in Parent Node throw exception
+ if (groupWritePropValues == null
+ || groupWritePropValues.isEmpty()) {
+ throw new PermissionDeniedException(path);
+ }
+
+ List<String> nodeGroups
+ = parsePropertyStringToList(groupWritePropValues.get(0));
+
+ if (nodeGroups.isEmpty()
+ || !nodeGroups.stream()
+ .anyMatch((i) -> userGroups.contains(i))) {
+ throw new PermissionDeniedException(path);
+ }
+
+ }
+
return node;
}
@@ -151,4 +164,19 @@ public class CreateNodeController extends BaseNodeController {
return propertyList;
}
+
+ private List<String> parsePropertyStringToList(String property) {
+ // If separator changes, this method should remain consistent
+ // For now it assumes that " " is the separator
+ String separator = " ";
+
+ String trimmedProperty = property.trim();
+ if (trimmedProperty.isEmpty()) {
+ return List.of();
+ }
+
+ return List.of(trimmedProperty.split(separator));
+
+ }
+
}
diff --git a/src/test/java/it/inaf/oats/vospace/CreateNodeControllerTest.java b/src/test/java/it/inaf/oats/vospace/CreateNodeControllerTest.java
index d812ef1..cdb1106 100644
--- a/src/test/java/it/inaf/oats/vospace/CreateNodeControllerTest.java
+++ b/src/test/java/it/inaf/oats/vospace/CreateNodeControllerTest.java
@@ -64,14 +64,10 @@ public class CreateNodeControllerTest {
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("group3");
Property creator = new Property();
creator.setUri("ivo://ivoa.net/vospace/core#creator");
creator.setValue("user2");
- parentNode.setProperties(List.of(groups,creator));
+ parentNode.setProperties(List.of(creator));
return parentNode;
}
--
GitLab