Preserve REST client auth despite 401 response

client/rest/src/main/java/org/elasticsearch/client/PersistentCredentialsAuthenticationStrategy.java

@@ -0,0 +1,59 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ *
+ */
+
+package org.elasticsearch.client;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.apache.http.HttpHost;
+import org.apache.http.auth.AuthScheme;
+import org.apache.http.impl.client.TargetAuthenticationStrategy;
+import org.apache.http.protocol.HttpContext;
+
+/**
+ * An {@link org.apache.http.client.AuthenticationStrategy} implementation that does <em>not</em> perform
+ * any special handling if authentication fails.
+ * The default handler in Apache HTTP client mimics standard browser behaviour of clearing authentication
+ * credentials if it receives a 401 response from the server. While this can be useful for browser, it is
+ * rarely the desired behaviour with the Elasticsearch REST API.
+ * If the code using the REST client has configured credentials for the REST API, then we can and should
+ * assume that this is intentional, and those credentials represent the best possible authentication
+ * mechanism to the Elasticsearch node.
+ * If we receive a 401 status, a probably cause is that the authentication mechanism in place was unable
+ * to perform the requisite password checks (the node has not yet recovered its state, or an external
+ * authentication provider was unavailable).
+ * If this occurs, then the desired behaviour is for the Rest client to retry with the same credentials
+ * (rather than trying with no credentials, or expecting the calling code to provide alternate credentials).
+ */
+final class PersistentCredentialsAuthenticationStrategy extends TargetAuthenticationStrategy {
+
+    private final Log logger = LogFactory.getLog(PersistentCredentialsAuthenticationStrategy.class);
+
+    @Override
+    public void authFailed(HttpHost host, AuthScheme authScheme, HttpContext context) {
+        if (logger.isDebugEnabled()) {
+            logger.debug("Authentication to " + host + " failed (scheme: " + authScheme.getSchemeName()
+                + "). Preserving credentials for next request");
+        }
+        // Do nothing.
+        // The superclass implementation of method will clear the credentials from the cache, but we don't
+    }
+}

client/rest/src/main/java/org/elasticsearch/client/RestClientBuilder.java

