From 43fc976ad700e4b21bf7c6e8ad9e87d20a030d21 Mon Sep 17 00:00:00 2001 From: Armin Schrenk Date: Wed, 19 Feb 2025 11:49:59 +0100 Subject: [PATCH] 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