Skip to content

Commit

Permalink
Limit token expiry to 1 hour maximum (#38244) (#38392)
Browse files Browse the repository at this point in the history
We mention in our documentation for the token
expiration configuration maximum value is 1 hour
but do not enforce it. This commit adds max limit
to the TOKEN_EXPIRATION setting.

Note: Since this is a backport and the min max
time value support was not there in 6.x, I have
selectively picked the change from Setting.
The changes were done for zen2.
  • Loading branch information
bizybot authored Feb 5, 2019
1 parent 6600806 commit c5bccb1
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -1367,6 +1367,12 @@ public static Setting<TimeValue> timeSetting(
return new Setting<>(simpleKey, s -> defaultValue.apply(s).getStringRep(), minTimeValueParser(key, minValue), properties);
}

public static Setting<TimeValue> timeSetting(
final String key, TimeValue defaultValue, final TimeValue minValue, final TimeValue maxValue, final Property... properties) {
final SimpleKey simpleKey = new SimpleKey(key);
return new Setting<>(simpleKey, s -> defaultValue.getStringRep(), minMaxTimeValueParser(key, minValue, maxValue), properties);
}

private static Function<String, TimeValue> minTimeValueParser(final String key, final TimeValue minValue) {
return s -> {
final TimeValue value = TimeValue.parseTimeValue(s, null, key);
Expand All @@ -1383,6 +1389,22 @@ private static Function<String, TimeValue> minTimeValueParser(final String key,
};
}

private static Function<String, TimeValue> minMaxTimeValueParser(final String key, final TimeValue minValue, final TimeValue maxValue) {
return s -> {
final TimeValue value = minTimeValueParser(key, minValue).apply(s);
if (value.millis() > maxValue.millis()) {
final String message = String.format(
Locale.ROOT,
"failed to parse value [%s] for setting [%s], must be <= [%s]",
s,
key,
maxValue.getStringRep());
throw new IllegalArgumentException(message);
}
return value;
};
}

public static Setting<TimeValue> timeSetting(String key, TimeValue defaultValue, TimeValue minValue, Property... properties) {
return timeSetting(key, (s) -> defaultValue, minValue, properties);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ public final class TokenService extends AbstractComponent {
public static final Setting<SecureString> TOKEN_PASSPHRASE = SecureSetting.secureString("xpack.security.authc.token.passphrase", null,
Property.Deprecated);
public static final Setting<TimeValue> TOKEN_EXPIRATION = Setting.timeSetting("xpack.security.authc.token.timeout",
TimeValue.timeValueMinutes(20L), TimeValue.timeValueSeconds(1L), Property.NodeScope);
TimeValue.timeValueMinutes(20L), TimeValue.timeValueSeconds(1L), TimeValue.timeValueHours(1L), Property.NodeScope);
public static final Setting<TimeValue> DELETE_INTERVAL = Setting.timeSetting("xpack.security.authc.token.delete.interval",
TimeValue.timeValueMinutes(30L), Property.NodeScope);
public static final Setting<TimeValue> DELETE_TIMEOUT = Setting.timeSetting("xpack.security.authc.token.delete.timeout",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@
import static java.time.Clock.systemUTC;
import static org.elasticsearch.repositories.ESBlobStoreTestCase.randomBytes;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;
import static org.mockito.Matchers.any;
Expand Down Expand Up @@ -496,6 +497,29 @@ public void testComputeSecretKeyIsConsistent() throws Exception {
assertArrayEquals(key.getEncoded(), key2.getEncoded());
}

public void testTokenExpiryConfig() {
TimeValue expiration = TokenService.TOKEN_EXPIRATION.get(tokenServiceEnabledSettings);
assertThat(expiration, equalTo(TimeValue.timeValueMinutes(20L)));
// Configure Minimum expiration
tokenServiceEnabledSettings = Settings.builder().put(TokenService.TOKEN_EXPIRATION.getKey(), "1s").build();
expiration = TokenService.TOKEN_EXPIRATION.get(tokenServiceEnabledSettings);
assertThat(expiration, equalTo(TimeValue.timeValueSeconds(1L)));
// Configure Maximum expiration
tokenServiceEnabledSettings = Settings.builder().put(TokenService.TOKEN_EXPIRATION.getKey(), "60m").build();
expiration = TokenService.TOKEN_EXPIRATION.get(tokenServiceEnabledSettings);
assertThat(expiration, equalTo(TimeValue.timeValueHours(1L)));
// Outside range should fail
tokenServiceEnabledSettings = Settings.builder().put(TokenService.TOKEN_EXPIRATION.getKey(), "1ms").build();
IllegalArgumentException ile = expectThrows(IllegalArgumentException.class,
() -> TokenService.TOKEN_EXPIRATION.get(tokenServiceEnabledSettings));
assertThat(ile.getMessage(),
containsString("failed to parse value [1ms] for setting [xpack.security.authc.token.timeout], must be >= [1s]"));
tokenServiceEnabledSettings = Settings.builder().put(TokenService.TOKEN_EXPIRATION.getKey(), "120m").build();
ile = expectThrows(IllegalArgumentException.class, () -> TokenService.TOKEN_EXPIRATION.get(tokenServiceEnabledSettings));
assertThat(ile.getMessage(),
containsString("failed to parse value [120m] for setting [xpack.security.authc.token.timeout], must be <= [1h]"));
}

public void testTokenExpiry() throws Exception {
ClockMock clock = ClockMock.frozen();
TokenService tokenService = new TokenService(tokenServiceEnabledSettings, clock, client, securityIndex, clusterService);
Expand Down

0 comments on commit c5bccb1

Please sign in to comment.