replace config manually instead of using CryptoFileSystem.init()

src/main/java/org/cryptomator/ui/convertvault/HubToPasswordConvertController.java

@@ -2,12 +2,12 @@ package org.cryptomator.ui.convertvault;
 
 import com.google.common.base.Preconditions;
 import dagger.Lazy;
+import org.cryptomator.common.Constants;
 import org.cryptomator.common.Passphrase;
 import org.cryptomator.common.vaults.Vault;
-import org.cryptomator.cryptofs.CryptoFileSystemProperties;
-import org.cryptomator.cryptofs.CryptoFileSystemProvider;
+import org.cryptomator.cryptofs.VaultConfig;
+import org.cryptomator.cryptofs.VaultVersionMismatchException;
 import org.cryptomator.cryptofs.common.BackupHelper;
-import org.cryptomator.cryptolib.api.MasterkeyLoader;
 import org.cryptomator.cryptolib.api.MasterkeyLoadingFailedException;
 import org.cryptomator.cryptolib.common.MasterkeyFileAccess;
 import org.cryptomator.ui.changepassword.NewPasswordController;
@@ -31,6 +31,7 @@ import javafx.scene.control.Button;
 import javafx.scene.control.ContentDisplay;
 import javafx.stage.Stage;
 import java.io.IOException;
+import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.StandardCopyOption;
@@ -39,7 +40,8 @@ import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.CompletionException;
 import java.util.concurrent.ExecutorService;
 
-import static org.cryptomator.common.Constants.DEFAULT_KEY_ID;
+import static java.nio.file.StandardOpenOption.CREATE_NEW;
+import static java.nio.file.StandardOpenOption.WRITE;
 import static org.cryptomator.common.Constants.MASTERKEY_BACKUP_SUFFIX;
 import static org.cryptomator.common.Constants.MASTERKEY_FILENAME;
 import static org.cryptomator.common.Constants.VAULTCONFIG_FILENAME;
