Merge pull request #947 from synfinatic/configprofilesaction

ConfigProfilesUrlAction defaults to UrlAction

CHANGELOG.md

@@ -14,6 +14,7 @@
 ### Changes
 
  * Bump cache file version to 4.
+ * `ConfigProfilesUrlAction` now defaults to value of `UrlAction` instead of `url` #946
 
 ## [v1.16.1] - 2024-06-13
 

Makefile

@@ -193,7 +193,7 @@ lint:  ## Run golangci-lint
 
 test-homebrew: $(DIST_DIR)$(PROJECT_NAME)  ## Run the homebrew tests
 	@$(DIST_DIR)$(PROJECT_NAME) --config /dev/null version 2>/dev/null | grep -q "AWS SSO CLI Version $(PROJECT_VERSION)"
-	@$(DIST_DIR)$(PROJECT_NAME) --config /dev/null 2>&1 | grep -q "No AWS SSO providers have been configured."
+	@$(DIST_DIR)$(PROJECT_NAME) --config /dev/null 2>&1 | grep -q "no AWS SSO providers have been configured."
 
 # Build targets for our supported plaforms
 windows: $(WINDOWS_BIN)  ## Build 64bit x86 Windows binary

cmd/aws-sso/main.go

@@ -91,7 +91,6 @@ var DEFAULT_CONFIG map[string]interface{} = map[string]interface{}{
 	"ListFields":                                DEFAULT_LIST_FIELDS,
 	"ConsoleDuration":                           60,
 	"UrlAction":                                 "open",
-	"ConfigProfilesUrlAction":                   "open",
 	"LogLevel":                                  "warn",
 	"DefaultSSO":                                "Default",
 	"FirefoxOpenUrlInContainer":                 false,

docs/config.md

@@ -353,10 +353,11 @@ https://docs.aws.amazon.com/singlesignon/latest/userguide/howtosessionduration.h
 
 #### ConfigProfilesUrlAction
 
-This works just like `UrlAction` above, but is used for setting the default
-`--url-action` in your `~/.aws/config` when generating named AWS profiles for
-use with `$AWS_PROFILE` and the default value for the [config-profiles](
-commands.md#config-profiles) command.
+The `aws-sso config-profiles` command by default will use the same action
+to open URL's as defined in [UrlAction](#browser--urlaction--urlexeccommand),
+but you can override it via the `ConfigProfilesUrlAction`.
+
+If `UrlAction` is `print` or `printurl`, then this settings will default to `open`.
 
 Due to limitations with the AWS SDK, only the following options are valid:
 
@@ -366,8 +367,7 @@ Due to limitations with the AWS SDK, only the following options are valid:
  * `granted-containers`
  * `open-url-in-container`
 
-**Note:** This option is required if you also want to use 
-[AutoConfigCheck](#autoconfigcheck).
+**Note:** This option is required if you also want to use [AutoConfigCheck](#autoconfigcheck).
 
 **Note:** This config option was previously known as `ConfigUrlAction` which
 has been deprecated.

internal/url/url.go

@@ -93,6 +93,16 @@ func (u Action) IsContainer() bool {
 	return u == GrantedContainer || u == OpenUrlContainer
 }
 
+// GetConfigProfileAction returns the ConfigProfilesAction for the given Action
+func (u Action) GetConfigProfilesAction() ConfigProfilesAction {
+	switch u {
+	case "print", "printurl", "":
+		return ConfigProfilesOpen
+	default:
+		return ConfigProfilesAction(u)
+	}
+}
+
 type ConfigProfilesAction string
 
 const (

internal/url/url_test.go

@@ -59,6 +59,7 @@ func testUrlOpenerWithError(url, browser string) error {
 }
 
 func TestHandleUrl(t *testing.T) {
+	t.Parallel()
 	noCommand := []string{}
 	assert.Panics(t, func() { NewHandleUrl(Exec, "foo", "browser", noCommand) })
 
@@ -142,6 +143,7 @@ func TestHandleUrl(t *testing.T) {
 }
 
 func TestFirefoxContainersUrl(t *testing.T) {
+	t.Parallel()
 	assert.Equal(t, "ext+container:name=Test&url=https%3A%2F%2Fsynfin.net&color=blue&icon=fingerprint",
 		formatContainerUrl(FIREFOX_CONTAINER_FORMAT, "https://synfin.net", "Test", "blue", "fingerprint"))
 
@@ -153,6 +155,7 @@ func TestFirefoxContainersUrl(t *testing.T) {
 }
 
 func TestCommandBuilder(t *testing.T) {
+	t.Parallel()
 	_, _, err := commandBuilder([]string{}, "url")
 	assert.Error(t, err)
 
@@ -181,6 +184,7 @@ func TestCommandBuilder(t *testing.T) {
 }
 
 func TestSelectElement(t *testing.T) {
+	t.Parallel()
 	check := map[string]string{
 		"a":  "turquoise", // 97 % 8 => 1
 		"aa": "green",     // 194 % 8 => 2
@@ -192,6 +196,7 @@ func TestSelectElement(t *testing.T) {
 }
 
 func TestIsContainer(t *testing.T) {
+	t.Parallel()
 	a := Action(Open)
 	assert.False(t, a.IsContainer())
 	a = Exec
@@ -207,6 +212,7 @@ func TestIsContainer(t *testing.T) {
 }
 
 func TestNewAction(t *testing.T) {
+	t.Parallel()
 	a, err := NewAction("clip")
 	assert.NoError(t, err)
 	assert.Equal(t, Clip, a)
@@ -235,6 +241,7 @@ func TestLogger(t *testing.T) {
 	assert.Equal(t, log, l)
 }
 func TestNewConfigProfilesAction(t *testing.T) {
+	t.Parallel()
 	a, err := NewConfigProfilesAction("exec")
 	assert.NoError(t, err)
 	assert.Equal(t, a, ConfigProfilesAction("exec"), a)
@@ -249,6 +256,7 @@ func TestNewConfigProfilesAction(t *testing.T) {
 }
 
 func TestSSOAuthAction(t *testing.T) {
+	t.Parallel()
 	// no change
 	a, _ := NewAction("clip")
 	assert.Equal(t, a, SSOAuthAction(Clip))
@@ -270,6 +278,7 @@ func TestSSOAuthAction(t *testing.T) {
 }
 
 func TestAWSFederatedUrl(t *testing.T) {
+	t.Parallel()
 	u := AWSFederatedUrl("us-east-1")
 	assert.Equal(t, u, "https://us-east-1.signin.aws.amazon.com/federation")
 	pUrl, err := url.Parse(u)
@@ -293,6 +302,7 @@ func TestAWSFederatedUrl(t *testing.T) {
 }
 
 func TestAWSConsoleUrl(t *testing.T) {
+	t.Parallel()
 	u := AWSConsoleUrl("cn-north-1", "cn-northwest-1")
 	assert.Equal(t, u, "https://console.amazonaws.cn/console/home?region=cn-northwest-1")
 	pUrl, err := url.Parse(u)
@@ -314,3 +324,15 @@ func TestAWSConsoleUrl(t *testing.T) {
 	_, err = net.LookupIP(pUrl.Hostname())
 	assert.NoError(t, err)
 }
+
+func TestGetConfigProfilesAction(t *testing.T) {
+	t.Parallel()
+	action := Open
+	assert.Equal(t, ConfigProfilesOpen, action.GetConfigProfilesAction())
+
+	action = Clip
+	assert.Equal(t, ConfigProfilesClip, action.GetConfigProfilesAction())
+
+	action = Print
+	assert.Equal(t, ConfigProfilesOpen, action.GetConfigProfilesAction())
+}

sso/settings.go

@@ -146,15 +146,15 @@ func LoadSettings(configFile, cacheFile string, defaults map[string]interface{},
 	// default values.  Can be overridden using:
 	// https://pkg.go.dev/github.com/c-bata/go-prompt?utm_source=godoc#Color
 	if err = konf.Load(confmap.Provider(defaults, "."), nil); err != nil {
-		return s, fmt.Errorf("Unable to load default settings: %s", err.Error())
+		return s, fmt.Errorf("unable to load default settings: %s", err.Error())
 	}
 
 	if err = konf.Load(file.Provider(configFile), yaml.Parser()); err != nil {
-		return s, fmt.Errorf("Unable to open config file %s: %s", configFile, err.Error())
+		return s, fmt.Errorf("unable to open config file %s: %s", configFile, err.Error())
 	}
 
 	if err = konf.Unmarshal("", s); err != nil {
-		return s, fmt.Errorf("Unable to process config file: %s", err.Error())
+		return s, fmt.Errorf("unable to process config file: %s", err.Error())
 	}
 
 	if len(s.AccountPrimaryTag) == 0 {
@@ -171,7 +171,7 @@ func LoadSettings(configFile, cacheFile string, defaults map[string]interface{},
 	if _, ok := s.SSO[s.DefaultSSO]; !ok {
 		// Select our SSO Provider
 		if len(s.SSO) == 0 {
-			return s, fmt.Errorf("No AWS SSO providers have been configured.")
+			return s, fmt.Errorf("no AWS SSO providers have been configured")
 		} else if len(s.SSO) == 1 {
 			// If we have only one SSO configured, always use that
 			for name := range s.SSO {
@@ -184,18 +184,24 @@ func LoadSettings(configFile, cacheFile string, defaults map[string]interface{},
 				names = append(names, sso)
 			}
 			if len(names) > 0 {
-				return s, fmt.Errorf("Invalid SSO name '%s'. Valid options: %s", s.DefaultSSO,
+				return s, fmt.Errorf("invalid SSO name '%s'. Valid options: %s", s.DefaultSSO,
 					strings.Join(names, ", "))
 			} else {
 				// couldn't find a valid provider
-				return s, fmt.Errorf("Please specify --sso, $AWS_SSO or set DefaultSSO in the config file")
+				return s, fmt.Errorf("please specify --sso, $AWS_SSO or set DefaultSSO in the config file")
 			}
 		}
 	}
 
 	s.SSO[s.DefaultSSO].Refresh(s)
 
 	s.applyDeprecations()
+
+	// ConfigProfilesUrlAction should track UrlAcition unless it is set
+	if s.ConfigProfilesUrlAction == "" {
+		s.ConfigProfilesUrlAction = s.UrlAction.GetConfigProfilesAction()
+	}
+
 	if err = s.Validate(); err != nil {
 		return s, err
 	}
@@ -212,7 +218,7 @@ func (s *Settings) Validate() error {
 	// Does either action call `exec` without firefox containers?
 	if s.UrlAction.IsContainer() != s.ConfigProfilesUrlAction.IsContainer() {
 		if s.UrlAction == url.Exec || s.ConfigProfilesUrlAction == url.ConfigProfilesExec {
-			return fmt.Errorf("Must not select `exec` and a Firefox container option")
+			return fmt.Errorf("must not select `exec` and a Firefox container option")
 		}
 	}
 
@@ -270,7 +276,7 @@ func (s *Settings) Save(configFile string, overwrite bool) error {
 	var err error
 
 	if _, err = os.Stat(configFile); !errors.Is(err, os.ErrNotExist) && !overwrite {
-		return fmt.Errorf("Refusing to overwrite %s", configFile)
+		return fmt.Errorf("refusing to overwrite %s", configFile)
 	}
 
 	var output bytes.Buffer
@@ -286,7 +292,7 @@ func (s *Settings) Save(configFile string, overwrite bool) error {
 	w.Flush()
 	fileBytes := output.Bytes()
 	if len(fileBytes) == 0 {
-		return fmt.Errorf("Refusing to write 0 bytes to config.yaml")
+		return fmt.Errorf("refusing to write 0 bytes to config.yaml")
 	}
 
 	configDir := utils.GetHomePath(filepath.Dir(configFile))
@@ -389,7 +395,7 @@ func (s *Settings) GetSelectedSSOName(name string) (string, error) {
 	if _, ok := s.SSO["Default"]; ok {
 		return "Default", nil
 	}
-	return "", fmt.Errorf("No available AWS SSO Instance")
+	return "", fmt.Errorf("no available AWS SSO Instance")
 }
 
 // Returns the Tag name => Environment variable name
@@ -478,7 +484,7 @@ func (p *ProfileMap) UniqueCheck(s *Settings) error {
 			}
 
 			if match, duplicate := profileUniqueCheck[profile]; duplicate {
-				return fmt.Errorf("Duplicate profile name '%s' for:\n%s: %s\n%s: %s",
+				return fmt.Errorf("duplicate profile name '%s' for:\n%s: %s\n%s: %s",
 					profile, match[0], match[1], ssoName, role.Arn)
 			}
 			profileUniqueCheck[profile] = []string{ssoName, role.Arn}

sso/settings_test.go

@@ -62,15 +62,32 @@ func (suite *SettingsTestSuite) TestLoadSettings() {
 
 	assert.Equal(t, TEST_SETTINGS_FILE, suite.settings.ConfigFile())
 
-	// ensure we upgraded ConfigUrlAction to ConfigProfilesUrlAction
-	assert.Equal(t, "", suite.settings.ConfigUrlAction)
+	assert.Equal(t, "", suite.settings.ConfigUrlAction) // deprecated
+	// ensure we upgraded ConfigUrlAction to UrlAction
+	assert.Equal(t, url.OpenUrlContainer, suite.settings.UrlAction)
+	// ensure we applied UrlAction to ConfigProfilesUrlAction
 	assert.Equal(t, url.ConfigProfilesOpen, suite.settings.ConfigProfilesUrlAction)
 
 	// ensure we upgraded FirefoxOpenUrlInContainer
 	assert.False(t, suite.settings.FirefoxOpenUrlInContainer)
 	assert.Equal(t, url.OpenUrlContainer, suite.settings.UrlAction)
 }
 
+func TestConfigProfilesUrlAction(t *testing.T) {
+	t.Parallel()
+	settingsFile := "./testdata/settings2.yaml"
+	settings, err := LoadSettings(settingsFile, TEST_CACHE_FILE, map[string]interface{}{}, OverrideSettings{})
+	assert.NoError(t, err)
+
+	assert.Equal(t, url.ConfigProfilesOpen, settings.ConfigProfilesUrlAction)
+
+	settingsFile = "./testdata/settings3.yaml"
+	settings, err = LoadSettings(settingsFile, TEST_CACHE_FILE, map[string]interface{}{}, OverrideSettings{})
+	assert.NoError(t, err)
+
+	assert.Equal(t, url.ConfigProfilesOpenUrlContainer, settings.ConfigProfilesUrlAction)
+}
+
 func (suite *SettingsTestSuite) TestGetSelectedSSO() {
 	t := suite.T()
 

sso/testdata/settings.yaml

@@ -60,13 +60,13 @@ SSOConfig:
 DefaultSSO: Default                       
 Browser: /Applications/Firefox.app
 UrlAction: print
+ConfigUrlAction: open
 FirefoxOpenUrlInContainer: true
 SecureStore: json
 JsonStore: ./testdata/store.json
 ProfileFormat: "{{.AccountName}}/{{.RoleName}}"
 AccountPrimaryTag:
   - AccountAlias
-ConfigUrlAction: open
 LogLevel: warn
 DefaultRegion: us-west-2
 EnvVarTags:

sso/testdata/settings2.yaml

@@ -0,0 +1,73 @@
+SSOConfig:
+    Default:
+        SSORegion: us-east-1
+        StartUrl: https://d-754545454.awsapps.com/start
+        DefaultRegion: us-east-1
+        Accounts:
+            258234615182:
+                Name: OurCompany Control Tower Playground
+                DefaultRegion: eu-west-1
+                Tags:
+                  - Type: Main Account
+                Roles:
+                  AWSAdministratorAccess:
+                    DefaultRegion: ca-central-1
+                    Tags:
+                      Test: value
+                      Foo: Bar
+                  LimitedAccess:
+                    Tags:
+                      Test: value
+                      Foo: Moo
+            833365043586:
+                Name: Log archive
+                Tags:
+                  Type: Sub Account
+                Roles:
+                  AWSAdministratorAccess:
+                    Tags:
+                      Test: logs
+                      Foo: Bar
+                      Can: Man
+            502470824893:
+                Name: Audit
+                Tags:
+                  Type: Sub Account
+            707513610766:
+                Name: Dev Account
+                Tags:
+                  Type: Sub Account
+    Another:
+        SSORegion: us-east-1
+        StartUrl: https://d-755555555.awsapps.com/start
+        Accounts:
+            182347455:
+                Name: Whatever
+                Roles:
+                  AWSAdministratorAccess:
+                    Tags:
+                      Test: moocow
+                      Bar: baz
+    Bug292:
+      SSORegion: us-east-1
+      StartUrl: https://d-88888888888.awsapps.com/start
+      Accounts:
+        0012345678912:
+          Name: MyTestName
+          Roles:
+            FooBar:
+
+DefaultSSO: Default                       
+Browser: /Applications/Firefox.app
+UrlAction: print
+SecureStore: json
+JsonStore: ./testdata/store.json
+ProfileFormat: "{{.AccountName}}/{{.RoleName}}"
+AccountPrimaryTag:
+  - AccountAlias
+LogLevel: warn
+DefaultRegion: us-west-2
+EnvVarTags:
+  - Role 
+  - Arn
+  - Foo