From f57216ad0f6c619de4f1f2f6378e98cd8c1b2be3 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Thu, 25 Jan 2024 13:45:41 +0100 Subject: [PATCH 1/7] add `GET /api/devices/{deviceId}/legacy-access-tokens` --- .../cryptomator/hub/api/DeviceResource.java | 20 ++++++++++++++++++- .../hub/entities/LegacyAccessToken.java | 15 ++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/backend/src/main/java/org/cryptomator/hub/api/DeviceResource.java b/backend/src/main/java/org/cryptomator/hub/api/DeviceResource.java index a043465f6..75db803d6 100644 --- a/backend/src/main/java/org/cryptomator/hub/api/DeviceResource.java +++ b/backend/src/main/java/org/cryptomator/hub/api/DeviceResource.java @@ -22,8 +22,8 @@ import jakarta.ws.rs.core.Response; import org.cryptomator.hub.entities.AuditEventDeviceRegister; import org.cryptomator.hub.entities.AuditEventDeviceRemove; -import org.cryptomator.hub.entities.AuditEventVaultAccessGrant; import org.cryptomator.hub.entities.Device; +import org.cryptomator.hub.entities.LegacyAccessToken; import org.cryptomator.hub.entities.LegacyDevice; import org.cryptomator.hub.entities.User; import org.cryptomator.hub.validation.NoHtmlOrScriptChars; @@ -41,6 +41,7 @@ import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.List; +import java.util.UUID; @Path("/devices") public class DeviceResource { @@ -117,6 +118,21 @@ public DeviceDto get(@PathParam("deviceId") @ValidId String deviceId) { } } + @Deprecated + @GET + @Path("/{deviceId}/legacy-access-tokens") + @RolesAllowed("user") + @Produces(MediaType.APPLICATION_JSON) + @NoCache + @Transactional + @Operation(summary = "list legacy access tokens", description = "get a list of all legacy access tokens for this device. The device must be owned by the currently logged-in user") + @APIResponse(responseCode = "200") + public List getLegacyAccessTokens(@PathParam("deviceId") @ValidId String deviceId) { + return LegacyAccessToken.getByDeviceAndOwner(deviceId, jwt.getSubject()) + .map(result -> new LegacyAccessTokenDto(result.id.vaultId, result.jwe)) + .toList(); + } + @DELETE @Path("/{deviceId}") @RolesAllowed("user") @@ -154,4 +170,6 @@ public static DeviceDto fromEntity(Device entity) { } } + + public record LegacyAccessTokenDto(@JsonProperty("vaultId") UUID vaultId, @JsonProperty("token") String token) {} } diff --git a/backend/src/main/java/org/cryptomator/hub/entities/LegacyAccessToken.java b/backend/src/main/java/org/cryptomator/hub/entities/LegacyAccessToken.java index 782f804db..0aaa6b8fa 100644 --- a/backend/src/main/java/org/cryptomator/hub/entities/LegacyAccessToken.java +++ b/backend/src/main/java/org/cryptomator/hub/entities/LegacyAccessToken.java @@ -12,6 +12,7 @@ import java.io.Serializable; import java.util.Objects; import java.util.UUID; +import java.util.stream.Stream; @Entity @Table(name = "access_token_legacy") @@ -22,6 +23,13 @@ INNER JOIN effective_vault_access a ON a.vault_id = t.vault_id AND a.authority_id = d.owner_id WHERE t.vault_id = :vaultId AND d.id = :deviceId AND d.owner_id = :userId """) +@NamedNativeQuery(name = "LegacyAccessToken.getByDevice", resultClass = LegacyAccessToken.class, query = """ + SELECT t.device_id, t.vault_id, t.jwe + FROM access_token_legacy t + INNER JOIN device_legacy d ON d.id = t.device_id + INNER JOIN effective_vault_access a ON a.vault_id = t.vault_id AND a.authority_id = d.owner_id + WHERE d.id = :deviceId AND d.owner_id = :userId + """) @Deprecated public class LegacyAccessToken extends PanacheEntityBase { @@ -43,6 +51,13 @@ public static LegacyAccessToken unlock(UUID vaultId, String deviceId, String use } } + public static Stream getByDeviceAndOwner(String deviceId, String userId) { + return getEntityManager().createNamedQuery("LegacyAccessToken.getByDevice", LegacyAccessToken.class) // + .setParameter("deviceId", deviceId) // + .setParameter("userId", userId) // + .getResultStream(); + } + @Override public boolean equals(Object o) { if (this == o) return true; From c53d3cb3074db7f65abd97d7056d8056601a6b20 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Thu, 25 Jan 2024 15:26:25 +0100 Subject: [PATCH 2/7] added `/api/me/access-tokens` --- .../cryptomator/hub/api/UsersResource.java | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/backend/src/main/java/org/cryptomator/hub/api/UsersResource.java b/backend/src/main/java/org/cryptomator/hub/api/UsersResource.java index 96efb20da..ec050ad5a 100644 --- a/backend/src/main/java/org/cryptomator/hub/api/UsersResource.java +++ b/backend/src/main/java/org/cryptomator/hub/api/UsersResource.java @@ -5,6 +5,7 @@ import jakarta.inject.Inject; import jakarta.transaction.Transactional; import jakarta.validation.Valid; +import jakarta.validation.constraints.NotEmpty; import jakarta.ws.rs.Consumes; import jakarta.ws.rs.GET; import jakarta.ws.rs.POST; @@ -15,8 +16,10 @@ import jakarta.ws.rs.core.MediaType; import jakarta.ws.rs.core.Response; import org.cryptomator.hub.entities.AccessToken; +import org.cryptomator.hub.entities.AuditEventVaultAccessGrant; import org.cryptomator.hub.entities.Device; import org.cryptomator.hub.entities.User; +import org.cryptomator.hub.entities.Vault; import org.eclipse.microprofile.jwt.JsonWebToken; import org.eclipse.microprofile.openapi.annotations.Operation; import org.eclipse.microprofile.openapi.annotations.responses.APIResponse; @@ -25,6 +28,7 @@ import java.net.URI; import java.time.temporal.ChronoUnit; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.function.Function; import java.util.stream.Collectors; @@ -62,6 +66,33 @@ public Response putMe(@Nullable @Valid UserDto dto) { return Response.created(URI.create(".")).build(); } + @POST + @Path("/me/access-tokens") + @RolesAllowed("user") + @Transactional + @Consumes(MediaType.APPLICATION_JSON) + @Operation(summary = "adds/updates user-specific vault keys", description = "Stores one or more vaultid-vaultkey-tuples for the currently logged-in user, as defined in the request body ({vault1: token1, vault2: token2, ...}).") + @APIResponse(responseCode = "200", description = "all keys stored") + public Response updateMyAccessTokens(@NotEmpty Map tokens) { + var user = User.findById(jwt.getSubject()); + for (var entry : tokens.entrySet()) { + var vault = Vault.findById(entry.getKey()); + if (vault == null || vault.archived) { + continue; // skip + } + var token = AccessToken.findById(new AccessToken.AccessId(user.id, vault.id)); + if (token == null) { + token = new AccessToken(); + token.vault = vault; + token.user = user; + } + token.vaultKey = entry.getValue(); + token.persist(); + AuditEventVaultAccessGrant.log(user.id, vault.id, user.id); + } + return Response.ok().build(); + } + @GET @Path("/me") @RolesAllowed("user") From 693cdb929076c2e30a3f40230b6e19dde17865d2 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Thu, 25 Jan 2024 15:38:42 +0100 Subject: [PATCH 3/7] adjust return type `{"vault1": "jwe1", "vault2": "jwe2"}` instead of `[{"vaultId": "vault1", "token": "jwe1"}, ...]` --- .../java/org/cryptomator/hub/api/DeviceResource.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/backend/src/main/java/org/cryptomator/hub/api/DeviceResource.java b/backend/src/main/java/org/cryptomator/hub/api/DeviceResource.java index 75db803d6..076447e22 100644 --- a/backend/src/main/java/org/cryptomator/hub/api/DeviceResource.java +++ b/backend/src/main/java/org/cryptomator/hub/api/DeviceResource.java @@ -41,7 +41,9 @@ import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.List; +import java.util.Map; import java.util.UUID; +import java.util.stream.Collectors; @Path("/devices") public class DeviceResource { @@ -125,12 +127,11 @@ public DeviceDto get(@PathParam("deviceId") @ValidId String deviceId) { @Produces(MediaType.APPLICATION_JSON) @NoCache @Transactional - @Operation(summary = "list legacy access tokens", description = "get a list of all legacy access tokens for this device. The device must be owned by the currently logged-in user") + @Operation(summary = "list legacy access tokens", description = "get all legacy access tokens for this device ({vault1: token1, vault1: token2, ...}). The device must be owned by the currently logged-in user") @APIResponse(responseCode = "200") - public List getLegacyAccessTokens(@PathParam("deviceId") @ValidId String deviceId) { + public Map getLegacyAccessTokens(@PathParam("deviceId") @ValidId String deviceId) { return LegacyAccessToken.getByDeviceAndOwner(deviceId, jwt.getSubject()) - .map(result -> new LegacyAccessTokenDto(result.id.vaultId, result.jwe)) - .toList(); + .collect(Collectors.toMap(token -> token.id.vaultId , token -> token.jwe)); } @DELETE From 94b34835e36e8cb43b237f3fed731e0da80bbfb4 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Thu, 25 Jan 2024 16:17:13 +0100 Subject: [PATCH 4/7] added tests --- .../cryptomator/hub/api/UsersResource.java | 3 ++- .../hub/api/DeviceResourceTest.java | 18 ++++++++++++++++++ .../hub/api/UsersResourceTest.java | 19 ++++++++++++++++--- 3 files changed, 36 insertions(+), 4 deletions(-) diff --git a/backend/src/main/java/org/cryptomator/hub/api/UsersResource.java b/backend/src/main/java/org/cryptomator/hub/api/UsersResource.java index ec050ad5a..8d3c26641 100644 --- a/backend/src/main/java/org/cryptomator/hub/api/UsersResource.java +++ b/backend/src/main/java/org/cryptomator/hub/api/UsersResource.java @@ -30,6 +30,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.UUID; import java.util.function.Function; import java.util.stream.Collectors; @@ -73,7 +74,7 @@ public Response putMe(@Nullable @Valid UserDto dto) { @Consumes(MediaType.APPLICATION_JSON) @Operation(summary = "adds/updates user-specific vault keys", description = "Stores one or more vaultid-vaultkey-tuples for the currently logged-in user, as defined in the request body ({vault1: token1, vault2: token2, ...}).") @APIResponse(responseCode = "200", description = "all keys stored") - public Response updateMyAccessTokens(@NotEmpty Map tokens) { + public Response updateMyAccessTokens(@NotEmpty Map tokens) { var user = User.findById(jwt.getSubject()); for (var entry : tokens.entrySet()) { var vault = Vault.findById(entry.getKey()); diff --git a/backend/src/test/java/org/cryptomator/hub/api/DeviceResourceTest.java b/backend/src/test/java/org/cryptomator/hub/api/DeviceResourceTest.java index afd40c3de..5d00d37d5 100644 --- a/backend/src/test/java/org/cryptomator/hub/api/DeviceResourceTest.java +++ b/backend/src/test/java/org/cryptomator/hub/api/DeviceResourceTest.java @@ -78,6 +78,24 @@ public void testGet1() { .body("userPrivateKey", is("jwe.jwe.jwe.user1.device1")); } + @Test + @Order(1) + @DisplayName("GET /devices/legacyDevice1/legacy-access-tokens returns 200") + public void testGetLegacyAccessTokens1() { + given().when().get("/devices/{deviceId}/legacy-access-tokens", "legacyDevice1") + .then().statusCode(200) + .body("7e57c0de-0000-4000-8000-000100001111", is("legacy.jwe.jwe.vault1.device1")); + } + + @Test + @Order(1) + @DisplayName("GET /devices/legacyDevice3/legacy-access-tokens returns 200") + public void testGetLegacyAccessTokens2() { + given().when().get("/devices/{deviceId}/legacy-access-tokens", "legacyDevice3") + .then().statusCode(200) + .body("7e57c0de-0000-4000-8000-000100002222", is("legacy.jwe.jwe.vault2.device3")); + } + @Test @Order(1) @DisplayName("GET /devices/device2 returns 404 (owned by other user)") diff --git a/backend/src/test/java/org/cryptomator/hub/api/UsersResourceTest.java b/backend/src/test/java/org/cryptomator/hub/api/UsersResourceTest.java index 150623494..f92af177b 100644 --- a/backend/src/test/java/org/cryptomator/hub/api/UsersResourceTest.java +++ b/backend/src/test/java/org/cryptomator/hub/api/UsersResourceTest.java @@ -5,8 +5,7 @@ import io.quarkus.test.security.oidc.Claim; import io.quarkus.test.security.oidc.OidcSecurity; import io.restassured.RestAssured; -import org.hamcrest.CoreMatchers; -import org.hamcrest.text.IsEqualIgnoringCase; +import io.restassured.http.ContentType; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Nested; @@ -14,11 +13,11 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; +import static io.restassured.RestAssured.given; import static io.restassured.RestAssured.when; import static org.hamcrest.CoreMatchers.hasItems; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.Matchers.empty; -import static org.hamcrest.text.IsEqualIgnoringCase.equalToIgnoringCase; @QuarkusTest @DisplayName("Resource /users") @@ -70,6 +69,20 @@ public void testGetAll() { .body("id", hasItems("user1", "user2")); } + @Test + @DisplayName("POST /users/me/access-tokens returns 200") + public void testPostAccessTokens() { + var body = """ + { + "7E57C0DE-0000-4000-8000-000100001111": "jwe.jwe.jwe.vault1.user1", + "7E57C0DE-0000-4000-8000-BADBADBADBAD": "noSuchVault" + }, + """; + given().contentType(ContentType.JSON).body(body) + .when().post("/users/me/access-tokens") + .then().statusCode(200); + } + } @Nested From ff0967edb07419bd34e5dd088e620ac2f950143a Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Fri, 26 Jan 2024 11:48:15 +0100 Subject: [PATCH 5/7] addressed remarks from PR review #252 --- .../java/org/cryptomator/hub/api/UsersResource.java | 2 +- .../org/cryptomator/hub/api/DeviceResourceTest.java | 11 ++++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/backend/src/main/java/org/cryptomator/hub/api/UsersResource.java b/backend/src/main/java/org/cryptomator/hub/api/UsersResource.java index 8d3c26641..cf83511b2 100644 --- a/backend/src/main/java/org/cryptomator/hub/api/UsersResource.java +++ b/backend/src/main/java/org/cryptomator/hub/api/UsersResource.java @@ -78,7 +78,7 @@ public Response updateMyAccessTokens(@NotEmpty Map tokens) { var user = User.findById(jwt.getSubject()); for (var entry : tokens.entrySet()) { var vault = Vault.findById(entry.getKey()); - if (vault == null || vault.archived) { + if (vault == null) { continue; // skip } var token = AccessToken.findById(new AccessToken.AccessId(user.id, vault.id)); diff --git a/backend/src/test/java/org/cryptomator/hub/api/DeviceResourceTest.java b/backend/src/test/java/org/cryptomator/hub/api/DeviceResourceTest.java index 5d00d37d5..b45b18753 100644 --- a/backend/src/test/java/org/cryptomator/hub/api/DeviceResourceTest.java +++ b/backend/src/test/java/org/cryptomator/hub/api/DeviceResourceTest.java @@ -89,8 +89,17 @@ public void testGetLegacyAccessTokens1() { @Test @Order(1) - @DisplayName("GET /devices/legacyDevice3/legacy-access-tokens returns 200") + @DisplayName("GET /devices/legacyDevice2/legacy-access-tokens returns empty list (owned by different user)") public void testGetLegacyAccessTokens2() { + given().when().get("/devices/{deviceId}/legacy-access-tokens", "legacyDevice2") + .then().statusCode(200) + .body(is("{}")); + } + + @Test + @Order(1) + @DisplayName("GET /devices/legacyDevice3/legacy-access-tokens returns 200") + public void testGetLegacyAccessTokens3() { given().when().get("/devices/{deviceId}/legacy-access-tokens", "legacyDevice3") .then().statusCode(200) .body("7e57c0de-0000-4000-8000-000100002222", is("legacy.jwe.jwe.vault2.device3")); From c7a2d7030c8d4f0918c5d34784a1ee87b8de641a Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Fri, 26 Jan 2024 12:20:35 +0100 Subject: [PATCH 6/7] added another test --- .../java/org/cryptomator/hub/api/DeviceResourceTest.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/backend/src/test/java/org/cryptomator/hub/api/DeviceResourceTest.java b/backend/src/test/java/org/cryptomator/hub/api/DeviceResourceTest.java index b45b18753..491f3e5cb 100644 --- a/backend/src/test/java/org/cryptomator/hub/api/DeviceResourceTest.java +++ b/backend/src/test/java/org/cryptomator/hub/api/DeviceResourceTest.java @@ -105,6 +105,15 @@ public void testGetLegacyAccessTokens3() { .body("7e57c0de-0000-4000-8000-000100002222", is("legacy.jwe.jwe.vault2.device3")); } + @Test + @Order(1) + @DisplayName("GET /devices/noSuchDevice/legacy-access-tokens returns empty list (no such device)") + public void testGetLegacyAccessTokens4() { + given().when().get("/devices/{deviceId}/legacy-access-tokens", "noSuchDevice") + .then().statusCode(200) + .body(is("{}")); + } + @Test @Order(1) @DisplayName("GET /devices/device2 returns 404 (owned by other user)") From 58ff0d684ba1eda836320430e58b44b48029baff Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Fri, 26 Jan 2024 12:30:34 +0100 Subject: [PATCH 7/7] relaxed validation --- .../org/cryptomator/hub/api/UsersResource.java | 3 ++- .../cryptomator/hub/api/UsersResourceTest.java | 18 +++++++++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/backend/src/main/java/org/cryptomator/hub/api/UsersResource.java b/backend/src/main/java/org/cryptomator/hub/api/UsersResource.java index cf83511b2..1707d7730 100644 --- a/backend/src/main/java/org/cryptomator/hub/api/UsersResource.java +++ b/backend/src/main/java/org/cryptomator/hub/api/UsersResource.java @@ -6,6 +6,7 @@ import jakarta.transaction.Transactional; import jakarta.validation.Valid; import jakarta.validation.constraints.NotEmpty; +import jakarta.validation.constraints.NotNull; import jakarta.ws.rs.Consumes; import jakarta.ws.rs.GET; import jakarta.ws.rs.POST; @@ -74,7 +75,7 @@ public Response putMe(@Nullable @Valid UserDto dto) { @Consumes(MediaType.APPLICATION_JSON) @Operation(summary = "adds/updates user-specific vault keys", description = "Stores one or more vaultid-vaultkey-tuples for the currently logged-in user, as defined in the request body ({vault1: token1, vault2: token2, ...}).") @APIResponse(responseCode = "200", description = "all keys stored") - public Response updateMyAccessTokens(@NotEmpty Map tokens) { + public Response updateMyAccessTokens(@NotNull Map tokens) { var user = User.findById(jwt.getSubject()); for (var entry : tokens.entrySet()) { var vault = Vault.findById(entry.getKey()); diff --git a/backend/src/test/java/org/cryptomator/hub/api/UsersResourceTest.java b/backend/src/test/java/org/cryptomator/hub/api/UsersResourceTest.java index f92af177b..7ddc75737 100644 --- a/backend/src/test/java/org/cryptomator/hub/api/UsersResourceTest.java +++ b/backend/src/test/java/org/cryptomator/hub/api/UsersResourceTest.java @@ -71,7 +71,7 @@ public void testGetAll() { @Test @DisplayName("POST /users/me/access-tokens returns 200") - public void testPostAccessTokens() { + public void testPostAccessTokens1() { var body = """ { "7E57C0DE-0000-4000-8000-000100001111": "jwe.jwe.jwe.vault1.user1", @@ -83,6 +83,22 @@ public void testPostAccessTokens() { .then().statusCode(200); } + @Test + @DisplayName("POST /users/me/access-tokens returns 200 for empty list") + public void testPostAccessTokens2() { + given().contentType(ContentType.JSON).body("{}") + .when().post("/users/me/access-tokens") + .then().statusCode(200); + } + + @Test + @DisplayName("POST /users/me/access-tokens returns 400 for malformed body") + public void testPostAccessTokens3() { + given().contentType(ContentType.JSON).body("") + .when().post("/users/me/access-tokens") + .then().statusCode(400); + } + } @Nested