michaeljmarshall opened a new pull request, #19280: URL: https://github.com/apache/pulsar/pull/19280
PIP 97: https://github.com/apache/pulsar/issues/12105 ### Motivation In order to implement PIP 97, it is essential that we remove all blocking calls to `provider.authenticate(authData)`. Two places where we currently call the synchronous `authenticate` method are in the `OneStageAuthenticationState` and the `TokenAuthenticationState` class constructors. Since `authenticate` will be replaced with `authenticateAsync`, we won't be able to rely on authenticating the `authData` in the constructor. Further, it is inefficient to verify authentication data twice, which currently happens for `TokenAuthenticationState` in the `ProxyConnection` and the `ServerCnx`. This PR seeks to make explicit a paradigm that is already followed in the Pulsar code base. Specifically, that the contract for using an `AuthenticationState` class is as follows: 1. Initializing the class by calling {@link AuthenticationProvider#newAuthState(SocketAddress, SSLSession)} 2. Calling {@link #authenticate(AuthData)} in a loop until the result is null. 3. Calling {@link #getAuthRole()} and {@link #getAuthDataSource()} to use for authentication. 4. Calling {@link #refreshAuthentication()} when {@link #isExpired()} returns true. 5. GoTo step 3. When PIP 97 is complete, step 2 will replace `authenticate` with `authenticateAsync`. ### Modifications * Add `AuthenticationProvider` method: `newAuthState(SocketAddress remoteAddress, SSLSession sslSession)`. This method does not take `AuthData` to make it a bit clearer that class constructors are not meant to perform authentication. This method's default definition is `OneStageAuthenticationState`. Users that want a custom `AuthenticationProvider` to use a different `AuthenticationState` implementation must override this method. * Add `@Deprecated` annotation to `newAuthState(AuthData authData, SocketAddress remoteAddress, SSLSession sslSession)` and to several class constructors that were used by that method. Note: leave the implementations the same for broker extensions that use this method. This is the only backwards compatible portion of this PR. * Update `ProxyConnection` and `ServerCnx` to use the new `newAuthState` method. This change could break custom `AuthenticationState` implementations as described in the first bullet point. * Update `TokenAuthenticationState` and `OneStageAuthenticationState` to only authenticate tokens in the `authenticate` method. ### Verifying this change There are new tests and there is already some test coverage. ### Does this pull request potentially affect one of the following parts: - [x] The public API This change affects the `AuthenticationProvider` interface. ### Documentation - [x] `doc-not-needed` We document the authentication interfaces in the code, and I've added docs there. ### Matching PR in forked repository PR in forked repository: https://github.com/michaeljmarshall/pulsar/pull/14 -- 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]
