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 <[email protected]> * Added log statement Signed-off-by: pmahindrakar-oss <[email protected]> * added a unit test Signed-off-by: pmahindrakar-oss <[email protected]> * nit : spellcheck Signed-off-by: pmahindrakar-oss <[email protected]> --------- Signed-off-by: pmahindrakar-oss <[email protected]>
flyteorg · Oct 24, 2023 · aaea967 · aaea967
1 parent 4fc4988
commit aaea967
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 2 deletions.
diff --git 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
@@ -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)
 	})
 }