AWS profiles in creds INI file w/ deprecated variables

README.md

@@ -165,7 +165,8 @@ Also see the CLI's online help `$ okta-aws-cli --help`
 | Display QR Code (optional) | `QR_CODE=true` | `--qr-code` | `true` if flag is present  |
 | Automatically open the activation URL with the system web browser (optional) | `OPEN_BROWSER=true` | `--open-browser` | `true` if flag is present  |
 | Alternate AWS credentials file path (optional) | `AWS_CREDENTIALS` | `--aws-credentials` | Path to alternative credentials file other than AWS CLI default |
-| Write to the AWS credentials file (optional). Default formatting is to append and not modify the file beyond adding new lines. WARNING: When enabled, writing can inadvertently remove dangling comments and extraneous formatting from the creds file. | `WRITE_AWS_CREDENTIALS=true` | `--write-aws-credentials` | `true` if flag is present  |
+| (Over)write the given profile to the AWS credentials file (optional). WARNING: When enabled, overwriting can inadvertently remove dangling comments and extraneous formatting from the creds file. | `WRITE_AWS_CREDENTIALS=true` | `--write-aws-credentials` | `true` if flag is present  |
+| Emit deprecated AWS variable `aws_security_token` with duplicated value from `aws_session_token` | `LEGACY_AWS_VARIABLES=true` | `--legacy-aws-variables` | `true` if flag is present  |
 | Verbosely print all API calls/responses to the screen | `DEBUG_API_CALLS=true` | `--debug-api-calls` | `true` if flag is present  |
 
 ### Allowed Web SSO Client
@@ -344,9 +345,9 @@ configuration file that is dropped somewhere in the user's `$HOME` directory to
 operate the CLI.
 
 The Okta CLI is CLI flag and environment variable oriented and its default
-output is as environment variables. It can write to an AWS credentials file but
-only in append mode. It never risks interpreting and re-writing the AWS
-credentials file potentially corrupting other valuable credentials saved there.
+output is as environment variables. It can also write to AWS credentials file.
+The default writing option is an apped operation and can be explicitly set to
+overwrite previous values for a profile with the `--write-aws-credentials` flag.
 
 ### Versent saml2aws
 

cmd/root/root.go

@@ -135,6 +135,13 @@ func init() {
 			usage:  fmt.Sprintf("Write the created/updated profile to the %q file. WARNING: This can inadvertently remove dangling comments and extraneous formatting from the creds file.", awsCredentialsFilename),
 			envVar: config.WriteAWSCredentialsEnvVar,
 		},
