diff --git a/presto-docs/src/main/sphinx/develop/presto-authenticator.rst b/presto-docs/src/main/sphinx/develop/presto-authenticator.rst index 44f2781bf8819..6f11be98c76a7 100644 --- 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> 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> 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 index 93ef5aa2e293a..6a6b192520e6d 100644 --- 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> 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 index f187c94954b5b..55e6c8ee24dc8 100644 --- 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 = 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 checkAuthentication(PrestoAuthenticator authenticator, HttpServletRequest request) + @Test + public void testPrestoAuthenticatorNotApplicable() { - try { - // Converting HttpServletRequest to Map - Map> 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> 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 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 index 3445d604dd389..426aab311608d 100644 --- 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 new file mode 100644 index 0000000000000..aa73f0bafa0bc --- /dev/null +++ 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 index c83fe711e36ff..ed1e770eb5241 100644 --- 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> headers); }