BewareMyPower commented on code in PR #19295:
URL: https://github.com/apache/pulsar/pull/19295#discussion_r1130341227
##########
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:
> Out of curiosity, what is the consequence of breaking source compatibility
by removing the exception from the method signature?
No. I just noticed this point when I reviewed again. After looking for some
documents and testing the ABI compatibility locally, it seems that removing the
exception signature does not affect the ABI compatibility.
--
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]
