diff --git a/backend/src/main/java/org/cryptomator/hub/api/VaultResource.java b/backend/src/main/java/org/cryptomator/hub/api/VaultResource.java index 98ac7ce24..48b923608 100644 --- a/backend/src/main/java/org/cryptomator/hub/api/VaultResource.java +++ b/backend/src/main/java/org/cryptomator/hub/api/VaultResource.java @@ -384,7 +384,7 @@ public Response grantAccess(@PathParam("vaultId") UUID vaultId, @NotEmpty Map u.getId().equals(jwt.getSubject())) && !identity.getRoles().contains("admin")) { - throw new ForbiddenException("Requesting user is not a member of the vault"); - } return VaultDto.fromEntity(vault); } diff --git a/backend/src/main/java/org/cryptomator/hub/entities/Authority.java b/backend/src/main/java/org/cryptomator/hub/entities/Authority.java index 87d0b3a10..fd9cb27c6 100644 --- a/backend/src/main/java/org/cryptomator/hub/entities/Authority.java +++ b/backend/src/main/java/org/cryptomator/hub/entities/Authority.java @@ -12,6 +12,7 @@ import jakarta.persistence.NamedQuery; import jakarta.persistence.Table; +import java.util.Collection; import java.util.List; import java.util.Objects; import java.util.stream.Stream; @@ -99,7 +100,10 @@ public Stream byName(String name) { } public Stream findAllInList(List ids) { - return find("#Authority.allInList", Parameters.with("ids", ids)).stream(); + return Batch.of(200).run(ids, Stream.empty(), (batch, result) -> { + var partial = find("#Authority.allInList", Parameters.with("ids", batch)); + return Stream.concat(result, partial.stream()); + }); } } } diff --git a/backend/src/main/java/org/cryptomator/hub/entities/Batch.java b/backend/src/main/java/org/cryptomator/hub/entities/Batch.java new file mode 100644 index 000000000..b5e8a89c8 --- /dev/null +++ b/backend/src/main/java/org/cryptomator/hub/entities/Batch.java @@ -0,0 +1,44 @@ +package org.cryptomator.hub.entities; + +import java.util.Collection; +import java.util.List; +import java.util.function.BiFunction; +import java.util.function.Consumer; + +public class Batch { + + private final int size; + + private Batch(int size) { + if (size <= 0) { + throw new IllegalArgumentException("Batch size must be positive"); + } + this.size = size; + } + + public static Batch of(int size) { + if (size <= 0) { + throw new IllegalArgumentException("Batch size must be positive"); + } + return new Batch(size); + } + + public void run(Collection collection, Consumer> job) { + List list = collection instanceof List l ? l : List.copyOf(collection); + for(int i = 0; i < list.size(); i += size) { + List sublist = list.subList(i, Math.min(i + size, list.size())); + job.accept(sublist); + } + } + + public R run(Collection collection, R initialValue, BiFunction, R, R> job) { + List list = collection instanceof List l ? l : List.copyOf(collection); + R result = initialValue; + for(int i = 0; i < list.size(); i += size) { + List sublist = list.subList(i, Math.min(i + size, list.size())); + result = job.apply(sublist, result); + } + return result; + } + +} diff --git a/backend/src/main/java/org/cryptomator/hub/entities/Device.java b/backend/src/main/java/org/cryptomator/hub/entities/Device.java index 04ba69e6d..079a914fa 100644 --- a/backend/src/main/java/org/cryptomator/hub/entities/Device.java +++ b/backend/src/main/java/org/cryptomator/hub/entities/Device.java @@ -158,7 +158,10 @@ public Device findByIdAndUser(String deviceId, String userId) throws NoResultExc } public Stream findAllInList(List ids) { - return find("#Device.allInList", Parameters.with("ids", ids)).stream(); + return Batch.of(200).run(ids, Stream.empty(), (batch, result) -> { + var partial = find("#Device.allInList", Parameters.with("ids", batch)); + return Stream.concat(result, partial.stream()); + }); } public void deleteByOwner(String userId) { diff --git a/backend/src/main/java/org/cryptomator/hub/entities/EffectiveGroupMembership.java b/backend/src/main/java/org/cryptomator/hub/entities/EffectiveGroupMembership.java index 26421c9eb..c51e4b781 100644 --- a/backend/src/main/java/org/cryptomator/hub/entities/EffectiveGroupMembership.java +++ b/backend/src/main/java/org/cryptomator/hub/entities/EffectiveGroupMembership.java @@ -1,16 +1,72 @@ package org.cryptomator.hub.entities; +import io.quarkus.hibernate.orm.panache.PanacheRepositoryBase; +import io.quarkus.panache.common.Parameters; +import jakarta.enterprise.context.ApplicationScoped; import jakarta.persistence.Column; import jakarta.persistence.Embeddable; import jakarta.persistence.EmbeddedId; import jakarta.persistence.Entity; +import jakarta.persistence.NamedNativeQuery; import jakarta.persistence.NamedQuery; import jakarta.persistence.Table; import org.hibernate.annotations.Immutable; import java.io.Serializable; +import java.util.Collection; import java.util.Objects; +@NamedNativeQuery(name = "EffectiveGroupMembership.fullUpdate", query = """ + INSERT INTO "effective_group_membership" ("group_id", "intermediate_group_ids", "member_id") + WITH RECURSIVE "members" ("root","intermediate_group_ids","member_id", "depth") AS ( + SELECT "group_id", ARRAY["group_id"]::varchar[], "member_id", 0 + FROM "group_membership" + UNION + SELECT "parent"."root", array_append("parent"."intermediate_group_ids", "child"."group_id"), "child"."member_id", "parent"."depth" + 1 + FROM "group_membership" "child" + INNER JOIN "members" "parent" ON "child"."group_id" = "parent"."member_id" + WHERE "parent"."depth" < 10 + ) SELECT "root", "intermediate_group_ids", "member_id" FROM "members" + ON CONFLICT DO NOTHING + """) +@NamedNativeQuery(name = "EffectiveGroupMembership.deleteGroups", query = """ + DELETE + FROM "effective_group_membership" + WHERE "intermediate_group_ids" && :groupIds + """) +@NamedNativeQuery(name = "EffectiveGroupMembership.updateGroups", query = """ + INSERT INTO "effective_group_membership" ("group_id", "intermediate_group_ids", "member_id") + WITH RECURSIVE "members" ("root", "intermediate_group_ids", "member_id", "depth") AS ( + SELECT "group_id", ARRAY["group_id"]::varchar[], "member_id", 0 + FROM "group_membership" + WHERE "group_id" IN :groupIds + UNION + SELECT "parent"."root", array_append("parent"."intermediate_group_ids", "child"."group_id"), "child"."member_id", "parent"."depth" + 1 + FROM "group_membership" "child" + INNER JOIN "members" "parent" ON "child"."group_id" = "parent"."member_id" + WHERE "parent"."depth" < 10 + ) SELECT "root", "intermediate_group_ids", "member_id" FROM "members" + ON CONFLICT DO NOTHING + """) +@NamedQuery(name = "EffectiveGroupMembership.deleteUsers", query = """ + DELETE + FROM EffectiveGroupMembership egm + WHERE egm.id.memberId IN :userIds + """) +@NamedNativeQuery(name = "EffectiveGroupMembership.updateUsers", query = """ + INSERT INTO "effective_group_membership" ("group_id", "intermediate_group_ids", "member_id") + WITH RECURSIVE "members" ("group_id", "intermediate_group_ids", "member_id", "depth") AS ( + SELECT "group_id", ARRAY["group_id"]::varchar[], "member_id", 0 + FROM "group_membership" + WHERE "member_id" IN :userIds + UNION + SELECT "parent"."group_id", array_prepend("parent"."group_id", "child"."intermediate_group_ids"), "child"."member_id", "child"."depth" + 1 + FROM "group_membership" "parent" + INNER JOIN "members" "child" ON "child"."group_id" = "parent"."member_id" + WHERE "child"."depth" < 10 + ) SELECT "group_id", "intermediate_group_ids", "member_id" FROM "members" + ON CONFLICT DO NOTHING + """) @Entity @Immutable @Table(name = "effective_group_membership") @@ -19,8 +75,6 @@ public class EffectiveGroupMembership { @EmbeddedId private Id id; - private String path; - @Embeddable public static class Id implements Serializable { @@ -53,4 +107,47 @@ public String toString() { '}'; } } + + @ApplicationScoped + public static class Repository implements PanacheRepositoryBase { + + public void fullUpdate() { + deleteAll(); + getEntityManager() + .createNamedQuery("EffectiveGroupMembership.fullUpdate") + .executeUpdate(); + } + + public void updateGroups(Collection groupIds) { + Batch.of(200).run(groupIds, (batch) -> { + getEntityManager() + .createNamedQuery("EffectiveGroupMembership.deleteGroups") + .setParameter("groupIds", batch.toArray(new String[0])) // explicit cast to array, so JPA maps to VARCHAR[] instead of VARCHAR + .executeUpdate(); + getEntityManager() + .createNamedQuery("EffectiveGroupMembership.updateGroups") + .setParameter("groupIds", batch) + .executeUpdate(); + }); + } + + public void updateUsers(Collection userIds) { + Batch.of(200).run(userIds, (batch) -> { + delete("#EffectiveGroupMembership.deleteUsers", Parameters.with("userIds", batch)); + getEntityManager() + .createNamedQuery("EffectiveGroupMembership.updateUsers") + .setParameter("userIds", batch) + .executeUpdate(); + }); + } + + // visible for testing + boolean isMember(String groupId, String memberId) { + EffectiveGroupMembership.Id id = new EffectiveGroupMembership.Id(); + id.groupId = groupId; + id.memberId = memberId; + return findById(id) != null; + } + + } } diff --git a/backend/src/main/java/org/cryptomator/hub/entities/EffectiveVaultAccess.java b/backend/src/main/java/org/cryptomator/hub/entities/EffectiveVaultAccess.java index e55f4b5b1..9edeead55 100644 --- a/backend/src/main/java/org/cryptomator/hub/entities/EffectiveVaultAccess.java +++ b/backend/src/main/java/org/cryptomator/hub/entities/EffectiveVaultAccess.java @@ -17,45 +17,49 @@ import java.util.Collection; import java.util.List; import java.util.Objects; +import java.util.Set; import java.util.UUID; import java.util.stream.Collectors; @Entity @Immutable @Table(name = "effective_vault_access") -@NamedQuery(name = "EffectiveVaultAccess.countSeatsOccupiedBySingleUser", query = """ - SELECT count(u) +@NamedQuery(name = "EffectiveVaultAccess.isUserOccupyingSeat", query = """ + SELECT 1 FROM User u INNER JOIN EffectiveVaultAccess eva ON u.id = eva.id.authorityId - WHERE eva.id.authorityId = :userId + INNER JOIN Vault v ON eva.id.vaultId = v.id + WHERE u.id = :userId AND NOT v.archived """) @NamedQuery(name = "EffectiveVaultAccess.countSeatsOccupiedByUsers", query = """ SELECT COUNT(DISTINCT u.id) FROM User u INNER JOIN EffectiveVaultAccess eva ON u.id = eva.id.authorityId - INNER JOIN Vault v ON eva.id.vaultId = v.id AND NOT v.archived - WHERE u.id IN :userIds + INNER JOIN Vault v ON eva.id.vaultId = v.id + WHERE u.id IN :userIds AND NOT v.archived """) @NamedQuery(name = "EffectiveVaultAccess.countSeatOccupyingUsers", query = """ - SELECT count(DISTINCT u) + SELECT COUNT(DISTINCT u.id) FROM User u INNER JOIN EffectiveVaultAccess eva ON u.id = eva.id.authorityId - INNER JOIN Vault v ON eva.id.vaultId = v.id AND NOT v.archived + INNER JOIN Vault v ON eva.id.vaultId = v.id + WHERE NOT v.archived """) @NamedQuery(name = "EffectiveVaultAccess.countSeatOccupyingUsersWithAccessToken", query = """ - SELECT count(DISTINCT u) + SELECT COUNT(DISTINCT u.id) FROM User u INNER JOIN EffectiveVaultAccess eva ON u.id = eva.id.authorityId - INNER JOIN Vault v ON eva.id.vaultId = v.id AND NOT v.archived + INNER JOIN Vault v ON eva.id.vaultId = v.id INNER JOIN AccessToken at ON eva.id.vaultId = at.id.vaultId AND eva.id.authorityId = at.id.userId + WHERE NOT v.archived """) @NamedQuery(name = "EffectiveVaultAccess.countSeatOccupyingUsersOfGroup", query = """ - SELECT count(DISTINCT u) + SELECT COUNT(DISTINCT u.id) FROM User u INNER JOIN EffectiveVaultAccess eva ON u.id = eva.id.authorityId INNER JOIN EffectiveGroupMembership egm ON u.id = egm.id.memberId - INNER JOIN Vault v ON eva.id.vaultId = v.id AND NOT v.archived - WHERE egm.id.groupId = :groupId + INNER JOIN Vault v ON eva.id.vaultId = v.id + WHERE egm.id.groupId = :groupId AND NOT v.archived """) @NamedQuery(name = "EffectiveVaultAccess.findByAuthorityAndVault", query = """ SELECT eva @@ -149,11 +153,14 @@ public String toString() { public static class Repository implements PanacheRepositoryBase { public boolean isUserOccupyingSeat(String userId) { - return count("#EffectiveVaultAccess.countSeatsOccupiedBySingleUser", Parameters.with("userId", userId)) > 0; + return find("#EffectiveVaultAccess.isUserOccupyingSeat", Parameters.with("userId", userId)).page(0, 1).firstResult() != null; } - public long countSeatsOccupiedByUsers(List userIds) { - return count("#EffectiveVaultAccess.countSeatsOccupiedByUsers", Parameters.with("userIds", userIds)); + public long countSeatsOccupiedByUsers(Collection userIds) { + return Batch.of(200).run(Set.copyOf(userIds), 0L, (batch, result) -> { + long partialCount = count("#EffectiveVaultAccess.countSeatsOccupiedByUsers", Parameters.with("userIds", batch)); + return result + partialCount; + }); } public long countSeatOccupyingUsers() { diff --git a/backend/src/main/java/org/cryptomator/hub/entities/Group.java b/backend/src/main/java/org/cryptomator/hub/entities/Group.java index 88a310370..898f19cde 100644 --- a/backend/src/main/java/org/cryptomator/hub/entities/Group.java +++ b/backend/src/main/java/org/cryptomator/hub/entities/Group.java @@ -1,6 +1,7 @@ package org.cryptomator.hub.entities; import io.quarkus.hibernate.orm.panache.PanacheRepositoryBase; +import io.quarkus.panache.common.Parameters; import jakarta.enterprise.context.ApplicationScoped; import jakarta.persistence.CascadeType; import jakarta.persistence.DiscriminatorValue; @@ -11,6 +12,7 @@ import jakarta.persistence.Table; import jakarta.persistence.Transient; +import java.util.Collection; import java.util.HashSet; import java.util.Set; @@ -41,5 +43,10 @@ public int getMemberSize() { @ApplicationScoped public static class Repository implements PanacheRepositoryBase { + + public long deleteByIds(Collection ids) { + return Batch.of(200).run(ids, 0L, (batch, result) -> result + delete("id IN :ids", Parameters.with("ids", batch))); + } + } } diff --git a/backend/src/main/java/org/cryptomator/hub/entities/User.java b/backend/src/main/java/org/cryptomator/hub/entities/User.java index 1bf3a49cb..14ce428d1 100644 --- a/backend/src/main/java/org/cryptomator/hub/entities/User.java +++ b/backend/src/main/java/org/cryptomator/hub/entities/User.java @@ -11,6 +11,7 @@ import jakarta.persistence.OneToMany; import jakarta.persistence.Table; +import java.util.Collection; import java.util.HashSet; import java.util.Objects; import java.util.Set; @@ -163,6 +164,10 @@ public int hashCode() { @ApplicationScoped public static class Repository implements PanacheRepositoryBase { + public long deleteByIds(Collection ids) { + return Batch.of(200).run(ids, 0L, (batch, result) -> result + delete("id IN :ids", Parameters.with("ids", batch))); + } + public Stream findRequiringAccessGrant(UUID vaultId) { return find("#User.requiringAccessGrant", Parameters.with("vaultId", vaultId)).stream(); } diff --git a/backend/src/main/java/org/cryptomator/hub/entities/Vault.java b/backend/src/main/java/org/cryptomator/hub/entities/Vault.java index ed1bdf077..20b9512e7 100644 --- a/backend/src/main/java/org/cryptomator/hub/entities/Vault.java +++ b/backend/src/main/java/org/cryptomator/hub/entities/Vault.java @@ -67,14 +67,6 @@ public class Vault { ) private Set directMembers = new HashSet<>(); - @ManyToMany - @Immutable - @JoinTable(name = "effective_vault_access", - joinColumns = @JoinColumn(name = "vault_id", referencedColumnName = "id"), - inverseJoinColumns = @JoinColumn(name = "authority_id", referencedColumnName = "id") - ) - private Set effectiveMembers = new HashSet<>(); - @OneToMany(mappedBy = "vault", fetch = FetchType.LAZY) private Set accessTokens = new HashSet<>(); @@ -142,14 +134,6 @@ public void setDirectMembers(Set directMembers) { this.directMembers = directMembers; } - public Set getEffectiveMembers() { - return effectiveMembers; - } - - public void setEffectiveMembers(Set effectiveMembers) { - this.effectiveMembers = effectiveMembers; - } - public Set getAccessTokens() { return accessTokens; } @@ -276,7 +260,10 @@ public Stream findAccessibleByUser(String userId, VaultAccess.Role role) } public Stream findAllInList(List ids) { - return find("#Vault.allInList", Parameters.with("ids", ids)).stream(); + return Batch.of(200).run(ids, Stream.of(), (batch, result) -> { + Stream partialResult = find("#Vault.allInList", Parameters.with("ids", batch)).stream(); + return Stream.concat(result, partialResult); + }); } } } diff --git a/backend/src/main/java/org/cryptomator/hub/filters/VaultRole.java b/backend/src/main/java/org/cryptomator/hub/filters/VaultRole.java index 39e4098ad..9711d77b0 100644 --- a/backend/src/main/java/org/cryptomator/hub/filters/VaultRole.java +++ b/backend/src/main/java/org/cryptomator/hub/filters/VaultRole.java @@ -33,10 +33,16 @@ OnMissingVault onMissingVault() default OnMissingVault.FORBIDDEN; enum OnMissingVault { FORBIDDEN, NOT_FOUND, PASS, REQUIRE_REALM_ROLE } + /** + * If set to true, skip the role check if the current user has the role {@link #realmRole()}. + * @return whether the given realm role allows bypassing the vault role check. + */ + boolean bypassForRealmRole() default false; + /** * Which additional realm role is required to access the annotated resource. * - * Only relevant if {@link #onMissingVault()} is set to {@link OnMissingVault#REQUIRE_REALM_ROLE}. + * Only relevant if {@link #bypassForRealmRole()} or {@link #onMissingVault()} is set to {@link OnMissingVault#REQUIRE_REALM_ROLE}. * @return realm role required to access the annotated resource. */ String realmRole() default ""; diff --git a/backend/src/main/java/org/cryptomator/hub/filters/VaultRoleFilter.java b/backend/src/main/java/org/cryptomator/hub/filters/VaultRoleFilter.java index ca94d360b..ef621cdf3 100644 --- a/backend/src/main/java/org/cryptomator/hub/filters/VaultRoleFilter.java +++ b/backend/src/main/java/org/cryptomator/hub/filters/VaultRoleFilter.java @@ -42,12 +42,17 @@ public class VaultRoleFilter implements ContainerRequestFilter { @Override public void filter(ContainerRequestContext requestContext) throws NotFoundException, ForbiddenException, NotAuthorizedException { var annotation = resourceInfo.getResourceMethod().getAnnotation(VaultRole.class); + if (annotation.bypassForRealmRole() && requestContext.getSecurityContext().isUserInRole(annotation.realmRole())) { + // user has required realm role, so we skip the vault role check: + return; + } + var vaultIdStr = requestContext.getUriInfo().getPathParameters().getFirst(annotation.vaultIdParam()); final UUID vaultId; try { vaultId = UUID.fromString(vaultIdStr); } catch (NullPointerException | IllegalArgumentException e) { - throw new ForbiddenException("@VaultRole not set up correctly (unknown vault id)", e); + throw new NotFoundException("@VaultRole not set up correctly (unknown vault id)", e); } var userId = jwt.getSubject(); diff --git a/backend/src/main/java/org/cryptomator/hub/keycloak/KeycloakAuthorityPuller.java b/backend/src/main/java/org/cryptomator/hub/keycloak/KeycloakAuthorityPuller.java index 164f6136f..0f2f80ae4 100644 --- a/backend/src/main/java/org/cryptomator/hub/keycloak/KeycloakAuthorityPuller.java +++ b/backend/src/main/java/org/cryptomator/hub/keycloak/KeycloakAuthorityPuller.java @@ -5,6 +5,7 @@ import jakarta.inject.Inject; import jakarta.transaction.Transactional; import org.cryptomator.hub.entities.Authority; +import org.cryptomator.hub.entities.EffectiveGroupMembership; import org.cryptomator.hub.entities.Group; import org.cryptomator.hub.entities.User; @@ -23,6 +24,8 @@ public class KeycloakAuthorityPuller { Group.Repository groupRepo; @Inject KeycloakAuthorityProvider remoteUserProvider; + @Inject + EffectiveGroupMembership.Repository effectiveGroupMembershipRepo; @Scheduled(every = "{hub.keycloak.syncer-period}") void sync() { @@ -46,23 +49,24 @@ void sync(Map keycloakGroups, Map keycloakUsers, Map databaseUsers) { var addedIds = diff(keycloakUsers.keySet(), databaseUsers.keySet()); - for (var id : addedIds) { + var added = addedIds.stream().map(id -> { var keycloakUser = keycloakUsers.get(id); var databaseUser = new User(); databaseUser.setId(keycloakUser.id()); databaseUser.setName(keycloakUser.name()); databaseUser.setEmail(keycloakUser.email()); databaseUser.setPictureUrl(keycloakUser.pictureUrl()); - userRepo.persist(databaseUser); - } + return databaseUser; + }); + userRepo.persist(added); + effectiveGroupMembershipRepo.updateUsers(addedIds); } //visible for testing Set syncDeletedUsers(Map keycloakUsers, Map databaseUsers) { var deletedIds = diff(databaseUsers.keySet(), keycloakUsers.keySet()); - for (var id : deletedIds) { - userRepo.delete(databaseUsers.get(id)); - } + userRepo.deleteByIds(deletedIds); + effectiveGroupMembershipRepo.updateUsers(deletedIds); return deletedIds; } @@ -81,7 +85,7 @@ void syncUpdatedUsers(Map keycloakUsers, Map keycloakGroups, Map databaseGroups, Map databaseUsers) { var addedIds = diff(keycloakGroups.keySet(), databaseGroups.keySet()); - for (var id : addedIds) { + var added = addedIds.stream().map(id -> { var keycloakGroup = keycloakGroups.get(id); var databaseGroup = new Group(); databaseGroup.setId(keycloakGroup.id()); @@ -94,26 +98,29 @@ void syncAddedGroups(Map keycloakGroups, Map syncDeletedGroups(Map keycloakGroups, Map databaseGroups) { var deletedIds = diff(databaseGroups.keySet(), keycloakGroups.keySet()); - for (var id : deletedIds) { - var databaseGroup = databaseGroups.get(id); - groupRepo.delete(databaseGroup); - } + groupRepo.deleteByIds(deletedIds); + effectiveGroupMembershipRepo.updateGroups(deletedIds); return deletedIds; } //visible for testing void syncUpdatedGroups(Map keycloakGroups, Map databaseGroups, Set deletedGroupIds, Map databaseUsers) { var toUpdateIds = diff(databaseGroups.keySet(), deletedGroupIds); + var idsOfGroupsWithChangedMembers = new HashSet(); for (var id : toUpdateIds) { var databaseGroup = databaseGroups.get(id); var keycloakGroup = keycloakGroups.get(id); @@ -127,12 +134,18 @@ void syncUpdatedGroups(Map keycloakGroups, Map u.getId().equals(removeId)); } + if (!wantIds.containsAll(haveIds) || !haveIds.containsAll(wantIds)) { + idsOfGroupsWithChangedMembers.add(id); + } } + effectiveGroupMembershipRepo.updateGroups(idsOfGroupsWithChangedMembers); } private Set diff(Set base, Set difference) { diff --git a/backend/src/main/resources/org/cryptomator/hub/flyway/V22__Optimize_Group_Membership.sql b/backend/src/main/resources/org/cryptomator/hub/flyway/V22__Optimize_Group_Membership.sql new file mode 100644 index 000000000..c6b768828 --- /dev/null +++ b/backend/src/main/resources/org/cryptomator/hub/flyway/V22__Optimize_Group_Membership.sql @@ -0,0 +1,47 @@ +-- noinspection SqlNoDataSourceInspectionForFile + +-- top down lookups ("get members of a group") is already efficient due to PK +-- bottom up lookups ("get groups of a member") need an additional index: +CREATE INDEX GROUP_MEMBERSHIP_IDX_MEMBER ON "group_membership" ("member_id"); + +-- temporarily drop views +DROP VIEW "effective_vault_access"; +DROP VIEW "effective_group_membership"; + +-- replace effective_group_membership with a table to optimize lookups +CREATE TABLE "effective_group_membership" ( + "group_id" VARCHAR(255) NOT NULL, + "intermediate_group_ids" VARCHAR[] NOT NULL, + "member_id" VARCHAR(255) NOT NULL, + PRIMARY KEY ("group_id", "member_id"), + CONSTRAINT "EFFECTIVE_GROUP_MEMBERSHIP_FK_GROUP" FOREIGN KEY ("group_id") REFERENCES "authority" ("id") ON DELETE CASCADE, + CONSTRAINT "EFFECTIVE_GROUP_MEMBERSHIP_FK_MEMBER" FOREIGN KEY ("member_id") REFERENCES "authority" ("id") ON DELETE CASCADE, + CONSTRAINT "EFFECTIVE_GROUP_MEMBERSHIP_CHK_NOTSAME" CHECK ("group_id" <> "member_id") +); + +-- top down lookups ("get members of a group") is already efficient due to PK +-- bottom up lookups ("get groups of a member") need an additional index: +CREATE INDEX EFFECTIVE_GROUP_MEMBERSHIP_IDX_MEMBER ON "effective_group_membership" ("member_id"); + +-- index to speed up queries filtering by intermediate groups +CREATE INDEX EFFECTIVE_GROUP_MEMBERSHIP_IDX_INTERMEDIATE on "effective_group_membership" USING GIN("intermediate_group_ids"); + +-- @formatter:off +INSERT INTO "effective_group_membership" ("group_id", "intermediate_group_ids", "member_id") +WITH RECURSIVE "members" ("root","intermediate_group_ids","member_id", "depth") AS ( + SELECT "group_id", ARRAY["group_id"]::varchar[], "member_id", 0 + FROM "group_membership" + UNION + SELECT "parent"."root", array_append("parent"."intermediate_group_ids", "child"."group_id"), "child"."member_id", "parent"."depth" + 1 + FROM "group_membership" "child" + INNER JOIN "members" "parent" ON "child"."group_id" = "parent"."member_id" + WHERE "parent"."depth" < 10 +) SELECT "root", "intermediate_group_ids", "member_id" FROM "members" +ON CONFLICT DO NOTHING; + +CREATE VIEW "effective_vault_access" ("vault_id", "authority_id", "role") AS + SELECT "va"."vault_id", "va"."authority_id", "va"."role" FROM "vault_access" "va" + UNION + SELECT "va"."vault_id", "gm"."member_id", "va"."role" FROM "vault_access" "va" + INNER JOIN "effective_group_membership" "gm" ON "va"."authority_id" = "gm"."group_id"; +-- @formatter:on \ No newline at end of file diff --git a/backend/src/test/java/org/cryptomator/hub/api/ExceedingLicenseLimitsIT.java b/backend/src/test/java/org/cryptomator/hub/api/ExceedingLicenseLimitsIT.java new file mode 100644 index 000000000..6393a3587 --- /dev/null +++ b/backend/src/test/java/org/cryptomator/hub/api/ExceedingLicenseLimitsIT.java @@ -0,0 +1,239 @@ +package org.cryptomator.hub.api; + +import io.quarkus.test.junit.QuarkusTest; +import io.quarkus.test.security.TestSecurity; +import io.quarkus.test.security.oidc.Claim; +import io.quarkus.test.security.oidc.OidcSecurity; +import io.restassured.http.ContentType; +import jakarta.inject.Inject; +import jakarta.transaction.Transactional; +import org.cryptomator.hub.entities.EffectiveGroupMembership; +import org.cryptomator.hub.entities.Group; +import org.cryptomator.hub.entities.User; +import org.cryptomator.hub.entities.Vault; +import org.cryptomator.hub.entities.VaultAccess; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.Assumptions; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.MethodOrderer; +import org.junit.jupiter.api.Order; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; +import org.junit.jupiter.api.TestMethodOrder; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; + +import java.sql.SQLException; +import java.time.Instant; +import java.util.List; +import java.util.Map; +import java.util.UUID; + +import static io.restassured.RestAssured.given; +import static io.restassured.RestAssured.when; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.text.IsEqualIgnoringCase.equalToIgnoringCase; + +@QuarkusTest +@TestSecurity(user = "User Name 1", roles = {"user", "create-vaults"}) +@OidcSecurity(claims = { + @Claim(key = "sub", value = "user1") +}) +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +@TestMethodOrder(MethodOrderer.OrderAnnotation.class) +public class ExceedingLicenseLimitsIT { + + @Inject + Group.Repository groupRepo; + @Inject + User.Repository userRepo; + @Inject + EffectiveGroupMembership.Repository effectiveGroupMembershipRepo; + @Inject + Vault.Repository vaultRepo; + @Inject + VaultAccess.Repository vaultAccessRepo; + + private final VaultResourceIT vaultResourceIT; + + public ExceedingLicenseLimitsIT(VaultResourceIT vaultResourceIT) { + this.vaultResourceIT = vaultResourceIT; + } + + @BeforeAll + @Transactional + public void setupTestData() { + var user91 = new User(); + user91.setId("user91"); + user91.setName("user name 91"); + + var user92 = new User(); + user92.setId("user92"); + user92.setName("user name 92"); + + var user93 = new User(); + user93.setId("user93"); + user93.setName("user name 93"); + + var user94 = new User(); + user94.setId("user94"); + user94.setName("user name 94"); + + var user95A = new User(); + user95A.setId("user95_A"); + user95A.setName("user name Archived"); + + userRepo.persist(user91, user92, user93, user94, user95A); + + var group91 = new Group(); + group91.setId("group91"); + group91.setName("Group 91"); + group91.getMembers().add(user91); + group91.getMembers().add(user92); + group91.getMembers().add(user93); + group91.getMembers().add(user94); + groupRepo.persist(group91); + + effectiveGroupMembershipRepo.updateGroups(List.of("group91")); + + var access = new VaultAccess(); + access.setVault(vaultRepo.findById(UUID.fromString("7E57C0DE-0000-4000-8000-00010000AAAA"))); + access.setAuthority(user95A); + access.setRole(VaultAccess.Role.MEMBER); + vaultAccessRepo.persist(access); + } + + @AfterAll + @Transactional + public void cleanupTestData() { + groupRepo.deleteById("group91"); + userRepo.deleteByIds(List.of("user91", "user92", "user93", "user94", "user95_A")); + } + + @Test + @Order(0) + @DisplayName("POST /vaults/7E57C0DE-0000-4000-8000-000100001111/access-tokens returns 402 for [user91, user92, user93, user94]") + public void grantAccessExceedingSeats() { + Assumptions.assumeTrue(vaultResourceIT.effectiveVaultAccessRepo.countSeatOccupyingUsers() == 2); + var body = Map.of( + "user91", "jwe.jwe.jwe.vault1.user91", // + "user92", "jwe.jwe.jwe.vault1.user92", // + "user93", "jwe.jwe.jwe.vault1.user93", // + "user94", "jwe.jwe.jwe.vault1.user94" // + ); + + given().contentType(ContentType.JSON).body(body) + .when().post("/vaults/{vaultId}/access-tokens/", "7E57C0DE-0000-4000-8000-000100001111") + .then().statusCode(402); + } + + @Test + @Order(1) + @DisplayName("PUT /vaults/7E57C0DE-0000-4000-8000-000100001111/groups/group91 returns 402") + public void addGroupToVaultExceedingSeats() { + Assumptions.assumeTrue(vaultResourceIT.effectiveVaultAccessRepo.countSeatOccupyingUsers() == 2); + + given().when().put("/vaults/{vaultId}/groups/{groupId}", "7E57C0DE-0000-4000-8000-000100001111", "group91") + .then().statusCode(402); + } + + @Order(2) + @DisplayName("PUT /vaults/7E57C0DE-0000-4000-8000-000100001111/users/userXX returns 201") + @ParameterizedTest(name = "Adding user {0} succeeds") + @CsvSource(value = {"0,user91", "1,user92", "2,user93"}) + public void addUserToVaultNotExceedingSeats(String run, String userId) { + Assumptions.assumeTrue(vaultResourceIT.effectiveVaultAccessRepo.countSeatOccupyingUsers() == 2 + Integer.parseInt(run)); + + given().when().put("/vaults/{vaultId}/users/{usersId}", "7E57C0DE-0000-4000-8000-000100001111", userId) + .then().statusCode(201); + } + + @Test + @Order(3) + @DisplayName("PUT /vaults/7E57C0DE-0000-4000-8000-000100001111/users/user94 returns 402") + public void addUserToVaultExceedingSeats() { + Assumptions.assumeTrue(vaultResourceIT.effectiveVaultAccessRepo.countSeatOccupyingUsers() == 5); + + given().when().put("/vaults/{vaultId}/users/{usersId}", "7E57C0DE-0000-4000-8000-000100001111", "user94") + .then().statusCode(402); + } + + @Test + @Order(4) + @DisplayName("PUT /vaults/7E57C0DE-0000-4000-8000-000100001111 (as user1) returns 200 with only updated name, description and archive flag, despite exceeding license") + public void testUpdateVaultDespiteLicenseExceeded() { + Assumptions.assumeTrue(vaultResourceIT.effectiveVaultAccessRepo.countSeatOccupyingUsers() == 5); + var vaultId = "7E57C0DE-0000-4000-8000-000100001111"; + + var vaultDto = new VaultResource.VaultDto(UUID.fromString(vaultId), "Vault 1", "This is a testvault.", false, Instant.parse("2222-11-11T11:11:11Z"), "someValue", -1, "doNotUpdate", "doNotUpdate", "doNotUpdate"); + given().contentType(ContentType.JSON) + .body(vaultDto) + .when().put("/vaults/{vaultId}", vaultId) + .then().statusCode(200) + .body("id", equalToIgnoringCase(vaultId)) + .body("name", equalTo("Vault 1")) + .body("description", equalTo("This is a testvault.")) + .body("archived", equalTo(false)); + } + + @Test + @Order(5) + @DisplayName("PUT /vaults/7E57C0DE-0000-4000-8000-0001FFFF3333 (as user1) exceeding the license returns 402") + public void testCreateVaultExceedingSeats() throws SQLException { + try (var c = vaultResourceIT.dataSource.getConnection(); var s = c.createStatement()) { + s.execute(""" + INSERT INTO "vault_access" ("vault_id", "authority_id") + VALUES + ('7E57C0DE-0000-4000-8000-000100001111', 'group91'); + """); + } + + Assumptions.assumeTrue(vaultResourceIT.effectiveVaultAccessRepo.countSeatOccupyingUsers() > 5); + + var uuid = UUID.fromString("7E57C0DE-0000-4000-8000-0001FFFF3333"); + var vaultDto = new VaultResource.VaultDto(uuid, "My Vault", "Test vault 4", false, Instant.parse("2112-12-21T21:12:21Z"), "masterkey3", 42, "NaCl", "authPubKey3", "authPrvKey3"); + given().contentType(ContentType.JSON).body(vaultDto) + .when().put("/vaults/{vaultId}", "7E57C0DE-0000-4000-8000-0001FFFF3333") + .then().statusCode(402); + } + + @Test + @Order(7) + @DisplayName("unlock/legacyUnlock is granted, if (effective vault user) > license seats but (effective vault user with access token) <= license seat") + public void testUnlockAllowedExceedingLicenseSoftLimit() { + Assumptions.assumeTrue(vaultResourceIT.effectiveVaultAccessRepo.countSeatOccupyingUsersWithAccessToken() <= 5); + + when().get("/vaults/{vaultId}/access-token", "7E57C0DE-0000-4000-8000-000100001111") + .then().statusCode(200); + when().get("/vaults/{vaultId}/keys/{deviceId}", "7E57C0DE-0000-4000-8000-000100002222", "legacyDevice3") + .then().statusCode(200) + .body(is("legacy.jwe.jwe.vault2.device3")); + } + + @Test + @Order(8) + @DisplayName("Unlock/legacyUnlock is blocked if (effective vault users with token) > license seats") + public void testUnlockBlockedExceedingLicenseHardLimit() throws SQLException { + try (var c = vaultResourceIT.dataSource.getConnection(); var s = c.createStatement()) { + s.execute(""" + INSERT INTO "access_token" ("user_id", "vault_id", "vault_masterkey") + VALUES ('user91', '7E57C0DE-0000-4000-8000-000100001111', 'jwe.jwe.jwe.vault1.user91'); + INSERT INTO "access_token" ("user_id", "vault_id", "vault_masterkey") + VALUES ('user92', '7E57C0DE-0000-4000-8000-000100001111', 'jwe.jwe.jwe.vault1.user92'); + INSERT INTO "access_token" ("user_id", "vault_id", "vault_masterkey") + VALUES ('user93', '7E57C0DE-0000-4000-8000-000100001111', 'jwe.jwe.jwe.vault1.user93'); + INSERT INTO "access_token" ("user_id", "vault_id", "vault_masterkey") + VALUES ('user94', '7E57C0DE-0000-4000-8000-000100001111', 'jwe.jwe.jwe.vault1.user94'); + """); + } + Assumptions.assumeTrue(vaultResourceIT.effectiveVaultAccessRepo.countSeatOccupyingUsersWithAccessToken() > 5); + + when().get("/vaults/{vaultId}/access-token", "7E57C0DE-0000-4000-8000-000100001111") + .then().statusCode(402); + when().get("/vaults/{vaultId}/keys/{deviceId}", "7E57C0DE-0000-4000-8000-000100002222", "legacyDevice3") + .then().statusCode(402); + } + +} diff --git a/backend/src/test/java/org/cryptomator/hub/api/GroupsResourceIT.java b/backend/src/test/java/org/cryptomator/hub/api/GroupsResourceIT.java index 41325d7d5..e1e3b7fd1 100644 --- a/backend/src/test/java/org/cryptomator/hub/api/GroupsResourceIT.java +++ b/backend/src/test/java/org/cryptomator/hub/api/GroupsResourceIT.java @@ -1,13 +1,18 @@ package org.cryptomator.hub.api; -import io.agroal.api.AgroalDataSource; import io.quarkus.test.junit.QuarkusTest; import io.quarkus.test.security.TestSecurity; import io.quarkus.test.security.oidc.Claim; import io.quarkus.test.security.oidc.OidcSecurity; import io.restassured.RestAssured; import jakarta.inject.Inject; +import jakarta.transaction.Transactional; +import org.cryptomator.hub.entities.EffectiveGroupMembership; +import org.cryptomator.hub.entities.Group; +import org.cryptomator.hub.entities.User; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; @@ -15,24 +20,57 @@ import org.junit.jupiter.params.provider.CsvSource; import java.sql.SQLException; +import java.util.List; -import static io.restassured.RestAssured.given; import static io.restassured.RestAssured.when; import static org.hamcrest.CoreMatchers.hasItems; -import static org.hamcrest.CoreMatchers.is; @QuarkusTest @DisplayName("Resource /groups") public class GroupsResourceIT { @Inject - AgroalDataSource dataSource; + Group.Repository groupRepo; + + @Inject + User.Repository userRepo; + + @Inject + EffectiveGroupMembership.Repository effectiveGroupMembershipRepo; @BeforeAll public static void beforeAll() { RestAssured.enableLoggingOfRequestAndResponseIfValidationFails(); } + @BeforeEach + @Transactional + public void setupTestData() { + var user999 = new User(); + user999.setId("user999"); + user999.setName("User 999"); + userRepo.persist(user999); + + var group999 = new Group(); + group999.setId("group999"); + group999.setName("Group 999"); + group999.getMembers().add(user999); + groupRepo.persist(group999); + + var group1 = groupRepo.findById("group1"); + group1.getMembers().add(group999); + groupRepo.persist(group1); + + effectiveGroupMembershipRepo.updateGroups(List.of("group1", "group999")); + } + + @AfterEach + @Transactional + public void cleanupTestData() { + groupRepo.deleteById("group999"); + userRepo.deleteById("user999"); + } + @Nested @DisplayName("As user1") @TestSecurity(user = "User Name 1", roles = {"user"}) @@ -52,32 +90,9 @@ public void testGetAll() { @Test @DisplayName("GET /groups/group1/effective-members contains direct and subgroup members") public void testGetEffectiveUsers() throws SQLException { - try (var c = dataSource.getConnection(); var s = c.createStatement()) { - s.execute(""" - INSERT INTO "authority" ("id", "type", "name") - VALUES - ('user999', 'USER', 'User 999'), - ('group999', 'GROUP', 'Group 999'); - - INSERT INTO "user_details" ("id") VALUES ('user999'); - INSERT INTO "group_details" ("id") VALUES ('group999'); - - INSERT INTO "group_membership" ("group_id", "member_id") - VALUES - ('group999', 'user999'), - ('group1', 'group999'); - """); - } - when().get("/groups/{groupId}/effective-members", "group1") .then().statusCode(200) .body("id", hasItems("user1", "user999")); - - try (var c = dataSource.getConnection(); var s = c.createStatement()) { - s.execute(""" - DELETE FROM "authority" WHERE "id" = 'user999' OR "id" = 'group999'; - """); - } } } diff --git a/backend/src/test/java/org/cryptomator/hub/api/VaultResourceIT.java b/backend/src/test/java/org/cryptomator/hub/api/VaultResourceIT.java index b80fa56be..83ebe2e33 100644 --- a/backend/src/test/java/org/cryptomator/hub/api/VaultResourceIT.java +++ b/backend/src/test/java/org/cryptomator/hub/api/VaultResourceIT.java @@ -4,6 +4,7 @@ import com.auth0.jwt.algorithms.Algorithm; import io.agroal.api.AgroalDataSource; import io.quarkus.narayana.jta.QuarkusTransaction; +import io.quarkus.test.TestTransaction; import io.quarkus.test.junit.QuarkusTest; import io.quarkus.test.security.TestSecurity; import io.quarkus.test.security.oidc.Claim; @@ -11,8 +12,12 @@ import io.restassured.RestAssured; import io.restassured.http.ContentType; import jakarta.inject.Inject; +import jakarta.transaction.Transactional; import jakarta.validation.Validator; +import org.cryptomator.hub.entities.EffectiveGroupMembership; import org.cryptomator.hub.entities.EffectiveVaultAccess; +import org.cryptomator.hub.entities.Group; +import org.cryptomator.hub.entities.User; import org.cryptomator.hub.entities.Vault; import org.cryptomator.hub.rollback.DBRollbackAfter; import org.cryptomator.hub.rollback.DBRollbackBefore; @@ -20,9 +25,11 @@ import org.hamcrest.MatcherAssert; import org.hamcrest.Matchers; import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.MethodOrderer; import org.junit.jupiter.api.Nested; @@ -45,7 +52,9 @@ import java.sql.SQLException; import java.time.Instant; import java.util.Base64; +import java.util.List; import java.util.Map; +import java.util.Set; import java.util.UUID; import static io.restassured.RestAssured.given; @@ -70,6 +79,12 @@ public class VaultResourceIT { @Inject Vault.Repository vaultRepo; @Inject + Group.Repository groupRepo; + @Inject + User.Repository userRepo; + @Inject + EffectiveGroupMembership.Repository effectiveGroupMembershipRepo; + @Inject Validator validator; @Inject public Flyway flyway; @@ -79,8 +94,36 @@ public static void beforeAll() { RestAssured.enableLoggingOfRequestAndResponseIfValidationFails(); } - private static PrivateKey getPrivateKey(String keyBytes) throws NoSuchAlgorithmException, InvalidKeySpecException { - return KeyFactory.getInstance("EC").generatePrivate(new PKCS8EncodedKeySpec(Base64.getDecoder().decode(keyBytes))); + @BeforeEach + @Transactional + public void setupTestData() { + var user998 = new User(); + user998.setId("user998"); + user998.setName("User 998"); + + var user999 = new User(); + user999.setId("user999"); + user999.setName("User 999"); + user999.setEcdhPublicKey("ecdh_public999"); + user999.setEcdsaPublicKey("ecdsa_public999"); + user999.setPrivateKeys("private999"); + user999.setSetupCode("setup999"); + + userRepo.persist(user998, user999); + + var group2 = groupRepo.findById("group2"); + group2.getMembers().add(user998); + group2.getMembers().add(user999); + groupRepo.persist(group2); + + effectiveGroupMembershipRepo.updateUsers(List.of("user998", "user999")); + effectiveGroupMembershipRepo.updateGroups(List.of("group2")); + } + + @AfterEach + @Transactional + public void cleanupTestData() { + userRepo.deleteByIds(List.of("user998", "user999")); } @Nested @@ -130,7 +173,7 @@ public void testGetVault1() { @Test @DisplayName("GET /vaults/nonExistingVault returns 404") public void testGetVault2() { - when().get("/vaults/{vaultId}", "nonExistingVault") + when().get("/vaults/{vaultId}", "7E57C0DE-0000-4000-8000-BADBADBADBAD") .then().statusCode(404); } @@ -362,28 +405,10 @@ public void testGrantAccess0() { @Test @DisplayName("POST /vaults/7E57C0DE-0000-4000-8000-000100001111/access-tokens returns 200 for [user998, user999]") - public void testGrantAccess1() throws SQLException { - try (var c = dataSource.getConnection(); var s = c.createStatement()) { - s.execute(""" - INSERT INTO "authority" ("id", "type", "name") VALUES ('user998', 'USER', 'User 998'); - INSERT INTO "authority" ("id", "type", "name") VALUES ('user999', 'USER', 'User 999'); - INSERT INTO "user_details" ("id") VALUES ('user998'); - INSERT INTO "user_details" ("id") VALUES ('user999'); - INSERT INTO "group_membership" ("group_id", "member_id") VALUES ('group2', 'user998'); - INSERT INTO "group_membership" ("group_id", "member_id") VALUES ('group2', 'user999'); - """); - } - + public void testGrantAccess1() { given().contentType(ContentType.JSON).body(Map.of("user998", "jwe.jwe.jwe.vault1.user998", "user999", "jwe.jwe.jwe.vault1.user999")) .when().post("/vaults/{vaultId}/access-tokens/", "7E57C0DE-0000-4000-8000-000100001111") .then().statusCode(200); - - try (var c = dataSource.getConnection(); var s = c.createStatement()) { - s.execute(""" - DELETE FROM "authority" WHERE "id" = 'user998'; - DELETE FROM "authority" WHERE "id" = 'user999'; - """); - } } @Test @@ -584,18 +609,6 @@ public void getMembersOfVault2c() { @TestMethodOrder(MethodOrderer.OrderAnnotation.class) public class ManageAccessAsUser1 { - @BeforeAll - public void setup() throws SQLException { - try (var c = dataSource.getConnection(); var s = c.createStatement()) { - // user999 will be deleted in #cleanup() - s.execute(""" - INSERT INTO "authority" ("id", "type", "name") VALUES ('user999', 'USER', 'User 999'); - INSERT INTO "user_details" ("id", "ecdh_publickey", "ecdsa_publickey", "privatekeys", "setupcode") VALUES ('user999', 'ecdh_public999', 'ecdsa_public999', 'private999', 'setup999'); - INSERT INTO "group_membership" ("group_id", "member_id") VALUES ('group2', 'user999') - """); - } - } - @Test @Order(1) @DisplayName("PUT /vaults/7E57C0DE-0000-4000-8000-000100001111/groups/group3000 returns 404") @@ -614,11 +627,11 @@ public void addGroupToVault() { @Test @Order(3) - @DisplayName("GET /vaults/7E57C0DE-0000-4000-8000-000100001111/members does contain group2 with memberSize=2") + @DisplayName("GET /vaults/7E57C0DE-0000-4000-8000-000100001111/members does contain group2 with memberSize=3") public void getMembersOfVault1a() { given().when().get("/vaults/{vaultId}/members", "7E57C0DE-0000-4000-8000-000100001111") .then().statusCode(200) - .body("find { it.id == 'group2' }.memberSize", equalTo(2)); + .body("find { it.id == 'group2' }.memberSize", equalTo(3)); } @Test @@ -645,12 +658,7 @@ public void testGrantAccess2() { given().contentType(ContentType.JSON).body(Map.of("user999", "jwe.jwe.jwe.vault2.user999")) .when().post("/vaults/{vaultId}/access-tokens/", "7E57C0DE-0000-4000-8000-000100001111") .then().statusCode(200); - } - @Test - @Order(6) - @DisplayName("GET /vaults/7E57C0DE-0000-4000-8000-000100001111/users-requiring-access-grant does no longer contain user999") - public void testGetUsersRequiringAccess4() { given().when().get("/vaults/{vaultId}/users-requiring-access-grant", "7E57C0DE-0000-4000-8000-000100001111") .then().statusCode(200) .body("id", not(hasItems("user999"))); @@ -676,191 +684,6 @@ public void getMembersOfVault1b() { } - @Nested - @DisplayName("When exceeding 5 seats in license") - @TestSecurity(user = "User Name 1", roles = {"user", "create-vaults"}) - @OidcSecurity(claims = { - @Claim(key = "sub", value = "user1") - }) - @TestInstance(TestInstance.Lifecycle.PER_CLASS) - @TestMethodOrder(MethodOrderer.OrderAnnotation.class) - public class ExceedingLicenseLimits { - - @BeforeAll - public void setup() throws SQLException { - try (var c = dataSource.getConnection(); var s = c.createStatement()) { - s.execute(""" - INSERT INTO "authority" ("id", "type", "name") - VALUES - ('user91', 'USER', 'user name 91'), - ('user92', 'USER', 'user name 92'), - ('user93', 'USER', 'user name 93'), - ('user94', 'USER', 'user name 94'), - ('user95_A', 'USER', 'user name Archived'), - ('group91', 'GROUP', 'group name 91'); - - INSERT INTO "group_details" ("id") - VALUES - ('group91'); - - INSERT INTO "user_details" ("id") - VALUES - ('user91'), - ('user92'), - ('user93'), - ('user94'), - ('user95_A'); - - INSERT INTO "group_membership" ("group_id", "member_id") - VALUES - ('group91', 'user91'), - ('group91', 'user92'), - ('group91', 'user93'), - ('group91', 'user94'); - - INSERT INTO "vault_access" ("vault_id", "authority_id") - VALUES - ('7E57C0DE-0000-4000-8000-00010000AAAA', 'user95_A'); - """); - } - } - - @Test - @Order(0) - @DisplayName("POST /vaults/7E57C0DE-0000-4000-8000-000100001111/access-tokens returns 402 for [user91, user92, user93, user94]") - public void grantAccessExceedingSeats() { - Assumptions.assumeTrue(effectiveVaultAccessRepo.countSeatOccupyingUsers() == 2); - var body = Map.of( - "user91", "jwe.jwe.jwe.vault1.user91", // - "user92", "jwe.jwe.jwe.vault1.user92", // - "user93", "jwe.jwe.jwe.vault1.user93", // - "user94", "jwe.jwe.jwe.vault1.user94" // - ); - - given().contentType(ContentType.JSON).body(body) - .when().post("/vaults/{vaultId}/access-tokens/", "7E57C0DE-0000-4000-8000-000100001111") - .then().statusCode(402); - } - - @Test - @Order(1) - @DisplayName("PUT /vaults/7E57C0DE-0000-4000-8000-000100001111/groups/group91 returns 402") - public void addGroupToVaultExceedingSeats() { - Assumptions.assumeTrue(effectiveVaultAccessRepo.countSeatOccupyingUsers() == 2); - - given().when().put("/vaults/{vaultId}/groups/{groupId}", "7E57C0DE-0000-4000-8000-000100001111", "group91") - .then().statusCode(402); - } - - @Order(2) - @DisplayName("PUT /vaults/7E57C0DE-0000-4000-8000-000100001111/users/userXX returns 201") - @ParameterizedTest(name = "Adding user {0} succeeds") - @CsvSource(value = {"0,user91", "1,user92", "2,user93"}) - public void addUserToVaultNotExceedingSeats(String run, String userId) { - Assumptions.assumeTrue(effectiveVaultAccessRepo.countSeatOccupyingUsers() == 2 + Integer.parseInt(run)); - - given().when().put("/vaults/{vaultId}/users/{usersId}", "7E57C0DE-0000-4000-8000-000100001111", userId) - .then().statusCode(201); - } - - @Test - @Order(3) - @DisplayName("PUT /vaults/7E57C0DE-0000-4000-8000-000100001111/users/user94 returns 402") - public void addUserToVaultExceedingSeats() { - Assumptions.assumeTrue(effectiveVaultAccessRepo.countSeatOccupyingUsers() == 5); - - given().when().put("/vaults/{vaultId}/users/{usersId}", "7E57C0DE-0000-4000-8000-000100001111", "user94") - .then().statusCode(402); - } - - @Test - @Order(4) - @DisplayName("PUT /vaults/7E57C0DE-0000-4000-8000-000100001111 (as user1) returns 200 with only updated name, description and archive flag, despite exceeding license") - public void testUpdateVaultDespiteLicenseExceeded() { - Assumptions.assumeTrue(effectiveVaultAccessRepo.countSeatOccupyingUsers() == 5); - var vaultId = "7E57C0DE-0000-4000-8000-000100001111"; - - var vaultDto = new VaultResource.VaultDto(UUID.fromString(vaultId), "Vault 1", "This is a testvault.", false, Instant.parse("2222-11-11T11:11:11Z"), "someVaule", -1, "doNotUpdate", "doNotUpdate", "doNotUpdate"); - given().contentType(ContentType.JSON) - .body(vaultDto) - .when().put("/vaults/{vaultId}", vaultId) - .then().statusCode(200) - .body("id", equalToIgnoringCase(vaultId)) - .body("name", equalTo("Vault 1")) - .body("description", equalTo("This is a testvault.")) - .body("archived", equalTo(false)); - } - - @Test - @Order(5) - @DisplayName("PUT /vaults/7E57C0DE-0000-4000-8000-0001FFFF3333 (as user1) exceeding the license returns 402") - public void testCreateVaultExceedingSeats() throws SQLException { - try (var c = dataSource.getConnection(); var s = c.createStatement()) { - s.execute(""" - INSERT INTO "vault_access" ("vault_id", "authority_id") - VALUES - ('7E57C0DE-0000-4000-8000-000100001111', 'group91'); - """); - } - - Assumptions.assumeTrue(effectiveVaultAccessRepo.countSeatOccupyingUsers() > 5); - - var uuid = UUID.fromString("7E57C0DE-0000-4000-8000-0001FFFF3333"); - var vaultDto = new VaultResource.VaultDto(uuid, "My Vault", "Test vault 4", false, Instant.parse("2112-12-21T21:12:21Z"), "masterkey3", 42, "NaCl", "authPubKey3", "authPrvKey3"); - given().contentType(ContentType.JSON).body(vaultDto) - .when().put("/vaults/{vaultId}", "7E57C0DE-0000-4000-8000-0001FFFF3333") - .then().statusCode(402); - } - - @Test - @Order(7) - @DisplayName("unlock/legacyUnlock is granted, if (effective vault user) > license seats but (effective vault user with access token) <= license seat") - public void testUnlockAllowedExceedingLicenseSoftLimit() { - Assumptions.assumeTrue(effectiveVaultAccessRepo.countSeatOccupyingUsersWithAccessToken() <= 5); - - when().get("/vaults/{vaultId}/access-token", "7E57C0DE-0000-4000-8000-000100001111") - .then().statusCode(200); - when().get("/vaults/{vaultId}/keys/{deviceId}", "7E57C0DE-0000-4000-8000-000100002222", "legacyDevice3") - .then().statusCode(200) - .body(is("legacy.jwe.jwe.vault2.device3")); - } - - @Test - @Order(8) - @DisplayName("Unlock/legacyUnlock is blocked if (effective vault users with toke) > license seats") - public void testUnockBlockedExceedingLicenseHardLimit() throws SQLException { - try (var c = dataSource.getConnection(); var s = c.createStatement()) { - s.execute(""" - INSERT INTO "access_token" ("user_id", "vault_id", "vault_masterkey") - VALUES ('user91', '7E57C0DE-0000-4000-8000-000100001111', 'jwe.jwe.jwe.vault1.user91'); - INSERT INTO "access_token" ("user_id", "vault_id", "vault_masterkey") - VALUES ('user92', '7E57C0DE-0000-4000-8000-000100001111', 'jwe.jwe.jwe.vault1.user92'); - INSERT INTO "access_token" ("user_id", "vault_id", "vault_masterkey") - VALUES ('user93', '7E57C0DE-0000-4000-8000-000100001111', 'jwe.jwe.jwe.vault1.user93'); - INSERT INTO "access_token" ("user_id", "vault_id", "vault_masterkey") - VALUES ('user94', '7E57C0DE-0000-4000-8000-000100001111', 'jwe.jwe.jwe.vault1.user94'); - """); - } - Assumptions.assumeTrue(effectiveVaultAccessRepo.countSeatOccupyingUsersWithAccessToken() > 5); - - when().get("/vaults/{vaultId}/access-token", "7E57C0DE-0000-4000-8000-000100001111") - .then().statusCode(402); - when().get("/vaults/{vaultId}/keys/{deviceId}", "7E57C0DE-0000-4000-8000-000100002222", "legacyDevice3") - .then().statusCode(402); - } - - @AfterAll - public void reset() throws SQLException { - try (var c = dataSource.getConnection(); var s = c.createStatement()) { - s.execute(""" - DELETE FROM "authority" - WHERE "id" IN ('user91', 'user92', 'user93', 'user94', 'user95_A', 'group91'); - """); - } - } - - } - @Nested @DisplayName("Claim Ownership") @TestSecurity(user = "User Name 1", roles = {"user"}) diff --git a/backend/src/test/java/org/cryptomator/hub/entities/BatchTest.java b/backend/src/test/java/org/cryptomator/hub/entities/BatchTest.java new file mode 100644 index 000000000..5e4443fd3 --- /dev/null +++ b/backend/src/test/java/org/cryptomator/hub/entities/BatchTest.java @@ -0,0 +1,64 @@ +package org.cryptomator.hub.entities; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; +import org.mockito.Mockito; + +import java.util.ArrayList; +import java.util.List; +import java.util.function.BiFunction; +import java.util.function.Consumer; + +class BatchTest { + + @ParameterizedTest + @ValueSource(ints = {-1, 0}) + public void testInvalidBatchSize(int batchSize) { + Assertions.assertThrows(IllegalArgumentException.class, () -> { + Batch.of(batchSize); + }); + } + + @ParameterizedTest + @ValueSource(ints = {1, 2, 3, 5, 10}) + public void testBatchSizes(int batchSize) { + List result = new ArrayList<>(); + Batch.of(batchSize).run(List.of(1, 2, 3, 4, 5, 6, 7), result::addAll); + + Assertions.assertEquals(List.of(1, 2, 3, 4, 5, 6, 7), result); + } + + @Test + public void testNoopIfInputIsEmpty() { + Consumer> job = Mockito.mock(); + + Batch.of(10).run(List.of(), job); + + Mockito.verifyNoInteractions(job); + } + + @ParameterizedTest + @ValueSource(ints = {1, 2, 3, 5, 10}) + public void testBatchSizesReduce(int batchSize) { + int result = Batch.of(batchSize).run(List.of(1, 2, 3, 4, 5, 6, 7), 0, (sublist, r) -> { + for (int i : sublist) { + r += i; + } + return r; + }); + + Assertions.assertEquals(1+2+3+4+5+6+7, result); + } + + @Test + public void testNoopIfReduceInputIsEmpty() { + BiFunction, Integer, Integer> job = Mockito.mock(); + + Batch.of(10).run(List.of(), 0, job); + + Mockito.verifyNoInteractions(job); + } + +} \ No newline at end of file diff --git a/backend/src/test/java/org/cryptomator/hub/entities/EffectiveGroupMembershipIT.java b/backend/src/test/java/org/cryptomator/hub/entities/EffectiveGroupMembershipIT.java new file mode 100644 index 000000000..3f236d64b --- /dev/null +++ b/backend/src/test/java/org/cryptomator/hub/entities/EffectiveGroupMembershipIT.java @@ -0,0 +1,196 @@ +package org.cryptomator.hub.entities; + +import io.quarkus.test.junit.QuarkusTest; +import jakarta.inject.Inject; +import jakarta.transaction.Transactional; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import java.util.List; + +@QuarkusTest +class EffectiveGroupMembershipIT { + + @Inject + User.Repository userRepo; + + @Inject + Group.Repository groupRepo; + + @Inject + Authority.Repository authorityRepo; + + @Inject + EffectiveGroupMembership.Repository effectiveGroupMembershipRepo; + + @BeforeEach + @Transactional + public void setup() { + for (int i = 1; i <= 5; i++) { + User u = new User(); + u.setId("u" + i); + u.setName("User " + i); + userRepo.persist(u); + } + + for (int i = 1; i <= 5; i++) { + Group g = new Group(); + g.setId("g" + i); + g.setName("Group " + i); + groupRepo.persist(g); + } + + /** + * + * g1/ + * ├─ u1/ + * ├─ g2/ + * ├─ u2 + * ├─ u4 + * ├─ g3/ + * ├─ u3 + * ├─ u4 + */ + addMembers("g1", "u1", "g2"); + addMembers("g2", "u2", "u4", "g3"); + addMembers("g3", "u3", "u4"); + effectiveGroupMembershipRepo.fullUpdate(); + } + + @AfterEach + @Transactional + public void teardown() { + userRepo.deleteByIds(List.of("u1", "u2", "u3", "u4", "u5")); + groupRepo.deleteByIds(List.of( "g1", "g2", "g3", "g4", "g5")); + } + + @Test + @DisplayName("validate data after full update") + @Transactional + public void testDataAfterFullUpdate() { + Assertions.assertTrue(effectiveGroupMembershipRepo.isMember("g1", "u1")); + Assertions.assertTrue(effectiveGroupMembershipRepo.isMember("g1", "u2")); + Assertions.assertTrue(effectiveGroupMembershipRepo.isMember("g1", "u3")); + Assertions.assertTrue(effectiveGroupMembershipRepo.isMember("g2", "u2")); + Assertions.assertTrue(effectiveGroupMembershipRepo.isMember("g2", "u3")); + Assertions.assertTrue(effectiveGroupMembershipRepo.isMember("g2", "u4")); + Assertions.assertTrue(effectiveGroupMembershipRepo.isMember("g3", "u3")); + Assertions.assertTrue(effectiveGroupMembershipRepo.isMember("g3", "u4")); + + Assertions.assertFalse(effectiveGroupMembershipRepo.isMember("g2", "u1")); + Assertions.assertFalse(effectiveGroupMembershipRepo.isMember("g3", "u1")); + Assertions.assertFalse(effectiveGroupMembershipRepo.isMember("g1", "u5")); + Assertions.assertFalse(effectiveGroupMembershipRepo.isMember("g2", "u5")); + Assertions.assertFalse(effectiveGroupMembershipRepo.isMember("g3", "u5")); + + } + + /* update groups */ + + @Test + @DisplayName("updateGroups after adding g5/u5") + @Transactional + public void updateGroupAfterAddingG5U5() { + addMembers("g5", "u5"); + effectiveGroupMembershipRepo.updateGroups(List.of("g5")); + Assertions.assertTrue(effectiveGroupMembershipRepo.isMember("g5", "u5")); + } + + @Test + @DisplayName("updateGroups after removing g2/g3") + @Transactional + public void updateGroupAfterRemovingG2G3() { + removeMembers("g2", "g3"); + effectiveGroupMembershipRepo.updateGroups(List.of("g2")); + Assertions.assertTrue(effectiveGroupMembershipRepo.isMember("g2", "u2")); + Assertions.assertFalse(effectiveGroupMembershipRepo.isMember("g2", "g3")); + Assertions.assertFalse(effectiveGroupMembershipRepo.isMember("g2", "u3")); + Assertions.assertFalse(effectiveGroupMembershipRepo.isMember("g1", "g3")); + Assertions.assertFalse(effectiveGroupMembershipRepo.isMember("g1", "u3")); + } + + /* update users */ + + @Test + @DisplayName("updateUsers after adding g5/u5") + @Transactional + public void updateUsersAfterAddingG5U5() { + addMembers("g5", "u5"); + effectiveGroupMembershipRepo.updateUsers(List.of("u5")); + Assertions.assertTrue(effectiveGroupMembershipRepo.isMember("g5", "u5")); + } + + @Test + @DisplayName("updateUsers after adding g3/u5") + @Transactional + public void updateUsersAfterAddingG3U5() { + addMembers("g3", "u5"); + effectiveGroupMembershipRepo.updateUsers(List.of("u5")); + Assertions.assertTrue(effectiveGroupMembershipRepo.isMember("g3", "u5")); + Assertions.assertTrue(effectiveGroupMembershipRepo.isMember("g2", "u5")); + Assertions.assertTrue(effectiveGroupMembershipRepo.isMember("g1", "u5")); + } + + @Test + @DisplayName("updateUsers after removing g2/u2") + @Transactional + public void updateUsersAfterRemovingG2U2() { + removeMembers("g2", "u2"); + effectiveGroupMembershipRepo.updateUsers(List.of("u2")); + Assertions.assertFalse(effectiveGroupMembershipRepo.isMember("g2", "u2")); + Assertions.assertFalse(effectiveGroupMembershipRepo.isMember("g1", "u2")); + } + + @Test + @DisplayName("updateUsers after removing g2/u4") + @Transactional + public void updateUsersAfterRemovingG2U4() { + removeMembers("g2", "u4"); + effectiveGroupMembershipRepo.updateUsers(List.of("u4")); + Assertions.assertTrue(effectiveGroupMembershipRepo.isMember("g3", "u4")); + Assertions.assertTrue(effectiveGroupMembershipRepo.isMember("g2", "u4")); + Assertions.assertTrue(effectiveGroupMembershipRepo.isMember("g1", "u4")); + } + + /* helpers */ + + private void addMembers(String groupId, String... memberIds) { + if (memberIds.length == 0) { + throw new IllegalArgumentException("At least one memberId must be provided"); + } + Group group = groupRepo.findById(groupId); + if (group == null) { + throw new IllegalArgumentException("Group " + groupId + " does not exist"); + } + for (String memberId : memberIds) { + Authority member = authorityRepo.findById(memberId); + if (member == null) { + throw new IllegalArgumentException("Member " + memberId + " does not exist"); + } + group.getMembers().add(member); + } + groupRepo.persist(group); + } + + private void removeMembers(String groupId, String... memberIds) { + if (memberIds.length == 0) { + throw new IllegalArgumentException("At least one memberId must be provided"); + } + Group group = groupRepo.findById(groupId); + if (group == null) { + throw new IllegalArgumentException("Group " + groupId + " does not exist"); + } + for (String memberId : memberIds) { + Authority member = authorityRepo.findById(memberId); + if (member == null) { + throw new IllegalArgumentException("Member " + memberId + " does not exist"); + } + group.getMembers().remove(member); + } + groupRepo.persist(group); + } + +} \ No newline at end of file diff --git a/backend/src/test/java/org/cryptomator/hub/filters/VaultRoleFilterTest.java b/backend/src/test/java/org/cryptomator/hub/filters/VaultRoleFilterTest.java index 34049a816..db8fcd178 100644 --- a/backend/src/test/java/org/cryptomator/hub/filters/VaultRoleFilterTest.java +++ b/backend/src/test/java/org/cryptomator/hub/filters/VaultRoleFilterTest.java @@ -47,12 +47,12 @@ public void setup() { } @Test - @DisplayName("error 403 if annotated resource has no vaultId path param") + @DisplayName("error 404 if annotated resource has no vaultId path param") public void testFilterWithMissingVaultId() throws NoSuchMethodException { Mockito.doReturn(VaultRoleFilterTest.class.getMethod("allowMember")).when(resourceInfo).getResourceMethod(); Mockito.doReturn(new MultivaluedHashMap<>()).when(uriInfo).getPathParameters(); - Assertions.assertThrows(ForbiddenException.class, () -> filter.filter(context)); + Assertions.assertThrows(NotFoundException.class, () -> filter.filter(context)); } @Test diff --git a/backend/src/test/java/org/cryptomator/hub/keycloak/KeycloakAuthorityPullerTest.java b/backend/src/test/java/org/cryptomator/hub/keycloak/KeycloakAuthorityPullerTest.java index 752536e83..9cc7ccf4f 100644 --- a/backend/src/test/java/org/cryptomator/hub/keycloak/KeycloakAuthorityPullerTest.java +++ b/backend/src/test/java/org/cryptomator/hub/keycloak/KeycloakAuthorityPullerTest.java @@ -1,6 +1,7 @@ package org.cryptomator.hub.keycloak; import org.cryptomator.hub.entities.Authority; +import org.cryptomator.hub.entities.EffectiveGroupMembership; import org.cryptomator.hub.entities.Group; import org.cryptomator.hub.entities.User; import org.hamcrest.MatcherAssert; @@ -15,22 +16,28 @@ import org.junit.jupiter.params.provider.CsvSource; import org.mockito.Mockito; +import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.function.Function; import java.util.stream.Collectors; +import java.util.stream.Stream; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.argThat; class KeycloakAuthorityPullerTest { private final KeycloakAuthorityProvider remoteUserProvider = Mockito.mock(KeycloakAuthorityProvider.class); private final User.Repository userRepo = Mockito.mock(User.Repository.class); private final Group.Repository groupRepo = Mockito.mock(Group.Repository.class); + private final EffectiveGroupMembership.Repository effectiveGroupMembershipRepo = Mockito.mock(EffectiveGroupMembership.Repository.class); + + private final List persistedUsers = new ArrayList<>(); + private final List persistedGroups = new ArrayList<>(); private KeycloakAuthorityPuller remoteUserPuller; @@ -40,8 +47,19 @@ void setUp() { remoteUserPuller.remoteUserProvider = remoteUserProvider; remoteUserPuller.userRepo = userRepo; remoteUserPuller.groupRepo = groupRepo; - Mockito.doNothing().when(userRepo).persist((User) Mockito.any()); - Mockito.doNothing().when(groupRepo).persist((Group) Mockito.any()); + remoteUserPuller.effectiveGroupMembershipRepo = effectiveGroupMembershipRepo; + persistedUsers.clear(); + Mockito.doAnswer(invocation -> { + Stream stream = invocation.getArgument(0); + persistedUsers.addAll(stream.toList()); + return null; + }).when(userRepo).persist(Mockito.>any()); + persistedGroups.clear(); + Mockito.doAnswer(invocation -> { + Stream stream = invocation.getArgument(0); + persistedGroups.addAll(stream.toList()); + return null; + }).when(groupRepo).persist(Mockito.>any()); } @Nested @@ -62,9 +80,9 @@ public void testAddUsers(@ConvertWith(StringArrayConverter.class) String[] keycl Map keycloakUsers = Mockito.mock(Map.class); Map databaseUsers = Mockito.mock(Map.class); - var keycloakUserIds = Arrays.stream(keycloakUserIdString).collect(Collectors.toSet()); - var databaseUserIds = Arrays.stream(databaseUserIdString).collect(Collectors.toSet()); - var addedUserIds = Arrays.stream(addedUserIdString).collect(Collectors.toSet()); + var keycloakUserIds = Set.of(keycloakUserIdString); + var databaseUserIds = Set.of(databaseUserIdString); + var addedUserIds = Set.of(addedUserIdString); Mockito.when(keycloakUsers.keySet()).thenReturn(keycloakUserIds); Mockito.when(databaseUsers.keySet()).thenReturn(databaseUserIds); @@ -76,12 +94,16 @@ public void testAddUsers(@ConvertWith(StringArrayConverter.class) String[] keycl remoteUserPuller.syncAddedUsers(keycloakUsers, databaseUsers); + Mockito.verify(userRepo).persist(Mockito.>any()); + Mockito.verify(effectiveGroupMembershipRepo).updateUsers(Mockito.argThat(addedUserIds::containsAll)); for (var userId : addedUserIds) { - Mockito.verify(userRepo).persist(argThat((User u) -> - u.getId().equals(userId) - && u.getName().equals("name " + userId) - && u.getEmail().equals("email " + userId) - && u.getPictureUrl().equals("pic " + userId) + MatcherAssert.assertThat(persistedUsers, Matchers.hasItem( + Matchers.allOf( + Matchers.hasProperty("id", Matchers.equalTo(userId)), + Matchers.hasProperty("name", Matchers.equalTo("name " + userId)), + Matchers.hasProperty("email", Matchers.equalTo("email " + userId)), + Matchers.hasProperty("pictureUrl", Matchers.equalTo("pic " + userId)) + ) )); } } @@ -112,12 +134,10 @@ public void testDeleteUsers(@ConvertWith(StringArrayConverter.class) String[] ke var result = remoteUserPuller.syncDeletedUsers(keycloakUsers, databaseUsers); - for (var id : deletedUserIdString) { - Mockito.verify(userRepo).delete(deletedMap.get(id)); - } - var expected = Arrays.stream(deletedUserIdString).collect(Collectors.toSet()); MatcherAssert.assertThat(result, Matchers.equalTo(expected)); + Mockito.verify(userRepo).deleteByIds(expected); + Mockito.verify(effectiveGroupMembershipRepo).updateUsers(Mockito.argThat(expected::containsAll)); } } @@ -189,7 +209,7 @@ public void testAddGroups(@ConvertWith(StringArrayConverter.class) String[] keyc Map keycloakGroups = new HashMap<>(); for (var gid : keycloakGroupIdString) { - var dto = new KeycloakGroupDto(gid, "Name " + gid, "pic " + gid, Set.of(kcUsers.get(gid))); + var dto = new KeycloakGroupDto(gid, "name " + gid, "pic " + gid, Set.of(kcUsers.get(gid))); keycloakGroups.put(gid, dto); } @@ -209,17 +229,18 @@ public void testAddGroups(@ConvertWith(StringArrayConverter.class) String[] keyc remoteUserPuller.syncAddedGroups(keycloakGroups, databaseGroups, databaseUsers); - for (var newGid : addedGroupIdString) { - Mockito.verify(groupRepo).persist(argThat((Group created) -> { - if (!created.getId().equals(newGid)) { - return false; - } - if (!created.getPictureUrl().equals("pic " + newGid)) { - return false; - } - var members = created.getMembers(); - return members.stream().anyMatch(m -> m.getId().equals(newGid)); - })); + var addedGroupIds = Set.of(addedGroupIdString); + Mockito.verify(groupRepo).persist(Mockito.>any()); + Mockito.verify(effectiveGroupMembershipRepo).updateGroups(Mockito.argThat(addedGroupIds::containsAll)); + for (var groupId : addedGroupIds) { + MatcherAssert.assertThat(persistedGroups, Matchers.hasItem( + Matchers.allOf( + Matchers.hasProperty("id", Matchers.equalTo(groupId)), + Matchers.hasProperty("pictureUrl", Matchers.equalTo("pic " + groupId)), + Matchers.hasProperty("name", Matchers.equalTo("name " + groupId)), + Matchers.hasProperty("members", Matchers.contains(Matchers.hasProperty("id", Matchers.equalTo(groupId)))) + ) + )); } } @@ -248,11 +269,10 @@ public void testDeleteGroups(@ConvertWith(StringArrayConverter.class) String[] k var result = remoteUserPuller.syncDeletedGroups(keycloakGroups, databaseGroups); - for (var id : deletedGroupIdString) { - Mockito.verify(groupRepo).delete(deletedMap.get(id)); - } var expected = Arrays.stream(deletedGroupIdString).collect(Collectors.toSet()); MatcherAssert.assertThat(result, Matchers.equalTo(expected)); + Mockito.verify(groupRepo).deleteByIds(expected); + Mockito.verify(effectiveGroupMembershipRepo).updateGroups(Mockito.argThat(expected::containsAll)); } } diff --git a/backend/src/test/resources/org/cryptomator/hub/flyway/V9999__Test_Data.sql b/backend/src/test/resources/org/cryptomator/hub/flyway/V9999__Test_Data.sql index ee02b474b..577a2825e 100644 --- a/backend/src/test/resources/org/cryptomator/hub/flyway/V9999__Test_Data.sql +++ b/backend/src/test/resources/org/cryptomator/hub/flyway/V9999__Test_Data.sql @@ -29,6 +29,11 @@ VALUES ('group1', 'user1'), ('group2', 'user2'); +INSERT INTO "effective_group_membership" ("group_id", "intermediate_group_ids", "member_id") +VALUES + ('group1', ARRAY['group1'], 'user1'), + ('group2', ARRAY['group2'], 'user2'); + INSERT INTO "vault" ("id", "name", "description", "creation_time", "salt", "iterations", "masterkey", "auth_pubkey", "auth_prvkey", "archived") VALUES ('7E57C0DE-0000-4000-8000-000100001111', 'Vault 1', 'This is a testvault.', '2020-02-20 20:20:20', 'salt1', 42, 'masterkey1',