Addressed all review comments and implemented requested changes for c…

…onsistency and alignment.

presto-lark-sheets/pom.xml

@@ -76,11 +76,6 @@
 
         <!-- Presto SPI -->
 
-        <dependency>
-            <groupId>javax.servlet</groupId>
-            <artifactId>javax.servlet-api</artifactId>
-        </dependency>
-
         <dependency>
             <groupId>com.facebook.presto</groupId>
             <artifactId>presto-common</artifactId>
@@ -154,18 +149,4 @@
             <scope>test</scope>
         </dependency>
     </dependencies>
-
-    <build>
-        <plugins>
-            <plugin>
-                <groupId>org.apache.maven.plugins</groupId>
-                <artifactId>maven-dependency-plugin</artifactId>
-                <configuration>
-                    <ignoredUnusedDeclaredDependencies>
-                        <ignoredUnusedDeclaredDependency>javax.servlet:javax.servlet-api:jar:3.1.0</ignoredUnusedDeclaredDependency>
-                    </ignoredUnusedDeclaredDependencies>
-                </configuration>
-            </plugin>
-        </plugins>
-    </build>
 </project>

presto-main/src/main/java/com/facebook/presto/server/PluginManager.java

@@ -26,9 +26,8 @@
 import com.facebook.presto.execution.resourceGroups.ResourceGroupManager;
 import com.facebook.presto.metadata.Metadata;
 import com.facebook.presto.security.AccessControlManager;
-import com.facebook.presto.server.security.IscustomAuthenticatorRequested;
 import com.facebook.presto.server.security.PasswordAuthenticatorManager;
-import com.facebook.presto.server.security.SecurityConfig;
+import com.facebook.presto.server.security.PrestoAuthenticatorManager;
 import com.facebook.presto.spi.CoordinatorPlugin;
 import com.facebook.presto.spi.Plugin;
 import com.facebook.presto.spi.analyzer.AnalyzerProvider;
@@ -119,7 +118,7 @@ public class PluginManager
     private final ResourceGroupManager<?> resourceGroupManager;
     private final AccessControlManager accessControlManager;
     private final PasswordAuthenticatorManager passwordAuthenticatorManager;
-    private final IscustomAuthenticatorRequested prestoAuthenticatorManager;
+    private final PrestoAuthenticatorManager prestoAuthenticatorManager;
     private final EventListenerManager eventListenerManager;
     private final BlockEncodingManager blockEncodingManager;
     private final TempStorageManager tempStorageManager;
@@ -151,8 +150,7 @@ public PluginManager(
             QueryPreparerProviderManager queryPreparerProviderManager,
             AccessControlManager accessControlManager,
             PasswordAuthenticatorManager passwordAuthenticatorManager,
-            SecurityConfig securityConfig,
-            IscustomAuthenticatorRequested prestoAuthenticatorManager,
+            PrestoAuthenticatorManager prestoAuthenticatorManager,
             EventListenerManager eventListenerManager,
             BlockEncodingManager blockEncodingManager,
             TempStorageManager tempStorageManager,

presto-main/src/main/java/com/facebook/presto/server/PrestoServer.java

@@ -48,8 +48,8 @@
 import com.facebook.presto.nodeManager.PluginNodeManager;
 import com.facebook.presto.security.AccessControlManager;
 import com.facebook.presto.security.AccessControlModule;
-import com.facebook.presto.server.security.IscustomAuthenticatorRequested;
 import com.facebook.presto.server.security.PasswordAuthenticatorManager;
+import com.facebook.presto.server.security.PrestoAuthenticatorManager;
 import com.facebook.presto.server.security.ServerSecurityModule;
 import com.facebook.presto.sql.analyzer.FeaturesConfig;
 import com.facebook.presto.sql.parser.SqlParserOptions;
@@ -176,7 +176,7 @@ public void run()
                 injector.getInstance(AccessControlManager.class).loadSystemAccessControl();
             }
             injector.getInstance(PasswordAuthenticatorManager.class).loadPasswordAuthenticator();
-            injector.getInstance(IscustomAuthenticatorRequested.class).loadPrestoAuthenticator();
+            injector.getInstance(PrestoAuthenticatorManager.class).loadPrestoAuthenticator();
             injector.getInstance(EventListenerManager.class).loadConfiguredEventListener();
             injector.getInstance(TempStorageManager.class).loadTempStorages();
             injector.getInstance(QueryPrerequisitesManager.class).loadQueryPrerequisites();

