From 43fc976ad700e4b21bf7c6e8ad9e87d20a030d21 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Wed, 19 Feb 2025 11:49:59 +0100 Subject: [PATCH 01/23] reduce #3311 to touchID --- pom.xml | 2 +- src/main/java/org/cryptomator/JavaFXUtil.java | 20 ++++++++++++++ .../common/keychain/KeychainManager.java | 27 ++++++++++++------- .../MasterkeyFileLoadingStrategy.java | 8 +++--- .../VaultDetailLockedController.java | 13 ++++----- .../GeneralPreferencesController.java | 9 ++++++- .../common/keychain/KeychainManagerTest.java | 19 +++++++------ .../ui/controls/SecurePasswordFieldTest.java | 7 +++-- 8 files changed, 67 insertions(+), 38 deletions(-) create mode 100644 src/main/java/org/cryptomator/JavaFXUtil.java diff --git a/pom.xml b/pom.xml index 5aef0b15b..bed8a6597 100644 --- a/pom.xml +++ b/pom.xml @@ -36,7 +36,7 @@ 2.8.0 1.5.0 1.3.0 - 1.2.4 + 1.3.0 1.5.2 5.0.2 2.0.7 diff --git a/src/main/java/org/cryptomator/JavaFXUtil.java b/src/main/java/org/cryptomator/JavaFXUtil.java new file mode 100644 index 000000000..44372d58a --- /dev/null +++ b/src/main/java/org/cryptomator/JavaFXUtil.java @@ -0,0 +1,20 @@ +package org.cryptomator; + +import javafx.application.Platform; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + +public class JavaFXUtil { + + public static boolean startPlatform() throws InterruptedException { + CountDownLatch latch = new CountDownLatch(1); + try { + Platform.startup(latch::countDown); + } catch (IllegalStateException e) { + //already initialized + latch.countDown(); + } + return latch.await(5, TimeUnit.SECONDS); + } + +} diff --git a/src/main/java/org/cryptomator/common/keychain/KeychainManager.java b/src/main/java/org/cryptomator/common/keychain/KeychainManager.java index ac03e5ed6..b941147bc 100644 --- a/src/main/java/org/cryptomator/common/keychain/KeychainManager.java +++ b/src/main/java/org/cryptomator/common/keychain/KeychainManager.java @@ -24,7 +24,7 @@ public class KeychainManager implements KeychainAccessProvider { KeychainManager(ObjectExpression selectedKeychain) { this.keychain = selectedKeychain; this.passphraseStoredProperties = Caffeine.newBuilder() // - .weakValues() // + .softValues() // .build(this::createStoredPassphraseProperty); keychain.addListener(ignored -> passphraseStoredProperties.invalidateAll()); } @@ -43,8 +43,13 @@ public class KeychainManager implements KeychainAccessProvider { } @Override - public void storePassphrase(String key, String displayName, CharSequence passphrase, boolean ignored) throws KeychainAccessException { - getKeychainOrFail().storePassphrase(key, displayName, passphrase); + 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 + } + + @Override + public void storePassphrase(String key, String displayName, CharSequence passphrase, boolean requireOsAuthentication) throws KeychainAccessException { + getKeychainOrFail().storePassphrase(key, displayName, passphrase, requireOsAuthentication); setPassphraseStored(key, true); } @@ -101,13 +106,11 @@ public class KeychainManager implements KeychainAccessProvider { } private void setPassphraseStored(String key, boolean value) { - BooleanProperty property = passphraseStoredProperties.getIfPresent(key); - if (property != null) { - if (Platform.isFxApplicationThread()) { - property.set(value); - } else { - Platform.runLater(() -> property.set(value)); - } + BooleanProperty property = passphraseStoredProperties.get(key, _ -> new SimpleBooleanProperty(value)); + if (Platform.isFxApplicationThread()) { + property.set(value); + } else { + Platform.runLater(() -> property.set(value)); } } @@ -134,4 +137,8 @@ public class KeychainManager implements KeychainAccessProvider { } } + public ObjectExpression getKeychainImplementation() { + return this.keychain; + } + } diff --git a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/MasterkeyFileLoadingStrategy.java b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/MasterkeyFileLoadingStrategy.java index 68877430a..3f9d5ba78 100644 --- a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/MasterkeyFileLoadingStrategy.java +++ b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/MasterkeyFileLoadingStrategy.java @@ -112,12 +112,12 @@ public class MasterkeyFileLoadingStrategy implements KeyLoadingStrategy { } private void savePasswordToSystemkeychain(Passphrase passphrase) { - if (keychain.isSupported()) { - try { + try { + if (keychain.isSupported() && !keychain.getPassphraseStoredProperty(vault.getId()).getValue()) { keychain.storePassphrase(vault.getId(), vault.getDisplayName(), passphrase); - } catch (KeychainAccessException e) { - LOG.error("Failed to store passphrase in system keychain.", e); } + } catch (KeychainAccessException e) { + LOG.error("Failed to store passphrase in system keychain.", e); } } diff --git a/src/main/java/org/cryptomator/ui/mainwindow/VaultDetailLockedController.java b/src/main/java/org/cryptomator/ui/mainwindow/VaultDetailLockedController.java index 8212f598f..6a7c046f2 100644 --- a/src/main/java/org/cryptomator/ui/mainwindow/VaultDetailLockedController.java +++ b/src/main/java/org/cryptomator/ui/mainwindow/VaultDetailLockedController.java @@ -8,9 +8,9 @@ import org.cryptomator.ui.vaultoptions.SelectedVaultOptionsTab; import org.cryptomator.ui.vaultoptions.VaultOptionsComponent; import javax.inject.Inject; +import javafx.beans.binding.Bindings; import javafx.beans.property.ObjectProperty; import javafx.beans.property.ReadOnlyObjectProperty; -import javafx.beans.property.SimpleBooleanProperty; import javafx.beans.value.ObservableValue; import javafx.fxml.FXML; import javafx.stage.Stage; @@ -21,7 +21,6 @@ public class VaultDetailLockedController implements FxController { private final ReadOnlyObjectProperty vault; private final FxApplicationWindows appWindows; private final VaultOptionsComponent.Factory vaultOptionsWindow; - private final KeychainManager keychain; private final Stage mainWindow; private final ObservableValue passwordSaved; @@ -30,13 +29,11 @@ public class VaultDetailLockedController implements FxController { this.vault = vault; this.appWindows = appWindows; this.vaultOptionsWindow = vaultOptionsWindow; - this.keychain = keychain; this.mainWindow = mainWindow; - if (keychain.isSupported() && !keychain.isLocked()) { - this.passwordSaved = vault.flatMap(v -> keychain.getPassphraseStoredProperty(v.getId())).orElse(false); - } else { - this.passwordSaved = new SimpleBooleanProperty(false); - } + this.passwordSaved = Bindings.createBooleanBinding(() -> { + var v = vault.get(); + return v != null && keychain.getPassphraseStoredProperty(v.getId()).getValue(); + }, vault, keychain.getKeychainImplementation()); } @FXML diff --git a/src/main/java/org/cryptomator/ui/preferences/GeneralPreferencesController.java b/src/main/java/org/cryptomator/ui/preferences/GeneralPreferencesController.java index 800a292a9..3bbc30474 100644 --- a/src/main/java/org/cryptomator/ui/preferences/GeneralPreferencesController.java +++ b/src/main/java/org/cryptomator/ui/preferences/GeneralPreferencesController.java @@ -1,10 +1,14 @@ package org.cryptomator.ui.preferences; +import org.apache.commons.lang3.SystemUtils; import org.cryptomator.common.Environment; +import org.cryptomator.common.Passphrase; +import org.cryptomator.common.keychain.KeychainManager; 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; @@ -14,6 +18,7 @@ import org.slf4j.LoggerFactory; import javax.inject.Inject; import javafx.application.Application; +import javafx.beans.Observable; import javafx.beans.binding.Bindings; import javafx.fxml.FXML; import javafx.scene.control.CheckBox; @@ -36,6 +41,7 @@ public class GeneralPreferencesController implements FxController { private final Application application; private final Environment environment; private final List keychainAccessProviders; + private final KeychainManager keychain; private final FxApplicationWindows appWindows; public CheckBox useKeychainCheckbox; public ChoiceBox keychainBackendChoiceBox; @@ -48,11 +54,12 @@ public class GeneralPreferencesController implements FxController { public ToggleGroup nodeOrientation; @Inject - GeneralPreferencesController(@PreferencesWindow Stage window, Settings settings, Optional autoStartProvider, List keychainAccessProviders, Application application, Environment environment, FxApplicationWindows appWindows) { + GeneralPreferencesController(@PreferencesWindow Stage window, Settings settings, Optional autoStartProvider, List keychainAccessProviders, KeychainManager keychain, Application application, Environment environment, FxApplicationWindows appWindows) { this.window = window; this.settings = settings; this.autoStartProvider = autoStartProvider; this.keychainAccessProviders = keychainAccessProviders; + this.keychain = keychain; this.quickAccessServices = QuickAccessService.get().toList(); this.application = application; this.environment = environment; diff --git a/src/test/java/org/cryptomator/common/keychain/KeychainManagerTest.java b/src/test/java/org/cryptomator/common/keychain/KeychainManagerTest.java index abf803e1e..6345faaf7 100644 --- a/src/test/java/org/cryptomator/common/keychain/KeychainManagerTest.java +++ b/src/test/java/org/cryptomator/common/keychain/KeychainManagerTest.java @@ -1,6 +1,7 @@ package org.cryptomator.common.keychain; +import org.cryptomator.JavaFXUtil; import org.cryptomator.integrations.keychain.KeychainAccessException; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Assumptions; @@ -19,6 +20,12 @@ import java.util.concurrent.atomic.AtomicBoolean; public class KeychainManagerTest { + @BeforeAll + public static void startup() throws InterruptedException { + var isRunning = JavaFXUtil.startPlatform(); + Assumptions.assumeTrue(isRunning); + } + @Test public void testStoreAndLoad() throws KeychainAccessException { KeychainManager keychainManager = new KeychainManager(new SimpleObjectProperty<>(new MapKeychainAccess())); @@ -27,15 +34,7 @@ public class KeychainManagerTest { } @Nested - public static class WhenObservingProperties { - - @BeforeAll - public static void startup() throws InterruptedException { - CountDownLatch latch = new CountDownLatch(1); - Platform.startup(latch::countDown); - var javafxStarted = latch.await(5, TimeUnit.SECONDS); - Assumptions.assumeTrue(javafxStarted); - } + public class WhenObservingProperties { @Test public void testPropertyChangesWhenStoringPassword() throws KeychainAccessException, InterruptedException { @@ -43,7 +42,7 @@ public class KeychainManagerTest { ReadOnlyBooleanProperty property = keychainManager.getPassphraseStoredProperty("test"); Assertions.assertFalse(property.get()); - keychainManager.storePassphrase("test", null,"bar"); + keychainManager.storePassphrase("test", null, "bar"); AtomicBoolean result = new AtomicBoolean(false); CountDownLatch latch = new CountDownLatch(1); diff --git a/src/test/java/org/cryptomator/ui/controls/SecurePasswordFieldTest.java b/src/test/java/org/cryptomator/ui/controls/SecurePasswordFieldTest.java index bfe31816e..b6052361b 100644 --- a/src/test/java/org/cryptomator/ui/controls/SecurePasswordFieldTest.java +++ b/src/test/java/org/cryptomator/ui/controls/SecurePasswordFieldTest.java @@ -1,5 +1,6 @@ package org.cryptomator.ui.controls; +import org.cryptomator.JavaFXUtil; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Assumptions; @@ -18,10 +19,8 @@ public class SecurePasswordFieldTest { @BeforeAll public static void initJavaFx() throws InterruptedException { - CountDownLatch latch = new CountDownLatch(1); - Platform.startup(latch::countDown); - var javafxStarted = latch.await(5, TimeUnit.SECONDS); - Assumptions.assumeTrue(javafxStarted); + var isRunning = JavaFXUtil.startPlatform(); + Assumptions.assumeTrue(isRunning); } @Nested From 3333dc7f22721a19eb325e053eb62a8b71231873 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Tue, 25 Feb 2025 11:52:00 +0100 Subject: [PATCH 02/23] Fix not appearing removeCert dialog --- src/main/java/org/cryptomator/ui/dialogs/Dialogs.java | 4 ++-- src/main/resources/i18n/strings.properties | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/cryptomator/ui/dialogs/Dialogs.java b/src/main/java/org/cryptomator/ui/dialogs/Dialogs.java index 5107fe740..837bea012 100644 --- a/src/main/java/org/cryptomator/ui/dialogs/Dialogs.java +++ b/src/main/java/org/cryptomator/ui/dialogs/Dialogs.java @@ -38,7 +38,7 @@ public class Dialogs { .setMessageKey("removeVault.message") // .setDescriptionKey("removeVault.description") // .setIcon(FontAwesome5Icon.QUESTION) // - .setOkButtonKey("removeVault.confirmBtn") // + .setOkButtonKey("generic.button.remove") // .setCancelButtonKey("generic.button.cancel") // .setOkAction(stage -> { LOG.debug("Removing vault {}.", vault.getDisplayName()); @@ -54,7 +54,7 @@ public class Dialogs { .setMessageKey("removeCert.message") // .setDescriptionKey("removeCert.description") // .setIcon(FontAwesome5Icon.QUESTION) // - .setOkButtonKey("removeCert.confirmBtn") // + .setOkButtonKey("generic.button.remove") // .setCancelButtonKey("generic.button.cancel") // .setOkAction(stage -> { settings.licenseKey.set(null); diff --git a/src/main/resources/i18n/strings.properties b/src/main/resources/i18n/strings.properties index b03c202bf..a4829138a 100644 --- a/src/main/resources/i18n/strings.properties +++ b/src/main/resources/i18n/strings.properties @@ -107,7 +107,6 @@ addvaultwizard.success.unlockNow=Unlock Now removeVault.title=Remove "%s" removeVault.message=Remove vault? removeVault.description=This will only make Cryptomator forget about this vault. You can add it again. No encrypted files will be deleted from your hard drive. -removeVault.confirmBtn=Remove Vault # Change Password changepassword.title=Change Password From e3073a36134ae69bc122085bf1867c7bccbdfef2 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Tue, 25 Feb 2025 15:38:32 +0100 Subject: [PATCH 03/23] adjust wording for autoLock feature [skip ci] --- src/main/resources/i18n/strings.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/resources/i18n/strings.properties b/src/main/resources/i18n/strings.properties index a4829138a..ca3546586 100644 --- a/src/main/resources/i18n/strings.properties +++ b/src/main/resources/i18n/strings.properties @@ -283,7 +283,7 @@ preferences.title=Preferences ## General preferences.general=General preferences.general.startHidden=Hide window when starting Cryptomator -preferences.general.autoCloseVaults=Lock open vaults automatically when quitting application +preferences.general.autoCloseVaults=Lock vaults without asking when quitting application preferences.general.debugLogging=Enable debug logging preferences.general.debugDirectory=Reveal log files preferences.general.autoStart=Launch Cryptomator on system start From 0bcbf9a13a472986cbeee08461035a688e86906e Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Wed, 19 Feb 2025 15:20:57 +0100 Subject: [PATCH 04/23] 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(); } From eb0e630a441d7258d2c20377698eb8b310acbc86 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Tue, 25 Feb 2025 17:33:14 +0100 Subject: [PATCH 05/23] move migration to KeychainManager --- .../common/keychain/KeychainManager.java | 26 +++++++++++++ .../GeneralPreferencesController.java | 37 +++++-------------- 2 files changed, 35 insertions(+), 28 deletions(-) diff --git a/src/main/java/org/cryptomator/common/keychain/KeychainManager.java b/src/main/java/org/cryptomator/common/keychain/KeychainManager.java index ab44a5003..8f589933d 100644 --- a/src/main/java/org/cryptomator/common/keychain/KeychainManager.java +++ b/src/main/java/org/cryptomator/common/keychain/KeychainManager.java @@ -2,8 +2,11 @@ package org.cryptomator.common.keychain; import com.github.benmanes.caffeine.cache.Caffeine; 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; @@ -13,11 +16,14 @@ import javafx.beans.property.BooleanProperty; import javafx.beans.property.ReadOnlyBooleanProperty; import javafx.beans.property.SimpleBooleanProperty; import java.util.Arrays; +import java.util.Map; 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; @@ -169,4 +175,24 @@ public class KeychainManager implements KeychainAccessProvider { return this.keychain; } + public static void migrate(KeychainAccessProvider oldProvider, KeychainAccessProvider newProvider, Map idsAndNames) { + 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); + } + }); + } } diff --git a/src/main/java/org/cryptomator/ui/preferences/GeneralPreferencesController.java b/src/main/java/org/cryptomator/ui/preferences/GeneralPreferencesController.java index f002d20fa..361728816 100644 --- a/src/main/java/org/cryptomator/ui/preferences/GeneralPreferencesController.java +++ b/src/main/java/org/cryptomator/ui/preferences/GeneralPreferencesController.java @@ -2,13 +2,11 @@ package org.cryptomator.ui.preferences; import org.apache.commons.lang3.SystemUtils; import org.cryptomator.common.Environment; -import org.cryptomator.common.Passphrase; import org.cryptomator.common.keychain.KeychainManager; 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; @@ -31,6 +29,7 @@ import java.util.Optional; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionStage; import java.util.concurrent.ExecutorService; +import java.util.stream.Collectors; @PreferencesScoped public class GeneralPreferencesController implements FxController { @@ -89,7 +88,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); + keychainBackendChoiceBox.valueProperty().addListener(this::migrateKeychainEntries); useQuickAccessCheckbox.selectedProperty().bindBidirectional(settings.useQuickAccess); var quickAccessSettingsConverter = new ServiceToSettingsConverter<>(quickAccessServices); @@ -100,32 +99,14 @@ 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; + private void migrateKeychainEntries(Observable observable, KeychainAccessProvider oldProvider, KeychainAccessProvider newProvider) { + //currently, we only migrate on macOS (touchID vs regular keychain) + 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); + } } - - 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() { From 478c69f82c990fe5b05917418d03fedfb1ff8946 Mon Sep 17 00:00:00 2001 From: Jan-Peter Klein Date: Tue, 25 Feb 2025 18:23:17 +0100 Subject: [PATCH 06/23] simple dialog without cancel button --- .../org/cryptomator/ui/dialogs/SimpleDialog.java | 7 +++---- .../ui/dialogs/SimpleDialogController.java | 12 ++++++++++++ src/main/resources/fxml/simple_dialog.fxml | 2 +- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/cryptomator/ui/dialogs/SimpleDialog.java b/src/main/java/org/cryptomator/ui/dialogs/SimpleDialog.java index 84d9e4f75..08f77849e 100644 --- a/src/main/java/org/cryptomator/ui/dialogs/SimpleDialog.java +++ b/src/main/java/org/cryptomator/ui/dialogs/SimpleDialog.java @@ -31,8 +31,9 @@ public class SimpleDialog { FxmlLoaderFactory loaderFactory = FxmlLoaderFactory.forController( // new SimpleDialogController(resolveText(builder.messageKey, null), // resolveText(builder.descriptionKey, null), // - builder.icon, resolveText(builder.okButtonKey, null), // - resolveText(builder.cancelButtonKey, null), // + builder.icon, // + resolveText(builder.okButtonKey, null), // + builder.cancelButtonKey != null ? resolveText(builder.cancelButtonKey, null) : null, // () -> builder.okAction.accept(dialogStage), // () -> builder.cancelAction.accept(dialogStage)), // Scene::new, builder.resourceBundle); @@ -67,7 +68,6 @@ public class SimpleDialog { private String descriptionKey; private String okButtonKey; private String cancelButtonKey; - private FontAwesome5Icon icon; private Consumer okAction = Stage::close; private Consumer cancelAction = Stage::close; @@ -128,7 +128,6 @@ public class SimpleDialog { Objects.requireNonNull(messageKey, "SimpleDialog messageKey must be set."); Objects.requireNonNull(descriptionKey, "SimpleDialog descriptionKey must be set."); Objects.requireNonNull(okButtonKey, "SimpleDialog okButtonKey must be set."); - Objects.requireNonNull(cancelButtonKey, "SimpleDialog cancelButtonKey must be set."); try { return new SimpleDialog(this); diff --git a/src/main/java/org/cryptomator/ui/dialogs/SimpleDialogController.java b/src/main/java/org/cryptomator/ui/dialogs/SimpleDialogController.java index 0eee1b308..9465bf642 100644 --- a/src/main/java/org/cryptomator/ui/dialogs/SimpleDialogController.java +++ b/src/main/java/org/cryptomator/ui/dialogs/SimpleDialogController.java @@ -3,6 +3,8 @@ package org.cryptomator.ui.dialogs; import org.cryptomator.ui.common.FxController; import org.cryptomator.ui.controls.FontAwesome5Icon; +import javafx.beans.property.BooleanProperty; +import javafx.beans.property.SimpleBooleanProperty; import javafx.fxml.FXML; public class SimpleDialogController implements FxController { @@ -14,6 +16,7 @@ public class SimpleDialogController implements FxController { private final String cancelButtonText; private final Runnable okAction; private final Runnable cancelAction; + private final BooleanProperty dismissed = new SimpleBooleanProperty(); public SimpleDialogController(String message, String description, FontAwesome5Icon icon, String okButtonText, String cancelButtonText, Runnable okAction, Runnable cancelAction) { this.message = message; @@ -24,6 +27,10 @@ public class SimpleDialogController implements FxController { this.okAction = okAction; this.cancelAction = cancelAction; } + + public void initialize() { + dismissed.set(cancelButtonText == null || cancelButtonText.isEmpty()); + } public String getMessage() { return message; @@ -45,6 +52,11 @@ public class SimpleDialogController implements FxController { return cancelButtonText; } + @FXML + public boolean getDismissed() { + return dismissed.get(); + } + @FXML private void handleOk() { if (okAction != null) { diff --git a/src/main/resources/fxml/simple_dialog.fxml b/src/main/resources/fxml/simple_dialog.fxml index 32ad63abf..f076ec267 100644 --- a/src/main/resources/fxml/simple_dialog.fxml +++ b/src/main/resources/fxml/simple_dialog.fxml @@ -41,7 +41,7 @@ -