nodece commented on code in PR #19519:
URL: https://github.com/apache/pulsar/pull/19519#discussion_r1106656201
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java:
##########
@@ -737,15 +737,15 @@ public void authChallengeSuccessCallback(AuthData
authChallenge,
// 2. an authentication refresh, in which case we need to
refresh authenticationData
AuthenticationState authState = useOriginalAuthState ?
originalAuthState : this.authState;
String newAuthRole = authState.getAuthRole();
+ AuthenticationDataSource newAuthDataSource =
authState.getAuthDataSource();
- // Refresh the auth data.
- this.authenticationData = authState.getAuthDataSource();
- if (log.isDebugEnabled()) {
- log.debug("[{}] Auth data refreshed for role={}",
remoteAddress, this.authRole);
- }
-
+ // Refresh the auth data and role.
if (!useOriginalAuthState) {
this.authRole = newAuthRole;
+ this.authenticationData = newAuthDataSource;
+ } else {
+ this.originalAuthData = newAuthDataSource;
+ this.originalPrincipal = newAuthRole;
Review Comment:
> My primary concern about this approach is that it doesn't address the fact
that the `authenticationData` will never be updated after the initial handshake
when a proxy is involved.
Sure, but I'm doing this to fix the store of the authentication data
correctly, and make sense.
> I have been thinking it might make sense to get rid of the
`originalAuthData` field entirely because when it is set, we won't ever get the
proxy's auth data. The drawback of my approach is that it also means we would
skip authorization of the proxy's role. I personally think this is reasonable
because we can verify that the proxy is in the `proxyRoles` initially and then
not worry about the actual permission later.
It makes sense to remove 'originalAuthData' to make the authentication
process simpler.
Thank you for your review.
--
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]
