feat(rest-connector): Add support for proxy auth #3894

ztefanie · 2025-01-31T15:08:22Z

Description

Added support for setting proxy via env var and to add proxy auth.

Proxy config can be set via system properties or via env vars. Setting via env vars is currently handled by using an RFC3986 string, e.g. http://my-user:demo@localhost:3128/
This means it is currently not possible to set nonProxyHosts via env vars and username and passwords must be url safe.
I think we could also change this, with some effort, to setting multiple env vars. I decided to go for this version, assuming setting via env vars is more for testing / fast setups, but not complex production setups, as setting passwords in ENV vars is worse than setting in system properties. If you disagree please let me know, then i will adjust it.

I tested different use cases manually, with the following results:

I tried to add Unit tests for nonProxyHosts but couldn't get it working as mocking only one host / localhost is possible with Wiremock. If you think nonProxyHost unit test should be added, please give me a hint how to achieve this.

Not tested for https.

I will add this to the docs, once this is approved or at least we agreed on how to set it with env vars.

Related issues

closes https://github.com/camunda/team-connectors/issues/989

Checklist

PR has a milestone or the no milestone label.

ztefanie · 2025-01-31T15:10:19Z

connectors/http/http-base/src/test/resources/squid.conf


+# Allow unauthenticated users on /path
+acl no_auth_url url_regex -i path


so with this setting every path is needs username and password except "/path". I would rather have only "/protected" be protected and everything else allowed, but I am not sure if this is possible. So if you have an idea let me know.

.../http/http-base/src/main/java/io/camunda/connector/http/base/client/apache/ProxyHandler.java

sbuettner · 2025-02-03T07:58:48Z

@ztefanie We currently only document environment variables for our component configuration and we should therefore continue to support using them also for this use-case: https://docs.camunda.io/docs/next/self-managed/connectors-deployment/connectors-configuration/

…nproxyhosts

.../http/http-base/src/main/java/io/camunda/connector/http/base/client/apache/ProxyHandler.java

