-
Notifications
You must be signed in to change notification settings - Fork 24.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support concurrent refresh of refresh tokens #38382
Changes from 10 commits
70e620f
318af42
541fc34
55b7428
27324ab
f51a8f8
44e0036
7b70ca5
2b54388
6a66302
7337574
e655b0d
0fa0f3c
4d1e1dc
d0971d1
03475ac
8f804c1
309c8d1
e13fd13
35eb8ef
cbc626d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -330,7 +330,7 @@ public void testRefreshingInvalidatedToken() { | |
assertEquals("token has been invalidated", e.getHeader("error_description").get(0)); | ||
} | ||
|
||
public void testRefreshingMultipleTimes() { | ||
public void testRefreshingMultipleTimesFails() throws Exception { | ||
Client client = client().filterWithHeader(Collections.singletonMap("Authorization", | ||
UsernamePasswordToken.basicAuthHeaderValue(SecuritySettingsSource.TEST_USER_NAME, | ||
SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING))); | ||
|
@@ -343,6 +343,30 @@ public void testRefreshingMultipleTimes() { | |
assertNotNull(createTokenResponse.getRefreshToken()); | ||
CreateTokenResponse refreshResponse = securityClient.prepareRefreshToken(createTokenResponse.getRefreshToken()).get(); | ||
assertNotNull(refreshResponse); | ||
// We now have two documents, the original(now refreshed) token doc and the new one with the new access doc | ||
AtomicReference<String> docId = new AtomicReference<>(); | ||
assertBusy(() -> { | ||
SearchResponse searchResponse = client.prepareSearch(SecurityIndexManager.SECURITY_INDEX_NAME) | ||
.setSource(SearchSourceBuilder.searchSource() | ||
.query(QueryBuilders.boolQuery() | ||
.must(QueryBuilders.termQuery("doc_type", "token")) | ||
.must(QueryBuilders.termQuery("refresh_token.refreshed", "true")))) | ||
.setSize(1) | ||
.setTerminateAfter(1) | ||
.get(); | ||
assertThat(searchResponse.getHits().getTotalHits().value, equalTo(1L)); | ||
docId.set(searchResponse.getHits().getAt(0).getId()); | ||
}); | ||
|
||
// hack doc to modify the refresh time to 10 seconds ago so that we don't hit the lenient refresh case | ||
Instant refreshed = Instant.now(); | ||
Instant aWhileAgo = refreshed.minus(10L, ChronoUnit.SECONDS); | ||
assertTrue(Instant.now().isAfter(aWhileAgo)); | ||
client.prepareUpdate(SecurityIndexManager.SECURITY_INDEX_NAME, "doc", docId.get()) | ||
.setDoc("refresh_token", Collections.singletonMap("refresh_time", aWhileAgo.toEpochMilli())) | ||
.setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE) | ||
.get(); | ||
|
||
|
||
ElasticsearchSecurityException e = expectThrows(ElasticsearchSecurityException.class, | ||
() -> securityClient.prepareRefreshToken(createTokenResponse.getRefreshToken()).get()); | ||
|
@@ -351,6 +375,38 @@ public void testRefreshingMultipleTimes() { | |
assertEquals("token has already been refreshed", e.getHeader("error_description").get(0)); | ||
} | ||
|
||
public void testRefreshingMultipleTimesWithinWindowSucceeds() throws Exception { | ||
Client client = client().filterWithHeader(Collections.singletonMap("Authorization", | ||
UsernamePasswordToken.basicAuthHeaderValue(SecuritySettingsSource.TEST_USER_NAME, | ||
SecuritySettingsSourceField.TEST_PASSWORD_SECURE_STRING))); | ||
SecurityClient securityClient = new SecurityClient(client); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. by doing it this way, all threads use the same client which hits the same node; maybe it would be worth having each thread create a client before it says that it is ready to run by counting down the new latch |
||
CreateTokenResponse createTokenResponse = securityClient.prepareCreateToken() | ||
.setGrantType("password") | ||
.setUsername(SecuritySettingsSource.TEST_USER_NAME) | ||
.setPassword(new SecureString(SecuritySettingsSourceField.TEST_PASSWORD.toCharArray())) | ||
.get(); | ||
assertNotNull(createTokenResponse.getRefreshToken()); | ||
|
||
CreateTokenResponse refreshResponse = securityClient.prepareRefreshToken(createTokenResponse.getRefreshToken()).get(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should fire a small number of requests in parallel and then check the responses have the same token string. |
||
assertNotNull(refreshResponse); | ||
assertNotNull(refreshResponse.getRefreshToken()); | ||
assertNotEquals(refreshResponse.getRefreshToken(), createTokenResponse.getRefreshToken()); | ||
assertNotEquals(refreshResponse.getTokenString(), createTokenResponse.getTokenString()); | ||
|
||
assertNoTimeout(client().filterWithHeader(Collections.singletonMap("Authorization", "Bearer " + refreshResponse.getTokenString())) | ||
.admin().cluster().prepareHealth().get()); | ||
|
||
CreateTokenResponse secondRefreshResponse = securityClient.prepareRefreshToken(createTokenResponse.getRefreshToken()).get(); | ||
assertNotNull(secondRefreshResponse); | ||
assertNotNull(secondRefreshResponse.getRefreshToken()); | ||
assertThat(secondRefreshResponse.getRefreshToken(), equalTo(refreshResponse.getRefreshToken())); | ||
assertThat(secondRefreshResponse.getTokenString(), equalTo(refreshResponse.getTokenString())); | ||
|
||
assertNoTimeout( | ||
client().filterWithHeader(Collections.singletonMap("Authorization", "Bearer " + secondRefreshResponse.getTokenString())) | ||
.admin().cluster().prepareHealth().get()); | ||
} | ||
|
||
public void testRefreshAsDifferentUser() { | ||
Client client = client().filterWithHeader(Collections.singletonMap("Authorization", | ||
UsernamePasswordToken.basicAuthHeaderValue(SecuritySettingsSource.TEST_USER_NAME, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -42,7 +42,6 @@ | |
import org.elasticsearch.xpack.core.security.authc.Authentication; | ||
import org.elasticsearch.xpack.core.security.authc.Authentication.AuthenticationType; | ||
import org.elasticsearch.xpack.core.security.authc.Authentication.RealmRef; | ||
import org.elasticsearch.xpack.core.security.authc.TokenMetaData; | ||
import org.elasticsearch.xpack.core.security.authc.support.TokensInvalidationResult; | ||
import org.elasticsearch.xpack.core.security.user.User; | ||
import org.elasticsearch.xpack.core.watcher.watch.ClockMock; | ||
|
@@ -187,189 +186,6 @@ public void testInvalidAuthorizationHeader() throws Exception { | |
} | ||
} | ||
|
||
public void testRotateKey() throws Exception { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This code is still present in the token service, right? If so I don't think we should delete the tests until we remove the code |
||
TokenService tokenService = new TokenService(tokenServiceEnabledSettings, systemUTC(), client, securityIndex, clusterService); | ||
Authentication authentication = new Authentication(new User("joe", "admin"), new RealmRef("native_realm", "native", "node1"), null); | ||
PlainActionFuture<Tuple<UserToken, String>> tokenFuture = new PlainActionFuture<>(); | ||
tokenService.createUserToken(authentication, authentication, tokenFuture, Collections.emptyMap(), true); | ||
final UserToken token = tokenFuture.get().v1(); | ||
assertNotNull(token); | ||
mockGetTokenFromId(token, false); | ||
authentication = token.getAuthentication(); | ||
|
||
ThreadContext requestContext = new ThreadContext(Settings.EMPTY); | ||
requestContext.putHeader("Authorization", "Bearer " + tokenService.getUserTokenString(token)); | ||
|
||
try (ThreadContext.StoredContext ignore = requestContext.newStoredContext(true)) { | ||
PlainActionFuture<UserToken> future = new PlainActionFuture<>(); | ||
tokenService.getAndValidateToken(requestContext, future); | ||
UserToken serialized = future.get(); | ||
assertAuthentication(authentication, serialized.getAuthentication()); | ||
} | ||
rotateKeys(tokenService); | ||
|
||
try (ThreadContext.StoredContext ignore = requestContext.newStoredContext(true)) { | ||
PlainActionFuture<UserToken> future = new PlainActionFuture<>(); | ||
tokenService.getAndValidateToken(requestContext, future); | ||
UserToken serialized = future.get(); | ||
assertAuthentication(authentication, serialized.getAuthentication()); | ||
} | ||
|
||
PlainActionFuture<Tuple<UserToken, String>> newTokenFuture = new PlainActionFuture<>(); | ||
tokenService.createUserToken(authentication, authentication, newTokenFuture, Collections.emptyMap(), true); | ||
final UserToken newToken = newTokenFuture.get().v1(); | ||
assertNotNull(newToken); | ||
assertNotEquals(tokenService.getUserTokenString(newToken), tokenService.getUserTokenString(token)); | ||
|
||
requestContext = new ThreadContext(Settings.EMPTY); | ||
requestContext.putHeader("Authorization", "Bearer " + tokenService.getUserTokenString(newToken)); | ||
mockGetTokenFromId(newToken, false); | ||
|
||
try (ThreadContext.StoredContext ignore = requestContext.newStoredContext(true)) { | ||
PlainActionFuture<UserToken> future = new PlainActionFuture<>(); | ||
tokenService.getAndValidateToken(requestContext, future); | ||
UserToken serialized = future.get(); | ||
assertAuthentication(authentication, serialized.getAuthentication()); | ||
} | ||
} | ||
|
||
private void rotateKeys(TokenService tokenService) { | ||
TokenMetaData tokenMetaData = tokenService.generateSpareKey(); | ||
tokenService.refreshMetaData(tokenMetaData); | ||
tokenMetaData = tokenService.rotateToSpareKey(); | ||
tokenService.refreshMetaData(tokenMetaData); | ||
} | ||
|
||
public void testKeyExchange() throws Exception { | ||
TokenService tokenService = new TokenService(tokenServiceEnabledSettings, systemUTC(), client, securityIndex, clusterService); | ||
int numRotations = randomIntBetween(1, 5); | ||
for (int i = 0; i < numRotations; i++) { | ||
rotateKeys(tokenService); | ||
} | ||
TokenService otherTokenService = new TokenService(tokenServiceEnabledSettings, systemUTC(), client, securityIndex, | ||
clusterService); | ||
otherTokenService.refreshMetaData(tokenService.getTokenMetaData()); | ||
Authentication authentication = new Authentication(new User("joe", "admin"), new RealmRef("native_realm", "native", "node1"), null); | ||
PlainActionFuture<Tuple<UserToken, String>> tokenFuture = new PlainActionFuture<>(); | ||
tokenService.createUserToken(authentication, authentication, tokenFuture, Collections.emptyMap(), true); | ||
final UserToken token = tokenFuture.get().v1(); | ||
assertNotNull(token); | ||
mockGetTokenFromId(token, false); | ||
authentication = token.getAuthentication(); | ||
|
||
ThreadContext requestContext = new ThreadContext(Settings.EMPTY); | ||
requestContext.putHeader("Authorization", "Bearer " + tokenService.getUserTokenString(token)); | ||
try (ThreadContext.StoredContext ignore = requestContext.newStoredContext(true)) { | ||
PlainActionFuture<UserToken> future = new PlainActionFuture<>(); | ||
otherTokenService.getAndValidateToken(requestContext, future); | ||
UserToken serialized = future.get(); | ||
assertAuthentication(authentication, serialized.getAuthentication()); | ||
} | ||
|
||
rotateKeys(tokenService); | ||
|
||
otherTokenService.refreshMetaData(tokenService.getTokenMetaData()); | ||
|
||
try (ThreadContext.StoredContext ignore = requestContext.newStoredContext(true)) { | ||
PlainActionFuture<UserToken> future = new PlainActionFuture<>(); | ||
otherTokenService.getAndValidateToken(requestContext, future); | ||
UserToken serialized = future.get(); | ||
assertEquals(authentication, serialized.getAuthentication()); | ||
} | ||
} | ||
|
||
public void testPruneKeys() throws Exception { | ||
TokenService tokenService = new TokenService(tokenServiceEnabledSettings, systemUTC(), client, securityIndex, clusterService); | ||
Authentication authentication = new Authentication(new User("joe", "admin"), new RealmRef("native_realm", "native", "node1"), null); | ||
PlainActionFuture<Tuple<UserToken, String>> tokenFuture = new PlainActionFuture<>(); | ||
tokenService.createUserToken(authentication, authentication, tokenFuture, Collections.emptyMap(), true); | ||
final UserToken token = tokenFuture.get().v1(); | ||
assertNotNull(token); | ||
mockGetTokenFromId(token, false); | ||
authentication = token.getAuthentication(); | ||
|
||
ThreadContext requestContext = new ThreadContext(Settings.EMPTY); | ||
requestContext.putHeader("Authorization", "Bearer " + tokenService.getUserTokenString(token)); | ||
|
||
try (ThreadContext.StoredContext ignore = requestContext.newStoredContext(true)) { | ||
PlainActionFuture<UserToken> future = new PlainActionFuture<>(); | ||
tokenService.getAndValidateToken(requestContext, future); | ||
UserToken serialized = future.get(); | ||
assertAuthentication(authentication, serialized.getAuthentication()); | ||
} | ||
TokenMetaData metaData = tokenService.pruneKeys(randomIntBetween(0, 100)); | ||
tokenService.refreshMetaData(metaData); | ||
|
||
int numIterations = scaledRandomIntBetween(1, 5); | ||
for (int i = 0; i < numIterations; i++) { | ||
rotateKeys(tokenService); | ||
} | ||
|
||
try (ThreadContext.StoredContext ignore = requestContext.newStoredContext(true)) { | ||
PlainActionFuture<UserToken> future = new PlainActionFuture<>(); | ||
tokenService.getAndValidateToken(requestContext, future); | ||
UserToken serialized = future.get(); | ||
assertAuthentication(authentication, serialized.getAuthentication()); | ||
} | ||
|
||
PlainActionFuture<Tuple<UserToken, String>> newTokenFuture = new PlainActionFuture<>(); | ||
tokenService.createUserToken(authentication, authentication, newTokenFuture, Collections.emptyMap(), true); | ||
final UserToken newToken = newTokenFuture.get().v1(); | ||
assertNotNull(newToken); | ||
assertNotEquals(tokenService.getUserTokenString(newToken), tokenService.getUserTokenString(token)); | ||
|
||
metaData = tokenService.pruneKeys(1); | ||
tokenService.refreshMetaData(metaData); | ||
|
||
try (ThreadContext.StoredContext ignore = requestContext.newStoredContext(true)) { | ||
PlainActionFuture<UserToken> future = new PlainActionFuture<>(); | ||
tokenService.getAndValidateToken(requestContext, future); | ||
UserToken serialized = future.get(); | ||
assertNull(serialized); | ||
} | ||
|
||
requestContext = new ThreadContext(Settings.EMPTY); | ||
requestContext.putHeader("Authorization", "Bearer " + tokenService.getUserTokenString(newToken)); | ||
mockGetTokenFromId(newToken, false); | ||
try (ThreadContext.StoredContext ignore = requestContext.newStoredContext(true)) { | ||
PlainActionFuture<UserToken> future = new PlainActionFuture<>(); | ||
tokenService.getAndValidateToken(requestContext, future); | ||
UserToken serialized = future.get(); | ||
assertAuthentication(authentication, serialized.getAuthentication()); | ||
} | ||
|
||
} | ||
|
||
public void testPassphraseWorks() throws Exception { | ||
TokenService tokenService = new TokenService(tokenServiceEnabledSettings, systemUTC(), client, securityIndex, clusterService); | ||
Authentication authentication = new Authentication(new User("joe", "admin"), new RealmRef("native_realm", "native", "node1"), null); | ||
PlainActionFuture<Tuple<UserToken, String>> tokenFuture = new PlainActionFuture<>(); | ||
tokenService.createUserToken(authentication, authentication, tokenFuture, Collections.emptyMap(), true); | ||
final UserToken token = tokenFuture.get().v1(); | ||
assertNotNull(token); | ||
mockGetTokenFromId(token, false); | ||
authentication = token.getAuthentication(); | ||
|
||
ThreadContext requestContext = new ThreadContext(Settings.EMPTY); | ||
requestContext.putHeader("Authorization", "Bearer " + tokenService.getUserTokenString(token)); | ||
|
||
try (ThreadContext.StoredContext ignore = requestContext.newStoredContext(true)) { | ||
PlainActionFuture<UserToken> future = new PlainActionFuture<>(); | ||
tokenService.getAndValidateToken(requestContext, future); | ||
UserToken serialized = future.get(); | ||
assertAuthentication(authentication, serialized.getAuthentication()); | ||
} | ||
|
||
try (ThreadContext.StoredContext ignore = requestContext.newStoredContext(true)) { | ||
// verify a second separate token service with its own passphrase cannot verify | ||
TokenService anotherService = new TokenService(Settings.EMPTY, systemUTC(), client, securityIndex, | ||
clusterService); | ||
PlainActionFuture<UserToken> future = new PlainActionFuture<>(); | ||
anotherService.getAndValidateToken(requestContext, future); | ||
assertNull(future.get()); | ||
} | ||
} | ||
|
||
public void testGetTokenWhenKeyCacheHasExpired() throws Exception { | ||
TokenService tokenService = new TokenService(tokenServiceEnabledSettings, systemUTC(), client, securityIndex, clusterService); | ||
Authentication authentication = new Authentication(new User("joe", "admin"), new RealmRef("native_realm", "native", "node1"), null); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's verify the update response here as that might help with debugging in case of test failure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point !