Allow dropping tokens from the session manager for easier recovery on lookup failures

[macstewart]

For context, I'm using TokenAuthentication with LifecycleAwareSessionManager. The token used is a periodic token provided to the service at install-time.

To provide resilience against network blips during the LifecycleAwareSessionManager's renewal loop, I've set it to drop the token on error, and configured an event listener to respond to failure events by calling LifecycleAwareSessionManager::getSessionToken after a delay to restart the renewals.

This works well when failures happen during the renewal call. However, if there's a failure in LifecycleAwareSessionManager::doGetSessionToken in the try block that surrounds this line:

token = LoginTokenAdapter.augmentWithSelfLookup(this.restOperations, (VaultToken)token);

the token wrapper token is never upgraded to a LoginToken, which means the token is not considered renewable and the renewal loop will not start. In addition, this failure does not allow the token to be dropped, meaning that any further attempts to trigger a new login will just use the token stored in the wrapper. This token will still work in the short term but because the renewal loop is broken it will eventually quietly expire and break the application.

[macstewart]

A near-solution was to wrap the TokenAuthentication in a LoginTokenAdaptor, which makes the augmentWithSelfLookup part of the login step, meaning any self lookup failures happen before the token is cached in LifecycleAwareSessionManager. However, this creates the side effect that the provided token is considered revokable and is therefore invalidated when the application stops. This is a deal-breaker for a service which may need to be restarted without providing a new token each time.

The only real workaround I found for the issue was to add a call to LifecycleAwareSessionManager::destroy in my login step that is triggered by the failure event listener. This is really misusing the DisposableBean method implementation though and there's nothing to say that LifecycleAwareSessionManager's implementation of this method won't affect more than just the cached token in future versions.

[mp911de]

What would be a proper workaround? Rethrowing the exception would leave a potentially created token active, and we would need to revoke the token as outlined in your approach.

An alternative could be us providing a public revoke() method to drop and revoke the token, which would move the existing code from the destroy method into a proper one.

Let me know what you think.

[nbx-rosi]

If I may join this discussion:
Having a public revoke() method (or dropCurrentToken() as its called in the ReactiveLifecycleAwareSessionManager ) would be highly appreciated! That is actually what we are missing in our setting as well, where we are using Kubernetes Authentication which fails from time to time due to network issues and/or rotating master. Currently we setup a dirty workaround with reflection to call this, followed by a manual renewToken() call to trigger a new login.

To make things worse, we observed, that having "an event listener to respond to failure events*" is not enough in case of the SecretLeaseContainers which are used to load spring configuration, because they have their own listeners and all they do by default is logging... So we ended up configuring custom LeaseErrorListeners here as well.

The only reason stopping us from setting up our own issue here is, that we can't clearly point to one problem, but seem to face a combination of a few.

(*) assuming you are talking about AuthenticationErrorEvent

[macstewart]

On my end, the public revoke() is an acceptable resolution here. Thanks for looking into it!

[mp911de]

Thanks a lot for having a look. If you should run into other issues, let us know and we can work on these.