From 0bece0f59124231176473c790fdb157e635c079c Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Thu, 16 Dec 2021 13:56:59 +0100 Subject: [PATCH 01/26] move passphrase entry to subcomponent and use CompletableFuture instead of UserInteractionLock + AtomicReference --- .../org/cryptomator/ui/common/FxmlFile.java | 2 +- .../ui/keyloading/KeyLoadingModule.java | 2 +- .../MasterkeyFileLoadingFinisher.java | 62 -------------- .../MasterkeyFileLoadingModule.java | 39 +-------- .../MasterkeyFileLoadingStrategy.java | 81 ++++++++++++------- .../PassphraseEntryComponent.java | 35 ++++++++ .../PassphraseEntryController.java | 80 +++++++----------- .../masterkeyfile/PassphraseEntryModule.java | 41 ++++++++++ .../masterkeyfile/PassphraseEntryResult.java | 6 ++ .../masterkeyfile/PassphraseEntryScoped.java | 13 +++ 10 files changed, 179 insertions(+), 182 deletions(-) delete mode 100644 src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/MasterkeyFileLoadingFinisher.java create mode 100644 src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryComponent.java create mode 100644 src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryModule.java create mode 100644 src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryResult.java create mode 100644 src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryScoped.java diff --git a/src/main/java/org/cryptomator/ui/common/FxmlFile.java b/src/main/java/org/cryptomator/ui/common/FxmlFile.java index b8d5bbff0..bc952f9d1 100644 --- a/src/main/java/org/cryptomator/ui/common/FxmlFile.java +++ b/src/main/java/org/cryptomator/ui/common/FxmlFile.java @@ -42,7 +42,7 @@ public enum FxmlFile { this.ressourcePathString = ressourcePathString; } - String getRessourcePathString() { + public String getRessourcePathString() { return ressourcePathString; } } diff --git a/src/main/java/org/cryptomator/ui/keyloading/KeyLoadingModule.java b/src/main/java/org/cryptomator/ui/keyloading/KeyLoadingModule.java index bff757b1a..616e7e5e0 100644 --- a/src/main/java/org/cryptomator/ui/keyloading/KeyLoadingModule.java +++ b/src/main/java/org/cryptomator/ui/keyloading/KeyLoadingModule.java @@ -26,7 +26,7 @@ abstract class KeyLoadingModule { @Provides @KeyLoading @KeyLoadingScoped - static KeyLoadingStrategy provideKeyLoaderProvider(@KeyLoading Vault vault, Map> strategies) { + static KeyLoadingStrategy provideKeyLoadingStrategy(@KeyLoading Vault vault, Map> strategies) { try { String scheme = vault.getVaultConfigCache().get().getKeyId().getScheme(); var fallback = KeyLoadingStrategy.failed(new IllegalArgumentException("Unsupported key id " + scheme)); diff --git a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/MasterkeyFileLoadingFinisher.java b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/MasterkeyFileLoadingFinisher.java deleted file mode 100644 index 44d7ebfb0..000000000 --- a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/MasterkeyFileLoadingFinisher.java +++ /dev/null @@ -1,62 +0,0 @@ -package org.cryptomator.ui.keyloading.masterkeyfile; - -import org.cryptomator.common.keychain.KeychainManager; -import org.cryptomator.common.vaults.Vault; -import org.cryptomator.integrations.keychain.KeychainAccessException; -import org.cryptomator.ui.keyloading.KeyLoading; -import org.cryptomator.ui.keyloading.KeyLoadingScoped; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import javax.inject.Inject; -import javax.inject.Named; -import java.nio.CharBuffer; -import java.util.Arrays; -import java.util.Optional; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicReference; - -@KeyLoadingScoped -class MasterkeyFileLoadingFinisher { - - private static final Logger LOG = LoggerFactory.getLogger(MasterkeyFileLoadingFinisher.class); - - private final Vault vault; - private final Optional storedPassword; - private final AtomicReference enteredPassword; - private final AtomicBoolean shouldSavePassword; - private final KeychainManager keychain; - - @Inject - MasterkeyFileLoadingFinisher(@KeyLoading Vault vault, @Named("savedPassword") Optional storedPassword, AtomicReference enteredPassword, @Named("savePassword") AtomicBoolean shouldSavePassword, KeychainManager keychain) { - this.vault = vault; - this.storedPassword = storedPassword; - this.enteredPassword = enteredPassword; - this.shouldSavePassword = shouldSavePassword; - this.keychain = keychain; - } - - public void cleanup(boolean successfullyUnlocked) { - if (successfullyUnlocked && shouldSavePassword.get()) { - savePasswordToSystemkeychain(); - } - wipePassword(storedPassword.orElse(null)); - wipePassword(enteredPassword.getAndSet(null)); - } - - private void savePasswordToSystemkeychain() { - if (keychain.isSupported()) { - try { - keychain.storePassphrase(vault.getId(), vault.getDisplayName(), CharBuffer.wrap(enteredPassword.get())); - } catch (KeychainAccessException e) { - LOG.error("Failed to store passphrase in system keychain.", e); - } - } - } - - private void wipePassword(char[] pw) { - if (pw != null) { - Arrays.fill(pw, ' '); - } - } -} 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 901eacfb9..7f55de898 100644 --- a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/MasterkeyFileLoadingModule.java +++ b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/MasterkeyFileLoadingModule.java @@ -25,30 +25,18 @@ import javax.inject.Named; import javafx.scene.Scene; import java.nio.file.Path; import java.util.Optional; -import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; -@Module(subcomponents = {ForgetPasswordComponent.class}) +@Module(subcomponents = {ForgetPasswordComponent.class, PassphraseEntryComponent.class}) public abstract class MasterkeyFileLoadingModule { private static final Logger LOG = LoggerFactory.getLogger(MasterkeyFileLoadingModule.class); - public enum PasswordEntry { - PASSWORD_ENTERED, - CANCELED - } - public enum MasterkeyFileProvision { MASTERKEYFILE_PROVIDED, CANCELED } - @Provides - @KeyLoadingScoped - static UserInteractionLock providePasswordEntryLock() { - return new UserInteractionLock<>(null); - } - @Provides @KeyLoadingScoped static UserInteractionLock provideMasterkeyFileProvisionLock() { @@ -77,26 +65,6 @@ public abstract class MasterkeyFileLoadingModule { return new AtomicReference<>(); } - @Provides - @KeyLoadingScoped - static AtomicReference providePassword(@Named("savedPassword") Optional storedPassword) { - return new AtomicReference<>(storedPassword.orElse(null)); - } - - @Provides - @Named("savePassword") - @KeyLoadingScoped - static AtomicBoolean provideSavePasswordFlag(@Named("savedPassword") Optional storedPassword) { - return new AtomicBoolean(storedPassword.isPresent()); - } - - @Provides - @FxmlScene(FxmlFile.UNLOCK_ENTER_PASSWORD) - @KeyLoadingScoped - static Scene provideUnlockScene(@KeyLoading FxmlLoaderFactory fxmlLoaders) { - return fxmlLoaders.createScene(FxmlFile.UNLOCK_ENTER_PASSWORD); - } - @Provides @FxmlScene(FxmlFile.UNLOCK_SELECT_MASTERKEYFILE) @KeyLoadingScoped @@ -104,11 +72,6 @@ public abstract class MasterkeyFileLoadingModule { return fxmlLoaders.createScene(FxmlFile.UNLOCK_SELECT_MASTERKEYFILE); } - @Binds - @IntoMap - @FxControllerKey(PassphraseEntryController.class) - abstract FxController bindUnlockController(PassphraseEntryController controller); - @Binds @IntoMap @FxControllerKey(SelectMasterkeyFileController.class) 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 1fa7dd986..2ee17a744 100644 --- a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/MasterkeyFileLoadingStrategy.java +++ b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/MasterkeyFileLoadingStrategy.java @@ -2,12 +2,14 @@ package org.cryptomator.ui.keyloading.masterkeyfile; import com.google.common.base.Preconditions; import dagger.Lazy; +import org.cryptomator.common.keychain.KeychainManager; import org.cryptomator.common.vaults.Vault; import org.cryptomator.cryptofs.common.BackupHelper; import org.cryptomator.cryptolib.api.InvalidPassphraseException; import org.cryptomator.cryptolib.api.Masterkey; 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; @@ -17,6 +19,7 @@ import org.cryptomator.ui.keyloading.KeyLoadingStrategy; 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; @@ -26,6 +29,10 @@ import java.net.URI; import java.nio.CharBuffer; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Arrays; +import java.util.Optional; +import java.util.concurrent.CancellationException; +import java.util.concurrent.ExecutionException; import java.util.concurrent.atomic.AtomicReference; @KeyLoading @@ -36,28 +43,28 @@ public class MasterkeyFileLoadingStrategy implements KeyLoadingStrategy { private final Vault vault; private final MasterkeyFileAccess masterkeyFileAccess; private final Stage window; - private final Lazy passphraseEntryScene; private final Lazy selectMasterkeyFileScene; - private final UserInteractionLock passwordEntryLock; + private final PassphraseEntryComponent.Builder passphraseEntry; private final UserInteractionLock masterkeyFileProvisionLock; - private final AtomicReference password; private final AtomicReference filePath; - private final MasterkeyFileLoadingFinisher finisher; + private final KeychainManager keychain; - private boolean wrongPassword; + private char[] passphrase; + private boolean savePassphrase; + private boolean wrongPassphrase; @Inject - public MasterkeyFileLoadingStrategy(@KeyLoading Vault vault, MasterkeyFileAccess masterkeyFileAccess, @KeyLoading Stage window, @FxmlScene(FxmlFile.UNLOCK_ENTER_PASSWORD) Lazy passphraseEntryScene, @FxmlScene(FxmlFile.UNLOCK_SELECT_MASTERKEYFILE) Lazy selectMasterkeyFileScene, UserInteractionLock passwordEntryLock, UserInteractionLock masterkeyFileProvisionLock, AtomicReference password, AtomicReference filePath, MasterkeyFileLoadingFinisher finisher) { + 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) { this.vault = vault; this.masterkeyFileAccess = masterkeyFileAccess; this.window = window; - this.passphraseEntryScene = passphraseEntryScene; this.selectMasterkeyFileScene = selectMasterkeyFileScene; - this.passwordEntryLock = passwordEntryLock; + this.passphraseEntry = passphraseEntry; this.masterkeyFileProvisionLock = masterkeyFileProvisionLock; - this.password = password; this.filePath = filePath; - this.finisher = finisher; + this.keychain = keychain; + this.passphrase = savedPassphrase.orElse(null); + this.savePassphrase = savedPassphrase.isPresent(); } @Override @@ -68,8 +75,10 @@ public class MasterkeyFileLoadingStrategy implements KeyLoadingStrategy { if (!Files.exists(filePath)) { filePath = getAlternateMasterkeyFilePath(); } - CharSequence passphrase = getPassphrase(); - var masterkey = masterkeyFileAccess.load(filePath, passphrase); + if (passphrase == null) { + askForPassphrase(); + } + var masterkey = masterkeyFileAccess.load(filePath, CharBuffer.wrap(passphrase)); //backup if (filePath.startsWith(vault.getPath())) { try { @@ -90,8 +99,8 @@ public class MasterkeyFileLoadingStrategy implements KeyLoadingStrategy { @Override public boolean recoverFromException(MasterkeyLoadingFailedException exception) { if (exception instanceof InvalidPassphraseException) { - this.wrongPassword = true; - password.set(null); + this.wrongPassphrase = true; + this.passphrase = null; return true; // reattempting key load } else { return false; // nothing we can do @@ -100,7 +109,20 @@ public class MasterkeyFileLoadingStrategy implements KeyLoadingStrategy { @Override public void cleanup(boolean unlockedSuccessfully) { - finisher.cleanup(unlockedSuccessfully); + if (unlockedSuccessfully && savePassphrase) { + savePasswordToSystemkeychain(passphrase); + } + Arrays.fill(passphrase, '\0'); + } + + private void savePasswordToSystemkeychain(char[] passphrase) { + if (keychain.isSupported()) { + try { + keychain.storePassphrase(vault.getId(), vault.getDisplayName(), CharBuffer.wrap(passphrase)); + } catch (KeychainAccessException e) { + LOG.error("Failed to store passphrase in system keychain.", e); + } + } } private Path getAlternateMasterkeyFilePath() throws UnlockCancelledException, InterruptedException { @@ -129,21 +151,10 @@ public class MasterkeyFileLoadingStrategy implements KeyLoadingStrategy { return masterkeyFileProvisionLock.awaitInteraction(); } - private CharSequence getPassphrase() throws UnlockCancelledException, InterruptedException { - if (password.get() == null) { - return switch (askForPassphrase()) { - case PASSWORD_ENTERED -> CharBuffer.wrap(password.get()); - case CANCELED -> throw new UnlockCancelledException("Password entry cancelled."); - }; - } else { - // e.g. pre-filled from keychain or previous unlock attempt - return CharBuffer.wrap(password.get()); - } - } - - private MasterkeyFileLoadingModule.PasswordEntry askForPassphrase() throws InterruptedException { + private void askForPassphrase() throws InterruptedException { + var comp = passphraseEntry.savedPassword(passphrase).build(); Platform.runLater(() -> { - window.setScene(passphraseEntryScene.get()); + window.setScene(comp.passphraseEntryScene()); window.show(); Window owner = window.getOwner(); if (owner != null) { @@ -152,11 +163,19 @@ public class MasterkeyFileLoadingStrategy implements KeyLoadingStrategy { } else { window.centerOnScreen(); } - if (wrongPassword) { + if (wrongPassphrase) { Animations.createShakeWindowAnimation(window).play(); } }); - return passwordEntryLock.awaitInteraction(); + try { + var result = comp.result().get(); + this.passphrase = result.passphrase(); + this.savePassphrase = result.savePassphrase(); + } catch (CancellationException e) { + throw new UnlockCancelledException("Password entry cancelled."); + } catch (ExecutionException e) { + throw new MasterkeyLoadingFailedException("Failed to ask for password.", e); + } } } diff --git a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryComponent.java b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryComponent.java new file mode 100644 index 000000000..637ffd5c6 --- /dev/null +++ b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryComponent.java @@ -0,0 +1,35 @@ +package org.cryptomator.ui.keyloading.masterkeyfile; + +import dagger.BindsInstance; +import dagger.Subcomponent; +import org.cryptomator.common.Nullable; +import org.cryptomator.ui.common.Animations; + +import javax.inject.Named; +import javafx.scene.Scene; +import javafx.stage.Stage; +import javafx.stage.Window; +import java.util.concurrent.CancellationException; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; + +@PassphraseEntryScoped +@Subcomponent(modules = {PassphraseEntryModule.class}) +public interface PassphraseEntryComponent { + + @PassphraseEntryScoped + Scene passphraseEntryScene(); + + @PassphraseEntryScoped + CompletableFuture result(); + + @Subcomponent.Builder + interface Builder { + + @BindsInstance + PassphraseEntryComponent.Builder savedPassword(@Nullable @Named("savedPassword") char[] savedPassword); + + PassphraseEntryComponent build(); + } + +} diff --git a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryController.java b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryController.java index f6ce79e51..d048cb776 100644 --- a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryController.java +++ b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryController.java @@ -1,16 +1,13 @@ package org.cryptomator.ui.keyloading.masterkeyfile; +import org.cryptomator.common.Nullable; import org.cryptomator.common.keychain.KeychainManager; import org.cryptomator.common.vaults.Vault; import org.cryptomator.ui.common.FxController; -import org.cryptomator.ui.common.UserInteractionLock; import org.cryptomator.ui.common.WeakBindings; -import org.cryptomator.ui.controls.FontAwesome5IconView; import org.cryptomator.ui.controls.NiceSecurePasswordField; import org.cryptomator.ui.forgetPassword.ForgetPasswordComponent; import org.cryptomator.ui.keyloading.KeyLoading; -import org.cryptomator.ui.keyloading.KeyLoadingScoped; -import org.cryptomator.ui.keyloading.masterkeyfile.MasterkeyFileLoadingModule.PasswordEntry; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -21,8 +18,8 @@ import javafx.animation.Interpolator; import javafx.animation.KeyFrame; import javafx.animation.KeyValue; import javafx.animation.Timeline; +import javafx.application.Platform; import javafx.beans.binding.Bindings; -import javafx.beans.binding.BooleanBinding; import javafx.beans.binding.ObjectBinding; import javafx.beans.binding.StringBinding; import javafx.beans.property.BooleanProperty; @@ -37,33 +34,27 @@ import javafx.scene.transform.Translate; import javafx.stage.Stage; import javafx.stage.WindowEvent; import javafx.util.Duration; -import java.util.Arrays; -import java.util.Optional; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicReference; +import java.util.concurrent.CompletableFuture; -@KeyLoadingScoped +@PassphraseEntryScoped public class PassphraseEntryController implements FxController { private static final Logger LOG = LoggerFactory.getLogger(PassphraseEntryController.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 CompletableFuture result; + private final char[] savedPassword; private final ForgetPasswordComponent.Builder forgetPassword; private final KeychainManager keychain; - private final ObjectBinding unlockButtonContentDisplay; - private final BooleanBinding userInteractionDisabled; - private final BooleanProperty unlockButtonDisabled; private final StringBinding vaultName; + private final BooleanProperty unlockInProgress = new SimpleBooleanProperty(); + private final ObjectBinding unlockButtonContentDisplay = Bindings.createObjectBinding(this::getUnlockButtonContentDisplay, unlockInProgress); + private final BooleanProperty unlockButtonDisabled = new SimpleBooleanProperty(); /* FXML */ public NiceSecurePasswordField passwordField; public CheckBox savePasswordCheckbox; - public FontAwesome5IconView unlockInProgressView; public ImageView face; public ImageView leftArm; public ImageView rightArm; @@ -72,29 +63,25 @@ public class PassphraseEntryController implements FxController { public Animation unlockAnimation; @Inject - public PassphraseEntryController(@KeyLoading Stage window, @KeyLoading Vault vault, AtomicReference password, @Named("savePassword") AtomicBoolean savePassword, @Named("savedPassword") Optional savedPassword, UserInteractionLock passwordEntryLock, ForgetPasswordComponent.Builder forgetPassword, KeychainManager keychain) { + public PassphraseEntryController(@KeyLoading Stage window, @KeyLoading Vault vault, CompletableFuture result, @Nullable @Named("savedPassword") char[] savedPassword, ForgetPasswordComponent.Builder forgetPassword, KeychainManager keychain) { this.window = window; this.vault = vault; - this.password = password; - this.savePassword = savePassword; + this.result = result; this.savedPassword = savedPassword; - this.passwordEntryLock = passwordEntryLock; this.forgetPassword = forgetPassword; this.keychain = keychain; - this.unlockButtonContentDisplay = Bindings.createObjectBinding(this::getUnlockButtonContentDisplay, passwordEntryLock.awaitingInteraction()); - this.userInteractionDisabled = passwordEntryLock.awaitingInteraction().not(); - this.unlockButtonDisabled = new SimpleBooleanProperty(); this.vaultName = WeakBindings.bindString(vault.displayNameProperty()); - this.window.setOnHiding(this::windowClosed); + window.setOnHiding(this::windowClosed); + result.whenCompleteAsync((r, t) -> unlockInProgress.set(false), Platform::runLater); } @FXML public void initialize() { - savePasswordCheckbox.setSelected(savedPassword.isPresent()); - if (password.get() != null) { - passwordField.setPassword(password.get()); + if (savedPassword != null) { + savePasswordCheckbox.setSelected(true); + passwordField.setPassword(savedPassword); } - unlockButtonDisabled.bind(userInteractionDisabled.or(passwordField.textProperty().isEmpty())); + unlockButtonDisabled.bind(unlockInProgress.or(passwordField.textProperty().isEmpty())); var leftArmTranslation = new Translate(24, 0); var leftArmRotation = new Rotate(60, 16, 30, 0); @@ -132,7 +119,7 @@ public class PassphraseEntryController implements FxController { new KeyFrame(Duration.millis(1000), faceVisible) // ); - passwordEntryLock.awaitingInteraction().addListener(observable -> stopUnlockAnimation()); + result.whenCompleteAsync((r, t) -> stopUnlockAnimation()); } @FXML @@ -141,26 +128,20 @@ public class PassphraseEntryController implements FxController { } private void windowClosed(WindowEvent windowEvent) { - // if not already interacted, mark this workflow as cancelled: - if (passwordEntryLock.awaitingInteraction().get()) { - LOG.debug("Unlock canceled by user."); - passwordEntryLock.interacted(PasswordEntry.CANCELED); - } + LOG.debug("Unlock canceled by user."); + result.cancel(true); } @FXML public void unlock() { LOG.trace("UnlockController.unlock()"); + unlockInProgress.set(true); CharSequence pwFieldContents = passwordField.getCharacters(); - char[] newPw = new char[pwFieldContents.length()]; + char[] pw = new char[pwFieldContents.length()]; for (int i = 0; i < pwFieldContents.length(); i++) { - newPw[i] = pwFieldContents.charAt(i); + pw[i] = pwFieldContents.charAt(i); } - char[] oldPw = password.getAndSet(newPw); - if (oldPw != null) { - Arrays.fill(oldPw, ' '); - } - passwordEntryLock.interacted(PasswordEntry.PASSWORD_ENTERED); + result.complete(new PassphraseEntryResult(pw, savePasswordCheckbox.isSelected())); startUnlockAnimation(); } @@ -184,8 +165,7 @@ public class PassphraseEntryController implements FxController { @FXML private void didClickSavePasswordCheckbox() { - savePassword.set(savePasswordCheckbox.isSelected()); - if (!savePasswordCheckbox.isSelected() && savedPassword.isPresent()) { + if (!savePasswordCheckbox.isSelected() && savedPassword != null) { forgetPassword.vault(vault).owner(window).build().showForgetPassword().thenAccept(forgotten -> savePasswordCheckbox.setSelected(!forgotten)); } } @@ -205,15 +185,15 @@ public class PassphraseEntryController implements FxController { } public ContentDisplay getUnlockButtonContentDisplay() { - return passwordEntryLock.awaitingInteraction().get() ? ContentDisplay.TEXT_ONLY : ContentDisplay.LEFT; + return unlockInProgress.get() ? ContentDisplay.LEFT : ContentDisplay.TEXT_ONLY; } - public BooleanBinding userInteractionDisabledProperty() { - return userInteractionDisabled; + public ReadOnlyBooleanProperty userInteractionDisabledProperty() { + return unlockInProgress; } public boolean isUserInteractionDisabled() { - return userInteractionDisabled.get(); + return unlockInProgress.get(); } public ReadOnlyBooleanProperty unlockButtonDisabledProperty() { @@ -227,4 +207,6 @@ public class PassphraseEntryController implements FxController { public boolean isKeychainAccessAvailable() { return keychain.isSupported(); } + + } diff --git a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryModule.java b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryModule.java new file mode 100644 index 000000000..aec32e5e6 --- /dev/null +++ b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryModule.java @@ -0,0 +1,41 @@ +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.util.ResourceBundle; +import java.util.concurrent.CompletableFuture; + +@Module +abstract class PassphraseEntryModule { + + @Provides + @PassphraseEntryScoped + static CompletableFuture provideResult() { + return new CompletableFuture<>(); + } + + @Provides + @PassphraseEntryScoped + static Scene provideUnlockScene(PassphraseEntryController controller, DefaultSceneFactory sceneFactory, ResourceBundle resourceBundle) { + // TODO: simplify FxmlLoaderFactory + try { + var url = FxmlLoaderFactory.class.getResource(FxmlFile.UNLOCK_ENTER_PASSWORD.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/PassphraseEntryResult.java b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryResult.java new file mode 100644 index 000000000..2c55c203e --- /dev/null +++ b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryResult.java @@ -0,0 +1,6 @@ +package org.cryptomator.ui.keyloading.masterkeyfile; + +// TODO needs to be public due to Dagger -.- +public record PassphraseEntryResult(char[] passphrase, boolean savePassphrase) { + +} diff --git a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryScoped.java b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryScoped.java new file mode 100644 index 000000000..a077bcf81 --- /dev/null +++ b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryScoped.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 PassphraseEntryScoped { + +} From 85a5146d4bce1ea6671913f3448ad93f30a92c5f Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Thu, 16 Dec 2021 13:59:10 +0100 Subject: [PATCH 02/26] wipe pw before losing reference [ci skip] --- .../keyloading/masterkeyfile/MasterkeyFileLoadingStrategy.java | 1 + 1 file changed, 1 insertion(+) 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 2ee17a744..3a5c7548b 100644 --- a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/MasterkeyFileLoadingStrategy.java +++ b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/MasterkeyFileLoadingStrategy.java @@ -100,6 +100,7 @@ public class MasterkeyFileLoadingStrategy implements KeyLoadingStrategy { public boolean recoverFromException(MasterkeyLoadingFailedException exception) { if (exception instanceof InvalidPassphraseException) { this.wrongPassphrase = true; + Arrays.fill(passphrase, '\0'); this.passphrase = null; return true; // reattempting key load } else { From d72d9f533c0ed6a8dd6c67ed628ccf841f31ba38 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Thu, 16 Dec 2021 16:41:07 +0100 Subject: [PATCH 03/26] Replace CharBuffer and char[] instances by destroyable Passphrase --- .../org/cryptomator/common/Passphrase.java | 115 ++++++++++++++++ .../ui/controls/NiceSecurePasswordField.java | 4 +- .../ui/controls/SecurePasswordField.java | 6 +- .../MasterkeyFileLoadingStrategy.java | 19 +-- .../PassphraseEntryComponent.java | 8 +- .../PassphraseEntryController.java | 10 +- .../masterkeyfile/PassphraseEntryResult.java | 4 +- .../cryptomator/common/PassphraseTest.java | 130 ++++++++++++++++++ 8 files changed, 270 insertions(+), 26 deletions(-) create mode 100644 src/main/java/org/cryptomator/common/Passphrase.java create mode 100644 src/test/java/org/cryptomator/common/PassphraseTest.java diff --git a/src/main/java/org/cryptomator/common/Passphrase.java b/src/main/java/org/cryptomator/common/Passphrase.java new file mode 100644 index 000000000..d57252507 --- /dev/null +++ b/src/main/java/org/cryptomator/common/Passphrase.java @@ -0,0 +1,115 @@ +package org.cryptomator.common; + +import org.cryptomator.cryptolib.common.MessageDigestSupplier; + +import javax.security.auth.Destroyable; +import java.nio.ByteBuffer; +import java.util.Arrays; + +/** + * A destroyable CharSequence. + */ +public class Passphrase implements Destroyable, CharSequence { + + private final char[] data; + private final int offset; + private final int length; + private boolean destroyed; + + /** + * Wraps (doesn't copy) the given data. + * + * @param data The wrapped data. Any changes to this will be reflected in this passphrase + */ + public Passphrase(char[] data) { + this(data, 0, data.length); + } + + /** + * Wraps (doesn't copy) a subarray of the given data. + * + * @param data The wrapped data. Any changes to this will be reflected in this passphrase + * @param offset The subarray offset, i.e. the first character of this passphrase + * @param length The subarray length, i.e. the length of this passphrase + */ + public Passphrase(char[] data, int offset, int length) { + if (offset < 0 || length < 0 || offset + length > data.length) { + throw new IndexOutOfBoundsException("[%1$d %1$d + %2$d[ not within [0, %3$d[".formatted(offset, length, data.length)); + } + this.data = data; + this.offset = offset; + this.length = length; + } + + public static Passphrase copyOf(CharSequence cs) { + char[] result = new char[cs.length()]; + for (int i = 0; i < cs.length(); i++) { + result[i] = cs.charAt(i); + } + return new Passphrase(result); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Passphrase that = (Passphrase) o; + // time-constant comparison + int diff = 0; + for (int i = 0; i < length; i++) { + diff |= charAt(i) ^ that.charAt(i); + } + return diff == 0; + } + + @Override + public int hashCode() { + // TODO: do we really need to a secure hashcode? toString leaks the pw anyway + var md = MessageDigestSupplier.SHA256.get(); + ByteBuffer buf = ByteBuffer.allocate(Character.BYTES * length); + for (int i = 0; i < length; i++) { + char c = charAt(i); + buf.putChar(i * Character.BYTES, c); + } + buf.flip(); + md.update(buf); + return Arrays.hashCode(md.digest()); + } + + @Override + public String toString() { + return new String(data, offset, length); + } + + @Override + public int length() { + return length; + } + + @Override + public char charAt(int index) { + if (index < 0 || index >= length) { + throw new IndexOutOfBoundsException("%d not within [0, %d[".formatted(index, length)); + } + return data[offset + index]; + } + + @Override + public Passphrase subSequence(int start, int end) { + if (start < 0 || end < 0 || end > length || start > end) { + throw new IndexOutOfBoundsException("[%d, %d[ not within [0, %d[".formatted(start, end, length)); + } + return new Passphrase(Arrays.copyOfRange(data, offset + start, offset + end)); + } + + @Override + public boolean isDestroyed() { + return destroyed; + } + + @Override + public void destroy() { + Arrays.fill(data, offset, offset + length, '\0'); + destroyed = true; + } +} diff --git a/src/main/java/org/cryptomator/ui/controls/NiceSecurePasswordField.java b/src/main/java/org/cryptomator/ui/controls/NiceSecurePasswordField.java index 4a4e43fff..4d09707b9 100644 --- a/src/main/java/org/cryptomator/ui/controls/NiceSecurePasswordField.java +++ b/src/main/java/org/cryptomator/ui/controls/NiceSecurePasswordField.java @@ -1,5 +1,7 @@ package org.cryptomator.ui.controls; +import org.cryptomator.common.Passphrase; + import javafx.beans.Observable; import javafx.beans.binding.Bindings; import javafx.beans.property.StringProperty; @@ -82,7 +84,7 @@ public class NiceSecurePasswordField extends StackPane { return passwordField.textProperty(); } - public CharSequence getCharacters() { + public Passphrase getCharacters() { return passwordField.getCharacters(); } diff --git a/src/main/java/org/cryptomator/ui/controls/SecurePasswordField.java b/src/main/java/org/cryptomator/ui/controls/SecurePasswordField.java index 0290f512d..66df79394 100644 --- a/src/main/java/org/cryptomator/ui/controls/SecurePasswordField.java +++ b/src/main/java/org/cryptomator/ui/controls/SecurePasswordField.java @@ -9,6 +9,7 @@ package org.cryptomator.ui.controls; import com.google.common.base.Strings; +import org.cryptomator.common.Passphrase; import javafx.application.Platform; import javafx.beans.NamedArg; @@ -28,7 +29,6 @@ import javafx.scene.input.KeyCodeCombination; import javafx.scene.input.KeyCombination; import javafx.scene.input.KeyEvent; import javafx.scene.input.TransferMode; -import java.nio.CharBuffer; import java.text.Normalizer; import java.text.Normalizer.Form; import java.util.Arrays; @@ -203,8 +203,8 @@ public class SecurePasswordField extends TextField { * @see #wipe() */ @Override - public CharSequence getCharacters() { - return CharBuffer.wrap(content, 0, length); + public Passphrase getCharacters() { + return new Passphrase(content, 0, length); } /** 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 3a5c7548b..a22903973 100644 --- a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/MasterkeyFileLoadingStrategy.java +++ b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/MasterkeyFileLoadingStrategy.java @@ -13,6 +13,7 @@ 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; @@ -26,10 +27,8 @@ import javafx.stage.Stage; import javafx.stage.Window; import java.io.IOException; import java.net.URI; -import java.nio.CharBuffer; import java.nio.file.Files; import java.nio.file.Path; -import java.util.Arrays; import java.util.Optional; import java.util.concurrent.CancellationException; import java.util.concurrent.ExecutionException; @@ -49,7 +48,7 @@ public class MasterkeyFileLoadingStrategy implements KeyLoadingStrategy { private final AtomicReference filePath; private final KeychainManager keychain; - private char[] passphrase; + private Passphrase passphrase; private boolean savePassphrase; private boolean wrongPassphrase; @@ -63,7 +62,7 @@ public class MasterkeyFileLoadingStrategy implements KeyLoadingStrategy { this.masterkeyFileProvisionLock = masterkeyFileProvisionLock; this.filePath = filePath; this.keychain = keychain; - this.passphrase = savedPassphrase.orElse(null); + this.passphrase = savedPassphrase.map(Passphrase::new).orElse(null); this.savePassphrase = savedPassphrase.isPresent(); } @@ -78,7 +77,7 @@ public class MasterkeyFileLoadingStrategy implements KeyLoadingStrategy { if (passphrase == null) { askForPassphrase(); } - var masterkey = masterkeyFileAccess.load(filePath, CharBuffer.wrap(passphrase)); + var masterkey = masterkeyFileAccess.load(filePath, passphrase); //backup if (filePath.startsWith(vault.getPath())) { try { @@ -100,7 +99,7 @@ public class MasterkeyFileLoadingStrategy implements KeyLoadingStrategy { public boolean recoverFromException(MasterkeyLoadingFailedException exception) { if (exception instanceof InvalidPassphraseException) { this.wrongPassphrase = true; - Arrays.fill(passphrase, '\0'); + passphrase.destroy(); this.passphrase = null; return true; // reattempting key load } else { @@ -113,13 +112,15 @@ public class MasterkeyFileLoadingStrategy implements KeyLoadingStrategy { if (unlockedSuccessfully && savePassphrase) { savePasswordToSystemkeychain(passphrase); } - Arrays.fill(passphrase, '\0'); + if (passphrase != null) { + passphrase.destroy(); + } } - private void savePasswordToSystemkeychain(char[] passphrase) { + private void savePasswordToSystemkeychain(Passphrase passphrase) { if (keychain.isSupported()) { try { - keychain.storePassphrase(vault.getId(), vault.getDisplayName(), CharBuffer.wrap(passphrase)); + keychain.storePassphrase(vault.getId(), vault.getDisplayName(), passphrase); } catch (KeychainAccessException e) { LOG.error("Failed to store passphrase in system keychain.", e); } diff --git a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryComponent.java b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryComponent.java index 637ffd5c6..5e072efd0 100644 --- a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryComponent.java +++ b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryComponent.java @@ -3,15 +3,11 @@ package org.cryptomator.ui.keyloading.masterkeyfile; import dagger.BindsInstance; import dagger.Subcomponent; import org.cryptomator.common.Nullable; -import org.cryptomator.ui.common.Animations; +import org.cryptomator.common.Passphrase; import javax.inject.Named; import javafx.scene.Scene; -import javafx.stage.Stage; -import javafx.stage.Window; -import java.util.concurrent.CancellationException; import java.util.concurrent.CompletableFuture; -import java.util.concurrent.ExecutionException; @PassphraseEntryScoped @Subcomponent(modules = {PassphraseEntryModule.class}) @@ -27,7 +23,7 @@ public interface PassphraseEntryComponent { interface Builder { @BindsInstance - PassphraseEntryComponent.Builder savedPassword(@Nullable @Named("savedPassword") char[] savedPassword); + PassphraseEntryComponent.Builder savedPassword(@Nullable @Named("savedPassword") Passphrase savedPassword); PassphraseEntryComponent build(); } diff --git a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryController.java b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryController.java index d048cb776..35b1b1903 100644 --- a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryController.java +++ b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryController.java @@ -4,6 +4,7 @@ import org.cryptomator.common.Nullable; import org.cryptomator.common.keychain.KeychainManager; import org.cryptomator.common.vaults.Vault; import org.cryptomator.ui.common.FxController; +import org.cryptomator.common.Passphrase; import org.cryptomator.ui.common.WeakBindings; import org.cryptomator.ui.controls.NiceSecurePasswordField; import org.cryptomator.ui.forgetPassword.ForgetPasswordComponent; @@ -44,7 +45,7 @@ public class PassphraseEntryController implements FxController { private final Stage window; private final Vault vault; private final CompletableFuture result; - private final char[] savedPassword; + private final Passphrase savedPassword; private final ForgetPasswordComponent.Builder forgetPassword; private final KeychainManager keychain; private final StringBinding vaultName; @@ -63,7 +64,7 @@ public class PassphraseEntryController implements FxController { public Animation unlockAnimation; @Inject - public PassphraseEntryController(@KeyLoading Stage window, @KeyLoading Vault vault, CompletableFuture result, @Nullable @Named("savedPassword") char[] savedPassword, ForgetPasswordComponent.Builder forgetPassword, KeychainManager keychain) { + public PassphraseEntryController(@KeyLoading Stage window, @KeyLoading Vault vault, CompletableFuture result, @Nullable @Named("savedPassword") Passphrase savedPassword, ForgetPasswordComponent.Builder forgetPassword, KeychainManager keychain) { this.window = window; this.vault = vault; this.result = result; @@ -137,10 +138,7 @@ public class PassphraseEntryController implements FxController { LOG.trace("UnlockController.unlock()"); unlockInProgress.set(true); CharSequence pwFieldContents = passwordField.getCharacters(); - char[] pw = new char[pwFieldContents.length()]; - for (int i = 0; i < pwFieldContents.length(); i++) { - pw[i] = pwFieldContents.charAt(i); - } + Passphrase pw = Passphrase.copyOf(pwFieldContents); result.complete(new PassphraseEntryResult(pw, savePasswordCheckbox.isSelected())); startUnlockAnimation(); } diff --git a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryResult.java b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryResult.java index 2c55c203e..f26184d9d 100644 --- a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryResult.java +++ b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryResult.java @@ -1,6 +1,8 @@ package org.cryptomator.ui.keyloading.masterkeyfile; +import org.cryptomator.common.Passphrase; + // TODO needs to be public due to Dagger -.- -public record PassphraseEntryResult(char[] passphrase, boolean savePassphrase) { +public record PassphraseEntryResult(Passphrase passphrase, boolean savePassphrase) { } diff --git a/src/test/java/org/cryptomator/common/PassphraseTest.java b/src/test/java/org/cryptomator/common/PassphraseTest.java new file mode 100644 index 000000000..7bd71beb2 --- /dev/null +++ b/src/test/java/org/cryptomator/common/PassphraseTest.java @@ -0,0 +1,130 @@ +package org.cryptomator.common; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; +import org.junit.jupiter.params.provider.ValueSource; + +class PassphraseTest { + + @ParameterizedTest + @CsvSource(value = { + "-1, 0", + "0, -1", + "0, 10", + "10, 0", + "10, 10" + }) + public void testInvalidConstructorArgs(int offset, int length) { + char[] data = "test".toCharArray(); + Assertions.assertThrows(IndexOutOfBoundsException.class, () -> { + new Passphrase(data, offset, length); + }); + } + + @ParameterizedTest + @CsvSource(value = { + "0, 4", + "0, 0", + "0, 1", + "1, 1", + "2, 2" + }) + public void testValidConstructorArgs(int offset, int length) { + char[] data = "test".toCharArray(); + var pw = new Passphrase(data, offset, length); + Assertions.assertEquals(length, pw.length()); + Assertions.assertEquals("test".substring(offset, offset + length), pw.toString()); + } + + @Test + public void testToString() { + Assertions.assertEquals("test", Passphrase.copyOf("test").toString()); + } + + @Nested + class WithInstances { + + private Passphrase pw1; + private Passphrase pw2; + + @BeforeEach + public void setup() { + char[] foo = "test test".toCharArray(); + pw1 = new Passphrase(foo, 5, 4); + pw2 = Passphrase.copyOf("test"); + } + + @Test + public void testEquals() { + Assertions.assertEquals(pw1, pw2); + Assertions.assertEquals("test", pw1.toString()); + Assertions.assertEquals("test", pw2.toString()); + } + + @Test + public void testHashcode() { + Assertions.assertEquals(pw1.hashCode(), pw2.hashCode()); + } + + @Test + public void testLength() { + Assertions.assertEquals(4, pw1.length()); + Assertions.assertEquals(4, pw2.length()); + } + + @Test + public void testCharAt() { + Assertions.assertEquals('s', pw1.charAt(2)); + } + + @ParameterizedTest + @ValueSource(ints = {-1, 4, 5}) + public void testInvalidCharAt(int idx) { + Assertions.assertThrows(IndexOutOfBoundsException.class, () -> pw1.charAt(idx)); + } + + @ParameterizedTest + @ValueSource(ints = {0, 1, 2, 3}) + public void testValidCharAt(int idx) { + Assertions.assertEquals("test".charAt(idx), pw1.charAt(idx)); + } + + @ParameterizedTest + @CsvSource(value = { + "-1, 0", + "0, -1", + "-1, -1", + "0, 5", + "3, 2" + }) + public void testInvalidSubSequence(int start, int end) { + Assertions.assertThrows(IndexOutOfBoundsException.class, () -> pw1.subSequence(start, end)); + } + + @ParameterizedTest + @CsvSource(value = { + "0, 4", + "1, 4", + "0, 2", + "2, 4", + "4, 4", + }) + public void testValidSubSequence(int start, int end) { + Assertions.assertEquals("test".substring(start, end), pw1.subSequence(start, end).toString()); + } + + @Test + public void testDestroy() { + pw2.destroy(); + Assertions.assertFalse(pw1.isDestroyed()); + Assertions.assertTrue(pw2.isDestroyed()); + Assertions.assertNotEquals(pw1, pw2); + } + + } + +} \ No newline at end of file From e95853deacab30afe0420449c05e0f338a561347 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Thu, 16 Dec 2021 16:46:57 +0100 Subject: [PATCH 04/26] make test public, fixing InaccessibleObject error in maven build --- .../org/cryptomator/common/PassphraseTest.java | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/test/java/org/cryptomator/common/PassphraseTest.java b/src/test/java/org/cryptomator/common/PassphraseTest.java index 7bd71beb2..02f640e94 100644 --- a/src/test/java/org/cryptomator/common/PassphraseTest.java +++ b/src/test/java/org/cryptomator/common/PassphraseTest.java @@ -8,7 +8,7 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; import org.junit.jupiter.params.provider.ValueSource; -class PassphraseTest { +public class PassphraseTest { @ParameterizedTest @CsvSource(value = { @@ -40,13 +40,8 @@ class PassphraseTest { Assertions.assertEquals("test".substring(offset, offset + length), pw.toString()); } - @Test - public void testToString() { - Assertions.assertEquals("test", Passphrase.copyOf("test").toString()); - } - @Nested - class WithInstances { + public class InstanceMethods { private Passphrase pw1; private Passphrase pw2; @@ -59,12 +54,16 @@ class PassphraseTest { } @Test - public void testEquals() { - Assertions.assertEquals(pw1, pw2); + public void testToString() { Assertions.assertEquals("test", pw1.toString()); Assertions.assertEquals("test", pw2.toString()); } + @Test + public void testEquals() { + Assertions.assertEquals(pw1, pw2); + } + @Test public void testHashcode() { Assertions.assertEquals(pw1.hashCode(), pw2.hashCode()); From 983a4d0b0faa554664127b41c3b52b67684849dc Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Thu, 16 Dec 2021 17:00:49 +0100 Subject: [PATCH 05/26] 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()); } } From 6ca87d13f57e281ca613f283bcbe75d4b3498ee1 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Thu, 16 Dec 2021 17:01:56 +0100 Subject: [PATCH 06/26] renamed class --- ...Controller.java => ChooseMasterkeyFileController.java} | 6 +++--- .../masterkeyfile/ChooseMasterkeyFileModule.java | 2 +- .../masterkeyfile/MasterkeyFileLoadingStrategy.java | 8 ++++---- src/main/resources/fxml/unlock_select_masterkeyfile.fxml | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) rename src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/{SelectMasterkeyFileController.java => ChooseMasterkeyFileController.java} (86%) diff --git a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/SelectMasterkeyFileController.java b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/ChooseMasterkeyFileController.java similarity index 86% rename from src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/SelectMasterkeyFileController.java rename to src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/ChooseMasterkeyFileController.java index fc225e0a8..11cf7bd6b 100644 --- a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/SelectMasterkeyFileController.java +++ b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/ChooseMasterkeyFileController.java @@ -16,16 +16,16 @@ import java.util.ResourceBundle; import java.util.concurrent.CompletableFuture; @ChooseMasterkeyFileScoped -public class SelectMasterkeyFileController implements FxController { +public class ChooseMasterkeyFileController implements FxController { - private static final Logger LOG = LoggerFactory.getLogger(SelectMasterkeyFileController.class); + private static final Logger LOG = LoggerFactory.getLogger(ChooseMasterkeyFileController.class); private final Stage window; private final CompletableFuture result; private final ResourceBundle resourceBundle; @Inject - public SelectMasterkeyFileController(@KeyLoading Stage window, CompletableFuture result, ResourceBundle resourceBundle) { + public ChooseMasterkeyFileController(@KeyLoading Stage window, CompletableFuture result, ResourceBundle resourceBundle) { this.window = window; this.result = result; this.resourceBundle = resourceBundle; diff --git a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/ChooseMasterkeyFileModule.java b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/ChooseMasterkeyFileModule.java index 1cc71cbfa..27f1247e4 100644 --- a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/ChooseMasterkeyFileModule.java +++ b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/ChooseMasterkeyFileModule.java @@ -26,7 +26,7 @@ abstract class ChooseMasterkeyFileModule { @Provides @ChooseMasterkeyFileScoped - static Scene provideChooseMasterkeyScene(SelectMasterkeyFileController controller, DefaultSceneFactory sceneFactory, ResourceBundle resourceBundle) { + static Scene provideChooseMasterkeyScene(ChooseMasterkeyFileController controller, DefaultSceneFactory sceneFactory, ResourceBundle resourceBundle) { // TODO: simplify FxmlLoaderFactory try { var url = FxmlLoaderFactory.class.getResource(FxmlFile.UNLOCK_SELECT_MASTERKEYFILE.getRessourcePathString()); 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 13a7dadda..a1d85cb36 100644 --- a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/MasterkeyFileLoadingStrategy.java +++ b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/MasterkeyFileLoadingStrategy.java @@ -37,7 +37,7 @@ public class MasterkeyFileLoadingStrategy implements KeyLoadingStrategy { private final MasterkeyFileAccess masterkeyFileAccess; private final Stage window; private final PassphraseEntryComponent.Builder passphraseEntry; - private final ChooseMasterkeyFileComponent.Builder masterkeyChooser; + private final ChooseMasterkeyFileComponent.Builder masterkeyFileChoice; private final KeychainManager keychain; private Passphrase passphrase; @@ -45,12 +45,12 @@ public class MasterkeyFileLoadingStrategy implements KeyLoadingStrategy { private boolean wrongPassphrase; @Inject - public MasterkeyFileLoadingStrategy(@KeyLoading Vault vault, MasterkeyFileAccess masterkeyFileAccess, @KeyLoading Stage window, @Named("savedPassword") Optional savedPassphrase, PassphraseEntryComponent.Builder passphraseEntry, ChooseMasterkeyFileComponent.Builder masterkeyChooser, KeychainManager keychain) { + public MasterkeyFileLoadingStrategy(@KeyLoading Vault vault, MasterkeyFileAccess masterkeyFileAccess, @KeyLoading Stage window, @Named("savedPassword") Optional savedPassphrase, PassphraseEntryComponent.Builder passphraseEntry, ChooseMasterkeyFileComponent.Builder masterkeyFileChoice, KeychainManager keychain) { this.vault = vault; this.masterkeyFileAccess = masterkeyFileAccess; this.window = window; this.passphraseEntry = passphraseEntry; - this.masterkeyChooser = masterkeyChooser; + this.masterkeyFileChoice = masterkeyFileChoice; this.keychain = keychain; this.passphrase = savedPassphrase.map(Passphrase::new).orElse(null); this.savePassphrase = savedPassphrase.isPresent(); @@ -118,7 +118,7 @@ public class MasterkeyFileLoadingStrategy implements KeyLoadingStrategy { } private Path askUserForMasterkeyFilePath() throws InterruptedException { - var comp = masterkeyChooser.build(); + var comp = masterkeyFileChoice.build(); Platform.runLater(() -> { window.setScene(comp.chooseMasterkeyScene()); window.show(); diff --git a/src/main/resources/fxml/unlock_select_masterkeyfile.fxml b/src/main/resources/fxml/unlock_select_masterkeyfile.fxml index b6539f88f..43f2f3e46 100644 --- a/src/main/resources/fxml/unlock_select_masterkeyfile.fxml +++ b/src/main/resources/fxml/unlock_select_masterkeyfile.fxml @@ -11,7 +11,7 @@ Date: Thu, 16 Dec 2021 17:09:25 +0100 Subject: [PATCH 07/26] convert Dagger modules to interfaces --- .../masterkeyfile/ChooseMasterkeyFileModule.java | 2 +- .../masterkeyfile/MasterkeyFileLoadingModule.java | 7 ++----- .../ui/keyloading/masterkeyfile/PassphraseEntryModule.java | 2 +- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/ChooseMasterkeyFileModule.java b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/ChooseMasterkeyFileModule.java index 27f1247e4..bd9ab42c0 100644 --- a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/ChooseMasterkeyFileModule.java +++ b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/ChooseMasterkeyFileModule.java @@ -16,7 +16,7 @@ import java.util.ResourceBundle; import java.util.concurrent.CompletableFuture; @Module -abstract class ChooseMasterkeyFileModule { +interface ChooseMasterkeyFileModule { @Provides @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 af592cb3b..9375b0cff 100644 --- a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/MasterkeyFileLoadingModule.java +++ b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/MasterkeyFileLoadingModule.java @@ -12,16 +12,13 @@ import org.cryptomator.ui.forgetPassword.ForgetPasswordComponent; import org.cryptomator.ui.keyloading.KeyLoading; import org.cryptomator.ui.keyloading.KeyLoadingScoped; import org.cryptomator.ui.keyloading.KeyLoadingStrategy; -import org.slf4j.Logger; import org.slf4j.LoggerFactory; import javax.inject.Named; import java.util.Optional; @Module(subcomponents = {ForgetPasswordComponent.class, PassphraseEntryComponent.class, ChooseMasterkeyFileComponent.class}) -public abstract class MasterkeyFileLoadingModule { - - private static final Logger LOG = LoggerFactory.getLogger(MasterkeyFileLoadingModule.class); +public interface MasterkeyFileLoadingModule { @Provides @Named("savedPassword") @@ -33,7 +30,7 @@ public abstract class MasterkeyFileLoadingModule { try { return Optional.ofNullable(keychain.loadPassphrase(vault.getId())); } catch (KeychainAccessException e) { - LOG.error("Failed to load entry from system keychain.", e); + LoggerFactory.getLogger(MasterkeyFileLoadingModule.class).error("Failed to load entry from system keychain.", e); return Optional.empty(); } } diff --git a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryModule.java b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryModule.java index aec32e5e6..0d67abd50 100644 --- a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryModule.java +++ b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryModule.java @@ -15,7 +15,7 @@ import java.util.ResourceBundle; import java.util.concurrent.CompletableFuture; @Module -abstract class PassphraseEntryModule { +interface PassphraseEntryModule { @Provides @PassphraseEntryScoped From 9856792921b5d553c6db195aeaca3e3199c904be Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Wed, 19 Jan 2022 17:21:32 +0100 Subject: [PATCH 08/26] replaced UserInteractionLock with CompletableFuture in LockWorkflow --- .../ui/common/UserInteractionLock.java | 61 ------------------- .../ui/lock/LockForcedController.java | 20 +++--- .../org/cryptomator/ui/lock/LockModule.java | 15 ++--- .../org/cryptomator/ui/lock/LockWorkflow.java | 39 +++++++----- 4 files changed, 36 insertions(+), 99 deletions(-) delete mode 100644 src/main/java/org/cryptomator/ui/common/UserInteractionLock.java diff --git a/src/main/java/org/cryptomator/ui/common/UserInteractionLock.java b/src/main/java/org/cryptomator/ui/common/UserInteractionLock.java deleted file mode 100644 index 12c394533..000000000 --- a/src/main/java/org/cryptomator/ui/common/UserInteractionLock.java +++ /dev/null @@ -1,61 +0,0 @@ -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.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; - -public class UserInteractionLock> { - - private final Lock lock = new ReentrantLock(); - private final Condition condition = lock.newCondition(); - private final BooleanProperty awaitingInteraction = new SimpleBooleanProperty(); - private final AtomicBoolean interacted = new AtomicBoolean(); - private final AtomicReference state; - - public UserInteractionLock(E initialValue) { - this.state = new AtomicReference<>(initialValue); - } - - public synchronized void reset(E value) { - state.set(value); - interacted.set(false); - } - - public void interacted(E result) { - assert Platform.isFxApplicationThread(); - lock.lock(); - try { - state.set(result); - interacted.set(true); - awaitingInteraction.set(false); - condition.signal(); - } finally { - lock.unlock(); - } - } - - public E awaitInteraction() throws InterruptedException { - assert !Platform.isFxApplicationThread(); - lock.lock(); - try { - Platform.runLater(() -> awaitingInteraction.set(true)); - while (!interacted.get()) { - condition.await(); - } - return state.get(); - } finally { - lock.unlock(); - } - } - - public ReadOnlyBooleanProperty awaitingInteraction() { - return awaitingInteraction; - } - -} diff --git a/src/main/java/org/cryptomator/ui/lock/LockForcedController.java b/src/main/java/org/cryptomator/ui/lock/LockForcedController.java index c3a452acc..0b4a23a18 100644 --- a/src/main/java/org/cryptomator/ui/lock/LockForcedController.java +++ b/src/main/java/org/cryptomator/ui/lock/LockForcedController.java @@ -2,7 +2,6 @@ package org.cryptomator.ui.lock; import org.cryptomator.common.vaults.Vault; import org.cryptomator.ui.common.FxController; -import org.cryptomator.ui.common.UserInteractionLock; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -10,6 +9,8 @@ import javax.inject.Inject; import javafx.fxml.FXML; import javafx.stage.Stage; import javafx.stage.WindowEvent; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.atomic.AtomicReference; @LockScoped public class LockForcedController implements FxController { @@ -18,40 +19,35 @@ public class LockForcedController implements FxController { private final Stage window; private final Vault vault; - private final UserInteractionLock forceLockDecisionLock; + private final AtomicReference> forceRetryDecision; @Inject - public LockForcedController(@LockWindow Stage window, @LockWindow Vault vault, UserInteractionLock forceLockDecisionLock) { + public LockForcedController(@LockWindow Stage window, @LockWindow Vault vault, AtomicReference> forceRetryDecision) { this.window = window; this.vault = vault; - this.forceLockDecisionLock = forceLockDecisionLock; + this.forceRetryDecision = forceRetryDecision; this.window.setOnHiding(this::windowClosed); } @FXML public void cancel() { - forceLockDecisionLock.interacted(LockModule.ForceLockDecision.CANCEL); window.close(); } @FXML public void retry() { - forceLockDecisionLock.interacted(LockModule.ForceLockDecision.RETRY); + forceRetryDecision.get().complete(false); window.close(); } @FXML public void force() { - forceLockDecisionLock.interacted(LockModule.ForceLockDecision.FORCE); + forceRetryDecision.get().complete(true); window.close(); } private void windowClosed(WindowEvent windowEvent) { - // if not already interacted, set the decision to CANCEL - if (forceLockDecisionLock.awaitingInteraction().get()) { - LOG.debug("Lock canceled in force-lock-phase by user."); - forceLockDecisionLock.interacted(LockModule.ForceLockDecision.CANCEL); - } + forceRetryDecision.get().cancel(true); } // ----- Getter & Setter ----- diff --git a/src/main/java/org/cryptomator/ui/lock/LockModule.java b/src/main/java/org/cryptomator/ui/lock/LockModule.java index d1eb5f189..ddee13dff 100644 --- a/src/main/java/org/cryptomator/ui/lock/LockModule.java +++ b/src/main/java/org/cryptomator/ui/lock/LockModule.java @@ -6,13 +6,12 @@ import dagger.Provides; import dagger.multibindings.IntoMap; import org.cryptomator.common.vaults.Vault; import org.cryptomator.ui.common.DefaultSceneFactory; -import org.cryptomator.ui.common.FxmlLoaderFactory; 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.StageFactory; -import org.cryptomator.ui.common.UserInteractionLock; import javax.inject.Named; import javax.inject.Provider; @@ -22,20 +21,16 @@ import javafx.stage.Stage; import java.util.Map; import java.util.Optional; import java.util.ResourceBundle; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.atomic.AtomicReference; @Module abstract class LockModule { - enum ForceLockDecision { - CANCEL, - RETRY, - FORCE; - } - @Provides @LockScoped - static UserInteractionLock provideForceLockDecisionLock() { - return new UserInteractionLock<>(null); + static AtomicReference> provideForceRetryDecisionRef() { + return new AtomicReference<>(); } @Provides diff --git a/src/main/java/org/cryptomator/ui/lock/LockWorkflow.java b/src/main/java/org/cryptomator/ui/lock/LockWorkflow.java index 00b25c507..1e05ceb73 100644 --- a/src/main/java/org/cryptomator/ui/lock/LockWorkflow.java +++ b/src/main/java/org/cryptomator/ui/lock/LockWorkflow.java @@ -8,7 +8,6 @@ import org.cryptomator.common.vaults.Volume; 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.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -18,6 +17,10 @@ import javafx.concurrent.Task; import javafx.scene.Scene; import javafx.stage.Stage; import javafx.stage.Window; +import java.util.concurrent.CancellationException; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.atomic.AtomicReference; /** * The sequence of actions performed and checked during lock of a vault. @@ -34,43 +37,48 @@ public class LockWorkflow extends Task { private final Stage lockWindow; private final Vault vault; - private final UserInteractionLock forceLockDecisionLock; + private final AtomicReference> forceRetryDecision; private final Lazy lockForcedScene; private final Lazy lockFailedScene; private final ErrorComponent.Builder errorComponent; @Inject - public LockWorkflow(@LockWindow Stage lockWindow, @LockWindow Vault vault, UserInteractionLock forceLockDecisionLock, @FxmlScene(FxmlFile.LOCK_FORCED) Lazy lockForcedScene, @FxmlScene(FxmlFile.LOCK_FAILED) Lazy lockFailedScene, ErrorComponent.Builder errorComponent) { + public LockWorkflow(@LockWindow Stage lockWindow, @LockWindow Vault vault, AtomicReference> forceRetryDecision, @FxmlScene(FxmlFile.LOCK_FORCED) Lazy lockForcedScene, @FxmlScene(FxmlFile.LOCK_FAILED) Lazy lockFailedScene, ErrorComponent.Builder errorComponent) { this.lockWindow = lockWindow; this.vault = vault; - this.forceLockDecisionLock = forceLockDecisionLock; + this.forceRetryDecision = forceRetryDecision; this.lockForcedScene = lockForcedScene; this.lockFailedScene = lockFailedScene; this.errorComponent = errorComponent; } @Override - protected Void call() throws Volume.VolumeException, InterruptedException, LockNotCompletedException { + protected Void call() throws Volume.VolumeException, InterruptedException, LockNotCompletedException, ExecutionException { lock(false); return null; } - private void lock(boolean forced) throws InterruptedException { + private void lock(boolean forced) throws InterruptedException, ExecutionException { try { vault.lock(forced); } catch (Volume.VolumeException | LockNotCompletedException e) { LOG.info("Locking {} failed (forced: {}).", vault.getDisplayName(), forced, e); - var decision = askUserForAction(); - switch (decision) { - case RETRY -> lock(false); - case FORCE -> lock(true); - case CANCEL -> cancel(false); - } + retryOrCancel(); } } - private LockModule.ForceLockDecision askUserForAction() throws InterruptedException { - forceLockDecisionLock.reset(null); + private void retryOrCancel() throws ExecutionException, InterruptedException { + try { + boolean forced = askWhetherToUseTheForce().get(); + lock(forced); + } catch (CancellationException e) { + cancel(false); + } + } + + private CompletableFuture askWhetherToUseTheForce() { + var decision = new CompletableFuture(); + forceRetryDecision.set(decision); // show forcedLock dialogue ... Platform.runLater(() -> { lockWindow.setScene(lockForcedScene.get()); @@ -83,8 +91,7 @@ public class LockWorkflow extends Task { lockWindow.centerOnScreen(); } }); - // ... and wait for answer - return forceLockDecisionLock.awaitInteraction(); + return decision; } @Override From d52e59d7a44090944e182bbd35ad16fcb01a76f4 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Wed, 19 Jan 2022 19:08:54 +0100 Subject: [PATCH 09/26] dedup by setting title when setting the scene --- .../masterkeyfile/ChooseMasterkeyFileModule.java | 15 +++------------ .../MasterkeyFileLoadingStrategy.java | 7 ++++++- .../masterkeyfile/PassphraseEntryModule.java | 13 ++----------- 3 files changed, 11 insertions(+), 24 deletions(-) diff --git a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/ChooseMasterkeyFileModule.java b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/ChooseMasterkeyFileModule.java index baa302170..34eb78443 100644 --- a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/ChooseMasterkeyFileModule.java +++ b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/ChooseMasterkeyFileModule.java @@ -2,16 +2,13 @@ package org.cryptomator.ui.keyloading.masterkeyfile; import dagger.Module; import dagger.Provides; -import org.cryptomator.common.vaults.Vault; import org.cryptomator.ui.common.DefaultSceneFactory; import org.cryptomator.ui.common.FxmlFile; import org.cryptomator.ui.common.FxmlLoaderFactory; -import org.cryptomator.ui.keyloading.KeyLoading; import javafx.fxml.FXMLLoader; import javafx.scene.Parent; import javafx.scene.Scene; -import javafx.stage.Stage; import java.io.IOException; import java.io.UncheckedIOException; import java.nio.file.Path; @@ -29,21 +26,15 @@ interface ChooseMasterkeyFileModule { @Provides @ChooseMasterkeyFileScoped - static Scene provideChooseMasterkeyScene(ChooseMasterkeyFileController controller, DefaultSceneFactory sceneFactory, ResourceBundle resourceBundle, @KeyLoading Stage window, @KeyLoading Vault v) { + static Scene provideChooseMasterkeyScene(ChooseMasterkeyFileController 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(); - var scene = sceneFactory.apply(root); - scene.windowProperty().addListener((prop, oldVal, newVal) -> { - if (window.equals(newVal)) { - window.setTitle(String.format(resourceBundle.getString("unlock.chooseMasterkey.title"), v.getDisplayName())); - } - }); - return scene; + return sceneFactory.apply(root); } catch (IOException e) { - throw new UncheckedIOException("Failed to load UnlockScene", e); + throw new UncheckedIOException("Failed to load ChooseMasterkeyScene", e); } } 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 a1d85cb36..b4964f9a0 100644 --- a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/MasterkeyFileLoadingStrategy.java +++ b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/MasterkeyFileLoadingStrategy.java @@ -25,6 +25,7 @@ import java.net.URI; import java.nio.file.Files; import java.nio.file.Path; import java.util.Optional; +import java.util.ResourceBundle; import java.util.concurrent.CancellationException; import java.util.concurrent.ExecutionException; @@ -39,19 +40,21 @@ public class MasterkeyFileLoadingStrategy implements KeyLoadingStrategy { private final PassphraseEntryComponent.Builder passphraseEntry; private final ChooseMasterkeyFileComponent.Builder masterkeyFileChoice; private final KeychainManager keychain; + private final ResourceBundle resourceBundle; private Passphrase passphrase; private boolean savePassphrase; private boolean wrongPassphrase; @Inject - public MasterkeyFileLoadingStrategy(@KeyLoading Vault vault, MasterkeyFileAccess masterkeyFileAccess, @KeyLoading Stage window, @Named("savedPassword") Optional savedPassphrase, PassphraseEntryComponent.Builder passphraseEntry, ChooseMasterkeyFileComponent.Builder masterkeyFileChoice, KeychainManager keychain) { + public MasterkeyFileLoadingStrategy(@KeyLoading Vault vault, MasterkeyFileAccess masterkeyFileAccess, @KeyLoading Stage window, @Named("savedPassword") Optional savedPassphrase, PassphraseEntryComponent.Builder passphraseEntry, ChooseMasterkeyFileComponent.Builder masterkeyFileChoice, KeychainManager keychain, ResourceBundle resourceBundle) { this.vault = vault; this.masterkeyFileAccess = masterkeyFileAccess; this.window = window; this.passphraseEntry = passphraseEntry; this.masterkeyFileChoice = masterkeyFileChoice; this.keychain = keychain; + this.resourceBundle = resourceBundle; this.passphrase = savedPassphrase.map(Passphrase::new).orElse(null); this.savePassphrase = savedPassphrase.isPresent(); } @@ -121,6 +124,7 @@ public class MasterkeyFileLoadingStrategy implements KeyLoadingStrategy { var comp = masterkeyFileChoice.build(); Platform.runLater(() -> { window.setScene(comp.chooseMasterkeyScene()); + window.setTitle(resourceBundle.getString("unlock.chooseMasterkey.title").formatted(vault.getDisplayName())); window.show(); Window owner = window.getOwner(); if (owner != null) { @@ -143,6 +147,7 @@ public class MasterkeyFileLoadingStrategy implements KeyLoadingStrategy { var comp = passphraseEntry.savedPassword(passphrase).build(); Platform.runLater(() -> { window.setScene(comp.passphraseEntryScene()); + window.setTitle(resourceBundle.getString("unlock.title").formatted(vault.getDisplayName())); window.show(); Window owner = window.getOwner(); if (owner != null) { diff --git a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryModule.java b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryModule.java index aaf65e3f8..0d67abd50 100644 --- a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryModule.java +++ b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryModule.java @@ -2,16 +2,13 @@ package org.cryptomator.ui.keyloading.masterkeyfile; import dagger.Module; import dagger.Provides; -import org.cryptomator.common.vaults.Vault; import org.cryptomator.ui.common.DefaultSceneFactory; import org.cryptomator.ui.common.FxmlFile; import org.cryptomator.ui.common.FxmlLoaderFactory; -import org.cryptomator.ui.keyloading.KeyLoading; import javafx.fxml.FXMLLoader; import javafx.scene.Parent; import javafx.scene.Scene; -import javafx.stage.Stage; import java.io.IOException; import java.io.UncheckedIOException; import java.util.ResourceBundle; @@ -28,19 +25,13 @@ interface PassphraseEntryModule { @Provides @PassphraseEntryScoped - static Scene provideUnlockScene(PassphraseEntryController controller, DefaultSceneFactory sceneFactory, ResourceBundle resourceBundle, @KeyLoading Stage window, @KeyLoading Vault v) { + static Scene provideUnlockScene(PassphraseEntryController controller, DefaultSceneFactory sceneFactory, ResourceBundle resourceBundle) { // TODO: simplify FxmlLoaderFactory try { var url = FxmlLoaderFactory.class.getResource(FxmlFile.UNLOCK_ENTER_PASSWORD.getRessourcePathString()); var loader = new FXMLLoader(url, resourceBundle, null, clazz -> controller); Parent root = loader.load(); - var scene = sceneFactory.apply(root); - scene.windowProperty().addListener((prop, oldVal, newVal) -> { - if (window.equals(newVal)) { - window.setTitle(String.format(resourceBundle.getString("unlock.title"), v.getDisplayName())); - } - }); - return scene; + return sceneFactory.apply(root); } catch (IOException e) { throw new UncheckedIOException("Failed to load UnlockScene", e); } From 4d4098e0e0e92f99313ea12dac4a30940e7362e0 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Wed, 19 Jan 2022 19:49:33 +0100 Subject: [PATCH 10/26] cleanup --- src/main/java/org/cryptomator/common/Passphrase.java | 12 ++++-------- .../org/cryptomator/ui/common/FxmlLoaderFactory.java | 4 ++++ .../masterkeyfile/ChooseMasterkeyFileModule.java | 11 +---------- .../masterkeyfile/PassphraseEntryModule.java | 11 +---------- .../masterkeyfile/PassphraseEntryResult.java | 2 +- .../cryptomator/ui/lock/LockForcedController.java | 4 ---- 6 files changed, 11 insertions(+), 33 deletions(-) diff --git a/src/main/java/org/cryptomator/common/Passphrase.java b/src/main/java/org/cryptomator/common/Passphrase.java index d57252507..036e87104 100644 --- a/src/main/java/org/cryptomator/common/Passphrase.java +++ b/src/main/java/org/cryptomator/common/Passphrase.java @@ -64,16 +64,12 @@ public class Passphrase implements Destroyable, CharSequence { @Override public int hashCode() { - // TODO: do we really need to a secure hashcode? toString leaks the pw anyway - var md = MessageDigestSupplier.SHA256.get(); - ByteBuffer buf = ByteBuffer.allocate(Character.BYTES * length); + // basically Arrays.hashCode, but only for a certain subarray + int result = 1; for (int i = 0; i < length; i++) { - char c = charAt(i); - buf.putChar(i * Character.BYTES, c); + result = 31 * result + charAt(i); } - buf.flip(); - md.update(buf); - return Arrays.hashCode(md.digest()); + return result; } @Override diff --git a/src/main/java/org/cryptomator/ui/common/FxmlLoaderFactory.java b/src/main/java/org/cryptomator/ui/common/FxmlLoaderFactory.java index c10054ef4..dcff93aab 100644 --- a/src/main/java/org/cryptomator/ui/common/FxmlLoaderFactory.java +++ b/src/main/java/org/cryptomator/ui/common/FxmlLoaderFactory.java @@ -22,6 +22,10 @@ public class FxmlLoaderFactory { this.resourceBundle = resourceBundle; } + public static FxmlLoaderFactory forController(T controller, Function sceneFactory, ResourceBundle resourceBundle) { + return new FxmlLoaderFactory(Map.of(controller.getClass(), () -> controller), sceneFactory, resourceBundle); + } + /** * @return A new FXMLLoader instance */ diff --git a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/ChooseMasterkeyFileModule.java b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/ChooseMasterkeyFileModule.java index 34eb78443..c7a084584 100644 --- a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/ChooseMasterkeyFileModule.java +++ b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/ChooseMasterkeyFileModule.java @@ -27,16 +27,7 @@ interface ChooseMasterkeyFileModule { @Provides @ChooseMasterkeyFileScoped static Scene provideChooseMasterkeyScene(ChooseMasterkeyFileController 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 ChooseMasterkeyScene", e); - } + return FxmlLoaderFactory.forController(controller, sceneFactory, resourceBundle).createScene(FxmlFile.UNLOCK_SELECT_MASTERKEYFILE); } - } diff --git a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryModule.java b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryModule.java index 0d67abd50..ed1feb7ee 100644 --- a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryModule.java +++ b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryModule.java @@ -26,16 +26,7 @@ interface PassphraseEntryModule { @Provides @PassphraseEntryScoped static Scene provideUnlockScene(PassphraseEntryController controller, DefaultSceneFactory sceneFactory, ResourceBundle resourceBundle) { - // TODO: simplify FxmlLoaderFactory - try { - var url = FxmlLoaderFactory.class.getResource(FxmlFile.UNLOCK_ENTER_PASSWORD.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); - } + return FxmlLoaderFactory.forController(controller, sceneFactory, resourceBundle).createScene(FxmlFile.UNLOCK_ENTER_PASSWORD); } - } diff --git a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryResult.java b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryResult.java index f26184d9d..19057acca 100644 --- a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryResult.java +++ b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryResult.java @@ -2,7 +2,7 @@ package org.cryptomator.ui.keyloading.masterkeyfile; import org.cryptomator.common.Passphrase; -// TODO needs to be public due to Dagger -.- +// TODO: change to package-private, as soon as this works for Dagger -.- public record PassphraseEntryResult(Passphrase passphrase, boolean savePassphrase) { } diff --git a/src/main/java/org/cryptomator/ui/lock/LockForcedController.java b/src/main/java/org/cryptomator/ui/lock/LockForcedController.java index 0b4a23a18..15cf119be 100644 --- a/src/main/java/org/cryptomator/ui/lock/LockForcedController.java +++ b/src/main/java/org/cryptomator/ui/lock/LockForcedController.java @@ -2,8 +2,6 @@ package org.cryptomator.ui.lock; import org.cryptomator.common.vaults.Vault; import org.cryptomator.ui.common.FxController; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import javax.inject.Inject; import javafx.fxml.FXML; @@ -15,8 +13,6 @@ import java.util.concurrent.atomic.AtomicReference; @LockScoped public class LockForcedController implements FxController { - private static final Logger LOG = LoggerFactory.getLogger(LockForcedController.class); - private final Stage window; private final Vault vault; private final AtomicReference> forceRetryDecision; From e1a72c41a5fcbe87c5f7d328ac456e514a59202b Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Wed, 19 Jan 2022 20:01:48 +0100 Subject: [PATCH 11/26] remove unused imports --- src/main/java/org/cryptomator/common/Passphrase.java | 3 --- .../keyloading/masterkeyfile/ChooseMasterkeyFileModule.java | 4 ---- .../ui/keyloading/masterkeyfile/PassphraseEntryModule.java | 4 ---- 3 files changed, 11 deletions(-) diff --git a/src/main/java/org/cryptomator/common/Passphrase.java b/src/main/java/org/cryptomator/common/Passphrase.java index 036e87104..cf64c7a10 100644 --- a/src/main/java/org/cryptomator/common/Passphrase.java +++ b/src/main/java/org/cryptomator/common/Passphrase.java @@ -1,9 +1,6 @@ package org.cryptomator.common; -import org.cryptomator.cryptolib.common.MessageDigestSupplier; - import javax.security.auth.Destroyable; -import java.nio.ByteBuffer; import java.util.Arrays; /** diff --git a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/ChooseMasterkeyFileModule.java b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/ChooseMasterkeyFileModule.java index c7a084584..21ae2b26c 100644 --- a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/ChooseMasterkeyFileModule.java +++ b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/ChooseMasterkeyFileModule.java @@ -6,11 +6,7 @@ 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; diff --git a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryModule.java b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryModule.java index ed1feb7ee..2c65d440b 100644 --- a/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryModule.java +++ b/src/main/java/org/cryptomator/ui/keyloading/masterkeyfile/PassphraseEntryModule.java @@ -6,11 +6,7 @@ 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.util.ResourceBundle; import java.util.concurrent.CompletableFuture; From 79e6a4cd488c4136229c533dd002c53141442f41 Mon Sep 17 00:00:00 2001 From: Kevin St-Sauveur Date: Mon, 14 Mar 2022 02:29:25 -0400 Subject: [PATCH 12/26] Modify vault title when unlocked --- .../java/org/cryptomator/ui/traymenu/TrayMenuController.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/org/cryptomator/ui/traymenu/TrayMenuController.java b/src/main/java/org/cryptomator/ui/traymenu/TrayMenuController.java index 4ce3808c4..66be4c652 100644 --- a/src/main/java/org/cryptomator/ui/traymenu/TrayMenuController.java +++ b/src/main/java/org/cryptomator/ui/traymenu/TrayMenuController.java @@ -91,6 +91,8 @@ class TrayMenuController { unlockItem.addActionListener(createActionListenerForVault(vault, this::unlockVault)); submenu.add(unlockItem); } else if (vault.isUnlocked()) { + submenu.setLabel("*".concat(submenu.getLabel())); + MenuItem lockItem = new MenuItem(resourceBundle.getString("traymenu.vault.lock")); lockItem.addActionListener(createActionListenerForVault(vault, this::lockVault)); submenu.add(lockItem); From 7b4a3e13138d2239965a8c139d36aef329860835 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Fri, 18 Mar 2022 18:06:08 +0100 Subject: [PATCH 13/26] make sure not to upload unrelated artifacts to a release --- .github/workflows/appimage.yml | 6 +++--- .github/workflows/mac-dmg.yml | 4 ++-- .github/workflows/win-exe.yml | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.github/workflows/appimage.yml b/.github/workflows/appimage.yml index 694f6ee45..aab954476 100644 --- a/.github/workflows/appimage.yml +++ b/.github/workflows/appimage.yml @@ -146,6 +146,6 @@ jobs: fail_on_unmatched_files: true token: ${{ secrets.CRYPTOBOT_RELEASE_TOKEN }} files: | - *.AppImage - *.zsync - *.asc \ No newline at end of file + cryptomator-*.AppImage + cryptomator-*.zsync + cryptomator-*.asc \ No newline at end of file diff --git a/.github/workflows/mac-dmg.yml b/.github/workflows/mac-dmg.yml index 58b7e9408..66af92d6d 100644 --- a/.github/workflows/mac-dmg.yml +++ b/.github/workflows/mac-dmg.yml @@ -226,7 +226,7 @@ jobs: fail_on_unmatched_files: true token: ${{ secrets.CRYPTOBOT_RELEASE_TOKEN }} files: | - *.dmg - *.asc + Cryptomator-*.dmg + Cryptomator-*.asc diff --git a/.github/workflows/win-exe.yml b/.github/workflows/win-exe.yml index 9a855b646..f024d7347 100644 --- a/.github/workflows/win-exe.yml +++ b/.github/workflows/win-exe.yml @@ -251,5 +251,5 @@ jobs: fail_on_unmatched_files: true token: ${{ secrets.CRYPTOBOT_RELEASE_TOKEN }} files: | - *.exe - *.asc \ No newline at end of file + Cryptomator-*.exe + Cryptomator-*.asc \ No newline at end of file From 77e2be22dea911e89f8c43033d8e99963067f946 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Fri, 18 Mar 2022 18:06:31 +0100 Subject: [PATCH 14/26] updated .gitignore [ci skip] --- dist/mac/dmg/.gitignore | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dist/mac/dmg/.gitignore b/dist/mac/dmg/.gitignore index c186170c9..b8ef35283 100644 --- a/dist/mac/dmg/.gitignore +++ b/dist/mac/dmg/.gitignore @@ -1,4 +1,5 @@ # created during build +Cryptomator.app/ runtime/ dmg/ -*.dmg +*.dmg \ No newline at end of file From 4a3d579b76b5e8f7906d447d40a03473e442b0aa Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Mon, 21 Mar 2022 14:01:35 +0100 Subject: [PATCH 15/26] changed button title as suggested in review --- src/main/resources/fxml/unlock_select_masterkeyfile.fxml | 2 +- src/main/resources/i18n/strings.properties | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/resources/fxml/unlock_select_masterkeyfile.fxml b/src/main/resources/fxml/unlock_select_masterkeyfile.fxml index 43f2f3e46..d37289fca 100644 --- a/src/main/resources/fxml/unlock_select_masterkeyfile.fxml +++ b/src/main/resources/fxml/unlock_select_masterkeyfile.fxml @@ -34,7 +34,7 @@