Read the --org flag when running the flyctl tokens list command (#3799)

* initial

* Update tokens list:
1. Read --org flag
2. Get the scope of tokens to list via detecting flags --a  and --o passed:
--- org scope: ONLY when the --org is passed
--- app scope: 1. By default( no flags passed ) 2. The --a flag passed, regardless if --org passed or not
3. Throw an error if both the --app and --org flags are passed, and the resulting app name does not belong to the selected org,

TODO: ADD TESTCASE PLEASE

* Include scope flag
Determine scope through flags provided
Include --config into consideration for setting scope as "app" scope
Throw an error if --scope provided string does not match "app" or "org"
Print logs indicating scope(app or org), and name of app or org to get tokens for

* trim trailing spaces

* see if this fixes test error about triming triling whitespaces

* Check if removing empty line removes fix trailing whitespace error

* remove empty lines as indicated from precommit test result error

* remove final trailing space please

* remove the empty list_test.go

* use gofmt on list.go

* Add testcases for testing correct scope identification:app or org

* Add final test for making sure app name is properly identified from --app flag, regardless if in a directory with a flytoml containing a diff app name

* Replace hardcoded organization name in tokenstest with org from env

* fix trailing whitespaces, gofmt the files please

* add preflight top two lines to fix make test

---------

Co-authored-by: Kathryn Anne S Tan <[email protected]>

Author: KTanAug21

internal/command/tokens/list.go

@@ -33,8 +33,9 @@ func newList() *cobra.Command {
 			Name:        "scope",
 			Shorthand:   "s",
 			Description: "either 'app' or 'org'",
-			Default:     "app",
+			Default:     "",
 		},
+		flag.Org(),
 	)
 
 	return cmd
@@ -45,19 +46,48 @@ func runList(ctx context.Context) (err error) {
 	out := iostreams.FromContext(ctx).Out
 	var rows [][]string
 
+	// Determine scope based on flags passed
 	scope := flag.GetString(ctx, "scope")
+	appFlag := flag.GetString(ctx, "app")
+	configFlag := flag.GetString(ctx, "config")
+	orgFlag := flag.GetString(ctx, "org")
+	scope, err = determineScope(scope, appFlag, orgFlag, configFlag)
+	if err != nil {
+		return fmt.Errorf("failed retrieving scope: %w", err)
+	}
+
+	// Apply scope to filter list of tokens to display
 	switch scope {
 	case "app":
 		appName := appconfig.NameFromContext(ctx)
 		if appName == "" {
 			return command.ErrRequireAppName
 		}
+		// --org passed must match the selected app's org
+		if orgFlag != "" {
+			// Get app details, so we can identify its organization slug
+			app, err := apiClient.GetAppCompact(ctx, appName)
+			if err != nil {
+				return fmt.Errorf("failed retrieving app %s: %w", appName, err)
+			}
 
+			// Get organization details, so we can get its slug
+			org, err := orgs.OrgFromEnvVarOrFirstArgOrSelect(ctx)
+			if err != nil {
+				return fmt.Errorf("failed retrieving org %w", err)
+			}
+
+			// Throw an error if app's org slug does not match --org slug
+			if app.Organization.Slug != org.Slug {
+				return fmt.Errorf("failed to retrieve tokens, selected application \"%s\" does not belong to selected organization \"%s\"", appName, org.Slug)
+			}
+		}
 		tokens, err := apiClient.GetAppLimitedAccessTokens(ctx, appName)
 		if err != nil {
 			return fmt.Errorf("failed retrieving tokens for app %s: %w", appName, err)
 		}
 
+		fmt.Fprintln(out, "Tokens for app \""+appName+"\":")
 		for _, token := range tokens {
 			rows = append(rows, []string{token.Id, token.Name, token.User.Email, token.ExpiresAt.String()})
 		}
@@ -68,10 +98,28 @@ func runList(ctx context.Context) (err error) {
 			return fmt.Errorf("failed retrieving org %w", err)
 		}
 
+		fmt.Fprintln(out, "Tokens for organization \""+org.Slug+"\":")
 		for _, token := range org.LimitedAccessTokens.Nodes {
 			rows = append(rows, []string{token.Id, token.Name, token.User.Email, token.ExpiresAt.String()})
 		}
 	}
+
 	_ = render.Table(out, "", rows, "ID", "Name", "Created By", "Expires At")
 	return nil
 }
+
+func determineScope(scopeStr string, appFlagStr string, orgFlagStr string, configFlagStr string) (scope string, err error) {
+	// app scope is given highest priority, as it is more granular than org, identified by --app|--config|--scope=app
+	// org scope is only used when specified by --org|--scope=org withought any app scope indicator
+	// If scope is neither app nor org, but provided by the user, throw an error; there are only two scopes: app|org
+	// the default scope, when all else fails, is app scope
+	if appFlagStr != "" || configFlagStr != "" || scopeStr == "app" {
+		return "app", nil
+	} else if orgFlagStr != "" || scopeStr == "org" {
+		return "org", nil
+	} else if scopeStr != "" && scopeStr != "app" && scopeStr != "org" {
+		return "", fmt.Errorf("Please provide a valid scope: \"app\" or \"org\".")
+	} else {
+		return "app", nil
+	}
+}

