docusign / docusign-esign-java-client

The Official Docusign Java Client Library used to interact with the eSignature REST API. Send, sign, and approve documents using this client.
https://javadoc.io/doc/com.docusign/docusign-esign-java/latest/index.html
MIT License
105 stars 96 forks source link

docusign client lib using old jersey client lib, and doesn't support proxy #86

Closed xuxiankun closed 5 years ago

xuxiankun commented 6 years ago

Docusign client should upgrade to new version of jersey client, and adding support proxy setting.

deshpandepd commented 6 years ago

It is using jeresey 1.19 version which is older one

xuxiankun commented 6 years ago

in our system, the system don't have direct access to internet, we need to use proxy server to access internet(docusign api), the current docusign java client, don't have the ability to support proxy sever to access docusign api.

mmallis87 commented 6 years ago

@xuxiankun @deshpandepd I created an internal ticket and we will be prioritizing it very soon. In the meanwhile feel free to share the jersey version that you would like to be used in this SDK. Also we appreciate pull requests.

deshpandepd commented 6 years ago

Main problem is proxy support if it is getting solve then jersey version is not a problem

deshpandepd commented 6 years ago

Latest version of jersey client is 2.27 which you can use for supporting

mmallis87 commented 5 years ago

@deshpandepd we are focusing on fixing the proxy issue right now so we will leave upgrading to jersey 2.27 for later because it seems to require a large code/dependencies change.

It will be helpful if you can share with us more information about what errors do you get and/or what is your proxy setup (does it use https or http, does it require credentials, does your corporate network restrict reaching out to all IPs/ports except the proxy...). Of course you don't need to share any private information.

deshpandepd commented 5 years ago

We are using DocuSign for our product so ideally I would like to consider all scenario like http or https proxy and also credentials based proxy as well

Because depending on customer environment we need to support

Thanks Prashant

On Thu, Oct 18, 2018, 2:04 AM Majid Mallis notifications@github.com wrote:

@deshpandepd https://github.com/deshpandepd we are focusing on fixing the proxy issue right now so we will leave upgrading to jersey 2.27 for later because it seems to require a large code/dependencies change.

It will be helpful if you can share with us more information about what errors do you get and/or what is your proxy setup (does it use https or http, does it require credentials, does your corporate network restrict reaching out to all IPs/ports except the proxy...). Of course you don't need to share any private information.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/docusign/docusign-java-client/issues/86#issuecomment-430778920, or mute the thread https://github.com/notifications/unsubscribe-auth/AkysgzQdTmHbdIJvKGLGcOPcK5hiFN0fks5ul5RxgaJpZM4WltAQ .

mmallis87 commented 5 years ago

@xuxiankun Is there a way you can help testing this branch behind a proxy: https://github.com/docusign/docusign-java-client/tree/httpProxy The attempted fix is included in this commit: https://github.com/docusign/docusign-java-client/commit/efaba3bbc9d25dab0bffc508742dd1f8ca231af3

psytester commented 5 years ago

I would like to contribute at this issue. The Proxy setup is not complete (1) for all use cases and implementation seems to be faulty (2) and make it more flexible if different proxies are used by different libraries in same application (3).

  1. used Proxy settings are only those 4 basics:
    
    http.proxyHost
    http.proxyPort

https.proxyHost https.proxyPort


missing are:
1a) the case where the client needs some authentication at proxy
for non TLS http connections:

http.proxyUser http.proxyPassword

for TLS https connections:

https.proxyUser https.proxyPassword


1b) List of hosts and IPs to keep network internally without any Proxy usage:

http.nonProxyHosts (setting is used for both http and httpS)


2. Implementation is wrong at Proxy Port setup.
Usually a proxy does not listen on port 80 nor 443. For example the default Squid Proxy config is using port 3128.
ALL traffic to proxy needs to go to this port, but your code is setting up in case of HTTP Port 80 and for HTTPS port 443
Do not set code internally the port and don't differentiate them.
Let the customer code choose the values by initialize the API

