Make sure that file based access control have coherent config properties

sopel39 · Oct 28, 2018 · 52cff69 · 52cff69
1 parent cd0f7ff
commit 52cff69
Show file tree

Hide file tree

Showing 4 changed files with 12 additions and 9 deletions.
diff --git a/presto-main/pom.xml b/presto-main/pom.xml
@@ -382,6 +382,10 @@
             <artifactId>jaxrs-testing</artifactId>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>com.facebook.presto</groupId>
+            <artifactId>presto-plugin-toolkit</artifactId>
+        </dependency>
     </dependencies>
 
     <build>

diff --git a/presto-main/src/main/java/com/facebook/presto/security/FileBasedSystemAccessControl.java b/presto-main/src/main/java/com/facebook/presto/security/FileBasedSystemAccessControl.java
@@ -38,6 +38,7 @@
 import java.util.Set;
 import java.util.regex.Pattern;
 
+import static com.facebook.presto.plugin.base.security.FileBasedAccessControlConfig.SECURITY_CONFIG_FILE;
 import static com.facebook.presto.spi.security.AccessDeniedException.denyCatalogAccess;
 import static com.facebook.presto.spi.security.AccessDeniedException.denySetUser;
 import static com.google.common.base.Preconditions.checkState;
@@ -61,8 +62,6 @@ private FileBasedSystemAccessControl(List<CatalogAccessControlRule> catalogRules
     public static class Factory
             implements SystemAccessControlFactory
     {
-        static final String CONFIG_FILE_NAME = "security.config-file";
-
         @Override
         public String getName()
         {
@@ -74,10 +73,8 @@ public SystemAccessControl create(Map<String, String> config)
         {
             requireNonNull(config, "config is null");
 
-            String configFileName = config.get(CONFIG_FILE_NAME);
-            checkState(
-                    configFileName != null,
-                    "Security configuration must contain the '%s' property", CONFIG_FILE_NAME);
+            String configFileName = config.get(SECURITY_CONFIG_FILE);
+            checkState(configFileName != null, "Security configuration must contain the '%s' property", SECURITY_CONFIG_FILE);
 
             try {
                 Path path = Paths.get(configFileName);

diff --git a/presto-main/src/test/java/com/facebook/presto/security/TestFileBasedSystemAccessControl.java b/presto-main/src/test/java/com/facebook/presto/security/TestFileBasedSystemAccessControl.java
@@ -29,7 +29,7 @@
 import java.util.Optional;
 import java.util.Set;
 
-import static com.facebook.presto.security.FileBasedSystemAccessControl.Factory.CONFIG_FILE_NAME;
+import static com.facebook.presto.plugin.base.security.FileBasedAccessControlConfig.SECURITY_CONFIG_FILE;
 import static com.facebook.presto.spi.security.Privilege.SELECT;
 import static com.facebook.presto.spi.testing.InterfaceTestUtils.assertAllMethodsOverridden;
 import static com.facebook.presto.transaction.InMemoryTransactionManager.createTestTransactionManager;
@@ -213,6 +213,6 @@ public void parseUnknownRules()
 
     private SystemAccessControl parse(String path)
     {
-        return new FileBasedSystemAccessControl.Factory().create(ImmutableMap.of(CONFIG_FILE_NAME, path));
+        return new FileBasedSystemAccessControl.Factory().create(ImmutableMap.of(SECURITY_CONFIG_FILE, path));
     }
 }
diff --git a/.../src/main/java/com/facebook/presto/plugin/base/security/FileBasedAccessControlConfig.java b/.../src/main/java/com/facebook/presto/plugin/base/security/FileBasedAccessControlConfig.java
@@ -19,6 +19,8 @@
 
 public class FileBasedAccessControlConfig
 {
+    public static final String SECURITY_CONFIG_FILE = "security.config-file";
+
     private String configFile;
 
     @NotNull
@@ -27,7 +29,7 @@ public String getConfigFile()
         return configFile;
     }
 
-    @Config("security.config-file")
+    @Config(SECURITY_CONFIG_FILE)
     public FileBasedAccessControlConfig setConfigFile(String configFile)
     {
         this.configFile = configFile;