Skip to content

Commit

Permalink
Preserve REST client auth despite 401 response (elastic#30558)
Browse files Browse the repository at this point in the history
The default behaviour for Apache HTTP client is to mimic the standard
browser behaviour of clearing the authentication cache (for a given
host) if that host responds with 401.
This behaviour is appropriate in a interactive browser environment
where the user is given the opportunity to provide alternative
credentials, but it is not the preferred behaviour for the ES REST
client.

X-Pack may respond with a 401 status if a request is made before the
node/cluster has recovered sufficient state to know how to handle the
provided authentication credentials - for example the security index
need to be recovered before we can authenticate native users.
In these cases the correct behaviour is to retry with the same
credentials (rather than discarding those credentials).
  • Loading branch information
tvernum authored and ywelsch committed May 23, 2018
1 parent b6b550c commit 3a6f73b
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 13 deletions.
Original file line number Diff line number Diff line change
@@ -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
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
Expand All @@ -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();
}

Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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 {
Expand All @@ -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());
Expand All @@ -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);
Expand Down

0 comments on commit 3a6f73b

Please sign in to comment.