Always return 401 for not valid tokens (#49736) (#50042)

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
Backport of #49736

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/TokenService.java

@@ -422,7 +422,7 @@ public final class TokenService {
         } else {
             final GetRequest getRequest = client.prepareGet(tokensIndex.aliasName(), SINGLE_MAPPING_NAME,
                     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,
@@ -442,8 +442,10 @@ public final class TokenService {
                                     listener.onResponse(UserToken.fromSourceMap(userTokenSource));
                                 }
                             } else {
-                                onFailure.accept(
+                                // The chances of a random token string decoding to something that we can read is minimal, so
-                                    new IllegalStateException("token document is missing and must be present"));
+                                // 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

x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecuritySettingsSource.java

@@ -7,6 +7,7 @@ package org.elasticsearch.test;
 
 import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.analysis.common.CommonAnalysisPlugin;
+import org.elasticsearch.client.RequestOptions;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.network.NetworkModule;
 import org.elasticsearch.common.settings.MockSecureSettings;
@@ -30,11 +31,13 @@ import org.elasticsearch.xpack.security.audit.logfile.LoggingAuditTrail;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.UncheckedIOException;
+import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.StandardCopyOption;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Base64;
 import java.util.Collection;
 import java.util.List;
 import java.util.function.Consumer;
@@ -57,6 +60,11 @@ public class SecuritySettingsSource extends NodeConfigurationSource {
     public static final String TEST_PASSWORD_HASHED =
         new String(Hasher.resolve(randomFrom("pbkdf2", "pbkdf2_1000", "bcrypt9", "bcrypt8", "bcrypt")).
             hash(new SecureString(SecuritySettingsSourceField.TEST_PASSWORD.toCharArray())));
+    public static final RequestOptions SECURITY_REQUEST_OPTIONS = RequestOptions.DEFAULT.toBuilder()
+        .addHeader("Authorization",
+            "Basic " + Base64.getEncoder().encodeToString(
+                (TEST_USER_NAME + ":" + SecuritySettingsSourceField.TEST_PASSWORD).getBytes(StandardCharsets.UTF_8)))
+        .build();
     public static final String TEST_ROLE = "user";
     public static final String TEST_SUPERUSER = "test_superuser";
 

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/TokenAuthIntegTests.java

@@ -8,14 +8,19 @@ package org.elasticsearch.xpack.security.authc;
 import org.apache.directory.api.util.Strings;
 import org.elasticsearch.ElasticsearchSecurityException;
 import org.elasticsearch.action.ActionListener;
+import org.elasticsearch.ElasticsearchStatusException;
+import org.elasticsearch.Version;
 import org.elasticsearch.action.admin.cluster.state.ClusterStateResponse;
 import org.elasticsearch.action.search.SearchResponse;
 import org.elasticsearch.action.support.PlainActionFuture;
 import org.elasticsearch.action.support.WriteRequest;
 import org.elasticsearch.action.update.UpdateResponse;
 import org.elasticsearch.client.Client;
+import org.elasticsearch.client.RequestOptions;
+import org.elasticsearch.client.RestHighLevelClient;
 import org.elasticsearch.cluster.ack.ClusterStateUpdateResponse;
 import org.elasticsearch.common.settings.SecureString;
+import org.elasticsearch.common.UUIDs;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.index.query.QueryBuilders;
@@ -53,6 +58,7 @@ import java.util.concurrent.atomic.AtomicReference;
 import java.util.stream.Collectors;
 
 import static org.elasticsearch.index.mapper.MapperService.SINGLE_MAPPING_NAME;
+import static org.elasticsearch.test.SecuritySettingsSource.SECURITY_REQUEST_OPTIONS;
 import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertNoTimeout;
 import static org.hamcrest.Matchers.equalTo;
 
@@ -75,6 +81,11 @@ public class TokenAuthIntegTests extends SecurityIntegTestCase {
         return defaultMaxNumberOfNodes() + 1;
     }
 
+    @Override
+    protected boolean addMockHttpTransport() {
+        return false; // need real http
+    }
+
     public void testTokenServiceBootstrapOnNodeJoin() throws Exception {
         final Client client = client();
         SecurityClient securityClient = new SecurityClient(client);
@@ -186,6 +197,7 @@ public class TokenAuthIntegTests extends SecurityIntegTestCase {
                             .actionGet();
                 } catch (ElasticsearchSecurityException e) {
                     assertEquals("token malformed", e.getMessage());
+                    assertThat(e.status(), equalTo(RestStatus.UNAUTHORIZED));
                 }
             }
             client.admin().indices().prepareRefresh(RestrictedIndicesNames.SECURITY_TOKENS_ALIAS).get();
@@ -302,7 +314,6 @@ public class TokenAuthIntegTests extends SecurityIntegTestCase {
         assertNoTimeout(client()
                 .filterWithHeader(Collections.singletonMap("Authorization", "Bearer " + createTokenResponse.getTokenString()))
                 .admin().cluster().prepareHealth().get());
-
         CreateTokenResponse refreshResponse = securityClient.prepareRefreshToken(createTokenResponse.getRefreshToken()).get();
         assertNotNull(refreshResponse.getRefreshToken());
         assertNotEquals(refreshResponse.getRefreshToken(), createTokenResponse.getRefreshToken());
@@ -552,6 +563,36 @@ public class TokenAuthIntegTests extends SecurityIntegTestCase {
         });
     }
 
+    public void testAuthenticateWithWrongToken() throws Exception {
+        final RestHighLevelClient restClient = new TestRestHighLevelClient();
+        org.elasticsearch.client.security.CreateTokenResponse response = restClient.security().createToken(
+            org.elasticsearch.client.security.CreateTokenRequest.passwordGrant(SecuritySettingsSource.TEST_USER_NAME,
+            SecuritySettingsSourceField.TEST_PASSWORD.toCharArray()), SECURITY_REQUEST_OPTIONS);
+        assertNotNull(response.getAccessToken());
+        // First authenticate with token
+        RequestOptions correctAuthOptions =
+            RequestOptions.DEFAULT.toBuilder().addHeader("Authorization", "Bearer " + response.getAccessToken()).build();
+        org.elasticsearch.client.security.AuthenticateResponse validResponse = restClient.security().authenticate(correctAuthOptions);
+        assertThat(validResponse.getUser().getUsername(), equalTo(SecuritySettingsSource.TEST_USER_NAME));
+        // 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
     public void waitForSecurityIndexWritable() throws Exception {
         assertSecurityIndexActive();
@@ -570,4 +611,13 @@ public class TokenAuthIntegTests extends SecurityIntegTestCase {
         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);
+    }
 }

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/TokenServiceTests.java

@@ -600,7 +600,7 @@ public class TokenServiceTests extends ESTestCase {
         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));
@@ -612,6 +612,36 @@ public class TokenServiceTests extends ESTestCase {
         }
     }
 
+    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);
@@ -823,4 +853,12 @@ public class TokenServiceTests extends ESTestCase {
         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);
+    }
+
 }