test/preflight/fly_tokens_test.go

@@ -0,0 +1,113 @@
+//go:build integration
+// +build integration
+
+package preflight
+
+import (
+	"github.com/stretchr/testify/require"
+	"github.com/superfly/flyctl/test/preflight/testlib"
+	"os"
+	"testing"
+)
+
+// TODO: list of things to test
+// - App scope is properly determined
+// - Org scope is properly determined
+// - App identified by flag is prioritized over the fly.toml file, because it's more visible
+
+func TestTokensListDeterminesScopeApp(t *testing.T) {
+	// Get library from preflight test lib using env variables form  .direnv/preflight
+	f := testlib.NewTestEnvFromEnv(t)
+	// No need to run this on alternate sizes as it only tests the generated config.
+	if f.VMSize != "" {
+		t.Skip()
+	}
+
+	// Create fly.toml in current directory for the purpose of testing --config purposes
+	appName := f.CreateRandomAppName()
+	f.Fly(
+		"launch --now --org %s --name %s --region %s --image nginx --internal-port 80 --ha=false",
+		f.OrgSlug(), appName, f.PrimaryRegion(),
+	)
+
+	// List of commands to verify scope app is selected for
+	commandList := [9](string){
+		// default, no flags
+		"tokens list",
+		// --app flag only
+		"tokens list -a " + appName,
+		// --config flag only
+		"tokens list -c fly.toml",
+		// --app flag, with --org flag
+		"tokens list -a " + appName + " -o " + f.OrgSlug(),
+		// --config flag, with --org flag
+		"tokens list -c fly.toml -o " + f.OrgSlug(),
+		// --scope is app
+		"tokens list -s app -a " + appName,
+		// --scope is app, with --org flag
+		"tokens list -s app -a " + appName + " -o " + f.OrgSlug(),
+		// --scope is org, but --config flag added
+		"tokens list -s app -c fly.toml -o " + f.OrgSlug(),
+		// --scope is org, but --app flag added
+		"tokens list -s org " + f.OrgSlug() + " -a " + appName,
+	}
+
+	for _, cmd_ := range commandList {
+		console_out := f.Fly(cmd_).StdOutString()
+		require.Contains(f, console_out, `Tokens for app`)
+	}
+}
+
+func TestTokensListDeterminesScopeOrg(t *testing.T) {
+	// Get library from preflight test lib using env variables form  .direnv/preflight
+	f := testlib.NewTestEnvFromEnv(t)
+	// No need to run this on alternate sizes as it only tests the generated config.
+	if f.VMSize != "" {
+		t.Skip()
+	}
+
+	// List of commands to verify scope org is selected for
+	commandList := [2](string){
+		// --org flag alone
+		"tokens list --org " + f.OrgSlug(),
+		// --scope org
+		"tokens list --scope org " + f.OrgSlug(),
+	}
+
+	for _, cmd_ := range commandList {
+		console_out := f.Fly(cmd_).StdOutString()
+		require.Contains(f, console_out, `Tokens for organization`)
+	}
+}
+
+func TestTokensListPrioritizesAppFromFlagOverToml(t *testing.T) {
+	// Get library from preflight test lib using env variables form  .direnv/preflight
+	f := testlib.NewTestEnvFromEnv(t)
+	// No need to run this on alternate sizes as it only tests the generated config.
+	if f.VMSize != "" {
+		t.Skip()
+	}
+
+	// Create fly.toml in current directory
+	// Get this appName for verifying that the flag is prioritized over toml appname content
+	appNameA := f.CreateRandomAppName()
+	f.Fly(
+		"launch --now --org %s --name %s --region %s --image nginx --internal-port 80 --ha=false",
+		f.OrgSlug(), appNameA, f.PrimaryRegion(),
+	)
+
+	// Delete this first app's toml file, so we can create a new one
+	os.Remove(f.WorkDir() + "/fly.toml")
+	appNameB := f.CreateRandomAppName()
+	f.Fly(
+		"launch --now --org %s --name %s --region %s --image nginx --internal-port 80 --ha=false",
+		f.OrgSlug(), appNameB, f.PrimaryRegion(),
+	)
+	// By default use the app name found in the current dir should be used ( appNameB )
+	console_out := f.Fly("tokens list").StdOutString()
+	require.Contains(f, console_out, `Tokens for app "`+appNameB+`"`)
+
+	// But, the app name passed in --app should be prioritized over the toml appname ( appNameA )
+	console_out = f.Fly("tokens list --app " + appNameA).StdOutString()
+	require.Contains(f, console_out, `Tokens for app "`+appNameA+`"`)
+}