+
+  private void setFromEnvVars() {
+    host = System.getenv("CONNECTOR_" + protocol.toUpperCase() + "_PROXY_HOST");
+    port = Integer.parseInt(System.getenv("CONNECTOR_" + protocol.toUpperCase() + "_PROXY_PORT"));


.../http/http-base/src/main/java/io/camunda/connector/http/base/client/apache/ProxyHandler.java

+
+  private void setFromSystemProperties() {
+    host = System.getProperty(protocol + ".proxyHost");
+    port = Integer.parseInt(System.getProperty(protocol + ".proxyPort"));


.../http/http-base/src/main/java/io/camunda/connector/http/base/client/apache/ProxyHandler.java

+    sourceIsSystemProperties = true;
+  }
+
+  public CredentialsProvider getCredentialsProvider(String uncheckedProtocol) {


To fix the problem, we need to remove the unused parameter uncheckedProtocol from the getCredentialsProvider method. This involves:

Removing the parameter from the method signature.

Updating any calls to this method to no longer pass this parameter.

This change should be made in the file connectors/http/http-base/src/main/java/io/camunda/connector/http/base/client/apache/ProxyHandler.java.

.../http/http-base/src/main/java/io/camunda/connector/http/base/client/apache/ProxyHandler.java

+    return provider;
+  }
+
+  public HttpHost getProxyHost(String uncheckedProtocol, String requestUri) {


To fix the problem, we should remove the unused requestUri parameter from the getProxyHost method. This will simplify the method signature and eliminate any confusion about the necessity of the parameter.

Remove the requestUri parameter from the getProxyHost method signature.

Update any calls to the getProxyHost method to remove the requestUri argument.

.../http/http-base/src/main/java/io/camunda/connector/http/base/client/apache/ProxyHandler.java

+        : new HttpHost(fallbackOnHttpForInvalidProtocol(uncheckedProtocol), host, port);
+  }
+
+  private boolean doesTargetMatchNonProxy(String protocol, String requestUri) {


To fix the problem, we need to remove the unused protocol parameter from the doesTargetMatchNonProxy method. This involves updating the method signature and any calls to this method to exclude the protocol parameter. This change will simplify the method's interface and eliminate the unnecessary parameter.

.../http/http-base/src/main/java/io/camunda/connector/http/base/client/apache/ProxyHandler.java

+                    requestUri.matches(nonProxyHost.replace(".", "\\.").replace("*", ".*"))));
+  }
+
+  public HttpRoutePlanner getRoutePlanner(String uncheckedProtocol, HttpHost proxyHost) {


To fix the problem, we need to remove the unused parameter uncheckedProtocol from the method getRoutePlanner. This involves updating the method signature and ensuring that any calls to this method are also updated to remove the unnecessary argument.

Remove the uncheckedProtocol parameter from the getRoutePlanner method signature.

Update any calls to getRoutePlanner to no longer pass the uncheckedProtocol argument.

sbuettner · 2025-02-10T07:57:55Z

...-base/src/main/java/io/camunda/connector/http/base/client/apache/CustomApacheHttpClient.java

+      ProxyHandler proxyHandler =
+          new ProxyHandler(apacheRequest.getScheme(), apacheRequest.getUri().getHost());


Looks like we are creating a new proxy handler for every new request. I think this should be done once.

Good point. Moved it to HttpClient

sbuettner · 2025-02-10T08:01:12Z

.../http/http-base/src/main/java/io/camunda/connector/http/base/client/apache/ProxyHandler.java

+    BasicCredentialsProvider provider = new BasicCredentialsProvider();
+    if (StringUtils.isNotBlank(host)
+        && StringUtils.isNotBlank(user)
+        && ArrayUtils.isNotEmpty(password)) {
+      provider.setCredentials(
+          new AuthScope(host, port), new UsernamePasswordCredentials(user, password));
+    }
+    return provider;


What happens if the password or another entry is empty? Should we log a warning or something?

Otherwise this will silently lead to a credentials provider without credentials.

Yes, if password or another entry is empty a credentials provider without credentials is created. This is expected and wanted behavior, because some proxy do not require credentials. Requests to these proxies are than handled without crendentials with success. If no credentials are provided, but expected by the proxy, the execution will fail with "Proxy Authentication required" error.

sbuettner · 2025-02-10T08:02:58Z

.../http/http-base/src/main/java/io/camunda/connector/http/base/client/apache/ProxyHandler.java

+    if (sourceIsSystemProperties) {
+      return new SystemDefaultRoutePlanner(
+          DefaultSchemePortResolver.INSTANCE, ProxySelector.getDefault());
+    } else if (proxyHost != null) {
+      return new DefaultProxyRoutePlanner(proxyHost);
+    }
+    return null;


Would be great to add some debug logs here. Otherwise it might be intransparent which config was applied.

989: Add support for proxy auth

84eca2d

ztefanie added this to the 8.7.0-alpha4 milestone Jan 31, 2025

ztefanie requested a review from a team as a code owner January 31, 2025 15:08

ztefanie commented Jan 31, 2025

View reviewed changes

ztefanie requested a review from johnBgood January 31, 2025 15:10

ztefanie changed the title ~~989: Add support for proxy auth~~ feat(rest-connector: Add support for proxy auth Jan 31, 2025

ztefanie changed the title ~~feat(rest-connector: Add support for proxy auth~~ feat(rest-connector): Add support for proxy auth Jan 31, 2025

github-advanced-security bot found potential problems Jan 31, 2025

View reviewed changes

.../http/http-base/src/main/java/io/camunda/connector/http/base/client/apache/ProxyHandler.java Fixed Show fixed Hide fixed

.../http/http-base/src/main/java/io/camunda/connector/http/base/client/apache/ProxyHandler.java Fixed Show fixed Hide fixed

johnBgood and others added 4 commits February 3, 2025 15:46

Merge branch 'main' into 989-add-support-for-proxy-auth

aeefe37

Merge branch 'main' into 989-add-support-for-proxy-auth

3cb38c8

989: Split proxy settings via env vars to multiple vars to support no…

43a5d25

…nproxyhosts

989: Improve exception handling

38f6516

sbuettner reviewed Feb 5, 2025

View reviewed changes

.../http/http-base/src/main/java/io/camunda/connector/http/base/client/apache/ProxyHandler.java Outdated Show resolved Hide resolved

sbuettner reviewed Feb 5, 2025

View reviewed changes

.../http/http-base/src/main/java/io/camunda/connector/http/base/client/apache/ProxyHandler.java Outdated Show resolved Hide resolved

sbuettner modified the milestones: 8.7.0-alpha4, 8.8.0-alpha2 Feb 5, 2025

989: make proxy handler non static

e6db9d7

ztefanie requested a review from sbuettner February 7, 2025 13:28

github-advanced-security bot found potential problems Feb 7, 2025

View reviewed changes

sbuettner reviewed Feb 10, 2025

View reviewed changes

@@ -78,3 +78,3 @@
-              public HttpHost getProxyHost(String uncheckedProtocol, String requestUri) {
+              public HttpHost getProxyHost(String uncheckedProtocol) {
                 return host == null

@@ -34,3 +34,3 @@
                     setFromEnvVars();
-                    if (doesTargetMatchNonProxy(protocol, requestUrl)) {
+                    if (doesTargetMatchNonProxy(requestUrl)) {
                       host = null;
@@ -84,3 +84,3 @@
-              private boolean doesTargetMatchNonProxy(String protocol, String requestUri) {
+              private boolean doesTargetMatchNonProxy(String requestUri) {
                 return (nonProxyHosts != null

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(rest-connector): Add support for proxy auth #3894

feat(rest-connector): Add support for proxy auth #3894

ztefanie commented Jan 31, 2025 •

edited

Loading

ztefanie Jan 31, 2025

sbuettner commented Feb 3, 2025

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

sbuettner Feb 10, 2025

ztefanie Feb 10, 2025

sbuettner Feb 10, 2025

ztefanie Feb 10, 2025

sbuettner Feb 10, 2025


		# Allow unauthenticated users on /path
		acl no_auth_url url_regex -i path

		ProxyHandler proxyHandler =
		new ProxyHandler(apacheRequest.getScheme(), apacheRequest.getUri().getHost());

feat(rest-connector): Add support for proxy auth #3894

Are you sure you want to change the base?

feat(rest-connector): Add support for proxy auth #3894

Conversation

ztefanie commented Jan 31, 2025 • edited Loading

Description

Related issues

Checklist

ztefanie Jan 31, 2025

Choose a reason for hiding this comment

sbuettner commented Feb 3, 2025

sbuettner Feb 10, 2025

Choose a reason for hiding this comment

ztefanie Feb 10, 2025

Choose a reason for hiding this comment

sbuettner Feb 10, 2025

Choose a reason for hiding this comment

ztefanie Feb 10, 2025

Choose a reason for hiding this comment

sbuettner Feb 10, 2025

Choose a reason for hiding this comment

ztefanie commented Jan 31, 2025 •

edited

Loading