rdhabalia commented on pull request #6782: URL: https://github.com/apache/pulsar/pull/6782#issuecomment-619296983
@shiv4289 > If authentication is non-transient, the auth token would get stored which has 2 implications: Umm.. If we serialize `AuthenticationToken` then it won't serialize token and store it instead it tries to serialize tokenSupplier and if supplier fetches token dynamically then there shouldn't be the issue. But in `AuthenticationToken` class it can be the issue as it keeps token in memory. [AuthenticationToken.java](https://github.com/apache/pulsar/blob/master/pulsar-client/src/main/java/org/apache/pulsar/client/impl/auth/AuthenticationToken.java#L41) is not serializble because `supplierToken` is not serializble. To address security issue and serialiable issue, can't we just mark `supplierToken` as a transient field and #4284 is anyway builds `Authentication` from auth-plugin classname and param. So, we can still have `Authentication` serializable and if any Auth-impl class has non-serializable variable then it can be transient and auth can be build using params. > I see the current patch keeps Authentication transient . Is this the final decision then? No, I was waiting for the reply with auth-classname which is not serializable. #4284 completely broke storm-adapter because user sets auths using ClientBuilder which doesn't set authPlugin/param into `ClientConfigurationData` and storm internally passes `ClientConfigurationData` to PulsarClient so, it always sees authentication null. I have added another commit to fix that issue: https://github.com/apache/pulsar/pull/6782/commits/92510e166ecbc17e78dd89ccae46049910c87c0c ---------------------------------------------------------------- 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]
