From 983a4d0b0faa554664127b41c3b52b67684849dc Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Thu, 16 Dec 2021 17:00:49 +0100 Subject: [PATCH] move masterkey selection to subcomponent and use CompletableFuture instead of UserInteractionLock + AtomicReference --- .../ChooseMasterkeyFileComponent.java | 25 +++++++++++ .../ChooseMasterkeyFileModule.java | 42 ++++++++++++++++++ .../ChooseMasterkeyFileScoped.java | 13 ++++++ .../MasterkeyFileLoadingModule.java | 40 +---------------- .../MasterkeyFileLoadingStrategy.java | 44 +++++++------------ .../SelectMasterkeyFileController.java | 24 +++------- 6 files changed, 103 insertions(+), 85 deletions(-) create mode 100644 src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/ChooseMasterkeyFileComponent.java create mode 100644 src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/ChooseMasterkeyFileModule.java create mode 100644 src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/ChooseMasterkeyFileScoped.java diff --git a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/ChooseMasterkeyFileComponent.java b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/ChooseMasterkeyFileComponent.java new file mode 100644 index 000000000..a548cd47d --- /dev/null +++ b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/ChooseMasterkeyFileComponent.java @@ -0,0 +1,25 @@ +package org.cryptomator.ui.keyloading.masterkeyfile; + +import dagger.Subcomponent; + +import javafx.scene.Scene; +import java.nio.file.Path; +import java.util.concurrent.CompletableFuture; + +@ChooseMasterkeyFileScoped +@Subcomponent(modules = {ChooseMasterkeyFileModule.class}) +public interface ChooseMasterkeyFileComponent { + + @ChooseMasterkeyFileScoped + Scene chooseMasterkeyScene(); + + @ChooseMasterkeyFileScoped + CompletableFuture result(); + + @Subcomponent.Builder + interface Builder { + + ChooseMasterkeyFileComponent build(); + } + +} diff --git a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/ChooseMasterkeyFileModule.java b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/ChooseMasterkeyFileModule.java new file mode 100644 index 000000000..1cc71cbfa --- /dev/null +++ b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/ChooseMasterkeyFileModule.java @@ -0,0 +1,42 @@ +package org.cryptomator.ui.keyloading.masterkeyfile; + +import dagger.Module; +import dagger.Provides; +import org.cryptomator.ui.common.DefaultSceneFactory; +import org.cryptomator.ui.common.FxmlFile; +import org.cryptomator.ui.common.FxmlLoaderFactory; + +import javafx.fxml.FXMLLoader; +import javafx.scene.Parent; +import javafx.scene.Scene; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.file.Path; +import java.util.ResourceBundle; +import java.util.concurrent.CompletableFuture; + +@Module +abstract class ChooseMasterkeyFileModule { + + @Provides + @ChooseMasterkeyFileScoped + static CompletableFuture provideResult() { + return new CompletableFuture<>(); + } + + @Provides + @ChooseMasterkeyFileScoped + static Scene provideChooseMasterkeyScene(SelectMasterkeyFileController controller, DefaultSceneFactory sceneFactory, ResourceBundle resourceBundle) { + // TODO: simplify FxmlLoaderFactory + try { + var url = FxmlLoaderFactory.class.getResource(FxmlFile.UNLOCK_SELECT_MASTERKEYFILE.getRessourcePathString()); + var loader = new FXMLLoader(url, resourceBundle, null, clazz -> controller); + Parent root = loader.load(); + return sceneFactory.apply(root); + } catch (IOException e) { + throw new UncheckedIOException("Failed to load UnlockScene", e); + } + } + + +} diff --git a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/ChooseMasterkeyFileScoped.java b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/ChooseMasterkeyFileScoped.java new file mode 100644 index 000000000..4bf8c5c24 --- /dev/null +++ b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/ChooseMasterkeyFileScoped.java @@ -0,0 +1,13 @@ +package org.cryptomator.ui.keyloading.masterkeyfile; + +import javax.inject.Scope; +import java.lang.annotation.Documented; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; + +@Scope +@Documented +@Retention(RetentionPolicy.RUNTIME) +@interface ChooseMasterkeyFileScoped { + +} diff --git a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/MasterkeyFileLoadingModule.java b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/MasterkeyFileLoadingModule.java index 7f55de898..af592cb3b 100644 --- a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/MasterkeyFileLoadingModule.java +++ b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/MasterkeyFileLoadingModule.java @@ -8,12 +8,6 @@ import dagger.multibindings.StringKey; import org.cryptomator.common.keychain.KeychainManager; import org.cryptomator.common.vaults.Vault; import org.cryptomator.integrations.keychain.KeychainAccessException; -import org.cryptomator.ui.common.FxController; -import org.cryptomator.ui.common.FxControllerKey; -import org.cryptomator.ui.common.FxmlFile; -import org.cryptomator.ui.common.FxmlLoaderFactory; -import org.cryptomator.ui.common.FxmlScene; -import org.cryptomator.ui.common.UserInteractionLock; import org.cryptomator.ui.forgetPassword.ForgetPasswordComponent; import org.cryptomator.ui.keyloading.KeyLoading; import org.cryptomator.ui.keyloading.KeyLoadingScoped; @@ -22,27 +16,13 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import javax.inject.Named; -import javafx.scene.Scene; -import java.nio.file.Path; import java.util.Optional; -import java.util.concurrent.atomic.AtomicReference; -@Module(subcomponents = {ForgetPasswordComponent.class, PassphraseEntryComponent.class}) +@Module(subcomponents = {ForgetPasswordComponent.class, PassphraseEntryComponent.class, ChooseMasterkeyFileComponent.class}) public abstract class MasterkeyFileLoadingModule { private static final Logger LOG = LoggerFactory.getLogger(MasterkeyFileLoadingModule.class); - public enum MasterkeyFileProvision { - MASTERKEYFILE_PROVIDED, - CANCELED - } - - @Provides - @KeyLoadingScoped - static UserInteractionLock provideMasterkeyFileProvisionLock() { - return new UserInteractionLock<>(null); - } - @Provides @Named("savedPassword") @KeyLoadingScoped @@ -59,24 +39,6 @@ public abstract class MasterkeyFileLoadingModule { } } - @Provides - @KeyLoadingScoped - static AtomicReference provideUserProvidedMasterkeyPath() { - return new AtomicReference<>(); - } - - @Provides - @FxmlScene(FxmlFile.UNLOCK_SELECT_MASTERKEYFILE) - @KeyLoadingScoped - static Scene provideUnlockSelectMasterkeyFileScene(@KeyLoading FxmlLoaderFactory fxmlLoaders) { - return fxmlLoaders.createScene(FxmlFile.UNLOCK_SELECT_MASTERKEYFILE); - } - - @Binds - @IntoMap - @FxControllerKey(SelectMasterkeyFileController.class) - abstract FxController bindUnlockSelectMasterkeyFileController(SelectMasterkeyFileController controller); - @Binds @IntoMap @KeyLoadingScoped 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 a22903973..13a7dadda 100644 --- a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/MasterkeyFileLoadingStrategy.java +++ b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/MasterkeyFileLoadingStrategy.java @@ -1,7 +1,7 @@ package org.cryptomator.ui.keyloading.masterkeyfile; import com.google.common.base.Preconditions; -import dagger.Lazy; +import org.cryptomator.common.Passphrase; import org.cryptomator.common.keychain.KeychainManager; import org.cryptomator.common.vaults.Vault; import org.cryptomator.cryptofs.common.BackupHelper; @@ -11,10 +11,6 @@ import org.cryptomator.cryptolib.api.MasterkeyLoadingFailedException; import org.cryptomator.cryptolib.common.MasterkeyFileAccess; import org.cryptomator.integrations.keychain.KeychainAccessException; import org.cryptomator.ui.common.Animations; -import org.cryptomator.ui.common.FxmlFile; -import org.cryptomator.ui.common.FxmlScene; -import org.cryptomator.common.Passphrase; -import org.cryptomator.ui.common.UserInteractionLock; import org.cryptomator.ui.keyloading.KeyLoading; import org.cryptomator.ui.keyloading.KeyLoadingStrategy; import org.cryptomator.ui.unlock.UnlockCancelledException; @@ -22,7 +18,6 @@ import org.cryptomator.ui.unlock.UnlockCancelledException; import javax.inject.Inject; import javax.inject.Named; import javafx.application.Platform; -import javafx.scene.Scene; import javafx.stage.Stage; import javafx.stage.Window; import java.io.IOException; @@ -32,7 +27,6 @@ import java.nio.file.Path; import java.util.Optional; import java.util.concurrent.CancellationException; import java.util.concurrent.ExecutionException; -import java.util.concurrent.atomic.AtomicReference; @KeyLoading public class MasterkeyFileLoadingStrategy implements KeyLoadingStrategy { @@ -42,10 +36,8 @@ public class MasterkeyFileLoadingStrategy implements KeyLoadingStrategy { private final Vault vault; private final MasterkeyFileAccess masterkeyFileAccess; private final Stage window; - private final Lazy selectMasterkeyFileScene; private final PassphraseEntryComponent.Builder passphraseEntry; - private final UserInteractionLock masterkeyFileProvisionLock; - private final AtomicReference filePath; + private final ChooseMasterkeyFileComponent.Builder masterkeyChooser; private final KeychainManager keychain; private Passphrase passphrase; @@ -53,14 +45,12 @@ public class MasterkeyFileLoadingStrategy implements KeyLoadingStrategy { private boolean wrongPassphrase; @Inject - public MasterkeyFileLoadingStrategy(@KeyLoading Vault vault, MasterkeyFileAccess masterkeyFileAccess, @KeyLoading Stage window, @FxmlScene(FxmlFile.UNLOCK_SELECT_MASTERKEYFILE) Lazy selectMasterkeyFileScene, @Named("savedPassword") Optional savedPassphrase, PassphraseEntryComponent.Builder passphraseEntry, UserInteractionLock masterkeyFileProvisionLock, AtomicReference filePath, KeychainManager keychain) { + public MasterkeyFileLoadingStrategy(@KeyLoading Vault vault, MasterkeyFileAccess masterkeyFileAccess, @KeyLoading Stage window, @Named("savedPassword") Optional savedPassphrase, PassphraseEntryComponent.Builder passphraseEntry, ChooseMasterkeyFileComponent.Builder masterkeyChooser, KeychainManager keychain) { this.vault = vault; this.masterkeyFileAccess = masterkeyFileAccess; this.window = window; - this.selectMasterkeyFileScene = selectMasterkeyFileScene; this.passphraseEntry = passphraseEntry; - this.masterkeyFileProvisionLock = masterkeyFileProvisionLock; - this.filePath = filePath; + this.masterkeyChooser = masterkeyChooser; this.keychain = keychain; this.passphrase = savedPassphrase.map(Passphrase::new).orElse(null); this.savePassphrase = savedPassphrase.isPresent(); @@ -72,7 +62,7 @@ public class MasterkeyFileLoadingStrategy implements KeyLoadingStrategy { try { Path filePath = vault.getPath().resolve(keyId.getSchemeSpecificPart()); if (!Files.exists(filePath)) { - filePath = getAlternateMasterkeyFilePath(); + filePath = askUserForMasterkeyFilePath(); } if (passphrase == null) { askForPassphrase(); @@ -127,20 +117,10 @@ public class MasterkeyFileLoadingStrategy implements KeyLoadingStrategy { } } - private Path getAlternateMasterkeyFilePath() throws UnlockCancelledException, InterruptedException { - if (filePath.get() == null) { - return switch (askUserForMasterkeyFilePath()) { - case MASTERKEYFILE_PROVIDED -> filePath.get(); - case CANCELED -> throw new UnlockCancelledException("Choosing masterkey file cancelled."); - }; - } else { - return filePath.get(); - } - } - - private MasterkeyFileLoadingModule.MasterkeyFileProvision askUserForMasterkeyFilePath() throws InterruptedException { + private Path askUserForMasterkeyFilePath() throws InterruptedException { + var comp = masterkeyChooser.build(); Platform.runLater(() -> { - window.setScene(selectMasterkeyFileScene.get()); + window.setScene(comp.chooseMasterkeyScene()); window.show(); Window owner = window.getOwner(); if (owner != null) { @@ -150,7 +130,13 @@ public class MasterkeyFileLoadingStrategy implements KeyLoadingStrategy { window.centerOnScreen(); } }); - return masterkeyFileProvisionLock.awaitInteraction(); + try { + return comp.result().get(); + } catch (CancellationException e) { + throw new UnlockCancelledException("Choosing masterkey file cancelled."); + } catch (ExecutionException e) { + throw new MasterkeyLoadingFailedException("Failed to select masterkey file.", e); + } } private void askForPassphrase() throws InterruptedException { diff --git a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/SelectMasterkeyFileController.java b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/SelectMasterkeyFileController.java index 39be2b36e..fc225e0a8 100644 --- a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/SelectMasterkeyFileController.java +++ b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/SelectMasterkeyFileController.java @@ -1,10 +1,7 @@ package org.cryptomator.ui.keyloading.masterkeyfile; import org.cryptomator.ui.common.FxController; -import org.cryptomator.ui.common.UserInteractionLock; import org.cryptomator.ui.keyloading.KeyLoading; -import org.cryptomator.ui.keyloading.KeyLoadingScoped; -import org.cryptomator.ui.keyloading.masterkeyfile.MasterkeyFileLoadingModule.MasterkeyFileProvision; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -16,23 +13,21 @@ import javafx.stage.WindowEvent; import java.io.File; import java.nio.file.Path; import java.util.ResourceBundle; -import java.util.concurrent.atomic.AtomicReference; +import java.util.concurrent.CompletableFuture; -@KeyLoadingScoped +@ChooseMasterkeyFileScoped public class SelectMasterkeyFileController implements FxController { private static final Logger LOG = LoggerFactory.getLogger(SelectMasterkeyFileController.class); private final Stage window; - private final AtomicReference masterkeyPath; - private final UserInteractionLock masterkeyFileProvisionLock; + private final CompletableFuture result; private final ResourceBundle resourceBundle; @Inject - public SelectMasterkeyFileController(@KeyLoading Stage window, AtomicReference masterkeyPath, UserInteractionLock masterkeyFileProvisionLock, ResourceBundle resourceBundle) { + public SelectMasterkeyFileController(@KeyLoading Stage window, CompletableFuture result, ResourceBundle resourceBundle) { this.window = window; - this.masterkeyPath = masterkeyPath; - this.masterkeyFileProvisionLock = masterkeyFileProvisionLock; + this.result = result; this.resourceBundle = resourceBundle; this.window.setOnHiding(this::windowClosed); } @@ -43,11 +38,7 @@ public class SelectMasterkeyFileController implements FxController { } private void windowClosed(WindowEvent windowEvent) { - // if not already interacted, mark this workflow as cancelled: - if (masterkeyFileProvisionLock.awaitingInteraction().get()) { - LOG.debug("Unlock canceled by user."); - masterkeyFileProvisionLock.interacted(MasterkeyFileProvision.CANCELED); - } + result.cancel(true); } @FXML @@ -59,8 +50,7 @@ public class SelectMasterkeyFileController implements FxController { File masterkeyFile = fileChooser.showOpenDialog(window); if (masterkeyFile != null) { LOG.debug("Chose masterkey file: {}", masterkeyFile); - masterkeyPath.set(masterkeyFile.toPath()); - masterkeyFileProvisionLock.interacted(MasterkeyFileProvision.MASTERKEYFILE_PROVIDED); + result.complete(masterkeyFile.toPath()); } }