Support HTTPS_PROXY env variable in authn-oidc #2902
Conversation
gl-johnson
force-pushed
the
update-openid-connect
branch
2 times, most recently
from
7356c46 to
fe1b50a
Compare
john-odonnell
force-pushed
the
update-openid-connect
branch
from
eaeee77 to
834e212
Compare
gl-johnson
force-pushed
the
update-openid-connect
branch
from
834e212 to
7ec4eb6
Compare
![codeclimate[bot]](https://avatars.githubusercontent.com/in/9403?s=60&v=4){kind=small}
| ### Fixed | ||
| - OIDC authenticator supports HTTPS_PROXY environment variable | ||
| [cyberark/conjur#2902](https://github.com/cyberark/conjur/pull/2902) | ||
| - Support plural syntax for revoke and deny |
CHANGELOG.md
Outdated
| ## [1.20.0] - 2023-08-16 | ||
|
|
||
| ### Fixed | ||
| - OIDC authenticator supports HTTPS_PROXY environment variable |
There was a problem hiding this comment.
Lists should be surrounded by blank lines
CHANGELOG.md
Outdated
| ## [1.20.0] - 2023-08-16 | ||
|
|
||
| ### Fixed | ||
| - OIDC authenticator supports HTTPS_PROXY environment variable |
There was a problem hiding this comment.
Does this apply to V1 and V2? Might want to specify authenticators plural or authentication general.
| @@ -104,7 +104,7 @@ def discovery_information(invalidate: false) | |||
| skip_nil: true | |||
| ) do | |||
| @discovery_configuration.discover!(@authenticator.provider_uri) | |||
| rescue HTTPClient::ConnectTimeoutError, Errno::ETIMEDOUT => e | |||
| rescue Errno::ETIMEDOUT => e | |||
There was a problem hiding this comment.
I was trying to figure out whether we should replace these HTTPClient errors with Faraday ones, but I'm not sure that it's necessary. The error returned by discover! can only be type OpenIDConnect::Discovery::DiscoveryFailed, which is too non-specific, I think.
There was a problem hiding this comment.
Yep it's the same exception type for basically any network error that occurs. I suppose it would be doable to parse it out of the error message but that seemed a bit hacky and unnecessary
gl-johnson
force-pushed
the
update-openid-connect
branch
from
7ec4eb6 to
384321f
Compare
| ## [1.20.0] - 2023-08-16 | ||
|
|
||
| ### Fixed | ||
| - OIDC authenticators support `https_proxy` and `HTTPS_PROXY` environment variables |
There was a problem hiding this comment.
Lists should be surrounded by blank lines
|
Code Climate has analyzed commit 384321f and detected 2 issues on this pull request. Here's the issue category breakdown:
The test coverage on the diff in this pull request is 100.0% (50% is the threshold). This pull request will bring the total coverage in the repository to 88.5% (0.0% change). View more on Code Climate. |
Desired Outcome
Fix support for
HTTPS_PROXYvariable when using authn-oidc. This issue was occuring due to the underlying HTTP client of the OpenIDConnect gem, and appears to be fixed in a more recent version when it switched to Faraday as its HTTP client.Implemented Changes
Updated OpenIDConnect gem to 2.x
Connected Issue/Story
CNJR-2390
Definition of Done
At least 1 todo must be completed in the sections below for the PR to be
merged.
Changelog
CHANGELOG update
Test coverage
changes, or
Documentation
READMEs) were updated in this PRBehavior
Security