Skip to content

Commit

Permalink
Always return 401 for not valid tokens (#49736)
Browse files Browse the repository at this point in the history 
Return a 401 in all cases when a request is submitted with an
access token that we can't consume. Before this change, we would
throw a 500 when a request came in with an access token that we
had generated but was then invalidated/expired and deleted from
the tokens index.

Resolves: #38866
  • Loading branch information
@jkakavas
jkakavas authored Dec 6, 2019
1 parent 5ab4591 commit 0b9a9b4
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 5 deletions.
8 changes: 5 additions & 3 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 @@ -421,7 +421,7 @@ private void getUserTokenFromId(String userTokenId, Version tokenVersion, Action
} else {
final GetRequest getRequest = client.prepareGet(tokensIndex.aliasName(),
getTokenDocumentId(userTokenId)).request();
final Consumer<Exception> onFailure = ex -> listener.onFailure(traceLog("decode token", userTokenId, ex));
final Consumer<Exception> onFailure = ex -> listener.onFailure(traceLog("get token from id", userTokenId, ex));
tokensIndex.checkIndexVersionThenExecute(
ex -> listener.onFailure(traceLog("prepare tokens index [" + tokensIndex.aliasName() +"]", userTokenId, ex)),
() -> executeAsyncWithOrigin(client.threadPool().getThreadContext(), SECURITY_ORIGIN, getRequest,
Expand All @@ -441,8 +441,10 @@ private void getUserTokenFromId(String userTokenId, Version tokenVersion, Action
listener.onResponse(UserToken.fromSourceMap(userTokenSource));
}
} else {
onFailure.accept(
new IllegalStateException("token document is missing and must be present"));
// The chances of a random token string decoding to something that we can read is minimal, so
// we assume that this was a token we have created but is now expired/revoked and deleted
logger.trace("The access token [{}] is expired and already deleted", userTokenId);
listener.onResponse(null);
}
}, e -> {
// if the index or the shard is not there / available we assume that
Expand Down
43 changes: 42 additions & 1 deletion ...in/security/src/test/java/org/elasticsearch/xpack/security/authc/TokenAuthIntegTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import org.apache.directory.api.util.Strings;
import org.elasticsearch.ElasticsearchStatusException;
import org.elasticsearch.Version;
import org.elasticsearch.action.admin.cluster.state.ClusterStateResponse;
import org.elasticsearch.action.admin.indices.refresh.RefreshRequest;
import org.elasticsearch.action.search.SearchRequest;
Expand All @@ -23,6 +24,7 @@
import org.elasticsearch.client.security.InvalidateTokenRequest;
import org.elasticsearch.client.security.InvalidateTokenResponse;
import org.elasticsearch.cluster.ack.ClusterStateUpdateResponse;
import org.elasticsearch.common.UUIDs;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.index.query.QueryBuilders;
Expand Down Expand Up @@ -171,6 +173,7 @@ public void testExpiredTokensDeletedAfterExpiration() throws Exception {
restClient.security().invalidateToken(new InvalidateTokenRequest("fooobar", null, null, null),
SECURITY_REQUEST_OPTIONS));
assertThat(e.getMessage(), containsString("token malformed"));
assertThat(e.status(), equalTo(RestStatus.UNAUTHORIZED));
}
restClient.indices().refresh(new RefreshRequest(RestrictedIndicesNames.SECURITY_TOKENS_ALIAS), SECURITY_REQUEST_OPTIONS);
SearchResponse searchResponse = restClient.search(new SearchRequest(RestrictedIndicesNames.SECURITY_TOKENS_ALIAS)
Expand Down Expand Up @@ -455,7 +458,36 @@ public void testClientCredentialsGrant() throws Exception {

ElasticsearchStatusException e = expectThrows(ElasticsearchStatusException.class,
() -> restClient.security().authenticate(tokenAuthOptions));
assertEquals(RestStatus.UNAUTHORIZED, e.status());
assertThat(e.status(), equalTo(RestStatus.UNAUTHORIZED));
}

public void testAuthenticateWithWrongToken() throws Exception {
final RestHighLevelClient restClient = new TestRestHighLevelClient();
CreateTokenResponse response = restClient.security().createToken(CreateTokenRequest.passwordGrant(
SecuritySettingsSource.TEST_USER_NAME, SecuritySettingsSourceField.TEST_PASSWORD.toCharArray()), SECURITY_REQUEST_OPTIONS);
assertNotNull(response.getRefreshToken());
// First check that the correct access token works by getting cluster health with token
assertNoTimeout(client()
.filterWithHeader(Collections.singletonMap("Authorization", "Bearer " + response.getAccessToken()))
.admin().cluster().prepareHealth().get());
// Now attempt to authenticate with an invalid access token string
RequestOptions wrongAuthOptions =
RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", "Bearer " + randomAlphaOfLengthBetween(0, 128)).build();
ElasticsearchStatusException e = expectThrows(ElasticsearchStatusException.class,
() -> restClient.security().authenticate(wrongAuthOptions));
assertThat(e.status(), equalTo(RestStatus.UNAUTHORIZED));
// Now attempt to authenticate with an invalid access token with valid structure (pre 7.2)
RequestOptions wrongAuthOptionsPre72 =
RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", "Bearer " + generateAccessToken(Version.V_7_1_0)).build();
ElasticsearchStatusException e1 = expectThrows(ElasticsearchStatusException.class,
() -> restClient.security().authenticate(wrongAuthOptionsPre72));
assertThat(e1.status(), equalTo(RestStatus.UNAUTHORIZED));
// Now attempt to authenticate with an invalid access token with valid structure (after 7.2)
RequestOptions wrongAuthOptionsAfter72 =
RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", "Bearer " + generateAccessToken(Version.V_7_4_0)).build();
ElasticsearchStatusException e2 = expectThrows(ElasticsearchStatusException.class,
() -> restClient.security().authenticate(wrongAuthOptionsAfter72));
assertThat(e2.status(), equalTo(RestStatus.UNAUTHORIZED));
}

@Before
Expand All @@ -476,4 +508,13 @@ public void testMetadataIsNotSentToClient() {
ClusterStateResponse clusterStateResponse = client().admin().cluster().prepareState().setCustoms(true).get();
assertFalse(clusterStateResponse.getState().customs().containsKey(TokenMetaData.TYPE));
}

private String generateAccessToken(Version version) throws Exception {
TokenService tokenService = internalCluster().getInstance(TokenService.class);
String accessTokenString = UUIDs.randomBase64UUID();
if (version.onOrAfter(TokenService.VERSION_ACCESS_TOKENS_AS_UUIDS)) {
accessTokenString = TokenService.hashTokenString(accessTokenString);
}
return tokenService.prependVersionAndEncodeAccessToken(version, accessTokenString);
}
}
40 changes: 39 additions & 1 deletion ...ugin/security/src/test/java/org/elasticsearch/xpack/security/authc/TokenServiceTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -598,7 +598,7 @@ public void testMalformedToken() throws Exception {
final int numBytes = randomIntBetween(1, TokenService.MINIMUM_BYTES + 32);
final byte[] randomBytes = new byte[numBytes];
random().nextBytes(randomBytes);
TokenService tokenService = createTokenService(Settings.EMPTY, systemUTC());
TokenService tokenService = createTokenService(tokenServiceEnabledSettings, systemUTC());

ThreadContext requestContext = new ThreadContext(Settings.EMPTY);
storeTokenHeader(requestContext, Base64.getEncoder().encodeToString(randomBytes));
Expand All @@ -610,6 +610,36 @@ public void testMalformedToken() throws Exception {
}
}

public void testNotValidPre72Tokens() throws Exception {
TokenService tokenService = createTokenService(tokenServiceEnabledSettings, systemUTC());
// mock another random token so that we don't find a token in TokenService#getUserTokenFromId
Authentication authentication = new Authentication(new User("joe", "admin"), new RealmRef("native_realm", "native", "node1"), null);
mockGetTokenFromId(tokenService, UUIDs.randomBase64UUID(), authentication, false);
ThreadContext requestContext = new ThreadContext(Settings.EMPTY);
storeTokenHeader(requestContext, generateAccessToken(tokenService, Version.V_7_1_0));

try (ThreadContext.StoredContext ignore = requestContext.newStoredContext(true)) {
PlainActionFuture<UserToken> future = new PlainActionFuture<>();
tokenService.getAndValidateToken(requestContext, future);
assertNull(future.get());
}
}

public void testNotValidAfter72Tokens() throws Exception {
TokenService tokenService = createTokenService(tokenServiceEnabledSettings, systemUTC());
// mock another random token so that we don't find a token in TokenService#getUserTokenFromId
Authentication authentication = new Authentication(new User("joe", "admin"), new RealmRef("native_realm", "native", "node1"), null);
mockGetTokenFromId(tokenService, UUIDs.randomBase64UUID(), authentication, false);
ThreadContext requestContext = new ThreadContext(Settings.EMPTY);
storeTokenHeader(requestContext, generateAccessToken(tokenService, randomFrom(Version.V_7_2_0, Version.V_7_3_2)));

try (ThreadContext.StoredContext ignore = requestContext.newStoredContext(true)) {
PlainActionFuture<UserToken> future = new PlainActionFuture<>();
tokenService.getAndValidateToken(requestContext, future);
assertNull(future.get());
}
}

public void testIndexNotAvailable() throws Exception {
TokenService tokenService = createTokenService(tokenServiceEnabledSettings, systemUTC());
Authentication authentication = new Authentication(new User("joe", "admin"), new RealmRef("native_realm", "native", "node1"), null);
Expand Down Expand Up @@ -821,4 +851,12 @@ private DiscoveryNode addAnotherDataNodeWithVersion(ClusterService clusterServic
return anotherDataNode;
}

private String generateAccessToken(TokenService tokenService, Version version) throws Exception {
String accessTokenString = UUIDs.randomBase64UUID();
if (version.onOrAfter(TokenService.VERSION_ACCESS_TOKENS_AS_UUIDS)) {
accessTokenString = TokenService.hashTokenString(accessTokenString);
}
return tokenService.prependVersionAndEncodeAccessToken(version, accessTokenString);
}

}

0 comments on commit 0b9a9b4

Please sign in to comment.