From 57d563d488b075094a111e575dcea4b6e1a5c2ba Mon Sep 17 00:00:00 2001
From: Jakub <jakub@cuth.sk>
Date: Mon, 12 Dec 2022 17:06:43 +0100
Subject: [PATCH] GODT-2173: fix: do not migrate keychain once migrated

---
 internal/app/migration.go      | 15 +++++++----
 internal/app/migration_test.go | 48 ++++++++++++++++++++++++++++++++++
 internal/vault/helper.go       |  8 ++++--
 3 files changed, 64 insertions(+), 7 deletions(-)

diff --git a/internal/app/migration.go b/internal/app/migration.go
index 127ca35e..2a91562e 100644
--- a/internal/app/migration.go
+++ b/internal/app/migration.go
@@ -43,6 +43,16 @@ import (
 func migrateKeychainHelper(locations *locations.Locations) error {
 	logrus.Info("Migrating keychain helper")
 
+	settings, err := locations.ProvideSettingsPath()
+	if err != nil {
+		return fmt.Errorf("failed to get settings path: %w", err)
+	}
+
+	if keychainName, _ := vault.GetHelper(settings); keychainName != "" {
+		// If uncorupted keychain file is already there do not migrate again.
+		return nil
+	}
+
 	configDir, err := os.UserConfigDir()
 	if err != nil {
 		return fmt.Errorf("failed to get user config dir: %w", err)
@@ -63,11 +73,6 @@ func migrateKeychainHelper(locations *locations.Locations) error {
 		return fmt.Errorf("failed to unmarshal old prefs file: %w", err)
 	}
 
-	settings, err := locations.ProvideSettingsPath()
-	if err != nil {
-		return fmt.Errorf("failed to get settings path: %w", err)
-	}
-
 	return vault.SetHelper(settings, prefs.Helper)
 }
 
diff --git a/internal/app/migration_test.go b/internal/app/migration_test.go
index ce811d34..3b0c432d 100644
--- a/internal/app/migration_test.go
+++ b/internal/app/migration_test.go
@@ -22,9 +22,12 @@ import (
 	"net/url"
 	"os"
 	"path/filepath"
+	"runtime"
 	"testing"
 
+	"github.com/ProtonMail/proton-bridge/v3/internal/bridge"
 	"github.com/ProtonMail/proton-bridge/v3/internal/cookies"
+	"github.com/ProtonMail/proton-bridge/v3/internal/locations"
 	"github.com/ProtonMail/proton-bridge/v3/internal/updater"
 	"github.com/ProtonMail/proton-bridge/v3/internal/vault"
 	"github.com/stretchr/testify/require"
@@ -79,3 +82,48 @@ func TestMigratePrefsToVault(t *testing.T) {
 	// There should be a cookie for the API.
 	require.NotEmpty(t, cookies.Cookies(url))
 }
+
+func TestKeychainMigration(t *testing.T) {
+	// migration needed only for linux
+	if runtime.GOOS != "linux" {
+		return
+	}
+
+	tmpDir := t.TempDir()
+	os.Setenv("XDG_CONFIG_HOME", tmpDir)
+
+	oldCacheDir := filepath.Join(tmpDir, "protonmail", "bridge")
+	require.NoError(t, os.MkdirAll(oldCacheDir, 0o700))
+
+	oldPrefs, err := os.ReadFile(filepath.Join("testdata", "prefs.json"))
+	require.NoError(t, err)
+
+	require.NoError(t, os.WriteFile(
+		filepath.Join(oldCacheDir, "prefs.json"),
+		oldPrefs, 0o600,
+	))
+
+	locations := locations.New(bridge.NewTestLocationsProvider(tmpDir), "config-name")
+
+	settingsFolder, err := locations.ProvideSettingsPath()
+	require.NoError(t, err)
+
+	keychainName, err := vault.GetHelper(settingsFolder)
+	require.NoError(t, err)
+	require.Equal(t, "", keychainName)
+
+	require.NoError(t, migrateKeychainHelper(locations))
+
+	keychainName, err = vault.GetHelper(settingsFolder)
+	require.NoError(t, err)
+	require.Equal(t, "secret-service", keychainName)
+
+	require.NoError(t, vault.SetHelper(settingsFolder, "different"))
+
+	// Calling migration again will not overwrite
+	require.NoError(t, migrateKeychainHelper(locations))
+	keychainName, err = vault.GetHelper(settingsFolder)
+	require.NoError(t, err)
+	require.Equal(t, "different", keychainName)
+
+}
diff --git a/internal/vault/helper.go b/internal/vault/helper.go
index a39ab01c..29302b88 100644
--- a/internal/vault/helper.go
+++ b/internal/vault/helper.go
@@ -29,8 +29,12 @@ type Keychain struct {
 	Helper string
 }
 
+func getKeychainPrefPath(vaultDir string) string {
+	return filepath.Clean(filepath.Join(vaultDir, "keychain.json"))
+}
+
 func GetHelper(vaultDir string) (string, error) {
-	filePath := filepath.Clean(filepath.Join(vaultDir, "keychain.json"))
+	filePath := getKeychainPrefPath(vaultDir)
 
 	if _, err := os.Stat(filePath); errors.Is(err, fs.ErrNotExist) {
 		return "", nil
@@ -56,5 +60,5 @@ func SetHelper(vaultDir, helper string) error {
 		return err
 	}
 
-	return os.WriteFile(filepath.Clean(filepath.Join(vaultDir, "keychain.json")), b, 0o600)
+	return os.WriteFile(getKeychainPrefPath(vaultDir), b, 0o600)
 }