From a852e448106e8edbfe85c4fb27c964056b7192b8 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 | 88 +++++++++++++++++-- ...LifecycleAwareSessionManagerUnitTests.java | 57 ++++++++++++ 2 files changed, 138 insertions(+), 7 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 357c8f81..feb768ce 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 @@ -34,6 +34,7 @@ import org.springframework.util.Assert; 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; @@ -159,14 +160,37 @@ public class LifecycleAwareSessionManager implements SessionManager, DisposableB logger.info("Renewing token"); + VaultToken token = this.token; if (token == null) { getSessionToken(); return false; } try { - restOperations.postForObject("auth/token/renew-self", - new HttpEntity(VaultHttpHeaders.from(token)), Map.class); + VaultResponse vaultResponse = restOperations.postForObject( + "auth/token/renew-self", + new HttpEntity(VaultHttpHeaders.from(token)), + VaultResponse.class); + LoginToken renewed = LoginTokenUtil.from(vaultResponse.getAuth()); + + long validTtlThreshold = refreshTrigger.getValidTtlThreshold(renewed); + if (renewed.getLeaseDuration() <= TimeUnit.MILLISECONDS + .toSeconds(validTtlThreshold)) { + + 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 = null; + return false; + } + + this.token = renewed; return true; } catch (HttpStatusCodeException e) { @@ -175,7 +199,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 = null; + this.token = null; return false; } @@ -210,6 +234,9 @@ public class LifecycleAwareSessionManager implements SessionManager, DisposableB return clientAuthentication.login(); } + /** + * @return {@literal true} if the token is renewable. + */ protected boolean isTokenRenewable() { if (token instanceof LoginToken) { @@ -242,10 +269,15 @@ public class LifecycleAwareSessionManager implements SessionManager, DisposableB } }; - taskScheduler.schedule(task, createTrigger()); + VaultToken token = this.token; + + if (token != null) { + + taskScheduler.schedule(task, createTrigger(token)); + } } - private OneShotTrigger createTrigger() { + private OneShotTrigger createTrigger(VaultToken token) { return new OneShotTrigger(refreshTrigger.nextExecutionTime((LoginToken) token)); } @@ -272,17 +304,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 duration in milliseconds to consider a token valid. + * @since 1.1.1 + */ + long getValidTtlThreshold(LoginToken loginToken); } /** @@ -295,12 +338,14 @@ public class LifecycleAwareSessionManager implements SessionManager, DisposableB public static class FixedTimeoutRefreshTrigger implements RefreshTrigger { private final long duration; + private final long validTtlThreshold; private final TimeUnit timeUnit; /** * Create a new {@link FixedTimeoutRefreshTrigger} to calculate execution times of - * {@code timeout} before the {@link LoginToken} expires + * {@code timeout} before the {@link LoginToken} expires. + * * @param timeout timeout value, non-negative long value. * @param timeUnit must not be {@literal null}. */ @@ -311,6 +356,30 @@ public class LifecycleAwareSessionManager implements SessionManager, DisposableB Assert.notNull(timeUnit, "TimeUnit must not be null"); this.duration = timeout; + this.validTtlThreshold = timeUnit.toMillis(duration) + 2000; + this.timeUnit = timeUnit; + } + + /** + * 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 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. + * @param timeUnit must not be {@literal null}. + * @since 1.1.1 + */ + public FixedTimeoutRefreshTrigger(long timeout, long validTtlThreshold, + TimeUnit timeUnit) { + + Assert.isTrue(timeout >= 0, + "Timeout duration must be greater or equal to zero"); + Assert.notNull(timeUnit, "TimeUnit must not be null"); + + this.duration = timeout; + this.validTtlThreshold = timeUnit.toMillis(validTtlThreshold); this.timeUnit = timeUnit; } @@ -324,5 +393,10 @@ public class LifecycleAwareSessionManager implements SessionManager, DisposableB return new Date(System.currentTimeMillis() + milliseconds); } + + @Override + public long 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 f219d11d..6452b882 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 @@ -16,6 +16,8 @@ package org.springframework.vault.authentication; import java.util.Date; +import java.util.HashMap; +import java.util.Map; import java.util.concurrent.TimeUnit; import org.junit.Before; @@ -32,6 +34,7 @@ import org.springframework.scheduling.TaskScheduler; import org.springframework.scheduling.Trigger; import org.springframework.vault.authentication.LifecycleAwareSessionManager.FixedTimeoutRefreshTrigger; import org.springframework.vault.client.VaultHttpHeaders; +import org.springframework.vault.support.VaultResponse; import org.springframework.vault.support.VaultToken; import org.springframework.web.client.HttpServerErrorException; import org.springframework.web.client.RestOperations; @@ -157,6 +160,8 @@ public class LifecycleAwareSessionManagerUnitTests { public void shouldReScheduleTokenRenewalAfterSucessfulRenewal() { when(clientAuthentication.login()).thenReturn(LoginToken.renewable("login", 5)); + when(restOperations.postForObject(anyString(), any(), eq(VaultResponse.class))) + .thenReturn(fromToken(LoginToken.of("foo".toCharArray(), 10))); ArgumentCaptor runnableCaptor = ArgumentCaptor.forClass(Runnable.class); @@ -168,6 +173,44 @@ 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(), 5)); + when(restOperations.postForObject(anyString(), any(), eq(VaultResponse.class))) + .thenReturn(fromToken(LoginToken.of("foo".toCharArray(), 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(), 5), + LoginToken.renewable("bar".toCharArray(), 5)); + when(restOperations.postForObject(anyString(), any(), eq(VaultResponse.class))) + .thenReturn(fromToken(LoginToken.of("foo".toCharArray(), 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(), 5)); + + verify(clientAuthentication, times(2)).login(); + } + @Test public void shouldUseTaskScheduler() { @@ -254,4 +297,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()); + + VaultResponse response = new VaultResponse(); + response.setAuth(auth); + + return response; + } }