Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 25 additions & 2 deletions presto-docs/src/main/sphinx/develop/presto-authenticator.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
-------------

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -30,6 +32,8 @@
public class CustomPrestoAuthenticator
implements Authenticator
{
private static final Logger log = Logger.get(CustomPrestoAuthenticator.class);

private PrestoAuthenticatorManager authenticatorManager;

@Inject
Expand All @@ -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);
}
Comment on lines +66 to +69
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: do you think it makes sense to include the message from AuthenticatorNotApplicableException here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hantangwangd, Actually, no, that’s exactly the purpose of AuthenticatorNotApplicableException. If a custom plugin throws this exception, we don’t want to return the error message to the user. Instead, I’ve added a debug log to capture and print the message returned by the plugin for troubleshooting.

The com.facebook.presto.spi.security.PrestoAuthenticator SPI forwards the request headers to the plugin implementation, which then uses them for authentication. A plugin implementing this SPI should only throw AuthenticatorNotApplicableException when the required header is missing or incomplete. This indicates that the current authentication method isn’t applicable, and Presto should skip it and move on to the next configured authenticator. As this isn't the intended authentication mechanism by the user, we don't want to pass the message back to the user.

On the other hand, if the required header is present but authentication fails, the custom plugin should throw an AccessDeniedException. In that case, the error message is passed back to the user.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know if this makes things clear. Otherwise, I can add more details. I have also explained a bit in the PR description.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it! Thanks for the explanation, makes sense to me.


// Utility method to extract headers from HttpServletRequest
private Map<String, List<String>> getHeadersMap(HttpServletRequest request)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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";
Expand All @@ -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);

Expand All @@ -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");
}

Expand All @@ -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]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Loading