feat(rest-connector): Add support for proxy auth #3894
Conversation
|
|
||
| # Allow unauthenticated users on /path | ||
| acl no_auth_url url_regex -i path |
There was a problem hiding this comment.
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.
ztefanie
changed the title
ztefanie
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
.../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
|
@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/ |
.../http/http-base/src/main/java/io/camunda/connector/http/base/client/apache/ProxyHandler.java
Outdated
Show resolved
Hide resolved
.../http/http-base/src/main/java/io/camunda/connector/http/base/client/apache/ProxyHandler.java
Outdated
Show resolved
Hide resolved
.../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
.../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
.../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
| ProxyHandler proxyHandler = | ||
| new ProxyHandler(apacheRequest.getScheme(), apacheRequest.getUri().getHost()); |
There was a problem hiding this comment.
Looks like we are creating a new proxy handler for every new request. I think this should be done once.
There was a problem hiding this comment.
Good point. Moved it to HttpClient
| 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; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
| if (sourceIsSystemProperties) { | ||
| return new SystemDefaultRoutePlanner( | ||
| DefaultSchemePortResolver.INSTANCE, ProxySelector.getDefault()); | ||
| } else if (proxyHost != null) { | ||
| return new DefaultProxyRoutePlanner(proxyHost); | ||
| } | ||
| return null; |
There was a problem hiding this comment.
Would be great to add some debug logs here. Otherwise it might be intransparent which config was applied.
There was a problem hiding this comment.
Added debug logging.
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
no milestonelabel.