This repository was archived by the owner on Mar 11, 2022. It is now read-only.
proxy support for cookie session and IAM authentication#531
Merged
Conversation
ricellis
suggested changes
Sep 6, 2021
Member
ricellis
left a comment
ricellis
left a comment
There was a problem hiding this comment.
Functionally it looks fine, but I've recommended some tidy up around the ctors. Also the files need copyright updates.
ricellis
approved these changes
Sep 7, 2021
Member
ricellis
left a comment
ricellis
left a comment
There was a problem hiding this comment.
+1 thanks
I noticed that we aren't using setProxyAuthentication in the session interceptor, but any configured ProxyAuthInterceptor will already get included when we copy the interceptors. AFAICT the non-interceptor branch is defunct (for https proxies that never worked) so I think this is fine.
mojito317
approved these changes
Sep 8, 2021
Contributor
mojito317
left a comment
mojito317
left a comment
There was a problem hiding this comment.
Overall it looks good to me! I found 2 things you have missed.
| * | ||
| * @param username The username to use when getting the cookie (not URL encoded) | ||
| * @param password The password to use when getting the cookie (not URL encoded) | ||
| * @param baseURL The base URL to use when constructing an `_session` request. |
Contributor
There was a problem hiding this comment.
@param proxyURL is missing
| @@ -42,12 +43,25 @@ public class CookieInterceptor extends CookieInterceptorBase { | |||
| */ | |||
| public CookieInterceptor(String username, String password, String baseURL) { | |||
| // Use form encoding for the user/pass submission | |||
Contributor
There was a problem hiding this comment.
This line is not necessary here anymore.
emlaver
force-pushed
the
528-connect-thru-proxy
branch
from
25b1078 to
290d1d8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist
CHANGES.md|CHANGELOG.md) or test/build only changesDescription
Enable proxy support for cookie session and IAM authentication.
fixes #528
Approach
Extend the
CookieInterceptorBaseconstructor with a proxy url parameter.Update both
IamCookieInterceptorandCookieInterceptorto support new proxy url parameter during initialization.Schema & API Changes
Security and Privacy
Testing
Manual testing against a local mock server. The following tests passed:
HttpUrlConnection) against CloudantMonitoring and Logging