fix(BRIDGE-335): store last sucessfully used keychain helper as user preference

internal/app/migration.go

@@ -138,7 +138,7 @@ func migrateOldAccounts(locations *locations.Locations, keychains *keychain.List
 	if err != nil {
 		return fmt.Errorf("failed to get helper: %w", err)
 	}
-	keychain, err := keychain.NewKeychain(helper, "bridge", keychains.GetHelpers(), keychains.GetDefaultHelper())
+	keychain, _, err := keychain.NewKeychain(helper, "bridge", keychains.GetHelpers(), keychains.GetDefaultHelper())
 	if err != nil {
 		return fmt.Errorf("failed to create keychain: %w", err)
 	}

internal/app/migration_test.go

@@ -134,7 +134,7 @@ func TestKeychainMigration(t *testing.T) {
 func TestUserMigration(t *testing.T) {
 	kcl := keychain.NewTestKeychainsList()
 
-	kc, err := keychain.NewKeychain("mock", "bridge", kcl.GetHelpers(), kcl.GetDefaultHelper())
+	kc, _, err := keychain.NewKeychain("mock", "bridge", kcl.GetHelpers(), kcl.GetDefaultHelper())
 	require.NoError(t, err)
 
 	require.NoError(t, kc.Put("brokenID", "broken"))

internal/app/vault.go

@@ -71,9 +71,10 @@ func newVault(reporter *sentry.Reporter, locations *locations.Locations, keychai
 	var (
 		vaultKey       []byte
 		insecure       bool
+		lastUsedHelper string
 	)
 
-	if key, err := loadVaultKey(vaultDir, keychains); err != nil {
+	if key, helper, err := loadVaultKey(vaultDir, keychains); err != nil {
 		if reporter != nil {
 			if rerr := reporter.ReportMessageWithContext("Could not load/create vault key", map[string]any{
 				"keychainDefaultHelper":       keychains.GetDefaultHelper(),
@@ -91,6 +92,7 @@ func newVault(reporter *sentry.Reporter, locations *locations.Locations, keychai
 		vaultDir = path.Join(vaultDir, "insecure")
 	} else {
 		vaultKey = key
+		lastUsedHelper = helper
 		logHashedVaultKey(vaultKey) // Log a hash of the vault key.
 	}
 
@@ -99,36 +101,43 @@ func newVault(reporter *sentry.Reporter, locations *locations.Locations, keychai
 		return nil, false, nil, fmt.Errorf("could not provide gluon path: %w", err)
 	}
 
-	vault, corrupt, err := vault.New(vaultDir, gluonCacheDir, vaultKey, panicHandler)
+	userVault, corrupt, err := vault.New(vaultDir, gluonCacheDir, vaultKey, panicHandler)
 	if err != nil {
 		return nil, false, corrupt, fmt.Errorf("could not create vault: %w", err)
 	}
 
-	return vault, insecure, corrupt, nil
+	// Remember the last successfully used keychain and store that as the user preference.
+	if err := vault.SetHelper(vaultDir, lastUsedHelper); err != nil {
+		logrus.WithError(err).Error("Could not store last used keychain helper")
+	}
+
+	return userVault, insecure, corrupt, nil
 }
 
-func loadVaultKey(vaultDir string, keychains *keychain.List) ([]byte, error) {
+// loadVaultKey - loads the key used to encrypt the vault alongside the keychain helper used to access it.
-	helper, err := vault.GetHelper(vaultDir)
+func loadVaultKey(vaultDir string, keychains *keychain.List) (key []byte, keychainHelper string, err error) {
+	keychainHelper, err = vault.GetHelper(vaultDir)
 	if err != nil {
-		return nil, fmt.Errorf("could not get keychain helper: %w", err)
+		return nil, keychainHelper, fmt.Errorf("could not get keychain helper: %w", err)
 	}
 
-	kc, err := keychain.NewKeychain(helper, constants.KeyChainName, keychains.GetHelpers(), keychains.GetDefaultHelper())
+	kc, keychainHelper, err := keychain.NewKeychain(keychainHelper, constants.KeyChainName, keychains.GetHelpers(), keychains.GetDefaultHelper())
 	if err != nil {
-		return nil, fmt.Errorf("could not create keychain: %w", err)
+		return nil, keychainHelper, fmt.Errorf("could not create keychain: %w", err)
 	}
 
-	key, err := vault.GetVaultKey(kc)
+	key, err = vault.GetVaultKey(kc)
 	if err != nil {
 		if keychain.IsErrKeychainNoItem(err) {
 			logrus.WithError(err).Warn("no vault key found, generating new")
-			return vault.NewVaultKey(kc)
+			key, err := vault.NewVaultKey(kc)
+			return key, keychainHelper, err
 		}
 
-		return nil, fmt.Errorf("could not check for vault key: %w", err)
+		return nil, keychainHelper, fmt.Errorf("could not check for vault key: %w", err)
 	}
 
-	return key, nil
+	return key, keychainHelper, nil
 }
 
 // logHashedVaultKey - computes a sha256 hash and encodes it to base 64. The resulting string is logged.

internal/vault/helper.go

@@ -63,6 +63,10 @@ func GetHelper(vaultDir string) (string, error) {
 }
 
 func SetHelper(vaultDir, helper string) error {
+	if helper == "" {
+		return nil
+	}
+
 	settings, err := LoadKeychainSettings(vaultDir)
 	if err != nil {
 		return err

pkg/keychain/keychain.go

@@ -82,11 +82,11 @@ func (kcl *List) GetDefaultHelper() string {
 	return kcl.defaultHelper
 }
 
-// NewKeychain creates a new native keychain.
+// NewKeychain creates a new native keychain. It also returns the keychain helper used to access the keychain.
-func NewKeychain(preferred, keychainName string, helpers Helpers, defaultHelper string) (*Keychain, error) {
+func NewKeychain(preferred, keychainName string, helpers Helpers, defaultHelper string) (kc *Keychain, usedKeychainHelper string, err error) {
 	// There must be at least one keychain helper available.
 	if len(helpers) < 1 {
-		return nil, ErrNoKeychain
+		return nil, "", ErrNoKeychain
 	}
 
 	// If the preferred keychain is unsupported, fallback to the default one.
@@ -97,16 +97,16 @@ func NewKeychain(preferred, keychainName string, helpers Helpers, defaultHelper
 	// Load the user's preferred keychain helper.
 	helperConstructor, ok := helpers[preferred]
 	if !ok {
-		return nil, ErrNoKeychain
+		return nil, "", ErrNoKeychain
 	}
 
 	// Construct the keychain helper.
 	helper, err := helperConstructor(hostURL(keychainName))
 	if err != nil {
-		return nil, err
+		return nil, preferred, err
 	}
 
-	return newKeychain(helper, hostURL(keychainName)), nil
+	return newKeychain(helper, hostURL(keychainName)), preferred, nil
 }
 
 func newKeychain(helper credentials.Helper, url string) *Keychain {

pkg/keychain/keychain_test.go

@@ -120,7 +120,7 @@ func TestIsErrKeychainNoItem(t *testing.T) {
 	helpers := NewList().GetHelpers()
 
 	for helperName := range helpers {
-		kc, err := NewKeychain(helperName, "bridge-test", helpers, helperName)
+		kc, _, err := NewKeychain(helperName, "bridge-test", helpers, helperName)
 		r.NoError(err)
 
 		_, _, err = kc.Get("non-existing")