BewareMyPower commented on code in PR #19295:
URL: https://github.com/apache/pulsar/pull/19295#discussion_r1141256918
##########
pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/OneStageAuthenticationState.java:
##########
@@ -20,39 +20,60 @@
import static java.nio.charset.StandardCharsets.UTF_8;
import java.net.SocketAddress;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutionException;
import javax.naming.AuthenticationException;
import javax.net.ssl.SSLSession;
import javax.servlet.http.HttpServletRequest;
import org.apache.pulsar.common.api.AuthData;
/**
- * Interface for authentication state.
- *
- * It tell broker whether the authentication is completed or not,
- * if completed, what is the AuthRole is.
+ * A class to track single stage authentication. This class assumes that:
+ * 1. {@link #authenticateAsync(AuthData)} is called once and when the {@link
CompletableFuture} completes,
+ * authentication is complete.
+ * 2. Authentication does not expire, so {@link #isExpired()} always returns
false.
+ * <p>
+ * See {@link AuthenticationState} for Pulsar's contract on how this interface
is used by Pulsar.
*/
public class OneStageAuthenticationState implements AuthenticationState {
- private final AuthenticationDataSource authenticationDataSource;
- private final String authRole;
+ private AuthenticationDataSource authenticationDataSource;
+ private final SocketAddress remoteAddress;
+ private final SSLSession sslSession;
+ private final AuthenticationProvider provider;
+ private volatile String authRole;
+
+ /**
+ * Constructor for a {@link OneStageAuthenticationState} where there is no
authentication performed during
+ * initialization.
+ * @param remoteAddress - remoteAddress associated with the {@link
AuthenticationState}
+ * @param sslSession - sslSession associated with the {@link
AuthenticationState}
+ * @param provider - {@link AuthenticationProvider} to use to verify
{@link AuthData}
+ */
public OneStageAuthenticationState(AuthData authData,
SocketAddress remoteAddress,
SSLSession sslSession,
- AuthenticationProvider provider) throws
AuthenticationException {
- this.authenticationDataSource = new AuthenticationDataCommand(
- new String(authData.getBytes(), UTF_8), remoteAddress, sslSession);
- this.authRole = provider.authenticate(authenticationDataSource);
+ AuthenticationProvider provider) {
+ this.provider = provider;
+ this.remoteAddress = remoteAddress;
+ this.sslSession = sslSession;
}
- public OneStageAuthenticationState(HttpServletRequest request,
AuthenticationProvider provider)
- throws AuthenticationException {
+ public OneStageAuthenticationState(HttpServletRequest request,
AuthenticationProvider provider) {
+ // Must initialize this here for backwards compatibility with http
authentication
this.authenticationDataSource = new AuthenticationDataHttps(request);
- this.authRole = provider.authenticate(authenticationDataSource);
+ this.provider = provider;
+ // These are not used when invoking this constructor.
+ this.remoteAddress = null;
+ this.sslSession = null;
}
@Override
- public String getAuthRole() {
+ public String getAuthRole() throws AuthenticationException {
+ if (authRole == null) {
+ throw new AuthenticationException("Must authenticate before
calling getAuthRole");
+ }
Review Comment:
> but the issue is that custom AuthenticationProvider implementations that
override the current newAuthState method likely don't want to use the
OneStageAuthenticationState, and that would lead to unexpected behavior.
Could you show an example of a possible unexpected behavior? IMO, the
existing authentication provider only overrides the previous `newAuthState`
method still has the previous behavior and is not affected by the new
`newAuthState` method.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
