michaeljmarshall commented on code in PR #18130:
URL: https://github.com/apache/pulsar/pull/18130#discussion_r1081872348
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java:
##########
@@ -707,73 +709,96 @@ private void completeConnect(int clientProtoVersion,
String clientVersion, boole
}
// According to auth result, send newConnected or newAuthChallenge command.
- private State doAuthentication(AuthData clientData,
- int clientProtocolVersion,
- String clientVersion) throws Exception {
-
+ private CompletableFuture<Void> doAuthenticationAsync(AuthData clientData,
int clientProtocolVersion,
+ String
clientVersion) {
// The original auth state can only be set on subsequent auth attempts
(and only
// in presence of a proxy and if the proxy is forwarding the
credentials).
// In this case, the re-validation needs to be done against the
original client
- // credentials.
- boolean useOriginalAuthState = (originalAuthState != null);
- AuthenticationState authState = useOriginalAuthState ?
originalAuthState : this.authState;
- String authRole = useOriginalAuthState ? originalPrincipal :
this.authRole;
- AuthData brokerData = authState.authenticate(clientData);
-
- if (log.isDebugEnabled()) {
- log.debug("Authenticate using original auth state : {}, role =
{}", useOriginalAuthState, authRole);
- }
+ // credentials, but we only can new an authentication state, because
some authentication data(TLS, SASL)
+ // based on outside service.
+ // If we can get the role from the authentication sate, the global
variable need to be updated.
- if (authState.isComplete()) {
- // Authentication has completed. It was either:
- // 1. the 1st time the authentication process was done, in which
case we'll send
- // a `CommandConnected` response
- // 2. an authentication refresh, in which case we need to refresh
authenticationData
-
- String newAuthRole = authState.getAuthRole();
-
- // Refresh the auth data.
- this.authenticationData = authState.getAuthDataSource();
Review Comment:
> > When the proxy is forwarding authentication data, it seems sufficient to
verify that the proxy's authentication data is valid for a proxy as a one time
check during connection initialization. All subsequent auth challenges will go
to the client anyway.
>
> This is `ProxyConnectionToBroker` operation, we also consider
`ProxyLookupRequests` logic.
After https://github.com/apache/pulsar/pull/17831, when a proxy connection
is in state `ProxyLookupRequests` with `forwardAuthorizationCredentials=true`,
the proxy connection responds to the `AuthChallenge` with the original client's
auth data by forwarding the broker's challenge to the client. That is why I
said the auth challenges go to the client in that case.
> Would you like to pass only the proxy's authentication data? If yes,
that's going to cross the line.
No, that's not my suggestion. I am suggesting that when the proxy sends both
its `authData` and the client's `authData`, the broker needs to only verify the
proxy's `authData` at the beginning and at no other time because in all of
those cases, the broker's auth challenge is going back to the original client
and the proxy's `authData` is never again updated. This is the case because the
proxy only forwards the client's auth data when
`forwardAuthorizationCredentials=true`. One issue with my proposed solution is
that the solution introduced in https://github.com/apache/pulsar/pull/17831 is
not present in older versions of the proxy, so we'd need a way to address those
proxies.
--
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]
