Move permissions function to a separate package scope function. Fix t…

…est case to only run on posix
snowflakedb · Jun 27, 2024 · 2a47f62 · 2a47f62
1 parent d36b9be
commit 2a47f62
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 14 deletions.
diff --git a/src/main/java/net/snowflake/client/config/SFClientConfigParser.java b/src/main/java/net/snowflake/client/config/SFClientConfigParser.java
@@ -67,9 +67,7 @@ public static SFClientConfig loadSFClientConfig(String configFilePath) throws IO
     }
     if (derivedConfigFilePath != null) {
       try {
-        if (!checkConfigFilePermissions(derivedConfigFilePath)) {
-          return null;
-        }
+        checkConfigFilePermissions(derivedConfigFilePath);
 
         File configFile = new File(derivedConfigFilePath);
         ObjectMapper objectMapper = new ObjectMapper();
@@ -124,28 +122,31 @@ && systemGetProperty("os.name").toLowerCase().startsWith("windows")) {
     }
   }
 
-  public static Boolean checkConfigFilePermissions(String derivedConfigFilePath)
+
+  private static void checkConfigFilePermissions(String derivedConfigFilePath)
       throws IOException {
     try {
       if (Constants.getOS() != Constants.OS.WINDOWS) {
         // Check permissions of config file
-        Set<PosixFilePermission> folderPermissions =
-            Files.getPosixFilePermissions(Paths.get(derivedConfigFilePath));
-        if (folderPermissions.contains(PosixFilePermission.GROUP_WRITE)
-            || folderPermissions.contains(PosixFilePermission.OTHERS_WRITE)) {
+        if (checkGroupOthersWritePermissions(derivedConfigFilePath)) {
           String error =
               String.format(
                   "Error due to other users having permission to modify the config file: %s",
                   derivedConfigFilePath);
           // TODO: SNOW-1503722 to change warning log to throw an error instead
           logger.warn(error);
-          return false;
         }
       }
     } catch (IOException e) {
       throw e;
     }
-    return true;
+  }
+
+  static Boolean checkGroupOthersWritePermissions(String configFilePath) throws IOException {
+    Set<PosixFilePermission> folderPermissions =
+            Files.getPosixFilePermissions(Paths.get(configFilePath));
+    return folderPermissions.contains(PosixFilePermission.GROUP_WRITE)
+            || folderPermissions.contains(PosixFilePermission.OTHERS_WRITE);
   }
 
   static String convertToWindowsPath(String filePath) {

diff --git a/src/test/java/net/snowflake/client/config/SFPermissionsTest.java b/src/test/java/net/snowflake/client/config/SFPermissionsTest.java
@@ -6,20 +6,22 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+import java.nio.file.attribute.PosixFilePermission;
 import java.nio.file.attribute.PosixFilePermissions;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Set;
 import net.snowflake.client.ConditionalIgnoreRule;
 import net.snowflake.client.RunningOnWin;
-import org.junit.After;
-import org.junit.Before;
-import org.junit.Test;
+import net.snowflake.client.core.Constants;
+import org.junit.*;
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
 
 @RunWith(Parameterized.class)
 public class SFPermissionsTest {
+  @Rule
+  public ConditionalIgnoreRule rule = new ConditionalIgnoreRule();
 
   @Parameterized.Parameters(name = "permission={0}")
   public static Set<Map.Entry<String, Boolean>> data() {
@@ -75,7 +77,7 @@ public void testLogDirectoryPermissions() throws IOException {
     // TODO: SNOW-1503722 Change to check for thrown exceptions
     // Don't run on Windows
     Files.setPosixFilePermissions(configFilePath, PosixFilePermissions.fromString(permission));
-    Boolean result = SFClientConfigParser.checkConfigFilePermissions(configFilePath.toString());
+    Boolean result = SFClientConfigParser.checkGroupOthersWritePermissions(configFilePath.toString());
     if (isSucceed != result) {
       fail("testLogDirectoryPermissions failed. Expected " + isSucceed);
     }