Gun adapter proxy auths #424
Merged
Conversation
This file contains 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
added 5 commits
February 25, 2020 21:05
|
@alex-strizhakov 👀 🙏 |
|
@alex-strizhakov Could you take a look at this PR? |
alex-strizhakov
suggested changes
Nov 6, 2020
lib/tesla/adapter/gun.ex
Outdated
| @@ -38,6 +38,8 @@ if Code.ensure_loaded?(:gun) do | |||
| - `:close_conn` - Close connection or not after receiving full response body. Is used for reusing gun connections. Defaults to `true`. | |||
| - `:certificates_verification` - Add SSL certificates verification. [erlang-certifi](https://github.com/certifi/erlang-certifi) [ssl_verify_fun.erl](https://github.com/deadtrickster/ssl_verify_fun.erl) | |||
| - `:proxy` - Proxy for requests. **Socks proxy are supported only for gun master branch**. Examples: `{'localhost', 1234}`, `{{127, 0, 0, 1}, 1234}`, `{:socks5, 'localhost', 1234}`. | |||
| NOTE: By default GUN uses TLS as transport if the specified port is 443, if TLS is required for proxy conection on another port please specify transport using the Gun options below otherwise tcp will be used | |||
There was a problem hiding this comment.
Suggested change
| NOTE: By default GUN uses TLS as transport if the specified port is 443, if TLS is required for proxy conection on another port please specify transport using the Gun options below otherwise tcp will be used | |
| NOTE: By default GUN uses TLS as transport if the specified port is 443, if TLS is required for proxy connection on another port please specify transport using the Gun options below otherwise tcp will be used |
There was a problem hiding this comment.
Nice catch, corrected :)
lib/tesla/adapter/gun.ex
Outdated
| @@ -345,6 +366,18 @@ if Code.ensure_loaded?(:gun) do | |||
|
|
|||
| defp tunnel_tls_opts(opts, _, _), do: opts | |||
|
|
|||
| defp add_proxy_auth_credentials(opts, %{proxy_auth: {username, password}}) | |||
| when not is_nil(username) and not is_nil(password), | |||
There was a problem hiding this comment.
maybe is_binary/1 guard will be more suitable here for checking username and password?
There was a problem hiding this comment.
You are absolutely right, corrected.
alex-strizhakov
approved these changes
Dec 4, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR aims to address the lack of ability to use Basic auth when using proxy (regular or Socks) via GUN adapter.
Also a pattern match is added to not force TLS in case proxy is used, and leave it to GUN to use TLS if the proxy port is 443, otherwise TCP will be used.
The combination of TLS and a port other than 443 for proxy can be accomplished by passing the transport option when creating the Tesla Client and adding it as Adapter opt.