Use Idp Account Name as key for credentials store

cmd/saml2aws/commands/configure.go

@@ -6,7 +6,7 @@ import (
 	"path"
 
 	"github.com/pkg/errors"
-	"github.com/versent/saml2aws/v2"
+	saml2aws "github.com/versent/saml2aws/v2"
 	"github.com/versent/saml2aws/v2/helper/credentials"
 	"github.com/versent/saml2aws/v2/pkg/cfg"
 	"github.com/versent/saml2aws/v2/pkg/flags"
@@ -68,14 +68,14 @@ func storeCredentials(configFlags *flags.CommonFlags, account *cfg.IDPAccount) e
 		return nil
 	}
 	if configFlags.Password != "" {
-		if err := credentials.SaveCredentials(account.URL, account.Username, configFlags.Password); err != nil {
+		if err := credentials.SaveCredentials(account.Name, account.URL, account.Username, configFlags.Password); err != nil {
 			return errors.Wrap(err, "error storing password in keychain")
 		}
 	} else {
 		password := prompter.Password("Password")
 		if password != "" {
 			if confirmPassword := prompter.Password("Confirm"); confirmPassword == password {
-				if err := credentials.SaveCredentials(account.URL, account.Username, password); err != nil {
+				if err := credentials.SaveCredentials(account.Name, account.URL, account.Username, password); err != nil {
 					return errors.Wrap(err, "error storing password in keychain")
 				}
 			} else {
@@ -91,7 +91,8 @@ func storeCredentials(configFlags *flags.CommonFlags, account *cfg.IDPAccount) e
 			log.Println("OneLogin provider requires --client-id and --client-secret flags to be set.")
 			os.Exit(1)
 		}
-		if err := credentials.SaveCredentials(path.Join(account.URL, OneLoginOAuthPath), configFlags.ClientID, configFlags.ClientSecret); err != nil {
+		// we store the OneLogin token in a different secret (idpName + the one login suffix)
+		if err := credentials.SaveCredentials(account.Name+credentials.OneLoginTokenSuffix, path.Join(account.URL, OneLoginOAuthPath), configFlags.ClientID, configFlags.ClientSecret); err != nil {
 			return errors.Wrap(err, "error storing client_id and client_secret in keychain")
 		}
 	}

cmd/saml2aws/commands/list_roles.go

@@ -8,7 +8,7 @@ import (
 
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
-	"github.com/versent/saml2aws/v2"
+	saml2aws "github.com/versent/saml2aws/v2"
 	"github.com/versent/saml2aws/v2/helper/credentials"
 	"github.com/versent/saml2aws/v2/pkg/flags"
 	"github.com/versent/saml2aws/v2/pkg/samlcache"
@@ -83,7 +83,7 @@ func ListRoles(loginFlags *flags.LoginExecFlags) error {
 	}
 
 	if !loginFlags.CommonFlags.DisableKeychain {
-		err = credentials.SaveCredentials(loginDetails.URL, loginDetails.Username, loginDetails.Password)
+		err = credentials.SaveCredentials(loginDetails.IdpName, loginDetails.URL, loginDetails.Username, loginDetails.Password)
 		if err != nil {
 			return errors.Wrap(err, "error storing password in keychain")
 		}

cmd/saml2aws/commands/login.go

@@ -14,7 +14,7 @@ import (
 	"github.com/aws/aws-sdk-go/service/sts"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
-	"github.com/versent/saml2aws/v2"
+	saml2aws "github.com/versent/saml2aws/v2"
 	"github.com/versent/saml2aws/v2/helper/credentials"
 	"github.com/versent/saml2aws/v2/pkg/awsconfig"
 	"github.com/versent/saml2aws/v2/pkg/cfg"
@@ -122,7 +122,7 @@ func Login(loginFlags *flags.LoginExecFlags) error {
 	}
 
 	if !loginFlags.CommonFlags.DisableKeychain {
-		err = credentials.SaveCredentials(loginDetails.URL, loginDetails.Username, loginDetails.Password)
+		err = credentials.SaveCredentials(loginDetails.IdpName, loginDetails.URL, loginDetails.Username, loginDetails.Password)
 		if err != nil {
 			return errors.Wrap(err, "Error storing password in keychain.")
 		}
@@ -174,15 +174,20 @@ func buildIdpAccount(loginFlags *flags.LoginExecFlags) (*cfg.IDPAccount, error)
 
 func resolveLoginDetails(account *cfg.IDPAccount, loginFlags *flags.LoginExecFlags) (*creds.LoginDetails, error) {
 
-	// log.Printf("loginFlags %+v", loginFlags)
-
-	loginDetails := &creds.LoginDetails{URL: account.URL, Username: account.Username, MFAToken: loginFlags.CommonFlags.MFAToken, DuoMFAOption: loginFlags.DuoMFAOption}
+	loginDetails := &creds.LoginDetails{
+		URL:          account.URL,
+		Username:     account.Username,
+		MFAToken:     loginFlags.CommonFlags.MFAToken,
+		DuoMFAOption: loginFlags.DuoMFAOption,
+		IdpName:      account.Name,
+		IdpProvider:  account.Provider,
+	}
 
 	log.Printf("Using IdP Account %s to access %s %s", loginFlags.CommonFlags.IdpAccount, account.Provider, account.URL)
 
 	var err error
 	if !loginFlags.CommonFlags.DisableKeychain {
-		err = credentials.LookupCredentials(loginDetails, account.Provider)
+		err = credentials.LookupCredentials(loginDetails)
 		if err != nil {
 			if !credentials.IsErrCredentialsNotFound(err) {
 				return nil, errors.Wrap(err, "Error loading saved password.")

cmd/saml2aws/commands/login_test.go

@@ -6,19 +6,27 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/assert"
-	"github.com/versent/saml2aws/v2"
+
+	saml2aws "github.com/versent/saml2aws/v2"
 	"github.com/versent/saml2aws/v2/pkg/awsconfig"
 	"github.com/versent/saml2aws/v2/pkg/cfg"
 	"github.com/versent/saml2aws/v2/pkg/creds"
 	"github.com/versent/saml2aws/v2/pkg/flags"
 )
 
 func TestResolveLoginDetailsWithFlags(t *testing.T) {
+	commonFlags := &flags.CommonFlags{
+		URL:        "https://id.example.com",
+		Username:   "wolfeidau",
+		Password:   "testtestlol",
+		MFAToken:   "123456",
+		SkipPrompt: true,
+	}
 
-	commonFlags := &flags.CommonFlags{URL: "https://id.example.com", Username: "wolfeidau", Password: "testtestlol", MFAIPAddress: "127.0.0.1", MFAToken: "123456", SkipPrompt: true}
 	loginFlags := &flags.LoginExecFlags{CommonFlags: commonFlags}
 
 	idpa := &cfg.IDPAccount{
+		Name:     "AccountName",
 		URL:      "https://id.example.com",
 		MFA:      "none",
 		Provider: "Ping",
@@ -27,16 +35,30 @@ func TestResolveLoginDetailsWithFlags(t *testing.T) {
 	loginDetails, err := resolveLoginDetails(idpa, loginFlags)
 
 	assert.Empty(t, err)
-	assert.Equal(t, &creds.LoginDetails{Username: "wolfeidau", Password: "testtestlol", URL: "https://id.example.com", MFAToken: "123456", MFAIPAddress: "127.0.0.1"}, loginDetails)
+	assert.Equal(t,
+		&creds.LoginDetails{
+			IdpName:     "AccountName",
+			IdpProvider: "Ping",
+			Username:    "wolfeidau",
+			Password:    "testtestlol",
+			URL:         "https://id.example.com",
+			MFAToken:    "123456",
+		}, loginDetails)
 }
 
 func TestOktaResolveLoginDetailsWithFlags(t *testing.T) {
-
 	// Default state - user did not supply values for DisableSessions and DisableSessions
-	commonFlags := &flags.CommonFlags{URL: "https://id.example.com", Username: "testuser", Password: "testtestlol", MFAToken: "123456", SkipPrompt: true}
+	commonFlags := &flags.CommonFlags{
+		URL:        "https://id.example.com",
+		Username:   "testuser",
+		Password:   "testtestlol",
+		MFAToken:   "123456",
+		SkipPrompt: true,
+	}
 	loginFlags := &flags.LoginExecFlags{CommonFlags: commonFlags}
 
 	idpa := &cfg.IDPAccount{
+		Name:     "AnotherAccountName",
 		URL:      "https://id.example.com",
 		MFA:      "none",
 		Provider: "Okta",
@@ -47,24 +69,45 @@ func TestOktaResolveLoginDetailsWithFlags(t *testing.T) {
 	assert.Nil(t, err)
 	assert.False(t, idpa.DisableSessions, fmt.Errorf("default state, DisableSessions should be false"))
 	assert.False(t, idpa.DisableRememberDevice, fmt.Errorf("default state, DisableRememberDevice should be false"))
-	assert.Equal(t, &creds.LoginDetails{Username: "testuser", Password: "testtestlol", URL: "https://id.example.com", MFAToken: "123456"}, loginDetails)
+	assert.Equal(t,
+		&creds.LoginDetails{
+			IdpName:     "AnotherAccountName",
+			IdpProvider: "Okta",
+			Username:    "testuser",
+			Password:    "testtestlol",
+			URL:         "https://id.example.com",
+			MFAToken:    "123456",
+		}, loginDetails)
 
 	// User disabled keychain, resolveLoginDetails should set the account's DisableSessions and DisableSessions fields to true
 
-	commonFlags = &flags.CommonFlags{URL: "https://id.example.com", Username: "testuser", Password: "testtestlol", MFAToken: "123456", SkipPrompt: true, DisableKeychain: true}
+	commonFlags = &flags.CommonFlags{
+		URL:             "https://id.example.com",
+		Username:        "testuser",
+		Password:        "testtestlol",
+		MFAToken:        "123456",
+		SkipPrompt:      true,
+		DisableKeychain: true,
+	}
 	loginFlags = &flags.LoginExecFlags{CommonFlags: commonFlags}
 
 	loginDetails, err = resolveLoginDetails(idpa, loginFlags)
 
 	assert.Nil(t, err)
 	assert.True(t, idpa.DisableSessions, fmt.Errorf("user disabled keychain, DisableSessions should be true"))
 	assert.True(t, idpa.DisableRememberDevice, fmt.Errorf("user disabled keychain, DisableRememberDevice should be true"))
-	assert.Equal(t, &creds.LoginDetails{Username: "testuser", Password: "testtestlol", URL: "https://id.example.com", MFAToken: "123456"}, loginDetails)
-
+	assert.Equal(t,
+		&creds.LoginDetails{
+			IdpName:     "AnotherAccountName",
+			IdpProvider: "Okta",
+			Username:    "testuser",
+			Password:    "testtestlol",
+			URL:         "https://id.example.com",
+			MFAToken:    "123456",
+		}, loginDetails)
 }
 
 func TestResolveRoleSingleEntry(t *testing.T) {
-
 	adminRole := &saml2aws.AWSRole{
 		Name:         "admin",
 		RoleARN:      "arn:aws:iam::456456456456:saml-provider/example-idp,arn:aws:iam::456456456456:role/admin",
@@ -81,7 +124,6 @@ func TestResolveRoleSingleEntry(t *testing.T) {
 }
 
 func TestCredentialsToCredentialProcess(t *testing.T) {
-
 	aws_creds := &awsconfig.AWSCredentials{
 		AWSAccessKey:    "someawsaccesskey",
 		AWSSecretKey:    "somesecretkey",

helper/credentials/credentials.go

@@ -2,6 +2,7 @@ package credentials
 
 import (
 	"errors"
+	"fmt"
 )
 
 var (
@@ -14,25 +15,38 @@ var (
 
 // Credentials holds the information shared between saml2aws and the credentials store.
 type Credentials struct {
+	IdpName   string
 	ServerURL string
 	Username  string
 	Secret    string
 }
 
-// CredsLabel saml2aws credentials should be labeled as such in credentials stores that allow labelling.
-// That label allows to filter out non-Docker credentials too at lookup/search in macOS keychain,
-// Windows credentials manager and Linux libsecret. Default value is "saml2aws Credentials"
-var CredsLabel = "saml2aws Credentials"
+const (
+	// CredsLabel saml2aws credentials should be labeled as such in credentials stores that allow labelling.
+	// That label allows to filter out non-Docker credentials too at lookup/search in macOS keychain,
+	// Windows credentials manager and Linux libsecret. Default value is "saml2aws Credentials"
+	CredsLabel              = "saml2aws Credentials"
+	CredsKeyPrefix          = "saml2aws_credentials"
+	OktaSessionCookieSuffix = "_okta_session"
+	OneLoginTokenSuffix     = "_onelogin_token"
+)
+
+func GetKeyFromAccount(accountName string) string {
+	return fmt.Sprintf("%s_%s", CredsKeyPrefix, accountName)
+}
 
 // Helper is the interface a credentials store helper must implement.
 type Helper interface {
 	// Add appends credentials to the store.
 	Add(*Credentials) error
 	// Delete removes credentials from the store.
-	Delete(serverURL string) error
+	Delete(keyName string) error
 	// Get retrieves credentials from the store.
 	// It returns username and secret as strings.
-	Get(serverURL string) (string, string, error)
+	Get(keyName string) (string, string, error)
+	// Legacy Get retrieves previously stored credentials
+	// this function is preserved for backward compatibility
+	LegacyGet(serverURL string) (string, string, error)
 	// SupportsCredentialStorage returns true or false if there is credential storage.
 	SupportsCredentialStorage() bool
 }
@@ -49,11 +63,15 @@ func (defaultHelper) Add(*Credentials) error {
 	return nil
 }
 
-func (defaultHelper) Delete(serverURL string) error {
+func (defaultHelper) Delete(keyName string) error {
 	return nil
 }
 
-func (defaultHelper) Get(serverURL string) (string, string, error) {
+func (defaultHelper) Get(keyName string) (string, string, error) {
+	return "", "", ErrCredentialsNotFound
+}
+
+func (defaultHelper) LegacyGet(serverURL string) (string, string, error) {
 	return "", "", ErrCredentialsNotFound
 }
 

helper/credentials/saml.go

@@ -1,34 +1,57 @@
 package credentials
 
 import (
+	"errors"
 	"path"
 
 	"github.com/versent/saml2aws/v2/pkg/creds"
 )
 
 // LookupCredentials lookup an existing set of credentials and validate it.
-func LookupCredentials(loginDetails *creds.LoginDetails, provider string) error {
+func LookupCredentials(loginDetails *creds.LoginDetails) error {
+	var username, password string
+	var err, err2 error
 
-	username, password, err := CurrentHelper.Get(loginDetails.URL)
+	username, password, err = CurrentHelper.Get(GetKeyFromAccount(loginDetails.IdpName))
 	if err != nil {
-		return err
+		// the credential keyname has changed from server URL to Identity Provider (#762)
+		// Falling back to old key name to preserve backward compatibility
+		username, password, err2 = CurrentHelper.LegacyGet(loginDetails.URL)
+		if err2 != nil {
+			// return the error from the current key name, not the historical one
+			return err
+		}
 	}
 
 	loginDetails.Username = username
 	loginDetails.Password = password
 
 	// If the provider is Okta, check for existing Okta Session Cookie (sid)
-	if provider == "Okta" {
-		_, oktaSessionCookie, err := CurrentHelper.Get(loginDetails.URL + "/sessionCookie")
-		if err == nil {
-			loginDetails.OktaSessionCookie = oktaSessionCookie
+	if loginDetails.IdpProvider == "Okta" {
+		// load up the Okta token from a different secret (idp name + Okta suffix)
+		var oktaSessionCookie string
+
+		_, oktaSessionCookie, err = CurrentHelper.Get(GetKeyFromAccount(loginDetails.IdpName + OktaSessionCookieSuffix))
+		if err != nil {
+			// the credential keyname has changed from server URL to Identity Provider (#762)
+			// Falling back to old key name to preserve backward compatibility
+			_, oktaSessionCookie, _ = CurrentHelper.LegacyGet(loginDetails.URL + "/sessionCookie")
 		}
+		loginDetails.OktaSessionCookie = oktaSessionCookie
 	}
 
-	if provider == "OneLogin" {
-		id, secret, err := CurrentHelper.Get(path.Join(loginDetails.URL, "/auth/oauth2/v2/token"))
+	if loginDetails.IdpProvider == "OneLogin" {
+		var id, secret string
+
+		// load up the one login token from a different secret (idp name + one login suffix)
+		id, secret, err = CurrentHelper.Get(GetKeyFromAccount(loginDetails.IdpName + OneLoginTokenSuffix))
 		if err != nil {
-			return err
+			// the credential keyname has changed from server URL to Identity Provider (#762)
+			// Falling back to old key name to preserve backward compatibility
+			id, secret, err2 = CurrentHelper.LegacyGet(path.Join(loginDetails.URL, "/auth/oauth2/v2/token"))
+			if err2 != nil {
+				return err
+			}
 		}
 		loginDetails.ClientID = id
 		loginDetails.ClientSecret = secret
@@ -37,14 +60,17 @@ func LookupCredentials(loginDetails *creds.LoginDetails, provider string) error
 }
 
 // SaveCredentials save the user credentials.
-func SaveCredentials(url, username, password string) error {
-
+func SaveCredentials(idpName, url, username, password string) error {
 	creds := &Credentials{
+		IdpName:   idpName,
 		ServerURL: url,
 		Username:  username,
 		Secret:    password,
 	}
 
+	if idpName == "" {
+		return errors.New("idpName is empty")
+	}
 	return CurrentHelper.Add(creds)
 }