From 850cf27e4cea5f5cb5db5757a03ad842d193c9ea Mon Sep 17 00:00:00 2001 From: KevyVo Date: Fri, 18 Apr 2025 12:31:24 -0700 Subject: [PATCH] Found that auth was not flowing prompt bool The higher level function(InteractiveLogin)-> (githubAuthService.login) -> GitHubAuth interface -> (githubAuthService.login) -> github.StartAuthCodeFlow need the prompt bool to be pass all the way down. I also reverse the prompt logic because I felt it was more intuitive to have the prompt like this. --- src/cmd/cli/command/commands.go | 4 ++-- src/pkg/cli/login.go | 10 +++++----- src/pkg/cli/login_test.go | 14 +++++++------- src/pkg/github/auth.go | 2 +- src/pkg/mcp/tools/login.go | 2 +- 5 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/cmd/cli/command/commands.go b/src/cmd/cli/command/commands.go index cebc5cd02..5aa4333d0 100644 --- a/src/cmd/cli/command/commands.go +++ b/src/cmd/cli/command/commands.go @@ -369,7 +369,7 @@ var RootCmd = &cobra.Command{ term.Warn("Please log in to continue.") defer func() { track.Cmd(nil, "Login", P("reason", err)) }() - if err = cli.InteractiveLogin(cmd.Context(), client, gitHubClientId, getCluster()); err != nil { + if err = cli.InteractiveLogin(cmd.Context(), client, gitHubClientId, getCluster(), false); err != nil { return err } @@ -407,7 +407,7 @@ var loginCmd = &cobra.Command{ return err } } else { - err := cli.InteractiveLogin(cmd.Context(), client, gitHubClientId, getCluster()) + err := cli.InteractiveLogin(cmd.Context(), client, gitHubClientId, getCluster(), false) if err != nil { return err } diff --git a/src/pkg/cli/login.go b/src/pkg/cli/login.go index 78b578d18..9069371ca 100644 --- a/src/pkg/cli/login.go +++ b/src/pkg/cli/login.go @@ -37,17 +37,17 @@ func GetExistingToken(fabric string) string { } type GitHubAuth interface { - login(ctx context.Context, client client.FabricClient, gitHubClientId, fabric string) (string, error) + login(ctx context.Context, client client.FabricClient, gitHubClientId, fabric string, prompt bool) (string, error) } type GitHubAuthService struct{} func (g GitHubAuthService) login( - ctx context.Context, client client.FabricClient, gitHubClientId, fabric string, + ctx context.Context, client client.FabricClient, gitHubClientId, fabric string, prompt bool, ) (string, error) { term.Debug("Logging in to", fabric) - code, err := github.StartAuthCodeFlow(ctx, gitHubClientId, false) + code, err := github.StartAuthCodeFlow(ctx, gitHubClientId, prompt) if err != nil { return "", err } @@ -68,8 +68,8 @@ func saveAccessToken(fabric, at string) error { return nil } -func InteractiveLogin(ctx context.Context, client client.FabricClient, gitHubClientId, fabric string) error { - at, err := githubAuthService.login(ctx, client, gitHubClientId, fabric) +func InteractiveLogin(ctx context.Context, client client.FabricClient, gitHubClientId, fabric string, prompt bool) error { + at, err := githubAuthService.login(ctx, client, gitHubClientId, fabric, prompt) if err != nil { return err } diff --git a/src/pkg/cli/login_test.go b/src/pkg/cli/login_test.go index 3ff5e3100..ea7962eb5 100644 --- a/src/pkg/cli/login_test.go +++ b/src/pkg/cli/login_test.go @@ -42,13 +42,13 @@ func TestGetExistingToken(t *testing.T) { type MockGitHubAuthService struct { GitHubAuthService - MockLogin func(ctx context.Context, client client.FabricClient, gitHubClientId, fabric string) (string, error) + MockLogin func(ctx context.Context, client client.FabricClient, gitHubClientId, fabric string, prompt bool) (string, error) } func (g MockGitHubAuthService) login( - ctx context.Context, client client.FabricClient, gitHubClientId, fabric string, + ctx context.Context, client client.FabricClient, gitHubClientId, fabric string, prompt bool, ) (string, error) { - return g.MockLogin(ctx, client, gitHubClientId, fabric) + return g.MockLogin(ctx, client, gitHubClientId, fabric, prompt) } func TestInteractiveLogin(t *testing.T) { @@ -69,13 +69,13 @@ func TestInteractiveLogin(t *testing.T) { t.Run("Expect accessToken to be stored when InteractiveLogin() succeeds", func(t *testing.T) { githubAuthService = MockGitHubAuthService{ MockLogin: func( - ctx context.Context, client client.FabricClient, gitHubClientId, fabric string, + ctx context.Context, client client.FabricClient, gitHubClientId, fabric string, prompt bool, ) (string, error) { return accessToken, nil }, } - err := InteractiveLogin(context.Background(), client.MockFabricClient{}, "github-client-id", fabric) + err := InteractiveLogin(context.Background(), client.MockFabricClient{}, "github-client-id", fabric, false) if err != nil { t.Fatalf("expected no error, got %v", err) } @@ -90,12 +90,12 @@ func TestInteractiveLogin(t *testing.T) { t.Run("Expect error when InteractiveLogin fails", func(t *testing.T) { githubAuthService = MockGitHubAuthService{ - MockLogin: func(ctx context.Context, client client.FabricClient, gitHubClientId, fabric string) (string, error) { + MockLogin: func(ctx context.Context, client client.FabricClient, gitHubClientId, fabric string, prompt bool) (string, error) { return "", errors.New("test-error") }, } - err := InteractiveLogin(context.Background(), client.MockFabricClient{}, "github-client-id", fabric) + err := InteractiveLogin(context.Background(), client.MockFabricClient{}, "github-client-id", fabric, false) if err == nil { t.Fatalf("expected no error, got %v", err) } diff --git a/src/pkg/github/auth.go b/src/pkg/github/auth.go index cab35c3e6..daba8f3b7 100644 --- a/src/pkg/github/auth.go +++ b/src/pkg/github/auth.go @@ -107,7 +107,7 @@ func StartAuthCodeFlow(ctx context.Context, clientId string, prompt bool) (strin defer term.Print(strings.Repeat(" ", n), "\r") // TODO: use termenv to clear line // TODO:This is used to open the browser for GitHub Auth before blocking - if !prompt { + if prompt { browser.OpenURL(server.URL) } diff --git a/src/pkg/mcp/tools/login.go b/src/pkg/mcp/tools/login.go index 65c002452..dd212cb94 100644 --- a/src/pkg/mcp/tools/login.go +++ b/src/pkg/mcp/tools/login.go @@ -23,7 +23,7 @@ func setupLoginTool(s *server.MCPServer, client client.GrpcClient, cluster strin term.Info("Adding login tool handler") s.AddTool(loginTool, func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { // Test token - err := cli.InteractiveLogin(ctx, client, gitHubClientId, cluster) + err := cli.InteractiveLogin(ctx, client, gitHubClientId, cluster, true) if err != nil { return mcp.NewToolResultText(fmt.Sprintf("Failed to login: %v", err)), nil }