diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncProtocolExec.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncProtocolExec.java index 706793e98b..3a7de2326a 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncProtocolExec.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/AsyncProtocolExec.java @@ -217,7 +217,8 @@ public AsyncDataConsumer handleResponse( if (needAuthentication( targetAuthExchange, proxyAuthExchange, - proxy != null ? proxy : target, + // A 407 received through a tunnel comes from the origin, not from the proxy + route.isTunnelled() ? null : proxy, target, pathPrefix, response, @@ -319,11 +320,11 @@ private boolean needAuthentication( } } - final boolean proxyAuthRequested = authenticator.isChallenged( + final boolean proxyAuthRequested = proxy != null && authenticator.isChallenged( proxy, ChallengeType.PROXY, response, proxyAuthExchange, context); - final boolean proxyMutualAuthRequired = authenticator.isChallengeExpected(proxyAuthExchange); + final boolean proxyMutualAuthRequired = proxy != null && authenticator.isChallengeExpected(proxyAuthExchange); - if (authCacheKeeper != null) { + if (proxy != null && authCacheKeeper != null) { if (proxyAuthRequested) { authCacheKeeper.updateOnChallenge(proxy, null, proxyAuthExchange, context); } else { diff --git a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ProtocolExec.java b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ProtocolExec.java index 4514ddfe94..cc73b224e5 100644 --- a/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ProtocolExec.java +++ b/httpclient5/src/main/java/org/apache/hc/client5/http/impl/classic/ProtocolExec.java @@ -207,7 +207,8 @@ public ClassicHttpResponse execute( if (needAuthentication( targetAuthExchange, proxyAuthExchange, - proxy != null ? proxy : target, + // A 407 received through a tunnel comes from the origin, not from the proxy + route.isTunnelled() ? null : proxy, target, pathPrefix, response, @@ -280,11 +281,11 @@ private boolean needAuthentication( } } - final boolean proxyAuthRequested = authenticator.isChallenged( + final boolean proxyAuthRequested = proxy != null && authenticator.isChallenged( proxy, ChallengeType.PROXY, response, proxyAuthExchange, context); - final boolean proxyMutualAuthRequired = authenticator.isChallengeExpected(proxyAuthExchange); + final boolean proxyMutualAuthRequired = proxy != null && authenticator.isChallengeExpected(proxyAuthExchange); - if (authCacheKeeper != null) { + if (proxy != null && authCacheKeeper != null) { if (proxyAuthRequested) { authCacheKeeper.updateOnChallenge(proxy, null, proxyAuthExchange, context); } else { diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/async/TestAsyncProtocolExec.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/async/TestAsyncProtocolExec.java index dff99e94e6..7fcae912dc 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/async/TestAsyncProtocolExec.java +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/async/TestAsyncProtocolExec.java @@ -27,11 +27,14 @@ package org.apache.hc.client5.http.impl.async; import java.io.IOException; +import java.util.ArrayList; import java.util.Collections; +import java.util.List; import java.util.concurrent.atomic.AtomicInteger; import org.apache.hc.client5.http.AuthenticationStrategy; import org.apache.hc.client5.http.HttpRoute; +import org.apache.hc.client5.http.RouteInfo; import org.apache.hc.client5.http.async.AsyncExecCallback; import org.apache.hc.client5.http.async.AsyncExecChain; import org.apache.hc.client5.http.async.AsyncExecRuntime; @@ -39,6 +42,7 @@ import org.apache.hc.client5.http.auth.AuthScope; import org.apache.hc.client5.http.auth.ChallengeType; import org.apache.hc.client5.http.auth.StandardAuthScheme; +import org.apache.hc.client5.http.impl.auth.BasicAuthCache; import org.apache.hc.client5.http.impl.auth.BasicScheme; import org.apache.hc.client5.http.impl.auth.CredentialsProviderBuilder; import org.apache.hc.client5.http.protocol.HttpClientContext; @@ -157,4 +161,111 @@ public void failed(final Exception cause) { final AuthExchange authExchange = context.getAuthExchange(target); Assertions.assertEquals("/blah/", authExchange.getPathPrefix()); } + + private static final class RecordingCallback implements AsyncExecCallback { + + private final List codes = new ArrayList<>(); + private final AtomicInteger completed = new AtomicInteger(0); + + @Override + public AsyncDataConsumer handleResponse(final HttpResponse response, final EntityDetails entityDetails) { + codes.add(response.getCode()); + return Mockito.mock(AsyncDataConsumer.class); + } + + @Override + public void handleInformationResponse(final HttpResponse response) { + } + + @Override + public void completed() { + completed.incrementAndGet(); + } + + @Override + public void failed(final Exception cause) { + Assertions.fail(cause); + } + + } + + private void stubChain(final AtomicInteger proceedCount, final List requests, final int... codes) + throws Exception { + Mockito.doAnswer(invocation -> { + requests.add(invocation.getArgument(0, HttpRequest.class)); + final AsyncExecCallback cb = invocation.getArgument(3, AsyncExecCallback.class); + final int code = codes[Math.min(proceedCount.getAndIncrement(), codes.length - 1)]; + final HttpResponse resp = new BasicHttpResponse(code, "Huh?"); + if (code == 407) { + resp.setHeader(HttpHeaders.PROXY_AUTHENTICATE, StandardAuthScheme.BASIC + " realm=test"); + } + cb.handleResponse(resp, (EntityDetails) null); + cb.completed(); + return null; + }).when(chain).proceed(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); + } + + @Test + void testProxyChallengeThroughTunnelWithAuthCacheIsReturnedToCaller() throws Exception { + final AsyncProtocolExec cachingExec = new AsyncProtocolExec(targetAuthStrategy, proxyAuthStrategy, null, false); + final HttpHost httpsTarget = new HttpHost("https", "foo", 443); + final HttpHost proxy = new HttpHost("http", "proxy", 8080); + final HttpRoute route = new HttpRoute(httpsTarget, null, proxy, true, + RouteInfo.TunnelType.TUNNELLED, RouteInfo.LayerType.LAYERED); + final BasicAuthCache authCache = new BasicAuthCache(); + final HttpClientContext context = HttpClientContext.create(); + context.setAuthCache(authCache); + context.setCredentialsProvider(CredentialsProviderBuilder.create() + .add(new AuthScope(proxy), "proxyuser", "proxypass".toCharArray()) + .build()); + Mockito.when(execRuntime.isEndpointConnected()).thenReturn(true); + final AtomicInteger proceedCount = new AtomicInteger(0); + final List requests = new ArrayList<>(); + stubChain(proceedCount, requests, 407); + + final HttpRequest request = BasicRequestBuilder.get("https://foo/test").build(); + final AsyncExecChain.Scope scope = new AsyncExecChain.Scope("test", route, request, + Mockito.mock(CancellableDependency.class), context, execRuntime, null, new AtomicInteger(1)); + final RecordingCallback callback = new RecordingCallback(); + + cachingExec.execute(request, (AsyncEntityProducer) null, scope, chain, callback); + + Assertions.assertEquals(Collections.singletonList(407), callback.codes); + Assertions.assertEquals(1, callback.completed.get()); + Assertions.assertEquals(1, proceedCount.get()); + Assertions.assertNull(requests.get(0).getFirstHeader(HttpHeaders.PROXY_AUTHORIZATION)); + Mockito.verifyNoInteractions(proxyAuthStrategy); + Assertions.assertEquals(AuthExchange.State.UNCHALLENGED, context.getAuthExchange(proxy).getState()); + Assertions.assertNull(authCache.get(proxy)); + } + + @Test + void testProxyChallengeOnPlainRouteStillAuthenticates() throws Exception { + final HttpHost proxy = new HttpHost("http", "proxy", 8080); + final HttpRoute route = new HttpRoute(target, null, proxy, false); + final HttpClientContext context = HttpClientContext.create(); + context.setCredentialsProvider(CredentialsProviderBuilder.create() + .add(new AuthScope(proxy), "proxyuser", "proxypass".toCharArray()) + .build()); + Mockito.when(proxyAuthStrategy.select( + Mockito.eq(ChallengeType.PROXY), + Mockito.any(), + Mockito.any())) + .thenReturn(Collections.singletonList(new BasicScheme())); + Mockito.when(execRuntime.isEndpointConnected()).thenReturn(true); + final AtomicInteger proceedCount = new AtomicInteger(0); + final List requests = new ArrayList<>(); + stubChain(proceedCount, requests, 407, 200); + + final HttpRequest request = BasicRequestBuilder.get("http://foo/test").build(); + final AsyncExecChain.Scope scope = new AsyncExecChain.Scope("test", route, request, + Mockito.mock(CancellableDependency.class), context, execRuntime, null, new AtomicInteger(1)); + final RecordingCallback callback = new RecordingCallback(); + + protocolExec.execute(request, (AsyncEntityProducer) null, scope, chain, callback); + + Assertions.assertEquals(2, proceedCount.get()); + Assertions.assertEquals(Collections.singletonList(200), callback.codes); + Assertions.assertNotNull(requests.get(1).getFirstHeader(HttpHeaders.PROXY_AUTHORIZATION)); + } } diff --git a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestProtocolExec.java b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestProtocolExec.java index 3417e66ff5..9588a42972 100644 --- a/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestProtocolExec.java +++ b/httpclient5/src/test/java/org/apache/hc/client5/http/impl/classic/TestProtocolExec.java @@ -34,6 +34,7 @@ import org.apache.hc.client5.http.AuthenticationStrategy; import org.apache.hc.client5.http.HttpRoute; +import org.apache.hc.client5.http.RouteInfo; import org.apache.hc.client5.http.auth.AuthExchange; import org.apache.hc.client5.http.auth.AuthScope; import org.apache.hc.client5.http.auth.ChallengeType; @@ -43,6 +44,7 @@ import org.apache.hc.client5.http.classic.methods.HttpGet; import org.apache.hc.client5.http.classic.methods.HttpPost; import org.apache.hc.client5.http.entity.EntityBuilder; +import org.apache.hc.client5.http.impl.auth.BasicAuthCache; import org.apache.hc.client5.http.impl.auth.BasicScheme; import org.apache.hc.client5.http.impl.auth.CredentialsProviderBuilder; import org.apache.hc.client5.http.protocol.HttpClientContext; @@ -58,6 +60,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; @@ -295,4 +298,105 @@ void testAuthExchangePathPrefixRestoredAfterChallenge() throws Exception { final AuthExchange authExchange = context.getAuthExchange(target); Assertions.assertEquals("/blah/", authExchange.getPathPrefix()); } + + @Test + void testProxyChallengeThroughTunnelIsReturnedToCaller() throws Exception { + final HttpHost httpsTarget = new HttpHost("https", "foo", 443); + final HttpHost proxy = new HttpHost("http", "proxy", 8080); + final HttpRoute route = new HttpRoute(httpsTarget, null, proxy, true, + RouteInfo.TunnelType.TUNNELLED, RouteInfo.LayerType.LAYERED); + final HttpClientContext context = HttpClientContext.create(); + context.setCredentialsProvider(CredentialsProviderBuilder.create() + .add(new AuthScope(proxy), "proxyuser", "proxypass".toCharArray()) + .build()); + final ClassicHttpRequest request = new HttpGet("https://foo/test"); + final ClassicHttpResponse response = new BasicClassicHttpResponse(407, "Huh?"); + response.setHeader(HttpHeaders.PROXY_AUTHENTICATE, StandardAuthScheme.BASIC + " realm=test"); + + Mockito.when(chain.proceed(Mockito.same(request), Mockito.any())).thenReturn(response); + + final ExecChain.Scope scope = new ExecChain.Scope("test", route, request, execRuntime, context); + final ClassicHttpResponse finalResponse = protocolExec.execute(request, scope, chain); + Assertions.assertEquals(407, finalResponse.getCode()); + Mockito.verify(chain, Mockito.times(1)).proceed(request, scope); + Mockito.verifyNoInteractions(proxyAuthStrategy); + Assertions.assertNull(context.getAuthExchange(proxy).getAuthScheme()); + } + + @Test + void testProxyChallengeWithoutProxyIsReturnedToCaller() throws Exception { + final HttpRoute route = new HttpRoute(target); + final HttpClientContext context = HttpClientContext.create(); + context.setCredentialsProvider(CredentialsProviderBuilder.create() + .add(new AuthScope(target), "user", "pass".toCharArray()) + .build()); + final ClassicHttpRequest request = new HttpGet("http://foo/test"); + final ClassicHttpResponse response = new BasicClassicHttpResponse(407, "Huh?"); + response.setHeader(HttpHeaders.PROXY_AUTHENTICATE, StandardAuthScheme.BASIC + " realm=test"); + + Mockito.when(chain.proceed(Mockito.same(request), Mockito.any())).thenReturn(response); + + final ExecChain.Scope scope = new ExecChain.Scope("test", route, request, execRuntime, context); + final ClassicHttpResponse finalResponse = protocolExec.execute(request, scope, chain); + Assertions.assertEquals(407, finalResponse.getCode()); + Mockito.verify(chain, Mockito.times(1)).proceed(request, scope); + Mockito.verifyNoInteractions(proxyAuthStrategy); + } + + @Test + void testProxyChallengeThroughTunnelWithAuthCacheIsReturnedToCaller() throws Exception { + final ProtocolExec cachingExec = new ProtocolExec(targetAuthStrategy, proxyAuthStrategy, null, false); + final HttpHost httpsTarget = new HttpHost("https", "foo", 443); + final HttpHost proxy = new HttpHost("http", "proxy", 8080); + final HttpRoute route = new HttpRoute(httpsTarget, null, proxy, true, + RouteInfo.TunnelType.TUNNELLED, RouteInfo.LayerType.LAYERED); + final BasicAuthCache authCache = new BasicAuthCache(); + final HttpClientContext context = HttpClientContext.create(); + context.setAuthCache(authCache); + context.setCredentialsProvider(CredentialsProviderBuilder.create() + .add(new AuthScope(proxy), "proxyuser", "proxypass".toCharArray()) + .build()); + final ClassicHttpRequest request = new HttpGet("https://foo/test"); + final ClassicHttpResponse response = new BasicClassicHttpResponse(407, "Huh?"); + response.setHeader(HttpHeaders.PROXY_AUTHENTICATE, StandardAuthScheme.BASIC + " realm=test"); + + Mockito.when(chain.proceed(Mockito.same(request), Mockito.any())).thenReturn(response); + + final ExecChain.Scope scope = new ExecChain.Scope("test", route, request, execRuntime, context); + final ClassicHttpResponse finalResponse = cachingExec.execute(request, scope, chain); + Assertions.assertSame(response, finalResponse); + Mockito.verify(chain, Mockito.times(1)).proceed(request, scope); + Mockito.verifyNoInteractions(proxyAuthStrategy); + Assertions.assertNull(request.getFirstHeader(HttpHeaders.PROXY_AUTHORIZATION)); + Assertions.assertEquals(AuthExchange.State.UNCHALLENGED, context.getAuthExchange(proxy).getState()); + Assertions.assertNull(authCache.get(proxy)); + } + + @Test + void testProxyChallengeOnPlainRouteStillAuthenticates() throws Exception { + final HttpHost proxy = new HttpHost("http", "proxy", 8080); + final HttpRoute route = new HttpRoute(target, null, proxy, false); + final HttpClientContext context = HttpClientContext.create(); + context.setCredentialsProvider(CredentialsProviderBuilder.create() + .add(new AuthScope(proxy), "proxyuser", "proxypass".toCharArray()) + .build()); + final ClassicHttpRequest request = new HttpGet("http://foo/test"); + final ClassicHttpResponse response1 = new BasicClassicHttpResponse(407, "Huh?"); + response1.setHeader(HttpHeaders.PROXY_AUTHENTICATE, StandardAuthScheme.BASIC + " realm=test"); + final ClassicHttpResponse response2 = new BasicClassicHttpResponse(200, "OK"); + + Mockito.when(chain.proceed(Mockito.any(), Mockito.any())).thenReturn(response1, response2); + Mockito.when(proxyAuthStrategy.select( + Mockito.eq(ChallengeType.PROXY), + Mockito.any(), + Mockito.any())).thenReturn(Collections.singletonList(new BasicScheme())); + Mockito.when(execRuntime.isConnectionReusable()).thenReturn(true); + + final ExecChain.Scope scope = new ExecChain.Scope("test", route, request, execRuntime, context); + final ClassicHttpResponse finalResponse = protocolExec.execute(request, scope, chain); + Assertions.assertEquals(200, finalResponse.getCode()); + final ArgumentCaptor captor = ArgumentCaptor.forClass(ClassicHttpRequest.class); + Mockito.verify(chain, Mockito.times(2)).proceed(captor.capture(), Mockito.same(scope)); + Assertions.assertNotNull(captor.getAllValues().get(1).getFirstHeader(HttpHeaders.PROXY_AUTHORIZATION)); + } }