Skip to content

Commit

Permalink
Fix TokenBackwardsCompatibility tests
Browse files Browse the repository at this point in the history 
This change

- Fixes TokenBackwardsCompatibilityIT: Existing tests seemed to made
  the assumption that in the oneThirdUpgraded stage the master node
  will be on the old version and in the twoThirdsUpgraded stage, the
  master node will be one of the upgraded ones. However, there is no
  guarantee that the master node in any of the states will or will
  not be one of the upgraded ones.
  This class now tests:
  - That we can generate and consume tokens before we start the
  rolling upgrade.
  - That we can consume tokens generated in the old cluster during
  all the stages of the rolling upgrade.
  - That while on a mixed cluster, when/if the master node is
  upgraded, we can generate, consume and refresh a token
  - That after the rolling upgrade, we can consume a token
  generated in an old cluster and can invalidate it so that it
  can't be used any more.
- Ensures that during the rolling upgrade, the upgraded nodes have
the same configuration as the old nodes. Specifically that the
file realm we use is explicitly named `file1`. This is needed
because while attempting to refresh a token in a mixed cluster
we might create a token hitting an old node and attempt to refresh
it hitting a new node. If the file realm name is not the same, the
refresh will be seen as being made by a "different" client, and
will, thus, fail.
- Renames the Authentication variable we check while refreshing a
token to be clientAuth in order to make the code more readable.

Some of the above were possibly causing the flakiness of elastic#37379
  • Loading branch information
