Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TT-5777] Fix JWK fetch doesn't support proxy configuration from environment (#4102) #6474

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jonathanfoster
Copy link

Description

This PR fixes a bug in the JWT middleware that causes the JWK fetch process to not honor HTTP proxy environment variables.

Related Issue

Motivation and Context

In restricted corporate environments, it's common to require all HTTP traffic go through a proxy server. The default transport pulls proxy settings from the standard environment variables, but it looks like this was inadvertently removed to support disabling TLS cert verification when fetching JWK sets. This fix restores the ability to support standard proxy environment variables.

How This Has Been Tested

All JWT middleware unit tests pass. I didn't add unit tests for this functionality because http.ProxyFromEnvironment caches the config making tests that rely on env vars very flaky.

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Refactoring or add test (improvements in base code or adds test coverage to functionality)

Checklist

  • I ensured that the documentation is up to date
  • I explained why this PR updates go.mod in detail with reasoning why it's required
  • I would like a code coverage CI quality gate exception and have explained why

@jonathanfoster
Copy link
Author

@jeffy-mathew Anything you need from me for the review? Happy to help however I can.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant