diff --git a/internal/app/migration.go b/internal/app/migration.go index 0f7b9f6b..d4b99581 100644 --- a/internal/app/migration.go +++ b/internal/app/migration.go @@ -43,7 +43,7 @@ import ( // nolint:gosec func migrateKeychainHelper(locations *locations.Locations) error { - logrus.Info("Migrating keychain helper") + logrus.Trace("Checking if keychain helper needs to be migrated") settings, err := locations.ProvideSettingsPath() if err != nil { @@ -75,7 +75,11 @@ func migrateKeychainHelper(locations *locations.Locations) error { return fmt.Errorf("failed to unmarshal old prefs file: %w", err) } - return vault.SetHelper(settings, prefs.Helper) + err = vault.SetHelper(settings, prefs.Helper) + if err == nil { + logrus.Info("Keychain helper has been migrated") + } + return err } // nolint:gosec diff --git a/pkg/keychain/keychain.go b/pkg/keychain/keychain.go index 21f276b5..0cf62506 100644 --- a/pkg/keychain/keychain.go +++ b/pkg/keychain/keychain.go @@ -22,9 +22,11 @@ import ( "errors" "fmt" "reflect" + "runtime" "sync" "time" + "github.com/ProtonMail/proton-bridge/v3/internal/constants" "github.com/docker/docker-credential-helpers/credentials" "github.com/sirupsen/logrus" ) @@ -216,10 +218,14 @@ func isUsable(helper credentials.Helper, err error) bool { return false } - creds := &credentials.Credentials{ - ServerURL: "bridge/check", - Username: "check", - Secret: "check", + creds := getTestCredentials() + + // If the test entry is already present from an interrupted previous test, we must wipe it first. + if _, _, err := helper.Get(creds.ServerURL); err == nil { + if err := helper.Delete(creds.ServerURL); err != nil { + l.WithError(err).Warn("Failed to delete existing test credentials from keychain") + return false + } } if err := retry(func() error { @@ -242,6 +248,23 @@ func isUsable(helper credentials.Helper, err error) bool { return true } +func getTestCredentials() *credentials.Credentials { + // On macOS, a handful of users experience failures of the test credentials. + if runtime.GOOS == "darwin" { + return &credentials.Credentials{ + ServerURL: hostURL(constants.KeyChainName) + "/check", + Username: "", // username is ignored on macOS, it's extracted from splitting the server URL + Secret: "check", + } + } + + return &credentials.Credentials{ + ServerURL: "bridge/check", + Username: "check", + Secret: "check", + } +} + func retry(condition func() error) error { var maxRetry = 5 for r := 0; ; r++ {