From 2f0de3520af260ce433871a8e43f4a710deab7e0 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Wed, 13 May 2020 19:59:32 +0200 Subject: [PATCH] Refactored KeychainAccess in preparation of #1183, #1182 --- main/keychain/pom.xml | 15 ++- .../cryptomator/keychain/KeychainAccess.java | 38 ------ .../keychain/KeychainAccessStrategy.java | 31 ++++- .../cryptomator/keychain/KeychainManager.java | 117 ++++++++++++++++++ .../cryptomator/keychain/KeychainModule.java | 32 +++-- .../LinuxSecretServiceKeychainAccess.java | 2 + .../keychain/MacSystemKeychainAccess.java | 2 + .../WindowsProtectedKeychainAccess.java | 2 + .../keychain/KeychainManagerTest.java | 55 ++++++++ .../keychain/KeychainModuleTest.java | 24 ---- .../keychain/TestKeychainComponent.java | 19 --- .../keychain/TestKeychainModule.java | 17 --- .../ChangePasswordController.java | 6 +- .../cryptomator/ui/common/VaultService.java | 9 +- .../ForgetPasswordController.java | 12 +- .../ui/migration/MigrationRunController.java | 14 +-- .../ui/unlock/UnlockController.java | 10 +- .../cryptomator/ui/unlock/UnlockModule.java | 12 +- .../cryptomator/ui/unlock/UnlockWorkflow.java | 6 +- 19 files changed, 273 insertions(+), 150 deletions(-) delete mode 100644 main/keychain/src/main/java/org/cryptomator/keychain/KeychainAccess.java create mode 100644 main/keychain/src/main/java/org/cryptomator/keychain/KeychainManager.java create mode 100644 main/keychain/src/test/java/org/cryptomator/keychain/KeychainManagerTest.java delete mode 100644 main/keychain/src/test/java/org/cryptomator/keychain/KeychainModuleTest.java delete mode 100644 main/keychain/src/test/java/org/cryptomator/keychain/TestKeychainComponent.java delete mode 100644 main/keychain/src/test/java/org/cryptomator/keychain/TestKeychainModule.java diff --git a/main/keychain/pom.xml b/main/keychain/pom.xml index 1d09fcd67..fd2b26c3a 100644 --- a/main/keychain/pom.xml +++ b/main/keychain/pom.xml @@ -15,16 +15,27 @@ commons + + + org.openjfx + javafx-base + + + org.openjfx + javafx-graphics + + + org.apache.commons commons-lang3 + + com.google.code.gson gson - - com.google.guava guava diff --git a/main/keychain/src/main/java/org/cryptomator/keychain/KeychainAccess.java b/main/keychain/src/main/java/org/cryptomator/keychain/KeychainAccess.java deleted file mode 100644 index 563794dc0..000000000 --- a/main/keychain/src/main/java/org/cryptomator/keychain/KeychainAccess.java +++ /dev/null @@ -1,38 +0,0 @@ -/******************************************************************************* - * Copyright (c) 2017 Skymatic UG (haftungsbeschränkt). - * All rights reserved. This program and the accompanying materials - * are made available under the terms of the accompanying LICENSE file. - *******************************************************************************/ -package org.cryptomator.keychain; - -public interface KeychainAccess { - - /** - * Associates a passphrase with a given key. - * - * @param key Key used to retrieve the passphrase via {@link #loadPassphrase(String)}. - * @param passphrase The secret to store in this keychain. - */ - void storePassphrase(String key, CharSequence passphrase) throws KeychainAccessException; - - /** - * @param key Unique key previously used while {@link #storePassphrase(String, CharSequence) storing a passphrase}. - * @return The stored passphrase for the given key or null if no value for the given key could be found. - */ - char[] loadPassphrase(String key) throws KeychainAccessException; - - /** - * Deletes a passphrase with a given key. - * - * @param key Unique key previously used while {@link #storePassphrase(String, CharSequence) storing a passphrase}. - */ - void deletePassphrase(String key) throws KeychainAccessException; - - /** - * Updates a passphrase with a given key. - * - * @param key Unique key previously used while {@link #storePassphrase(String, CharSequence) storing a passphrase}. - * @param passphrase The secret to be updated in this keychain. - */ - void changePassphrase(String key, CharSequence passphrase) throws KeychainAccessException; -} diff --git a/main/keychain/src/main/java/org/cryptomator/keychain/KeychainAccessStrategy.java b/main/keychain/src/main/java/org/cryptomator/keychain/KeychainAccessStrategy.java index a248d08f5..8976e6c55 100644 --- a/main/keychain/src/main/java/org/cryptomator/keychain/KeychainAccessStrategy.java +++ b/main/keychain/src/main/java/org/cryptomator/keychain/KeychainAccessStrategy.java @@ -5,7 +5,36 @@ *******************************************************************************/ package org.cryptomator.keychain; -interface KeychainAccessStrategy extends KeychainAccess { +interface KeychainAccessStrategy { + + /** + * Associates a passphrase with a given key. + * + * @param key Key used to retrieve the passphrase via {@link #loadPassphrase(String)}. + * @param passphrase The secret to store in this keychain. + */ + void storePassphrase(String key, CharSequence passphrase) throws KeychainAccessException; + + /** + * @param key Unique key previously used while {@link #storePassphrase(String, CharSequence) storing a passphrase}. + * @return The stored passphrase for the given key or null if no value for the given key could be found. + */ + char[] loadPassphrase(String key) throws KeychainAccessException; + + /** + * Deletes a passphrase with a given key. + * + * @param key Unique key previously used while {@link #storePassphrase(String, CharSequence) storing a passphrase}. + */ + void deletePassphrase(String key) throws KeychainAccessException; + + /** + * Updates a passphrase with a given key. + * + * @param key Unique key previously used while {@link #storePassphrase(String, CharSequence) storing a passphrase}. + * @param passphrase The secret to be updated in this keychain. + */ + void changePassphrase(String key, CharSequence passphrase) throws KeychainAccessException; /** * @return true if this KeychainAccessStrategy works on the current machine. diff --git a/main/keychain/src/main/java/org/cryptomator/keychain/KeychainManager.java b/main/keychain/src/main/java/org/cryptomator/keychain/KeychainManager.java new file mode 100644 index 000000000..300b5b035 --- /dev/null +++ b/main/keychain/src/main/java/org/cryptomator/keychain/KeychainManager.java @@ -0,0 +1,117 @@ +package org.cryptomator.keychain; + +import com.google.common.cache.CacheBuilder; +import com.google.common.cache.CacheLoader; +import com.google.common.cache.LoadingCache; +import javafx.application.Platform; +import javafx.beans.property.BooleanProperty; +import javafx.beans.property.ReadOnlyBooleanProperty; +import javafx.beans.property.SimpleBooleanProperty; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.Arrays; + +public class KeychainManager implements KeychainAccessStrategy { + + private static final Logger LOG = LoggerFactory.getLogger(KeychainManager.class); + + private final KeychainAccessStrategy keychain; + private LoadingCache passphraseStoredProperties; + + KeychainManager(KeychainAccessStrategy keychain) { + assert keychain.isSupported(); + this.keychain = keychain; + this.passphraseStoredProperties = CacheBuilder.newBuilder() // + .weakValues() // + .build(CacheLoader.from(this::createStoredPassphraseProperty)); + } + + @Override + public void storePassphrase(String key, CharSequence passphrase) throws KeychainAccessException { + keychain.storePassphrase(key, passphrase); + setPassphraseStored(key, true); + } + + @Override + public char[] loadPassphrase(String key) throws KeychainAccessException { + char[] passphrase = keychain.loadPassphrase(key); + setPassphraseStored(key, passphrase != null); + return passphrase; + } + + @Override + public void deletePassphrase(String key) throws KeychainAccessException { + keychain.deletePassphrase(key); + setPassphraseStored(key, false); + } + + @Override + public void changePassphrase(String key, CharSequence passphrase) throws KeychainAccessException { + keychain.changePassphrase(key, passphrase); + setPassphraseStored(key, true); + } + + @Override + public boolean isSupported() { + return true; + } + + /** + * Checks if the keychain knows a passphrase for the given key. + *

