From 0bcbf9a13a472986cbeee08461035a688e86906e Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Wed, 19 Feb 2025 15:20:57 +0100 Subject: [PATCH] add migration code and synchronize keychain edits --- .../common/keychain/KeychainManager.java | 40 ++++++++++++++++--- .../GeneralPreferencesController.java | 40 ++++++++++++++++++- 2 files changed, 73 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/cryptomator/common/keychain/KeychainManager.java b/src/main/java/org/cryptomator/common/keychain/KeychainManager.java index b941147bc..ab44a5003 100644 --- a/src/main/java/org/cryptomator/common/keychain/KeychainManager.java +++ b/src/main/java/org/cryptomator/common/keychain/KeychainManager.java @@ -13,12 +13,14 @@ import javafx.beans.property.BooleanProperty; import javafx.beans.property.ReadOnlyBooleanProperty; import javafx.beans.property.SimpleBooleanProperty; import java.util.Arrays; +import java.util.concurrent.locks.ReentrantReadWriteLock; @Singleton public class KeychainManager implements KeychainAccessProvider { private final ObjectExpression keychain; private final LoadingCache passphraseStoredProperties; + private final ReentrantReadWriteLock lock; @Inject KeychainManager(ObjectExpression selectedKeychain) { @@ -27,6 +29,7 @@ public class KeychainManager implements KeychainAccessProvider { .softValues() // .build(this::createStoredPassphraseProperty); keychain.addListener(ignored -> passphraseStoredProperties.invalidateAll()); + this.lock = new ReentrantReadWriteLock(false); } private KeychainAccessProvider getKeychainOrFail() throws KeychainAccessException { @@ -44,32 +47,57 @@ public class KeychainManager implements KeychainAccessProvider { @Override public void storePassphrase(String key, String displayName, CharSequence passphrase) throws KeychainAccessException { - storePassphrase(key, displayName, passphrase, true); //TODO: currently only TouchID is using this parameter, so this is okayish + storePassphrase(key, displayName, passphrase, true); } + //TODO: remove ignored parameter once the API is fixed @Override - public void storePassphrase(String key, String displayName, CharSequence passphrase, boolean requireOsAuthentication) throws KeychainAccessException { - getKeychainOrFail().storePassphrase(key, displayName, passphrase, requireOsAuthentication); + public void storePassphrase(String key, String displayName, CharSequence passphrase, boolean ignored) throws KeychainAccessException { + try { + lock.writeLock().lock(); + var kc = getKeychainOrFail(); + //this is the only keychain actually using the parameter + var usesOSAuth = (kc.getClass().getName().equals("org.cryptomator.macos.keychain.TouchIdKeychainAccess")); + kc.storePassphrase(key, displayName, passphrase, usesOSAuth); + } finally { + lock.writeLock().unlock(); + } setPassphraseStored(key, true); } @Override public char[] loadPassphrase(String key) throws KeychainAccessException { - char[] passphrase = getKeychainOrFail().loadPassphrase(key); + char[] passphrase = null; + try { + lock.readLock().lock(); + passphrase = getKeychainOrFail().loadPassphrase(key); + } finally { + lock.readLock().unlock(); + } setPassphraseStored(key, passphrase != null); return passphrase; } @Override public void deletePassphrase(String key) throws KeychainAccessException { - getKeychainOrFail().deletePassphrase(key); + try { + lock.writeLock().lock(); + getKeychainOrFail().deletePassphrase(key); + } finally { + lock.writeLock().unlock(); + } setPassphraseStored(key, false); } @Override public void changePassphrase(String key, String displayName, CharSequence passphrase) throws KeychainAccessException { if (isPassphraseStored(key)) { - getKeychainOrFail().changePassphrase(key, displayName, passphrase); + try { + lock.writeLock().lock(); + getKeychainOrFail().changePassphrase(key, displayName, passphrase); + } finally { + lock.writeLock().unlock(); + } setPassphraseStored(key, true); } } diff --git a/src/main/java/org/cryptomator/ui/preferences/GeneralPreferencesController.java b/src/main/java/org/cryptomator/ui/preferences/GeneralPreferencesController.java index 3bbc30474..f002d20fa 100644 --- a/src/main/java/org/cryptomator/ui/preferences/GeneralPreferencesController.java +++ b/src/main/java/org/cryptomator/ui/preferences/GeneralPreferencesController.java @@ -28,6 +28,9 @@ import javafx.stage.Stage; import javafx.util.StringConverter; import java.util.List; import java.util.Optional; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionStage; +import java.util.concurrent.ExecutorService; @PreferencesScoped public class GeneralPreferencesController implements FxController { @@ -42,6 +45,7 @@ public class GeneralPreferencesController implements FxController { private final Environment environment; private final List keychainAccessProviders; private final KeychainManager keychain; + private final ExecutorService backgroundExecutor; private final FxApplicationWindows appWindows; public CheckBox useKeychainCheckbox; public ChoiceBox keychainBackendChoiceBox; @@ -53,13 +57,18 @@ public class GeneralPreferencesController implements FxController { public CheckBox autoStartCheckbox; public ToggleGroup nodeOrientation; + private CompletionStage keychainMigrations = CompletableFuture.completedFuture(null); + @Inject - GeneralPreferencesController(@PreferencesWindow Stage window, Settings settings, Optional autoStartProvider, List keychainAccessProviders, KeychainManager keychain, Application application, Environment environment, FxApplicationWindows appWindows) { + GeneralPreferencesController(@PreferencesWindow Stage window, Settings settings, Optional autoStartProvider, // + List keychainAccessProviders, KeychainManager keychain, Application application, // + Environment environment, FxApplicationWindows appWindows, ExecutorService backgroundExecutor) { this.window = window; this.settings = settings; this.autoStartProvider = autoStartProvider; this.keychainAccessProviders = keychainAccessProviders; this.keychain = keychain; + this.backgroundExecutor = backgroundExecutor; this.quickAccessServices = QuickAccessService.get().toList(); this.application = application; this.environment = environment; @@ -80,6 +89,7 @@ public class GeneralPreferencesController implements FxController { Bindings.bindBidirectional(settings.keychainProvider, keychainBackendChoiceBox.valueProperty(), keychainSettingsConverter); useKeychainCheckbox.selectedProperty().bindBidirectional(settings.useKeychain); keychainBackendChoiceBox.disableProperty().bind(useKeychainCheckbox.selectedProperty().not()); + keychainBackendChoiceBox.valueProperty().addListener(this::migrateMacKeychainEntries); useQuickAccessCheckbox.selectedProperty().bindBidirectional(settings.useQuickAccess); var quickAccessSettingsConverter = new ServiceToSettingsConverter<>(quickAccessServices); @@ -90,6 +100,34 @@ public class GeneralPreferencesController implements FxController { quickAccessServiceChoiceBox.disableProperty().bind(useQuickAccessCheckbox.selectedProperty().not()); } + public void migrateMacKeychainEntries(Observable observable, KeychainAccessProvider oldProvider, KeychainAccessProvider newProvider) { + if (!SystemUtils.IS_OS_MAC) { + return; + } + + record VIdAndName(String id, String name) {} + var vaults = settings.directories.stream().map(vault -> new VIdAndName(vault.id, vault.displayName.getValue())).toList(); + + keychainMigrations = keychainMigrations.thenRunAsync(() -> { + if (!vaults.isEmpty()) { + LOG.info("Migrating keychain entries for vaults: {}", vaults.stream().map(VIdAndName::id)); + } + for (var v : vaults) { //TODO: migrate to pattern matching once supported + try { + var passphrase = oldProvider.loadPassphrase(v.id); + if (passphrase != null) { + var wrapper = new Passphrase(passphrase); + newProvider.storePassphrase(v.id, v.name, wrapper); + oldProvider.deletePassphrase(v.id); + wrapper.destroy(); + } + } catch (KeychainAccessException e) { + LOG.error("Failed to migrate keychain entry for vault {}.", v.id, e); + } + } + }, backgroundExecutor); + } + public boolean isAutoStartSupported() { return autoStartProvider.isPresent(); }