diff --git a/src/main/java/org/cryptomator/common/keychain/KeychainManager.java b/src/main/java/org/cryptomator/common/keychain/KeychainManager.java index 8f589933d..04a46e742 100644 --- a/src/main/java/org/cryptomator/common/keychain/KeychainManager.java +++ b/src/main/java/org/cryptomator/common/keychain/KeychainManager.java @@ -5,8 +5,6 @@ import com.github.benmanes.caffeine.cache.LoadingCache; import org.cryptomator.common.Passphrase; import org.cryptomator.integrations.keychain.KeychainAccessException; import org.cryptomator.integrations.keychain.KeychainAccessProvider; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import javax.inject.Inject; import javax.inject.Singleton; @@ -22,8 +20,6 @@ import java.util.concurrent.locks.ReentrantReadWriteLock; @Singleton public class KeychainManager implements KeychainAccessProvider { - private static final Logger LOG = LoggerFactory.getLogger(KeychainManager.class); - private final ObjectExpression keychain; private final LoadingCache passphraseStoredProperties; private final ReentrantReadWriteLock lock; @@ -175,24 +171,18 @@ public class KeychainManager implements KeychainAccessProvider { return this.keychain; } - public static void migrate(KeychainAccessProvider oldProvider, KeychainAccessProvider newProvider, Map idsAndNames) { + public static void migrate(KeychainAccessProvider oldProvider, KeychainAccessProvider newProvider, Map idsAndNames) throws KeychainAccessException { if (oldProvider instanceof KeychainManager || newProvider instanceof KeychainManager) { throw new IllegalArgumentException("KeychainManger must not be the source or target of migration"); } - - LOG.info("Migrating keychain entries {} from {} to {}", idsAndNames.keySet(), oldProvider.displayName(), newProvider.displayName()); - idsAndNames.forEach((id, name) -> { - try { - var passphrase = oldProvider.loadPassphrase(id); - if (passphrase != null) { - var wrapper = new Passphrase(passphrase); - newProvider.storePassphrase(id, name, wrapper); - oldProvider.deletePassphrase(id); - wrapper.destroy(); - } - } catch (KeychainAccessException e) { - LOG.error("Failed to migrate keychain entry for vault {}.", id, e); + for (var entry : idsAndNames.entrySet()) { + var passphrase = oldProvider.loadPassphrase(entry.getKey()); + if (passphrase != null) { + var wrapper = new Passphrase(passphrase); + oldProvider.deletePassphrase(entry.getKey()); //we cannot apply "first-write-then-delete" pattern here, since we can potentially write to the same passphrase store (e.g., touchID and regular keychain) + newProvider.storePassphrase(entry.getKey(), entry.getValue(), wrapper); + wrapper.destroy(); } - }); + } } } diff --git a/src/main/java/org/cryptomator/ui/preferences/GeneralPreferencesController.java b/src/main/java/org/cryptomator/ui/preferences/GeneralPreferencesController.java index d42dabc80..ce286c9e7 100644 --- a/src/main/java/org/cryptomator/ui/preferences/GeneralPreferencesController.java +++ b/src/main/java/org/cryptomator/ui/preferences/GeneralPreferencesController.java @@ -7,6 +7,7 @@ import org.cryptomator.common.settings.Settings; import org.cryptomator.integrations.autostart.AutoStartProvider; import org.cryptomator.integrations.autostart.ToggleAutoStartFailedException; import org.cryptomator.integrations.common.NamedServiceProvider; +import org.cryptomator.integrations.keychain.KeychainAccessException; import org.cryptomator.integrations.keychain.KeychainAccessProvider; import org.cryptomator.integrations.quickaccess.QuickAccessService; import org.cryptomator.ui.common.FxController; @@ -104,11 +105,14 @@ public class GeneralPreferencesController implements FxController { if (SystemUtils.IS_OS_MAC) { var idsAndNames = settings.directories.stream().collect(Collectors.toMap(vs -> vs.id, vs -> vs.displayName.getValue())); if (!idsAndNames.isEmpty()) { - keychainMigrations = keychainMigrations.thenRunAsync(() -> KeychainManager.migrate(oldProvider, newProvider, idsAndNames), backgroundExecutor) // - .exceptionally(e -> { - LOG.warn("Failed to migrate entries", e); - return null; - }); + LOG.debug("Migrating keychain entries {} from {} to {}", idsAndNames.keySet(), oldProvider.displayName(), newProvider.displayName()); + keychainMigrations = keychainMigrations.thenRunAsync(() -> { + try { + KeychainManager.migrate(oldProvider, newProvider, idsAndNames); + } catch (KeychainAccessException e) { + LOG.warn("Failed to migrate all entries from {} to {}", oldProvider.displayName(), newProvider.displayName(), e); + } + }, backgroundExecutor); } } }