mirrors/cryptomator
1
0
Fork 0
mirror of https://github.com/cryptomator/cryptomator.git synced 2026-08-24 16:16:03 +00:00
Code Issues Packages Projects Releases Wiki Activity

simplify

Browse Source 
there is no intermediate directory in the zip. Also clean up tests

Signed-off-by: Armin Schrenk <armin.schrenk@skymatic.de>
This commit is contained in:
Armin Schrenk
2026-07-21 17:06:17 +02:00
parent ae84f2f2fd
commit 2c53afd70f
4 changed files with 104 additions and 138 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/main/java/org/cryptomator/ui/importtemplate/VaultTemplate.java
+22 -30
View File
@@ -13,39 +13,31 @@ import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CompletionException;
import java.util.concurrent.CompletionStage;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Executor;
/**
* A vault template that has been unpacked to a temporary directory and found sound.
* <p>
* Holding an instance means the template <em>can</em> be imported: it was a readable ZIP within the entry and size
* limits, held exactly one decodable {@value Constants#VAULTCONFIG_FILENAME}, and no entry escaped the extraction
* directory. Unpacking therefore happens once, when the deeplink arrives, so a template that could never be imported is
* rejected before the user is asked to choose a storage location - and {@link #moveTo(Path)} is left with nothing to
* validate but the destination.
* Holding an instance means the template <em>is</em> valid: it was a readable ZIP within the entry and size
* limits, held a decodable {@value Constants#VAULTCONFIG_FILENAME} directly in its root and no entry escaped
* the extraction directory.
* <p>
* The instance owns a temporary directory and <strong>must be {@link #close() closed}</strong>, whether or not the
* Because the vault lies in the archive root, the temporary directory <em>is</em> the vault directory: importing moves
* it wholesale to its destination.
* <p>
* The instance owns that temporary directory and <strong>must be {@link #close() closed}</strong>, whether or not the
* import completes.
* <p>
* {@link #hubUrl()} is read from the <em>unverified</em> vault config: its signature is keyed on the masterkey, which is
* only obtained by completing the Hub flow. It is shown so the user can recognize an unexpected Hub; the authoritative
* trust decision remains with {@code CheckHostTrustController} at unlock time.
* {@link #hubUrl()} is read from the <em>unverified</em> vault config.
*/
public final class VaultTemplate implements AutoCloseable {
private static final Logger LOG = LoggerFactory.getLogger(VaultTemplate.class);
private final Path tempDir;
private final Path vaultRoot;
private final Path vaultDir;
private final @Nullable String hubUrl;
private VaultTemplate(Path tempDir, Path vaultRoot, @Nullable String hubUrl) {
this.tempDir = tempDir;
this.vaultRoot = vaultRoot;
private VaultTemplate(Path vaultDir, @Nullable String hubUrl) {
this.vaultDir = vaultDir;
this.hubUrl = hubUrl;
}
@@ -65,21 +57,21 @@ public final class VaultTemplate implements AutoCloseable {
@VisibleForTesting
static VaultTemplate extract(byte[] archive, @Nullable Path tempParent) throws IOException {
Path tempDir = tempParent == null //
Path vaultDir = tempParent == null //
? Files.createTempDirectory("vault-template-") //
: Files.createTempDirectory(tempParent, "vault-template-");
try {
var vaultRoot = VaultTemplateExtractor.extractTo(archive, tempDir);
var hubURL = readHubUrl(vaultRoot);
return new VaultTemplate(tempDir, vaultRoot, hubURL);
VaultTemplateExtractor.extractTo(archive, vaultDir);
var hubUrl = readHubUrl(vaultDir);
return new VaultTemplate(vaultDir, hubUrl);
} catch (IOException | RuntimeException e) {
VaultTemplateExtractor.deleteQuietly(tempDir);
VaultTemplateExtractor.deleteQuietly(vaultDir);
throw e;
}
}
private static @Nullable String readHubUrl(Path vaultRoot) throws IOException {
var token = Files.readString(vaultRoot.resolve(Constants.VAULTCONFIG_FILENAME), StandardCharsets.US_ASCII).trim();
private static @Nullable String readHubUrl(Path vaultDir) throws IOException {
var token = Files.readString(vaultDir.resolve(Constants.VAULTCONFIG_FILENAME), StandardCharsets.US_ASCII).trim();
HubConfig hubConfig;
try {
hubConfig = VaultConfig.decode(token).getHeader("hub", HubConfig.class);
@@ -115,15 +107,15 @@ public final class VaultTemplate implements AutoCloseable {
* @throws IOException if the vault cannot be moved
*/
public void moveTo(Path destination) throws IOException {
VaultTemplateExtractor.moveToDestination(vaultRoot, destination);
VaultTemplateExtractor.moveToDestination(vaultDir, destination);
}
/**
* Deletes the temporary directory. Safe to call after a successful {@link #moveTo(Path)}, which only moves the
* vault directory out of it.
* Discards the unpacked vault. A no-op after a successful {@link #moveTo(Path)}, which relocates the directory
* this would otherwise delete.
*/
@Override
public void close() {
VaultTemplateExtractor.deleteQuietly(tempDir);
VaultTemplateExtractor.deleteQuietly(vaultDir);
}
}
src/main/java/org/cryptomator/ui/importtemplate/VaultTemplateExtractor.java
+14 -29
View File
@@ -22,9 +22,10 @@ import java.util.zip.ZipException;
/**
* Low-level mechanics of unpacking a vault template (a ZIP archive holding a ready-made vault).
* <p>
* The archive is expanded into a directory using the Java ZIP {@link FileSystem}, and the vault directory is located by
* finding the {@value Constants#VAULTCONFIG_FILENAME} it contains. Moving that directory to its final location is a
* separate step, so the vault only ever appears complete at its destination.
* Per the template spec the vault lies directly in the archive root, i.e. the archive holds
* {@value Constants#VAULTCONFIG_FILENAME} at its top level rather than inside an enclosing folder. The extraction
* directory therefore <em>is</em> the vault directory. Moving it to its final location is a separate step, so the vault
* only ever appears complete at its destination.
* <p>
* Lifecycle of the temporary directory is owned by {@link VaultTemplate}, not by this class.
*/
@@ -41,39 +42,35 @@ final class VaultTemplateExtractor {
}
/**
* Unpacks the given template into {@code targetDir} and locates the vault it contains.
* Unpacks the given template into {@code targetDir}, which then holds the vault itself.
*
* @param template the ZIP archive bytes
* @param targetDir an existing, empty directory to unpack into
* @return the unpacked vault directory below {@code targetDir}
* @throws MalformedTemplateException if the template is not a readable ZIP, exceeds the entry or size limits, or
* does not contain exactly one {@value Constants#VAULTCONFIG_FILENAME}
* does not hold {@value Constants#VAULTCONFIG_FILENAME} in its root
* @throws IOException if the files cannot be written
*/
static Path extractTo(byte[] template, Path targetDir) throws IOException {
static void extractTo(byte[] template, Path targetDir) throws IOException {
Path tmpZip = Files.createTempFile("vault-template-", ".zip");
try {
Files.write(tmpZip, template);
return unzip(tmpZip, targetDir);
unzip(tmpZip, targetDir);
} finally {
deleteQuietly(tmpZip);
}
}
private static Path unzip(Path zipFile, Path targetDir) throws IOException {
private static void unzip(Path zipFile, Path targetDir) throws IOException {
Path normalizedTarget = targetDir.normalize();
try (FileSystem zipFs = FileSystems.newFileSystem(zipFile)) {
Path zipRoot = zipFs.getRootDirectories().iterator().next(); //we take the first available root and ignore others
var templateExtractor = new TemplateExtractionVisitor(zipRoot, normalizedTarget, MAX_ENTRIES, MAX_TOTAL_BYTES);
Files.walkFileTree(zipRoot, templateExtractor);
var vaultConfig = templateExtractor.getVaultConfig();
if (vaultConfig == null) {
throw new MalformedTemplateException("Template does not contain a vault (no " + Constants.VAULTCONFIG_FILENAME + " found).");
}
return vaultConfig.getParent();
Files.walkFileTree(zipRoot, new TemplateExtractionVisitor(zipRoot, normalizedTarget, MAX_ENTRIES, MAX_TOTAL_BYTES));
} catch (ZipException e) {
throw new MalformedTemplateException("Template is not a readable ZIP archive.", e);
}
if (!Files.isRegularFile(normalizedTarget.resolve(Constants.VAULTCONFIG_FILENAME))) {
throw new MalformedTemplateException("Template does not hold " + Constants.VAULTCONFIG_FILENAME + " in its root directory.");
}
}
static class TemplateExtractionVisitor extends SimpleFileVisitor<Path> {
@@ -85,7 +82,6 @@ final class VaultTemplateExtractor {
private int totalEntries = 0;
private long totalBytes = 0;
private Path vaultConfig = null;
TemplateExtractionVisitor(Path zipRoot, Path target, int maxEntries, long maxSize) {
this.zipRoot = zipRoot;
@@ -111,14 +107,7 @@ final class VaultTemplateExtractor {
throw new MalformedTemplateException("Vault template exceeds the maximum allowed size of " + maxSize + " bytes.");
}
var actualTarget = Files.copy(file, resolveSafely(target, zipRoot, file), StandardCopyOption.REPLACE_EXISTING);
if (Constants.VAULTCONFIG_FILENAME.equals(file.getFileName().toString())) {
if (vaultConfig != null) {
throw new MalformedTemplateException("Vault template contains more than one " + Constants.VAULTCONFIG_FILENAME);
}
vaultConfig = actualTarget;
}
Files.copy(file, resolveSafely(target, zipRoot, file), StandardCopyOption.REPLACE_EXISTING);
return FileVisitResult.CONTINUE;
}
@@ -128,10 +117,6 @@ final class VaultTemplateExtractor {
throw new MalformedTemplateException("Vault template contains more than the maximum allowed " + maxEntries + " entries.");
}
}
Path getVaultConfig() {
return vaultConfig;
}
}
private static Path resolveSafely(Path targetDir, Path zipRoot, Path entry) throws IOException {
src/test/java/org/cryptomator/ui/importtemplate/VaultTemplateExtractorTest.java
+48 -15
View File
@@ -8,6 +8,7 @@ import org.junit.jupiter.api.io.TempDir;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.FileAlreadyExistsException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.LinkedHashMap;
@@ -21,31 +22,29 @@ public class VaultTemplateExtractorTest {
private static final byte[] CIPHERTEXT = "some-ciphertext".getBytes(StandardCharsets.UTF_8);
@Test
@DisplayName("a vault at the archive root is unpacked and located")
@DisplayName("a vault at the archive root is unpacked into the target directory")
public void testVaultAtArchiveRoot(@TempDir Path tmp) throws IOException {
var zip = zip(Map.of( //
"vault.cryptomator", CONFIG, //
"d/AB/CDEF/0.c9r", CIPHERTEXT //
));
var vaultRoot = VaultTemplateExtractor.extractTo(zip, tmp);
VaultTemplateExtractor.extractTo(zip, tmp);
Assertions.assertArrayEquals(CONFIG, Files.readAllBytes(vaultRoot.resolve("vault.cryptomator")));
Assertions.assertArrayEquals(CIPHERTEXT, Files.readAllBytes(vaultRoot.resolve("d/AB/CDEF/0.c9r")));
Assertions.assertArrayEquals(CONFIG, Files.readAllBytes(tmp.resolve("vault.cryptomator")));
Assertions.assertArrayEquals(CIPHERTEXT, Files.readAllBytes(tmp.resolve("d/AB/CDEF/0.c9r")));
}
@Test
@DisplayName("a vault nested in a single top-level folder is located")
@DisplayName("a vault nested in a top-level folder fails as malformed")
public void testVaultInSubfolder(@TempDir Path tmp) throws IOException {
// requirement: the vault starts in the archive root, an enclosing folder is not a supported template
var zip = zip(Map.of( //
"TemplateVault/vault.cryptomator", CONFIG, //
"TemplateVault/d/AB/CDEF/0.c9r", CIPHERTEXT //
));
var vaultRoot = VaultTemplateExtractor.extractTo(zip, tmp);
Assertions.assertEquals("TemplateVault", vaultRoot.getFileName().toString());
Assertions.assertArrayEquals(CONFIG, Files.readAllBytes(vaultRoot.resolve("vault.cryptomator")));
Assertions.assertThrows(MalformedTemplateException.class, () -> VaultTemplateExtractor.extractTo(zip, tmp));
}
@Test
@@ -65,14 +64,19 @@ public class VaultTemplateExtractorTest {
}
@Test
@DisplayName("an archive with more than one vault config fails as malformed")
public void testMultipleVaultConfigs(@TempDir Path tmp) throws IOException {
@DisplayName("a further vault config below the root is ordinary content, the root one identifies the vault")
public void testFurtherVaultConfigBelowRoot(@TempDir Path tmp) throws IOException {
// with the vault fixed at the archive root a nested config is simply extracted along with everything
// else and counts against the entry limit
var zip = zip(Map.of( //
"vault.cryptomator", CONFIG, //
"nested/vault.cryptomator", CONFIG //
"nested/vault.cryptomator", CIPHERTEXT //
));
Assertions.assertThrows(MalformedTemplateException.class, () -> VaultTemplateExtractor.extractTo(zip, tmp));
VaultTemplateExtractor.extractTo(zip, tmp);
Assertions.assertArrayEquals(CONFIG, Files.readAllBytes(tmp.resolve("vault.cryptomator")));
Assertions.assertArrayEquals(CIPHERTEXT, Files.readAllBytes(tmp.resolve("nested/vault.cryptomator")));
}
@Test
@@ -92,9 +96,9 @@ public class VaultTemplateExtractorTest {
public void testAtEntryLimit(@TempDir Path tmp) throws IOException {
var zip = zip(flatEntries(VaultTemplateExtractor.MAX_ENTRIES));
var vaultRoot = VaultTemplateExtractor.extractTo(zip, tmp);
VaultTemplateExtractor.extractTo(zip, tmp);
Assertions.assertArrayEquals(CONFIG, Files.readAllBytes(vaultRoot.resolve("vault.cryptomator")));
Assertions.assertArrayEquals(CONFIG, Files.readAllBytes(tmp.resolve("vault.cryptomator")));
}
@Test
@@ -124,6 +128,35 @@ public class VaultTemplateExtractorTest {
Assertions.assertTrue(Files.notExists(tmp.resolve("escaped.txt")), "entry escaped the extraction directory");
}
@Test
@DisplayName("an unpacked vault is moved to the destination, creating missing parents")
public void testMoveToDestination(@TempDir Path tmp) throws IOException {
var source = unpackedVault(tmp.resolve("unpacked"));
var destination = tmp.resolve("parent").resolve("MyVault"); // parent does not exist yet
VaultTemplateExtractor.moveToDestination(source, destination);
Assertions.assertArrayEquals(CONFIG, Files.readAllBytes(destination.resolve("vault.cryptomator")));
Assertions.assertArrayEquals(CIPHERTEXT, Files.readAllBytes(destination.resolve("d/AB/0.c9r")));
Assertions.assertTrue(Files.notExists(source), "the vault should no longer be at its old location");
}
@Test
@DisplayName("an existing destination is not overwritten")
public void testMoveToExistingDestination(@TempDir Path tmp) throws IOException {
var source = unpackedVault(tmp.resolve("unpacked"));
var destination = Files.createDirectory(tmp.resolve("MyVault"));
Assertions.assertThrows(FileAlreadyExistsException.class, () -> VaultTemplateExtractor.moveToDestination(source, destination));
}
private static Path unpackedVault(Path dir) throws IOException {
Files.createDirectories(dir.resolve("d/AB"));
Files.write(dir.resolve("vault.cryptomator"), CONFIG);
Files.write(dir.resolve("d/AB/0.c9r"), CIPHERTEXT);
return dir;
}
/**
* @param count total number of entries below the zip root, the vault config included
*/
src/test/java/org/cryptomator/ui/importtemplate/VaultTemplateTest.java
+20 -64
View File
@@ -8,7 +8,6 @@ import org.junit.jupiter.api.io.TempDir;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.FileAlreadyExistsException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Base64;
@@ -50,43 +49,6 @@ public class VaultTemplateTest {
Assertions.assertThrows(MalformedTemplateException.class, () -> VaultTemplate.extract(zip));
}
@Test
@DisplayName("data that is not a zip is rejected")
public void testNotAZip() {
var notAZip = "this is not a zip archive".getBytes(StandardCharsets.UTF_8);
Assertions.assertThrows(MalformedTemplateException.class, () -> VaultTemplate.extract(notAZip));
}
@Test
@DisplayName("the unpacked vault is moved to the destination")
public void testMoveTo(@TempDir Path tmp) throws IOException {
var zip = zip(Map.of( //
"vault.cryptomator", hubVaultConfig(API_BASE_URL), //
"d/AB/CDEF/0.c9r", CIPHERTEXT //
));
var destination = tmp.resolve("MyVault");
try (var inTest = VaultTemplate.extract(zip)) {
inTest.moveTo(destination);
}
Assertions.assertArrayEquals(CIPHERTEXT, Files.readAllBytes(destination.resolve("d/AB/CDEF/0.c9r")));
Assertions.assertTrue(Files.exists(destination.resolve("vault.cryptomator")));
}
@Test
@DisplayName("an existing destination is not overwritten")
public void testDestinationExists(@TempDir Path tmp) throws IOException {
var destination = tmp.resolve("MyVault");
Files.createDirectory(destination);
var zip = zip(Map.of("vault.cryptomator", hubVaultConfig(API_BASE_URL)));
try (var inTest = VaultTemplate.extract(zip)) {
Assertions.assertThrows(FileAlreadyExistsException.class, () -> inTest.moveTo(destination));
}
}
@Test
@DisplayName("closing discards the temporary directory, also after a completed import")
public void testCloseCleansUpAfterMove(@TempDir Path tmp) throws IOException {
@@ -107,24 +69,6 @@ public class VaultTemplateTest {
Assertions.assertArrayEquals(CIPHERTEXT, Files.readAllBytes(destination.resolve("d/AB/CDEF/0.c9r")));
}
@Test
@DisplayName("closing discards the temporary directory a nested vault leaves behind")
public void testCloseCleansUpNestedRemainder(@TempDir Path tmp) throws IOException {
// a vault at the archive root IS the temporary directory, so moveTo relocates it wholesale; a nested vault
// leaves its enclosing temporary directory behind, and only close() removes that
var zip = zip(Map.of("TemplateVault/vault.cryptomator", hubVaultConfig(API_BASE_URL)));
var workDir = Files.createDirectory(tmp.resolve("work"));
var destination = tmp.resolve("MyVault");
try (var inTest = VaultTemplate.extract(zip, workDir)) {
inTest.moveTo(destination);
Assertions.assertFalse(isEmpty(workDir), "the enclosing temporary directory should outlive the move");
}
Assertions.assertTrue(isEmpty(workDir), "temporary extraction directory was not cleaned up");
Assertions.assertTrue(Files.exists(destination.resolve("vault.cryptomator")));
}
@Test
@DisplayName("closing without importing discards the temporary directory")
public void testCloseCleansUpWithoutMove(@TempDir Path tmp) throws IOException {
@@ -157,11 +101,15 @@ public class VaultTemplateTest {
private static byte[] hubVaultConfig(String apiBaseUrl) {
return vaultConfig("""
,"hub":{"clientId":"cryptomator","authEndpoint":"https://login.example.com/auth",\
"tokenEndpoint":"https://login.example.com/token",\
"authSuccessUrl":"https://hub.example.com/app/unlock-success",\
"authErrorUrl":"https://hub.example.com/app/unlock-error",\
"apiBaseUrl":"%s"}""".formatted(apiBaseUrl));
,
"hub": {
"clientId":"cryptomator",\
"authEndpoint":"https://login.example.com/auth",\
"tokenEndpoint":"https://login.example.com/token",\
"authSuccessUrl":"https://hub.example.com/app/unlock-success",\
"authErrorUrl":"https://hub.example.com/app/unlock-error",\
"apiBaseUrl":"%s"
}""".formatted(apiBaseUrl));
}
/**
@@ -169,9 +117,17 @@ public class VaultTemplateTest {
* the import dialog operates in - so a dummy signature is sufficient here.
*/
private static byte[] vaultConfig(String extraHeaderFields) {
var header = "{\"kid\":\"hub+https://hub.example.com/api/vaults/123\",\"typ\":\"JWT\",\"alg\":\"HS256\"%s}" //
.formatted(extraHeaderFields == null ? "" : extraHeaderFields);
var payload = "{\"format\":8,\"cipherCombo\":\"SIV_GCM\",\"shorteningThreshold\":220}";
var header = """
{ "kid":"hub+https://hub.example.com/api/vaults/123",\
"typ":"JWT",\
"alg":"HS256"\
%s
}""".formatted(extraHeaderFields == null ? "" : extraHeaderFields);
var payload = """
{ "format":8,\
"cipherCombo":"SIV_GCM",\
"shorteningThreshold":220\
}""";
var encoder = Base64.getUrlEncoder().withoutPadding();
var token = encoder.encodeToString(header.getBytes(StandardCharsets.UTF_8)) //
+ "." + encoder.encodeToString(payload.getBytes(StandardCharsets.UTF_8)) //