Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apache channel supports proxy credentials #383

Merged
merged 3 commits into from
Feb 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions changelog/@unreleased/pr-383.v2.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
type: improvement
improvement:
description: Apache channel supports proxy credentials
links:
- https://github.com/palantir/dialogue/pull/383
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,9 @@ private static final class RequestBodyEntity implements HttpEntity {

@Override
public boolean isRepeatable() {
return false;
// TODO(#328): Binary bodies are not repeatable, however all our structured bodies are.
// Marking the entity repeatable allows proxy authentication to work.
return true;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

import com.google.common.collect.ImmutableList;
import com.google.common.primitives.Ints;
import com.palantir.conjure.java.api.config.service.BasicCredentials;
import com.palantir.conjure.java.api.config.service.UserAgent;
import com.palantir.conjure.java.client.config.CipherSuites;
import com.palantir.conjure.java.client.config.ClientConfiguration;
Expand All @@ -27,15 +28,35 @@
import com.palantir.logsafe.exceptions.SafeIllegalArgumentException;
import java.net.MalformedURLException;
import java.net.URL;
import java.util.ArrayDeque;
import java.util.Collections;
import java.util.Map;
import java.util.Queue;
import java.util.concurrent.TimeUnit;
import org.apache.http.Header;
import org.apache.http.HttpHost;
import org.apache.http.HttpResponse;
import org.apache.http.auth.AuthOption;
import org.apache.http.auth.AuthScheme;
import org.apache.http.auth.AuthSchemeProvider;
import org.apache.http.auth.AuthScope;
import org.apache.http.auth.Credentials;
import org.apache.http.auth.UsernamePasswordCredentials;
import org.apache.http.client.AuthenticationStrategy;
import org.apache.http.client.CredentialsProvider;
import org.apache.http.client.config.AuthSchemes;
import org.apache.http.client.config.RequestConfig;
import org.apache.http.config.RegistryBuilder;
import org.apache.http.config.SocketConfig;
import org.apache.http.conn.ssl.DefaultHostnameVerifier;
import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
import org.apache.http.impl.auth.BasicSchemeFactory;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.HttpClientBuilder;
import org.apache.http.impl.client.HttpClients;
import org.apache.http.impl.client.ProxyAuthenticationStrategy;
import org.apache.http.impl.conn.SystemDefaultRoutePlanner;
import org.apache.http.protocol.HttpContext;

public final class ApacheHttpClientChannels {
private ApacheHttpClientChannels() {}
Expand All @@ -44,12 +65,12 @@ public static Channel create(ClientConfiguration conf, UserAgent baseAgent) {
Preconditions.checkArgument(
!conf.fallbackToCommonNameVerification(), "fallback-to-common-name-verification is not supported");
Preconditions.checkArgument(!conf.meshProxy().isPresent(), "Mesh proxy is not supported");
Preconditions.checkArgument(!conf.proxyCredentials().isPresent(), "Proxy credentials are not supported");

long socketTimeoutMillis =
Math.max(conf.readTimeout().toMillis(), conf.writeTimeout().toMillis());
int connectTimeout = Ints.checkedCast(conf.connectTimeout().toMillis());
// TODO(ckozak): close resources?
CloseableHttpClient client = HttpClients.custom()
HttpClientBuilder builder = HttpClients.custom()
.setDefaultRequestConfig(RequestConfig.custom()
.setSocketTimeout(Ints.checkedCast(socketTimeoutMillis))
.setConnectTimeout(connectTimeout)
Expand All @@ -66,7 +87,6 @@ public static Channel create(ClientConfiguration conf, UserAgent baseAgent) {
.setMaxConnTotal(Integer.MAX_VALUE)
// TODO(ckozak): proxy credentials
.setRoutePlanner(new SystemDefaultRoutePlanner(null, conf.proxy()))
.setProxyAuthenticationStrategy(ProxyAuthenticationStrategy.INSTANCE)
.disableAutomaticRetries()
// Must be disabled otherwise connections are not reused when client certificates are provided
.disableConnectionState()
Expand All @@ -82,7 +102,19 @@ public static Channel create(ClientConfiguration conf, UserAgent baseAgent) {
? CipherSuites.allCipherSuites()
: CipherSuites.fastCipherSuites(),
new DefaultHostnameVerifier()))
.build();
.setDefaultCredentialsProvider(NullCredentialsProvider.INSTANCE)
.setTargetAuthenticationStrategy(NullAuthenticationStrategy.INSTANCE)
.setProxyAuthenticationStrategy(NullAuthenticationStrategy.INSTANCE)
.setDefaultAuthSchemeRegistry(
RegistryBuilder.<AuthSchemeProvider>create().build());
conf.proxyCredentials().ifPresent(credentials -> {
builder.setDefaultCredentialsProvider(new SingleCredentialsProvider(credentials))
.setProxyAuthenticationStrategy(ProxyAuthenticationStrategy.INSTANCE)
.setDefaultAuthSchemeRegistry(RegistryBuilder.<AuthSchemeProvider>create()
.register(AuthSchemes.BASIC, new BasicSchemeFactory())
.build());
});
CloseableHttpClient client = builder.build();
ImmutableList<Channel> channels = conf.uris().stream()
.map(uri -> BlockingChannelAdapter.of(new ApacheHttpClientBlockingChannel(client, url(uri))))
.collect(ImmutableList.toImmutableList());
Expand All @@ -97,4 +129,64 @@ private static URL url(String uri) {
throw new SafeIllegalArgumentException("Failed to parse URL", e);
}
}

private enum NullCredentialsProvider implements CredentialsProvider {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh my boilerplate. Why do we need to provide null implementations?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to avoid dealing with credentials in all possible cases so we don't have to deal with folks getting used to client-specific behavior.
When proxy credentials are provided I want to make sure they can only be used by proxies by disabling the target auth impl, and all schemes except basic to match okhttp.

INSTANCE;

@Override
public void setCredentials(AuthScope _authscope, Credentials _credentials) {}

@Override
public Credentials getCredentials(AuthScope _authscope) {
return null;
}

@Override
public void clear() {}
}

private static final class SingleCredentialsProvider implements CredentialsProvider {
private final Credentials credentials;

SingleCredentialsProvider(BasicCredentials basicCredentials) {
credentials = new UsernamePasswordCredentials(basicCredentials.username(), basicCredentials.password());
}

@Override
public void setCredentials(AuthScope _authscope, Credentials _credentials) {}

@Override
public Credentials getCredentials(AuthScope _authscope) {
return credentials;
}

@Override
public void clear() {}
}

private enum NullAuthenticationStrategy implements AuthenticationStrategy {
INSTANCE;

@Override
public boolean isAuthenticationRequested(HttpHost _authhost, HttpResponse _response, HttpContext _context) {
return false;
}

@Override
public Map<String, Header> getChallenges(HttpHost _authhost, HttpResponse _response, HttpContext _context) {
return Collections.emptyMap();
}

@Override
public Queue<AuthOption> select(
Map<String, Header> _challenges, HttpHost _authhost, HttpResponse _response, HttpContext _context) {
return new ArrayDeque<>(1);
}

@Override
public void authSucceeded(HttpHost _authhost, AuthScheme _authScheme, HttpContext _context) {}

@Override
public void authFailed(HttpHost _authhost, AuthScheme _authScheme, HttpContext _context) {}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
import java.net.URL;
import java.nio.file.Paths;

public final class ApacheApacheHttpClientChannelsTest extends AbstractChannelTest {
public final class ApacheHttpClientChannelsTest extends AbstractChannelTest {

private static final SslConfiguration SSL_CONFIG = SslConfiguration.of(
Paths.get("src/test/resources/trustStore.jks"), Paths.get("src/test/resources/keyStore.jks"), "keystore");
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* (c) Copyright 2020 Palantir Technologies Inc. All rights reserved.
*
* Licensed 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 com.palantir.dialogue.hc4;

import com.palantir.conjure.java.api.config.service.UserAgent;
import com.palantir.conjure.java.client.config.ClientConfiguration;
import com.palantir.dialogue.AbstractProxyConfigTest;
import com.palantir.dialogue.Channel;

public final class ApacheProxyConfigTest extends AbstractProxyConfigTest {
@Override
protected Channel create(ClientConfiguration config, UserAgent agent) {
return ApacheHttpClientChannels.create(config, agent);
}
}
1 change: 1 addition & 0 deletions dialogue-client-test-lib/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ dependencies {
compile project(':dialogue-example')

compile 'com.google.guava:guava'
compile 'com.palantir.conjure.java.runtime:client-config'
compile 'com.palantir.conjure.java.runtime:keystores'
compile 'com.squareup.okhttp3:mockwebserver'
compile 'junit:junit'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,177 @@
/*
* (c) Copyright 2020 Palantir Technologies Inc. All rights reserved.
*
* Licensed 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 com.palantir.dialogue;

import static org.assertj.core.api.Assertions.assertThat;

import com.google.common.collect.ImmutableList;
import com.palantir.conjure.java.api.config.service.BasicCredentials;
import com.palantir.conjure.java.api.config.service.UserAgent;
import com.palantir.conjure.java.api.config.ssl.SslConfiguration;
import com.palantir.conjure.java.client.config.ClientConfiguration;
import com.palantir.conjure.java.client.config.ClientConfigurations;
import com.palantir.conjure.java.config.ssl.SslSocketFactories;
import java.io.IOException;
import java.io.OutputStream;
import java.net.InetSocketAddress;
import java.net.Proxy;
import java.net.ProxySelector;
import java.net.SocketAddress;
import java.net.URI;
import java.nio.charset.StandardCharsets;
import java.nio.file.Paths;
import java.util.List;
import java.util.Map;
import okhttp3.mockwebserver.MockResponse;
import okhttp3.mockwebserver.MockWebServer;
import okhttp3.mockwebserver.RecordedRequest;
import org.junit.Rule;
import org.junit.Test;

public abstract class AbstractProxyConfigTest {

protected static final UserAgent AGENT = UserAgent.of(UserAgent.Agent.of("test", "0.0.1"));
private static final SslConfiguration SSL_CONFIG = SslConfiguration.of(
Paths.get("../dialogue-client-test-lib/src/main/resources/trustStore.jks"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these paths seem a little off

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to reuse this config in other tests too, but avoided modifying them here. It's a bit odd, but it would allow us to check in a single ssl config with our abstract tests and reuse it in all client implementations. We have a lot of boilerplate to implement a test.

Paths.get("../dialogue-client-test-lib/src/main/resources/keyStore.jks"),
"keystore");

private static final Request request = Request.builder()
.body(new RequestBody() {
@Override
public void writeTo(OutputStream output) throws IOException {
output.write("Hello, World".getBytes(StandardCharsets.UTF_8));
}

@Override
public String contentType() {
return "text/plain";
}
})
.build();

@Rule
public final MockWebServer server = new MockWebServer();

@Rule
public final MockWebServer proxyServer = new MockWebServer();

protected abstract Channel create(ClientConfiguration config, UserAgent agent);

@Test
public void testDirectVersusProxyConnection() throws Exception {
server.enqueue(new MockResponse().setBody("server"));
proxyServer.enqueue(new MockResponse().setBody("proxyServer"));

Channel directChannel = create(createTestConfig("http://localhost:" + server.getPort()), AGENT);
ClientConfiguration proxiedConfig = ClientConfiguration.builder()
.from(createTestConfig("http://localhost:" + server.getPort()))
.proxy(createProxySelector("localhost", proxyServer.getPort()))
.build();
Channel proxiedChannel = create(proxiedConfig, AGENT);

try (Response response =
directChannel.execute(FakeEndpoint.INSTANCE, request).get()) {
assertThat(response.code()).isEqualTo(200);
assertThat(response.body()).hasContent("server");
}
try (Response response =
proxiedChannel.execute(FakeEndpoint.INSTANCE, request).get()) {
assertThat(response.code()).isEqualTo(200);
assertThat(response.body()).hasContent("proxyServer");
}
RecordedRequest proxyRequest = proxyServer.takeRequest();
assertThat(proxyRequest.getHeader("Host")).isEqualTo("localhost:" + server.getPort());
}

@Test
public void testAuthenticatedProxy() throws Exception {
proxyServer.enqueue(new MockResponse()
.addHeader("Proxy-Authenticate", "Basic realm=test")
.setResponseCode(407)); // indicates authenticated proxy
proxyServer.enqueue(new MockResponse().setBody("proxyServer"));

ClientConfiguration proxiedConfig = ClientConfiguration.builder()
.from(createTestConfig("http://localhost:" + server.getPort()))
.proxy(createProxySelector("localhost", proxyServer.getPort()))
.proxyCredentials(BasicCredentials.of("fakeUser", "fakePassword"))
.build();
Channel proxiedChannel = create(proxiedConfig, AGENT);

try (Response response =
proxiedChannel.execute(FakeEndpoint.INSTANCE, request).get()) {
assertThat(response.code()).isEqualTo(200);
assertThat(response.body()).hasContent("proxyServer");
}
RecordedRequest firstRequest = proxyServer.takeRequest();
assertThat(firstRequest.getHeader("Proxy-Authorization")).isNull();
RecordedRequest secondRequest = proxyServer.takeRequest();
assertThat(secondRequest.getHeader("Proxy-Authorization")).isEqualTo("Basic ZmFrZVVzZXI6ZmFrZVBhc3N3b3Jk");
}

protected static ClientConfiguration createTestConfig(String... uri) {
return ClientConfiguration.builder()
.from(ClientConfigurations.of(
ImmutableList.copyOf(uri),
SslSocketFactories.createSslSocketFactory(SSL_CONFIG),
SslSocketFactories.createX509TrustManager(SSL_CONFIG)))
.maxNumRetries(0)
.build();
}

private static ProxySelector createProxySelector(String host, int port) {
return new ProxySelector() {
@Override
public List<Proxy> select(URI _uri) {
InetSocketAddress addr = new InetSocketAddress(host, port);
return ImmutableList.of(new Proxy(Proxy.Type.HTTP, addr));
}

@Override
public void connectFailed(URI _uri, SocketAddress _sa, IOException _ioe) {}
};
}

private enum FakeEndpoint implements Endpoint {
INSTANCE;

@Override
public void renderPath(Map<String, String> _params, UrlBuilder url) {
url.pathSegment("/string");
}

@Override
public HttpMethod httpMethod() {
return HttpMethod.POST;
}

@Override
public String serviceName() {
return "service";
}

@Override
public String endpointName() {
return "endpoint";
}

@Override
public String version() {
return "1.0.0";
}
}
}
Binary file not shown.
Binary file not shown.
Loading