michaeljmarshall opened a new pull request #12068: URL: https://github.com/apache/pulsar/pull/12068
### Motivation Remove `AuthenticationProviderSasl`'s overridden method declaration for `authenticate`. The default implementation will be used, which always throws an `AuthenticationException`. I remove it first because the `AuthenticationProvider` interface's Javadoc indicates that the method should not be implemented in the case of multi-stage authentication providers. In those cases, the authentication provider ought to implement the `AuthenticationState` class, which the `pulsar-broker-auth-sasl` module does do (`AuthenticationProviderSasl`). Here is a reference to the Javadoc: https://github.com/apache/pulsar/blob/23ffdb778e25169bfc7343920912521b41bf95f5/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProvider.java#L54-L67 Second, this `authenticate` method is never called successfully. By removing it, it should make the implementation a bit easier to understand. Here is an enumeration of the method calls to `AuthenticationProvider#authenticate`: 1. https://github.com/apache/pulsar/blob/23ffdb778e25169bfc7343920912521b41bf95f5/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationService.java#L92 2. https://github.com/apache/pulsar/blob/23ffdb778e25169bfc7343920912521b41bf95f5/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationService.java#L113 3. https://github.com/apache/pulsar/blob/23ffdb778e25169bfc7343920912521b41bf95f5/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationService.java#L125 4. https://github.com/apache/pulsar/blob/23ffdb778e25169bfc7343920912521b41bf95f5/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/OneStageAuthenticationState.java#L46 5. https://github.com/apache/pulsar/blob/23ffdb778e25169bfc7343920912521b41bf95f5/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/authentication/AuthenticationProviderList.java#L159 In case 1 above, the calling method `AuthenticationService#authenticate` is only called from the `DiscoveryService`'s class. This class does not create an `SaslAuthenticationDataSource`, so the call will fail. In cases 2 and 3 above, the `AuthenticationDataSource` that is passed in is always of type `AuthenticationDataHttps`, so the call will fail. In case 4 above, that is used in `OneStageAuthenticationState`, so it is never called when using sasl. In çase 5 above, the `AuthenticationProviderList` is recursively implementing the `authenticate` method, so it is covered by cases 1 through 4. ### Modifications * Remove overriding of `authenticate` method in the sasl authentication provider implementation. * Add Javadoc explaining why it is not overridden. ### Verifying this change * Tests pass for the `pulsar-broker-auth-sasl` module * Since this change does not _actually_ change any behavior, I don't see any need to add more tests. ### Does this pull request potentially affect one of the following parts: No. ### Documentation This is an internal change. I did leave some comments for future reference. I don't believe any additional documentation is required. -- 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]