+		{
+			name:   config.LegacyAWSVariablesFlag,
+			short:  "l",
+			value:  false,
+			usage:  "Emit deprecated AWS Security Token value. WARNING: AWS CLI deprecated this value in November 2014 and is no longer documented",
+			envVar: config.LegacyAWSVariablesEnvVar,
+		},
 		{
 			name:   config.DebugAPICallsFlag,
 			short:  "x",

internal/aws/aws.go

@@ -22,3 +22,11 @@ type Credential struct {
 	SecretAccessKey string `ini:"aws_secret_access_key"`
 	SessionToken    string `ini:"aws_session_token"`
 }
+
+// LegacyCredential Convenience representation of an AWS credential.
+type LegacyCredential struct {
+	AccessKeyID     string `ini:"aws_access_key_id"`
+	SecretAccessKey string `ini:"aws_secret_access_key"`
+	SessionToken    string `ini:"aws_session_token"`
+	SecurityToken   string `ini:"aws_security_token"`
+}

internal/config/config.go

@@ -60,6 +60,8 @@ const (
 	SessionDurationFlag = "session-duration"
 	// WriteAWSCredentialsFlag cli flag const
 	WriteAWSCredentialsFlag = "write-aws-credentials"
+	// LegacyAWSVariablesFlag cli flag const
+	LegacyAWSVariablesFlag = "legacy-aws-variables"
 
 	// AWSCredentialsEnvVar env var const
 	AWSCredentialsEnvVar = "AWS_CREDENTIALS"
@@ -87,6 +89,8 @@ const (
 	WriteAWSCredentialsEnvVar = "WRITE_AWS_CREDENTIALS"
 	// DebugAPICallsEnvVar env var const
 	DebugAPICallsEnvVar = "DEBUG_API_CALLS"
+	// LegacyAWSVariablesEnvVar env var const
+	LegacyAWSVariablesEnvVar = "LEGACY_AWS_VARIABLES"
 )
 
 // Config A config object for the CLI
@@ -104,6 +108,7 @@ type Config struct {
 	WriteAWSCredentials bool
 	OpenBrowser         bool
 	DebugAPICalls       bool
+	LegacyAWSVariables  bool
 	HTTPClient          *http.Client
 }
 
@@ -120,6 +125,7 @@ func NewConfig() *Config {
 		DebugAPICalls:       viper.GetBool(DebugAPICallsFlag),
 		FedAppID:            viper.GetString(AWSAcctFedAppIDFlag),
 		Format:              viper.GetString(FormatFlag),
+		LegacyAWSVariables:  viper.GetBool(LegacyAWSVariablesFlag),
 		OIDCAppID:           viper.GetString(OIDCClientIDFlag),
 		OpenBrowser:         viper.GetBool(OpenBrowserFlag),
 		OrgDomain:           viper.GetString(OrgDomainFlag),
@@ -177,14 +183,15 @@ func NewConfig() *Config {
 		// writing aws creds option implies "aws-credentials" format
 		cfg.Format = AWSCredentialsFormat
 	}
-
 	if !cfg.OpenBrowser {
 		cfg.OpenBrowser = viper.GetBool(downCase(OpenBrowserEnvVar))
 	}
-
 	if !cfg.DebugAPICalls {
 		cfg.DebugAPICalls = viper.GetBool(downCase(DebugAPICallsEnvVar))
 	}
+	if !cfg.LegacyAWSVariables {
+		cfg.LegacyAWSVariables = viper.GetBool(downCase(LegacyAWSVariablesEnvVar))
+	}
 	httpClient := &http.Client{
 		Transport: newConfigTransport(cfg.DebugAPICalls),
 		Timeout:   time.Second * time.Duration(60),

internal/output/aws_credentials_file.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"strings"
 
 	"github.com/okta/okta-aws-cli/internal/aws"
 	"github.com/okta/okta-aws-cli/internal/config"
@@ -52,8 +53,8 @@ func ensureConfigExists(filename string, profile string) error {
 	return nil
 }
 
-func saveProfile(filename, profile string, awsCreds *aws.Credential) error {
-	config, err := updateConfig(filename, profile, awsCreds)
+func saveProfile(filename, profile string, awsCreds *aws.Credential, legacyVars bool) error {
+	config, err := updateConfig(filename, profile, awsCreds, legacyVars)
 	if err != nil {
 		return err
 	}
@@ -67,7 +68,7 @@ func saveProfile(filename, profile string, awsCreds *aws.Credential) error {
 	return nil
 }
 
-func updateConfig(filename, profile string, awsCreds *aws.Credential) (config *ini.File, err error) {
+func updateConfig(filename, profile string, awsCreds *aws.Credential, legacyVars bool) (config *ini.File, err error) {
 	config, err = ini.Load(filename)
 	if err != nil {
 		return
@@ -77,18 +78,77 @@ func updateConfig(filename, profile string, awsCreds *aws.Credential) (config *i
 	if err != nil {
 		return
 	}
+	var creds interface{}
+	if legacyVars {
+		creds = &aws.LegacyCredential{
+			AccessKeyID:     awsCreds.AccessKeyID,
+			SecretAccessKey: awsCreds.SecretAccessKey,
+			SessionToken:    awsCreds.SessionToken,
+			SecurityToken:   awsCreds.SessionToken,
+		}
+	} else {
+		creds = awsCreds
+	}
+	err = iniProfile.ReflectFrom(creds)
+	if err != nil {
+		return
+	}
+
+	return updateINI(config, profile, legacyVars)
+}
+
+// updateIni will comment out any keys that are not "aws_access_key_id",
+// "aws_secret_access_key", or "aws_session_token"
+func updateINI(config *ini.File, profile string, legacyVars bool) (*ini.File, error) {
+	ignore := []string{
+		"aws_access_key_id",
+		"aws_secret_access_key",
+		"aws_session_token",
+	}
+	if legacyVars {
+		ignore = append(ignore, "aws_security_token")
+	}
+	section := config.Section(profile)
+	comments := []string{}
+	for _, name := range section.KeyStrings() {
+		if contains(ignore, name) {
+			continue
+		}
+		if len(name) > 0 && string(name[0]) == "#" {
+			continue
+		}
 
-	err = iniProfile.ReflectFrom(awsCreds)
+		key, err := section.GetKey(name)
+		if err != nil {
+			continue
+		}
 
-	return
+		// The named key is in the profile but it's not utilized by
+		// okta-aws-cli. Therefore comment it out but do not delete.
+		_, _ = section.NewKey(fmt.Sprintf("# %s", name), key.Value())
+		section.DeleteKey(name)
+		comments = append(comments, name)
+	}
+	if len(comments) > 0 {
+		fmt.Fprintf(os.Stderr, "WARNING: Commented out %q profile keys \"%s\". Uncomment if third party tools use these values.\n", profile, strings.Join(comments, "\", \""))
+	}
+	if legacyVars {
+		fmt.Fprintf(os.Stderr, "WARNING: %q includes legacy variable \"aws_security_token\". Update tools making use of this deprecated value.", profile)
+	}
+
+	return config, nil
 }
 
 // AWSCredentialsFile AWS credentials file output formatter
-type AWSCredentialsFile struct{}
+type AWSCredentialsFile struct {
+	LegacyAWSVariables bool
+}
 
 // NewAWSCredentialsFile Creates a new
-func NewAWSCredentialsFile() *AWSCredentialsFile {
-	return &AWSCredentialsFile{}
+func NewAWSCredentialsFile(legacyVars bool) *AWSCredentialsFile {
+	return &AWSCredentialsFile{
+		LegacyAWSVariables: legacyVars,
+	}
 }
 
 // Output Satisfies the Outputter interface and appends AWS credentials to
@@ -110,13 +170,26 @@ func (e *AWSCredentialsFile) appendConfig(c *config.Config, ac *aws.Credential)
 		_ = f.Close()
 	}()
 
-	creds := `
+	var creds string
+
+	if e.LegacyAWSVariables {
+		creds = `
 [%s]
 aws_access_key_id = %s
 aws_secret_access_key = %s
 aws_session_token = %s
+aws_security_token = %s
 `
-	creds = fmt.Sprintf(creds, c.Profile, ac.AccessKeyID, ac.SecretAccessKey, ac.SessionToken)
+		creds = fmt.Sprintf(creds, c.Profile, ac.AccessKeyID, ac.SecretAccessKey, ac.SessionToken, ac.SessionToken)
+	} else {
+		creds = `
+[%s]
+aws_access_key_id = %s
+aws_secret_access_key = %s
+aws_session_token = %s
+`
+		creds = fmt.Sprintf(creds, c.Profile, ac.AccessKeyID, ac.SecretAccessKey, ac.SessionToken)
+	}
 	_, err = f.WriteString(creds)
 	if err != nil {
 		return err
@@ -137,5 +210,15 @@ func (e *AWSCredentialsFile) writeConfig(c *config.Config, ac *aws.Credential) e
 		return err
 	}
 
-	return saveProfile(filename, profile, ac)
+	return saveProfile(filename, profile, ac, e.LegacyAWSVariables)
+}
+
+func contains(ignore []string, name string) bool {
+	for _, v := range ignore {
+		if v == name {
+			return true
+		}
+	}
+
+	return false
 }

internal/output/aws_credentials_file_test.go

@@ -23,6 +23,8 @@ import (
 
 	"github.com/okta/okta-aws-cli/internal/aws"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"gopkg.in/ini.v1"
 )
 
 // TestINIFormatCredentialsContent provides a litmus test on how well
@@ -33,6 +35,8 @@ import (
 // At the time this test was written the INI package would trim out extra new
 // lines and dangling comments.
 func TestINIFormatCredentialsContent(t *testing.T) {
+	t.Skip("leaving for INI package investigations in the future")
+
 	have, err := credsTemplate([]interface{}{"A", "B", "C", "D", "E", "F"})
 	assert.NoError(t, err)
 	want, err := credsTemplate([]interface{}{"A", "B", "C", "d", "e", "f"})
@@ -54,7 +58,7 @@ func TestINIFormatCredentialsContent(t *testing.T) {
 		SecretAccessKey: "e",
 		SessionToken:    "f",
 	}
-	config, err := updateConfig(filename, "test", awsCreds)
+	config, err := updateConfig(filename, "test", awsCreds, false)
 	assert.NoError(t, err)
 
 	err = config.SaveTo(filename)
@@ -69,6 +73,87 @@ func TestINIFormatCredentialsContent(t *testing.T) {
 	}
 }
 
+func TestINIComments(t *testing.T) {
+	tests := []struct {
+		name    string
+		section string
+		config  []byte
+		want    map[string]string
+		legacy  bool
+	}{
+		{
+			name:    "default",
+			section: "default",
+			config: []byte(`
+[default]
+aws_session_token     = abc
+aws_access_key_id     = def
+aws_secret_access_key = ghi
+`),
+			want: map[string]string{
+				"aws_session_token":     "abc",
+				"aws_access_key_id":     "def",
+				"aws_secret_access_key": "ghi",
+			},
+			legacy: false,
+		},
+		{
+			name:    "obsolete variables",
+			section: "default",
+			config: []byte(`
+[default]
+aws_access_key_id     = abc
+aws_secret_access_key = def
+aws_session_token     = ghi
+aws_security_token    = jkl
+`),
+			want: map[string]string{
+				"aws_access_key_id":     "abc",
+				"aws_secret_access_key": "def",
+				"aws_session_token":     "ghi",
+				"# aws_security_token":  "jkl",
+			},
+			legacy: false,
+		},
+		{
+			name:    "legacy variables",
+			section: "default",
+			config: []byte(`
+[default]
+other                 = test
+aws_access_key_id     = abc
+aws_secret_access_key = def
+aws_session_token     = ghi
+aws_security_token    = ghi
+`),
+			want: map[string]string{
+				"# other":               "test",
+				"aws_access_key_id":     "abc",
+				"aws_secret_access_key": "def",
+				"aws_session_token":     "ghi",
+				"aws_security_token":    "ghi",
+			},
+			legacy: true,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			config, err := ini.Load(test.config)
+			require.NoError(t, err)
+			ini, err := updateINI(config, "default", test.legacy)
+			require.NoError(t, err)
+			section := ini.Section(test.section)
+			require.Equal(t, len(test.want), len(section.KeyStrings()))
+			for k := range test.want {
+				val, err := section.GetKey(k)
+				require.NoError(t, err)
+				require.Equal(t, test.want[k], val.Value())
+			}
+		})
+	}
+}
+
 func credsTemplate(vars []any) (string, error) {
 	if len(vars) != 6 {
 		return "", fmt.Errorf("expected 6 vars got %d", len(vars))