Browse Source

Cut off the application from accessing keychains at the earliest point possible
as suggested in the discussion to #2445

Ralph Plawetzki 2 years ago
parent
commit
2e3d2e86e2

+ 2 - 9
src/main/java/org/cryptomator/common/keychain/KeychainManager.java

@@ -3,7 +3,6 @@ package org.cryptomator.common.keychain;
 import com.google.common.cache.CacheBuilder;
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
-import org.cryptomator.common.settings.Settings;
 import org.cryptomator.integrations.keychain.KeychainAccessException;
 import org.cryptomator.integrations.keychain.KeychainAccessProvider;
 
@@ -21,12 +20,10 @@ public class KeychainManager implements KeychainAccessProvider {
 
 	private final ObjectExpression<KeychainAccessProvider> keychain;
 	private final LoadingCache<String, BooleanProperty> passphraseStoredProperties;
-	private final Settings settings;
 
 	@Inject
-	KeychainManager(ObjectExpression<KeychainAccessProvider> selectedKeychain, Settings settings) {
+	KeychainManager(ObjectExpression<KeychainAccessProvider> selectedKeychain) {
 		this.keychain = selectedKeychain;
-		this.settings = settings;
 		this.passphraseStoredProperties = CacheBuilder.newBuilder() //
 				.weakValues() //
 				.build(CacheLoader.from(this::createStoredPassphraseProperty));
@@ -74,9 +71,7 @@ public class KeychainManager implements KeychainAccessProvider {
 	}
 
 	@Override
-	public boolean isSupported() {
-		return keychain.getValue() != null && settings.useKeychain().get();
-	}
+	public boolean isSupported() { return keychain.getValue() != null; }
 
 	@Override
 	public boolean isLocked() {
@@ -93,8 +88,6 @@ public class KeychainManager implements KeychainAccessProvider {
 	 * @throws KeychainAccessException
 	 */
 	public boolean isPassphraseStored(String key) throws KeychainAccessException {
-		// check if keyrings are disabled; in this case we don't need to ask the backend
-		if ( !settings.useKeychain().get() ) return false;
 		char[] storedPw = null;
 		try {
 			storedPw = getKeychainOrFail().loadPassphrase(key);

+ 2 - 1
src/main/java/org/cryptomator/common/keychain/KeychainModule.java

@@ -23,11 +23,12 @@ public class KeychainModule {
 	@Singleton
 	static ObjectExpression<KeychainAccessProvider> provideKeychainAccessProvider(Settings settings, List<KeychainAccessProvider> providers) {
 		return Bindings.createObjectBinding(() -> {
+			if ( !settings.useKeychain().get() ) return null;
 			var selectedProviderClass = settings.keychainProvider().get();
 			var selectedProvider = providers.stream().filter(provider -> provider.getClass().getName().equals(selectedProviderClass)).findAny();
 			var fallbackProvider = providers.stream().findFirst().orElse(null);
 			return selectedProvider.orElse(fallbackProvider);
-		}, settings.keychainProvider());
+		}, settings.keychainProvider(), settings.useKeychain());
 	}
 
 }

+ 2 - 5
src/test/java/org/cryptomator/common/keychain/KeychainManagerTest.java

@@ -1,14 +1,12 @@
 package org.cryptomator.common.keychain;
 
 
-import org.cryptomator.common.settings.Settings;
 import org.cryptomator.integrations.keychain.KeychainAccessException;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Assumptions;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
-import org.mockito.Mockito;
 
 import javafx.application.Platform;
 import javafx.beans.property.ReadOnlyBooleanProperty;
@@ -21,10 +19,9 @@ import java.util.concurrent.atomic.AtomicBoolean;
 
 public class KeychainManagerTest {
 
-	private static final Settings settings = Mockito.mock(Settings.class);
 	@Test
 	public void testStoreAndLoad() throws KeychainAccessException {
-		KeychainManager keychainManager = new KeychainManager(new SimpleObjectProperty<>(new MapKeychainAccess()), settings);
+		KeychainManager keychainManager = new KeychainManager(new SimpleObjectProperty<>(new MapKeychainAccess()));
 		keychainManager.storePassphrase("test", "Test", "asd");
 		Assertions.assertArrayEquals("asd".toCharArray(), keychainManager.loadPassphrase("test"));
 	}
@@ -42,7 +39,7 @@ public class KeychainManagerTest {
 
 		@Test
 		public void testPropertyChangesWhenStoringPassword() throws KeychainAccessException, InterruptedException {
-			KeychainManager keychainManager = new KeychainManager(new SimpleObjectProperty<>(new MapKeychainAccess()), settings);
+			KeychainManager keychainManager = new KeychainManager(new SimpleObjectProperty<>(new MapKeychainAccess()));
 			ReadOnlyBooleanProperty property = keychainManager.getPassphraseStoredProperty("test");
 			Assertions.assertFalse(property.get());