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@d4bf53e7bc
This commit is contained in:
parent
53753311f3
commit
f5cd833af4
|
@ -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) {
|
||||
|
|
|
@ -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<UserToken> listener) throws IOException {
|
||||
void decodeToken(String token, ActionListener<UserToken> 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<IndexResponse>() {
|
||||
@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() {
|
||||
|
|
|
@ -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;
|
||||
}
|
||||
|
||||
|
|
|
@ -138,7 +138,7 @@
|
|||
},
|
||||
"expiration_time": {
|
||||
"type": "date",
|
||||
"format": "date_time"
|
||||
"format": "epoch_millis"
|
||||
},
|
||||
"roles": {
|
||||
"type": "keyword"
|
||||
|
|
|
@ -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<UserToken> 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!
|
||||
|
|
|
@ -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();
|
||||
|
|
Loading…
Reference in New Issue