-
Notifications
You must be signed in to change notification settings - Fork 157
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update AuthToken rotation to support more auth types #1481
Conversation
238a7b2
to
56919fe
Compare
56919fe
to
ad1ba27
Compare
This update extends the `AuthToken` rotation beyond supporting just the expiring (bearer) tokens to a more flexible solution that allows supporting various auth types. An important part of the `AuthToken` rotation support is retryability, which is either managed automatically by the driver via the Managed Transaction API (`Session.executeRead(TransactionCallback)`, `Session.executeWrite(TransactionCallback)`, etc.) or explicitly by the user. The rotation support relies on the existing retry approach used for the other retryable conditions. If a given unit of work fails due to credentials rejection by the server and the `AuthTokenManager` is able to supply valid credentials, the failure must be marked as retryable to indicate that the given unit of work is worth retrying. The driver provides the `RetryableException` marker interface for that. However, the credentials rejection server security error depends on auth type used. Therefore, it was decided that the `AuthTokenManager` implementations should have access to the security error details and should make a decision on whether such error should be considered as retryable or not. To achieve this, the `AuthTokenManager.onExpired(AuthToken)` method is replaced with a new `AuthTokenManager.handleSecurityException(AuthToken authToken, SecurityException exception)` method that returns a `boolean` value to determine if the error is retryable. By default, the `SecurityException` and its subclasses are not retryable. If an error is determined to be retryable by the `AuthTokenManager`, then the driver wraps the current error into a new `SecurityRetryableException`. The latter is a subclass of the `SecurityException` and is also a `RetryableException`. It contains the original exception as a cause and transparently proxies calls of both the `SecurityRetryableException.code()` and `SecurityRetryableException.getMessage()` methods to the original exception. The `SecurityRetryableException` has been introduced to allow marking `SecurityException` and its subclasses as retryable and is not currently meant to fork a separate class hierarchy as a single class is sufficient for this purpose at this point. Following these updates, the `TokenExpiredRetryableException` has been deleted as no longer needed. The `AuthTokenManagers.expirationBased(Supplier<AuthTokenAndExpiration>)` and `AuthTokenManagers.expirationBasedAsync(Supplier<CompletionStage<AuthTokenAndExpiration>>)` methods have been replaced with `AuthTokenManagers.bearer(Supplier<AuthTokenAndExpiration>)` and `AuthTokenManagers.bearerAsync(Supplier<CompletionStage<AuthTokenAndExpiration>>)` methods respectively. The new medhods are tailored for the bearer token auth support specifically. In addition, 2 new methods have been introduced for the basic (type) `AuthToken` rotation support: `AuthTokenManagers.basic(Supplier<AuthToken>)` and `AuthTokenManagers.basicAsync(Supplier<CompletionStage<AuthToken>>)`. The code inspection profile has been updated to enable the `SerializableHasSerialVersionUIDField` warning.
ad1ba27
to
9af810c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved, but I am still struggling with the boolean handleSecurityException
naming. I can see in the ValidatingAuthTokenManager
that it is not this easy to split this into boolean canHandle/isRetryable
and void handle/unsetToken/..
or calling the necessary code directly in the handleFailureMessage
.
This update extends the
AuthToken
rotation beyond supporting just the expiring (bearer) tokens to a more flexible solution that allows supporting various auth types.An important part of the
AuthToken
rotation support is retryability, which is either managed automatically by the driver via the Managed Transaction API (Session.executeRead(TransactionCallback)
,Session.executeWrite(TransactionCallback)
, etc.) or explicitly by the user. The rotation support relies on the existing retry approach used for the other retryable conditions. If a given unit of work fails due to credentials rejection by the server and theAuthTokenManager
is able to supply valid credentials, the failure must be marked as retryable to indicate that the given unit of work is worth retrying. The driver provides theRetryableException
marker interface for that.However, the credentials rejection server security error depends on auth type used. Therefore, it was decided that the
AuthTokenManager
implementations should have access to the security error details and should make a decision on whether such error should be considered as retryable or not. To achieve this, theAuthTokenManager.onExpired(AuthToken)
method is replaced with a newAuthTokenManager.handleSecurityException(AuthToken authToken, SecurityException exception)
method that returns aboolean
value to determine if the error is retryable.By default, the
SecurityException
and its subclasses are not retryable. If an error is determined to be retryable by theAuthTokenManager
, then the driver wraps the current error into a newSecurityRetryableException
. The latter is a subclass of theSecurityException
and is also aRetryableException
. It contains the original exception as a cause and transparently proxies calls of both theSecurityRetryableException.code()
andSecurityRetryableException.getMessage()
methods to the original exception. TheSecurityRetryableException
has been introduced to allow markingSecurityException
and its subclasses as retryable and is not currently meant to fork a separate class hierarchy as a single class is sufficient for this purpose at this point. Following these updates, theTokenExpiredRetryableException
has been deleted as no longer needed.The
AuthTokenManagers.expirationBased(Supplier<AuthTokenAndExpiration>)
andAuthTokenManagers.expirationBasedAsync(Supplier<CompletionStage<AuthTokenAndExpiration>>)
methods have been replaced withAuthTokenManagers.bearer(Supplier<AuthTokenAndExpiration>)
andAuthTokenManagers.bearerAsync(Supplier<CompletionStage<AuthTokenAndExpiration>>)
methods respectively. The new medhods are tailored for the bearer token auth support specifically. In addition, 2 new methods have been introduced for the basic (type)AuthToken
rotation support:AuthTokenManagers.basic(Supplier<AuthToken>)
andAuthTokenManagers.basicAsync(Supplier<CompletionStage<AuthToken>>)
.The code inspection profile has been updated to enable the
SerializableHasSerialVersionUIDField
warning.