@jkakavas
jkakavas committed Feb 21, 2019
1 parent e9156f9 commit 37cc7f1
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 74 deletions.
24 changes: 12 additions & 12 deletions ...ck/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -656,9 +656,9 @@ public void refreshToken(String refreshToken, ActionListener<Tuple<UserToken, St
ensureEnabled();
findTokenFromRefreshToken(refreshToken,
ActionListener.wrap(tuple -> {
final Authentication userAuth = Authentication.readFromContext(client.threadPool().getThreadContext());
final Authentication clientAuth = Authentication.readFromContext(client.threadPool().getThreadContext());
final String tokenDocId = tuple.v1().getHits().getHits()[0].getId();
innerRefresh(tokenDocId, userAuth, listener, tuple.v2());
innerRefresh(tokenDocId, clientAuth, listener, tuple.v2());
}, listener::onFailure),
new AtomicInteger(0));
}
Expand Down Expand Up @@ -719,7 +719,7 @@ private void findTokenFromRefreshToken(String refreshToken, ActionListener<Tuple
* may be recoverable. The refresh involves retrieval of the token document and then
* updating the token document to indicate that the document has been refreshed.
*/
private void innerRefresh(String tokenDocId, Authentication userAuth, ActionListener<Tuple<UserToken, String>> listener,
private void innerRefresh(String tokenDocId, Authentication clientAuth, ActionListener<Tuple<UserToken, String>> listener,
AtomicInteger attemptCount) {
if (attemptCount.getAndIncrement() > MAX_RETRY_ATTEMPTS) {
logger.warn("Failed to refresh token for doc [{}] after [{}] attempts", tokenDocId, attemptCount.get());
Expand All @@ -731,7 +731,7 @@ private void innerRefresh(String tokenDocId, Authentication userAuth, ActionList
ActionListener.<GetResponse>wrap(response -> {
if (response.isExists()) {
final Map<String, Object> source = response.getSource();
final Optional<ElasticsearchSecurityException> invalidSource = checkTokenDocForRefresh(source, userAuth);
final Optional<ElasticsearchSecurityException> invalidSource = checkTokenDocForRefresh(source, clientAuth);

if (invalidSource.isPresent()) {
onFailure.accept(invalidSource.get());
Expand All @@ -754,12 +754,12 @@ private void innerRefresh(String tokenDocId, Authentication userAuth, ActionList
updateRequest.setIfPrimaryTerm(response.getPrimaryTerm());
executeAsyncWithOrigin(client.threadPool().getThreadContext(), SECURITY_ORIGIN, updateRequest.request(),
ActionListener.<UpdateResponse>wrap(
updateResponse -> createUserToken(authentication, userAuth, listener, metadata, true),
updateResponse -> createUserToken(authentication, clientAuth, listener, metadata, true),
e -> {
Throwable cause = ExceptionsHelper.unwrapCause(e);
if (cause instanceof VersionConflictEngineException ||
isShardNotAvailableException(e)) {
innerRefresh(tokenDocId, userAuth,
innerRefresh(tokenDocId, clientAuth,
listener, attemptCount);
} else {
onFailure.accept(e);
Expand All @@ -774,7 +774,7 @@ private void innerRefresh(String tokenDocId, Authentication userAuth, ActionList
}
}, e -> {
if (isShardNotAvailableException(e)) {
innerRefresh(tokenDocId, userAuth, listener, attemptCount);
innerRefresh(tokenDocId, clientAuth, listener, attemptCount);
} else {
listener.onFailure(e);
}
Expand All @@ -786,7 +786,7 @@ private void innerRefresh(String tokenDocId, Authentication userAuth, ActionList
* Performs checks on the retrieved source and returns an {@link Optional} with the exception
* if there is an issue
*/
private Optional<ElasticsearchSecurityException> checkTokenDocForRefresh(Map<String, Object> source, Authentication userAuth) {
private Optional<ElasticsearchSecurityException> checkTokenDocForRefresh(Map<String, Object> source, Authentication clientAuth) {
final Map<String, Object> refreshTokenSrc = (Map<String, Object>) source.get("refresh_token");
final Map<String, Object> accessTokenSrc = (Map<String, Object>) source.get("access_token");
if (refreshTokenSrc == null || refreshTokenSrc.isEmpty()) {
Expand Down Expand Up @@ -820,18 +820,18 @@ private Optional<ElasticsearchSecurityException> checkTokenDocForRefresh(Map<Str
} else if (userTokenSrc.get("metadata") == null) {
return Optional.of(invalidGrantException("token is missing metadata"));
} else {
return checkClient(refreshTokenSrc, userAuth);
return checkClient(refreshTokenSrc, clientAuth);
}
}
}

private Optional<ElasticsearchSecurityException> checkClient(Map<String, Object> refreshTokenSource, Authentication userAuth) {
private Optional<ElasticsearchSecurityException> checkClient(Map<String, Object> refreshTokenSource, Authentication clientAuth) {
Map<String, Object> clientInfo = (Map<String, Object>) refreshTokenSource.get("client");
if (clientInfo == null) {
return Optional.of(invalidGrantException("token is missing client information"));
} else if (userAuth.getUser().principal().equals(clientInfo.get("user")) == false) {
} else if (clientAuth.getUser().principal().equals(clientInfo.get("user")) == false) {
return Optional.of(invalidGrantException("tokens must be refreshed by the creating client"));
} else if (userAuth.getAuthenticatedBy().getName().equals(clientInfo.get("realm")) == false) {
} else if (clientAuth.getAuthenticatedBy().getName().equals(clientInfo.get("realm")) == false) {
return Optional.of(invalidGrantException("tokens must be refreshed by the creating client"));
} else {
return Optional.empty();
Expand Down
9 changes: 9 additions & 0 deletions x-pack/qa/rolling-upgrade/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,15 @@ subprojects {
setting 'node.name', "upgraded-node-${stopNode}"
dependsOn copyTestNodeKeystore
extraConfigFile 'testnode.jks', new File(outputDir + '/testnode.jks')
if (version.onOrAfter('7.0.0')) {
setting 'xpack.security.authc.realms.file.file1.order', '0'
setting 'xpack.security.authc.realms.native.native1.order', '1'
} else {
setting 'xpack.security.authc.realms.file1.type', 'file'
setting 'xpack.security.authc.realms.file1.order', '0'
setting 'xpack.security.authc.realms.native1.type', 'native'
setting 'xpack.security.authc.realms.native1.order', '1'
}
if (withSystemKey) {
setting 'xpack.watcher.encrypt_sensitive_data', 'true'
keystoreFile 'xpack.watcher.encryption_key', "${mainProject.projectDir}/src/test/resources/system_key"
Expand Down
75 changes: 13 additions & 62 deletions ...lling-upgrade/src/test/java/org/elasticsearch/upgrades/TokenBackwardsCompatibilityIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ public class TokenBackwardsCompatibilityIT extends AbstractUpgradeTestCase {

public void testGeneratingTokenInOldCluster() throws Exception {
assumeTrue("this test should only run against the old cluster", CLUSTER_TYPE == ClusterType.OLD);

// Create a couple of tokens and store them in the token_backwards_compatibility_it index to be used for tests in the mixed/upgraded
// clusters
Request createTokenRequest = new Request("POST", "/_security/oauth2/token");
createTokenRequest.setJsonEntity(
"{\n" +
Expand All @@ -44,8 +45,8 @@ public void testGeneratingTokenInOldCluster() throws Exception {
"{\n" +
" \"token\": \"" + token + "\"\n" +
"}");
client().performRequest(indexRequest1);

Response indexResponse1 = client().performRequest(indexRequest1);
assertOK(indexResponse1);
Request createSecondTokenRequest = new Request("POST", "/_security/oauth2/token");
createSecondTokenRequest.setEntity(createTokenRequest.getEntity());
response = client().performRequest(createSecondTokenRequest);
Expand All @@ -58,34 +59,23 @@ public void testGeneratingTokenInOldCluster() throws Exception {
"{\n" +
" \"token\": \"" + token + "\"\n" +
"}");
client().performRequest(indexRequest2);
Response indexResponse2 = client().performRequest(indexRequest2);
assertOK(indexResponse2);
}

public void testTokenWorksInMixedOrUpgradedCluster() throws Exception {
assumeTrue("this test should only run against the mixed or upgraded cluster",
CLUSTER_TYPE == ClusterType.MIXED || CLUSTER_TYPE == ClusterType.UPGRADED);
public void testTokenWorksInMixedCluster() throws Exception {
// Verify that an old token continues to work during all stages of the rolling upgrade
assumeTrue("this test should only run against the mixed cluster", CLUSTER_TYPE == ClusterType.MIXED);
Request getRequest = new Request("GET", "token_backwards_compatibility_it/_doc/old_cluster_token1");
Response getResponse = client().performRequest(getRequest);
assertOK(getResponse);
Map<String, Object> source = (Map<String, Object>) entityAsMap(getResponse).get("_source");
assertTokenWorks((String) source.get("token"));
}

public void testMixedCluster() throws Exception {
public void testMixedClusterWithUpgradedMaster() throws Exception {
assumeTrue("this test should only run against the mixed cluster", CLUSTER_TYPE == ClusterType.MIXED);
assumeTrue("the master must be on the latest version before we can write", isMasterOnLatestVersion());
Request getRequest = new Request("GET", "token_backwards_compatibility_it/_doc/old_cluster_token2");

Response getResponse = client().performRequest(getRequest);
Map<String, Object> source = (Map<String, Object>) entityAsMap(getResponse).get("_source");
final String token = (String) source.get("token");
assertTokenWorks(token);

Request invalidateRequest = new Request("DELETE", "/_security/oauth2/token");
invalidateRequest.setJsonEntity("{\"token\": \"" + token + "\"}");
invalidateRequest.addParameter("error_trace", "true");
client().performRequest(invalidateRequest);
assertTokenDoesNotWork(token);

// create token and refresh on version that supports it
Request createTokenRequest = new Request("POST", "/_security/oauth2/token");
Expand Down Expand Up @@ -124,60 +114,21 @@ public void testMixedCluster() throws Exception {
}

public void testUpgradedCluster() throws Exception {
assumeTrue("this test should only run against the mixed cluster", CLUSTER_TYPE == ClusterType.UPGRADED);
Request getRequest = new Request("GET", "token_backwards_compatibility_it/_doc/old_cluster_token2");
assumeTrue("this test should only run against the upgraded cluster", CLUSTER_TYPE == ClusterType.UPGRADED);

// Use an old token to authenticate, then invalidate it and verify that it can no longer be used
Request getRequest = new Request("GET", "token_backwards_compatibility_it/_doc/old_cluster_token1");
Response getResponse = client().performRequest(getRequest);
assertOK(getResponse);
Map<String, Object> source = (Map<String, Object>) entityAsMap(getResponse).get("_source");
final String token = (String) source.get("token");

// invalidate again since this may not have been invalidated in the mixed cluster
Request invalidateRequest = new Request("DELETE", "/_security/oauth2/token");
invalidateRequest.setJsonEntity("{\"token\": \"" + token + "\"}");
invalidateRequest.addParameter("error_trace", "true");
Response invalidationResponse = client().performRequest(invalidateRequest);
assertOK(invalidationResponse);
assertTokenDoesNotWork(token);

getRequest = new Request("GET", "token_backwards_compatibility_it/_doc/old_cluster_token1");

getResponse = client().performRequest(getRequest);
source = (Map<String, Object>) entityAsMap(getResponse).get("_source");
final String workingToken = (String) source.get("token");
assertTokenWorks(workingToken);

Request getTokenRequest = new Request("POST", "/_security/oauth2/token");
getTokenRequest.setJsonEntity(
"{\n" +
" \"username\": \"test_user\",\n" +
" \"password\": \"x-pack-test-password\",\n" +
" \"grant_type\": \"password\"\n" +
"}");
Response response = client().performRequest(getTokenRequest);
Map<String, Object> responseMap = entityAsMap(response);
String accessToken = (String) responseMap.get("access_token");
String refreshToken = (String) responseMap.get("refresh_token");
assertNotNull(accessToken);
assertNotNull(refreshToken);
assertTokenWorks(accessToken);

Request refreshTokenRequest = new Request("POST", "/_security/oauth2/token");
refreshTokenRequest.setJsonEntity(
"{\n" +
" \"refresh_token\": \"" + refreshToken + "\",\n" +
" \"grant_type\": \"refresh_token\"\n" +
"}");
response = client().performRequest(refreshTokenRequest);
responseMap = entityAsMap(response);
String updatedAccessToken = (String) responseMap.get("access_token");
String updatedRefreshToken = (String) responseMap.get("refresh_token");
assertNotNull(updatedAccessToken);
assertNotNull(updatedRefreshToken);
assertTokenWorks(updatedAccessToken);
assertTokenWorks(accessToken);
assertNotEquals(accessToken, updatedAccessToken);
assertNotEquals(refreshToken, updatedRefreshToken);
}

private void assertTokenWorks(String token) throws IOException {
Expand Down

0 comments on commit 37cc7f1

Please sign in to comment.