From b74059038b406c9afb0bd2eb2f4b800cb817f363 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Wed, 15 Nov 2017 15:53:43 +0100 Subject: [PATCH] Consider login token TTL after renewal. We now consider the token TTL after renewal to calculate the next renewal time to prevent stale token use. Previously, we assumed the TTL to be the same as the initial TTL. This caused the token to render invalid for a period of time. We now also check the TTL after renewal whether a subsequent renewal run makes sense for the token. If the remaining TTL exceeds the minimum valid TTL we drop the token and re-login upon the next session token request. We do not revoke the token explicitly to not interrupt processes that obtained the token during the renewal period. Since the remaining TTL is rather short the token will silently expire. Closes gh-176. --- .../LifecycleAwareSessionManager.java | 93 +++++++++++++++++-- ...LifecycleAwareSessionManagerUnitTests.java | 62 +++++++++++++ 2 files changed, 146 insertions(+), 9 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 69ad95f4..6b7cecac 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 @@ -30,6 +30,7 @@ import org.apache.commons.logging.LogFactory; import org.springframework.beans.factory.DisposableBean; import org.springframework.core.task.AsyncTaskExecutor; import org.springframework.http.HttpEntity; +import org.springframework.lang.Nullable; import org.springframework.scheduling.TaskScheduler; import org.springframework.scheduling.Trigger; import org.springframework.scheduling.TriggerContext; @@ -38,6 +39,7 @@ import org.springframework.util.ClassUtils; import org.springframework.vault.VaultException; import org.springframework.vault.client.VaultHttpHeaders; import org.springframework.vault.client.VaultResponses; +import org.springframework.vault.support.VaultResponse; import org.springframework.vault.support.VaultToken; import org.springframework.web.client.HttpStatusCodeException; import org.springframework.web.client.RestClientException; @@ -228,14 +230,41 @@ public class LifecycleAwareSessionManager implements SessionManager, DisposableB logger.info("Renewing token"); + Optional token = this.token; if (!token.isPresent()) { getSessionToken(); return false; } + TokenWrapper wrapper = token.get(); + try { - restOperations.postForObject("auth/token/renew-self", new HttpEntity<>( - VaultHttpHeaders.from(token.get().getToken())), Map.class); + + VaultResponse vaultResponse = restOperations.postForObject( + "auth/token/renew-self", + new HttpEntity<>(VaultHttpHeaders.from(token.get().getToken())), + VaultResponse.class); + + LoginToken renewed = LoginTokenUtil.from(vaultResponse.getRequiredAuth()); + + Duration validTtlThreshold = refreshTrigger.getValidTtlThreshold(renewed); + if (renewed.getLeaseDuration().compareTo(validTtlThreshold) <= 0) { + + if (logger.isDebugEnabled()) { + logger.info(String + .format("Token TTL (%s) exceeded validity TTL threshold (%s). Dropping token.", + renewed.getLeaseDuration(), validTtlThreshold)); + } + else { + logger.info("Token TTL exceeded validity TTL threshold. Dropping token."); + } + + this.token = Optional.empty(); + return false; + } + + this.token = Optional.of(new TokenWrapper(renewed, wrapper.revocable)); + return true; } catch (HttpStatusCodeException e) { @@ -244,7 +273,7 @@ public class LifecycleAwareSessionManager implements SessionManager, DisposableB logger.debug(String .format("Cannot refresh token, resetting token and performing re-login: %s", VaultResponses.getError(e.getResponseBodyAsString()))); - token = Optional.empty(); + this.token = Optional.empty(); return false; } @@ -299,6 +328,9 @@ public class LifecycleAwareSessionManager implements SessionManager, DisposableB return clientAuthentication.login(); } + /** + * @return {@literal true} if the token is renewable. + */ protected boolean isTokenRenewable() { return token.map(TokenWrapper::getToken) @@ -333,12 +365,16 @@ public class LifecycleAwareSessionManager implements SessionManager, DisposableB } }; - taskScheduler.schedule(task, createTrigger()); + Optional token = this.token; + + token.ifPresent(tokenWrapper -> taskScheduler.schedule(task, + createTrigger(tokenWrapper))); } - private OneShotTrigger createTrigger() { - return new OneShotTrigger(refreshTrigger.nextExecutionTime((LoginToken) token - .map(TokenWrapper::getToken).get())); + private OneShotTrigger createTrigger(TokenWrapper tokenWrapper) { + + return new OneShotTrigger( + refreshTrigger.nextExecutionTime((LoginToken) tokenWrapper.getToken())); } /** @@ -352,6 +388,7 @@ public class LifecycleAwareSessionManager implements SessionManager, DisposableB private final Date nextExecutionTime; + @Nullable public Date nextExecutionTime(TriggerContext triggerContext) { if (fired.compareAndSet(false, true)) { @@ -364,17 +401,28 @@ public class LifecycleAwareSessionManager implements SessionManager, DisposableB /** * Common interface for trigger objects that determine the next execution time of a - * refresh task that they get associated with. + * refresh task. */ public interface RefreshTrigger { /** * Determine the next execution time according to the given trigger context. + * * @param loginToken login token encapsulating renewability and lease duration. * @return the next execution time as defined by the trigger, or {@code null} if * the trigger won't fire anymore */ Date nextExecutionTime(LoginToken loginToken); + + /** + * Returns the minimum TTL duration to consider a token valid after renewal. + * Tokens with a shorter TTL are revoked and considered expired. + * + * @param loginToken the login token after renewal. + * @return minimum TTL {@link Duration} to consider a token valid. + * @since 2.0 + */ + Duration getValidTtlThreshold(LoginToken loginToken); } /** @@ -389,10 +437,12 @@ public class LifecycleAwareSessionManager implements SessionManager, DisposableB private static final Duration ONE_SECOND = Duration.ofSeconds(1); private final Duration duration; + private final Duration validTtlThreshold; /** * Create a new {@link FixedTimeoutRefreshTrigger} to calculate execution times of * {@code timeout} before the {@link LoginToken} expires + * * @param timeout timeout value, non-negative long value. * @param timeUnit must not be {@literal null}. */ @@ -403,20 +453,40 @@ public class LifecycleAwareSessionManager implements SessionManager, DisposableB Assert.notNull(timeUnit, "TimeUnit must not be null"); this.duration = Duration.ofMillis(timeUnit.toMillis(timeout)); + this.validTtlThreshold = Duration.ofMillis(timeUnit.toMillis(timeout) + 2000); } /** * Create a new {@link FixedTimeoutRefreshTrigger} to calculate execution times of - * {@code timeout} before the {@link LoginToken} expires + * {@code timeout} before the {@link LoginToken} expires. Valid TTL threshold is + * set to two seconds longer to compensate for timing issues during scheduling. + * * @param timeout timeout value. * @since 2.0 */ public FixedTimeoutRefreshTrigger(Duration timeout) { + this(timeout, timeout.plus(Duration.ofSeconds(2))); + } + + /** + * Create a new {@link FixedTimeoutRefreshTrigger} to calculate execution times of + * {@code timeout} before the {@link LoginToken} expires. + * + * @param timeout timeout value. + * @param validTtlThreshold minimum TTL duration to consider a Token as valid. + * Tokens with a shorter TTL are not used anymore. Should be greater than + * {@code timeout} to prevent token expiry. + * @since 2.0 + */ + public FixedTimeoutRefreshTrigger(Duration timeout, Duration validTtlThreshold) { Assert.isTrue(timeout.toMillis() >= 0, "Timeout duration must be greater or equal to zero"); + Assert.notNull(validTtlThreshold, "Valid TTL threshold must not be null"); + this.duration = timeout; + this.validTtlThreshold = validTtlThreshold; } @Override @@ -427,6 +497,11 @@ public class LifecycleAwareSessionManager implements SessionManager, DisposableB return new Date(System.currentTimeMillis() + milliseconds); } + + @Override + public Duration getValidTtlThreshold(LoginToken loginToken) { + return validTtlThreshold; + } } /** 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 1bffab38..2906d31c 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 @@ -18,6 +18,8 @@ package org.springframework.vault.authentication; import java.time.Duration; import java.util.Collections; import java.util.Date; +import java.util.HashMap; +import java.util.Map; import java.util.concurrent.TimeUnit; import org.junit.Before; @@ -207,6 +209,10 @@ public class LifecycleAwareSessionManagerUnitTests { when(clientAuthentication.login()).thenReturn( LoginToken.renewable("login".toCharArray(), Duration.ofSeconds(5))); + when(restOperations.postForObject(anyString(), any(), eq(VaultResponse.class))) + .thenReturn( + fromToken(LoginToken.of("foo".toCharArray(), + Duration.ofSeconds(10)))); ArgumentCaptor runnableCaptor = ArgumentCaptor.forClass(Runnable.class); @@ -218,6 +224,48 @@ public class LifecycleAwareSessionManagerUnitTests { verify(taskScheduler, times(2)).schedule(any(Runnable.class), any(Trigger.class)); } + @Test + public void shouldNotScheduleRenewalIfRenewalTtlExceedsThreshold() { + + when(clientAuthentication.login()).thenReturn( + LoginToken.renewable("login".toCharArray(), Duration.ofSeconds(5))); + when(restOperations.postForObject(anyString(), any(), eq(VaultResponse.class))) + .thenReturn( + fromToken(LoginToken.of("foo".toCharArray(), + Duration.ofSeconds(2)))); + + ArgumentCaptor runnableCaptor = ArgumentCaptor.forClass(Runnable.class); + + sessionManager.getSessionToken(); + verify(taskScheduler).schedule(runnableCaptor.capture(), any(Trigger.class)); + + runnableCaptor.getValue().run(); + + verify(taskScheduler, times(1)).schedule(any(Runnable.class), any(Trigger.class)); + } + + @Test + public void shouldReLoginIfRenewalTtlExceedsThreshold() { + + when(clientAuthentication.login()).thenReturn( + LoginToken.renewable("login".toCharArray(), Duration.ofSeconds(5)), + LoginToken.renewable("bar".toCharArray(), Duration.ofSeconds(5))); + when(restOperations.postForObject(anyString(), any(), eq(VaultResponse.class))) + .thenReturn( + fromToken(LoginToken.of("foo".toCharArray(), + Duration.ofSeconds(2)))); + + ArgumentCaptor runnableCaptor = ArgumentCaptor.forClass(Runnable.class); + sessionManager.getSessionToken(); + verify(taskScheduler).schedule(runnableCaptor.capture(), any(Trigger.class)); + runnableCaptor.getValue().run(); + + assertThat(sessionManager.getSessionToken()).isEqualTo( + LoginToken.renewable("bar".toCharArray(), Duration.ofSeconds(5))); + + verify(clientAuthentication, times(2)).login(); + } + @Test public void shouldUseTaskScheduler() { @@ -310,4 +358,18 @@ public class LifecycleAwareSessionManagerUnitTests { new Date(System.currentTimeMillis() + TimeUnit.SECONDS.toMillis(0)), new Date(System.currentTimeMillis() + TimeUnit.SECONDS.toMillis(2))); } + + private static VaultResponse fromToken(LoginToken loginToken) { + + Map auth = new HashMap<>(); + + auth.put("client_token", loginToken.getToken()); + auth.put("renewable", loginToken.isRenewable()); + auth.put("lease_duration", loginToken.getLeaseDuration().getSeconds()); + + VaultResponse response = new VaultResponse(); + response.setAuth(auth); + + return response; + } }