Merge pull request #112 from DataONEorg/feature-111-directory-permiss…

…ions Feature-111: Directory Permissions
DataONEorg · Nov 13, 2024 · e8f8f11 · e8f8f11
2 parents 509735d + b7fbaa3
commit e8f8f11
Show file tree

Hide file tree

Showing 4 changed files with 73 additions and 16 deletions.
diff --git a/src/main/java/org/dataone/hashstore/filehashstore/FileHashStore.java b/src/main/java/org/dataone/hashstore/filehashstore/FileHashStore.java
@@ -1519,18 +1519,8 @@ protected void move(File source, File target, String entity)
             return;
         }
 
-        File destinationDirectory = new File(target.getParent());
-        // Create parent directory if it doesn't exist
-        if (!destinationDirectory.exists()) {
-            Path destinationDirectoryPath = destinationDirectory.toPath();
-            try {
-                Files.createDirectories(destinationDirectoryPath);
-
-            } catch (FileAlreadyExistsException faee) {
-                logFileHashStore.warn("Directory already exists at: " + destinationDirectoryPath
-                                          + " - Skipping directory creation");
-            }
-        }
+        // Create parent directories if they don't exist
+        FileHashStoreUtility.createParentDirectories(target.toPath());
 
         // Move file
         Path sourceFilePath = source.toPath();
@@ -1541,7 +1531,7 @@ protected void move(File source, File target, String entity)
                 "File moved from: " + sourceFilePath + ", to: " + targetFilePath);
 
         } catch (FileAlreadyExistsException faee) {
-            logFileHashStore.warn(
+            logFileHashStore.debug(
                 "File already exists, skipping request to move object. Source: " + source
                     + ". Target: " + target);
 

diff --git a/src/main/java/org/dataone/hashstore/filehashstore/FileHashStoreUtility.java b/src/main/java/org/dataone/hashstore/filehashstore/FileHashStoreUtility.java
@@ -8,11 +8,13 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.attribute.FileAttribute;
 import java.nio.file.attribute.PosixFilePermission;
 import java.nio.file.FileAlreadyExistsException;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.StandardCopyOption;
+import java.nio.file.attribute.PosixFilePermissions;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
 import java.util.ArrayList;
@@ -116,7 +118,12 @@ public static void createParentDirectories(Path desiredPath) throws IOException
         if (!desiredPathParentDirs.exists()) {
             Path destinationDirectoryPath = desiredPathParentDirs.toPath();
             try {
-                Files.createDirectories(destinationDirectoryPath);
+                // The execute permission must be added to the owner/group as it is crucial for
+                // users (ex. maven/junit or a group) to access directories and subdirectories
+                Set<PosixFilePermission> perms = PosixFilePermissions.fromString("rwxr-x---");
+                FileAttribute<Set<PosixFilePermission>> attr =
+                    PosixFilePermissions.asFileAttribute(perms);
+                Files.createDirectories(destinationDirectoryPath, attr);
 
             } catch (FileAlreadyExistsException faee) {
                 log.warn("Directory already exists at: " + destinationDirectoryPath

diff --git a/src/main/java/org/dataone/hashstore/hashstoreconverter/FileHashStoreLinks.java b/src/main/java/org/dataone/hashstore/hashstoreconverter/FileHashStoreLinks.java
@@ -18,7 +18,6 @@
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
 import java.util.HashMap;
-import java.util.List;
 import java.util.Map;
 import java.util.Properties;
 
@@ -118,7 +117,7 @@ public ObjectMetadata storeHardLink(
                 Files.createLink(objHardLinkPath, filePath);
 
             } catch (FileAlreadyExistsException faee) {
-                logFileHashStoreLinks.warn("Data object already exists at: " + objHardLinkPath);
+                logFileHashStoreLinks.debug("Data object already exists at: " + objHardLinkPath);
             }
 
             // This method is thread safe and synchronized

diff --git a/src/test/java/org/dataone/hashstore/filehashstore/FileHashStoreProtectedTest.java b/src/test/java/org/dataone/hashstore/filehashstore/FileHashStoreProtectedTest.java
@@ -985,6 +985,67 @@ public void move() throws Exception {
         assertTrue(targetFile.exists());
     }
 
+    /**
+     * Confirm directories have 'rwxr-x---' permissions
+     */
+    @Test
+    public void move_directoryPermissions() throws Exception {
+        File newTmpFile = generateTemporaryFile();
+        String targetString = tempFolder.toString() + "/testmove/subdir1/subdir2/test_tmp_object"
+            + ".tmp";
+        File targetFile = new File(targetString);
+
+        fileHashStore.move(newTmpFile, targetFile, "object");
+
+        Path path = Paths.get(targetFile.toString());
+        while (path.getParent() != null) {
+            path = path.getParent();
+            // Check if the directory name starts with "testmove"
+            if (path.getFileName().toString().startsWith("junit")) {
+                break;
+            } else {
+                System.out.println(path);
+                Set<PosixFilePermission> actualPermissions = Files.getPosixFilePermissions(path);
+
+                assertTrue(actualPermissions.contains(PosixFilePermission.OWNER_READ));
+                assertTrue(actualPermissions.contains(PosixFilePermission.OWNER_WRITE));
+                assertTrue(actualPermissions.contains(PosixFilePermission.OWNER_EXECUTE));
+                assertTrue(actualPermissions.contains(PosixFilePermission.GROUP_READ));
+                assertFalse(actualPermissions.contains(PosixFilePermission.GROUP_WRITE));
+                assertTrue(actualPermissions.contains(PosixFilePermission.GROUP_EXECUTE));
+                assertFalse(actualPermissions.contains(PosixFilePermission.OTHERS_READ));
+                assertFalse(actualPermissions.contains(PosixFilePermission.OTHERS_WRITE));
+                assertFalse(actualPermissions.contains(PosixFilePermission.OTHERS_EXECUTE));
+            }
+        }
+    }
+
+    /**
+     * Confirm file has 'rw-r-----' permissions
+     */
+    @Test
+    public void move_filePermissions() throws Exception {
+        File newTmpFile = generateTemporaryFile();
+        String targetString = tempFolder.toString() + "/testmove/subdir1/subdir2/test_tmp_object"
+            + ".tmp";
+        File targetFile = new File(targetString);
+
+        fileHashStore.move(newTmpFile, targetFile, "object");
+
+        Set<PosixFilePermission> actualPermissions =
+            Files.getPosixFilePermissions(targetFile.toPath());
+
+        assertTrue(actualPermissions.contains(PosixFilePermission.OWNER_READ));
+        assertTrue(actualPermissions.contains(PosixFilePermission.OWNER_WRITE));
+        assertFalse(actualPermissions.contains(PosixFilePermission.OWNER_EXECUTE));
+        assertTrue(actualPermissions.contains(PosixFilePermission.GROUP_READ));
+        assertFalse(actualPermissions.contains(PosixFilePermission.GROUP_WRITE));
+        assertFalse(actualPermissions.contains(PosixFilePermission.GROUP_EXECUTE));
+        assertFalse(actualPermissions.contains(PosixFilePermission.OTHERS_READ));
+        assertFalse(actualPermissions.contains(PosixFilePermission.OTHERS_WRITE));
+        assertFalse(actualPermissions.contains(PosixFilePermission.OTHERS_EXECUTE));
+    }
+
     /**
      * Confirm that exceptions are not thrown when move is called on an object that already exists
      */