diff --git a/CHANGELOG.md b/CHANGELOG.md index 1824ff1d3..a0f3a2a5a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - User and group management (#376) - Emergency Access: Allow a council to restore access to a orphaned vault (#390) - Show pictures of the groups in the Vaults member list (#375) -- Disable users to exclude them from license seat count (#427) +- Allow admins to archive and unarchive any vault (#283, #430) +- Disable users to exclude them from license seat count (#427, #428) ### Changed 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 a851c53b9..6a4899c91 100644 --- a/backend/src/main/java/org/cryptomator/hub/api/VaultResource.java +++ b/backend/src/main/java/org/cryptomator/hub/api/VaultResource.java @@ -501,7 +501,7 @@ public Response grantAccess(@PathParam("vaultId") UUID vaultId, @NotEmpty Map + * Only applies when the vault exists. For non-existing vaults, use {@link #onMissingVault()} instead. * * @return whether the given realm role allows bypassing the vault role check. */ boolean bypassForRealmRole() default false; + /** + * Which realm role bypasses the vault role check when {@link #bypassForRealmRole()} is true. + *

+ * Separate from {@link #realmRole()}, which is only used for {@link OnMissingVault#REQUIRE_REALM_ROLE}. + * + * @return realm role that bypasses the vault role check. + */ + RealmRole bypassRealmRole() default RealmRole.ADMIN; + /** * Which additional realm role is required to access the annotated resource. *

- * Only relevant if {@link #bypassForRealmRole()} or {@link #onMissingVault()} is set to {@link OnMissingVault#REQUIRE_REALM_ROLE}. + * Only relevant if {@link #onMissingVault()} is set to {@link OnMissingVault#REQUIRE_REALM_ROLE}. * * @return realm role required to access the annotated resource. */ 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 02cfb95dc..4f36b6c8a 100644 --- a/backend/src/main/java/org/cryptomator/hub/filters/VaultRoleFilter.java +++ b/backend/src/main/java/org/cryptomator/hub/filters/VaultRoleFilter.java @@ -48,11 +48,6 @@ 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().kcName())) { - // 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 { @@ -67,22 +62,25 @@ public void filter(ContainerRequestContext requestContext) throws NotFoundExcept } var vault = vaultRepo.findById(vaultId); - if (vault != null && annotation.bypassForEmergencyAccess() && isEmergencyAccessCouncilMember(userId, vault)) { - // user is a member of the emergency access council, so we skip the role check: - return; - } - - var forbiddenMsg = "Vault role required: " + Arrays.stream(annotation.value()).map(VaultAccess.Role::name).collect(Collectors.joining(", ")); if (vault != null) { + if (annotation.bypassForRealmRole() && requestContext.getSecurityContext().isUserInRole(annotation.bypassRealmRole().kcName())) { + // user has required realm role, so we skip the vault role check: + return; + } + if (annotation.bypassForEmergencyAccess() && isEmergencyAccessCouncilMember(userId, vault)) { + // user is a member of the emergency access council, so we skip the role check: + return; + } // check permissions for existing vault: var effectiveRoles = effectiveVaultAccessRepo.listRoles(vaultId, userId); - if (Arrays.stream(annotation.value()).noneMatch(effectiveRoles::contains)) { - throw new ForbiddenException(forbiddenMsg); + var requiredRoles = annotation.value(); + if (Arrays.stream(requiredRoles).noneMatch(effectiveRoles::contains)) { + throw new ForbiddenException("Vault role required: " + Arrays.stream(requiredRoles).map(VaultAccess.Role::name).collect(Collectors.joining(", "))); } } else { // how to treat non-existing vault: switch (annotation.onMissingVault()) { - case FORBIDDEN -> throw new ForbiddenException(forbiddenMsg); + case FORBIDDEN -> throw new ForbiddenException("Vault role required: " + Arrays.stream(annotation.value()).map(VaultAccess.Role::name).collect(Collectors.joining(", "))); case NOT_FOUND -> throw new NotFoundException("Vault not found"); case PASS -> { } 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 8295e5542..1f58e6ae2 100644 --- a/backend/src/test/java/org/cryptomator/hub/api/VaultResourceIT.java +++ b/backend/src/test/java/org/cryptomator/hub/api/VaultResourceIT.java @@ -962,6 +962,67 @@ void cleanup() throws SQLException { } + @Nested + @DisplayName("As admin user2 (non-owner)") + @TestSecurity(user = "User Name 2", roles = {"user", "admin"}) + @OidcSecurity(claims = { + @Claim(key = "sub", value = "user2") + }) + @TestMethodOrder(MethodOrderer.OrderAnnotation.class) + class AdminArchiveVault { + + @Test + @Order(1) + @DisplayName("PUT /vaults/7E57C0DE-0000-4000-8000-000100001111/archived returns 200 for admin archiving vault") + @DBRollbackAfter + void testAdminArchiveVault() { + given().contentType(ContentType.TEXT).body("true") + .when().put("/vaults/{vaultId}/archived", "7E57C0DE-0000-4000-8000-000100001111") + .then().statusCode(200) + .body("id", equalToIgnoringCase("7E57C0DE-0000-4000-8000-000100001111")) + .body("name", equalTo("Vault 1")) + .body("archived", equalTo(true)); + } + + @Test + @Order(2) + @DisplayName("PUT /vaults/7E57C0DE-0000-4000-8000-00010000AAAA/archived returns 200 for admin unarchiving vault") + @DBRollbackAfter + void testAdminUnarchiveVault() { + given().contentType(ContentType.TEXT).body("false") + .when().put("/vaults/{vaultId}/archived", "7E57C0DE-0000-4000-8000-00010000AAAA") + .then().statusCode(200) + .body("id", equalToIgnoringCase("7E57C0DE-0000-4000-8000-00010000AAAA")) + .body("name", equalTo("Vault Archived")) + .body("archived", equalTo(false)); + } + + @Test + @Order(3) + @DisplayName("PUT /vaults/7E57C0DE-0000-4000-8000-000100001111 returns 403 for admin updating vault they don't own") + void testAdminCannotUpdateVaultTheyDontOwn() { + var uuid = UUID.fromString("7E57C0DE-0000-4000-8000-000100001111"); + var vaultDto = new VaultResource.VaultDto(uuid, "Vault 1", Instant.parse("2020-02-20T20:20:20Z"), "This is a testvault.", true, 0, Map.of(), "masterkey1", 42, "salt1", "authPubKey1", "authPrvKey1"); + + given().contentType(ContentType.JSON).body(vaultDto) + .when().put("/vaults/{vaultId}", "7E57C0DE-0000-4000-8000-000100001111") + .then().statusCode(403); + } + + @Test + @Order(4) + @DisplayName("PUT /vaults/7E57C0DE-0000-4000-8000-000100005555 returns 403 for admin creating vault without create-vaults role") + void testAdminCannotCreateVaultWithoutCreateVaultsRole() { + var uuid = UUID.fromString("7E57C0DE-0000-4000-8000-000100005555"); + var vaultDto = new VaultResource.VaultDto(uuid, "New Vault", Instant.parse("2112-12-21T21:12:21Z"), "Should not be created", false, 0, Map.of(), "masterkey5", 42, "NaCl", "authPubKey5", "authPrvKey5"); + + given().contentType(ContentType.JSON).body(vaultDto) + .when().put("/vaults/{vaultId}", "7E57C0DE-0000-4000-8000-000100005555") + .then().statusCode(403); + } + + } + @Nested @DisplayName("As unauthenticated user") class AsAnonymous { @@ -976,7 +1037,8 @@ class AsAnonymous { "PUT, /vaults/7E57C0DE-0000-4000-8000-000100001111/users/user1", "DELETE, /vaults/7E57C0DE-0000-4000-8000-000100001111/authority/user1", "GET, /vaults/7E57C0DE-0000-4000-8000-000100001111/users-requiring-access-grant", - "GET, /vaults/7E57C0DE-0000-4000-8000-000100001111/access-token" + "GET, /vaults/7E57C0DE-0000-4000-8000-000100001111/access-token", + "PUT, /vaults/7E57C0DE-0000-4000-8000-000100001111/archived" }) void testGet(String method, String path) { when().request(method, path) 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 a5762115c..a748ab063 100644 --- a/backend/src/test/java/org/cryptomator/hub/filters/VaultRoleFilterTest.java +++ b/backend/src/test/java/org/cryptomator/hub/filters/VaultRoleFilterTest.java @@ -140,6 +140,35 @@ void testFilterSuccess4() throws NoSuchMethodException { Mockito.verify(effectiveVaultAccessRepo, Mockito.never()).listRoles(Mockito.any(), Mockito.any()); } + @Test + @DisplayName("pass if admin user tries to access 7E57C0DE-0000-4000-8000-000100001111 (admin bypasses vault role check)") + void testFilterBypassForAdmin() throws NoSuchMethodException { + Mockito.doReturn(VaultRoleFilterTest.class.getMethod("byPassForRealmRole")).when(resourceInfo).getResourceMethod(); + Mockito.doReturn(new MultivaluedHashMap<>(Map.of(VaultRole.DEFAULT_VAULT_ID_PARAM, "7E57C0DE-0000-4000-8000-000100001111"))).when(uriInfo).getPathParameters(); + Mockito.doReturn("user2").when(jwt).getSubject(); + Mockito.when(vaultRepo.findById(uuid("7E57C0DE-0000-4000-8000-000100001111"))).thenReturn(Mockito.mock(Vault.class)); + Mockito.doReturn(true).when(securityContext).isUserInRole("admin"); + + Assertions.assertDoesNotThrow(() -> filter.filter(context)); + + Mockito.verify(effectiveVaultAccessRepo, Mockito.never()).listRoles(Mockito.any(), Mockito.any()); + } + + @Test + @DisplayName("error 403 if non-admin user tries to access 7E57C0DE-0000-4000-8000-000100001111 (bypassForRealmRole has no effect)") + void testFilterNoBypassForNonAdmin() throws NoSuchMethodException { + Mockito.doReturn(VaultRoleFilterTest.class.getMethod("byPassForRealmRole")).when(resourceInfo).getResourceMethod(); + Mockito.doReturn(new MultivaluedHashMap<>(Map.of(VaultRole.DEFAULT_VAULT_ID_PARAM, "7E57C0DE-0000-4000-8000-000100001111"))).when(uriInfo).getPathParameters(); + Mockito.doReturn("user2").when(jwt).getSubject(); + Mockito.when(vaultRepo.findById(uuid("7E57C0DE-0000-4000-8000-000100001111"))).thenReturn(Mockito.mock(Vault.class)); + Mockito.when(effectiveVaultAccessRepo.listRoles(uuid("7E57C0DE-0000-4000-8000-000100001111"), Mockito.eq("user2"))).thenReturn(Set.of()); + Mockito.doReturn(false).when(securityContext).isUserInRole("admin"); + + var e = Assertions.assertThrows(ForbiddenException.class, () -> filter.filter(context)); + + Assertions.assertEquals("Vault role required: OWNER", e.getMessage()); + } + @Nested @DisplayName("when attempting to access archived vault") class OnArchivedVault { @@ -239,6 +268,35 @@ void testHasRole() { } + @Nested + @DisplayName("if @VaultRole(bypassForRealmRole = true, onMissingVault = REQUIRE_REALM_ROLE, realmRole = CREATE_VAULTS)") + class BypassRealmRoleWithRequireRealmRole { + + @BeforeEach + void setup() throws NoSuchMethodException { + Mockito.doReturn(NonExistingVault.class.getMethod("bypassRealmRoleWithRequireRealmRole")).when(resourceInfo).getResourceMethod(); + } + + @Test + @DisplayName("error 403 if admin lacks create-vaults role (bypass does not apply for non-existing vault)") + void testAdminWithoutCreateVaultsRole() { + Mockito.doReturn(true).when(securityContext).isUserInRole("admin"); + Mockito.doReturn(false).when(securityContext).isUserInRole("create-vaults"); + + Assertions.assertThrows(ForbiddenException.class, () -> filter.filter(context)); + } + + @Test + @DisplayName("pass if user has create-vaults role (onMissingVault checks realmRole, not bypassRealmRole)") + void testUserWithCreateVaultsRole() { + Mockito.doReturn(false).when(securityContext).isUserInRole("admin"); + Mockito.doReturn(true).when(securityContext).isUserInRole("create-vaults"); + + Assertions.assertDoesNotThrow(() -> filter.filter(context)); + } + + } + } /* @@ -249,6 +307,10 @@ void testHasRole() { public void byPassRecoveryCouncilMembers() { } + @VaultRole(value = {VaultAccess.Role.OWNER}, bypassForRealmRole = true) + public void byPassForRealmRole() { + } + @VaultRole({VaultAccess.Role.MEMBER}) public void allowMember() { } @@ -270,6 +332,10 @@ public void notFound() { public void pass() { } + @VaultRole(value = {VaultAccess.Role.OWNER}, bypassForRealmRole = true, onMissingVault = VaultRole.OnMissingVault.REQUIRE_REALM_ROLE, realmRole = RealmRole.CREATE_VAULTS) + public void bypassRealmRoleWithRequireRealmRole() { + } + @VaultRole(value = {VaultAccess.Role.OWNER}, onMissingVault = VaultRole.OnMissingVault.REQUIRE_REALM_ROLE, realmRole = RealmRole.ADMIN) public void requireRealmRole() { } diff --git a/frontend/src/common/backend.ts b/frontend/src/common/backend.ts index 4a923804e..97a5bdd7d 100644 --- a/frontend/src/common/backend.ts +++ b/frontend/src/common/backend.ts @@ -361,6 +361,15 @@ class VaultService { return addFallbackPictures ? users.map(fillInMissingPicture) : users; } + public async setArchived(vaultId: string, archived: boolean): Promise { + return axiosAuth.put(`/vaults/${vaultId}/archived`, String(archived), { headers: { 'Content-Type': 'text/plain' } }) + .then(response => { + response.data.creationTime = new Date(response.data.creationTime); + return response.data; + }) + .catch((error) => rethrowAndConvertIfExpected(error, 403, 404)); + } + public async createOrUpdateVault(vaultId: string, name: string, archived: boolean, requiredEmergencyKeyShares: number, emergencyKeyShares: Record, description?: string): Promise { const body: VaultDto = { id: vaultId, diff --git a/frontend/src/components/ArchiveVaultDialog.vue b/frontend/src/components/ArchiveVaultDialog.vue index 0392621d3..9b584f1de 100644 --- a/frontend/src/components/ArchiveVaultDialog.vue +++ b/frontend/src/components/ArchiveVaultDialog.vue @@ -81,7 +81,7 @@ async function archiveVault() { onArchiveVaultError.value = undefined; const v = props.vault; try { - const vaultDto = await backend.vaults.createOrUpdateVault(v.id, v.name, true, v.requiredEmergencyKeyShares, v.emergencyKeyShares, v.description); + const vaultDto = await backend.vaults.setArchived(v.id, true); emit('archived', vaultDto); open.value = false; } catch (error) { diff --git a/frontend/src/components/ReactivateVaultDialog.vue b/frontend/src/components/ReactivateVaultDialog.vue index 047aab3e0..38575321e 100644 --- a/frontend/src/components/ReactivateVaultDialog.vue +++ b/frontend/src/components/ReactivateVaultDialog.vue @@ -81,7 +81,7 @@ async function reactivateVault() { onReactivateVaultError.value = undefined; const v = props.vault; try { - const vaultDto = await backend.vaults.createOrUpdateVault(v.id, v.name, false, v.requiredEmergencyKeyShares, v.emergencyKeyShares, v.description); + const vaultDto = await backend.vaults.setArchived(v.id, false); emit('reactivated', vaultDto); open.value = false; } catch (error) { diff --git a/frontend/src/components/VaultDetails.vue b/frontend/src/components/VaultDetails.vue index 9baf93ae6..fd5cb878e 100644 --- a/frontend/src/components/VaultDetails.vue +++ b/frontend/src/components/VaultDetails.vue @@ -130,9 +130,13 @@ {{ t('vaultDetails.actions.editVaultMetadata') }} - + + @@ -147,6 +151,14 @@ + + + + @@ -160,7 +172,7 @@ {{ t('vaultDetails.actions.displayRecoveryKey') }} - @@ -205,7 +217,7 @@ {{ t('vaultDetails.emergencyAccess.fixCouncil') }} - @@ -253,6 +265,7 @@ const { t, d } = useI18n({ useScope: 'global' }); const props = defineProps<{ vaultId: string, vaultRole: VaultRole | 'NONE', + isAdmin: boolean, }>(); const emit = defineEmits<{ @@ -260,6 +273,8 @@ const emit = defineEmits<{ licenseStatusUpdated: [license: LicenseUserInfoDto] }>(); +const canToggleArchive = computed(() => props.vaultRole === 'OWNER' || props.isAdmin); + const onFetchError = ref(); const allowRetryFetch = computed(() => onFetchError.value && !(onFetchError.value instanceof NotFoundError)); //fetch requests either list something, or query from th vault. In the latter, a 404 indicates the vault does not exists anymore. diff --git a/frontend/src/components/VaultList.vue b/frontend/src/components/VaultList.vue index 5649a30da..6fcc2d00e 100644 --- a/frontend/src/components/VaultList.vue +++ b/frontend/src/components/VaultList.vue @@ -126,7 +126,7 @@ - +