presto-main/src/main/java/com/facebook/presto/server/security/CustomPrestoAuthenticator.java

@@ -21,52 +21,58 @@
 import javax.servlet.http.HttpServletRequest;
 
 import java.security.Principal;
+import java.util.ArrayList;
 import java.util.Enumeration;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 
 import static java.util.Objects.requireNonNull;
 
 public class CustomPrestoAuthenticator
         implements Authenticator
 {
-    private IscustomAuthenticatorRequested authenticatorManager;
+    private PrestoAuthenticatorManager authenticatorManager;
 
     @Inject
-    public CustomPrestoAuthenticator(IscustomAuthenticatorRequested authenticatorManager)
+    public CustomPrestoAuthenticator(PrestoAuthenticatorManager authenticatorManager)
     {
         this.authenticatorManager = requireNonNull(authenticatorManager, "authenticatorManager is null");
     }
 
-    // Utility method to extract headers from HttpServletRequest
-    private Map<String, String> getHeadersMap(HttpServletRequest request)
-    {
-        Map<String, String> headers = new HashMap<>();
-        Enumeration<String> headerNames = request.getHeaderNames();
-        while (headerNames.hasMoreElements()) {
-            String headerName = headerNames.nextElement();
-            String headerValue = request.getHeader(headerName);
-            headers.put(headerName, headerValue);
-        }
-        return headers;
-    }
-
     @Override
     public Principal authenticate(HttpServletRequest request)
             throws AuthenticationException
     {
         try {
             // Extracting headers into a Map
-            Map<String, String> headers = getHeadersMap(request);
+            Map<String, List<String>> headers = getHeadersMap(request);
 
             // Passing the header map to the authenticator (instead of HttpServletRequest)
             return authenticatorManager.getAuthenticator().createAuthenticatedPrincipal(headers);
         }
         catch (AccessDeniedException e) {
-            throw e;
+            throw new AuthenticationException(e.getMessage());
         }
         catch (RuntimeException e) {
-            throw new AuthenticationException("Authentication failed due to an unexpected runtime error.");
+            throw new RuntimeException("Authentication failed due to an unexpected runtime error", e);
         }
     }
+
+    // Utility method to extract headers from HttpServletRequest
+    private Map<String, List<String>> getHeadersMap(HttpServletRequest request)
+    {
+        Map<String, List<String>> headers = new HashMap<>();
+        Enumeration<String> headerNames = request.getHeaderNames();
+        while (headerNames.hasMoreElements()) {
+            String headerName = headerNames.nextElement();
+            Enumeration<String> headerValues = request.getHeaders(headerName);
+            List<String> valuesList = new ArrayList<>();
+            while (headerValues.hasMoreElements()) {
+                valuesList.add(headerValues.nextElement());
+            }
+            headers.put(headerName, valuesList);
+        }
+        return headers;
+    }
 }

presto-main/src/main/java/com/facebook/presto/server/security/IscustomAuthenticatorRequested.java → presto-main/src/main/java/com/facebook/presto/server/security/PrestoAuthenticatorManager.java

@@ -25,7 +25,6 @@
 import java.util.HashMap;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicReference;
 
 import static com.facebook.presto.util.PropertiesUtil.loadProperties;
@@ -34,20 +33,19 @@
 import static com.google.common.base.Strings.isNullOrEmpty;
 import static java.util.Objects.requireNonNull;
 
