Skip to content

Commit

Permalink
[fix][broker] Continue using the next provider for http authenticatio…
Browse files Browse the repository at this point in the history 
…n if one fails (#23842)

Signed-off-by: Zixuan Liu <nodeces@gmail.com>
(cherry picked from commit f1f65a5)
  • Loading branch information
@nodece
nodece committed Jan 14, 2025
1 parent 57a7e0a commit 3a8582a
Show file tree
Hide file tree
Showing 2 changed files with 133 additions and 15 deletions.
22 changes: 7 additions & 15 deletions ...r-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,10 @@
import java.io.Closeable;
import java.io.IOException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.ExecutionException;
import java.util.stream.Collectors;
import javax.naming.AuthenticationException;
import javax.servlet.http.HttpServletRequest;
Expand All @@ -48,13 +47,13 @@ public class AuthenticationService implements Closeable {
private static final Logger LOG = LoggerFactory.getLogger(AuthenticationService.class);
private final String anonymousUserRole;

private final Map<String, AuthenticationProvider> providers = new HashMap<>();
private final Map<String, AuthenticationProvider> providers = new LinkedHashMap<>();

public AuthenticationService(ServiceConfiguration conf) throws PulsarServerException {
anonymousUserRole = conf.getAnonymousUserRole();
if (conf.isAuthenticationEnabled()) {
try {
Map<String, List<AuthenticationProvider>> providerMap = new HashMap<>();
Map<String, List<AuthenticationProvider>> providerMap = new LinkedHashMap<>();
for (String className : conf.getAuthenticationProviders()) {
if (className.isEmpty()) {
continue;
Expand Down Expand Up @@ -121,7 +120,7 @@ public boolean authenticateHttpRequest(HttpServletRequest request, HttpServletRe
AuthenticationProvider providerToUse = getAuthProvider(authMethodName);
try {
return providerToUse.authenticateHttpRequest(request, response);
} catch (AuthenticationException e) {
} catch (Exception e) {
if (LOG.isDebugEnabled()) {
LOG.debug("Authentication failed for provider " + providerToUse.getAuthMethodName() + " : "
+ e.getMessage(), e);
Expand All @@ -132,7 +131,7 @@ public boolean authenticateHttpRequest(HttpServletRequest request, HttpServletRe
for (AuthenticationProvider provider : providers.values()) {
try {
return provider.authenticateHttpRequest(request, response);
} catch (AuthenticationException e) {
} catch (Exception e) {
if (LOG.isDebugEnabled()) {
LOG.debug("Authentication failed for provider " + provider.getAuthMethodName() + ": "
+ e.getMessage(), e);
Expand Down Expand Up @@ -173,25 +172,18 @@ public String authenticateHttpRequest(HttpServletRequest request, Authentication
}
// Backward compatible, the authData value was null in the previous implementation
return providerToUse.authenticateAsync(authData).get();
} catch (AuthenticationException e) {
} catch (Exception e) {
if (LOG.isDebugEnabled()) {
LOG.debug("Authentication failed for provider " + providerToUse.getAuthMethodName() + " : "
+ e.getMessage(), e);
}
throw e;
} catch (ExecutionException | InterruptedException e) {
if (LOG.isDebugEnabled()) {
LOG.debug("Authentication failed for provider " + providerToUse.getAuthMethodName() + " : "
+ e.getMessage(), e);
}
throw new RuntimeException(e);
}
} else {
for (AuthenticationProvider provider : providers.values()) {
try {
AuthenticationState authenticationState = provider.newHttpAuthState(request);
return provider.authenticateAsync(authenticationState.getAuthDataSource()).get();
} catch (ExecutionException | InterruptedException | AuthenticationException e) {
} catch (Exception e) {
if (LOG.isDebugEnabled()) {
LOG.debug("Authentication failed for provider " + provider.getAuthMethodName() + ": "
+ e.getMessage(), e);
Expand Down
126 changes: 126 additions & 0 deletions pulsar-broker/src/test/java/org/apache/pulsar/broker/auth/AuthenticationServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@

import static org.apache.pulsar.broker.web.AuthenticationFilter.AuthenticatedDataAttributeName;
import static org.apache.pulsar.broker.web.AuthenticationFilter.AuthenticatedRoleAttributeName;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.mock;
Expand All @@ -29,15 +31,22 @@
import static org.testng.Assert.assertTrue;
import com.google.common.collect.Sets;
import java.io.IOException;
import java.util.LinkedHashSet;
import java.util.Set;
import java.util.concurrent.CompletableFuture;
import javax.naming.AuthenticationException;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import lombok.Cleanup;
import org.apache.pulsar.broker.ServiceConfiguration;
import org.apache.pulsar.broker.authentication.AuthenticationDataCommand;
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;
import org.apache.pulsar.broker.authentication.AuthenticationState;
import org.apache.pulsar.broker.web.AuthenticationFilter;
import org.apache.pulsar.common.api.AuthData;
import org.testng.Assert;
import org.testng.annotations.Test;

Expand Down Expand Up @@ -166,6 +175,123 @@ public void testAuthenticationHttpRequestResponseWithAnonymousRole() throws Exce
service.close();
}

@Test
public void testHttpRequestWithMultipleProviders() throws Exception {
ServiceConfiguration config = new ServiceConfiguration();
Set<String> providersClassNames = new LinkedHashSet<>();
providersClassNames.add(MockAuthenticationProviderAlwaysFail.class.getName());
providersClassNames.add(MockHttpAuthenticationProvider.class.getName());
config.setAuthenticationProviders(providersClassNames);
config.setAuthenticationEnabled(true);
@Cleanup
AuthenticationService service = new AuthenticationService(config);

HttpServletRequest request = mock(HttpServletRequest.class);

when(request.getParameter("role")).thenReturn("success-role1");
assertTrue(service.authenticateHttpRequest(request, (HttpServletResponse) null));

when(request.getParameter("role")).thenReturn("");
assertThatThrownBy(() -> service.authenticateHttpRequest(request, (HttpServletResponse) null))
.isInstanceOf(AuthenticationException.class);

when(request.getParameter("role")).thenReturn("error-role1");
assertThatThrownBy(() -> service.authenticateHttpRequest(request, (HttpServletResponse) null))
.isInstanceOf(AuthenticationException.class);

when(request.getHeader(AuthenticationFilter.PULSAR_AUTH_METHOD_NAME)).thenReturn("http-auth");
assertThatThrownBy(() -> service.authenticateHttpRequest(request, (HttpServletResponse) null))
.isInstanceOf(RuntimeException.class);

HttpServletRequest requestForAuthenticationDataSource = mock(HttpServletRequest.class);
assertThatThrownBy(() -> service.authenticateHttpRequest(requestForAuthenticationDataSource,
(AuthenticationDataSource) null))
.isInstanceOf(AuthenticationException.class);

when(requestForAuthenticationDataSource.getParameter("role")).thenReturn("error-role2");
assertThatThrownBy(() -> service.authenticateHttpRequest(requestForAuthenticationDataSource,
(AuthenticationDataSource) null))
.isInstanceOf(AuthenticationException.class);

when(requestForAuthenticationDataSource.getParameter("role")).thenReturn("success-role2");
assertThat(service.authenticateHttpRequest(requestForAuthenticationDataSource,
(AuthenticationDataSource) null)).isEqualTo("role2");
}

public static class MockHttpAuthenticationProvider implements AuthenticationProvider {
@Override
public void close() throws IOException {
}

@Override
public void initialize(ServiceConfiguration config) throws IOException {
}

@Override
public String getAuthMethodName() {
return "http-auth";
}

private String getRole(HttpServletRequest request) {
String role = request.getParameter("role");
if (role != null) {
String[] s = role.split("-");
if (s.length == 2 && s[0].equals("success")) {
return s[1];
}
}
return null;
}

@Override
public boolean authenticateHttpRequest(HttpServletRequest request, HttpServletResponse response) {
String role = getRole(request);
if (role != null) {
return true;
}
throw new RuntimeException("test authentication failed");
}

@Override
public String authenticate(AuthenticationDataSource authData) throws AuthenticationException {
return authData.getCommandData();
}

@Override
public AuthenticationState newHttpAuthState(HttpServletRequest request) throws AuthenticationException {
String role = getRole(request);
if (role != null) {
return new AuthenticationState() {
@Override
public String getAuthRole() throws AuthenticationException {
return role;
}

@Override
public AuthData authenticate(AuthData authData) throws AuthenticationException {
return null;
}

@Override
public AuthenticationDataSource getAuthDataSource() {
return new AuthenticationDataCommand(role);
}

@Override
public boolean isComplete() {
return true;
}

@Override
public CompletableFuture<AuthData> authenticateAsync(AuthData authData) {
return AuthenticationState.super.authenticateAsync(authData);
}
};
}
throw new RuntimeException("new http auth failed");
}
}

public static class MockAuthenticationProvider implements AuthenticationProvider {

@Override
Expand Down

0 comments on commit 3a8582a

Please sign in to comment.