Refactored KeychainAccess in preparation of #1183, #1182

main/keychain/src/main/java/org/cryptomator/keychain/KeychainAccess.java

@@ -1,38 +0,0 @@
-/*******************************************************************************
- * Copyright (c) 2017 Skymatic UG (haftungsbeschränkt).
- * All rights reserved. This program and the accompanying materials
- * are made available under the terms of the accompanying LICENSE file.
- *******************************************************************************/
-package org.cryptomator.keychain;
-
-public interface KeychainAccess {
-
-	/**
-	 * Associates a passphrase with a given key.
-	 * 
-	 * @param key Key used to retrieve the passphrase via {@link #loadPassphrase(String)}.
-	 * @param passphrase The secret to store in this keychain.
-	 */
-	void storePassphrase(String key, CharSequence passphrase) throws KeychainAccessException;
-
-	/**
-	 * @param key Unique key previously used while {@link #storePassphrase(String, CharSequence) storing a passphrase}.
-	 * @return The stored passphrase for the given key or <code>null</code> if no value for the given key could be found.
-	 */
-	char[] loadPassphrase(String key) throws KeychainAccessException;
-
-	/**
-	 * Deletes a passphrase with a given key.
-	 * 
-	 * @param key Unique key previously used while {@link #storePassphrase(String, CharSequence) storing a passphrase}.
-	 */
-	void deletePassphrase(String key) throws KeychainAccessException;
-
-	/**
-	 * Updates a passphrase with a given key.
-	 *
-	 * @param key Unique key previously used while {@link #storePassphrase(String, CharSequence) storing a passphrase}.
-	 * @param passphrase The secret to be updated in this keychain.
-	 */
-	void changePassphrase(String key, CharSequence passphrase) throws KeychainAccessException;
-}

main/keychain/src/main/java/org/cryptomator/keychain/KeychainAccessStrategy.java

@@ -5,7 +5,36 @@
  *******************************************************************************/
 package org.cryptomator.keychain;
 
