diff --git a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProvider.java b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProvider.java index 9e7b25930e0be..f150a084237f5 100644 --- a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProvider.java +++ b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProvider.java @@ -18,6 +18,8 @@ */ package org.apache.pulsar.broker.authentication; +import static org.apache.pulsar.broker.web.AuthenticationFilter.AuthenticatedDataAttributeName; +import static org.apache.pulsar.broker.web.AuthenticationFilter.AuthenticatedRoleAttributeName; import java.io.Closeable; import java.io.IOException; import java.net.SocketAddress; @@ -28,7 +30,6 @@ import javax.servlet.http.HttpServletResponse; import org.apache.pulsar.broker.ServiceConfiguration; import org.apache.pulsar.common.api.AuthData; -import org.apache.pulsar.common.classification.InterfaceStability; import org.apache.pulsar.common.util.FutureUtil; /** @@ -103,7 +104,16 @@ default AuthenticationState newAuthState(AuthData authData, /** * Create an http authentication data State use passed in AuthenticationDataSource. + * @deprecated implementations that previously relied on this should update their implementation of + * {@link #authenticateHttpRequest(HttpServletRequest, HttpServletResponse)} or of + * {@link #authenticateHttpRequestAsync(HttpServletRequest, HttpServletResponse)} so that the desired attributes + * are added in those methods. + * + *

Note: this method was only ever used to generate an {@link AuthenticationState} object in order to generate + * an {@link AuthenticationDataSource} that was added as the {@link AuthenticatedDataAttributeName} attribute to + * the http request. Removing this method removes an unnecessary step in the authentication flow.

*/ + @Deprecated(since = "2.12.0") default AuthenticationState newHttpAuthState(HttpServletRequest request) throws AuthenticationException { return new OneStageAuthenticationState(request, this); @@ -112,20 +122,17 @@ default AuthenticationState newHttpAuthState(HttpServletRequest request) /** * Validate the authentication for the given credentials with the specified authentication data. * + *

Implementations of this method MUST modify the request by adding the {@link AuthenticatedRoleAttributeName} + * and the {@link AuthenticatedDataAttributeName} attributes.

+ * *

Warning: the calling thread is an IO thread. Any implementations that rely on blocking behavior * must ensure that the execution is completed on using a separate thread pool to ensure IO threads * are never blocked.

* - *

Note: this method is marked as unstable because the Pulsar code base only calls it for the - * Pulsar Broker Auth SASL plugin. All non SASL HTTP requests are authenticated using the - * {@link AuthenticationProvider#authenticateAsync(AuthenticationDataSource)} method. As such, - * this method might be removed in favor of the SASL provider implementing the - * {@link AuthenticationProvider#authenticateAsync(AuthenticationDataSource)} method.

- * - * @return Set response, according to passed in request. + * @return Set response, according to passed in request, and return whether we should do following chain.doFilter. + * @throws Exception when authentication failed * and return whether we should do following chain.doFilter or not. */ - @InterfaceStability.Unstable default CompletableFuture authenticateHttpRequestAsync(HttpServletRequest request, HttpServletResponse response) { try { @@ -138,10 +145,20 @@ default CompletableFuture authenticateHttpRequestAsync(HttpServletReque /** * Set response, according to passed in request. * and return whether we should do following chain.doFilter or not. + * + *

Implementations of this method MUST modify the request by adding the {@link AuthenticatedRoleAttributeName} + * and the {@link AuthenticatedDataAttributeName} attributes.

+ * + * @return Set response, according to passed in request, and return whether we should do following chain.doFilter. + * @throws Exception when authentication failed * @deprecated use and implement {@link AuthenticationProvider#authenticateHttpRequestAsync} instead. */ @Deprecated default boolean authenticateHttpRequest(HttpServletRequest request, HttpServletResponse response) throws Exception { - throw new AuthenticationException("Not supported"); + AuthenticationState authenticationState = newHttpAuthState(request); + String role = authenticate(authenticationState.getAuthDataSource()); + request.setAttribute(AuthenticatedRoleAttributeName, role); + request.setAttribute(AuthenticatedDataAttributeName, authenticationState.getAuthDataSource()); + return true; } } diff --git a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderToken.java b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderToken.java index 7fe20d5c5421a..bc72d4b4ea9a8 100644 --- a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderToken.java +++ b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderToken.java @@ -19,6 +19,8 @@ package org.apache.pulsar.broker.authentication; import static java.nio.charset.StandardCharsets.UTF_8; +import static org.apache.pulsar.broker.web.AuthenticationFilter.AuthenticatedDataAttributeName; +import static org.apache.pulsar.broker.web.AuthenticationFilter.AuthenticatedRoleAttributeName; import com.google.common.annotations.VisibleForTesting; import io.jsonwebtoken.Claims; import io.jsonwebtoken.ExpiredJwtException; @@ -39,6 +41,7 @@ import javax.naming.AuthenticationException; import javax.net.ssl.SSLSession; import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; import org.apache.commons.lang3.StringUtils; import org.apache.pulsar.broker.ServiceConfiguration; import org.apache.pulsar.broker.authentication.metrics.AuthenticationMetrics; @@ -160,6 +163,20 @@ public String authenticate(AuthenticationDataSource authData) throws Authenticat } } + @Override + public boolean authenticateHttpRequest(HttpServletRequest request, HttpServletResponse response) throws Exception { + HttpServletRequestWrapper wrappedRequest = new HttpServletRequestWrapper(request); + String httpHeaderValue = wrappedRequest.getHeader(HTTP_HEADER_NAME); + if (httpHeaderValue == null || !httpHeaderValue.startsWith(HTTP_HEADER_VALUE_PREFIX)) { + throw new AuthenticationException("Invalid HTTP Authorization header"); + } + AuthenticationDataSource authenticationDataSource = new AuthenticationDataHttps(wrappedRequest); + String role = authenticate(authenticationDataSource); + request.setAttribute(AuthenticatedRoleAttributeName, role); + request.setAttribute(AuthenticatedDataAttributeName, authenticationDataSource); + return true; + } + @Override public AuthenticationState newAuthState(AuthData authData, SocketAddress remoteAddress, SSLSession sslSession) throws AuthenticationException { diff --git a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationService.java b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationService.java index c63f5505e5974..5f92453238f4d 100644 --- a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationService.java +++ b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationService.java @@ -18,6 +18,8 @@ */ package org.apache.pulsar.broker.authentication; +import static org.apache.pulsar.broker.web.AuthenticationFilter.AuthenticatedDataAttributeName; +import static org.apache.pulsar.broker.web.AuthenticationFilter.AuthenticatedRoleAttributeName; import java.io.Closeable; import java.io.IOException; import java.util.ArrayList; @@ -28,10 +30,12 @@ import java.util.stream.Collectors; import javax.naming.AuthenticationException; import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; import org.apache.commons.lang3.StringUtils; import org.apache.pulsar.broker.PulsarServerException; import org.apache.pulsar.broker.ServiceConfiguration; import org.apache.pulsar.broker.web.AuthenticationFilter; +import org.apache.pulsar.common.sasl.SaslConstants; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -85,16 +89,77 @@ public AuthenticationService(ServiceConfiguration conf) throws PulsarServerExcep } } + private String getAuthMethodName(HttpServletRequest request) { + return request.getHeader(AuthenticationFilter.PULSAR_AUTH_METHOD_NAME); + } + + private AuthenticationProvider getAuthProvider(String authMethodName) throws AuthenticationException { + AuthenticationProvider providerToUse = providers.get(authMethodName); + if (providerToUse == null) { + throw new AuthenticationException( + String.format("Unsupported authentication method: [%s].", authMethodName)); + } + return providerToUse; + } + + public boolean authenticateHttpRequest(HttpServletRequest request, HttpServletResponse response) + throws Exception { + String authMethodName = getAuthMethodName(request); + if (authMethodName == null + && SaslConstants.SASL_TYPE_VALUE.equalsIgnoreCase(request.getHeader(SaslConstants.SASL_HEADER_TYPE))) { + // This edge case must be handled because the Pulsar SASL implementation does not add the + // X-Pulsar-Auth-Method-Name header. + authMethodName = SaslConstants.AUTH_METHOD_NAME; + } + if (authMethodName != null) { + AuthenticationProvider providerToUse = getAuthProvider(authMethodName); + try { + return providerToUse.authenticateHttpRequest(request, response); + } catch (AuthenticationException e) { + if (LOG.isDebugEnabled()) { + LOG.debug("Authentication failed for provider " + providerToUse.getAuthMethodName() + " : " + + e.getMessage(), e); + } + throw e; + } + } else { + for (AuthenticationProvider provider : providers.values()) { + try { + return provider.authenticateHttpRequest(request, response); + } catch (AuthenticationException e) { + if (LOG.isDebugEnabled()) { + LOG.debug("Authentication failed for provider " + provider.getAuthMethodName() + ": " + + e.getMessage(), e); + } + // Ignore the exception because we don't know which authentication method is expected here. + } + } + // No authentication provided + if (!providers.isEmpty()) { + if (StringUtils.isNotBlank(anonymousUserRole)) { + request.setAttribute(AuthenticatedRoleAttributeName, anonymousUserRole); + request.setAttribute(AuthenticatedDataAttributeName, new AuthenticationDataHttps(request)); + return true; + } + // If at least a provider was configured, then the authentication needs to be provider + throw new AuthenticationException("Authentication required"); + } else { + // No authentication required + return true; + } + } + } + + /** + * @deprecated use {@link #authenticateHttpRequest(HttpServletRequest, HttpServletResponse)} + */ + @Deprecated(since = "2.12.0") public String authenticateHttpRequest(HttpServletRequest request, AuthenticationDataSource authData) throws AuthenticationException { - String authMethodName = request.getHeader(AuthenticationFilter.PULSAR_AUTH_METHOD_NAME); + String authMethodName = getAuthMethodName(request); if (authMethodName != null) { - AuthenticationProvider providerToUse = providers.get(authMethodName); - if (providerToUse == null) { - throw new AuthenticationException( - String.format("Unsupported authentication method: [%s].", authMethodName)); - } + AuthenticationProvider providerToUse = getAuthProvider(authMethodName); try { if (authData == null) { AuthenticationState authenticationState = providerToUse.newHttpAuthState(request); @@ -140,10 +205,11 @@ public String authenticateHttpRequest(HttpServletRequest request, Authentication /** * Mark this function as deprecated, it is recommended to use a method with the AuthenticationDataSource * signature to implement it. + * @deprecated use {@link #authenticateHttpRequest(HttpServletRequest, HttpServletResponse)}. */ @Deprecated public String authenticateHttpRequest(HttpServletRequest request) throws AuthenticationException { - return authenticateHttpRequest(request, null); + return authenticateHttpRequest(request, (AuthenticationDataSource) null); } public AuthenticationProvider getAuthenticationProvider(String authMethodName) { diff --git a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/web/AuthenticationFilter.java b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/web/AuthenticationFilter.java index 55265b21b3b1b..6f13185ca7540 100644 --- a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/web/AuthenticationFilter.java +++ b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/web/AuthenticationFilter.java @@ -28,10 +28,7 @@ import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import org.apache.pulsar.broker.authentication.AuthenticationDataHttps; import org.apache.pulsar.broker.authentication.AuthenticationService; -import org.apache.pulsar.broker.authentication.AuthenticationState; -import org.apache.pulsar.common.sasl.SaslConstants; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -52,54 +49,12 @@ public AuthenticationFilter(AuthenticationService authenticationService) { this.authenticationService = authenticationService; } - private boolean isSaslRequest(HttpServletRequest request) { - if (request.getHeader(SaslConstants.SASL_HEADER_TYPE) == null - || request.getHeader(SaslConstants.SASL_HEADER_TYPE).isEmpty()) { - return false; - } - if (request.getHeader(SaslConstants.SASL_HEADER_TYPE) - .equalsIgnoreCase(SaslConstants.SASL_TYPE_VALUE)) { - return true; - } else { - return false; - } - } - @Override public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { try { - HttpServletRequest httpRequest = (HttpServletRequest) request; - HttpServletResponse httpResponse = (HttpServletResponse) response; - - if (!isSaslRequest(httpRequest)) { - // not sasl type, return role directly. - String authMethodName = httpRequest.getHeader(PULSAR_AUTH_METHOD_NAME); - String role; - if (authMethodName != null && authenticationService.getAuthenticationProvider(authMethodName) != null) { - AuthenticationState authenticationState = authenticationService - .getAuthenticationProvider(authMethodName).newHttpAuthState(httpRequest); - request.setAttribute(AuthenticatedDataAttributeName, authenticationState.getAuthDataSource()); - role = authenticationService.authenticateHttpRequest( - (HttpServletRequest) request, authenticationState.getAuthDataSource()); - } else { - request.setAttribute(AuthenticatedDataAttributeName, - new AuthenticationDataHttps((HttpServletRequest) request)); - role = authenticationService.authenticateHttpRequest((HttpServletRequest) request); - } - request.setAttribute(AuthenticatedRoleAttributeName, role); - - if (LOG.isDebugEnabled()) { - LOG.debug("[{}] Authenticated HTTP request with role {}", request.getRemoteAddr(), role); - } - chain.doFilter(request, response); - return; - } - boolean doFilter = authenticationService - .getAuthenticationProvider(SaslConstants.AUTH_METHOD_NAME) - .authenticateHttpRequest(httpRequest, httpResponse); - + .authenticateHttpRequest((HttpServletRequest) request, (HttpServletResponse) response); if (doFilter) { chain.doFilter(request, response); } @@ -111,7 +66,6 @@ public void doFilter(ServletRequest request, ServletResponse response, FilterCha } else { LOG.error("[{}] Error performing authentication for HTTP", request.getRemoteAddr(), e); } - return; } } diff --git a/pulsar-broker-common/src/test/java/org/apache/pulsar/broker/authentication/AuthenticationProviderTokenTest.java b/pulsar-broker-common/src/test/java/org/apache/pulsar/broker/authentication/AuthenticationProviderTokenTest.java index 82e8a8f87ee55..ee4534e1ab7b7 100644 --- a/pulsar-broker-common/src/test/java/org/apache/pulsar/broker/authentication/AuthenticationProviderTokenTest.java +++ b/pulsar-broker-common/src/test/java/org/apache/pulsar/broker/authentication/AuthenticationProviderTokenTest.java @@ -885,8 +885,8 @@ public void testTokenFromHttpParams() throws Exception { doReturn("127.0.0.1").when(servletRequest).getRemoteAddr(); doReturn(0).when(servletRequest).getRemotePort(); - AuthenticationState authState = provider.newHttpAuthState(servletRequest); - provider.authenticate(authState.getAuthDataSource()); + boolean doFilter = provider.authenticateHttpRequest(servletRequest, null); + assertTrue(doFilter, "Authentication should have passed"); } @Test @@ -910,8 +910,8 @@ public void testTokenFromHttpHeaders() throws Exception { doReturn("127.0.0.1").when(servletRequest).getRemoteAddr(); doReturn(0).when(servletRequest).getRemotePort(); - AuthenticationState authState = provider.newHttpAuthState(servletRequest); - provider.authenticate(authState.getAuthDataSource()); + boolean doFilter = provider.authenticateHttpRequest(servletRequest, null); + assertTrue(doFilter, "Authentication should have passed"); } private static String createTokenWithAudience(Key signingKey, String audienceClaim, List audience) { diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/broker/auth/AuthenticationServiceTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/broker/auth/AuthenticationServiceTest.java index d9ef59afd1e22..78ae046b0c8c8 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/broker/auth/AuthenticationServiceTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/broker/auth/AuthenticationServiceTest.java @@ -18,15 +18,23 @@ */ package org.apache.pulsar.broker.auth; +import static org.apache.pulsar.broker.web.AuthenticationFilter.AuthenticatedDataAttributeName; +import static org.apache.pulsar.broker.web.AuthenticationFilter.AuthenticatedRoleAttributeName; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertTrue; import com.google.common.collect.Sets; import java.io.IOException; import java.util.Set; import javax.naming.AuthenticationException; import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; import org.apache.pulsar.broker.ServiceConfiguration; +import org.apache.pulsar.broker.authentication.AuthenticationDataHttps; import org.apache.pulsar.broker.authentication.AuthenticationDataSource; import org.apache.pulsar.broker.authentication.AuthenticationProvider; import org.apache.pulsar.broker.authentication.AuthenticationService; @@ -83,6 +91,81 @@ public void testAuthenticationHttpWithMultipleProviders() throws Exception { service.close(); } + @Test(timeOut = 10000) + public void testAuthenticationHttpRequestResponse() throws Exception { + ServiceConfiguration config = new ServiceConfiguration(); + Set providersClassNames = Sets.newHashSet(MockAuthenticationProvider.class.getName()); + config.setAuthenticationProviders(providersClassNames); + config.setAuthenticationEnabled(true); + AuthenticationService service = new AuthenticationService(config); + HttpServletRequest request = mock(HttpServletRequest.class); + when(request.getRemoteAddr()).thenReturn("192.168.1.1"); + when(request.getRemotePort()).thenReturn(8080); + when(request.getHeader("X-Pulsar-Auth-Method-Name")).thenReturn("auth"); + boolean doFilter = service.authenticateHttpRequest(request, (HttpServletResponse) null); + assertTrue(doFilter, "Authentication should have succeeded"); + verify(request).setAttribute(AuthenticatedRoleAttributeName, s_authentication_success); + verify(request).setAttribute(eq(AuthenticatedDataAttributeName), any(AuthenticationDataHttps.class)); + service.close(); + } + + @Test(timeOut = 10000) + public void testAuthenticationHttpRequestResponseWithMultipleProviders() throws Exception { + boolean doFilter; + ServiceConfiguration config = new ServiceConfiguration(); + Set providersClassNames = Sets.newHashSet(MockAuthenticationProvider.class.getName(), + MockAuthenticationProviderWithDifferentName.class.getName()); + config.setAuthenticationProviders(providersClassNames); + config.setAuthenticationEnabled(true); + AuthenticationService service = new AuthenticationService(config); + HttpServletRequest requestDefaultAuthProvider = mock(HttpServletRequest.class); + when(requestDefaultAuthProvider.getRemoteAddr()).thenReturn("192.168.1.1"); + when(requestDefaultAuthProvider.getRemotePort()).thenReturn(8080); + when(requestDefaultAuthProvider.getHeader("X-Pulsar-Auth-Method-Name")).thenReturn("auth"); + doFilter = service.authenticateHttpRequest(requestDefaultAuthProvider, (HttpServletResponse) null); + assertTrue(doFilter, "Authentication should have succeeded"); + verify(requestDefaultAuthProvider).setAttribute(AuthenticatedRoleAttributeName, s_authentication_success); + + HttpServletRequest requestCustomAuthProvider = mock(HttpServletRequest.class); + when(requestCustomAuthProvider.getRemoteAddr()).thenReturn("192.168.1.1"); + when(requestCustomAuthProvider.getRemotePort()).thenReturn(8080); + when(requestCustomAuthProvider.getHeader("X-Pulsar-Auth-Method-Name")).thenReturn("customAuthProvider"); + doFilter = service.authenticateHttpRequest(requestCustomAuthProvider, (HttpServletResponse) null); + assertTrue(doFilter, "Authentication should have succeeded"); + verify(requestCustomAuthProvider).setAttribute(AuthenticatedRoleAttributeName, s_authentication_success); + + HttpServletRequest requestUnsupportedAuthProvider = mock(HttpServletRequest.class); + when(requestUnsupportedAuthProvider.getRemoteAddr()).thenReturn("192.168.1.1"); + when(requestUnsupportedAuthProvider.getRemotePort()).thenReturn(8080); + when(requestUnsupportedAuthProvider.getHeader("X-Pulsar-Auth-Method-Name")).thenReturn("unsupportedAuthProvider"); + Assert.assertThrows(() -> + service.authenticateHttpRequest(requestUnsupportedAuthProvider, (HttpServletResponse) null)); + + service.close(); + } + + @Test(timeOut = 10000) + public void testAuthenticationHttpRequestResponseWithAnonymousRole() throws Exception { + boolean doFilter; + String anonRole = "anon"; + ServiceConfiguration config = new ServiceConfiguration(); + Set providersClassNames = Sets.newHashSet(MockAuthenticationProviderAlwaysFail.class.getName()); + config.setAuthenticationProviders(providersClassNames); + config.setAuthenticationEnabled(true); + config.setAnonymousUserRole(anonRole); + AuthenticationService service = new AuthenticationService(config); + + // No auth header specified + HttpServletRequest requestCustomAuthProvider = mock(HttpServletRequest.class); + when(requestCustomAuthProvider.getRemoteAddr()).thenReturn("192.168.1.1"); + when(requestCustomAuthProvider.getRemotePort()).thenReturn(8080); + doFilter = service.authenticateHttpRequest(requestCustomAuthProvider, (HttpServletResponse) null); + assertTrue(doFilter, "Authentication should have succeeded"); + verify(requestCustomAuthProvider).setAttribute(AuthenticatedRoleAttributeName, anonRole); + + service.close(); + } + public static class MockAuthenticationProvider implements AuthenticationProvider { @Override @@ -124,4 +207,25 @@ public String authenticate(AuthenticationDataSource authData) throws Authenticat return s_authentication_success; } } + + public static class MockAuthenticationProviderAlwaysFail implements AuthenticationProvider { + + @Override + public void close() throws IOException { + } + + @Override + public void initialize(ServiceConfiguration config) throws IOException { + } + + @Override + public String getAuthMethodName() { + return "auth"; + } + + @Override + public String authenticate(AuthenticationDataSource authData) throws AuthenticationException { + throw new AuthenticationException("I failed"); + } + } }