From f5cd833af40dbf50aa776c8be94d726542bba4f8 Mon Sep 17 00:00:00 2001 From: Jay Modi Date: Mon, 15 May 2017 14:20:12 -0600 Subject: [PATCH] Use epoch millis for token expiration instead of ZonedDateTime (elastic/x-pack-elasticsearch#1417) For user tokens, we were storing the expiration time as a date with the date time format in the mappings. Occasionally, we would get CI failures due to parsing the date and having an invalid format. This change simplifies the user tokens to simply use an Instant and convert it to the epoch millis when we index the document. This eliminates the need for a date formatter and should ensure we no longer have issues with parsing the dates. Additionally, the TokenAuthIntegTests#testExpireMultipleTimes could fail if the token was expired but the approximation for the expiration time was after the current time. In order to resolve this we get the exact expiration time from the token and use that in the test. relates elastic/x-pack-elasticsearch#1255 Original commit: elastic/x-pack-elasticsearch@d4bf53e7bc1e9068559b823f7605cd4cf144e82c --- .../security/authc/ExpiredTokenRemover.java | 5 ++--- .../xpack/security/authc/TokenService.java | 18 ++++++++--------- .../xpack/security/authc/UserToken.java | 14 +++++-------- .../resources/security-index-template.json | 2 +- .../security/authc/TokenAuthIntegTests.java | 20 ++++++++++++++++--- .../xpack/security/authc/UserTokenTests.java | 5 ++--- 6 files changed, 35 insertions(+), 29 deletions(-) diff --git a/plugin/src/main/java/org/elasticsearch/xpack/security/authc/ExpiredTokenRemover.java b/plugin/src/main/java/org/elasticsearch/xpack/security/authc/ExpiredTokenRemover.java index a8ee5cc4b7b..5f6926d9399 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/security/authc/ExpiredTokenRemover.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/security/authc/ExpiredTokenRemover.java @@ -18,9 +18,8 @@ import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.threadpool.ThreadPool.Names; import org.elasticsearch.xpack.security.InternalClient; -import org.joda.time.DateTime; -import org.joda.time.DateTimeZone; +import java.time.Instant; import java.util.concurrent.atomic.AtomicBoolean; import static org.elasticsearch.action.support.TransportActions.isShardNotAvailableException; @@ -52,7 +51,7 @@ final class ExpiredTokenRemover extends AbstractRunnable { searchRequest.source() .query(QueryBuilders.boolQuery() .filter(QueryBuilders.termQuery("doc_type", TokenService.DOC_TYPE)) - .filter(QueryBuilders.rangeQuery("expiration_time").lte(DateTime.now(DateTimeZone.UTC)))); + .filter(QueryBuilders.rangeQuery("expiration_time").lte(Instant.now().toEpochMilli()))); client.execute(DeleteByQueryAction.INSTANCE, dbq, ActionListener.wrap(r -> markComplete(), e -> { if (isShardNotAvailableException(e) == false) { diff --git a/plugin/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java b/plugin/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java index b09b07761cd..8f4ba321a42 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java @@ -60,9 +60,8 @@ import java.security.NoSuchAlgorithmException; import java.security.SecureRandom; import java.security.spec.InvalidKeySpecException; import java.time.Clock; +import java.time.Instant; import java.time.ZoneOffset; -import java.time.ZonedDateTime; -import java.time.format.DateTimeFormatter; import java.util.Arrays; import java.util.Base64; import java.util.List; @@ -87,7 +86,6 @@ public final class TokenService extends AbstractComponent { private static final int IV_BYTES = 12; private static final int VERSION_BYTES = 4; private static final String ENCRYPTION_CIPHER = "AES/GCM/NoPadding"; - private static final DateTimeFormatter DEFAULT_DATE_PRINTER = DateTimeFormatter.ISO_DATE_TIME.withZone(ZoneOffset.UTC); private static final String EXPIRED_TOKEN_WWW_AUTH_VALUE = "Bearer realm=\"" + XPackPlugin.SECURITY + "\", error=\"invalid_token\", error_description=\"The access token expired\""; private static final String MALFORMED_TOKEN_WWW_AUTH_VALUE = "Bearer realm=\"" + XPackPlugin.SECURITY + @@ -170,7 +168,7 @@ public final class TokenService extends AbstractComponent { public UserToken createUserToken(Authentication authentication) throws IOException, GeneralSecurityException { ensureEnabled(); - final ZonedDateTime expiration = getExpirationTime(); + final Instant expiration = getExpirationTime(); return new UserToken(authentication, expiration); } @@ -186,7 +184,7 @@ public final class TokenService extends AbstractComponent { try { decodeToken(token, ActionListener.wrap(userToken -> { if (userToken != null) { - ZonedDateTime currentTime = clock.instant().atZone(ZoneOffset.UTC); + Instant currentTime = clock.instant(); if (currentTime.isAfter(userToken.getExpirationTime())) { // token expired listener.onFailure(expiredTokenException()); @@ -208,7 +206,7 @@ public final class TokenService extends AbstractComponent { } } - private void decodeToken(String token, ActionListener listener) throws IOException { + void decodeToken(String token, ActionListener listener) throws IOException { // We intentionally do not use try-with resources since we need to keep the stream open if we need to compute a key! StreamInput in = new InputStreamStreamInput( Base64.getDecoder().wrap(new ByteArrayInputStream(token.getBytes(StandardCharsets.UTF_8)))); @@ -267,14 +265,14 @@ public final class TokenService extends AbstractComponent { decodeToken(tokenString, ActionListener.wrap(userToken -> { if (userToken == null) { listener.onFailure(malformedTokenException()); - } else if (userToken.getExpirationTime().isBefore(clock.instant().atZone(ZoneOffset.UTC))) { + } else if (userToken.getExpirationTime().isBefore(clock.instant())) { // no need to invalidate - it's already expired listener.onResponse(false); } else { final String id = userToken.getId(); internalClient.prepareIndex(INDEX_NAME, TYPE, id) .setOpType(OpType.CREATE) - .setSource("doc_type", DOC_TYPE, "expiration_time", DEFAULT_DATE_PRINTER.format(getExpirationTime())) + .setSource("doc_type", DOC_TYPE, "expiration_time", getExpirationTime().toEpochMilli()) .setRefreshPolicy(RefreshPolicy.WAIT_UNTIL) .execute(new ActionListener() { @Override @@ -353,8 +351,8 @@ public final class TokenService extends AbstractComponent { return expirationDelay; } - private ZonedDateTime getExpirationTime() { - return clock.instant().plusSeconds(expirationDelay.getSeconds()).atZone(ZoneOffset.UTC); + private Instant getExpirationTime() { + return clock.instant().plusSeconds(expirationDelay.getSeconds()); } private void maybeStartTokenRemover() { diff --git a/plugin/src/main/java/org/elasticsearch/xpack/security/authc/UserToken.java b/plugin/src/main/java/org/elasticsearch/xpack/security/authc/UserToken.java index 3089aadd53c..ca97569da0a 100644 --- a/plugin/src/main/java/org/elasticsearch/xpack/security/authc/UserToken.java +++ b/plugin/src/main/java/org/elasticsearch/xpack/security/authc/UserToken.java @@ -13,8 +13,6 @@ import org.elasticsearch.common.io.stream.Writeable; import java.io.IOException; import java.time.Instant; -import java.time.ZoneId; -import java.time.ZonedDateTime; import java.util.Objects; /** @@ -31,12 +29,12 @@ public final class UserToken implements Writeable { private final Version version; private final String id; private final Authentication authentication; - private final ZonedDateTime expirationTime; + private final Instant expirationTime; /** * Create a new token with an autogenerated id */ - UserToken(Authentication authentication, ZonedDateTime expirationTime) { + UserToken(Authentication authentication, Instant expirationTime) { this.version = Version.CURRENT; this.id = UUIDs.base64UUID(); this.authentication = Objects.requireNonNull(authentication); @@ -50,17 +48,15 @@ public final class UserToken implements Writeable { this.version = input.getVersion(); this.id = input.readString(); this.authentication = new Authentication(input); - this.expirationTime = Instant.ofEpochSecond(input.readLong(), input.readInt()) - .atZone(ZoneId.of(input.readString())); + this.expirationTime = Instant.ofEpochSecond(input.readLong(), input.readInt()); } @Override public void writeTo(StreamOutput out) throws IOException { out.writeString(id); authentication.writeTo(out); - out.writeLong(expirationTime.toEpochSecond()); + out.writeLong(expirationTime.getEpochSecond()); out.writeInt(expirationTime.getNano()); - out.writeString(expirationTime.getZone().getId()); } /** @@ -73,7 +69,7 @@ public final class UserToken implements Writeable { /** * Get the expiration time */ - ZonedDateTime getExpirationTime() { + Instant getExpirationTime() { return expirationTime; } diff --git a/plugin/src/main/resources/security-index-template.json b/plugin/src/main/resources/security-index-template.json index d29b5ffd6a5..6acac33b688 100644 --- a/plugin/src/main/resources/security-index-template.json +++ b/plugin/src/main/resources/security-index-template.json @@ -138,7 +138,7 @@ }, "expiration_time": { "type": "date", - "format": "date_time" + "format": "epoch_millis" }, "roles": { "type": "keyword" diff --git a/plugin/src/test/java/org/elasticsearch/xpack/security/authc/TokenAuthIntegTests.java b/plugin/src/test/java/org/elasticsearch/xpack/security/authc/TokenAuthIntegTests.java index bf283a7bc7e..2a5ab002482 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/security/authc/TokenAuthIntegTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/security/authc/TokenAuthIntegTests.java @@ -7,6 +7,7 @@ package org.elasticsearch.xpack.security.authc; import org.elasticsearch.ElasticsearchSecurityException; import org.elasticsearch.action.search.SearchResponse; +import org.elasticsearch.action.support.PlainActionFuture; import org.elasticsearch.client.Client; import org.elasticsearch.common.settings.SecureString; import org.elasticsearch.common.settings.Settings; @@ -21,6 +22,8 @@ import org.elasticsearch.xpack.security.action.token.InvalidateTokenResponse; import org.elasticsearch.xpack.security.client.SecurityClient; import org.junit.After; +import java.io.IOException; +import java.io.UncheckedIOException; import java.time.Instant; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; @@ -83,21 +86,32 @@ public class TokenAuthIntegTests extends SecurityIntegTestCase { }, 30, TimeUnit.SECONDS); } - public void testExpireMultipleTimes() { + public void testExpireMultipleTimes() throws Exception { CreateTokenResponse response = securityClient().prepareCreateToken() .setGrantType("password") .setUsername(SecuritySettingsSource.DEFAULT_USER_NAME) .setPassword(new SecureString(SecuritySettingsSource.DEFAULT_PASSWORD.toCharArray())) .get(); - Instant created = Instant.now(); InvalidateTokenResponse invalidateResponse = securityClient().prepareInvalidateToken(response.getTokenString()).get(); + // if the token is expired then the API will return false for created so we need to handle that - final boolean correctResponse = invalidateResponse.isCreated() || created.plusSeconds(1L).isBefore(Instant.now()); + final boolean correctResponse = invalidateResponse.isCreated() || isTokenExpired(response.getTokenString()); assertTrue(correctResponse); assertFalse(securityClient().prepareInvalidateToken(response.getTokenString()).get().isCreated()); } + private static boolean isTokenExpired(String token) { + try { + TokenService tokenService = internalCluster().getInstance(TokenService.class); + PlainActionFuture tokenFuture = new PlainActionFuture<>(); + tokenService.decodeToken(token, tokenFuture); + return tokenFuture.actionGet().getExpirationTime().isBefore(Instant.now()); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + @After public void wipeSecurityIndex() throws InterruptedException { // get the token service and wait until token expiration is not in progress! diff --git a/plugin/src/test/java/org/elasticsearch/xpack/security/authc/UserTokenTests.java b/plugin/src/test/java/org/elasticsearch/xpack/security/authc/UserTokenTests.java index bc170de7d18..16cbaa2211a 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/security/authc/UserTokenTests.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/security/authc/UserTokenTests.java @@ -13,15 +13,14 @@ import org.elasticsearch.xpack.security.user.User; import java.io.IOException; import java.time.Clock; -import java.time.ZoneOffset; -import java.time.ZonedDateTime; +import java.time.Instant; public class UserTokenTests extends ESTestCase { public void testSerialization() throws IOException { final Authentication authentication = new Authentication(new User("joe", "a role"), new RealmRef("realm", "native", "node1"), null); final int seconds = randomIntBetween(0, Math.toIntExact(TimeValue.timeValueMinutes(30L).getSeconds())); - final ZonedDateTime expirationTime = Clock.systemUTC().instant().atZone(ZoneOffset.UTC).plusSeconds(seconds); + final Instant expirationTime = Clock.systemUTC().instant().plusSeconds(seconds); final UserToken userToken = new UserToken(authentication, expirationTime); BytesStreamOutput output = new BytesStreamOutput();