Improve authentication flow when migrating from heroku (#1371)

* pull dry run into new package

* automatically authenticate with heroku

* move login and agree_tos into login package

* factor out RequireLoginAndToS

* move prettyError into client

* push up interactivity check

* break out cluster package

* pas fabric and cluster addr into InteractiveRequireLoginAndToS

* mv InteractiveRequireLoginAndToS into login package

* use login.InteractiveRequireLoginAndToS in MigrateFromHeroku

Author: jordanstephens

src/cmd/cli/command/commands.go

@@ -20,6 +20,9 @@ import (
 	"github.com/DefangLabs/defang/src/pkg/cli/client/byoc/gcp"
 	"github.com/DefangLabs/defang/src/pkg/cli/compose"
 	"github.com/DefangLabs/defang/src/pkg/clouds/aws"
+	pcluster "github.com/DefangLabs/defang/src/pkg/cluster"
+	"github.com/DefangLabs/defang/src/pkg/dryrun"
+	"github.com/DefangLabs/defang/src/pkg/login"
 	"github.com/DefangLabs/defang/src/pkg/logs"
 	"github.com/DefangLabs/defang/src/pkg/mcp"
 	"github.com/DefangLabs/defang/src/pkg/migrate"
@@ -64,19 +67,6 @@ func getCluster() string {
 	return org + "@" + cluster
 }
 
-func prettyError(err error) error {
-	// To avoid printing the internal gRPC error code
-	var cerr *connect.Error
-	if errors.As(err, &cerr) {
-		term.Debug("Server error:", cerr)
-		err = errors.Unwrap(cerr)
-	}
-	if cli.IsNetworkError(err) {
-		return fmt.Errorf("%w; please check network settings and try again", err)
-	}
-	return err
-}
-
 func Execute(ctx context.Context) error {
 	if term.StdoutCanColor() {
 		restore := term.EnableANSI()
@@ -85,10 +75,10 @@ func Execute(ctx context.Context) error {
 
 	if err := RootCmd.ExecuteContext(ctx); err != nil {
 		if !(errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded)) {
-			term.Error("Error:", prettyError(err))
+			term.Error("Error:", cliClient.PrettyError(err))
 		}
 
-		if err == cli.ErrDryRun {
+		if err == dryrun.ErrDryRun {
 			return nil
 		}
 
@@ -169,14 +159,14 @@ func SetupCommands(ctx context.Context, version string) {
 
 	RootCmd.Version = version
 	RootCmd.PersistentFlags().Var(&colorMode, "color", fmt.Sprintf(`colorize output; one of %v`, allColorModes))
-	RootCmd.PersistentFlags().StringVarP(&cluster, "cluster", "s", cli.DefangFabric, "Defang cluster to connect to")
+	RootCmd.PersistentFlags().StringVarP(&cluster, "cluster", "s", pcluster.DefangFabric, "Defang cluster to connect to")
 	RootCmd.PersistentFlags().MarkHidden("cluster")
 	RootCmd.PersistentFlags().StringVar(&org, "org", os.Getenv("DEFANG_ORG"), "override GitHub organization name (tenant)")
 	RootCmd.PersistentFlags().VarP(&providerID, "provider", "P", fmt.Sprintf(`bring-your-own-cloud provider; one of %v`, cliClient.AllProviders()))
 	// RootCmd.Flag("provider").NoOptDefVal = "auto" NO this will break the "--provider aws"
 	RootCmd.PersistentFlags().BoolVarP(&verbose, "verbose", "v", false, "verbose logging") // backwards compat: only used by tail
 	RootCmd.PersistentFlags().BoolVar(&doDebug, "debug", pkg.GetenvBool("DEFANG_DEBUG"), "debug logging for troubleshooting the CLI")
-	RootCmd.PersistentFlags().BoolVar(&cli.DoDryRun, "dry-run", false, "dry run (don't actually change anything)")
+	RootCmd.PersistentFlags().BoolVar(&dryrun.DoDryRun, "dry-run", false, "dry run (don't actually change anything)")
 	RootCmd.PersistentFlags().BoolVarP(&nonInteractive, "non-interactive", "T", !hasTty, "disable interactive prompts / no TTY")
 	RootCmd.PersistentFlags().StringP("project-name", "p", "", "project name")
 	RootCmd.PersistentFlags().StringP("cwd", "C", "", "change directory before running the command")
@@ -346,6 +336,7 @@ var RootCmd = &cobra.Command{
 	Args:          cobra.NoArgs,
 	Short:         "Defang CLI is used to take your app from Docker Compose to a secure and scalable deployment on your favorite cloud in minutes.",
 	PersistentPreRunE: func(cmd *cobra.Command, args []string) (err error) {
+		ctx := cmd.Context()
 		term.SetDebug(doDebug)
 
 		// Don't track/connect the completion commands
@@ -373,9 +364,9 @@ var RootCmd = &cobra.Command{
 			}
 		}
 
-		client, err = cli.Connect(cmd.Context(), getCluster())
+		client, err = cli.Connect(ctx, getCluster())
 
-		if v, err := client.GetVersions(cmd.Context()); err == nil {
+		if v, err := client.GetVersions(ctx); err == nil {
 			version := cmd.Root().Version // HACK to avoid circular dependency with RootCmd
 			term.Debug("Fabric:", v.Fabric, "CLI:", version, "CLI-Min:", v.CliMin)
 			if hasTty && isNewer(version, v.CliMin) && !isUpgradeCommand(cmd) {
@@ -389,40 +380,10 @@ var RootCmd = &cobra.Command{
 			return nil
 		}
 
-		if err = client.CheckLoginAndToS(cmd.Context()); err != nil {
-			if nonInteractive {
-				return err
-			}
-			// Login interactively now; only do this for authorization-related errors
-			if connect.CodeOf(err) == connect.CodeUnauthenticated {
-				term.Debug("Server error:", err)
-				term.Warn("Please log in to continue.")
-				term.ResetWarnings() // clear any previous warnings so we don't show them again
-
-				defer func() { track.Cmd(nil, "Login", P("reason", err)) }()
-				if err = cli.InteractiveLogin(cmd.Context(), client, getCluster()); err != nil {
-					return err
-				}
-
-				// Reconnect with the new token
-				if client, err = cli.Connect(cmd.Context(), getCluster()); err != nil {
-					return err
-				}
-
-				if err = client.CheckLoginAndToS(cmd.Context()); err == nil { // recheck (new token = new user)
-					return nil // success
-				}
-			}
-
-			// Check if the user has agreed to the terms of service and show a prompt if needed
-			if connect.CodeOf(err) == connect.CodeFailedPrecondition {
-				term.Warn(prettyError(err))
-
-				defer func() { track.Cmd(nil, "Terms", P("reason", err)) }()
-				if err = cli.InteractiveAgreeToS(cmd.Context(), client); err != nil {
-					return err // fatal
-				}
-			}
+		if nonInteractive {
+			err = client.CheckLoginAndToS(ctx)
+		} else {
+			err = login.InteractiveRequireLoginAndToS(ctx, client, getCluster())
 		}
 
 		return err
@@ -437,11 +398,11 @@ var loginCmd = &cobra.Command{
 		trainingOptOut, _ := cmd.Flags().GetBool("training-opt-out")
 
 		if nonInteractive {
-			if err := cli.NonInteractiveGitHubLogin(cmd.Context(), client, getCluster()); err != nil {
+			if err := login.NonInteractiveGitHubLogin(cmd.Context(), client, getCluster()); err != nil {
 				return err
 			}
 		} else {
-			err := cli.InteractiveLogin(cmd.Context(), client, getCluster())
+			err := login.InteractiveLogin(cmd.Context(), client, getCluster())
 			if err != nil {
 				return err
 			}
@@ -755,7 +716,7 @@ var configDeleteCmd = &cobra.Command{
 		if err := cli.ConfigDelete(cmd.Context(), projectName, provider, names...); err != nil {
 			// Show a warning (not an error) if the config was not found
 			if connect.CodeOf(err) == connect.CodeNotFound {
-				term.Warn(prettyError(err))
+				term.Warn(cliClient.PrettyError(err))
 				return nil
 			}
 			return err
@@ -870,7 +831,7 @@ var deleteCmd = &cobra.Command{
 		if err != nil {
 			if connect.CodeOf(err) == connect.CodeNotFound {
 				// Show a warning (not an error) if the service was not found
-				term.Warn(prettyError(err))
+				term.Warn(cliClient.PrettyError(err))
 				return nil
 			}
 			return err
@@ -998,15 +959,15 @@ var tosCmd = &cobra.Command{
 		agree, _ := cmd.Flags().GetBool("agree-tos")
 
 		if agree {
-			return cli.NonInteractiveAgreeToS(cmd.Context(), client)
+			return login.NonInteractiveAgreeToS(cmd.Context(), client)
 		}
 
 		if nonInteractive {
 			printDefangHint("To agree to the terms of service, do:", cmd.CalledAs()+" --agree-tos")
 			return nil
 		}
 
-		return cli.InteractiveAgreeToS(cmd.Context(), client)
+		return login.InteractiveAgreeToS(cmd.Context(), client)
 	},
 }
 

src/cmd/cli/command/compose.go

@@ -17,6 +17,7 @@ import (
 	cliClient "github.com/DefangLabs/defang/src/pkg/cli/client"
 	"github.com/DefangLabs/defang/src/pkg/cli/client/byoc"
 	"github.com/DefangLabs/defang/src/pkg/cli/compose"
+	"github.com/DefangLabs/defang/src/pkg/dryrun"
 	"github.com/DefangLabs/defang/src/pkg/logs"
 	"github.com/DefangLabs/defang/src/pkg/term"
 	"github.com/DefangLabs/defang/src/pkg/track"
@@ -229,7 +230,7 @@ func handleComposeUpErr(ctx context.Context, err error, project *compose.Project
 
 	if strings.Contains(err.Error(), "maximum number of projects") {
 		if projectName, err2 := provider.RemoteProjectName(ctx); err2 == nil {
-			term.Error("Error:", prettyError(err))
+			term.Error("Error:", cliClient.PrettyError(err))
 			if _, err := cli.InteractiveComposeDown(ctx, provider, projectName); err != nil {
 				term.Debug("ComposeDown failed:", err)
 				printDefangHint("To deactivate a project, do:", "compose down --project-name "+projectName)
@@ -242,7 +243,7 @@ func handleComposeUpErr(ctx context.Context, err error, project *compose.Project
 		return err
 	}
 
-	term.Error("Error:", prettyError(err))
+	term.Error("Error:", cliClient.PrettyError(err))
 	track.Evt("Debug Prompted", P("composeErr", err))
 	return cli.InteractiveDebugForClientError(ctx, client, project, err)
 }
@@ -375,7 +376,7 @@ func makeComposeDownCmd() *cobra.Command {
 			if err != nil {
 				if connect.CodeOf(err) == connect.CodeNotFound {
 					// Show a warning (not an error) if the service was not found
-					term.Warn(prettyError(err))
+					term.Warn(cliClient.PrettyError(err))
 					return nil
 				}
 				return err
@@ -464,7 +465,7 @@ func makeComposeConfigCmd() *cobra.Command {
 			}
 
 			_, _, err = cli.ComposeUp(ctx, project, client, provider, compose.UploadModeIgnore, defangv1.DeploymentMode_MODE_UNSPECIFIED)
-			if !errors.Is(err, cli.ErrDryRun) {
+			if !errors.Is(err, dryrun.ErrDryRun) {
 				return err
 			}
 			return nil

src/cmd/cli/command/deploymentinfo.go

@@ -4,8 +4,8 @@ import (
 	"regexp"
 	"strings"
 
-	"github.com/DefangLabs/defang/src/pkg/cli"
 	cliClient "github.com/DefangLabs/defang/src/pkg/cli/client"
+	pcluster "github.com/DefangLabs/defang/src/pkg/cluster"
 	"github.com/DefangLabs/defang/src/pkg/term"
 	defangv1 "github.com/DefangLabs/defang/src/protos/io/defang/v1"
 )
@@ -15,7 +15,7 @@ const SERVICE_PORTAL_URL = "https://" + DEFANG_PORTAL_HOST + "/service"
 
 func printPlaygroundPortalServiceURLs(serviceInfos []*defangv1.ServiceInfo) {
 	// We can only show services deployed to the prod1 defang SaaS environment.
-	if providerID == cliClient.ProviderDefang && cluster == cli.DefaultCluster {
+	if providerID == cliClient.ProviderDefang && cluster == pcluster.DefaultCluster {
 		term.Info("Monitor your services' status in the defang portal")
 		for _, serviceInfo := range serviceInfos {
 			term.Println("   -", SERVICE_PORTAL_URL+"/"+serviceInfo.Service.Name)

src/cmd/cli/command/deploymentinfo_test.go

@@ -6,8 +6,8 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/DefangLabs/defang/src/pkg/cli"
 	cliClient "github.com/DefangLabs/defang/src/pkg/cli/client"
+	pcluster "github.com/DefangLabs/defang/src/pkg/cluster"
 	"github.com/DefangLabs/defang/src/pkg/term"
 	defangv1 "github.com/DefangLabs/defang/src/protos/io/defang/v1"
 )
@@ -22,7 +22,7 @@ func TestPrintPlaygroundPortalServiceURLs(t *testing.T) {
 	term.DefaultTerm = term.NewTerm(os.Stdin, &stdout, &stderr)
 
 	providerID = cliClient.ProviderDefang
-	cluster = cli.DefaultCluster
+	cluster = pcluster.DefaultCluster
 	printPlaygroundPortalServiceURLs([]*defangv1.ServiceInfo{
 		{
 			Service: &defangv1.Service{Name: "service1"},

src/cmd/cli/command/mcp.go

@@ -5,8 +5,8 @@ import (
 	"os"
 	"path/filepath"
 
-	"github.com/DefangLabs/defang/src/pkg/cli"
 	cliClient "github.com/DefangLabs/defang/src/pkg/cli/client"
+	"github.com/DefangLabs/defang/src/pkg/login"
 	"github.com/DefangLabs/defang/src/pkg/mcp"
 	"github.com/DefangLabs/defang/src/pkg/mcp/resources"
 	"github.com/DefangLabs/defang/src/pkg/mcp/tools"
@@ -84,7 +84,7 @@ set_config - This tool sets or updates configuration variables for a deployed ap
 			term.Debug("Function invoked: cli.InteractiveLoginInsideDocker")
 
 			go func() {
-				if err := cli.InteractiveLoginInsideDocker(cmd.Context(), getCluster(), authPort); err != nil {
+				if err := login.InteractiveLoginInsideDocker(cmd.Context(), getCluster(), authPort); err != nil {
 					term.Error("Failed to start auth server", "error", err)
 				}
 			}()

src/pkg/cli/bootstrap.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/DefangLabs/defang/src/pkg"
 	"github.com/DefangLabs/defang/src/pkg/cli/client"
+	"github.com/DefangLabs/defang/src/pkg/dryrun"
 	"github.com/DefangLabs/defang/src/pkg/logs"
 	"github.com/DefangLabs/defang/src/pkg/term"
 )
@@ -19,8 +20,8 @@ func BootstrapCommand(ctx context.Context, projectName string, verbose bool, pro
 	} else {
 		term.Infof("Running CD command %q in project %q", cmd, projectName)
 	}
-	if DoDryRun {
-		return ErrDryRun
+	if dryrun.DoDryRun {
+		return dryrun.ErrDryRun
 	}
 
 	since := time.Now()
@@ -67,8 +68,8 @@ func SplitProjectStack(name string) (projectName string, stackName string) {
 
 func BootstrapLocalList(ctx context.Context, provider client.Provider) error {
 	term.Debug("Running CD list")
-	if DoDryRun {
-		return ErrDryRun
+	if dryrun.DoDryRun {
+		return dryrun.ErrDryRun
 	}
 
 	stacks, err := provider.BootstrapList(ctx)

src/pkg/cli/client/pretty_error.go

@@ -0,0 +1,41 @@
+package client
+
+import (
+	"errors"
+	"fmt"
+	"strings"
+
+	"github.com/DefangLabs/defang/src/pkg/term"
+	"github.com/bufbuild/connect-go"
+)
+
+func PrettyError(err error) error {
+	// To avoid printing the internal gRPC error code
+	var cerr *connect.Error
+	if errors.As(err, &cerr) {
+		term.Debug("Server error:", cerr)
+		err = errors.Unwrap(cerr)
+	}
+	if IsNetworkError(err) {
+		return fmt.Errorf("%w; please check network settings and try again", err)
+	}
+	return err
+}
+
+func IsNetworkError(err error) bool {
+	if err == nil {
+		return false
+	}
+	errStr := err.Error()
+	lastColon := strings.LastIndexByte(errStr, ':')
+	switch errStr[lastColon+1:] { // +1 to skip the colon and handle the case where there is no colon
+	case " connection refused",
+		" i/o timeout",
+		" network is unreachable",
+		" no such host",
+		" unexpected EOF",
+		" device or resource busy":
+		return true
+	}
+	return false
+}

src/pkg/cli/common.go

@@ -2,20 +2,13 @@ package cli
 
 import (
 	"encoding/json"
-	"errors"
 
 	"github.com/DefangLabs/defang/src/pkg/term"
 	"google.golang.org/protobuf/encoding/protojson"
 	"google.golang.org/protobuf/proto"
 	"gopkg.in/yaml.v3"
 )
 
-var (
-	DoDryRun = false
-
-	ErrDryRun = errors.New("dry run")
-)
-
 func MarshalPretty(root string, data proto.Message) ([]byte, error) {
 	// HACK: convert to JSON first so we respect the json tags (like "omitempty")
 	bytes, err := protojson.Marshal(data)

src/pkg/cli/composeDown.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/AlecAivazis/survey/v2"
 	"github.com/DefangLabs/defang/src/pkg/cli/client"
+	"github.com/DefangLabs/defang/src/pkg/dryrun"
 	"github.com/DefangLabs/defang/src/pkg/term"
 	"github.com/DefangLabs/defang/src/pkg/track"
 	"github.com/DefangLabs/defang/src/pkg/types"
@@ -17,8 +18,8 @@ import (
 func ComposeDown(ctx context.Context, projectName string, c client.FabricClient, provider client.Provider, names ...string) (types.ETag, error) {
 	term.Debugf("Destroying project %q %q", projectName, names)
 
-	if DoDryRun {
-		return "", ErrDryRun
+	if dryrun.DoDryRun {
+		return "", dryrun.ErrDryRun
 	}
 
 	if len(names) == 0 {