From 68c50cbbec041dda4194f6dbc9fbfd21ce00897a Mon Sep 17 00:00:00 2001 From: Dou Mok Date: Wed, 13 Nov 2024 10:53:16 -0800 Subject: [PATCH 1/4] Revise 'createParentDirectories' method in 'FileHashStoreUtility' to set 'rwxr-x---' permissions --- .../hashstore/filehashstore/FileHashStoreUtility.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/dataone/hashstore/filehashstore/FileHashStoreUtility.java b/src/main/java/org/dataone/hashstore/filehashstore/FileHashStoreUtility.java index 5c15c769..b18fac88 100644 --- 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 perms = PosixFilePermissions.fromString("rwxr-x---"); + FileAttribute> attr = + PosixFilePermissions.asFileAttribute(perms); + Files.createDirectories(destinationDirectoryPath, attr); } catch (FileAlreadyExistsException faee) { log.warn("Directory already exists at: " + destinationDirectoryPath From 024b2709f965489a5b6e78ba26cdb641bbcfccf1 Mon Sep 17 00:00:00 2001 From: Dou Mok Date: Wed, 13 Nov 2024 10:53:47 -0800 Subject: [PATCH 2/4] Refactor 'FileHashStore' move method to use 'FileHashStoreUtility.createParentDirectories' and add new junit tests --- .../filehashstore/FileHashStore.java | 19 +++--- .../FileHashStoreProtectedTest.java | 62 +++++++++++++++++++ 2 files changed, 69 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/dataone/hashstore/filehashstore/FileHashStore.java b/src/main/java/org/dataone/hashstore/filehashstore/FileHashStore.java index cc293616..bc40ead3 100644 --- a/src/main/java/org/dataone/hashstore/filehashstore/FileHashStore.java +++ b/src/main/java/org/dataone/hashstore/filehashstore/FileHashStore.java @@ -17,15 +17,20 @@ import java.nio.file.Paths; import java.nio.file.StandardCopyOption; import java.nio.file.StandardOpenOption; +import java.nio.file.attribute.FileAttribute; +import java.nio.file.attribute.PosixFilePermission; +import java.nio.file.attribute.PosixFilePermissions; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Properties; +import java.util.Set; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; @@ -1519,18 +1524,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(); diff --git a/src/test/java/org/dataone/hashstore/filehashstore/FileHashStoreProtectedTest.java b/src/test/java/org/dataone/hashstore/filehashstore/FileHashStoreProtectedTest.java index 64522145..d495cc2f 100644 --- a/src/test/java/org/dataone/hashstore/filehashstore/FileHashStoreProtectedTest.java +++ b/src/test/java/org/dataone/hashstore/filehashstore/FileHashStoreProtectedTest.java @@ -27,6 +27,7 @@ import java.util.Objects; import java.util.Properties; import java.util.Set; +import java.util.stream.Stream; import javax.xml.bind.DatatypeConverter; @@ -985,6 +986,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 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 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 */ From a71a6bf99c338f139e9c0bae20a34330726fa3f0 Mon Sep 17 00:00:00 2001 From: Dou Mok Date: Wed, 13 Nov 2024 10:58:11 -0800 Subject: [PATCH 3/4] Change logging to debug from warn in 'FileHashStore' and 'FileHashStoreLinks' when duplicate object has been encountered --- .../java/org/dataone/hashstore/filehashstore/FileHashStore.java | 2 +- .../hashstore/hashstoreconverter/FileHashStoreLinks.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/dataone/hashstore/filehashstore/FileHashStore.java b/src/main/java/org/dataone/hashstore/filehashstore/FileHashStore.java index bc40ead3..b3145c06 100644 --- a/src/main/java/org/dataone/hashstore/filehashstore/FileHashStore.java +++ b/src/main/java/org/dataone/hashstore/filehashstore/FileHashStore.java @@ -1536,7 +1536,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/hashstoreconverter/FileHashStoreLinks.java b/src/main/java/org/dataone/hashstore/hashstoreconverter/FileHashStoreLinks.java index 084f2f96..d5cba553 100644 --- a/src/main/java/org/dataone/hashstore/hashstoreconverter/FileHashStoreLinks.java +++ b/src/main/java/org/dataone/hashstore/hashstoreconverter/FileHashStoreLinks.java @@ -118,7 +118,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 From b7fbaa3acf813e58989d0fa011999b34a9b65b19 Mon Sep 17 00:00:00 2001 From: Dou Mok Date: Wed, 13 Nov 2024 11:02:49 -0800 Subject: [PATCH 4/4] Remove unused imports --- .../org/dataone/hashstore/filehashstore/FileHashStore.java | 5 ----- .../hashstore/hashstoreconverter/FileHashStoreLinks.java | 1 - .../hashstore/filehashstore/FileHashStoreProtectedTest.java | 1 - 3 files changed, 7 deletions(-) diff --git a/src/main/java/org/dataone/hashstore/filehashstore/FileHashStore.java b/src/main/java/org/dataone/hashstore/filehashstore/FileHashStore.java index b3145c06..4bcccf95 100644 --- a/src/main/java/org/dataone/hashstore/filehashstore/FileHashStore.java +++ b/src/main/java/org/dataone/hashstore/filehashstore/FileHashStore.java @@ -17,20 +17,15 @@ import java.nio.file.Paths; import java.nio.file.StandardCopyOption; import java.nio.file.StandardOpenOption; -import java.nio.file.attribute.FileAttribute; -import java.nio.file.attribute.PosixFilePermission; -import java.nio.file.attribute.PosixFilePermissions; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Properties; -import java.util.Set; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; diff --git a/src/main/java/org/dataone/hashstore/hashstoreconverter/FileHashStoreLinks.java b/src/main/java/org/dataone/hashstore/hashstoreconverter/FileHashStoreLinks.java index d5cba553..8a959592 100644 --- 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; diff --git a/src/test/java/org/dataone/hashstore/filehashstore/FileHashStoreProtectedTest.java b/src/test/java/org/dataone/hashstore/filehashstore/FileHashStoreProtectedTest.java index d495cc2f..0bd99379 100644 --- a/src/test/java/org/dataone/hashstore/filehashstore/FileHashStoreProtectedTest.java +++ b/src/test/java/org/dataone/hashstore/filehashstore/FileHashStoreProtectedTest.java @@ -27,7 +27,6 @@ import java.util.Objects; import java.util.Properties; import java.util.Set; -import java.util.stream.Stream; import javax.xml.bind.DatatypeConverter;