@@ -204,7 +204,8 @@ private CloseableHttpAsyncClient createHttpClient() {
             HttpAsyncClientBuilder httpClientBuilder = HttpAsyncClientBuilder.create().setDefaultRequestConfig(requestConfigBuilder.build())
                 //default settings for connection pooling may be too constraining
                 .setMaxConnPerRoute(DEFAULT_MAX_CONN_PER_ROUTE).setMaxConnTotal(DEFAULT_MAX_CONN_TOTAL)
-                .setSSLContext(SSLContext.getDefault());
+                .setSSLContext(SSLContext.getDefault())
+                .setTargetAuthenticationStrategy(new PersistentCredentialsAuthenticationStrategy());
             if (httpClientConfigCallback != null) {
                 httpClientBuilder = httpClientConfigCallback.customizeHttpClient(httpClientBuilder);
             }

client/rest/src/test/java/org/elasticsearch/client/RestClientSingleHostIntegTests.java

@@ -31,14 +31,14 @@
 import org.apache.http.entity.ContentType;
 import org.apache.http.entity.StringEntity;
 import org.apache.http.impl.client.BasicCredentialsProvider;
+import org.apache.http.impl.client.TargetAuthenticationStrategy;
 import org.apache.http.impl.nio.client.HttpAsyncClientBuilder;
+import org.apache.http.message.BasicHeader;
 import org.apache.http.nio.entity.NStringEntity;
 import org.apache.http.util.EntityUtils;
 import org.elasticsearch.mocksocket.MockHttpServer;
 import org.junit.After;
-import org.junit.AfterClass;
 import org.junit.Before;
-import org.junit.BeforeClass;
 
 import java.io.IOException;
 import java.io.InputStreamReader;
@@ -147,6 +147,8 @@ public HttpAsyncClientBuilder customizeHttpClient(final HttpAsyncClientBuilder h
                     if (usePreemptiveAuth == false) {
                         // disable preemptive auth by ignoring any authcache
                         httpClientBuilder.disableAuthCaching();
+                        // don't use the "persistent credentials strategy"
+                        httpClientBuilder.setTargetAuthenticationStrategy(new TargetAuthenticationStrategy());
                     }
 
                     return httpClientBuilder.setDefaultCredentialsProvider(credentialsProvider);
@@ -193,7 +195,7 @@ public void onFailure(Exception exception) {
         assertTrue("timeout waiting for requests to be sent", latch.await(10, TimeUnit.SECONDS));
         if (exceptions.isEmpty() == false) {
             AssertionError error = new AssertionError("expected no failures but got some. see suppressed for first 10 of ["
-                                        + exceptions.size() + "] failures");
+                + exceptions.size() + "] failures");
             for (Exception exception : exceptions.subList(0, Math.min(10, exceptions.size()))) {
                 error.addSuppressed(exception);
             }
@@ -217,7 +219,7 @@ public void testHeaders() throws IOException {
             Response esResponse;
             try {
                 esResponse = restClient.performRequest(method, "/" + statusCode, Collections.<String, String>emptyMap(), requestHeaders);
-            } catch(ResponseException e) {
+            } catch (ResponseException e) {
                 esResponse = e.getResponse();
             }
 
@@ -291,8 +293,8 @@ public void testEncodeParams() throws IOException {
     /**
      * Verify that credentials are sent on the first request with preemptive auth enabled (default when provided with credentials).
      */
-    public void testPreemptiveAuthEnabled() throws IOException  {
-        final String[] methods = { "POST", "PUT", "GET", "DELETE" };
+    public void testPreemptiveAuthEnabled() throws IOException {
+        final String[] methods = {"POST", "PUT", "GET", "DELETE"};
 
         try (RestClient restClient = createRestClient(true, true)) {
             for (final String method : methods) {
@@ -306,8 +308,8 @@ public void testPreemptiveAuthEnabled() throws IOException  {
     /**
      * Verify that credentials are <em>not</em> sent on the first request with preemptive auth disabled.
      */
-    public void testPreemptiveAuthDisabled() throws IOException  {
-        final String[] methods = { "POST", "PUT", "GET", "DELETE" };
+    public void testPreemptiveAuthDisabled() throws IOException {
+        final String[] methods = {"POST", "PUT", "GET", "DELETE"};
 
         try (RestClient restClient = createRestClient(true, false)) {
             for (final String method : methods) {
@@ -318,12 +320,31 @@ public void testPreemptiveAuthDisabled() throws IOException  {
         }
     }
 
+    /**
+     * Verify that credentials continue to be sent even if a 401 (Unauthorized) response is received
+     */
+    public void testAuthCredentialsAreNotClearedOnAuthChallenge() throws IOException {
+        final String[] methods = {"POST", "PUT", "GET", "DELETE"};
+
+        try (RestClient restClient = createRestClient(true, true)) {
+            for (final String method : methods) {
+                Header realmHeader = new BasicHeader("WWW-Authenticate", "Basic realm=\"test\"");
+                final Response response401 = bodyTest(restClient, method, 401, new Header[]{realmHeader});
+                assertThat(response401.getHeader("Authorization"), startsWith("Basic"));
+
+                final Response response200 = bodyTest(restClient, method, 200, new Header[0]);
+                assertThat(response200.getHeader("Authorization"), startsWith("Basic"));
+            }
+        }
+
+    }
+
     public void testUrlWithoutLeadingSlash() throws Exception {
         if (pathPrefix.length() == 0) {
             try {
                 restClient.performRequest("GET", "200");
                 fail("request should have failed");
-            } catch(ResponseException e) {
+            } catch (ResponseException e) {
                 assertEquals(404, e.getResponse().getStatusLine().getStatusCode());
             }
         } else {
@@ -335,8 +356,8 @@ public void testUrlWithoutLeadingSlash() throws Exception {
             {
                 //pathPrefix is not required to start with '/', will be added automatically
                 try (RestClient restClient = RestClient.builder(
-                        new HttpHost(httpServer.getAddress().getHostString(), httpServer.getAddress().getPort()))
-                        .setPathPrefix(pathPrefix.substring(1)).build()) {
+                    new HttpHost(httpServer.getAddress().getHostString(), httpServer.getAddress().getPort()))
+                    .setPathPrefix(pathPrefix.substring(1)).build()) {
                     Response response = restClient.performRequest("GET", "200");
                     //a trailing slash gets automatically added if a pathPrefix is configured
                     assertEquals(200, response.getStatusLine().getStatusCode());
@@ -350,10 +371,15 @@ private Response bodyTest(final String method) throws IOException {
     }
 
     private Response bodyTest(final RestClient restClient, final String method) throws IOException {
-        String requestBody = "{ \"field\": \"value\" }";
         int statusCode = randomStatusCode(getRandom());
+        return bodyTest(restClient, method, statusCode, new Header[0]);
+    }
+
+    private Response bodyTest(RestClient restClient, String method, int statusCode, Header[] headers) throws IOException {
+        String requestBody = "{ \"field\": \"value\" }";
         Request request = new Request(method, "/" + statusCode);
         request.setJsonEntity(requestBody);
+        request.setHeaders(headers);
         Response esResponse;
         try {
             esResponse = restClient.performRequest(request);