-public class IscustomAuthenticatorRequested
+public class PrestoAuthenticatorManager
 {
-    private static final Logger log = Logger.get(IscustomAuthenticatorRequested.class);
+    private static final Logger log = Logger.get(PrestoAuthenticatorManager.class);
 
     private static final File CONFIG_FILE = new File("etc/presto-authenticator.properties");
     private static final String NAME_PROPERTY = "presto-authenticator.name";
 
-    private final AtomicBoolean required = new AtomicBoolean();
     private final Map<String, PrestoAuthenticatorFactory> factories = new ConcurrentHashMap<>();
     private final AtomicReference<PrestoAuthenticator> authenticator = new AtomicReference<>();
     private final boolean customAuthenticatorRequested;
 
     @Inject
-    public IscustomAuthenticatorRequested(SecurityConfig securityConfig)
+    public PrestoAuthenticatorManager(SecurityConfig securityConfig)
     {
         this.customAuthenticatorRequested = securityConfig.getAuthenticationTypes().contains(SecurityConfig.AuthenticationType.CUSTOM);
     }
@@ -70,7 +68,7 @@ public void loadAuthenticator(String authenticatorName)
     public void loadPrestoAuthenticator()
             throws Exception
     {
-        if (!required.get()) {
+        if (!customAuthenticatorRequested) {
             return;
         }
 

presto-main/src/main/java/com/facebook/presto/server/security/ServerSecurityModule.java

@@ -40,7 +40,7 @@ public class ServerSecurityModule
     protected void setup(Binder binder)
     {
         binder.bind(PasswordAuthenticatorManager.class).in(Scopes.SINGLETON);
-        binder.bind(IscustomAuthenticatorRequested.class).in(Scopes.SINGLETON);
+        binder.bind(PrestoAuthenticatorManager.class).in(Scopes.SINGLETON);
 
         List<AuthenticationType> authTypes = buildConfigObject(SecurityConfig.class).getAuthenticationTypes();
         Multibinder<Authenticator> authBinder = newSetBinder(binder, Authenticator.class);

presto-main/src/main/java/com/facebook/presto/testing/LocalQueryRunner.java

@@ -123,8 +123,8 @@
 import com.facebook.presto.server.PluginManager;
 import com.facebook.presto.server.PluginManagerConfig;
 import com.facebook.presto.server.SessionPropertyDefaults;
-import com.facebook.presto.server.security.IscustomAuthenticatorRequested;
 import com.facebook.presto.server.security.PasswordAuthenticatorManager;
+import com.facebook.presto.server.security.PrestoAuthenticatorManager;
 import com.facebook.presto.server.security.SecurityConfig;
 import com.facebook.presto.spi.ConnectorId;
 import com.facebook.presto.spi.PageIndexerFactory;
@@ -521,8 +521,7 @@ private LocalQueryRunner(Session defaultSession, FeaturesConfig featuresConfig,
                 new QueryPreparerProviderManager(queryPreparerProvider),
                 accessControl,
                 new PasswordAuthenticatorManager(),
-                new SecurityConfig(),
-                new IscustomAuthenticatorRequested(new SecurityConfig()),
+                new PrestoAuthenticatorManager(new SecurityConfig()),
                 new EventListenerManager(),
                 blockEncodingManager,
                 new TestingTempStorageManager(),

presto-main/src/test/java/com/facebook/presto/server/security/TestPrestoAuthenticator.java → presto-main/src/test/java/com/facebook/presto/server/security/TestCustomPrestoAuthenticator.java

@@ -26,8 +26,10 @@
 import javax.servlet.http.HttpServletRequest;
 
 import java.security.Principal;
+import java.util.ArrayList;
 import java.util.Enumeration;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 
@@ -36,7 +38,7 @@
 import static org.testng.Assert.assertFalse;
 import static org.testng.Assert.assertTrue;
 
-public class TestPrestoAuthenticator
+public class TestCustomPrestoAuthenticator
 {
     private static final String TEST_HEADER = "test_header";
     private static final String TEST_HEADER_VALID_VALUE = "VALID";
@@ -50,7 +52,7 @@ public void testPrestoAuthenticator()
     {
         SecurityConfig mockSecurityConfig = new SecurityConfig();
         mockSecurityConfig.setAuthenticationTypes(ImmutableList.of(SecurityConfig.AuthenticationType.CUSTOM));
-        IscustomAuthenticatorRequested prestoAuthenticatorManager = new IscustomAuthenticatorRequested(mockSecurityConfig);
+        PrestoAuthenticatorManager prestoAuthenticatorManager = new PrestoAuthenticatorManager(mockSecurityConfig);
         // Add Test Presto Authenticator Factory
         prestoAuthenticatorManager.addPrestoAuthenticatorFactory(
                 new TestingPrestoAuthenticatorFactory(
@@ -83,7 +85,7 @@ private Optional<Principal> checkAuthentication(PrestoAuthenticator authenticato
     {
         try {
             // Converting HttpServletRequest to Map<String, String>
-            Map<String, String> headers = getHeadersMap(request);
+            Map<String, List<String>> headers = getHeadersMap(request);
 
             // Passing the headers Map to the authenticator
             return Optional.of(authenticator.createAuthenticatedPrincipal(headers));
@@ -93,16 +95,20 @@ private Optional<Principal> checkAuthentication(PrestoAuthenticator authenticato
         }
     }
 
-    private Map<String, String> getHeadersMap(HttpServletRequest request)
+    private Map<String, List<String>> getHeadersMap(HttpServletRequest request)
     {
-        Map<String, String> headersMap = new HashMap<>();
+        Map<String, List<String>> headers = new HashMap<>();
         Enumeration<String> headerNames = request.getHeaderNames();
         while (headerNames.hasMoreElements()) {
             String headerName = headerNames.nextElement();
-            String headerValue = request.getHeader(headerName);
-            headersMap.put(headerName, headerValue);
+            Enumeration<String> headerValues = request.getHeaders(headerName);
+            List<String> valuesList = new ArrayList<>();
+            while (headerValues.hasMoreElements()) {
+                valuesList.add(headerValues.nextElement());
+            }
+            headers.put(headerName, valuesList);
         }
-        return headersMap;
+        return headers;
     }
 
     private static class TestingPrestoAuthenticatorFactory
@@ -126,9 +132,9 @@ public String getName()
         @Override
         public PrestoAuthenticator create(Map<String, String> config)
         {
-            return (httpRequest) -> {
+            return (headers) -> {
                 // TEST_HEADER will have value of the form PART1:PART2
-                String[] header = httpRequest.get(TEST_HEADER).split(":");
+                String[] header = headers.get(TEST_HEADER).get(0).split(":");
 
                 if (header[0].equals(this.validHeaderValue)) {
                     return new BasicPrincipal(header[1]);

presto-password-authenticators/pom.xml

@@ -63,11 +63,6 @@
             <version>0.9.0</version>
         </dependency>
 
-        <dependency>
-            <groupId>javax.servlet</groupId>
-            <artifactId>javax.servlet-api</artifactId>
-        </dependency>
-
         <!-- Presto SPI -->
         <dependency>
             <groupId>com.facebook.presto</groupId>
@@ -149,15 +144,6 @@
                     </ignorePackages>
                 </configuration>
             </plugin>
-            <plugin>
-                <groupId>org.apache.maven.plugins</groupId>
-                <artifactId>maven-dependency-plugin</artifactId>
-                <configuration>
-                    <ignoredUnusedDeclaredDependencies>
-                        <ignoredUnusedDeclaredDependency>javax.servlet:javax.servlet-api:jar:3.1.0</ignoredUnusedDeclaredDependency>
-                    </ignoredUnusedDeclaredDependencies>
-                </configuration>
-            </plugin>
         </plugins>
     </build>
 </project>

presto-spark-base/src/main/java/com/facebook/presto/spark/PrestoSparkInjectorFactory.java

@@ -26,8 +26,8 @@
 import com.facebook.presto.security.AccessControlModule;
 import com.facebook.presto.server.PluginManager;
 import com.facebook.presto.server.SessionPropertyDefaults;
-import com.facebook.presto.server.security.IscustomAuthenticatorRequested;
 import com.facebook.presto.server.security.PasswordAuthenticatorManager;
+import com.facebook.presto.server.security.PrestoAuthenticatorManager;
 import com.facebook.presto.spark.classloader_interface.PrestoSparkBootstrapTimer;
 import com.facebook.presto.spark.classloader_interface.SparkProcessType;
 import com.facebook.presto.spi.security.AccessControl;
@@ -183,7 +183,7 @@ public Injector create(PrestoSparkBootstrapTimer bootstrapTimer)
             injector.getInstance(StaticCatalogStore.class).loadCatalogs(catalogProperties);
             injector.getInstance(ResourceGroupManager.class).loadConfigurationManager();
             injector.getInstance(PasswordAuthenticatorManager.class).loadPasswordAuthenticator();
-            injector.getInstance(IscustomAuthenticatorRequested.class).loadPrestoAuthenticator();
+            injector.getInstance(PrestoAuthenticatorManager.class).loadPrestoAuthenticator();
             eventListenerProperties.ifPresent(properties -> injector.getInstance(EventListenerManager.class).loadConfiguredEventListener(properties));
             bootstrapTimer.endSharedModulesLoading();