Use Idp Account Name as key for credentials store

At the moment the credentials are stored using the idp server url as a primary key for the keychain store. This is very useful if you need to authenticate through different providers or environment (and thus different endpoints), however this prevents the use case where users may want to use different idp accounts to authenticate using different users. A typical example of this is that some users often have a normal user and an admin user with different properties, roles and limitations. This change is refactoring how credentials are stored across the code base to use the idp Name instead of the server URL.
Versent · May 3, 2023 · 76bbe0e · 76bbe0e
1 parent 68d09f2
commit 76bbe0e
Show file tree

Hide file tree

Showing 13 changed files with 128 additions and 74 deletions.
diff --git a/cmd/saml2aws/commands/configure.go b/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")
 		}
 	}

diff --git a/cmd/saml2aws/commands/list_roles.go b/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")
 		}

diff --git a/cmd/saml2aws/commands/login.go b/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.")

diff --git a/cmd/saml2aws/commands/login_test.go b/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",

diff --git a/helper/credentials/credentials.go b/helper/credentials/credentials.go
@@ -2,6 +2,7 @@ package credentials
 
 import (
 	"errors"
+	"fmt"
 )
 
 var (
@@ -14,25 +15,35 @@ 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(idpName string) error
 	// Get retrieves credentials from the store.
 	// It returns username and secret as strings.
-	Get(serverURL string) (string, string, error)
+	Get(idpName string) (string, string, error)
 	// SupportsCredentialStorage returns true or false if there is credential storage.
 	SupportsCredentialStorage() bool
 }

diff --git a/helper/credentials/saml.go b/helper/credentials/saml.go
@@ -1,15 +1,13 @@
 package credentials
 
 import (
-	"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 {
 
-	username, password, err := CurrentHelper.Get(loginDetails.URL)
+	username, password, err := CurrentHelper.Get(loginDetails.IdpName)
 	if err != nil {
 		return err
 	}
@@ -18,15 +16,17 @@ func LookupCredentials(loginDetails *creds.LoginDetails, provider string) error
 	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 loginDetails.IdpProvider == "Okta" {
+		// load up the Okta token from a different secret (idp name + Okta suffix)
+		_, oktaSessionCookie, err := CurrentHelper.Get(loginDetails.IdpName + OktaSessionCookieSuffix)
 		if err == nil {
 			loginDetails.OktaSessionCookie = oktaSessionCookie
 		}
 	}
 
-	if provider == "OneLogin" {
-		id, secret, err := CurrentHelper.Get(path.Join(loginDetails.URL, "/auth/oauth2/v2/token"))
+	if loginDetails.IdpProvider == "OneLogin" {
+		// load up the one login token from a different secret (idp name + one login suffix)
+		id, secret, err := CurrentHelper.Get(loginDetails.IdpName + OneLoginTokenSuffix)
 		if err != nil {
 			return err
 		}
@@ -37,9 +37,10 @@ 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,

diff --git a/helper/linuxkeyring/linuxkeyring_linux.go b/helper/linuxkeyring/linuxkeyring_linux.go
@@ -52,19 +52,19 @@ func (kr *KeyringHelper) Add(creds *credentials.Credentials) error {
 	}
 
 	return kr.keyring.Set(keyring.Item{
-		Key:                         creds.ServerURL,
+		Key:                         credentials.GetKeyFromAccount(creds.IdpName),
 		Label:                       credentials.CredsLabel,
 		Data:                        encoded,
 		KeychainNotTrustApplication: false,
 	})
 }
 
-func (kr *KeyringHelper) Delete(serverURL string) error {
-	return kr.keyring.Remove(serverURL)
+func (kr *KeyringHelper) Delete(idpName string) error {
+	return kr.keyring.Remove(credentials.GetKeyFromAccount(idpName))
 }
 
-func (kr *KeyringHelper) Get(serverURL string) (string, string, error) {
-	item, err := kr.keyring.Get(serverURL)
+func (kr *KeyringHelper) Get(idpName string) (string, string, error) {
+	item, err := kr.keyring.Get(credentials.GetKeyFromAccount(idpName))
 	if err != nil {
 		logger.WithField("err", err).Error("keychain Get returned error")
 		return "", "", credentials.ErrCredentialsNotFound