argo-workflows icon indicating copy to clipboard operation
argo-workflows copied to clipboard

The SSO integration does not consider HTTP_PROXY env vars when making requests

Open sarahhenkens opened this issue 3 years ago • 4 comments

Checklist

  • [x] Double-checked my configuration.
  • [x] Tested using the latest version.
  • [x] Used the Emissary executor.

Summary

The github.com/coreos/go-oidc/v3/oidc library by default uses an http client that is not reading the env vars for HTTP_PROXY variables. This results in SSO failing to initialize at startup.

Diagnostics

I added several log.Print statements in my fork of the release-3.3 branch and I discovered that the root cause is occuring within the oidc.NewProvider call:

func providerFactoryOIDC(ctx context.Context, issuer string) (providerInterface, error) {
	log2.Print("Attemping to create new OIDC provider")
	provider, err := oidc.NewProvider(ctx, issuer)
	log2.Print("Finished oidc.NewProvider(ctx, issuer)")
	return provider, err
}

By setting the context myself with the native http client of go, it works correctly:

func providerFactoryOIDC(ctx context.Context, issuer string) (providerInterface, error) {
	log2.Print("Attemping to create new OIDC provider")
	// Set the client context explicitly in order to use proxy configuration from environment(if any)
	// See https://github.com/coreos/go-oidc/blob/8d771559cf6e5111c9b9159810d0e4538e7cdc82/oidc.go#L43-L53
	oidc_ctx := oidc.ClientContext(ctx, &http.Client{})
	provider, err := oidc.NewProvider(oidc_ctx, issuer)
	log2.Print("Finished oidc.NewProvider(ctx, issuer)")
	return provider, err
}

Message from the maintainers:

Impacted by this bug? Give it a 👍. We prioritise the issues with the most 👍.

sarahhenkens avatar Jul 29 '22 17:07 sarahhenkens

The HandleCallback function seems to be impacted by this bug as well

sarahhenkens avatar Jul 29 '22 17:07 sarahhenkens

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is a mentoring request, please provide an update here. Thank you for your contributions.

stale[bot] avatar Aug 13 '22 05:08 stale[bot]

Still pending

sarahhenkens avatar Aug 22 '22 17:08 sarahhenkens

@sarahhenkens Do you like to submit the PR for the above change?

sarabala1979 avatar Aug 23 '22 15:08 sarabala1979

We are impacted by the same issue. When can we get a resolution please?

simox-83 avatar Sep 19 '22 13:09 simox-83

Hello. For us this is a blocker for upgrading.

tvalasek avatar Sep 19 '22 14:09 tvalasek

This is blocking us as well, thx.

Evalle avatar Sep 19 '22 15:09 Evalle

@simox-83 @tvalasek @Evalle would you like to submit fix please?

alexec avatar Oct 06 '22 16:10 alexec

@alexec @rohankmr414, this is still broken due to the custom http.Transport created inside newSso. All the OAuth2 related API calls are missing the proxy information because of:

// Create http client with TLSConfig to allow skipping of CA validation if InsecureSkipVerify is set.
httpClient := &http.Client{Transport: &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: c.InsecureSkipVerify}}}

Which is used in:

func (s *sso) HandleCallback(w http.ResponseWriter, r *http.Request)

...

oauth2Context := context.WithValue(ctx, oauth2.HTTPClient, s.httpClient)
oauth2Token, err := s.config.Exchange(oauth2Context, r.URL.Query().Get("code"), redirectOption)

The DefaultTransport of http sets the Proxy. And since s.httpClient was created with a custom Transport, its missing the proxy config. And as a result, the s.config.Exchange fails to use the http proxies configured.

sarahhenkens avatar Nov 16 '22 21:11 sarahhenkens