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

Conversation

carterkozak
Copy link
Contributor

==COMMIT_MSG==
Apache channel supports proxy credentials
==COMMIT_MSG==

mesh proxy is still a todo.

@changelog-app
Copy link

changelog-app bot commented Feb 19, 2020

Generate changelog in changelog/@unreleased

Type

  • Feature
  • Improvement
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description

Apache channel supports proxy credentials

Check the box to generate changelog(s)

  • Generate changelog entry

@@ -97,4 +128,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.

.setProxyAuthenticationStrategy(NullAuthenticationStrategy.INSTANCE)
.setDefaultAuthSchemeRegistry(
RegistryBuilder.<AuthSchemeProvider>create().build());
conf.proxyCredentials().ifPresent(credentials -> builder.setDefaultCredentialsProvider(
Copy link
Contributor

Choose a reason for hiding this comment

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

could you add {} I think it makes it much more readable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ack


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.

@ferozco
Copy link
Contributor

ferozco commented Feb 20, 2020

👍

@bulldozer-bot bulldozer-bot bot merged commit 20e3469 into develop Feb 20, 2020
@bulldozer-bot bulldozer-bot bot deleted the ckozak/proxy_credentials branch February 20, 2020 03:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants