This is an automated email from the ASF dual-hosted git repository.
mmarshall pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pulsar.git
The following commit(s) were added to refs/heads/master by this push:
new 03dc3db0a63 [feat][ws] Use async auth method to support OIDC (#20238)
03dc3db0a63 is described below
commit 03dc3db0a6357dcde294e55c1933bd13275f3f73
Author: Michael Marshall <[email protected]>
AuthorDate: Mon May 8 14:15:32 2023 -0500
[feat][ws] Use async auth method to support OIDC (#20238)
Fixes #20236
PIP: #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 #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. https://github.com/apache/pulsar-site/pull/558
### Matching PR in forked repository
PR in forked repository: skipping for this trivial PR
---
.../pulsar/broker/authentication/AuthenticationService.java | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
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 d11bb6d76e8..22296b86b4e 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
@@ -27,6 +27,7 @@ import java.util.HashMap;
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;
@@ -171,20 +172,26 @@ public class AuthenticationService implements Closeable {
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);
