From 117fe78a4a271590ee0590c21bebc3f45e4e9b94 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Thu, 7 May 2020 12:35:01 +0200 Subject: [PATCH 01/17] Refactored stage creation --- .../ui/addvaultwizard/AddVaultModule.java | 6 ++--- .../changepassword/ChangePasswordModule.java | 6 ++--- .../cryptomator/ui/common/StageFactory.java | 26 +++++++++++++++++++ .../forgetPassword/ForgetPasswordModule.java | 6 ++--- .../cryptomator/ui/fxapp/FxApplication.java | 23 +++++----------- .../ui/fxapp/FxApplicationModule.java | 26 ++++++++++++++++++- .../ui/mainwindow/MainWindowModule.java | 6 ++--- .../ui/migration/MigrationModule.java | 6 ++--- .../ui/preferences/PreferencesModule.java | 6 ++--- .../org/cryptomator/ui/quit/QuitModule.java | 6 ++--- .../ui/recoverykey/RecoveryKeyModule.java | 6 ++--- .../ui/removevault/RemoveVaultModule.java | 6 ++--- .../cryptomator/ui/unlock/UnlockModule.java | 6 ++--- .../ui/vaultoptions/VaultOptionsModule.java | 6 ++--- .../wrongfilealert/WrongFileAlertModule.java | 6 ++--- 15 files changed, 94 insertions(+), 53 deletions(-) create mode 100644 main/ui/src/main/java/org/cryptomator/ui/common/StageFactory.java diff --git a/main/ui/src/main/java/org/cryptomator/ui/addvaultwizard/AddVaultModule.java b/main/ui/src/main/java/org/cryptomator/ui/addvaultwizard/AddVaultModule.java index 08f499c92..d3e97e8d7 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/addvaultwizard/AddVaultModule.java +++ b/main/ui/src/main/java/org/cryptomator/ui/addvaultwizard/AddVaultModule.java @@ -21,6 +21,7 @@ import org.cryptomator.ui.common.FxmlFile; import org.cryptomator.ui.common.FxmlScene; import org.cryptomator.ui.common.NewPasswordController; import org.cryptomator.ui.common.PasswordStrengthUtil; +import org.cryptomator.ui.common.StageFactory; import org.cryptomator.ui.mainwindow.MainWindow; import org.cryptomator.ui.recoverykey.RecoveryKeyDisplayController; @@ -51,13 +52,12 @@ public abstract class AddVaultModule { @Provides @AddVaultWizardWindow @AddVaultWizardScoped - static Stage provideStage(@MainWindow Stage owner, ResourceBundle resourceBundle, @Named("windowIcons") List windowIcons) { - Stage stage = new Stage(); + static Stage provideStage(StageFactory factory, @MainWindow Stage owner, ResourceBundle resourceBundle) { + Stage stage = factory.create(); stage.setTitle(resourceBundle.getString("addvaultwizard.title")); stage.setResizable(false); stage.initModality(Modality.WINDOW_MODAL); stage.initOwner(owner); - stage.getIcons().addAll(windowIcons); return stage; } diff --git a/main/ui/src/main/java/org/cryptomator/ui/changepassword/ChangePasswordModule.java b/main/ui/src/main/java/org/cryptomator/ui/changepassword/ChangePasswordModule.java index 41e0ede1d..c6367d124 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/changepassword/ChangePasswordModule.java +++ b/main/ui/src/main/java/org/cryptomator/ui/changepassword/ChangePasswordModule.java @@ -18,6 +18,7 @@ import org.cryptomator.ui.common.FxmlFile; import org.cryptomator.ui.common.FxmlScene; import org.cryptomator.ui.common.NewPasswordController; import org.cryptomator.ui.common.PasswordStrengthUtil; +import org.cryptomator.ui.common.StageFactory; import javax.inject.Named; import javax.inject.Provider; @@ -46,13 +47,12 @@ abstract class ChangePasswordModule { @Provides @ChangePasswordWindow @ChangePasswordScoped - static Stage provideStage(@Named("changePasswordOwner") Stage owner, ResourceBundle resourceBundle, @Named("windowIcons") List windowIcons) { - Stage stage = new Stage(); + static Stage provideStage(StageFactory factory, @Named("changePasswordOwner") Stage owner, ResourceBundle resourceBundle) { + Stage stage = factory.create(); stage.setTitle(resourceBundle.getString("changepassword.title")); stage.setResizable(false); stage.initModality(Modality.WINDOW_MODAL); stage.initOwner(owner); - stage.getIcons().addAll(windowIcons); return stage; } diff --git a/main/ui/src/main/java/org/cryptomator/ui/common/StageFactory.java b/main/ui/src/main/java/org/cryptomator/ui/common/StageFactory.java new file mode 100644 index 000000000..cb980d523 --- /dev/null +++ b/main/ui/src/main/java/org/cryptomator/ui/common/StageFactory.java @@ -0,0 +1,26 @@ +package org.cryptomator.ui.common; + +import javafx.stage.Stage; +import javafx.stage.StageStyle; + +import java.util.function.Consumer; + +public class StageFactory { + + private final Consumer initializer; + + public StageFactory(Consumer initializer) { + this.initializer = initializer; + } + + public Stage create() { + return create(StageStyle.DECORATED); + } + + public Stage create(StageStyle stageStyle) { + Stage stage = new Stage(stageStyle); + initializer.accept(stage); + return stage; + } + +} diff --git a/main/ui/src/main/java/org/cryptomator/ui/forgetPassword/ForgetPasswordModule.java b/main/ui/src/main/java/org/cryptomator/ui/forgetPassword/ForgetPasswordModule.java index a6d45fa41..930a2638b 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/forgetPassword/ForgetPasswordModule.java +++ b/main/ui/src/main/java/org/cryptomator/ui/forgetPassword/ForgetPasswordModule.java @@ -17,6 +17,7 @@ import org.cryptomator.ui.common.FxController; import org.cryptomator.ui.common.FxControllerKey; import org.cryptomator.ui.common.FxmlFile; import org.cryptomator.ui.common.FxmlScene; +import org.cryptomator.ui.common.StageFactory; import javax.inject.Named; import javax.inject.Provider; @@ -37,13 +38,12 @@ abstract class ForgetPasswordModule { @Provides @ForgetPasswordWindow @ForgetPasswordScoped - static Stage provideStage(ResourceBundle resourceBundle, @Named("windowIcons") List windowIcons, @Named("forgetPasswordOwner") Stage owner) { - Stage stage = new Stage(); + static Stage provideStage(StageFactory factory, ResourceBundle resourceBundle, @Named("forgetPasswordOwner") Stage owner) { + Stage stage = factory.create(); stage.setTitle(resourceBundle.getString("forgetPassword.title")); stage.setResizable(false); stage.initModality(Modality.WINDOW_MODAL); stage.initOwner(owner); - stage.getIcons().addAll(windowIcons); return stage; } diff --git a/main/ui/src/main/java/org/cryptomator/ui/fxapp/FxApplication.java b/main/ui/src/main/java/org/cryptomator/ui/fxapp/FxApplication.java index 289497647..291e1a347 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/fxapp/FxApplication.java +++ b/main/ui/src/main/java/org/cryptomator/ui/fxapp/FxApplication.java @@ -43,11 +43,10 @@ public class FxApplication extends Application { private final Optional macFunctions; private final VaultService vaultService; private final LicenseHolder licenseHolder; - private final ObservableSet visibleStages = FXCollections.observableSet(); - private final BooleanBinding hasVisibleStages = Bindings.isNotEmpty(visibleStages); + private final BooleanBinding hasVisibleStages; @Inject - FxApplication(Settings settings, Lazy mainWindow, Lazy preferencesWindow, UnlockComponent.Builder unlockWindowBuilder, QuitComponent.Builder quitWindowBuilder, Optional macFunctions, VaultService vaultService, LicenseHolder licenseHolder) { + FxApplication(Settings settings, Lazy mainWindow, Lazy preferencesWindow, UnlockComponent.Builder unlockWindowBuilder, QuitComponent.Builder quitWindowBuilder, Optional macFunctions, VaultService vaultService, LicenseHolder licenseHolder, ObservableSet visibleStages) { this.settings = settings; this.mainWindow = mainWindow; this.preferencesWindow = preferencesWindow; @@ -56,6 +55,7 @@ public class FxApplication extends Application { this.macFunctions = macFunctions; this.vaultService = vaultService; this.licenseHolder = licenseHolder; + this.hasVisibleStages = Bindings.isNotEmpty(visibleStages); } public void start() { @@ -73,11 +73,6 @@ public class FxApplication extends Application { throw new UnsupportedOperationException("Use start() instead."); } - private void addVisibleStage(Stage stage) { - visibleStages.add(stage); - stage.setOnHidden(evt -> visibleStages.remove(stage)); - } - private void hasVisibleStagesChanged(@SuppressWarnings("unused") ObservableValue observableValue, @SuppressWarnings("unused") boolean oldValue, boolean newValue) { if (newValue) { macFunctions.map(MacFunctions::uiState).ifPresent(MacApplicationUiState::transformToForegroundApplication); @@ -88,32 +83,28 @@ public class FxApplication extends Application { public void showPreferencesWindow(SelectedPreferencesTab selectedTab) { Platform.runLater(() -> { - Stage stage = preferencesWindow.get().showPreferencesWindow(selectedTab); - addVisibleStage(stage); + preferencesWindow.get().showPreferencesWindow(selectedTab); LOG.debug("Showing Preferences"); }); } public void showMainWindow() { Platform.runLater(() -> { - Stage stage = mainWindow.get().showMainWindow(); - addVisibleStage(stage); + mainWindow.get().showMainWindow(); LOG.debug("Showing MainWindow"); }); } public void showUnlockWindow(Vault vault) { Platform.runLater(() -> { - Stage stage = unlockWindowBuilder.vault(vault).build().showUnlockWindow(); - addVisibleStage(stage); + unlockWindowBuilder.vault(vault).build().showUnlockWindow(); LOG.debug("Showing UnlockWindow for {}", vault.getDisplayableName()); }); } public void showQuitWindow(QuitResponse response) { Platform.runLater(() -> { - Stage stage = quitWindowBuilder.quitResponse(response).build().showQuitWindow(); - addVisibleStage(stage); + quitWindowBuilder.quitResponse(response).build().showQuitWindow(); LOG.debug("Showing QuitWindow"); }); } diff --git a/main/ui/src/main/java/org/cryptomator/ui/fxapp/FxApplicationModule.java b/main/ui/src/main/java/org/cryptomator/ui/fxapp/FxApplicationModule.java index 4fd18202d..65801b1d0 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/fxapp/FxApplicationModule.java +++ b/main/ui/src/main/java/org/cryptomator/ui/fxapp/FxApplicationModule.java @@ -11,10 +11,14 @@ import dagger.Provides; import javafx.application.Application; import javafx.beans.property.ObjectProperty; import javafx.beans.property.SimpleObjectProperty; +import javafx.collections.FXCollections; +import javafx.collections.ObservableSet; import javafx.scene.image.Image; +import javafx.stage.Stage; import org.apache.commons.lang3.SystemUtils; import org.cryptomator.common.vaults.Vault; import org.cryptomator.ui.common.ErrorComponent; +import org.cryptomator.ui.common.StageFactory; import org.cryptomator.ui.mainwindow.MainWindowComponent; import org.cryptomator.ui.preferences.PreferencesComponent; import org.cryptomator.ui.quit.QuitComponent; @@ -36,6 +40,12 @@ abstract class FxApplicationModule { return new SimpleObjectProperty<>(); } + @Provides + @FxApplicationScoped + static ObservableSet provideVisibleStages() { + return FXCollections.observableSet(); + } + @Provides @Named("windowIcons") @FxApplicationScoped @@ -43,7 +53,6 @@ abstract class FxApplicationModule { if (SystemUtils.IS_OS_MAC) { return Collections.emptyList(); } - try { return List.of( // createImageFromResource("/window_icon_32.png"), // @@ -53,6 +62,21 @@ abstract class FxApplicationModule { throw new UncheckedIOException("Failed to load embedded resource.", e); } } + + @Provides + @FxApplicationScoped + static StageFactory provideStageFactory(@Named("windowIcons") List windowIcons, ObservableSet visibleStages) { + return new StageFactory(stage -> { + stage.getIcons().addAll(windowIcons); + stage.showingProperty().addListener((observableValue, wasShowing, isShowing) -> { + if (isShowing) { + visibleStages.add(stage); + } else { + visibleStages.remove(stage); + } + }); + }); + } private static Image createImageFromResource(String resourceName) throws IOException { try (InputStream in = FxApplicationModule.class.getResourceAsStream(resourceName)) { diff --git a/main/ui/src/main/java/org/cryptomator/ui/mainwindow/MainWindowModule.java b/main/ui/src/main/java/org/cryptomator/ui/mainwindow/MainWindowModule.java index 04a047424..1b9ad08d8 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/mainwindow/MainWindowModule.java +++ b/main/ui/src/main/java/org/cryptomator/ui/mainwindow/MainWindowModule.java @@ -14,6 +14,7 @@ import org.cryptomator.ui.common.FxController; import org.cryptomator.ui.common.FxControllerKey; import org.cryptomator.ui.common.FxmlFile; import org.cryptomator.ui.common.FxmlScene; +import org.cryptomator.ui.common.StageFactory; import org.cryptomator.ui.migration.MigrationComponent; import org.cryptomator.ui.removevault.RemoveVaultComponent; import org.cryptomator.ui.vaultoptions.VaultOptionsComponent; @@ -38,15 +39,14 @@ abstract class MainWindowModule { @Provides @MainWindow @MainWindowScoped - static Stage provideStage(@Named("windowIcons") List windowIcons) { - Stage stage = new Stage(StageStyle.UNDECORATED); + static Stage provideStage(StageFactory factory) { + Stage stage = factory.create(StageStyle.UNDECORATED); // TODO: min/max values chosen arbitrarily. We might wanna take a look at the user's resolution... stage.setMinWidth(650); stage.setMinHeight(440); stage.setMaxWidth(1000); stage.setMaxHeight(700); stage.setTitle("Cryptomator"); - stage.getIcons().addAll(windowIcons); return stage; } diff --git a/main/ui/src/main/java/org/cryptomator/ui/migration/MigrationModule.java b/main/ui/src/main/java/org/cryptomator/ui/migration/MigrationModule.java index 0c36818a2..79803f861 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/migration/MigrationModule.java +++ b/main/ui/src/main/java/org/cryptomator/ui/migration/MigrationModule.java @@ -17,6 +17,7 @@ import org.cryptomator.ui.common.FxController; import org.cryptomator.ui.common.FxControllerKey; import org.cryptomator.ui.common.FxmlFile; import org.cryptomator.ui.common.FxmlScene; +import org.cryptomator.ui.common.StageFactory; import org.cryptomator.ui.mainwindow.MainWindow; import javax.inject.Named; @@ -38,13 +39,12 @@ abstract class MigrationModule { @Provides @MigrationWindow @MigrationScoped - static Stage provideStage(@MainWindow Stage owner, ResourceBundle resourceBundle, @Named("windowIcons") List windowIcons) { - Stage stage = new Stage(); + static Stage provideStage(StageFactory factory, @MainWindow Stage owner, ResourceBundle resourceBundle) { + Stage stage = factory.create(); stage.setTitle(resourceBundle.getString("migration.title")); stage.setResizable(false); stage.initModality(Modality.WINDOW_MODAL); stage.initOwner(owner); - stage.getIcons().addAll(windowIcons); return stage; } diff --git a/main/ui/src/main/java/org/cryptomator/ui/preferences/PreferencesModule.java b/main/ui/src/main/java/org/cryptomator/ui/preferences/PreferencesModule.java index b9438e35c..7a9dc2027 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/preferences/PreferencesModule.java +++ b/main/ui/src/main/java/org/cryptomator/ui/preferences/PreferencesModule.java @@ -15,6 +15,7 @@ import org.cryptomator.ui.common.FxController; import org.cryptomator.ui.common.FxControllerKey; import org.cryptomator.ui.common.FxmlFile; import org.cryptomator.ui.common.FxmlScene; +import org.cryptomator.ui.common.StageFactory; import javax.inject.Named; import javax.inject.Provider; @@ -41,11 +42,10 @@ abstract class PreferencesModule { @Provides @PreferencesWindow @PreferencesScoped - static Stage provideStage(ResourceBundle resourceBundle, @Named("windowIcons") List windowIcons) { - Stage stage = new Stage(); + static Stage provideStage(StageFactory factory, ResourceBundle resourceBundle) { + Stage stage = factory.create(); stage.setTitle(resourceBundle.getString("preferences.title")); stage.setResizable(false); - stage.getIcons().addAll(windowIcons); return stage; } diff --git a/main/ui/src/main/java/org/cryptomator/ui/quit/QuitModule.java b/main/ui/src/main/java/org/cryptomator/ui/quit/QuitModule.java index afdcb78a9..d79cabfcb 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/quit/QuitModule.java +++ b/main/ui/src/main/java/org/cryptomator/ui/quit/QuitModule.java @@ -17,6 +17,7 @@ import org.cryptomator.ui.common.FxController; import org.cryptomator.ui.common.FxControllerKey; import org.cryptomator.ui.common.FxmlFile; import org.cryptomator.ui.common.FxmlScene; +import org.cryptomator.ui.common.StageFactory; import javax.inject.Named; import javax.inject.Provider; @@ -37,12 +38,11 @@ abstract class QuitModule { @Provides @QuitWindow @QuitScoped - static Stage provideStage(@Named("windowIcons") List windowIcons) { - Stage stage = new Stage(); + static Stage provideStage(StageFactory factory) { + Stage stage = factory.create(); stage.setMinWidth(300); stage.setMinHeight(100); stage.initModality(Modality.APPLICATION_MODAL); - stage.getIcons().addAll(windowIcons); return stage; } diff --git a/main/ui/src/main/java/org/cryptomator/ui/recoverykey/RecoveryKeyModule.java b/main/ui/src/main/java/org/cryptomator/ui/recoverykey/RecoveryKeyModule.java index 3926f14ae..48e81f3a5 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/recoverykey/RecoveryKeyModule.java +++ b/main/ui/src/main/java/org/cryptomator/ui/recoverykey/RecoveryKeyModule.java @@ -21,6 +21,7 @@ import org.cryptomator.ui.common.FxmlFile; import org.cryptomator.ui.common.FxmlScene; import org.cryptomator.ui.common.NewPasswordController; import org.cryptomator.ui.common.PasswordStrengthUtil; +import org.cryptomator.ui.common.StageFactory; import javax.inject.Named; import javax.inject.Provider; @@ -41,13 +42,12 @@ abstract class RecoveryKeyModule { @Provides @RecoveryKeyWindow @RecoveryKeyScoped - static Stage provideStage(ResourceBundle resourceBundle, @Named("windowIcons") List windowIcons, @Named("keyRecoveryOwner") Stage owner) { - Stage stage = new Stage(); + static Stage provideStage(StageFactory factory, ResourceBundle resourceBundle, @Named("keyRecoveryOwner") Stage owner) { + Stage stage = factory.create(); stage.setTitle(resourceBundle.getString("recoveryKey.title")); stage.setResizable(false); stage.initModality(Modality.WINDOW_MODAL); stage.initOwner(owner); - stage.getIcons().addAll(windowIcons); return stage; } diff --git a/main/ui/src/main/java/org/cryptomator/ui/removevault/RemoveVaultModule.java b/main/ui/src/main/java/org/cryptomator/ui/removevault/RemoveVaultModule.java index 2bf44b106..5f61ec7ed 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/removevault/RemoveVaultModule.java +++ b/main/ui/src/main/java/org/cryptomator/ui/removevault/RemoveVaultModule.java @@ -17,6 +17,7 @@ import org.cryptomator.ui.common.FxController; import org.cryptomator.ui.common.FxControllerKey; import org.cryptomator.ui.common.FxmlFile; import org.cryptomator.ui.common.FxmlScene; +import org.cryptomator.ui.common.StageFactory; import org.cryptomator.ui.mainwindow.MainWindow; import javax.inject.Named; @@ -38,13 +39,12 @@ abstract class RemoveVaultModule { @Provides @RemoveVaultWindow @RemoveVaultScoped - static Stage provideStage(@MainWindow Stage owner, ResourceBundle resourceBundle, @Named("windowIcons") List windowIcons) { - Stage stage = new Stage(); + static Stage provideStage(StageFactory factory, @MainWindow Stage owner, ResourceBundle resourceBundle) { + Stage stage = factory.create(); stage.setTitle(resourceBundle.getString("removeVault.title")); stage.setResizable(false); stage.initModality(Modality.WINDOW_MODAL); stage.initOwner(owner); - stage.getIcons().addAll(windowIcons); return stage; } 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 a6e5202d8..0bc238f22 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 @@ -15,6 +15,7 @@ import org.cryptomator.ui.common.FxController; import org.cryptomator.ui.common.FxControllerKey; import org.cryptomator.ui.common.FxmlFile; import org.cryptomator.ui.common.FxmlScene; +import org.cryptomator.ui.common.StageFactory; import org.cryptomator.ui.forgetPassword.ForgetPasswordComponent; import javax.inject.Named; @@ -36,12 +37,11 @@ abstract class UnlockModule { @Provides @UnlockWindow @UnlockScoped - static Stage provideStage(@UnlockWindow Vault vault, @Named("windowIcons") List windowIcons) { - Stage stage = new Stage(); + static Stage provideStage(StageFactory factory, @UnlockWindow Vault vault) { + Stage stage = factory.create(); stage.setTitle(vault.getDisplayableName()); stage.setResizable(false); stage.initModality(Modality.APPLICATION_MODAL); - stage.getIcons().addAll(windowIcons); return stage; } diff --git a/main/ui/src/main/java/org/cryptomator/ui/vaultoptions/VaultOptionsModule.java b/main/ui/src/main/java/org/cryptomator/ui/vaultoptions/VaultOptionsModule.java index 639f5b36b..d0aa49281 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/vaultoptions/VaultOptionsModule.java +++ b/main/ui/src/main/java/org/cryptomator/ui/vaultoptions/VaultOptionsModule.java @@ -16,6 +16,7 @@ import org.cryptomator.ui.common.FxController; import org.cryptomator.ui.common.FxControllerKey; import org.cryptomator.ui.common.FxmlFile; import org.cryptomator.ui.common.FxmlScene; +import org.cryptomator.ui.common.StageFactory; import org.cryptomator.ui.mainwindow.MainWindow; import org.cryptomator.ui.recoverykey.RecoveryKeyComponent; @@ -38,15 +39,14 @@ abstract class VaultOptionsModule { @Provides @VaultOptionsWindow @VaultOptionsScoped - static Stage provideStage(@MainWindow Stage owner, @VaultOptionsWindow Vault vault, ResourceBundle resourceBundle, @Named("windowIcons") List windowIcons) { - Stage stage = new Stage(); + static Stage provideStage(StageFactory factory, @MainWindow Stage owner, @VaultOptionsWindow Vault vault) { + Stage stage = factory.create(); stage.setTitle(vault.getDisplayableName()); stage.setResizable(true); stage.setMinWidth(400); stage.setMinHeight(300); stage.initModality(Modality.WINDOW_MODAL); stage.initOwner(owner); - stage.getIcons().addAll(windowIcons); return stage; } diff --git a/main/ui/src/main/java/org/cryptomator/ui/wrongfilealert/WrongFileAlertModule.java b/main/ui/src/main/java/org/cryptomator/ui/wrongfilealert/WrongFileAlertModule.java index 7c64a079d..cca3b0269 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/wrongfilealert/WrongFileAlertModule.java +++ b/main/ui/src/main/java/org/cryptomator/ui/wrongfilealert/WrongFileAlertModule.java @@ -14,6 +14,7 @@ import org.cryptomator.ui.common.FxController; import org.cryptomator.ui.common.FxControllerKey; import org.cryptomator.ui.common.FxmlFile; import org.cryptomator.ui.common.FxmlScene; +import org.cryptomator.ui.common.StageFactory; import org.cryptomator.ui.mainwindow.MainWindow; import javax.inject.Named; @@ -35,13 +36,12 @@ abstract class WrongFileAlertModule { @Provides @WrongFileAlertWindow @WrongFileAlertScoped - static Stage provideStage(@MainWindow Stage mainWindow, ResourceBundle resourceBundle, @Named("windowIcons") List windowIcons) { - Stage stage = new Stage(); + static Stage provideStage(StageFactory factory, @MainWindow Stage mainWindow, ResourceBundle resourceBundle) { + Stage stage = factory.create(); stage.setTitle(resourceBundle.getString("wrongFileAlert.title")); stage.setResizable(false); stage.initOwner(mainWindow); stage.initModality(Modality.WINDOW_MODAL); - stage.getIcons().addAll(windowIcons); return stage; } From fecf9c04236267757fe4ceaec6e563849156c544 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Thu, 7 May 2020 14:18:09 +0200 Subject: [PATCH 02/17] Fixes #1088 --- .../AddVaultSuccessController.java | 2 +- .../ui/common/UserInteractionLock.java | 51 +++++ .../cryptomator/ui/common/VaultService.java | 166 ---------------- .../ui/controls/NiceSecurePasswordField.java | 4 +- .../ui/controls/SecurePasswordField.java | 20 +- .../cryptomator/ui/fxapp/FxApplication.java | 17 +- .../cryptomator/ui/launcher/UiLauncher.java | 6 +- .../VaultDetailLockedController.java | 2 +- .../ui/migration/MigrationRunController.java | 2 +- .../migration/MigrationSuccessController.java | 5 +- .../ui/traymenu/TrayMenuController.java | 2 +- .../ui/unlock/UnlockComponent.java | 22 ++- .../ui/unlock/UnlockController.java | 158 +++++----------- .../cryptomator/ui/unlock/UnlockModule.java | 49 +++++ .../cryptomator/ui/unlock/UnlockWorkflow.java | 179 ++++++++++++++++++ main/ui/src/main/resources/fxml/unlock.fxml | 8 +- .../ui/controls/SecurePasswordFieldTest.java | 2 +- 17 files changed, 375 insertions(+), 320 deletions(-) create mode 100644 main/ui/src/main/java/org/cryptomator/ui/common/UserInteractionLock.java create mode 100644 main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java diff --git a/main/ui/src/main/java/org/cryptomator/ui/addvaultwizard/AddVaultSuccessController.java b/main/ui/src/main/java/org/cryptomator/ui/addvaultwizard/AddVaultSuccessController.java index 88862894e..08ae84369 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/addvaultwizard/AddVaultSuccessController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/addvaultwizard/AddVaultSuccessController.java @@ -27,7 +27,7 @@ public class AddVaultSuccessController implements FxController { @FXML public void unlockAndClose() { close(); - fxApplication.showUnlockWindow(vault.get()); + fxApplication.startUnlockWorkflow(vault.get()); } @FXML diff --git a/main/ui/src/main/java/org/cryptomator/ui/common/UserInteractionLock.java b/main/ui/src/main/java/org/cryptomator/ui/common/UserInteractionLock.java new file mode 100644 index 000000000..91259ce8e --- /dev/null +++ b/main/ui/src/main/java/org/cryptomator/ui/common/UserInteractionLock.java @@ -0,0 +1,51 @@ +package org.cryptomator.ui.common; + +import javafx.application.Platform; +import javafx.beans.property.BooleanProperty; +import javafx.beans.property.ReadOnlyBooleanProperty; +import javafx.beans.property.SimpleBooleanProperty; + +import java.util.concurrent.locks.Condition; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; + +public class UserInteractionLock { + + private final Lock lock = new ReentrantLock(); + private final Condition condition = lock.newCondition(); + private final BooleanProperty awaitingInteraction = new SimpleBooleanProperty(); + private volatile E state; + + public UserInteractionLock(E initialValue) { + state = initialValue; + } + + public void interacted(E result) { + assert Platform.isFxApplicationThread(); + lock.lock(); + try { + state = result; + awaitingInteraction.set(false); + condition.signal(); + } finally { + lock.unlock(); + } + } + + public E awaitInteraction() throws InterruptedException { + assert !Platform.isFxApplicationThread(); + lock.lock(); + try { + Platform.runLater(() -> awaitingInteraction.set(true)); + condition.await(); + return state; + } finally { + lock.unlock(); + } + } + + public ReadOnlyBooleanProperty awaitingInteraction() { + return awaitingInteraction; + } + +} 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 b215c0c36..c96202dda 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 @@ -52,62 +52,6 @@ public class VaultService { return task; } - /** - * Attempts to unlock all given vaults in a background thread using passwords stored in the system keychain. - * - * @param vaults The vaults to unlock - * @implNote No-op if no system keychain is present - */ - public void attemptAutoUnlock(Collection vaults) { - if (!keychain.isPresent()) { - LOG.debug("No system keychain found. Unable to auto unlock without saved passwords."); - } else { - List> unlockTasks = vaults.stream().map(v -> createAutoUnlockTask(v, keychain.get())).collect(Collectors.toList()); - Task> runSequentiallyTask = new RunSequentiallyTask(unlockTasks); - executorService.execute(runSequentiallyTask); - } - } - - /** - * Creates but doesn't start an auto-unlock task. - * - * @param vault The vault to unlock - * @param keychainAccess The system keychain holding the passphrase for the vault - * @return The task - */ - public Task createAutoUnlockTask(Vault vault, KeychainAccess keychainAccess) { - Task task = new AutoUnlockVaultTask(vault, keychainAccess); - task.setOnSucceeded(evt -> LOG.info("Auto-unlocked {}", vault.getDisplayableName())); - task.setOnFailed(evt -> LOG.error("Failed to auto-unlock " + vault.getDisplayableName(), evt.getSource().getException())); - return task; - } - - /** - * Unlocks a vault in a background thread - * - * @param vault The vault to unlock - * @param passphrase The password to use - wipe this param asap - * @implNote A copy of the passphrase will be made, which is wiped as soon as the task ran. - */ - public void unlock(Vault vault, CharSequence passphrase) { - executorService.execute(createUnlockTask(vault, passphrase)); - } - - /** - * Creates but doesn't start an unlock task. - * - * @param vault The vault to unlock - * @param passphrase The password to use - wipe this param asap - * @return The task - * @implNote A copy of the passphrase will be made, which is wiped as soon as the task ran. - */ - public Task createUnlockTask(Vault vault, CharSequence passphrase) { - Task task = new UnlockVaultTask(vault, passphrase); - task.setOnSucceeded(evt -> LOG.info("Unlocked {}", vault.getDisplayableName())); - task.setOnFailed(evt -> LOG.error("Failed to unlock " + vault.getDisplayableName(), evt.getSource().getException())); - return task; - } - /** * Locks a vault in a background thread. * @@ -209,116 +153,6 @@ public class VaultService { } } - /** - * A task that runs a list of tasks in their given order - */ - private static class RunSequentiallyTask extends Task> { - - private final List> tasks; - - public RunSequentiallyTask(List> tasks) { - this.tasks = List.copyOf(tasks); - } - - @Override - protected List call() throws ExecutionException, InterruptedException { - List completed = new ArrayList<>(); - for (Task task : tasks) { - task.run(); - Vault done = task.get(); - completed.add(done); - } - return completed; - } - } - - private static class AutoUnlockVaultTask extends Task { - - private final Vault vault; - private final KeychainAccess keychain; - - public AutoUnlockVaultTask(Vault vault, KeychainAccess keychain) { - this.vault = vault; - this.keychain = keychain; - } - - @Override - protected Vault call() throws Exception { - char[] storedPw = null; - try { - storedPw = keychain.loadPassphrase(vault.getId()); - if (storedPw == null) { - throw new InvalidPassphraseException(); - } - vault.unlock(CharBuffer.wrap(storedPw)); - } finally { - if (storedPw != null) { - Arrays.fill(storedPw, ' '); - } - } - return vault; - } - - @Override - protected void scheduled() { - vault.setState(VaultState.PROCESSING); - } - - @Override - protected void succeeded() { - vault.setState(VaultState.UNLOCKED); - } - - @Override - protected void failed() { - vault.setState(VaultState.LOCKED); - } - } - - private static class UnlockVaultTask extends Task { - - private final Vault vault; - private final CharBuffer passphrase; - - /** - * @param vault The vault to unlock - * @param passphrase The password to use - wipe this param asap - * @implNote A copy of the passphrase will be made, which is wiped as soon as the task ran. - */ - public UnlockVaultTask(Vault vault, CharSequence passphrase) { - this.vault = vault; - this.passphrase = CharBuffer.allocate(passphrase.length()); - for (int i = 0; i < passphrase.length(); i++) { - this.passphrase.put(i, passphrase.charAt(i)); - } - } - - @Override - protected Vault call() throws Exception { - try { - vault.unlock(passphrase); - } finally { - Arrays.fill(passphrase.array(), ' '); - } - return vault; - } - - @Override - protected void scheduled() { - vault.setState(VaultState.PROCESSING); - } - - @Override - protected void succeeded() { - vault.setState(VaultState.UNLOCKED); - } - - @Override - protected void failed() { - vault.setState(VaultState.LOCKED); - } - } - /** * A task that locks a vault */ diff --git a/main/ui/src/main/java/org/cryptomator/ui/controls/NiceSecurePasswordField.java b/main/ui/src/main/java/org/cryptomator/ui/controls/NiceSecurePasswordField.java index 497913e65..928cfc40e 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/controls/NiceSecurePasswordField.java +++ b/main/ui/src/main/java/org/cryptomator/ui/controls/NiceSecurePasswordField.java @@ -94,8 +94,8 @@ public class NiceSecurePasswordField extends StackPane { passwordField.setPassword(password); } - public void swipe() { - passwordField.swipe(); + public void wipe() { + passwordField.wipe(); } public void selectAll() { diff --git a/main/ui/src/main/java/org/cryptomator/ui/controls/SecurePasswordField.java b/main/ui/src/main/java/org/cryptomator/ui/controls/SecurePasswordField.java index f5aa6dbd5..ee8bda5bc 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/controls/SecurePasswordField.java +++ b/main/ui/src/main/java/org/cryptomator/ui/controls/SecurePasswordField.java @@ -40,7 +40,7 @@ import java.util.Arrays; */ public class SecurePasswordField extends TextField { - private static final char SWIPE_CHAR = ' '; + private static final char WIPE_CHAR = ' '; private static final int INITIAL_BUFFER_SIZE = 50; private static final int GROW_BUFFER_SIZE = 50; private static final String DEFAULT_PLACEHOLDER = "●"; @@ -103,7 +103,7 @@ public class SecurePasswordField extends TextField { if (e.getCode() == KeyCode.CAPS) { updateCapsLocked(); } else if (SHORTCUT_BACKSPACE.match(e)) { - swipe(); + wipe(); } } @@ -189,7 +189,7 @@ public class SecurePasswordField extends TextField { if (length > content.length) { char[] newContent = new char[length + GROW_BUFFER_SIZE]; System.arraycopy(content, 0, newContent, 0, content.length); - swipe(content); + wipe(content); this.content = newContent; } } @@ -201,7 +201,7 @@ public class SecurePasswordField extends TextField { * @implNote The CharSequence will not copy the backing char[]. * Therefore any mutation to the SecurePasswordField's content will mutate or eventually swipe the returned CharSequence. * @implSpec The CharSequence is usually in NFC representation (unless NFD-encoded char[] is set via {@link #setPassword(char[])}). - * @see #swipe() + * @see #wipe() */ @Override public CharSequence getCharacters() { @@ -220,7 +220,7 @@ public class SecurePasswordField extends TextField { buf[i] = password.charAt(i); } setPassword(buf); - Arrays.fill(buf, SWIPE_CHAR); + Arrays.fill(buf, WIPE_CHAR); } /** @@ -231,7 +231,7 @@ public class SecurePasswordField extends TextField { * @param password */ public void setPassword(char[] password) { - swipe(); + wipe(); content = Arrays.copyOf(password, password.length); length = password.length; @@ -242,14 +242,14 @@ public class SecurePasswordField extends TextField { /** * Destroys the stored password by overriding each character with a different character. */ - public void swipe() { - swipe(content); + public void wipe() { + wipe(content); length = 0; setText(null); } - private void swipe(char[] buffer) { - Arrays.fill(buffer, SWIPE_CHAR); + private void wipe(char[] buffer) { + Arrays.fill(buffer, WIPE_CHAR); } /* Observable Properties */ diff --git a/main/ui/src/main/java/org/cryptomator/ui/fxapp/FxApplication.java b/main/ui/src/main/java/org/cryptomator/ui/fxapp/FxApplication.java index 291e1a347..e0d1ecf93 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/fxapp/FxApplication.java +++ b/main/ui/src/main/java/org/cryptomator/ui/fxapp/FxApplication.java @@ -27,6 +27,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import javax.inject.Inject; +import javax.inject.Provider; import java.awt.desktop.QuitResponse; import java.util.Optional; @@ -38,20 +39,20 @@ public class FxApplication extends Application { private final Settings settings; private final Lazy mainWindow; private final Lazy preferencesWindow; - private final UnlockComponent.Builder unlockWindowBuilder; - private final QuitComponent.Builder quitWindowBuilder; + private final Provider unlockWindowBuilderProvider; + private final Provider quitWindowBuilderProvider; private final Optional macFunctions; private final VaultService vaultService; private final LicenseHolder licenseHolder; private final BooleanBinding hasVisibleStages; @Inject - FxApplication(Settings settings, Lazy mainWindow, Lazy preferencesWindow, UnlockComponent.Builder unlockWindowBuilder, QuitComponent.Builder quitWindowBuilder, Optional macFunctions, VaultService vaultService, LicenseHolder licenseHolder, ObservableSet visibleStages) { + FxApplication(Settings settings, Lazy mainWindow, Lazy preferencesWindow, Provider unlockWindowBuilderProvider, Provider quitWindowBuilderProvider, Optional macFunctions, VaultService vaultService, LicenseHolder licenseHolder, ObservableSet visibleStages) { this.settings = settings; this.mainWindow = mainWindow; this.preferencesWindow = preferencesWindow; - this.unlockWindowBuilder = unlockWindowBuilder; - this.quitWindowBuilder = quitWindowBuilder; + this.unlockWindowBuilderProvider = unlockWindowBuilderProvider; + this.quitWindowBuilderProvider = quitWindowBuilderProvider; this.macFunctions = macFunctions; this.vaultService = vaultService; this.licenseHolder = licenseHolder; @@ -95,16 +96,16 @@ public class FxApplication extends Application { }); } - public void showUnlockWindow(Vault vault) { + public void startUnlockWorkflow(Vault vault) { Platform.runLater(() -> { - unlockWindowBuilder.vault(vault).build().showUnlockWindow(); + unlockWindowBuilderProvider.get().vault(vault).build().startUnlockWorkflow(); LOG.debug("Showing UnlockWindow for {}", vault.getDisplayableName()); }); } public void showQuitWindow(QuitResponse response) { Platform.runLater(() -> { - quitWindowBuilder.quitResponse(response).build().showQuitWindow(); + quitWindowBuilderProvider.get().quitResponse(response).build().showQuitWindow(); LOG.debug("Showing QuitWindow"); }); } diff --git a/main/ui/src/main/java/org/cryptomator/ui/launcher/UiLauncher.java b/main/ui/src/main/java/org/cryptomator/ui/launcher/UiLauncher.java index f44071d86..7024de477 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/launcher/UiLauncher.java +++ b/main/ui/src/main/java/org/cryptomator/ui/launcher/UiLauncher.java @@ -64,7 +64,11 @@ public class UiLauncher { // auto unlock Collection vaultsWithAutoUnlockEnabled = vaults.filtered(v -> v.getVaultSettings().unlockAfterStartup().get()); if (!vaultsWithAutoUnlockEnabled.isEmpty()) { - fxApplicationStarter.get(hasTrayIcon).thenAccept(app -> app.getVaultService().attemptAutoUnlock(vaultsWithAutoUnlockEnabled)); + fxApplicationStarter.get(hasTrayIcon).thenAccept(app -> { + for (Vault vault : vaultsWithAutoUnlockEnabled){ + app.startUnlockWorkflow(vault); + } + }); } launchEventHandler.startHandlingLaunchEvents(hasTrayIcon); diff --git a/main/ui/src/main/java/org/cryptomator/ui/mainwindow/VaultDetailLockedController.java b/main/ui/src/main/java/org/cryptomator/ui/mainwindow/VaultDetailLockedController.java index f5b1d1d3a..434a5b7ef 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/mainwindow/VaultDetailLockedController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/mainwindow/VaultDetailLockedController.java @@ -26,7 +26,7 @@ public class VaultDetailLockedController implements FxController { @FXML public void unlock() { - application.showUnlockWindow(vault.get()); + application.startUnlockWorkflow(vault.get()); } @FXML 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 aad1914d7..05b460348 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 @@ -121,7 +121,7 @@ public class MigrationRunController implements FxController { } else { LOG.info("Migration of '{}' succeeded.", vault.getDisplayableName()); vault.setState(VaultState.LOCKED); - passwordField.swipe(); + passwordField.wipe(); window.setScene(successScene.get()); } }).onError(InvalidPassphraseException.class, e -> { diff --git a/main/ui/src/main/java/org/cryptomator/ui/migration/MigrationSuccessController.java b/main/ui/src/main/java/org/cryptomator/ui/migration/MigrationSuccessController.java index 4d31c377f..6a64e5509 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/migration/MigrationSuccessController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/migration/MigrationSuccessController.java @@ -1,8 +1,5 @@ package org.cryptomator.ui.migration; -import javafx.beans.property.ObjectProperty; -import javafx.beans.property.ReadOnlyObjectProperty; -import javafx.event.ActionEvent; import javafx.fxml.FXML; import javafx.stage.Stage; import org.cryptomator.common.vaults.Vault; @@ -28,7 +25,7 @@ public class MigrationSuccessController implements FxController { @FXML public void unlockAndClose() { close(); - fxApplication.showUnlockWindow(vault); + fxApplication.startUnlockWorkflow(vault); } @FXML diff --git a/main/ui/src/main/java/org/cryptomator/ui/traymenu/TrayMenuController.java b/main/ui/src/main/java/org/cryptomator/ui/traymenu/TrayMenuController.java index 55735f5b1..58c0f3bda 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/traymenu/TrayMenuController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/traymenu/TrayMenuController.java @@ -103,7 +103,7 @@ class TrayMenuController { } private void unlockVault(Vault vault) { - fxApplicationStarter.get(true).thenAccept(app -> app.showUnlockWindow(vault)); + fxApplicationStarter.get(true).thenAccept(app -> app.startUnlockWorkflow(vault)); } private void lockVault(Vault vault) { diff --git a/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockComponent.java b/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockComponent.java index 7c0d9f704..f321f5890 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockComponent.java +++ b/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockComponent.java @@ -14,21 +14,23 @@ import org.cryptomator.ui.common.FxmlFile; import org.cryptomator.ui.common.FxmlScene; import org.cryptomator.common.vaults.Vault; +import java.util.concurrent.Executor; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Future; +import java.util.concurrent.FutureTask; + @UnlockScoped @Subcomponent(modules = {UnlockModule.class}) public interface UnlockComponent { - @UnlockWindow - Stage window(); + ExecutorService defaultExecutorService(); - @FxmlScene(FxmlFile.UNLOCK) - Lazy scene(); - - default Stage showUnlockWindow() { - Stage stage = window(); - stage.setScene(scene().get()); - stage.show(); - return stage; + UnlockWorkflow unlockWorkflow(); + + default Future startUnlockWorkflow() { + UnlockWorkflow workflow = unlockWorkflow(); + defaultExecutorService().submit(workflow); + return workflow; } @Subcomponent.Builder 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 6f754b4c1..8aa39c77d 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 @@ -1,39 +1,29 @@ package org.cryptomator.ui.unlock; -import dagger.Lazy; import javafx.beans.binding.Bindings; +import javafx.beans.binding.BooleanBinding; import javafx.beans.binding.ObjectBinding; import javafx.beans.property.BooleanProperty; import javafx.beans.property.ReadOnlyBooleanProperty; import javafx.beans.property.SimpleBooleanProperty; -import javafx.concurrent.Task; import javafx.fxml.FXML; -import javafx.scene.Scene; import javafx.scene.control.CheckBox; import javafx.scene.control.ContentDisplay; import javafx.stage.Stage; import org.cryptomator.common.vaults.Vault; -import org.cryptomator.common.vaults.VaultState; -import org.cryptomator.cryptolib.api.InvalidPassphraseException; import org.cryptomator.keychain.KeychainAccess; -import org.cryptomator.keychain.KeychainAccessException; -import org.cryptomator.ui.common.Animations; -import org.cryptomator.ui.common.ErrorComponent; import org.cryptomator.ui.common.FxController; -import org.cryptomator.ui.common.FxmlFile; -import org.cryptomator.ui.common.FxmlScene; -import org.cryptomator.ui.common.VaultService; +import org.cryptomator.ui.common.UserInteractionLock; import org.cryptomator.ui.controls.NiceSecurePasswordField; import org.cryptomator.ui.forgetPassword.ForgetPasswordComponent; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import javax.inject.Inject; -import java.nio.file.DirectoryNotEmptyException; -import java.nio.file.NotDirectoryException; -import java.util.Arrays; +import javax.inject.Named; import java.util.Optional; -import java.util.concurrent.ExecutorService; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; @UnlockScoped public class UnlockController implements FxController { @@ -42,124 +32,67 @@ public class UnlockController implements FxController { private final Stage window; private final Vault vault; - private final ExecutorService executor; - private final ObjectBinding unlockButtonState; - private final Optional keychainAccess; - private final VaultService vaultService; - private final Lazy successScene; - private final Lazy invalidMountPointScene; - private final ErrorComponent.Builder errorComponent; + private final AtomicReference password; + private final AtomicBoolean savePassword; + private final Optional savedPassword; + private final UserInteractionLock passwordEntryLock; private final ForgetPasswordComponent.Builder forgetPassword; + private final Optional keychainAccess; + private final ObjectBinding unlockButtonContentDisplay; + private final BooleanBinding userInteractionDisabled; private final BooleanProperty unlockButtonDisabled; public NiceSecurePasswordField passwordField; - public CheckBox savePassword; + public CheckBox savePasswordCheckbox; @Inject - public UnlockController(@UnlockWindow Stage window, @UnlockWindow Vault vault, ExecutorService executor, Optional keychainAccess, VaultService vaultService, @FxmlScene(FxmlFile.UNLOCK_SUCCESS) Lazy successScene, @FxmlScene(FxmlFile.UNLOCK_INVALID_MOUNT_POINT) Lazy invalidMountPointScene, ErrorComponent.Builder errorComponent, ForgetPasswordComponent.Builder forgetPassword) { + 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) { this.window = window; this.vault = vault; - this.executor = executor; - this.unlockButtonState = Bindings.createObjectBinding(this::getUnlockButtonState, vault.stateProperty()); - this.keychainAccess = keychainAccess; - this.vaultService = vaultService; - this.successScene = successScene; - this.invalidMountPointScene = invalidMountPointScene; - this.errorComponent = errorComponent; + this.password = password; + this.savePassword = savePassword; + this.savedPassword = savedPassword; + this.passwordEntryLock = passwordEntryLock; this.forgetPassword = forgetPassword; + this.keychainAccess = keychainAccess; + this.unlockButtonContentDisplay = Bindings.createObjectBinding(this::getUnlockButtonContentDisplay, passwordEntryLock.awaitingInteraction()); + this.userInteractionDisabled = passwordEntryLock.awaitingInteraction().not(); this.unlockButtonDisabled = new SimpleBooleanProperty(); } public void initialize() { - if (keychainAccess.isPresent()) { - loadStoredPassword(); - } else { - savePassword.setSelected(false); + savePasswordCheckbox.setSelected(savedPassword.isPresent()); + if (password.get() != null) { + passwordField.setPassword(password.get()); } - unlockButtonDisabled.bind(vault.stateProperty().isNotEqualTo(VaultState.LOCKED).or(passwordField.textProperty().isEmpty())); + unlockButtonDisabled.bind(userInteractionDisabled.or(passwordField.textProperty().isEmpty())); } @FXML public void cancel() { LOG.debug("Unlock canceled by user."); window.close(); + passwordEntryLock.interacted(UnlockModule.PasswordEntry.CANCELED); } @FXML public void unlock() { LOG.trace("UnlockController.unlock()"); - CharSequence password = passwordField.getCharacters(); - - Task task = vaultService.createUnlockTask(vault, password); - passwordField.setDisable(true); - task.setOnSucceeded(event -> { - passwordField.setDisable(false); - if (keychainAccess.isPresent() && savePassword.isSelected()) { - try { - keychainAccess.get().storePassphrase(vault.getId(), password); - } catch (KeychainAccessException e) { - LOG.error("Failed to store passphrase in system keychain.", e); - } - } - passwordField.swipe(); - LOG.info("Unlock of '{}' succeeded.", vault.getDisplayableName()); - window.setScene(successScene.get()); - }); - task.setOnFailed(event -> { - passwordField.setDisable(false); - if (task.getException() instanceof InvalidPassphraseException) { - Animations.createShakeWindowAnimation(window).play(); - passwordField.selectAll(); - passwordField.requestFocus(); - } else if (task.getException() instanceof NotDirectoryException || task.getException() instanceof DirectoryNotEmptyException) { - LOG.error("Unlock failed. Mount point not an empty directory: {}", task.getException().getMessage()); - window.setScene(invalidMountPointScene.get()); - } else { - LOG.error("Unlock failed for technical reasons.", task.getException()); - errorComponent.cause(task.getException()).window(window).returnToScene(window.getScene()).build().showErrorScene(); - } - }); - executor.execute(task); + CharSequence pwFieldContents = passwordField.getCharacters(); + char[] pw = new char[pwFieldContents.length()]; + for (int i = 0; i < pwFieldContents.length(); i++) { + pw[i] = pwFieldContents.charAt(i); + } + password.set(pw); + passwordEntryLock.interacted(UnlockModule.PasswordEntry.PASSWORD_ENTERED); } /* Save Password */ @FXML private void didClickSavePasswordCheckbox() { - if (!savePassword.isSelected() && hasStoredPassword()) { - forgetPassword.vault(vault).owner(window).build().showForgetPassword().thenAccept(forgotten -> savePassword.setSelected(!forgotten)); - } - } - - private void loadStoredPassword() { - assert keychainAccess.isPresent(); - char[] storedPw = null; - try { - storedPw = keychainAccess.get().loadPassphrase(vault.getId()); - if (storedPw != null) { - savePassword.setSelected(true); - passwordField.setPassword(storedPw); - passwordField.selectRange(storedPw.length, storedPw.length); - } - } catch (KeychainAccessException e) { - LOG.error("Failed to load entry from system keychain.", e); - } finally { - if (storedPw != null) { - Arrays.fill(storedPw, ' '); - } - } - } - - private boolean hasStoredPassword() { - char[] storedPw = null; - try { - storedPw = keychainAccess.get().loadPassphrase(vault.getId()); - return storedPw != null; - } catch (KeychainAccessException e) { - return false; - } finally { - if (storedPw != null) { - Arrays.fill(storedPw, ' '); - } + savePassword.set(savePasswordCheckbox.isSelected()); + if (!savePasswordCheckbox.isSelected() && savedPassword.isPresent()) { + forgetPassword.vault(vault).owner(window).build().showForgetPassword().thenAccept(forgotten -> savePasswordCheckbox.setSelected(!forgotten)); } } @@ -169,15 +102,20 @@ public class UnlockController implements FxController { return vault; } - public ObjectBinding unlockButtonStateProperty() { - return unlockButtonState; + public ObjectBinding unlockButtonContentDisplayProperty() { + return unlockButtonContentDisplay; } - public ContentDisplay getUnlockButtonState() { - return switch (vault.getState()) { - case PROCESSING -> ContentDisplay.LEFT; - default -> ContentDisplay.TEXT_ONLY; - }; + public ContentDisplay getUnlockButtonContentDisplay() { + return passwordEntryLock.awaitingInteraction().get() ? ContentDisplay.TEXT_ONLY : ContentDisplay.LEFT; + } + + public BooleanBinding userInteractionDisabledProperty() { + return userInteractionDisabled; + } + + public boolean isUserInteractionDisabled() { + return userInteractionDisabled.get(); } public ReadOnlyBooleanProperty unlockButtonDisabledProperty() { 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 0bc238f22..4b1ba3cfe 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 @@ -9,6 +9,8 @@ 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.ui.common.DefaultSceneFactory; import org.cryptomator.ui.common.FXMLLoaderFactory; import org.cryptomator.ui.common.FxController; @@ -16,17 +18,64 @@ import org.cryptomator.ui.common.FxControllerKey; import org.cryptomator.ui.common.FxmlFile; import org.cryptomator.ui.common.FxmlScene; import org.cryptomator.ui.common.StageFactory; +import org.cryptomator.ui.common.UserInteractionLock; import org.cryptomator.ui.forgetPassword.ForgetPasswordComponent; +import org.slf4j.Logger; +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 { + private static final Logger LOG = LoggerFactory.getLogger(UnlockModule.class); + + public enum PasswordEntry {PASSWORD_ENTERED, CANCELED} + + @Provides + @UnlockScoped + static UserInteractionLock providePasswordEntryLock() { + return new UserInteractionLock<>(null); + } + + @Provides + @Named("savedPassword") + @UnlockScoped + static Optional provideStoredPassword(Optional keychainAccess, @UnlockWindow Vault vault) { + return keychainAccess.map(k -> { + try { + return k.loadPassphrase(vault.getId()); + } catch (KeychainAccessException e) { + LOG.error("Failed to load entry from system keychain.", e); + return null; + } + }); + } + + @Provides + @UnlockScoped + static AtomicReference providePassword(@Named("savedPassword") Optional storedPassword) { + return new AtomicReference(storedPassword.orElse(null)); + } + + @Provides + @Named("savePassword") + @UnlockScoped + static AtomicBoolean provideSavePasswordFlag(@Named("savedPassword") Optional storedPassword) { + return new AtomicBoolean(storedPassword.isPresent()); + } + @Provides @UnlockWindow @UnlockScoped 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 new file mode 100644 index 000000000..3828a1c24 --- /dev/null +++ b/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java @@ -0,0 +1,179 @@ +package org.cryptomator.ui.unlock; + +import dagger.Lazy; +import javafx.application.Platform; +import javafx.concurrent.Task; +import javafx.scene.Scene; +import javafx.stage.Stage; +import org.cryptomator.common.vaults.Vault; +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.ui.common.Animations; +import org.cryptomator.ui.common.ErrorComponent; +import org.cryptomator.ui.common.FxmlFile; +import org.cryptomator.ui.common.FxmlScene; +import org.cryptomator.ui.common.UserInteractionLock; +import org.cryptomator.ui.unlock.UnlockModule.PasswordEntry; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javax.inject.Inject; +import javax.inject.Named; +import java.io.IOException; +import java.nio.CharBuffer; +import java.nio.file.DirectoryNotEmptyException; +import java.nio.file.FileSystemException; +import java.nio.file.NotDirectoryException; +import java.util.Arrays; +import java.util.Optional; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; + +/** + * A multi-step task that consists of background activities as well as user interaction. + *

+ * This class runs the unlock process and controls when to display which UI. + */ +@UnlockScoped +public class UnlockWorkflow extends Task { + + private static final Logger LOG = LoggerFactory.getLogger(UnlockWorkflow.class); + + private final Stage window; + private final Vault vault; + private final AtomicReference password; + private final AtomicBoolean savePassword; + private final Optional savedPassword; + private final UserInteractionLock passwordEntryLock; + 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, 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.password = password; + this.savePassword = savePassword; + this.savedPassword = savedPassword; + this.passwordEntryLock = passwordEntryLock; + this.keychain = keychain; + this.unlockScene = unlockScene; + this.successScene = successScene; + this.invalidMountPointScene = invalidMountPointScene; + this.errorComponent = errorComponent; + } + + @Override + protected Boolean call() throws InterruptedException, IOException, Volume.VolumeException { + try { + if (attemptUnlock()) { + handleSuccess(); + return true; + } else { + cancel(false); // set Tasks state to cancelled + return false; + } + } catch (NotDirectoryException | DirectoryNotEmptyException e) { + handleInvalidMountPoint(e); + throw e; // rethrow to trigger correct exception handling in Task + } catch (CryptoException | Volume.VolumeException | IOException e) { + handleGenericError(e); + throw e; // rethrow to trigger correct exception handling in Task + } finally { + wipePassword(password.get()); + wipePassword(savedPassword.orElse(null)); + } + } + + private boolean attemptUnlock() throws InterruptedException, IOException, Volume.VolumeException { + boolean proceed = password.get() != null || askForPassword(false) == PasswordEntry.PASSWORD_ENTERED; + while (proceed) { + try { + vault.unlock(CharBuffer.wrap(password.get())); + return true; + } catch (InvalidPassphraseException e) { + proceed = askForPassword(true) == PasswordEntry.PASSWORD_ENTERED; + } + } + return false; + } + + private PasswordEntry askForPassword(boolean animateShake) throws InterruptedException { + Platform.runLater(() -> { + window.setScene(unlockScene.get()); + window.show(); + if (animateShake) { + Animations.createShakeWindowAnimation(window).play(); + } + }); + return passwordEntryLock.awaitInteraction(); + } + + private void handleSuccess() { + LOG.info("Unlock of '{}' succeeded.", vault.getDisplayableName()); + if (savePassword.get()) { + savePasswordToSystemkeychain(); + } + Platform.runLater(() -> { + window.setScene(successScene.get()); // TODO only if enabled (see issue #1083) + }); + } + + private void savePasswordToSystemkeychain() { + if (keychain.isPresent()) { + try { + keychain.get().storePassphrase(vault.getId(), CharBuffer.wrap(password.get())); + } catch (KeychainAccessException e) { + LOG.error("Failed to store passphrase in system keychain.", e); + } + } + } + + private void handleInvalidMountPoint(FileSystemException e) { + LOG.error("Unlock failed. Mount point not an empty directory: {}", e.getMessage()); + Platform.runLater(() -> { + window.setScene(invalidMountPointScene.get()); + }); + } + + private void handleGenericError(Exception e) { + LOG.error("Unlock failed for technical reasons.", e); + Platform.runLater(() -> { + errorComponent.cause(e).window(window).returnToScene(window.getScene()).build().showErrorScene(); + }); + } + + private void wipePassword(char[] pw) { + if (pw != null) { + Arrays.fill(pw, ' '); + } + } + + @Override + protected void scheduled() { + vault.setState(VaultState.PROCESSING); + } + + @Override + protected void succeeded() { + vault.setState(VaultState.UNLOCKED); + } + + @Override + protected void failed() { + vault.setState(VaultState.LOCKED); + } + + @Override + protected void cancelled() { + vault.setState(VaultState.LOCKED); + } + +} diff --git a/main/ui/src/main/resources/fxml/unlock.fxml b/main/ui/src/main/resources/fxml/unlock.fxml index e2b2217ff..099a04148 100644 --- a/main/ui/src/main/resources/fxml/unlock.fxml +++ b/main/ui/src/main/resources/fxml/unlock.fxml @@ -21,15 +21,15 @@ - - + + -