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

fix: self signed certificate handling in v7.7.0 #2803

Merged

Conversation

tuunit
Copy link
Member

@tuunit tuunit commented Oct 5, 2024

Description

PR #2570 introduced a custom user-agent for oauth2-proxy when making requests to the configured identity provider. This introduced custom http.Client and http.Transport handling. Which wasn't reflected in the whole code base. Therefore with v7.7.0 the custom SSL handling for private CAs --provider-ca-files or allowing insecure connections --ssl-insecure-skip-verify didn't have any effect anymore because they were trying to configure golangs http.DefaultClient instead of our custom http client.

Furthermore, I checked the whole code base for custom transport settings and think I got all related to this custom http.Client.

Fixes #2802

How Has This Been Tested?

  • Tested it with local go build
  • Tested it with local docker build
  • Tested both flags together and independently
    • --provider-ca-files
    • --ssl-insecure-skip-verify
  • Tested that user-agent is still set to oauth2-proxy/<version>
  • Written new e2e testing: https://github.com/oauth2-proxy/e2e-testing/pull/13 (Not yet public or integrated into the main repository)

Checklist:

  • My change requires a change to the documentation or CHANGELOG.
  • I have updated the documentation/CHANGELOG accordingly.
  • I have created a feature (non-master) branch for my PR.
  • I have written tests for my code changes.

transport.TLSClientConfig = &tls.Config{
RootCAs: pool,
MinVersion: tls.VersionTLS12,
}

http.DefaultClient = &http.Client{Transport: transport}
requests.DefaultHTTPClient = &http.Client{Transport: transport}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This breaks the user agent handling

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
requests.DefaultHTTPClient = &http.Client{Transport: transport}
requests.DefaultHTTPClient.Next = transport

@@ -7,20 +7,22 @@ import (
)

type userAgentTransport struct {
next http.RoundTripper
Next http.RoundTripper
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of exposing Next maybe a setter like function would be a better idea

@tuunit tuunit changed the title [WIP] fix: self signed certificate handling fix: self signed certificate handling in v7.7.0 Oct 6, 2024
@github-actions github-actions bot added the docs label Oct 6, 2024
@JoelSpeed JoelSpeed merged commit d68336d into oauth2-proxy:master Oct 7, 2024
6 checks passed
@tuunit tuunit deleted the bugfix/self-signed-certificate-handling branch October 7, 2024 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: provider-ca-file command line option is not working
3 participants