merlimat commented on a change in pull request #6074: PIP-55: Refresh
Authentication Credentials
URL: https://github.com/apache/pulsar/pull/6074#discussion_r375081645
##########
File path:
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java
##########
@@ -497,8 +523,60 @@ private void doAuthentication(AuthData clientData,
log.debug("[{}] Authentication in progress client by method {}.",
remoteAddress, authMethod);
}
- state = State.Connecting;
- return;
+ return State.Connecting;
+ }
+
+ public void refreshAuthenticationCredentials() {
+ if (getState() != State.Connected || !isActive) {
+ // Connection is either still being established or already closed.
+ return;
+ }
+
+ AuthenticationState authState = this.originalAuthState != null ?
originalAuthState : this.authState;
+ if (authState != null && !authState.isExpired()) {
+ // Credentials are still valid. Nothing to do at this point
+ return;
+ }
+
+ if (originalPrincipal != null && originalAuthState == null) {
+ log.info(
+ "[{}] Cannot revalidate user credential when using proxy
and not forwarding the credentials. Closing connection",
+ remoteAddress);
+ return;
+ }
+
+ ctx.executor().execute(SafeRun.safeRun(() -> {
+ log.info("[{}] Refreshing authentication credentials",
remoteAddress);
+
+ if (!supportsAuthenticationRefresh()) {
+ log.warn("[{}] Closing connection because client doesn't
support auth credentials refresh", remoteAddress);
+ ctx.close();
Review comment:
> if the client library is not upgraded then it closes the connection and
disrupts all producers connection.
This is only if the credentials are expiring and if the authentication
provider enforces that (which right now will only be the token auth provider).
Avoiding to challenge older client defeats the purpose of this feature in
that it opens a backdoor to completely avoid the check.
If older clients are a concern, I'd suggest to either:
* not use this feature (or similarly working feature)
* use non-expiring credentials for older clients
* use credentials with long expire time to minimize the number of
reconnections
* force clients to upgrade version
I honestly don't see any better solution for that. I'd categorically exclude
the "ignore old clients" for the false sense of security and the massive sec
issue it opens.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
