Add AuthenticatorNotApplicableException in presto-spi

imjalpreet · imjalpreet · commit a1f44c145cae · 2025-09-18T22:24:40.000+05:30
diff --git a/presto-docs/src/main/sphinx/develop/presto-authenticator.rst b/presto-docs/src/main/sphinx/develop/presto-authenticator.rst
@@ -12,13 +12,36 @@ Implementation
 ``PrestoAuthenticator`` instance. It also defines the name of this
 authenticator which is used by the administrator in a Presto configuration.
 
-``PrestoAuthenticator`` contains a single method, ``createAuthenticatedPrincipal()``,
-that validates the request and returns a ``Principal``, which is then
+``PrestoAuthenticator`` contains a single method, ``createAuthenticatedPrincipal(Map<String, List<String>> headers)``,
+that validates the request headers and returns a ``Principal``, which is then
 authorized by the :doc:`system-access-control`.
 
 The implementation of ``PrestoAuthenticatorFactory`` must be wrapped
 as a plugin and installed on the Presto cluster.
 
+Error Handling
+--------------
+
+The ``createAuthenticatedPrincipal(Map<String, List<String>> headers)`` method can throw two types of exceptions,
+depending on the authentication outcome:
+
+* ``AuthenticatorNotApplicableException``:
+
+  Thrown when the required authentication header is missing or invalid. This signals
+  to Presto that the current authentication method is not applicable, so it should
+  skip this authenticator and try the next configured one. The exception message is
+  not returned to the user, since authentication was never intended for this request.
+
+* ``AccessDeniedException``:
+
+  Thrown when the required header is present but authentication fails. In this case,
+  Presto will still try the next configured authenticator but the error message is
+  passed back to the user, indicating that the authentication attempt was valid but
+  unsuccessful.
+
+This distinction ensures that Presto can properly chain multiple authenticators
+while providing meaningful feedback to the user only when appropriate.
+
 Configuration
 -------------
 
diff --git a/presto-main/src/main/java/com/facebook/presto/server/security/CustomPrestoAuthenticator.java b/presto-main/src/main/java/com/facebook/presto/server/security/CustomPrestoAuthenticator.java
@@ -15,7 +15,9 @@
 
 import com.facebook.airlift.http.server.AuthenticationException;
 import com.facebook.airlift.http.server.Authenticator;
+import com.facebook.airlift.log.Logger;
 import com.facebook.presto.spi.security.AccessDeniedException;
+import com.facebook.presto.spi.security.AuthenticatorNotApplicableException;
 import jakarta.inject.Inject;
 import jakarta.servlet.http.HttpServletRequest;
 
@@ -30,6 +32,8 @@
 public class CustomPrestoAuthenticator
         implements Authenticator
 {
+    private static final Logger log = Logger.get(CustomPrestoAuthenticator.class);
+
     private PrestoAuthenticatorManager authenticatorManager;
 
     @Inject
@@ -49,11 +53,21 @@ public Principal authenticate(HttpServletRequest request)
             // Passing the header map to the authenticator (instead of HttpServletRequest)
             return authenticatorManager.getAuthenticator().createAuthenticatedPrincipal(headers);
         }
+        catch (AuthenticatorNotApplicableException e) {
+            // Presto will gracefully handle this exception and will not propagate it back to the client
+            log.debug(e, e.getMessage());
+            throw needAuthentication();
+        }
         catch (AccessDeniedException e) {
             throw new AuthenticationException(e.getMessage());
         }
     }
 
