detect if we're using compatible tokens for MPG commands (#4534)

* detect if we're using compatible tokens for MPG commands

Author: jphenow

internal/command/mpg/attach.go

@@ -43,6 +43,11 @@ func newAttach() *cobra.Command {
 }
 
 func runAttach(ctx context.Context) error {
+	// Check token compatibility early
+	if err := validateMPGTokenCompatibility(ctx); err != nil {
+		return err
+	}
+
 	var (
 		clusterId  = flag.FirstArg(ctx)
 		appName    = appconfig.NameFromContext(ctx)

internal/command/mpg/connect.go

@@ -33,6 +33,11 @@ func newConnect() (cmd *cobra.Command) {
 }
 
 func runConnect(ctx context.Context) (err error) {
+	// Check token compatibility early
+	if err := validateMPGTokenCompatibility(ctx); err != nil {
+		return err
+	}
+
 	io := iostreams.FromContext(ctx)
 
 	localProxyPort := "16380"

internal/command/mpg/create.go

@@ -69,6 +69,11 @@ func newCreate() *cobra.Command {
 }
 
 func runCreate(ctx context.Context) error {
+	// Check token compatibility early
+	if err := validateMPGTokenCompatibility(ctx); err != nil {
+		return err
+	}
+
 	var (
 		io      = iostreams.FromContext(ctx)
 		appName = flag.GetString(ctx, "name")

internal/command/mpg/destroy.go

@@ -36,6 +36,11 @@ This action is not reversible.`
 }
 
 func runDestroy(ctx context.Context) error {
+	// Check token compatibility early
+	if err := validateMPGTokenCompatibility(ctx); err != nil {
+		return err
+	}
+
 	var (
 		clusterId  = flag.FirstArg(ctx)
 		uiexClient = uiexutil.ClientFromContext(ctx)

internal/command/mpg/list.go

@@ -40,6 +40,11 @@ If no organization is specified, the user's personal organization is used.`
 }
 
 func runList(ctx context.Context) error {
+	// Check token compatibility early
+	if err := validateMPGTokenCompatibility(ctx); err != nil {
+		return err
+	}
+
 	cfg := config.FromContext(ctx)
 	out := iostreams.FromContext(ctx).Out
 

internal/command/mpg/mpg.go

@@ -8,6 +8,7 @@ import (
 	fly "github.com/superfly/fly-go"
 	"github.com/superfly/flyctl/gql"
 	"github.com/superfly/flyctl/internal/command"
+	"github.com/superfly/flyctl/internal/config"
 	"github.com/superfly/flyctl/internal/flag"
 	"github.com/superfly/flyctl/internal/flyutil"
 	"github.com/superfly/flyctl/internal/prompt"
@@ -277,3 +278,30 @@ func ResolveOrganizationSlug(ctx context.Context, inputSlug string) (string, err
 	// Return the canonical slug from the API response
 	return resp.Organization.RawSlug, nil
 }
+
+// detectTokenHasMacaroon determines if the current context has macaroon-style tokens.
+// MPG commands require macaroon tokens to function properly.
+func detectTokenHasMacaroon(ctx context.Context) bool {
+	tokens := config.Tokens(ctx)
+	if tokens == nil {
+		return false
+	}
+
+	// Check for macaroon tokens (newer style)
+	return len(tokens.GetMacaroonTokens()) > 0
+}
+
+// validateMPGTokenCompatibility checks if the current authentication tokens are compatible
+// with MPG commands. MPG requires macaroon-style tokens and cannot work with older bearer tokens.
+// Returns an error if bearer tokens are detected, suggesting the user upgrade their tokens.
+func validateMPGTokenCompatibility(ctx context.Context) error {
+	if !detectTokenHasMacaroon(ctx) {
+		return fmt.Errorf(`MPG commands require updated tokens but found older-style tokens.
+
+Please upgrade your authentication by running:
+  flyctl auth logout
+  flyctl auth login
+`)
+	}
+	return nil
+}

internal/command/mpg/mpg_test.go

@@ -10,7 +10,9 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	fly "github.com/superfly/fly-go"
+	"github.com/superfly/fly-go/tokens"
 	"github.com/superfly/flyctl/internal/command_context"
+	"github.com/superfly/flyctl/internal/config"
 	"github.com/superfly/flyctl/internal/flag/flagctx"
 	"github.com/superfly/flyctl/internal/uiex"
 	"github.com/superfly/flyctl/internal/uiexutil"
@@ -783,3 +785,148 @@ func TestCreateCommand_RegionValidation(t *testing.T) {
 	require.NoError(t, err)
 	assert.False(t, valid, "Should not find invalid region")
 }
+
+// Test actual MPG token validation functions
+func TestMPGTokenValidation(t *testing.T) {
+	t.Run("detectTokenHasMacaroon with actual contexts", func(t *testing.T) {
+		// Test case 1: Context with no config (should handle gracefully)
+		emptyCtx := context.Background()
+		// This should panic or return false - let's catch the panic
+		func() {
+			defer func() {
+				if r := recover(); r != nil {
+					// Expected panic due to no config in context
+					t.Logf("Expected panic caught: %v", r)
+				}
+			}()
+			result := detectTokenHasMacaroon(emptyCtx)
+			// If we get here without panicking, it should return false
+			assert.False(t, result, "Should return false when config is nil")
+		}()
+
+		// Test case 2: Context with nil tokens
+		configWithNilTokens := &config.Config{
+			Tokens: nil,
+		}
+		ctxWithNilTokens := config.NewContext(context.Background(), configWithNilTokens)
+		result := detectTokenHasMacaroon(ctxWithNilTokens)
+		assert.False(t, result, "Should return false when tokens are nil")
+
+		// Test case 3: Context with empty tokens (no macaroons)
+		emptyTokens := tokens.Parse("") // Parse empty string creates empty tokens
+		configWithEmptyTokens := &config.Config{
+			Tokens: emptyTokens,
+		}
+		ctxWithEmptyTokens := config.NewContext(context.Background(), configWithEmptyTokens)
+		result = detectTokenHasMacaroon(ctxWithEmptyTokens)
+		assert.False(t, result, "Should return false when no macaroon tokens exist")
+
+		// Test case 4: Context with bearer tokens only (no macaroons)
+		bearerTokens := tokens.Parse("some_bearer_token_here") // This won't be recognized as macaroon
+		configWithBearerTokens := &config.Config{
+			Tokens: bearerTokens,
+		}
+		ctxWithBearerTokens := config.NewContext(context.Background(), configWithBearerTokens)
+		result = detectTokenHasMacaroon(ctxWithBearerTokens)
+		assert.False(t, result, "Should return false when only bearer tokens exist")
+
+		// Test case 5: Context with macaroon tokens
+		macaroonTokens := tokens.Parse("fm1r_test_macaroon_token,fm2_another_macaroon") // fm1r and fm2 prefixes are macaroon tokens
+		configWithMacaroonTokens := &config.Config{
+			Tokens: macaroonTokens,
+		}
+		ctxWithMacaroonTokens := config.NewContext(context.Background(), configWithMacaroonTokens)
+		result = detectTokenHasMacaroon(ctxWithMacaroonTokens)
+		assert.True(t, result, "Should return true when macaroon tokens exist")
+
+		// Test case 6: Context with mixed tokens (including macaroons)
+		mixedTokens := tokens.Parse("bearer_token,fm1a_macaroon_token,oauth_token")
+		configWithMixedTokens := &config.Config{
+			Tokens: mixedTokens,
+		}
+		ctxWithMixedTokens := config.NewContext(context.Background(), configWithMixedTokens)
+		result = detectTokenHasMacaroon(ctxWithMixedTokens)
+		assert.True(t, result, "Should return true when macaroon tokens exist among mixed tokens")
+	})
+
+	t.Run("validateMPGTokenCompatibility with actual contexts", func(t *testing.T) {
+		// Test case 1: Context with nil tokens - should fail
+		configWithNilTokens := &config.Config{
+			Tokens: nil,
+		}
+		ctxWithNilTokens := config.NewContext(context.Background(), configWithNilTokens)
+		err := validateMPGTokenCompatibility(ctxWithNilTokens)
+		assert.Error(t, err, "Should return error when no macaroon tokens")
+		assert.Contains(t, err.Error(), "MPG commands require updated tokens")
+		assert.Contains(t, err.Error(), "flyctl auth logout")
+		assert.Contains(t, err.Error(), "flyctl auth login")
+
+		// Test case 2: Context with empty tokens - should fail
+		emptyTokens := tokens.Parse("")
+		configWithEmptyTokens := &config.Config{
+			Tokens: emptyTokens,
+		}
+		ctxWithEmptyTokens := config.NewContext(context.Background(), configWithEmptyTokens)
+		err = validateMPGTokenCompatibility(ctxWithEmptyTokens)
+		assert.Error(t, err, "Should return error when no macaroon tokens")
+		assert.Contains(t, err.Error(), "MPG commands require updated tokens")
+
+		// Test case 3: Context with bearer tokens only - should fail
+		bearerTokens := tokens.Parse("some_bearer_token")
+		configWithBearerTokens := &config.Config{
+			Tokens: bearerTokens,
+		}
+		ctxWithBearerTokens := config.NewContext(context.Background(), configWithBearerTokens)
+		err = validateMPGTokenCompatibility(ctxWithBearerTokens)
+		assert.Error(t, err, "Should return error when no macaroon tokens")
+		assert.Contains(t, err.Error(), "MPG commands require updated tokens")
+
+		// Test case 4: Context with macaroon tokens - should pass
+		macaroonTokens := tokens.Parse("fm1r_test_macaroon_token")
+		configWithMacaroonTokens := &config.Config{
+			Tokens: macaroonTokens,
+		}
+		ctxWithMacaroonTokens := config.NewContext(context.Background(), configWithMacaroonTokens)
+		err = validateMPGTokenCompatibility(ctxWithMacaroonTokens)
+		assert.NoError(t, err, "Should not return error when macaroon tokens exist")
+
+		// Test case 5: Context with mixed tokens including macaroons - should pass
+		mixedTokens := tokens.Parse("bearer_token,fm1a_macaroon_token,oauth_token")
+		configWithMixedTokens := &config.Config{
+			Tokens: mixedTokens,
+		}
+		ctxWithMixedTokens := config.NewContext(context.Background(), configWithMixedTokens)
+		err = validateMPGTokenCompatibility(ctxWithMixedTokens)
+		assert.NoError(t, err, "Should not return error when macaroon tokens exist among mixed tokens")
+	})
+
+	t.Run("MPG commands reject non-macaroon tokens", func(t *testing.T) {
+		// This test verifies that actual MPG command functions call the validation
+		// and properly reject contexts without macaroon tokens
+
+		// Create a context with bearer tokens only (no macaroons)
+		bearerTokens := tokens.Parse("some_bearer_token")
+		configWithBearerTokens := &config.Config{
+			Tokens: bearerTokens,
+		}
+		ctxWithBearerTokens := config.NewContext(context.Background(), configWithBearerTokens)
+
+		// Test that the actual run functions would reject this context
+		// We can't easily test the full run functions due to their dependencies,
+		// but we can verify the validation function they call would fail
+		err := validateMPGTokenCompatibility(ctxWithBearerTokens)
+		assert.Error(t, err, "MPG commands should reject contexts with only bearer tokens")
+		assert.Contains(t, err.Error(), "MPG commands require updated tokens")
+
+		// Create a context with macaroon tokens
+		macaroonTokens := tokens.Parse("fm1r_macaroon_token")
+		configWithMacaroonTokens := &config.Config{
+			Tokens: macaroonTokens,
+		}
+		ctxWithMacaroonTokens := config.NewContext(context.Background(), configWithMacaroonTokens)
+
+		// Test that the validation would pass for macaroon tokens
+		err = validateMPGTokenCompatibility(ctxWithMacaroonTokens)
+		assert.NoError(t, err, "MPG commands should accept contexts with macaroon tokens")
+	})
+}

internal/command/mpg/proxy.go

@@ -43,6 +43,11 @@ func newProxy() (cmd *cobra.Command) {
 }
 
 func runProxy(ctx context.Context) (err error) {
+	// Check token compatibility early
+	if err := validateMPGTokenCompatibility(ctx); err != nil {
+		return err
+	}
+
 	localProxyPort := "16380"
 	_, params, _, err := getMpgProxyParams(ctx, localProxyPort)
 	if err != nil {

internal/command/mpg/status.go

@@ -35,6 +35,11 @@ func newStatus() *cobra.Command {
 }
 
 func runStatus(ctx context.Context) error {
+	// Check token compatibility early
+	if err := validateMPGTokenCompatibility(ctx); err != nil {
+		return err
+	}
+
 	cfg := config.FromContext(ctx)
 	out := iostreams.FromContext(ctx).Out
 	uiexClient := uiexutil.ClientFromContext(ctx)