From a6ff9f3135a50c8ff2223b2e8feaa57acfdd8441 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Fri, 19 Nov 2021 11:53:11 +0100 Subject: [PATCH] Consider LeaseStrategy outcome whether to retry session token renewal. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit LeaseStrategy.shouldDrop(…) now controls whether to retry the session token. Dropping the token terminates renewals while retaining the token leads to another renewal. Additionally, we introduced LeaseStrategy.retainOnIoError() to retain tokens on network failures (IOException). Closes gh-646 --- .../LifecycleAwareSessionManager.java | 66 ++++++++++++------- .../ReactiveLifecycleAwareSessionManager.java | 22 ++----- .../vault/support/LeaseStrategy.java | 25 +++++++ ...leAwareSessionManagerSupportUnitTests.java | 2 +- ...LifecycleAwareSessionManagerUnitTests.java | 44 ++++++------- 5 files changed, 94 insertions(+), 65 deletions(-) diff --git a/spring-vault-core/src/main/java/org/springframework/vault/authentication/LifecycleAwareSessionManager.java b/spring-vault-core/src/main/java/org/springframework/vault/authentication/LifecycleAwareSessionManager.java index f308e4d8..c8e704aa 100644 --- a/spring-vault-core/src/main/java/org/springframework/vault/authentication/LifecycleAwareSessionManager.java +++ b/spring-vault-core/src/main/java/org/springframework/vault/authentication/LifecycleAwareSessionManager.java @@ -25,18 +25,7 @@ import org.springframework.scheduling.TaskScheduler; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; import org.springframework.vault.VaultException; -import org.springframework.vault.authentication.event.AfterLoginEvent; -import org.springframework.vault.authentication.event.AfterLoginTokenRenewedEvent; -import org.springframework.vault.authentication.event.AfterLoginTokenRevocationEvent; -import org.springframework.vault.authentication.event.AuthenticationErrorEvent; -import org.springframework.vault.authentication.event.AuthenticationErrorListener; -import org.springframework.vault.authentication.event.AuthenticationListener; -import org.springframework.vault.authentication.event.BeforeLoginTokenRenewedEvent; -import org.springframework.vault.authentication.event.BeforeLoginTokenRevocationEvent; -import org.springframework.vault.authentication.event.LoginFailedEvent; -import org.springframework.vault.authentication.event.LoginTokenExpiredEvent; -import org.springframework.vault.authentication.event.LoginTokenRenewalFailedEvent; -import org.springframework.vault.authentication.event.LoginTokenRevocationFailedEvent; +import org.springframework.vault.authentication.event.*; import org.springframework.vault.client.VaultHttpHeaders; import org.springframework.vault.client.VaultResponses; import org.springframework.vault.support.VaultResponse; @@ -192,13 +181,17 @@ public class LifecycleAwareSessionManager extends LifecycleAwareSessionManagerSu * token was obtained or refresh failed. */ public boolean renewToken() { + return tryRenewToken().successful; + } + + private RenewOutcome tryRenewToken() { this.logger.info("Renewing token"); Optional token = getToken(); if (!token.isPresent()) { getSessionToken(); - return false; + return RenewOutcome.TERMINAL_ERROR; } TokenWrapper tokenWrapper = token.get(); @@ -209,7 +202,8 @@ public class LifecycleAwareSessionManager extends LifecycleAwareSessionManagerSu VaultTokenRenewalException exception = new VaultTokenRenewalException(format("Cannot renew token", e), e); - if (getLeaseStrategy().shouldDrop(exception)) { + boolean shouldDrop = getLeaseStrategy().shouldDrop(exception); + if (shouldDrop) { setToken(Optional.empty()); } @@ -221,11 +215,11 @@ public class LifecycleAwareSessionManager extends LifecycleAwareSessionManagerSu } dispatch(new LoginTokenRenewalFailedEvent(tokenWrapper.getToken(), exception)); - return false; + return shouldDrop ? RenewOutcome.TERMINAL_ERROR : RenewOutcome.RENEWABLE_ERROR; } } - private boolean doRenew(TokenWrapper wrapper) { + private RenewOutcome doRenew(TokenWrapper wrapper) { dispatch(new BeforeLoginTokenRenewedEvent(wrapper.getToken())); VaultResponse vaultResponse = this.restOperations.postForObject("auth/token/renew-self", @@ -246,13 +240,13 @@ public class LifecycleAwareSessionManager extends LifecycleAwareSessionManagerSu setToken(Optional.empty()); dispatch(new LoginTokenExpiredEvent(renewed)); - return false; + return RenewOutcome.TERMINAL_ERROR; } setToken(Optional.of(new TokenWrapper(renewed, wrapper.revocable))); dispatch(new AfterLoginTokenRenewedEvent(renewed)); - return true; + return RenewOutcome.SUCCESS; } @Override @@ -314,13 +308,11 @@ public class LifecycleAwareSessionManager extends LifecycleAwareSessionManagerSu */ protected boolean isTokenRenewable() { - return getToken().map(TokenWrapper::getToken).filter(LoginToken.class::isInstance) - // - .filter(it -> { + return getToken().map(TokenWrapper::getToken).filter(LoginToken.class::isInstance).filter(it -> { - LoginToken loginToken = (LoginToken) it; - return !loginToken.getLeaseDuration().isZero() && loginToken.isRenewable(); - }).isPresent(); + LoginToken loginToken = (LoginToken) it; + return !loginToken.getLeaseDuration().isZero() && loginToken.isRenewable(); + }).isPresent(); } private void scheduleRenewal() { @@ -338,7 +330,8 @@ public class LifecycleAwareSessionManager extends LifecycleAwareSessionManagerSu try { if (isTokenRenewable()) { - if (renewToken()) { + RenewOutcome result = tryRenewToken(); + if (result.shouldRenew()) { scheduleRenewal(); } } @@ -398,4 +391,27 @@ public class LifecycleAwareSessionManager extends LifecycleAwareSessionManagerSu } + static class RenewOutcome { + + private static final RenewOutcome SUCCESS = new RenewOutcome(false, true); + + private static final RenewOutcome TERMINAL_ERROR = new RenewOutcome(true, false); + + private static final RenewOutcome RENEWABLE_ERROR = new RenewOutcome(false, false); + + private final boolean terminalError; + + private final boolean successful; + + private RenewOutcome(boolean terminalError, boolean successful) { + this.terminalError = terminalError; + this.successful = successful; + } + + public boolean shouldRenew() { + return !terminalError; + } + + } + } diff --git a/spring-vault-core/src/main/java/org/springframework/vault/authentication/ReactiveLifecycleAwareSessionManager.java b/spring-vault-core/src/main/java/org/springframework/vault/authentication/ReactiveLifecycleAwareSessionManager.java index 8288cf47..48feda69 100644 --- a/spring-vault-core/src/main/java/org/springframework/vault/authentication/ReactiveLifecycleAwareSessionManager.java +++ b/spring-vault-core/src/main/java/org/springframework/vault/authentication/ReactiveLifecycleAwareSessionManager.java @@ -27,18 +27,7 @@ import org.springframework.scheduling.TaskScheduler; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; import org.springframework.vault.VaultException; -import org.springframework.vault.authentication.event.AfterLoginEvent; -import org.springframework.vault.authentication.event.AfterLoginTokenRenewedEvent; -import org.springframework.vault.authentication.event.AfterLoginTokenRevocationEvent; -import org.springframework.vault.authentication.event.AuthenticationErrorEvent; -import org.springframework.vault.authentication.event.AuthenticationErrorListener; -import org.springframework.vault.authentication.event.AuthenticationListener; -import org.springframework.vault.authentication.event.BeforeLoginTokenRenewedEvent; -import org.springframework.vault.authentication.event.BeforeLoginTokenRevocationEvent; -import org.springframework.vault.authentication.event.LoginFailedEvent; -import org.springframework.vault.authentication.event.LoginTokenExpiredEvent; -import org.springframework.vault.authentication.event.LoginTokenRenewalFailedEvent; -import org.springframework.vault.authentication.event.LoginTokenRevocationFailedEvent; +import org.springframework.vault.authentication.event.*; import org.springframework.vault.client.VaultHttpHeaders; import org.springframework.vault.client.VaultResponses; import org.springframework.vault.support.VaultResponse; @@ -226,7 +215,8 @@ public class ReactiveLifecycleAwareSessionManager extends LifecycleAwareSessionM VaultTokenRenewalException exception = new VaultTokenRenewalException(format("Cannot renew token", e), e); - if (getLeaseStrategy().shouldDrop(exception)) { + boolean shouldDrop = getLeaseStrategy().shouldDrop(exception); + if (shouldDrop) { dropCurrentToken(); } @@ -238,10 +228,8 @@ public class ReactiveLifecycleAwareSessionManager extends LifecycleAwareSessionM } dispatch(new LoginTokenRenewalFailedEvent(wrapper.getToken(), exception)); - return EMPTY; - } - - ); + return shouldDrop ? EMPTY : Mono.just(wrapper); + }); } private Mono doRenew(TokenWrapper tokenWrapper) { diff --git a/spring-vault-core/src/main/java/org/springframework/vault/support/LeaseStrategy.java b/spring-vault-core/src/main/java/org/springframework/vault/support/LeaseStrategy.java index e2dab28c..211e92a9 100644 --- a/spring-vault-core/src/main/java/org/springframework/vault/support/LeaseStrategy.java +++ b/spring-vault-core/src/main/java/org/springframework/vault/support/LeaseStrategy.java @@ -15,6 +15,8 @@ */ package org.springframework.vault.support; +import java.io.IOException; + /** * Strategy interface to control whether to retain or drop a * {@link org.springframework.vault.core.lease.domain.Lease} after a failure. @@ -50,4 +52,27 @@ public interface LeaseStrategy { return error -> false; } + /** + * Predefined strategy to retain leases on I/O errors. + * @return the retain on I/O error strategy. + * @since 2.3.3 + */ + static LeaseStrategy retainOnIoError() { + return error -> { + + Throwable inspect = error; + + do { + if (inspect instanceof IOException) { + return false; + } + + inspect = inspect.getCause(); + } + while (inspect != null); + + return true; + }; + } + } diff --git a/spring-vault-core/src/test/java/org/springframework/vault/authentication/LifecycleAwareSessionManagerSupportUnitTests.java b/spring-vault-core/src/test/java/org/springframework/vault/authentication/LifecycleAwareSessionManagerSupportUnitTests.java index 1d2516bd..1f4b2c8a 100644 --- a/spring-vault-core/src/test/java/org/springframework/vault/authentication/LifecycleAwareSessionManagerSupportUnitTests.java +++ b/spring-vault-core/src/test/java/org/springframework/vault/authentication/LifecycleAwareSessionManagerSupportUnitTests.java @@ -23,7 +23,7 @@ import org.junit.jupiter.api.Test; import org.springframework.vault.authentication.LifecycleAwareSessionManagerSupport.FixedTimeoutRefreshTrigger; -import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.*; /** * Unit tests for {@link LifecycleAwareSessionManagerSupport} . diff --git a/spring-vault-core/src/test/java/org/springframework/vault/authentication/LifecycleAwareSessionManagerUnitTests.java b/spring-vault-core/src/test/java/org/springframework/vault/authentication/LifecycleAwareSessionManagerUnitTests.java index 253c2720..e2b6003e 100644 --- a/spring-vault-core/src/test/java/org/springframework/vault/authentication/LifecycleAwareSessionManagerUnitTests.java +++ b/spring-vault-core/src/test/java/org/springframework/vault/authentication/LifecycleAwareSessionManagerUnitTests.java @@ -21,6 +21,8 @@ import java.util.HashMap; import java.util.Map; import java.util.concurrent.atomic.AtomicReference; +import javax.net.ssl.SSLException; + import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -36,18 +38,7 @@ import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.scheduling.TaskScheduler; import org.springframework.scheduling.Trigger; -import org.springframework.vault.authentication.event.AfterLoginEvent; -import org.springframework.vault.authentication.event.AfterLoginTokenRenewedEvent; -import org.springframework.vault.authentication.event.AfterLoginTokenRevocationEvent; -import org.springframework.vault.authentication.event.AuthenticationErrorEvent; -import org.springframework.vault.authentication.event.AuthenticationErrorListener; -import org.springframework.vault.authentication.event.AuthenticationEvent; -import org.springframework.vault.authentication.event.AuthenticationListener; -import org.springframework.vault.authentication.event.BeforeLoginTokenRenewedEvent; -import org.springframework.vault.authentication.event.BeforeLoginTokenRevocationEvent; -import org.springframework.vault.authentication.event.LoginFailedEvent; -import org.springframework.vault.authentication.event.LoginTokenExpiredEvent; -import org.springframework.vault.authentication.event.LoginTokenRevocationFailedEvent; +import org.springframework.vault.authentication.event.*; import org.springframework.vault.client.VaultHttpHeaders; import org.springframework.vault.support.LeaseStrategy; import org.springframework.vault.support.VaultResponse; @@ -57,16 +48,9 @@ import org.springframework.web.client.HttpServerErrorException; import org.springframework.web.client.ResourceAccessException; import org.springframework.web.client.RestOperations; -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatExceptionOfType; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoMoreInteractions; -import static org.mockito.Mockito.verifyZeroInteractions; -import static org.mockito.Mockito.when; +import static org.assertj.core.api.Assertions.*; +import static org.mockito.ArgumentMatchers.*; +import static org.mockito.Mockito.*; /** * Unit tests for {@link LifecycleAwareSessionManager}. @@ -433,6 +417,22 @@ class LifecycleAwareSessionManagerUnitTests { verify(this.clientAuthentication, times(1)).login(); } + @Test + void renewShouldRetainTokenOnIoError() { + + when(this.clientAuthentication.login()) + .thenReturn(LoginToken.renewable("login".toCharArray(), Duration.ofSeconds(5))); + when(this.restOperations.postForObject(anyString(), ArgumentMatchers.any(), ArgumentMatchers.any())) + .thenThrow(new ResourceAccessException("err", new SSLException("foo"))); + + this.sessionManager.setLeaseStrategy(LeaseStrategy.retainOnIoError()); + this.sessionManager.getSessionToken(); + + assertThat(this.sessionManager.renewToken()).isFalse(); + assertThat(this.sessionManager.getToken()).isNotEmpty(); + verify(this.clientAuthentication, times(1)).login(); + } + private static VaultResponse fromToken(LoginToken loginToken) { Map auth = new HashMap<>();