+    private static AuthenticationException needAuthentication()
+    {
+        return new AuthenticationException(null);
+    }
+
     // Utility method to extract headers from HttpServletRequest
     private Map<String, List<String>> getHeadersMap(HttpServletRequest request)
     {
diff --git a/presto-main/src/test/java/com/facebook/presto/server/security/TestCustomPrestoAuthenticator.java b/presto-main/src/test/java/com/facebook/presto/server/security/TestCustomPrestoAuthenticator.java
@@ -13,9 +13,11 @@
  */
 package com.facebook.presto.server.security;
 
+import com.facebook.airlift.http.server.AuthenticationException;
 import com.facebook.presto.security.BasicPrincipal;
 import com.facebook.presto.server.MockHttpServletRequest;
 import com.facebook.presto.spi.security.AccessDeniedException;
+import com.facebook.presto.spi.security.AuthenticatorNotApplicableException;
 import com.facebook.presto.spi.security.PrestoAuthenticator;
 import com.facebook.presto.spi.security.PrestoAuthenticatorFactory;
 import com.google.common.collect.ImmutableList;
@@ -25,20 +27,16 @@
 import org.testng.annotations.Test;
 
 import java.security.Principal;
-import java.util.List;
 import java.util.Map;
-import java.util.Optional;
 
-import static com.google.common.collect.ImmutableMap.toImmutableMap;
-import static java.util.Collections.list;
 import static java.util.Objects.requireNonNull;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.testng.Assert.assertEquals;
-import static org.testng.Assert.assertFalse;
-import static org.testng.Assert.assertTrue;
 
 public class TestCustomPrestoAuthenticator
 {
     private static final String TEST_HEADER = "test_header";
+    private static final String TEST_INVALID_HEADER = "test_invalid_header";
     private static final String TEST_HEADER_VALID_VALUE = "VALID";
     private static final String TEST_HEADER_INVALID_VALUE = "INVALID";
     private static final String TEST_FACTORY = "test_factory";
@@ -47,15 +45,9 @@ public class TestCustomPrestoAuthenticator
 
     @Test
     public void testPrestoAuthenticator()
+            throws Exception
     {
-        SecurityConfig mockSecurityConfig = new SecurityConfig();
-        mockSecurityConfig.setAuthenticationTypes(ImmutableList.of(SecurityConfig.AuthenticationType.CUSTOM));
-        PrestoAuthenticatorManager prestoAuthenticatorManager = new PrestoAuthenticatorManager(mockSecurityConfig);
-        // Add Test Presto Authenticator Factory
-        prestoAuthenticatorManager.addPrestoAuthenticatorFactory(
-                new TestingPrestoAuthenticatorFactory(
-                        TEST_FACTORY,
-                        TEST_HEADER_VALID_VALUE));
+        PrestoAuthenticatorManager prestoAuthenticatorManager = getPrestoAuthenticatorManager();
 
         prestoAuthenticatorManager.loadAuthenticator(TEST_FACTORY);
 
@@ -65,52 +57,77 @@ public void testPrestoAuthenticator()
                 TEST_REMOTE_ADDRESS,
                 ImmutableMap.of());
 
-        Optional<Principal> principal = checkAuthentication(prestoAuthenticatorManager.getAuthenticator(), request);
-        assertTrue(principal.isPresent());
-        assertEquals(principal.get().getName(), TEST_USER);
+        CustomPrestoAuthenticator customPrestoAuthenticator = new CustomPrestoAuthenticator(prestoAuthenticatorManager);
+        Principal principal = customPrestoAuthenticator.authenticate(request);
+
+        assertEquals(principal.getName(), TEST_USER);
+    }
+
+    @Test(expectedExceptions = AuthenticationException.class, expectedExceptionsMessageRegExp = "Access Denied: Authentication Failed!")
+    public void testPrestoAuthenticatorFailedAuthentication()
+            throws AuthenticationException
+    {
+        PrestoAuthenticatorManager prestoAuthenticatorManager = getPrestoAuthenticatorManager();
+
+        prestoAuthenticatorManager.loadAuthenticator(TEST_FACTORY);
 
         // Test failed authentication
-        request = new MockHttpServletRequest(
+        HttpServletRequest request = new MockHttpServletRequest(
                 ImmutableListMultimap.of(TEST_HEADER, TEST_HEADER_INVALID_VALUE + ":" + TEST_USER),
                 TEST_REMOTE_ADDRESS,
                 ImmutableMap.of());
 
-        principal = checkAuthentication(prestoAuthenticatorManager.getAuthenticator(), request);
-        assertFalse(principal.isPresent());
+        CustomPrestoAuthenticator customPrestoAuthenticator = new CustomPrestoAuthenticator(prestoAuthenticatorManager);
+        customPrestoAuthenticator.authenticate(request);
     }
 
-    private Optional<Principal> checkAuthentication(PrestoAuthenticator authenticator, HttpServletRequest request)
+    @Test
+    public void testPrestoAuthenticatorNotApplicable()
     {
-        try {
-            // Converting HttpServletRequest to Map<String, String>
-            Map<String, List<String>> headers = getHeadersMap(request);
+        PrestoAuthenticatorManager prestoAuthenticatorManager = getPrestoAuthenticatorManager();
 
-            // Passing the headers Map to the authenticator
-            return Optional.of(authenticator.createAuthenticatedPrincipal(headers));
-        }
-        catch (AccessDeniedException e) {
-            return Optional.empty();
-        }
+        prestoAuthenticatorManager.loadAuthenticator(TEST_FACTORY);
+
+        // Test invalid authenticator
+        HttpServletRequest request = new MockHttpServletRequest(
+                ImmutableListMultimap.of(TEST_INVALID_HEADER, TEST_HEADER_VALID_VALUE + ":" + TEST_USER),
+                TEST_REMOTE_ADDRESS,
+                ImmutableMap.of());
+
+        CustomPrestoAuthenticator customPrestoAuthenticator = new CustomPrestoAuthenticator(prestoAuthenticatorManager);
+
+        assertThatThrownBy(() -> customPrestoAuthenticator.authenticate(request))
+                .isInstanceOf(AuthenticationException.class)
+                .hasMessage(null);
     }
 
-    private Map<String, List<String>> getHeadersMap(HttpServletRequest request)
+    private static PrestoAuthenticatorManager getPrestoAuthenticatorManager()
     {
-        return list(request.getHeaderNames())
-                .stream()
-                .collect(toImmutableMap(
-                        headerName -> headerName,
-                        headerName -> list(request.getHeaders(headerName))));
+        SecurityConfig mockSecurityConfig = new SecurityConfig();
+        mockSecurityConfig.setAuthenticationTypes(ImmutableList.of(SecurityConfig.AuthenticationType.CUSTOM));
+        PrestoAuthenticatorManager prestoAuthenticatorManager = new PrestoAuthenticatorManager(mockSecurityConfig);
+
+        // Add Test Presto Authenticator Factory
+        prestoAuthenticatorManager.addPrestoAuthenticatorFactory(
+                new TestingPrestoAuthenticatorFactory(
+                        TEST_FACTORY,
+                        TEST_HEADER,
+                        TEST_HEADER_VALID_VALUE));
+
+        return prestoAuthenticatorManager;
     }
 
     private static class TestingPrestoAuthenticatorFactory
             implements PrestoAuthenticatorFactory
     {
         private final String name;
+        private final String validHeaderName;
         private final String validHeaderValue;
 
-        TestingPrestoAuthenticatorFactory(String name, String validHeaderValue)
+        TestingPrestoAuthenticatorFactory(String name, String validHeaderName, String validHeaderValue)
         {
             this.name = requireNonNull(name, "name is null");
+            this.validHeaderName = requireNonNull(validHeaderName, "validHeaderName is null");
             this.validHeaderValue = requireNonNull(validHeaderValue, "validHeaderValue is null");
         }
 
@@ -124,8 +141,12 @@ public String getName()
         public PrestoAuthenticator create(Map<String, String> config)
         {
             return (headers) -> {
-                // TEST_HEADER will have value of the form PART1:PART2
-                String[] header = headers.get(TEST_HEADER).get(0).split(":");
+                if (!headers.containsKey(this.validHeaderName)) {
+                    throw new AuthenticatorNotApplicableException("Invalid authenticator: required headers are missing");
+                }
+
+                // HEADER will have value of the form PART1:PART2
+                String[] header = headers.get(this.validHeaderName).get(0).split(":");
 
                 if (header[0].equals(this.validHeaderValue)) {
                     return new BasicPrincipal(header[1]);
diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/StandardErrorCode.java b/presto-spi/src/main/java/com/facebook/presto/spi/StandardErrorCode.java
@@ -150,6 +150,7 @@ public enum StandardErrorCode
     HEADER_MODIFICATION_ATTEMPT(0x0002_0015, INTERNAL_ERROR),
     DUPLICATE_FUNCTION_ERROR(0x0002_0016, INTERNAL_ERROR),
     MEMORY_ARBITRATION_FAILURE(0x0002_0017, INSUFFICIENT_RESOURCES),
+    AUTHENTICATOR_NOT_APPLICABLE(0x0002_0018, INTERNAL_ERROR),
     /**/;
 
     // Error code range 0x0003 is reserved for Presto-on-Spark
diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/security/AuthenticatorNotApplicableException.java b/presto-spi/src/main/java/com/facebook/presto/spi/security/AuthenticatorNotApplicableException.java
@@ -0,0 +1,27 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.facebook.presto.spi.security;
+
+import com.facebook.presto.spi.PrestoException;
+
+import static com.facebook.presto.spi.StandardErrorCode.AUTHENTICATOR_NOT_APPLICABLE;
+
+public class AuthenticatorNotApplicableException
+        extends PrestoException
+{
+    public AuthenticatorNotApplicableException(String message)
+    {
+        super(AUTHENTICATOR_NOT_APPLICABLE, message);
+    }
+}
diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/security/PrestoAuthenticator.java b/presto-spi/src/main/java/com/facebook/presto/spi/security/PrestoAuthenticator.java
@@ -24,6 +24,7 @@ public interface PrestoAuthenticator
      *
      * @return the authenticated Principal
      * @throws AccessDeniedException if not allowed
+     * @throws AuthenticatorNotApplicableException if authenticator is not valid
      */
     Principal createAuthenticatedPrincipal(Map<String, List<String>> headers);
 }

Original file line number	Diff line number	Diff line change
`@@ -24,6 +24,7 @@ public interface PrestoAuthenticator`
`24`	`24`	`*`
`25`	`25`	`* @return the authenticated Principal`
`26`	`26`	`* @throws AccessDeniedException if not allowed`
	`27`	`+ * @throws AuthenticatorNotApplicableException if authenticator is not valid`
`27`	`28`	`*/`
`28`	`29`	`Principal createAuthenticatedPrincipal(Map<String, List<String>> headers);`
`29`	`30`	`}`