Don't panic if UrlExecCommand is not set during config

- Dont generate warning if icon/color aren't set

Fixes #385

CHANGELOG.md

@@ -1,6 +1,11 @@
 # AWS SSO CLI Changelog
 
-## Unreleased
+## [v1.9.1] - 2022-05-09
+
+## Bugs
+
+ * Fix `config` command when user has no `UrlExecCommand` defined #385
+ * `console` no longer warns when a role is missing the Color or Icon tag 
 
 ## [v1.9.0] - 2022-05-08
 

Makefile

@@ -1,4 +1,4 @@
-PROJECT_VERSION := 1.9.0
+PROJECT_VERSION := 1.9.1
 DOCKER_REPO     := synfinatic
 PROJECT_NAME    := aws-sso
 

cmd/setup_cmd.go

@@ -106,7 +106,7 @@ func setupWizard(ctx *RunContext, reconfig, addSSO bool) error {
 	var hLimit, hMinutes, cacheRefresh int64
 	var consoleDuration int32
 	var autoConfigCheck bool
-	urlExecCommand := []interface{}{}
+	var urlExecCommand []string
 
 	// Don't run setup twice
 	if ranSetup {
@@ -126,9 +126,9 @@ func setupWizard(ctx *RunContext, reconfig, addSSO bool) error {
 		defaultLevel = ctx.Settings.LogLevel
 		defaultRegion = ctx.Settings.DefaultRegion
 		urlAction = ctx.Settings.UrlAction
-		urlExecCommand = ctx.Settings.UrlExecCommand.([]interface{})
+		urlExecCommand = ctx.Settings.UrlExecCommand
 		if ctx.Settings.FirefoxOpenUrlInContainer {
-			firefoxBrowserPath = urlExecCommand[0].(string)
+			firefoxBrowserPath = urlExecCommand[0]
 		}
 		autoConfigCheck = ctx.Settings.AutoConfigCheck
 		cacheRefresh = ctx.Settings.CacheRefresh

cmd/setup_prompt.go

@@ -338,8 +338,8 @@ func promptUrlAction(defaultValue string, useFirefox bool) string {
 	return items[i].Value
 }
 
-func promptUrlExecCommand(defaultValue []interface{}) []interface{} {
-	var val []interface{}
+func promptUrlExecCommand(defaultValue []string) []string {
+	var val []string
 	var err error
 	var line string
 	argNum := 1
@@ -348,7 +348,7 @@ func promptUrlExecCommand(defaultValue []interface{}) []interface{} {
 
 	fmt.Printf("Please enter one per line, the command and list of arguments for UrlExecCommand:\n")
 
-	command := defaultValue[0].(string)
+	command := defaultValue[0]
 	label := "Binary to execute to open URLs (UrlExecCommand)"
 	prompt := promptui.Prompt{
 		Label:     label,
@@ -363,17 +363,17 @@ func promptUrlExecCommand(defaultValue []interface{}) []interface{} {
 		log.Fatal(err)
 	}
 
-	val = append(val, interface{}(line))
+	val = append(val, line)
 
 	// zero out the defaults if we change the command to execute
 	if line != defaultValue[0] {
-		defaultValue = []interface{}{}
+		defaultValue = []string{}
 	}
 
 	for line != "" {
 		arg := ""
 		if argNum < len(defaultValue) {
-			arg = defaultValue[argNum].(string)
+			arg = defaultValue[argNum]
 		}
 		label := fmt.Sprintf("Enter argument #%d or empty string to stop", argNum)
 		prompt = promptui.Prompt{

internal/utils/url.go

@@ -64,12 +64,16 @@ const FIREFOX_CONTAINER_FORMAT = "ext+container:name=%s&url=%s&color=%s&icon=%s"
 // FirefoxContainerUrl generates a URL for Firefox Containers
 func FirefoxContainerUrl(target, name, color, icon string) string {
 	if !StrListContains(color, FIREFOX_PLUGIN_COLORS) {
-		log.Warnf("Invalid Firefox Container color: %s", color)
+		if color != "" {
+			log.Warnf("Invalid Firefox Container color: %s", color)
+		}
 		color = FIREFOX_PLUGIN_COLORS[0]
 	}
 
 	if !StrListContains(icon, FIREFOX_PLUGIN_ICONS) {
-		log.Warnf("Invalid Firefox Container icon: %s", icon)
+		if icon != "" {
+			log.Warnf("Invalid Firefox Container icon: %s", icon)
+		}
 		icon = FIREFOX_PLUGIN_ICONS[0]
 	}
 
@@ -78,44 +82,53 @@ func FirefoxContainerUrl(target, name, color, icon string) string {
 
 var printWriter io.Writer = os.Stderr
 
-func execWithUrl(command interface{}, url string) error {
-	var cmd *exec.Cmd
-	var cmdStr string
-
-	switch command.(type) {
-	case []interface{}:
-		program := ""
-		x := []string{}
-		for _, iface := range command.([]interface{}) {
-			v := iface.(string)
-			if strings.Contains(v, "%s") {
-				if program == "" {
-					program = fmt.Sprintf(v, url)
-				} else {
-					x = append(x, fmt.Sprintf(v, url))
-				}
-			} else {
-				if program == "" {
-					program = v
-				} else {
-					x = append(x, v)
-				}
-			}
+// used by execWithUrl to build our actual command
+func commandBuilder(command []string, url string) (string, []string, error) {
+	var program string
+	cmdList := []string{}
+	replaced := false
+
+	if len(command) < 2 {
+		return program, cmdList, fmt.Errorf("Invalid UrlExecCommand has fewer than 2 arguments")
+	}
+
+	for i, v := range command {
+		if i == 0 {
+			program = v
+			continue
+		} else if strings.Contains(v, "%s") {
+			replaced = true
+			v = fmt.Sprintf(v, url)
 		}
-		cmdStr = fmt.Sprintf("%s %s", program, strings.Join(x, " "))
-		log.Debugf("exec command as array: %s", cmdStr)
-		cmd = exec.Command(program, x...)
-	default:
-		return fmt.Errorf("Invalid UrlExecCommand type: %v", command)
+		cmdList = append(cmdList, v)
+	}
+
+	if !replaced {
+		return program, cmdList, fmt.Errorf("Invalid UrlExecCommand has no `%%s` for URL")
 	}
 
+	return program, cmdList, nil
+}
+
+func execWithUrl(command []string, url string) error {
+	var cmd *exec.Cmd
+
+	program, cmdList, err := commandBuilder(command, url)
+	if err != nil {
+		return err
+	}
+
+	cmdStr := fmt.Sprintf("%s %s", program, strings.Join(cmdList, " "))
+	log.Debugf("exec command as array: %s", cmdStr)
+	cmd = exec.Command(program, cmdList...)
+
 	//	var stderr bytes.Buffer
 	//	cmd.Stderr = &stderr
-	err := cmd.Start()
+	err = cmd.Start() // Don't use Run() because sometimes firefox does bad things?
 	if err != nil {
 		err = fmt.Errorf("Unable to exec `%s`: %s", cmdStr, err)
 	}
-	log.Debugf("Opened our URL with %s", command.([]interface{})[0])
+	log.Debugf("Opened our URL with %s", command[0])
 	return err
 }
 
@@ -140,14 +153,14 @@ const (
 
 type HandleUrl struct {
 	Action  UrlAction
-	ExecCmd interface{}
+	ExecCmd []string
 	Browser string
 	Url     string
 	PreMsg  string
 	PostMsg string
 }
 
-func NewHandleUrl(action, browser string, command interface{}) *HandleUrl {
+func NewHandleUrl(action, browser string, command []string) *HandleUrl {
 	var a UrlAction
 
 	switch action {

internal/utils/url_test.go

@@ -57,18 +57,19 @@ func testUrlOpenerWithError(url, browser string) error {
 func (suite *UtilsTestSuite) TestHandleUrl() {
 	t := suite.T()
 
-	assert.Panics(t, func() { NewHandleUrl("foo", "browser", "") })
+	noCommand := []string{}
+	assert.Panics(t, func() { NewHandleUrl("foo", "browser", noCommand) })
 
 	// override the print method
 	printWriter = new(bytes.Buffer)
-	h := NewHandleUrl("print", "browser", "")
+	h := NewHandleUrl("print", "browser", noCommand)
 	assert.NotNil(t, h)
 	assert.NoError(t, h.Open("bar", "pre", "post"))
 	assert.Equal(t, "prebarpost", printWriter.(*bytes.Buffer).String())
 
 	// new print method for printurl
 	printWriter = new(bytes.Buffer)
-	h = NewHandleUrl("printurl", "browser", "")
+	h = NewHandleUrl("printurl", "browser", noCommand)
 	assert.NotNil(t, h)
 	assert.NoError(t, h.Open("bar", "pre", "post"))
 	assert.Equal(t, "bar\n", printWriter.(*bytes.Buffer).String())
@@ -78,18 +79,18 @@ func (suite *UtilsTestSuite) TestHandleUrl() {
 	urlOpenerWith = testUrlOpenerWith
 	clipboardWriter = testClipboardWriter
 
-	h = NewHandleUrl("clip", "browser", "")
+	h = NewHandleUrl("clip", "browser", noCommand)
 	assert.NotNil(t, h)
 	assert.NoError(t, h.Open("url", "pre", "post"))
 	assert.Equal(t, "url", checkValue)
 
-	h = NewHandleUrl("open", "other-browser", "")
+	h = NewHandleUrl("open", "other-browser", noCommand)
 	assert.NotNil(t, h)
 	assert.NoError(t, h.Open("other-url", "pre", "post"))
 	assert.Equal(t, "other-browser", checkBrowser)
 	assert.Equal(t, "other-url", checkValue)
 
-	h = NewHandleUrl("open", "", "")
+	h = NewHandleUrl("open", "", noCommand)
 	assert.NotNil(t, h)
 	assert.NoError(t, h.Open("some-url", "pre", "post"))
 	assert.Equal(t, "default browser", checkBrowser)
@@ -99,37 +100,41 @@ func (suite *UtilsTestSuite) TestHandleUrl() {
 	assert.Error(t, h.Open("url", "pre", "post"))
 
 	urlOpenerWith = testUrlOpenerWithError
-	h = NewHandleUrl("open", "foo", "")
+	h = NewHandleUrl("open", "foo", noCommand)
 	assert.NotNil(t, h)
 	assert.Error(t, h.Open("url", "pre", "post"))
 
 	clipboardWriter = testUrlOpenerError
-	h = NewHandleUrl("clip", "", "")
+	h = NewHandleUrl("clip", "", noCommand)
 	assert.NotNil(t, h)
 	assert.Error(t, h.Open("url", "pre", "post"))
 
 	// Exec tests
-	h = NewHandleUrl("exec", "", []interface{}{"echo", "foo", "%s"})
+	h = NewHandleUrl("exec", "", []string{"echo", "foo", "%s"})
 	assert.NotNil(t, h)
 	assert.NoError(t, h.Open("url", "pre", "post"))
 
-	h = NewHandleUrl("exec", "", []interface{}{"%s"})
+	h = NewHandleUrl("exec", "", []string{})
 	assert.NotNil(t, h)
-	assert.NoError(t, h.Open("sh", "pre", "post"))
+	assert.Error(t, h.Open("sh", "pre", "post"))
 
-	h = NewHandleUrl("exec", "", []interface{}{"/dev/null", "%s"})
+	h = NewHandleUrl("exec", "", []string{"%s"})
+	assert.NotNil(t, h)
+	assert.Error(t, h.Open("sh", "pre", "post"))
+
+	h = NewHandleUrl("exec", "", []string{"/dev/null", "%s"})
 	assert.NotNil(t, h)
 	assert.Error(t, h.Open("url", "pre", "post"))
 
-	h = NewHandleUrl("exec", "", []interface{}{"/dev/null"})
+	h = NewHandleUrl("exec", "", []string{"/dev/null"})
 	assert.NotNil(t, h)
 	assert.Error(t, h.Open("url", "pre", "post"))
 
-	h = NewHandleUrl("exec", "", []interface{}{"%s"})
+	h = NewHandleUrl("exec", "", []string{"%s"})
 	assert.NotNil(t, h)
 	assert.Error(t, h.Open("url", "pre", "post"))
 
-	h = NewHandleUrl("exec", "", "")
+	h = NewHandleUrl("exec", "", noCommand)
 	assert.NotNil(t, h)
 	assert.Error(t, h.Open("url", "pre", "post"))
 }
@@ -141,3 +146,27 @@ func TestFirefoxContainersUrl(t *testing.T) {
 	assert.Equal(t, "ext+container:name=Test&url=https%3A%2F%2Fsynfin.net&color=blue&icon=fingerprint",
 		FirefoxContainerUrl("https://synfin.net", "Test", "Bad", "Value"))
 }
+
+func TestCommandBuilder(t *testing.T) {
+	_, _, err := commandBuilder([]string{}, "url")
+	assert.Error(t, err)
+
+	_, _, err = commandBuilder([]string{"foo"}, "url")
+	assert.Error(t, err)
+
+	_, _, err = commandBuilder([]string{"%s"}, "url")
+	assert.Error(t, err)
+
+	_, _, err = commandBuilder([]string{"foo", "bar"}, "url")
+	assert.Error(t, err)
+
+	cmd, l, err := commandBuilder([]string{"foo", "%s"}, "url")
+	assert.NoError(t, err)
+	assert.Equal(t, "foo", cmd)
+	assert.Equal(t, []string{"url"}, l)
+
+	cmd, l, err = commandBuilder([]string{"foo", "bar", "%s"}, "url")
+	assert.NoError(t, err)
+	assert.Equal(t, "foo", cmd)
+	assert.Equal(t, []string{"bar", "url"}, l)
+}

sso/awssso.go

@@ -67,7 +67,7 @@ type AWSSSO struct {
 	SSOConfig      *SSOConfig                  `json:"SSOConfig"`
 	urlAction      string                      // cache for future calls
 	browser        string                      // cache for future calls
-	urlExecCommand interface{}                 // cache for future calls
+	urlExecCommand []string                    // cache for future calls
 }
 
 func NewAWSSSO(s *SSOConfig, store *storage.SecureStorage) *AWSSSO {

sso/awssso_auth_test.go

@@ -372,7 +372,7 @@ func TestReauthenticate(t *testing.T) {
 		store:          jstore,
 		urlAction:      "invalid",
 		browser:        "no-such-browser",
-		urlExecCommand: []interface{}{"/dev/null"},
+		urlExecCommand: []string{"/dev/null", "%s"},
 		SSOConfig: &SSOConfig{
 			settings: &Settings{},
 		},

sso/settings.go

@@ -59,7 +59,7 @@ type Settings struct {
 	Browser                   string                 `koanf:"Browser" yaml:"Browser,omitempty"`
 	ConfigUrlAction           string                 `koanf:"ConfigUrlAction" yaml:"ConfigUrlAction,omitempty"` // deprecated
 	ConfigProfilesUrlAction   string                 `koanf:"ConfigProfilesUrlAction" yaml:"ConfigProfilesUrlAction,omitempty"`
-	UrlExecCommand            interface{}            `koanf:"UrlExecCommand" yaml:"UrlExecCommand,omitempty"` // string or list
+	UrlExecCommand            []string               `koanf:"UrlExecCommand" yaml:"UrlExecCommand,omitempty"` // string or list
 	LogLevel                  string                 `koanf:"LogLevel" yaml:"LogLevel,omitempty"`
 	LogLines                  bool                   `koanf:"LogLines" yaml:"LogLines,omitempty"`
 	HistoryLimit              int64                  `koanf:"HistoryLimit" yaml:"HistoryLimit,omitempty"`