-interface KeychainAccessStrategy extends KeychainAccess {
+interface KeychainAccessStrategy {
+
+	/**
+	 * Associates a passphrase with a given key.
+	 *
+	 * @param key Key used to retrieve the passphrase via {@link #loadPassphrase(String)}.
+	 * @param passphrase The secret to store in this keychain.
+	 */
+	void storePassphrase(String key, CharSequence passphrase) throws KeychainAccessException;
+
+	/**
+	 * @param key Unique key previously used while {@link #storePassphrase(String, CharSequence) storing a passphrase}.
+	 * @return The stored passphrase for the given key or <code>null</code> if no value for the given key could be found.
+	 */
+	char[] loadPassphrase(String key) throws KeychainAccessException;
+
+	/**
+	 * Deletes a passphrase with a given key.
+	 *
+	 * @param key Unique key previously used while {@link #storePassphrase(String, CharSequence) storing a passphrase}.
+	 */
+	void deletePassphrase(String key) throws KeychainAccessException;
+
+	/**
+	 * Updates a passphrase with a given key.
+	 *
+	 * @param key Unique key previously used while {@link #storePassphrase(String, CharSequence) storing a passphrase}.
+	 * @param passphrase The secret to be updated in this keychain.
+	 */
+	void changePassphrase(String key, CharSequence passphrase) throws KeychainAccessException;
 
 	/**
 	 * @return <code>true</code> if this KeychainAccessStrategy works on the current machine.

main/keychain/src/main/java/org/cryptomator/keychain/KeychainManager.java

@@ -0,0 +1,117 @@
+package org.cryptomator.keychain;
+
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
+import javafx.application.Platform;
+import javafx.beans.property.BooleanProperty;
+import javafx.beans.property.ReadOnlyBooleanProperty;
+import javafx.beans.property.SimpleBooleanProperty;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Arrays;
+
+public class KeychainManager implements KeychainAccessStrategy {
+
+	private static final Logger LOG = LoggerFactory.getLogger(KeychainManager.class);
+
+	private final KeychainAccessStrategy keychain;
+	private LoadingCache<String, BooleanProperty> passphraseStoredProperties;
+
+	KeychainManager(KeychainAccessStrategy keychain) {
+		assert keychain.isSupported();
+		this.keychain = keychain;
+		this.passphraseStoredProperties = CacheBuilder.newBuilder() //
+				.weakValues() //
+				.build(CacheLoader.from(this::createStoredPassphraseProperty));
+	}
+
+	@Override
+	public void storePassphrase(String key, CharSequence passphrase) throws KeychainAccessException {
+		keychain.storePassphrase(key, passphrase);
+		setPassphraseStored(key, true);
+	}
+
+	@Override
+	public char[] loadPassphrase(String key) throws KeychainAccessException {
+		char[] passphrase = keychain.loadPassphrase(key);
+		setPassphraseStored(key, passphrase != null);
+		return passphrase;
+	}
+
+	@Override
+	public void deletePassphrase(String key) throws KeychainAccessException {
+		keychain.deletePassphrase(key);
+		setPassphraseStored(key, false);
+	}
+
+	@Override
+	public void changePassphrase(String key, CharSequence passphrase) throws KeychainAccessException {
+		keychain.changePassphrase(key, passphrase);
+		setPassphraseStored(key, true);
+	}
+
+	@Override
+	public boolean isSupported() {
+		return true;
+	}
+
+	/**
+	 * Checks if the keychain knows a passphrase for the given key.
+	 * <p>
+	 * Expensive operation. If possible, use {@link #getPassphraseStoredProperty(String)} instead.
+	 *
+	 * @param key The key to look up
+	 * @return <code>true</code> if a password for <code>key</code> is stored.
+	 * @throws KeychainAccessException
+	 */
+	public boolean isPassphraseStored(String key) throws KeychainAccessException {
+		char[] storedPw = null;
+		try {
+			storedPw = keychain.loadPassphrase(key);
+			return storedPw != null;
+		} finally {
+			if (storedPw != null) {
+				Arrays.fill(storedPw, ' ');
+			}
+		}
+	}
+	
+	private void setPassphraseStored(String key, boolean value) {
+		BooleanProperty property = passphraseStoredProperties.getIfPresent(key);
+		if (property != null) {
+			if (Platform.isFxApplicationThread()) {
+				property.set(value);
+			} else {
+				LOG.warn("");
+				Platform.runLater(() -> property.set(value));
+			}
+		}
+	}
+
+	/**
+	 * Returns an observable property for use in the UI that tells whether a passphrase is stored for the given key.
+	 * <p>
+	 * Assuming that this process is the only process modifying Cryptomator-related items in the system keychain, this
+	 * property stays in memory in an attempt to avoid unnecessary calls to the system keychain. Note that due to this
+	 * fact the value stored in the returned property is not 100% reliable. Code defensively!
+	 *
+	 * @param key The key to look up
+	 * @return An observable property which is <code>true</code> when it almost certain that a password for <code>key</code> is stored.
+	 * @see #isPassphraseStored(String) 
+	 */
+	public ReadOnlyBooleanProperty getPassphraseStoredProperty(String key) {
+		return passphraseStoredProperties.getUnchecked(key);
+	}
+
+	private BooleanProperty createStoredPassphraseProperty(String key) {
+		try {
+			LOG.warn("LOAD"); // TODO remove
+			return new SimpleBooleanProperty(isPassphraseStored(key));
+		} catch (KeychainAccessException e) {
+			return new SimpleBooleanProperty(false);
+		}
+	}
+
+}

main/keychain/src/main/java/org/cryptomator/keychain/KeychainModule.java

@@ -5,10 +5,10 @@
  *******************************************************************************/
 package org.cryptomator.keychain;
 
-import com.google.common.collect.Sets;
+import dagger.Binds;
 import dagger.Module;
 import dagger.Provides;
-import dagger.multibindings.ElementsIntoSet;
+import dagger.multibindings.IntoSet;
 import org.cryptomator.common.JniModule;
 
 import javax.inject.Singleton;
@@ -16,18 +16,30 @@ import java.util.Optional;
 import java.util.Set;
 
 @Module(includes = {JniModule.class})
-public class KeychainModule {
+public abstract class KeychainModule {
 
-	@Provides
-	@ElementsIntoSet
-	Set<KeychainAccessStrategy> provideKeychainAccessStrategies(MacSystemKeychainAccess macKeychain, WindowsProtectedKeychainAccess winKeychain, LinuxSecretServiceKeychainAccess linKeychain) {
-		return Sets.newHashSet(macKeychain, winKeychain, linKeychain);
-	}
+	@Binds
+	@IntoSet
+	abstract KeychainAccessStrategy bindMacSystemKeychainAccess(MacSystemKeychainAccess keychainAccessStrategy);
+
+	@Binds
+	@IntoSet
+	abstract KeychainAccessStrategy bindWindowsProtectedKeychainAccess(WindowsProtectedKeychainAccess keychainAccessStrategy);
+	
+	@Binds
+	@IntoSet
+	abstract KeychainAccessStrategy bindLinuxSecretServiceKeychainAccess(LinuxSecretServiceKeychainAccess keychainAccessStrategy);
 
 	@Provides
 	@Singleton
-	public Optional<KeychainAccess> provideSupportedKeychain(Set<KeychainAccessStrategy> keychainAccessStrategies) {
-		return keychainAccessStrategies.stream().filter(KeychainAccessStrategy::isSupported).map(KeychainAccess.class::cast).findFirst();
+	static Optional<KeychainAccessStrategy> provideSupportedKeychain(Set<KeychainAccessStrategy> keychainAccessStrategies) {
+		return keychainAccessStrategies.stream().filter(KeychainAccessStrategy::isSupported).findFirst();
+	}
+	
+	@Provides
+	@Singleton
+	public static Optional<KeychainManager> provideKeychainManager(Optional<KeychainAccessStrategy> keychainAccess) {
+		return keychainAccess.map(KeychainManager::new);
 	}
 
 }

main/keychain/src/main/java/org/cryptomator/keychain/LinuxSecretServiceKeychainAccess.java

@@ -3,11 +3,13 @@ package org.cryptomator.keychain;
 import org.apache.commons.lang3.SystemUtils;
 
 import javax.inject.Inject;
+import javax.inject.Singleton;
 import java.util.Optional;
 
 /**
  * A facade to LinuxSecretServiceKeychainAccessImpl that doesn't depend on libraries that are unavailable on Mac and Windows.
  */
+@Singleton
 public class LinuxSecretServiceKeychainAccess implements KeychainAccessStrategy {
 
 	// the actual implementation is hidden in this delegate object which is loaded via reflection,

main/keychain/src/main/java/org/cryptomator/keychain/MacSystemKeychainAccess.java

@@ -8,11 +8,13 @@ package org.cryptomator.keychain;
 import java.util.Optional;
 
 import javax.inject.Inject;
+import javax.inject.Singleton;
 
 import org.apache.commons.lang3.SystemUtils;
 import org.cryptomator.jni.MacFunctions;
 import org.cryptomator.jni.MacKeychainAccess;
 
+@Singleton
 class MacSystemKeychainAccess implements KeychainAccessStrategy {
 
 	private final Optional<MacFunctions> macFunctions;

main/keychain/src/main/java/org/cryptomator/keychain/WindowsProtectedKeychainAccess.java

@@ -25,6 +25,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import javax.inject.Inject;
+import javax.inject.Singleton;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
@@ -50,6 +51,7 @@ import java.util.stream.Collectors;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
 
+@Singleton
 class WindowsProtectedKeychainAccess implements KeychainAccessStrategy {
 
 	private static final Logger LOG = LoggerFactory.getLogger(WindowsProtectedKeychainAccess.class);

main/keychain/src/test/java/org/cryptomator/keychain/KeychainManagerTest.java

@@ -0,0 +1,55 @@
+package org.cryptomator.keychain;
+
+
+import javafx.application.Platform;
+import javafx.beans.property.ReadOnlyBooleanProperty;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.Test;
+
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+
+
+class KeychainManagerTest {
+	
+	@Test
+	public void testStoreAndLoad() throws KeychainAccessException {
+		KeychainManager keychainManager = new KeychainManager(new MapKeychainAccess());
+		keychainManager.storePassphrase("test", "asd");
+		Assertions.assertArrayEquals("asd".toCharArray(), keychainManager.loadPassphrase("test"));
+	}
+	
+	@Nested
+	public static class WhenObservingProperties {
+
+		@BeforeAll
+		public static void startup() throws InterruptedException {
+			CountDownLatch latch = new CountDownLatch(1);
+			Platform.startup(latch::countDown);
+			latch.await(5, TimeUnit.SECONDS);
+		}
+		
+		@Test
+		public void testPropertyChangesWhenStoringPassword() throws KeychainAccessException, InterruptedException {
+			KeychainManager keychainManager = new KeychainManager(new MapKeychainAccess());
+			ReadOnlyBooleanProperty property = keychainManager.getPassphraseStoredProperty("test");
+			Assertions.assertEquals(false, property.get());
+			
+			keychainManager.storePassphrase("test", "bar");
+			
+			AtomicBoolean result = new AtomicBoolean(false);
+			CountDownLatch latch = new CountDownLatch(1);
+			Platform.runLater(() -> {
+				result.set(property.get());
+				latch.countDown();
+			});
+			latch.await(1, TimeUnit.SECONDS);
+			Assertions.assertEquals(true, result.get());
+		}
+		
+	}
+
+}

main/keychain/src/test/java/org/cryptomator/keychain/KeychainModuleTest.java

@@ -1,24 +0,0 @@
-/*******************************************************************************
- * Copyright (c) 2017 Skymatic UG (haftungsbeschränkt).
- * All rights reserved. This program and the accompanying materials
- * are made available under the terms of the accompanying LICENSE file.
- *******************************************************************************/
-package org.cryptomator.keychain;
-
-import org.junit.jupiter.api.Assertions;
-import org.junit.jupiter.api.Test;
-
-import java.util.Optional;
-
-public class KeychainModuleTest {
-
-	@Test
-	public void testGetKeychain() throws KeychainAccessException {
-		Optional<KeychainAccess> keychainAccess = DaggerTestKeychainComponent.builder().keychainModule(new TestKeychainModule()).build().keychainAccess();
-		Assertions.assertTrue(keychainAccess.isPresent());
-		Assertions.assertTrue(keychainAccess.get() instanceof MapKeychainAccess);
-		keychainAccess.get().storePassphrase("test", "asd");
-		Assertions.assertArrayEquals("asd".toCharArray(), keychainAccess.get().loadPassphrase("test"));
-	}
-
-}

main/keychain/src/test/java/org/cryptomator/keychain/TestKeychainComponent.java

@@ -1,19 +0,0 @@
-/*******************************************************************************
- * Copyright (c) 2017 Skymatic UG (haftungsbeschränkt).
- * All rights reserved. This program and the accompanying materials
- * are made available under the terms of the accompanying LICENSE file.
- *******************************************************************************/
-package org.cryptomator.keychain;
-
-import dagger.Component;
-
-import javax.inject.Singleton;
-import java.util.Optional;
-
-@Singleton
-@Component(modules = KeychainModule.class)
-interface TestKeychainComponent {
-
-	Optional<KeychainAccess> keychainAccess();
-
-}

main/keychain/src/test/java/org/cryptomator/keychain/TestKeychainModule.java

@@ -1,17 +0,0 @@
-/*******************************************************************************
- * Copyright (c) 2017 Skymatic UG (haftungsbeschränkt).
- * All rights reserved. This program and the accompanying materials
- * are made available under the terms of the accompanying LICENSE file.
- *******************************************************************************/
-package org.cryptomator.keychain;
-
-import java.util.Set;
-
-public class TestKeychainModule extends KeychainModule {
-
-	@Override
-	Set<KeychainAccessStrategy> provideKeychainAccessStrategies(MacSystemKeychainAccess macKeychain, WindowsProtectedKeychainAccess winKeychain, LinuxSecretServiceKeychainAccess linKeychain) {
-		return Set.of(new MapKeychainAccess());
-	}
-
-}