@@ -100,9 +102,9 @@ public class HubToPasswordConvertController implements FxController {
 	@FXML
 	public void convert() {
 		Preconditions.checkState(newPasswordController.isGoodPassword());
-		LOG.info("Converting hub vault {} to password", vault.getPath());
+		LOG.info("Converting access method of vault {} from hub to password", vault.getPath());
 		CompletableFuture.runAsync(() -> conversionStarted.setValue(true), Platform::runLater) //
-				.thenRunAsync(this::convertInternal, backgroundExecutorService)
+				.thenRunAsync(this::convertInternal, backgroundExecutorService) //
 				.whenCompleteAsync((result, exception) -> {
 					if (exception == null) {
 						LOG.info("Conversion of vault {} succeeded.", vault.getPath());
@@ -117,11 +119,19 @@ public class HubToPasswordConvertController implements FxController {
 	//visible for testing
 	void convertInternal() throws CompletionException, IllegalArgumentException {
 		var passphrase = newPasswordController.getNewPassword();
+		var vaultPath = vault.getPath();
 		try {
-			recoveryKeyFactory.newMasterkeyFileWithPassphrase(vault.getPath(), recoveryKey.get(), passphrase);
-			LOG.debug("Successfully created masterkey file for vault {}", vault.getPath());
-			backupHubConfig(vault.getPath().resolve(VAULTCONFIG_FILENAME));
-			replaceWithPasswordConfig(passphrase);
+			//create masterkey
+			recoveryKeyFactory.newMasterkeyFileWithPassphrase(vaultPath, recoveryKey.get(), passphrase);
+			LOG.debug("Successfully created masterkey file for vault {}", vaultPath);
+			//create password config
+			Path passwordConfigPath = vaultPath.resolve("passwordBased." + VAULTCONFIG_FILENAME + ".tmp");
+			passwordConfigPath = createPasswordConfig(passwordConfigPath, vaultPath.resolve(MASTERKEY_FILENAME), passphrase);
+			//backup hub config
+			var hubConfigPath = vaultPath.resolve(VAULTCONFIG_FILENAME);
+			backupHubConfig(hubConfigPath);
+			//replace hub by password
+			Files.move(passwordConfigPath, hubConfigPath, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.ATOMIC_MOVE);
 		} catch (MasterkeyLoadingFailedException e) {
 			throw new CompletionException(new IOException("Vault conversion failed", e));
 		} catch (IOException e) {
@@ -134,25 +144,28 @@ public class HubToPasswordConvertController implements FxController {
 	//visible for testing
 	void backupHubConfig(Path hubConfigPath) throws IOException {
 		byte[] hubConfigBytes = Files.readAllBytes(hubConfigPath);
-		Path backupPath = vault.getPath().resolve(VAULTCONFIG_FILENAME + BackupHelper.generateFileIdSuffix(hubConfigBytes) + MASTERKEY_BACKUP_SUFFIX);
-		Files.move(hubConfigPath, backupPath, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.ATOMIC_MOVE); //TODO: should this be an atomic move?
-		LOG.debug("Successfully created vault config backup {} for vault {}", backupPath.getFileName(), vault.getPath());
+		Path backupPath = hubConfigPath.resolveSibling(VAULTCONFIG_FILENAME + BackupHelper.generateFileIdSuffix(hubConfigBytes) + MASTERKEY_BACKUP_SUFFIX);
+		Files.copy(hubConfigPath, backupPath, StandardCopyOption.REPLACE_EXISTING);
+		LOG.debug("Successfully created hub config backup {}", backupPath.getFileName());
 	}
 
 	//visible for testing
-	void replaceWithPasswordConfig(Passphrase passphrase) throws IOException, MasterkeyLoadingFailedException {
+	Path createPasswordConfig(Path passwordConfigPath, Path masterkeyFile, Passphrase passphrase) throws IOException, MasterkeyLoadingFailedException {
 		var unverifiedVaultConfig = vault.getVaultConfigCache().get();
-		try (var masterkey = masterkeyFileAccess.load(vault.getPath().resolve(MASTERKEY_FILENAME), passphrase)) {
-			var vaultConfig = unverifiedVaultConfig.verify(masterkey.getEncoded(), unverifiedVaultConfig.allegedVaultVersion());
-			MasterkeyLoader loader = ignored -> masterkey.copy();
-			CryptoFileSystemProperties fsProps = CryptoFileSystemProperties.cryptoFileSystemProperties() //
-					.withCipherCombo(vaultConfig.getCipherCombo()) //
-					.withKeyLoader(loader) //
+		try (var masterkey = masterkeyFileAccess.load(masterkeyFile, passphrase)) {
+			var hubConfig = unverifiedVaultConfig.verify(masterkey.getEncoded(), unverifiedVaultConfig.allegedVaultVersion());
+			var passwordConfig = VaultConfig.createNew() //
+					.cipherCombo(hubConfig.getCipherCombo()) //
+					.shorteningThreshold(hubConfig.getShorteningThreshold()) //
 					.build();
-			CryptoFileSystemProvider.initialize(vault.getPath(), fsProps, DEFAULT_KEY_ID);
+			if (passwordConfig.getVaultVersion() != hubConfig.getVaultVersion()) {
+				throw new VaultVersionMismatchException("Only vaults of version " + passwordConfig.getVaultVersion() + "can be converted.");
+			}
+			var token = passwordConfig.toToken(Constants.DEFAULT_KEY_ID.toString(), masterkey.getEncoded());
+			Files.writeString(passwordConfigPath, token, StandardCharsets.US_ASCII, WRITE, CREATE_NEW);
+			LOG.debug("Successfully created password config {}", passwordConfigPath);
+			return passwordConfigPath;
 		}
 	}
 
-	/* Getter/Setter */
-
 }

src/test/java/org/cryptomator/ui/convertvault/HubToPasswordConvertControllerTest.java

@@ -4,6 +4,10 @@ import dagger.Lazy;
 import org.cryptomator.common.Constants;
 import org.cryptomator.common.Passphrase;
 import org.cryptomator.common.vaults.Vault;
+import org.cryptomator.common.vaults.VaultConfigCache;
+import org.cryptomator.cryptofs.VaultConfig;
+import org.cryptomator.cryptolib.api.CryptorProvider;
+import org.cryptomator.cryptolib.api.Masterkey;
 import org.cryptomator.cryptolib.api.MasterkeyLoadingFailedException;
 import org.cryptomator.cryptolib.common.MasterkeyFileAccess;
 import org.cryptomator.ui.changepassword.NewPasswordController;
@@ -11,6 +15,7 @@ import org.cryptomator.ui.fxapp.FxApplicationWindows;
 import org.cryptomator.ui.recoverykey.RecoveryKeyFactory;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.io.TempDir;
@@ -25,17 +30,24 @@ import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.StandardOpenOption;
+import java.security.NoSuchAlgorithmException;
+import java.security.SecureRandom;
 import java.util.Optional;
 import java.util.ResourceBundle;
 import java.util.concurrent.CompletionException;
 import java.util.concurrent.ExecutorService;
+import java.util.concurrent.atomic.AtomicReference;
 
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.times;
 
 public class HubToPasswordConvertControllerTest {
 
+	@TempDir
+	Path tmpDir;
+
 	Stage window;
 	Vault vault;
 	StringProperty recoveryKey;
@@ -66,14 +78,14 @@ public class HubToPasswordConvertControllerTest {
 		newPasswordController = Mockito.mock(NewPasswordController.class);
 		inTest = new HubToPasswordConvertController(window, successScene, appWindows, vault, recoveryKey, recoveryKeyFactory, masterkeyFileAccess, backgroundExecutorService, resourceBundle);
 		inTest.newPasswordController = newPasswordController;
+		Mockito.when(vault.getPath()).thenReturn(tmpDir);
 	}
 
 	@Test
-	public void testBackupHubConfig(@TempDir Path tmpDir) throws IOException {
+	public void testBackupHubConfig() throws IOException {
+		var configContent = "Hello Config!".getBytes();
 		Path configPath = tmpDir.resolve(Constants.VAULTCONFIG_FILENAME);
-		Files.writeString(configPath, "hello config", StandardCharsets.UTF_8, StandardOpenOption.CREATE, StandardOpenOption.TRUNCATE_EXISTING);
-
-		Mockito.when(vault.getPath()).thenReturn(tmpDir);
+		Files.write(configPath, configContent, StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE);
 
 		inTest.backupHubConfig(configPath);
 		Optional<Path> result = Files.list(tmpDir).filter(p -> {
@@ -81,17 +93,60 @@ public class HubToPasswordConvertControllerTest {
 			return fileName.startsWith(Constants.VAULTCONFIG_FILENAME) && fileName.endsWith(Constants.MASTERKEY_BACKUP_SUFFIX);
 		}).findAny();
 
-		Assertions.assertTrue(Files.notExists(configPath));
+		Assertions.assertTrue(Files.exists(configPath));
 		Assertions.assertTrue(result.isPresent());
-		Assertions.assertEquals("hello config", Files.readString(result.get(), StandardCharsets.UTF_8));
+		Assertions.assertArrayEquals(configContent, Files.readAllBytes(result.get()));
+	}
+
+
+	@Test
+	@DisplayName("createPasswordConfig creates valid config with password key id")
+	public void integrationTestCreatePasswordConfig(@TempDir Path tmpDir) throws NoSuchAlgorithmException, IOException {
+		//prepare
+		var csprng = SecureRandom.getInstanceStrong();
+		var key = Masterkey.generate(csprng);
+		var masterkeyPath = tmpDir.resolve("masterkey");
+		MasterkeyFileAccess mkAccess = new MasterkeyFileAccess(Constants.PEPPER, csprng);
+		mkAccess.persist(key, masterkeyPath, "");
+
+		var config = VaultConfig.createNew().cipherCombo(CryptorProvider.Scheme.SIV_GCM).shorteningThreshold(42).build();
+		var token = config.toToken("test", key.getEncoded());
+		var hubConfig = VaultConfig.decode(token);
+		var configCache = Mockito.mock(VaultConfigCache.class);
+		Mockito.when(vault.getVaultConfigCache()).thenReturn(configCache);
+		Mockito.when(configCache.get()).thenReturn(hubConfig);
+
+		var passwordConfigPath = tmpDir.resolve("passwordConfig");
+
+		inTest = new HubToPasswordConvertController(window, successScene, appWindows, vault, recoveryKey, recoveryKeyFactory, mkAccess, backgroundExecutorService, resourceBundle);
+
+		//execute
+		Path result = inTest.createPasswordConfig(passwordConfigPath, masterkeyPath, Passphrase.copyOf(""));
+
+		//check
+		AtomicReference<VaultConfig.UnverifiedVaultConfig> unverifiedCfg = new AtomicReference<>();
+		AtomicReference<VaultConfig> cfg = new AtomicReference<>();
+		Assertions.assertTrue(Files.exists(result));
+		Assertions.assertDoesNotThrow(() -> {
+			var tmp = VaultConfig.decode(Files.readString(result, StandardCharsets.US_ASCII));
+			unverifiedCfg.set(tmp);
+		});
+		Assertions.assertDoesNotThrow(() -> {
+			var tmp = unverifiedCfg.get().verify(key.getEncoded(), config.getVaultVersion());
+			cfg.set(tmp);
+		});
+		Assertions.assertAll( //
+				() -> Assertions.assertEquals(config.getCipherCombo(), cfg.get().getCipherCombo()), //
+				() -> Assertions.assertEquals(config.getVaultVersion(), cfg.get().getVaultVersion()), //
+				() -> Assertions.assertEquals(config.getShorteningThreshold(), cfg.get().getShorteningThreshold()), //
+				() -> Assertions.assertEquals(Constants.DEFAULT_KEY_ID, unverifiedCfg.get().getKeyId()));
 	}
 
 	@Nested
 	class ConvertInternalTests {
 
+
 		Passphrase passphrase = Mockito.mock(Passphrase.class);
-		Path vaultPath = Mockito.mock(Path.class, "/vault/path");
-		Path configPath = Mockito.mock(Path.class, "/vault/path/config");
 		String actualRecoveryKey = "recoveryKey";
 		HubToPasswordConvertController inSpy;
 
@@ -100,28 +155,33 @@ public class HubToPasswordConvertControllerTest {
 			inSpy = Mockito.spy(inTest);
 			Mockito.when(newPasswordController.getNewPassword()).thenReturn(passphrase);
 			Mockito.when(recoveryKey.get()).thenReturn(actualRecoveryKey);
-			Mockito.when(vault.getPath()).thenReturn(vaultPath);
-			Mockito.when(vaultPath.resolve(anyString())).thenReturn(configPath);
 			Mockito.doNothing().when(recoveryKeyFactory).newMasterkeyFileWithPassphrase(any(), anyString(), any());
 			Mockito.doNothing().when(inSpy).backupHubConfig(any());
-			Mockito.doNothing().when(inSpy).replaceWithPasswordConfig(any());
 			Mockito.doNothing().when(passphrase).destroy();
 		}
 
 
 		@Test
 		public void testConvertInternal() throws IOException {
+			var passwordConfigContent = "Hello Config!".getBytes();
+			Path passwordConfigPath = tmpDir.resolve("passwordConfig");
+			Path configPath = tmpDir.resolve(Constants.VAULTCONFIG_FILENAME);
+			Files.write(passwordConfigPath, passwordConfigContent, StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE);
+			Mockito.doReturn(passwordConfigPath).when(inSpy).createPasswordConfig(any(), any(), eq(passphrase));
+
 			inSpy.convertInternal();
 
-			Mockito.verify(recoveryKeyFactory, times(1)).newMasterkeyFileWithPassphrase(vaultPath, actualRecoveryKey, passphrase);
-			Mockito.verify(inSpy, times(1)).backupHubConfig(configPath);
-			Mockito.verify(inSpy, times(1)).replaceWithPasswordConfig(passphrase);
-			Mockito.verify(passphrase, times(1)).destroy();
+			var inOrder = Mockito.inOrder(inSpy, recoveryKeyFactory, passphrase);
+			inOrder.verify(recoveryKeyFactory).newMasterkeyFileWithPassphrase(tmpDir, actualRecoveryKey, passphrase);
+			inOrder.verify(inSpy).createPasswordConfig(any(), Mockito.any(), eq(passphrase));
+			inOrder.verify(inSpy).backupHubConfig(configPath);
+			inOrder.verify(passphrase).destroy();
+			Assertions.assertArrayEquals(passwordConfigContent, Files.readAllBytes(configPath));
 		}
 
 		@Test
 		public void testConvertInternalWrapsCryptoException() throws IOException {
-			Mockito.doThrow(new MasterkeyLoadingFailedException("yadda")).when(inSpy).replaceWithPasswordConfig(any());
+			Mockito.doThrow(new MasterkeyLoadingFailedException("yadda")).when(inSpy).createPasswordConfig(any(), any(), any());
 
 			Assertions.assertThrows(CompletionException.class, inSpy::convertInternal);
 
@@ -130,6 +190,7 @@ public class HubToPasswordConvertControllerTest {
 
 		@Test
 		public void testConvertInternalWrapsIOException() throws IOException {
+			Mockito.doReturn(Mockito.mock(Path.class)).when(inSpy).createPasswordConfig(any(), any(), eq(passphrase));
 			Mockito.doThrow(new IOException("yudu")).when(inSpy).backupHubConfig(any());
 
 			Assertions.assertThrows(CompletionException.class, inSpy::convertInternal);
@@ -139,7 +200,7 @@ public class HubToPasswordConvertControllerTest {
 
 		@Test
 		public void testConvertInternalNotWrapsIAE() throws IOException {
-			Mockito.doThrow(new IllegalArgumentException("yudu")).when(recoveryKeyFactory).newMasterkeyFileWithPassphrase(any(), anyString(), any());
+			Mockito.doThrow(new IllegalArgumentException("yolo")).when(recoveryKeyFactory).newMasterkeyFileWithPassphrase(any(), anyString(), any());
 
 			Assertions.assertThrows(IllegalArgumentException.class, inSpy::convertInternal);