From aaea967e05e2bcbe78e969b73b2e4d22996c38c4 Mon Sep 17 00:00:00 2001 From: Prafulla Mahindrakar Date: Tue, 24 Oct 2023 15:43:08 -0700 Subject: [PATCH] Make fetching HttpClient optional from context in PKCE auth flow (#4295) * Make fetching HttpClient optional from context in PKCE auth flow Signed-off-by: pmahindrakar-oss * Added log statement Signed-off-by: pmahindrakar-oss * added a unit test Signed-off-by: pmahindrakar-oss * nit : spellcheck Signed-off-by: pmahindrakar-oss --------- Signed-off-by: pmahindrakar-oss --- flyteidl/clients/go/admin/pkce/auth_flow_orchestrator.go | 4 ++-- .../clients/go/admin/pkce/auth_flow_orchestrator_test.go | 6 ++++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/flyteidl/clients/go/admin/pkce/auth_flow_orchestrator.go b/flyteidl/clients/go/admin/pkce/auth_flow_orchestrator.go index cf5a85671e..5d194851f2 100644 --- a/flyteidl/clients/go/admin/pkce/auth_flow_orchestrator.go +++ b/flyteidl/clients/go/admin/pkce/auth_flow_orchestrator.go @@ -2,7 +2,6 @@ package pkce import ( "context" - "errors" "fmt" "net/http" "net/url" @@ -68,7 +67,8 @@ func (f TokenOrchestrator) FetchTokenFromAuthFlow(ctx context.Context) (*oauth2. // Pass along http client used in oauth2 httpClient, ok := ctx.Value(oauth2.HTTPClient).(*http.Client) if !ok { - return nil, errors.New("Unable to retrieve httpClient used in oauth2 from context") + logger.Debugf(ctx, "using default http client instead") + httpClient = http.DefaultClient } serveMux.HandleFunc(redirectURL.Path, getAuthServerCallbackHandler(f.ClientConfig, pkceCodeVerifier, diff --git a/flyteidl/clients/go/admin/pkce/auth_flow_orchestrator_test.go b/flyteidl/clients/go/admin/pkce/auth_flow_orchestrator_test.go index 4799e3fabd..dc1c80f63a 100644 --- a/flyteidl/clients/go/admin/pkce/auth_flow_orchestrator_test.go +++ b/flyteidl/clients/go/admin/pkce/auth_flow_orchestrator_test.go @@ -2,6 +2,7 @@ package pkce import ( "context" + "os/exec" "testing" "github.com/stretchr/testify/assert" @@ -29,5 +30,10 @@ func TestFetchFromAuthFlow(t *testing.T) { refreshedToken, err := orchestrator.FetchTokenFromAuthFlow(ctx) assert.Nil(t, refreshedToken) assert.NotNil(t, err) + // Throws an exitError since browser cannot be opened. + // But this makes sure that at least code is tested till the browser action is invoked. + // Earlier change had broken this by introducing a hard dependency on the http Client from the context + _, isAnExitError := err.(*exec.ExitError) + assert.True(t, isAnExitError) }) }