Skip to content
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

Merged
merged 2 commits into from
Aug 22, 2023

Conversation

injectives
Copy link
Contributor

@injectives injectives commented Aug 15, 2023

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.

@injectives injectives force-pushed the feature/auth branch 12 times, most recently from 238a7b2 to 56919fe Compare August 16, 2023 09:58
@injectives injectives changed the title [WIP] Auth update Update AuthToken rotation to support more auth types Aug 16, 2023
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.
Copy link
Contributor

@meistermeier meistermeier left a 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.

@injectives injectives merged commit c3b10e3 into neo4j:5.0 Aug 22, 2023
@injectives injectives deleted the feature/auth branch August 22, 2023 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants