Skip to content

Commit

Permalink
[feat][ws] Use async auth method to support OIDC (apache#20238)
Browse files Browse the repository at this point in the history
Fixes apache#20236
PIP: apache#19409

### Motivation

In the `AuthenticationService`, we are currently using the deprecated `authenticate` methods. As a result, we hit the `Not Implemented` exception when using the `AuthenticationProviderOpenID`. This PR updates the implementation so that we're able

This solution isn't ideal for two reasons.

1. We are not using the `authenticationHttpRequest` method, which seems like the right method for the WebSocket proxy. However, this is not a viable option, as I documented in apache#20237.
2. We are calling `.get()` on a future. However, it is expected that the `AuthenticationProvider` not block forever, so I think this is acceptable for now. Please let me know if you disagree.

### Modifications

* Replace `authenticate` with `authenticateAsync`.

### Verifying this change

This change is a trivial rework / code cleanup without any test coverage.

### Documentation

- [x] `doc-not-needed`

Note that I do have documentation showing that 3.0.x does not support OIDC in the WebSocket Proxy. The `next` docs don't need that limitation since this PR fixes that and targets 3.1.0. apache/pulsar-site#558

### Matching PR in forked repository

PR in forked repository: skipping for this trivial PR

(cherry picked from commit 03dc3db)
  • Loading branch information
michaeljmarshall committed May 8, 2023
1 parent e2e2c4a commit 02d6c16
Showing 1 changed file with 10 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
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 Down Expand Up @@ -171,20 +172,26 @@ public String authenticateHttpRequest(HttpServletRequest request, Authentication
authData = authenticationState.getAuthDataSource();
}
// Backward compatible, the authData value was null in the previous implementation
return providerToUse.authenticate(authData);
return providerToUse.authenticateAsync(authData).get();
} catch (AuthenticationException 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.authenticate(authenticationState.getAuthDataSource());
} catch (AuthenticationException e) {
return provider.authenticateAsync(authenticationState.getAuthDataSource()).get();
} catch (ExecutionException | InterruptedException | AuthenticationException e) {
if (LOG.isDebugEnabled()) {
LOG.debug("Authentication failed for provider " + provider.getAuthMethodName() + ": "
+ e.getMessage(), e);
Expand Down

0 comments on commit 02d6c16

Please sign in to comment.