+ * Expensive operation. If possible, use {@link #getPassphraseStoredProperty(String)} instead. + * + * @param key The key to look up + * @return true if a password for key is stored. + * @throws KeychainAccessException + */ + public boolean isPassphraseStored(String key) throws KeychainAccessException { + char[] storedPw = null; + try { + storedPw = keychain.loadPassphrase(key); + return storedPw != null; + } finally { + if (storedPw != null) { + Arrays.fill(storedPw, ' '); + } + } + } + + private void setPassphraseStored(String key, boolean value) { + BooleanProperty property = passphraseStoredProperties.getIfPresent(key); + if (property != null) { + if (Platform.isFxApplicationThread()) { + property.set(value); + } else { + LOG.warn(""); + Platform.runLater(() -> property.set(value)); + } + } + } + + /** + * Returns an observable property for use in the UI that tells whether a passphrase is stored for the given key. + *

+ * Assuming that this process is the only process modifying Cryptomator-related items in the system keychain, this + * property stays in memory in an attempt to avoid unnecessary calls to the system keychain. Note that due to this + * fact the value stored in the returned property is not 100% reliable. Code defensively! + * + * @param key The key to look up + * @return An observable property which is true when it almost certain that a password for key is stored. + * @see #isPassphraseStored(String) + */ + public ReadOnlyBooleanProperty getPassphraseStoredProperty(String key) { + return passphraseStoredProperties.getUnchecked(key); + } + + private BooleanProperty createStoredPassphraseProperty(String key) { + try { + LOG.warn("LOAD"); // TODO remove + return new SimpleBooleanProperty(isPassphraseStored(key)); + } catch (KeychainAccessException e) { + return new SimpleBooleanProperty(false); + } + } + +} diff --git a/main/keychain/src/main/java/org/cryptomator/keychain/KeychainModule.java b/main/keychain/src/main/java/org/cryptomator/keychain/KeychainModule.java index d36c8e4b5..1db94fec6 100644 --- a/main/keychain/src/main/java/org/cryptomator/keychain/KeychainModule.java +++ b/main/keychain/src/main/java/org/cryptomator/keychain/KeychainModule.java @@ -5,10 +5,10 @@ *******************************************************************************/ package org.cryptomator.keychain; -import com.google.common.collect.Sets; +import dagger.Binds; import dagger.Module; import dagger.Provides; -import dagger.multibindings.ElementsIntoSet; +import dagger.multibindings.IntoSet; import org.cryptomator.common.JniModule; import javax.inject.Singleton; @@ -16,18 +16,30 @@ import java.util.Optional; import java.util.Set; @Module(includes = {JniModule.class}) -public class KeychainModule { +public abstract class KeychainModule { - @Provides - @ElementsIntoSet - Set provideKeychainAccessStrategies(MacSystemKeychainAccess macKeychain, WindowsProtectedKeychainAccess winKeychain, LinuxSecretServiceKeychainAccess linKeychain) { - return Sets.newHashSet(macKeychain, winKeychain, linKeychain); - } + @Binds + @IntoSet + abstract KeychainAccessStrategy bindMacSystemKeychainAccess(MacSystemKeychainAccess keychainAccessStrategy); + + @Binds + @IntoSet + abstract KeychainAccessStrategy bindWindowsProtectedKeychainAccess(WindowsProtectedKeychainAccess keychainAccessStrategy); + + @Binds + @IntoSet + abstract KeychainAccessStrategy bindLinuxSecretServiceKeychainAccess(LinuxSecretServiceKeychainAccess keychainAccessStrategy); @Provides @Singleton - public Optional provideSupportedKeychain(Set keychainAccessStrategies) { - return keychainAccessStrategies.stream().filter(KeychainAccessStrategy::isSupported).map(KeychainAccess.class::cast).findFirst(); + static Optional provideSupportedKeychain(Set keychainAccessStrategies) { + return keychainAccessStrategies.stream().filter(KeychainAccessStrategy::isSupported).findFirst(); + } + + @Provides + @Singleton + public static Optional provideKeychainManager(Optional keychainAccess) { + return keychainAccess.map(KeychainManager::new); } } diff --git a/main/keychain/src/main/java/org/cryptomator/keychain/LinuxSecretServiceKeychainAccess.java b/main/keychain/src/main/java/org/cryptomator/keychain/LinuxSecretServiceKeychainAccess.java index 7cf0b5e7b..f11bdbd45 100644 --- a/main/keychain/src/main/java/org/cryptomator/keychain/LinuxSecretServiceKeychainAccess.java +++ b/main/keychain/src/main/java/org/cryptomator/keychain/LinuxSecretServiceKeychainAccess.java @@ -3,11 +3,13 @@ package org.cryptomator.keychain; import org.apache.commons.lang3.SystemUtils; import javax.inject.Inject; +import javax.inject.Singleton; import java.util.Optional; /** * A facade to LinuxSecretServiceKeychainAccessImpl that doesn't depend on libraries that are unavailable on Mac and Windows. */ +@Singleton public class LinuxSecretServiceKeychainAccess implements KeychainAccessStrategy { // the actual implementation is hidden in this delegate object which is loaded via reflection, diff --git a/main/keychain/src/main/java/org/cryptomator/keychain/MacSystemKeychainAccess.java b/main/keychain/src/main/java/org/cryptomator/keychain/MacSystemKeychainAccess.java index 382ec0aff..95531b7ae 100644 --- a/main/keychain/src/main/java/org/cryptomator/keychain/MacSystemKeychainAccess.java +++ b/main/keychain/src/main/java/org/cryptomator/keychain/MacSystemKeychainAccess.java @@ -8,11 +8,13 @@ package org.cryptomator.keychain; import java.util.Optional; import javax.inject.Inject; +import javax.inject.Singleton; import org.apache.commons.lang3.SystemUtils; import org.cryptomator.jni.MacFunctions; import org.cryptomator.jni.MacKeychainAccess; +@Singleton class MacSystemKeychainAccess implements KeychainAccessStrategy { private final Optional macFunctions; diff --git a/main/keychain/src/main/java/org/cryptomator/keychain/WindowsProtectedKeychainAccess.java b/main/keychain/src/main/java/org/cryptomator/keychain/WindowsProtectedKeychainAccess.java index 15525aca3..408d7d6e5 100644 --- a/main/keychain/src/main/java/org/cryptomator/keychain/WindowsProtectedKeychainAccess.java +++ b/main/keychain/src/main/java/org/cryptomator/keychain/WindowsProtectedKeychainAccess.java @@ -25,6 +25,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import javax.inject.Inject; +import javax.inject.Singleton; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; @@ -50,6 +51,7 @@ import java.util.stream.Collectors; import static java.nio.charset.StandardCharsets.UTF_8; +@Singleton class WindowsProtectedKeychainAccess implements KeychainAccessStrategy { private static final Logger LOG = LoggerFactory.getLogger(WindowsProtectedKeychainAccess.class); diff --git a/main/keychain/src/test/java/org/cryptomator/keychain/KeychainManagerTest.java b/main/keychain/src/test/java/org/cryptomator/keychain/KeychainManagerTest.java new file mode 100644 index 000000000..67f10e035 --- /dev/null +++ b/main/keychain/src/test/java/org/cryptomator/keychain/KeychainManagerTest.java @@ -0,0 +1,55 @@ +package org.cryptomator.keychain; + + +import javafx.application.Platform; +import javafx.beans.property.ReadOnlyBooleanProperty; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; + +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; + + +class KeychainManagerTest { + + @Test + public void testStoreAndLoad() throws KeychainAccessException { + KeychainManager keychainManager = new KeychainManager(new MapKeychainAccess()); + keychainManager.storePassphrase("test", "asd"); + Assertions.assertArrayEquals("asd".toCharArray(), keychainManager.loadPassphrase("test")); + } + + @Nested + public static class WhenObservingProperties { + + @BeforeAll + public static void startup() throws InterruptedException { + CountDownLatch latch = new CountDownLatch(1); + Platform.startup(latch::countDown); + latch.await(5, TimeUnit.SECONDS); + } + + @Test + public void testPropertyChangesWhenStoringPassword() throws KeychainAccessException, InterruptedException { + KeychainManager keychainManager = new KeychainManager(new MapKeychainAccess()); + ReadOnlyBooleanProperty property = keychainManager.getPassphraseStoredProperty("test"); + Assertions.assertEquals(false, property.get()); + + keychainManager.storePassphrase("test", "bar"); + + AtomicBoolean result = new AtomicBoolean(false); + CountDownLatch latch = new CountDownLatch(1); + Platform.runLater(() -> { + result.set(property.get()); + latch.countDown(); + }); + latch.await(1, TimeUnit.SECONDS); + Assertions.assertEquals(true, result.get()); + } + + } + +} \ No newline at end of file diff --git a/main/keychain/src/test/java/org/cryptomator/keychain/KeychainModuleTest.java b/main/keychain/src/test/java/org/cryptomator/keychain/KeychainModuleTest.java deleted file mode 100644 index 7b3c59899..000000000 --- a/main/keychain/src/test/java/org/cryptomator/keychain/KeychainModuleTest.java +++ /dev/null @@ -1,24 +0,0 @@ -/******************************************************************************* - * Copyright (c) 2017 Skymatic UG (haftungsbeschränkt). - * All rights reserved. This program and the accompanying materials - * are made available under the terms of the accompanying LICENSE file. - *******************************************************************************/ -package org.cryptomator.keychain; - -import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.Test; - -import java.util.Optional; - -public class KeychainModuleTest { - - @Test - public void testGetKeychain() throws KeychainAccessException { - Optional keychainAccess = DaggerTestKeychainComponent.builder().keychainModule(new TestKeychainModule()).build().keychainAccess(); - Assertions.assertTrue(keychainAccess.isPresent()); - Assertions.assertTrue(keychainAccess.get() instanceof MapKeychainAccess); - keychainAccess.get().storePassphrase("test", "asd"); - Assertions.assertArrayEquals("asd".toCharArray(), keychainAccess.get().loadPassphrase("test")); - } - -} diff --git a/main/keychain/src/test/java/org/cryptomator/keychain/TestKeychainComponent.java b/main/keychain/src/test/java/org/cryptomator/keychain/TestKeychainComponent.java deleted file mode 100644 index 82c4b82ae..000000000 --- a/main/keychain/src/test/java/org/cryptomator/keychain/TestKeychainComponent.java +++ /dev/null @@ -1,19 +0,0 @@ -/******************************************************************************* - * Copyright (c) 2017 Skymatic UG (haftungsbeschränkt). - * All rights reserved. This program and the accompanying materials - * are made available under the terms of the accompanying LICENSE file. - *******************************************************************************/ -package org.cryptomator.keychain; - -import dagger.Component; - -import javax.inject.Singleton; -import java.util.Optional; - -@Singleton -@Component(modules = KeychainModule.class) -interface TestKeychainComponent { - - Optional keychainAccess(); - -} diff --git a/main/keychain/src/test/java/org/cryptomator/keychain/TestKeychainModule.java b/main/keychain/src/test/java/org/cryptomator/keychain/TestKeychainModule.java deleted file mode 100644 index d6e183a22..000000000 --- a/main/keychain/src/test/java/org/cryptomator/keychain/TestKeychainModule.java +++ /dev/null @@ -1,17 +0,0 @@ -/******************************************************************************* - * Copyright (c) 2017 Skymatic UG (haftungsbeschränkt). - * All rights reserved. This program and the accompanying materials - * are made available under the terms of the accompanying LICENSE file. - *******************************************************************************/ -package org.cryptomator.keychain; - -import java.util.Set; - -public class TestKeychainModule extends KeychainModule { - - @Override - Set provideKeychainAccessStrategies(MacSystemKeychainAccess macKeychain, WindowsProtectedKeychainAccess winKeychain, LinuxSecretServiceKeychainAccess linKeychain) { - return Set.of(new MapKeychainAccess()); - } - -} diff --git a/main/ui/src/main/java/org/cryptomator/ui/changepassword/ChangePasswordController.java b/main/ui/src/main/java/org/cryptomator/ui/changepassword/ChangePasswordController.java index acb2fd12d..7a641b1e7 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/changepassword/ChangePasswordController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/changepassword/ChangePasswordController.java @@ -10,8 +10,8 @@ import javafx.stage.Stage; import org.cryptomator.common.vaults.Vault; import org.cryptomator.cryptofs.CryptoFileSystemProvider; import org.cryptomator.cryptolib.api.InvalidPassphraseException; -import org.cryptomator.keychain.KeychainAccess; import org.cryptomator.keychain.KeychainAccessException; +import org.cryptomator.keychain.KeychainManager; import org.cryptomator.ui.common.Animations; import org.cryptomator.ui.common.ErrorComponent; import org.cryptomator.ui.common.FxController; @@ -36,14 +36,14 @@ public class ChangePasswordController implements FxController { private final Vault vault; private final ObjectProperty newPassword; private final ErrorComponent.Builder errorComponent; - private final Optional keychain; + private final Optional keychain; public NiceSecurePasswordField oldPasswordField; public CheckBox finalConfirmationCheckbox; public Button finishButton; @Inject - public ChangePasswordController(@ChangePasswordWindow Stage window, @ChangePasswordWindow Vault vault, @Named("newPassword") ObjectProperty newPassword, ErrorComponent.Builder errorComponent, Optional keychain) { + public ChangePasswordController(@ChangePasswordWindow Stage window, @ChangePasswordWindow Vault vault, @Named("newPassword") ObjectProperty newPassword, ErrorComponent.Builder errorComponent, Optional keychain) { this.window = window; this.vault = vault; this.newPassword = newPassword; diff --git a/main/ui/src/main/java/org/cryptomator/ui/common/VaultService.java b/main/ui/src/main/java/org/cryptomator/ui/common/VaultService.java index c96202dda..3e09e1fad 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/common/VaultService.java +++ b/main/ui/src/main/java/org/cryptomator/ui/common/VaultService.java @@ -4,16 +4,13 @@ import javafx.concurrent.Task; import org.cryptomator.common.vaults.Vault; import org.cryptomator.common.vaults.VaultState; import org.cryptomator.common.vaults.Volume; -import org.cryptomator.cryptolib.api.InvalidPassphraseException; -import org.cryptomator.keychain.KeychainAccess; +import org.cryptomator.keychain.KeychainManager; import org.cryptomator.ui.fxapp.FxApplicationScoped; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import javax.inject.Inject; -import java.nio.CharBuffer; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.Iterator; import java.util.List; @@ -28,10 +25,10 @@ public class VaultService { private static final Logger LOG = LoggerFactory.getLogger(VaultService.class); private final ExecutorService executorService; - private final Optional keychain; + private final Optional keychain; @Inject - public VaultService(ExecutorService executorService, Optional keychain) { + public VaultService(ExecutorService executorService, Optional keychain) { this.executorService = executorService; this.keychain = keychain; } diff --git a/main/ui/src/main/java/org/cryptomator/ui/forgetPassword/ForgetPasswordController.java b/main/ui/src/main/java/org/cryptomator/ui/forgetPassword/ForgetPasswordController.java index e072840ba..ea1e0530e 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/forgetPassword/ForgetPasswordController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/forgetPassword/ForgetPasswordController.java @@ -4,8 +4,8 @@ import javafx.beans.property.BooleanProperty; import javafx.fxml.FXML; import javafx.stage.Stage; import org.cryptomator.common.vaults.Vault; -import org.cryptomator.keychain.KeychainAccess; import org.cryptomator.keychain.KeychainAccessException; +import org.cryptomator.keychain.KeychainManager; import org.cryptomator.ui.common.FxController; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -20,14 +20,14 @@ public class ForgetPasswordController implements FxController { private final Stage window; private final Vault vault; - private final Optional keychainAccess; + private final Optional keychain; private final BooleanProperty confirmedResult; @Inject - public ForgetPasswordController(@ForgetPasswordWindow Stage window, @ForgetPasswordWindow Vault vault, Optional keychainAccess, @ForgetPasswordWindow BooleanProperty confirmedResult) { + public ForgetPasswordController(@ForgetPasswordWindow Stage window, @ForgetPasswordWindow Vault vault, Optional keychain, @ForgetPasswordWindow BooleanProperty confirmedResult) { this.window = window; this.vault = vault; - this.keychainAccess = keychainAccess; + this.keychain = keychain; this.confirmedResult = confirmedResult; } @@ -38,9 +38,9 @@ public class ForgetPasswordController implements FxController { @FXML public void finish() { - if (keychainAccess.isPresent()) { + if (keychain.isPresent()) { try { - keychainAccess.get().deletePassphrase(vault.getId()); + keychain.get().deletePassphrase(vault.getId()); LOG.debug("Forgot password for vault {}.", vault.getDisplayableName()); confirmedResult.setValue(true); } catch (KeychainAccessException e) { diff --git a/main/ui/src/main/java/org/cryptomator/ui/migration/MigrationRunController.java b/main/ui/src/main/java/org/cryptomator/ui/migration/MigrationRunController.java index 05b460348..3123d4cb2 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/migration/MigrationRunController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/migration/MigrationRunController.java @@ -22,8 +22,8 @@ import org.cryptomator.cryptofs.migration.Migrators; import org.cryptomator.cryptofs.migration.api.MigrationContinuationListener; import org.cryptomator.cryptofs.migration.api.MigrationProgressListener; import org.cryptomator.cryptolib.api.InvalidPassphraseException; -import org.cryptomator.keychain.KeychainAccess; import org.cryptomator.keychain.KeychainAccessException; +import org.cryptomator.keychain.KeychainManager; import org.cryptomator.ui.common.Animations; import org.cryptomator.ui.common.ErrorComponent; import org.cryptomator.ui.common.FxController; @@ -55,7 +55,7 @@ public class MigrationRunController implements FxController { private final Vault vault; private final ExecutorService executor; private final ScheduledExecutorService scheduler; - private final Optional keychainAccess; + private final Optional keychain; private final ObjectProperty missingCapability; private final ErrorComponent.Builder errorComponent; private final Lazy startScene; @@ -69,13 +69,13 @@ public class MigrationRunController implements FxController { public NiceSecurePasswordField passwordField; @Inject - public MigrationRunController(@MigrationWindow Stage window, @MigrationWindow Vault vault, ExecutorService executor, ScheduledExecutorService scheduler, Optional keychainAccess, @Named("capabilityErrorCause") ObjectProperty missingCapability, @FxmlScene(FxmlFile.MIGRATION_START) Lazy startScene, @FxmlScene(FxmlFile.MIGRATION_SUCCESS) Lazy successScene, @FxmlScene(FxmlFile.MIGRATION_CAPABILITY_ERROR) Lazy capabilityErrorScene, @FxmlScene(FxmlFile.MIGRATION_IMPOSSIBLE) Lazy impossibleScene, ErrorComponent.Builder errorComponent) { + public MigrationRunController(@MigrationWindow Stage window, @MigrationWindow Vault vault, ExecutorService executor, ScheduledExecutorService scheduler, Optional keychain, @Named("capabilityErrorCause") ObjectProperty missingCapability, @FxmlScene(FxmlFile.MIGRATION_START) Lazy startScene, @FxmlScene(FxmlFile.MIGRATION_SUCCESS) Lazy successScene, @FxmlScene(FxmlFile.MIGRATION_CAPABILITY_ERROR) Lazy capabilityErrorScene, @FxmlScene(FxmlFile.MIGRATION_IMPOSSIBLE) Lazy impossibleScene, ErrorComponent.Builder errorComponent) { this.window = window; this.vault = vault; this.executor = executor; this.scheduler = scheduler; - this.keychainAccess = keychainAccess; + this.keychain = keychain; this.missingCapability = missingCapability; this.errorComponent = errorComponent; this.startScene = startScene; @@ -88,7 +88,7 @@ public class MigrationRunController implements FxController { } public void initialize() { - if (keychainAccess.isPresent()) { + if (keychain.isPresent()) { loadStoredPassword(); } migrationButtonDisabled.bind(vault.stateProperty().isNotEqualTo(VaultState.NEEDS_MIGRATION).or(passwordField.textProperty().isEmpty())); @@ -167,10 +167,10 @@ public class MigrationRunController implements FxController { } private void loadStoredPassword() { - assert keychainAccess.isPresent(); + assert keychain.isPresent(); char[] storedPw = null; try { - storedPw = keychainAccess.get().loadPassphrase(vault.getId()); + storedPw = keychain.get().loadPassphrase(vault.getId()); if (storedPw != null) { passwordField.setPassword(storedPw); passwordField.selectRange(storedPw.length, storedPw.length); diff --git a/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockController.java b/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockController.java index 38e41323c..e08164167 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockController.java @@ -11,7 +11,7 @@ import javafx.scene.control.CheckBox; import javafx.scene.control.ContentDisplay; import javafx.stage.Stage; import org.cryptomator.common.vaults.Vault; -import org.cryptomator.keychain.KeychainAccess; +import org.cryptomator.keychain.KeychainManager; import org.cryptomator.ui.common.FxController; import org.cryptomator.ui.common.UserInteractionLock; import org.cryptomator.ui.controls.NiceSecurePasswordField; @@ -38,7 +38,7 @@ public class UnlockController implements FxController { private final Optional savedPassword; private final UserInteractionLock passwordEntryLock; private final ForgetPasswordComponent.Builder forgetPassword; - private final Optional keychainAccess; + private final Optional keychain; private final ObjectBinding unlockButtonContentDisplay; private final BooleanBinding userInteractionDisabled; private final BooleanProperty unlockButtonDisabled; @@ -46,7 +46,7 @@ public class UnlockController implements FxController { public CheckBox savePasswordCheckbox; @Inject - public UnlockController(@UnlockWindow Stage window, @UnlockWindow Vault vault, AtomicReference password, @Named("savePassword") AtomicBoolean savePassword, @Named("savedPassword") Optional savedPassword, UserInteractionLock passwordEntryLock, ForgetPasswordComponent.Builder forgetPassword, Optional keychainAccess) { + public UnlockController(@UnlockWindow Stage window, @UnlockWindow Vault vault, AtomicReference password, @Named("savePassword") AtomicBoolean savePassword, @Named("savedPassword") Optional savedPassword, UserInteractionLock passwordEntryLock, ForgetPasswordComponent.Builder forgetPassword, Optional keychain) { this.window = window; this.vault = vault; this.password = password; @@ -54,7 +54,7 @@ public class UnlockController implements FxController { this.savedPassword = savedPassword; this.passwordEntryLock = passwordEntryLock; this.forgetPassword = forgetPassword; - this.keychainAccess = keychainAccess; + this.keychain = keychain; this.unlockButtonContentDisplay = Bindings.createObjectBinding(this::getUnlockButtonContentDisplay, passwordEntryLock.awaitingInteraction()); this.userInteractionDisabled = passwordEntryLock.awaitingInteraction().not(); this.unlockButtonDisabled = new SimpleBooleanProperty(); @@ -131,6 +131,6 @@ public class UnlockController implements FxController { } public boolean isKeychainAccessAvailable() { - return keychainAccess.isPresent(); + return keychain.isPresent(); } } diff --git a/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockModule.java b/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockModule.java index 4b1ba3cfe..a126f4be3 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockModule.java +++ b/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockModule.java @@ -5,12 +5,11 @@ import dagger.Module; import dagger.Provides; import dagger.multibindings.IntoMap; import javafx.scene.Scene; -import javafx.scene.image.Image; import javafx.stage.Modality; import javafx.stage.Stage; import org.cryptomator.common.vaults.Vault; -import org.cryptomator.keychain.KeychainAccess; import org.cryptomator.keychain.KeychainAccessException; +import org.cryptomator.keychain.KeychainManager; import org.cryptomator.ui.common.DefaultSceneFactory; import org.cryptomator.ui.common.FXMLLoaderFactory; import org.cryptomator.ui.common.FxController; @@ -25,16 +24,11 @@ import org.slf4j.LoggerFactory; import javax.inject.Named; import javax.inject.Provider; -import java.nio.CharBuffer; -import java.util.List; import java.util.Map; import java.util.Optional; import java.util.ResourceBundle; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; -import java.util.concurrent.locks.Condition; -import java.util.concurrent.locks.Lock; -import java.util.concurrent.locks.ReentrantLock; @Module(subcomponents = {ForgetPasswordComponent.class}) abstract class UnlockModule { @@ -52,8 +46,8 @@ abstract class UnlockModule { @Provides @Named("savedPassword") @UnlockScoped - static Optional provideStoredPassword(Optional keychainAccess, @UnlockWindow Vault vault) { - return keychainAccess.map(k -> { + static Optional provideStoredPassword(Optional keychain, @UnlockWindow Vault vault) { + return keychain.map(k -> { try { return k.loadPassphrase(vault.getId()); } catch (KeychainAccessException e) { diff --git a/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java b/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java index c2f6e47f4..d9f9affd0 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java +++ b/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java @@ -10,8 +10,8 @@ import org.cryptomator.common.vaults.VaultState; import org.cryptomator.common.vaults.Volume; import org.cryptomator.cryptolib.api.CryptoException; import org.cryptomator.cryptolib.api.InvalidPassphraseException; -import org.cryptomator.keychain.KeychainAccess; import org.cryptomator.keychain.KeychainAccessException; +import org.cryptomator.keychain.KeychainManager; import org.cryptomator.ui.common.Animations; import org.cryptomator.ui.common.ErrorComponent; import org.cryptomator.ui.common.FxmlFile; @@ -51,14 +51,14 @@ public class UnlockWorkflow extends Task { private final AtomicBoolean savePassword; private final Optional savedPassword; private final UserInteractionLock passwordEntryLock; - private final Optional keychain; + private final Optional keychain; private final Lazy unlockScene; private final Lazy successScene; private final Lazy invalidMountPointScene; private final ErrorComponent.Builder errorComponent; @Inject - UnlockWorkflow(@UnlockWindow Stage window, @UnlockWindow Vault vault, VaultService vaultService, AtomicReference password, @Named("savePassword") AtomicBoolean savePassword, @Named("savedPassword") Optional savedPassword, UserInteractionLock passwordEntryLock, Optional keychain, @FxmlScene(FxmlFile.UNLOCK) Lazy unlockScene, @FxmlScene(FxmlFile.UNLOCK_SUCCESS) Lazy successScene, @FxmlScene(FxmlFile.UNLOCK_INVALID_MOUNT_POINT) Lazy invalidMountPointScene, ErrorComponent.Builder errorComponent) { + UnlockWorkflow(@UnlockWindow Stage window, @UnlockWindow Vault vault, VaultService vaultService, AtomicReference password, @Named("savePassword") AtomicBoolean savePassword, @Named("savedPassword") Optional savedPassword, UserInteractionLock passwordEntryLock, Optional keychain, @FxmlScene(FxmlFile.UNLOCK) Lazy unlockScene, @FxmlScene(FxmlFile.UNLOCK_SUCCESS) Lazy successScene, @FxmlScene(FxmlFile.UNLOCK_INVALID_MOUNT_POINT) Lazy invalidMountPointScene, ErrorComponent.Builder errorComponent) { this.window = window; this.vault = vault; this.vaultService = vaultService;