3. for some issues our application is using different interfaces and frameworks. Jersey is one of them and this connections goes over Proxy A.
Another part is using Apache CFX and those connections are going via Proxy B.
Your code expects the setting via common java Systemproperties, better would be to set them by calling the API in client customer code with some values.
At the end I'm able to setup in my own code and configuration something like:
For the ds connection part

dsProxy.http.proxyHost = not used as HTTPS is used only / enforced dsProxy.http.proxyPort = not used as HTTPS is used only / enforced dsProxy.https.proxyHost = Proxy A dsProxy.https.proxyPort = 3118

dsProxy.http.proxyUser = dsProxy.http.proxyPassword = dsProxy.https.proxyUser = username dsProxy.https.proxyPassword = password

dsProxy.http.nonProxyHosts = all network internal systems and special hosts

and for the CFX connection part:

cfxProxy.http.proxyHost = Proxy B cfxProxy.http.proxyPort = 3118 cfxProxy.https.proxyHost = Proxy B cfxProxy.https.proxyPort = 3118

cfxProxy.http.proxyUser = cfxProxy.http.proxyPassword = cfxProxy.https.proxyUser = cfxProxy.https.proxyPassword =

cfxProxy.http.nonProxyHosts = all network internal systems and special hosts



I hope you got my point
mmallis87 commented 5 years ago

@psytester Thank you for contributing to this. We appreciate if you can help us testing the feature branch.

The goal for this first version of the proxy support is to unlock developers who would like to use this API client behind http(s) proxies. It's by no means a comprehensive solution for all proxy cases. So at the moment we won't have support for SOCKS or FTP protocols. Also we won't have support for nonProxyHosts or useSystemProxies.

For your comments, here is my feedback: 1a) should be fixed by https://github.com/docusign/docusign-java-client/commit/b29225e4bb2edb46da2a6d94c7d5720bbbbfb224 1b) like said, http.nonProxyHosts is out of the scope of this version. The caller has the responsibility of checking the list of hosts that should be accessed without going through the proxy and turning the proxy settings off in case DocuSign API host is in the list. 2) this API client does not hardcode the http port to 80 (nor the https port to 443). The initial idea was to use the default HTTP protocol port but based on your comment, we think it's better to not set any default value. 3) we understand that different DocuSign customers have different setups and we try to support as many scenarios and cases as possible but we also try to keep the balance between that and making sure that the API client interface stays simple. For that reason, we decided to use System.properties. So in your case, you will have the responsibility of calling System.setProperty before consuming this API client methods and, maybe, System.clearProperty after you're done.

mmallis87 commented 5 years ago

Hi, We have fixed the issues with new eSignature API version and new SDK release candidate. Please have a look at v2.9.0 of the Maven/Gradle package.

You can now use https(s) proxy with this SDK. We have a 2.9.0 branch where you can check the code.

Let us know if this resolves the issue.

mmallis87 commented 5 years ago

@xuxiankun @deshpandepd @psytester Please note that this ticket will be closed in 3 days if we don't hear from you.

psytester commented 5 years ago

@mmallis87 you wrote ....goal for this first version of the proxy support.... We can not work with that approach in production system. For some (security and performance) reasons we need the http.nonProxyHosts settings. It's safer to keep internal traffic really internal instead of going through a proxy which decides to go back to the internal network.

mmallis87 commented 5 years ago

@psytester version 2.10.0-RC1 has support for http.nonProxyHosts setting please give it a try. You can see the code change as part of #108 .

deshpandepd commented 5 years ago

Thank you so much ..Will try at our end

On Fri 3 May, 2019, 2:12 PM Majid Mallis, notifications@github.com wrote:

@psytester https://github.com/psytester version 2.10.0-RC1 has support for http.nonProxyHosts setting please give it a try. You can see the code change as part of #108 https://github.com/docusign/docusign-java-client/pull/108 .

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/docusign/docusign-java-client/issues/86#issuecomment-489012241, or mute the thread https://github.com/notifications/unsubscribe-auth/AJGKZA7U5KOQWGVYBQ7XFQLPTP3ILANCNFSM4FUW2AIA .

mmallis87 commented 5 years ago

Please reopen if you find an issue. For new feature requests, open a new issue.