diff --git a/RELEASE_NOTES.txt b/RELEASE_NOTES.txt index b725a6a8d..8b11549f1 100644 --- a/RELEASE_NOTES.txt +++ b/RELEASE_NOTES.txt @@ -1,6 +1,10 @@ Changes since 4.2.1 ------------------- +* [HTTPCLIENT-1241] (regression) Preemptive BASIC authentication failure should be considered + final and no further attempts to re-authenticate using the same credentials should be made. + Contributed by Oleg Kalnichevski + * [HTTPCLIENT-1229] Fixed NPE in BasicClientConnectionManager that can be triggered by releasing connection after the connection manager has already been shut down. Contributed by Oleg Kalnichevski diff --git a/httpclient/src/main/java/org/apache/http/client/protocol/RequestAuthCache.java b/httpclient/src/main/java/org/apache/http/client/protocol/RequestAuthCache.java index efa23e1d9..fd0e1fc45 100644 --- a/httpclient/src/main/java/org/apache/http/client/protocol/RequestAuthCache.java +++ b/httpclient/src/main/java/org/apache/http/client/protocol/RequestAuthCache.java @@ -127,7 +127,11 @@ public class RequestAuthCache implements HttpRequestInterceptor { Credentials creds = credsProvider.getCredentials(authScope); if (creds != null) { - authState.setState(AuthProtocolState.SUCCESS); + if ("BASIC".equalsIgnoreCase(authScheme.getSchemeName())) { + authState.setState(AuthProtocolState.CHALLENGED); + } else { + authState.setState(AuthProtocolState.SUCCESS); + } authState.update(authScheme, creds); } else { this.log.debug("No credentials for preemptive authentication"); diff --git a/httpclient/src/test/java/org/apache/http/impl/client/integration/TestClientAuthentication.java b/httpclient/src/test/java/org/apache/http/impl/client/integration/TestClientAuthentication.java index 8f45d3cde..e10e1c56f 100644 --- a/httpclient/src/test/java/org/apache/http/impl/client/integration/TestClientAuthentication.java +++ b/httpclient/src/test/java/org/apache/http/impl/client/integration/TestClientAuthentication.java @@ -27,6 +27,7 @@ package org.apache.http.impl.client.integration; import java.io.ByteArrayInputStream; import java.io.IOException; +import java.util.concurrent.atomic.AtomicLong; import org.apache.http.Consts; import org.apache.http.HttpEntity; @@ -38,17 +39,21 @@ import org.apache.http.HttpStatus; import org.apache.http.auth.AuthScope; import org.apache.http.auth.Credentials; import org.apache.http.auth.UsernamePasswordCredentials; +import org.apache.http.client.AuthCache; import org.apache.http.client.ClientProtocolException; import org.apache.http.client.CredentialsProvider; import org.apache.http.client.NonRepeatableRequestException; import org.apache.http.client.methods.HttpGet; import org.apache.http.client.methods.HttpPost; import org.apache.http.client.methods.HttpPut; +import org.apache.http.client.protocol.ClientContext; import org.apache.http.entity.InputStreamEntity; import org.apache.http.entity.StringEntity; +import org.apache.http.impl.client.BasicAuthCache; import org.apache.http.impl.client.BasicCredentialsProvider; import org.apache.http.impl.client.HttpClientBuilder; import org.apache.http.impl.client.TargetAuthenticationStrategy; +import org.apache.http.impl.auth.BasicScheme; import org.apache.http.localserver.BasicAuthTokenExtractor; import org.apache.http.localserver.LocalTestServer; import org.apache.http.localserver.RequestBasicAuth; @@ -322,11 +327,11 @@ public class TestClientAuthentication extends IntegrationTestBase { static class TestTargetAuthenticationStrategy extends TargetAuthenticationStrategy { - private int count; + private AtomicLong count; public TestTargetAuthenticationStrategy() { super(); - this.count = 0; + this.count = new AtomicLong(); } @Override @@ -336,17 +341,13 @@ public class TestClientAuthentication extends IntegrationTestBase { final HttpContext context) { boolean res = super.isAuthenticationRequested(host, response, context); if (res == true) { - synchronized (this) { - this.count++; - } + this.count.incrementAndGet(); } return res; } - public int getCount() { - synchronized (this) { - return this.count; - } + public long getCount() { + return this.count.get(); } } @@ -367,15 +368,16 @@ public class TestClientAuthentication extends IntegrationTestBase { HttpContext context = new BasicHttpContext(); + HttpHost targethost = getServerHttp(); HttpGet httpget = new HttpGet("/"); - HttpResponse response1 = this.httpclient.execute(getServerHttp(), httpget, context); + HttpResponse response1 = this.httpclient.execute(targethost, httpget, context); HttpEntity entity1 = response1.getEntity(); Assert.assertEquals(HttpStatus.SC_OK, response1.getStatusLine().getStatusCode()); Assert.assertNotNull(entity1); EntityUtils.consume(entity1); - HttpResponse response2 = this.httpclient.execute(getServerHttp(), httpget, context); + HttpResponse response2 = this.httpclient.execute(targethost, httpget, context); HttpEntity entity2 = response1.getEntity(); Assert.assertEquals(HttpStatus.SC_OK, response2.getStatusLine().getStatusCode()); Assert.assertNotNull(entity2); @@ -416,4 +418,96 @@ public class TestClientAuthentication extends IntegrationTestBase { EntityUtils.consume(entity); } + static class CountingAuthHandler implements HttpRequestHandler { + + private AtomicLong count; + + public CountingAuthHandler() { + super(); + this.count = new AtomicLong(); + } + + public void handle( + final HttpRequest request, + final HttpResponse response, + final HttpContext context) throws HttpException, IOException { + this.count.incrementAndGet(); + String creds = (String) context.getAttribute("creds"); + if (creds == null || !creds.equals("test:test")) { + response.setStatusCode(HttpStatus.SC_UNAUTHORIZED); + } else { + response.setStatusCode(HttpStatus.SC_OK); + StringEntity entity = new StringEntity("success", Consts.ASCII); + response.setEntity(entity); + } + } + + public long getCount() { + return this.count.get(); + } + + } + + @Test + public void testPreemptiveAuthentication() throws Exception { + CountingAuthHandler requestHandler = new CountingAuthHandler(); + this.localServer.register("*", requestHandler); + + BasicCredentialsProvider credsProvider = new BasicCredentialsProvider(); + credsProvider.setCredentials(AuthScope.ANY, + new UsernamePasswordCredentials("test", "test")); + + this.httpclient = new HttpClientBuilder() + .setCredentialsProvider(credsProvider) + .build(); + + HttpHost targethost = getServerHttp(); + + HttpContext context = new BasicHttpContext(); + AuthCache authCache = new BasicAuthCache(); + authCache.put(targethost, new BasicScheme()); + context.setAttribute(ClientContext.AUTH_CACHE, authCache); + + HttpGet httpget = new HttpGet("/"); + + HttpResponse response1 = this.httpclient.execute(targethost, httpget, context); + HttpEntity entity1 = response1.getEntity(); + Assert.assertEquals(HttpStatus.SC_OK, response1.getStatusLine().getStatusCode()); + Assert.assertNotNull(entity1); + EntityUtils.consume(entity1); + + Assert.assertEquals(1, requestHandler.getCount()); + } + + @Test + public void testPreemptiveAuthenticationFailure() throws Exception { + CountingAuthHandler requestHandler = new CountingAuthHandler(); + this.localServer.register("*", requestHandler); + + BasicCredentialsProvider credsProvider = new BasicCredentialsProvider(); + credsProvider.setCredentials(AuthScope.ANY, + new UsernamePasswordCredentials("test", "stuff")); + + this.httpclient = new HttpClientBuilder() + .setCredentialsProvider(credsProvider) + .build(); + + HttpHost targethost = getServerHttp(); + + HttpContext context = new BasicHttpContext(); + AuthCache authCache = new BasicAuthCache(); + authCache.put(targethost, new BasicScheme()); + context.setAttribute(ClientContext.AUTH_CACHE, authCache); + + HttpGet httpget = new HttpGet("/"); + + HttpResponse response1 = this.httpclient.execute(targethost, httpget, context); + HttpEntity entity1 = response1.getEntity(); + Assert.assertEquals(HttpStatus.SC_UNAUTHORIZED, response1.getStatusLine().getStatusCode()); + Assert.assertNotNull(entity1); + EntityUtils.consume(entity1); + + Assert.assertEquals(1, requestHandler.getCount()); + } + } diff --git a/httpclient/src/test/java/org/apache/http/impl/client/integration/TestClientReauthentication.java b/httpclient/src/test/java/org/apache/http/impl/client/integration/TestClientReauthentication.java index 89ac5ea23..42c0b711b 100644 --- a/httpclient/src/test/java/org/apache/http/impl/client/integration/TestClientReauthentication.java +++ b/httpclient/src/test/java/org/apache/http/impl/client/integration/TestClientReauthentication.java @@ -26,6 +26,7 @@ package org.apache.http.impl.client.integration; import java.io.IOException; +import java.util.Collections; import java.util.concurrent.atomic.AtomicLong; import org.apache.http.Consts; @@ -36,15 +37,21 @@ import org.apache.http.HttpResponse; import org.apache.http.HttpResponseInterceptor; import org.apache.http.HttpStatus; import org.apache.http.auth.AUTH; +import org.apache.http.auth.AuthScheme; import org.apache.http.auth.AuthScope; import org.apache.http.auth.Credentials; import org.apache.http.auth.UsernamePasswordCredentials; +import org.apache.http.auth.params.AuthPNames; import org.apache.http.client.CredentialsProvider; import org.apache.http.client.methods.HttpGet; import org.apache.http.entity.StringEntity; +import org.apache.http.impl.auth.BasicScheme; +import org.apache.http.impl.auth.BasicSchemeFactory; import org.apache.http.impl.client.HttpClientBuilder; +import org.apache.http.impl.client.TargetAuthenticationStrategy; import org.apache.http.localserver.LocalTestServer; import org.apache.http.localserver.RequestBasicAuth; +import org.apache.http.params.HttpParams; import org.apache.http.protocol.BasicHttpContext; import org.apache.http.protocol.BasicHttpProcessor; import org.apache.http.protocol.HttpContext; @@ -66,7 +73,7 @@ public class TestClientReauthentication extends IntegrationTestBase { final HttpResponse response, final HttpContext context) throws HttpException, IOException { if (response.getStatusLine().getStatusCode() == HttpStatus.SC_UNAUTHORIZED) { - response.addHeader(AUTH.WWW_AUTH, "Basic realm=\"test realm\""); + response.addHeader(AUTH.WWW_AUTH, "MyBasic realm=\"test realm\""); } } @@ -142,10 +149,42 @@ public class TestClientReauthentication extends IntegrationTestBase { public void testBasicAuthenticationSuccess() throws Exception { this.localServer.register("*", new AuthHandler()); + BasicSchemeFactory myBasicAuthSchemeFactory = new BasicSchemeFactory() { + + @Override + public AuthScheme newInstance(HttpParams params) { + return new BasicScheme() { + + @Override + public String getSchemeName() { + return "MyBasic"; + } + + }; + } + + }; + + TargetAuthenticationStrategy myAuthStrategy = new TargetAuthenticationStrategy() { + + @Override + protected boolean isCachable(final AuthScheme authScheme) { + return "MyBasic".equalsIgnoreCase(authScheme.getSchemeName()); + } + + }; + TestCredentialsProvider credsProvider = new TestCredentialsProvider( new UsernamePasswordCredentials("test", "test")); - this.httpclient = new HttpClientBuilder().setCredentialsProvider(credsProvider).build(); + this.httpclient = new HttpClientBuilder() + .registerAuthScheme("MyBasic", myBasicAuthSchemeFactory) + .setTargetAuthenticationStrategy(myAuthStrategy) + .setCredentialsProvider(credsProvider) + .build(); + + this.httpclient.getParams().setParameter(AuthPNames.TARGET_AUTH_PREF, + Collections.singletonList("MyBasic")); HttpContext context = new BasicHttpContext(); for (int i = 0; i < 10; i++) {