From 8e82fd58a4268c5c0eb15d85d0d68e57484ced9d Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Fri, 11 Oct 2024 07:32:43 +0200 Subject: [PATCH 01/10] require `create-vauilts` role for: * `PUT /api/vaults/{vaultId}` * `POST /api/vaults/{vaultId}/claim-ownership` --- .../cryptomator/hub/api/VaultResource.java | 4 +-- backend/src/main/resources/dev-realm.json | 12 +++++-- .../cryptomator/hub/api/VaultResourceIT.java | 36 +++++++++++++++++-- 3 files changed, 44 insertions(+), 8 deletions(-) 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 ffd76610..e760f723 100644 --- a/backend/src/main/java/org/cryptomator/hub/api/VaultResource.java +++ b/backend/src/main/java/org/cryptomator/hub/api/VaultResource.java @@ -388,7 +388,7 @@ public VaultDto get(@PathParam("vaultId") UUID vaultId) { @PUT @Path("/{vaultId}") - @RolesAllowed("user") + @RolesAllowed("create-vaults") @VaultRole(value = VaultAccess.Role.OWNER, onMissingVault = VaultRole.OnMissingVault.PASS) @Consumes(MediaType.APPLICATION_JSON) @Produces(MediaType.APPLICATION_JSON) @@ -439,7 +439,7 @@ public Response createOrUpdate(@PathParam("vaultId") UUID vaultId, @Valid @NotNu @POST @Path("/{vaultId}/claim-ownership") - @RolesAllowed("user") + @RolesAllowed("create-vaults") @Consumes(MediaType.APPLICATION_FORM_URLENCODED) @Transactional @Operation(summary = "claims ownership of a vault", diff --git a/backend/src/main/resources/dev-realm.json b/backend/src/main/resources/dev-realm.json index f8554550..8f76ff83 100644 --- a/backend/src/main/resources/dev-realm.json +++ b/backend/src/main/resources/dev-realm.json @@ -16,13 +16,19 @@ "description": "User", "composite": false }, + { + "name": "create-vaults", + "description": "Can create vaults", + "composite": false + }, { "name": "admin", "description": "Administrator", "composite": true, "composites": { "realm": [ - "user" + "user", + "create-vaults" ], "client": { "realm-management": [ @@ -73,7 +79,7 @@ "email": "alice@localhost", "enabled": true, "credentials": [{"type": "password", "value": "asd"}], - "realmRoles": ["user"] + "realmRoles": ["user", "create-vaults"] }, { "username": "bob", @@ -82,7 +88,7 @@ "email": "bob@localhost", "enabled": true, "credentials": [{"type": "password", "value": "asd"}], - "realmRoles": ["user"] + "realmRoles": ["user", "create-vaults"] }, { "username": "carol", 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 5d254fba..d2511338 100644 --- a/backend/src/test/java/org/cryptomator/hub/api/VaultResourceIT.java +++ b/backend/src/test/java/org/cryptomator/hub/api/VaultResourceIT.java @@ -245,13 +245,26 @@ public void testUnlock() { @Nested @DisplayName("As vault admin user1") - @TestSecurity(user = "User Name 1", roles = {"user"}) + @TestSecurity(user = "User Name 1", roles = {"create-vaults"}) @OidcSecurity(claims = { @Claim(key = "sub", value = "user1") }) @TestMethodOrder(MethodOrderer.OrderAnnotation.class) public class CreateVaults { + @Test + @Order(1) + @TestSecurity(user = "User Name 1", roles = {"user"}) + @DisplayName("PUT /vaults/7E57C0DE-0000-4000-8000-000100003333 returns 403 for missing role") + public void testCreteVaultWithMissingRole() { + var uuid = UUID.fromString("7E57C0DE-0000-4000-8000-000100003333"); + var vaultDto = new VaultResource.VaultDto(uuid, "My Vault", "Test vault 3", 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-000100003333") + .then().statusCode(403); + } + @Test @Order(1) @DisplayName("PUT /vaults/7E57C0DE-0000-4000-8000-000100003333 returns 201") @@ -647,7 +660,7 @@ public void getMembersOfVault1b() { @Nested @DisplayName("When exceeding 5 seats in license") - @TestSecurity(user = "User Name 1", roles = {"user"}) + @TestSecurity(user = "User Name 1", roles = {"user", "create-vaults"}) @OidcSecurity(claims = { @Claim(key = "sub", value = "user1") }) @@ -832,7 +845,7 @@ public void reset() throws SQLException { @Nested @DisplayName("Claim Ownership") - @TestSecurity(user = "User Name 1", roles = {"user"}) + @TestSecurity(user = "User Name 1", roles = {"create-vaults"}) @OidcSecurity(claims = { @Claim(key = "sub", value = "user1") }) @@ -994,6 +1007,23 @@ public void testClaimOwnershipNoSuchVault() { .then().statusCode(404); } + @Test + @Order(1) + @TestSecurity(user = "User Name 1", roles = {"user"}) + @DisplayName("POST /vaults/7E57C0DE-0000-4000-8000-000100009999/claim-ownership returns 403 for missing role") + public void testClaimOwnershipWithMissingRole() { + var proof = JWT.create() + .withNotBefore(Instant.now().minusSeconds(10)) + .withExpiresAt(Instant.now().plusSeconds(10)) + .withSubject("user1") + .withClaim("vaultId", "7E57C0DE-0000-4000-8000-000100009999".toLowerCase()) + .sign(JWT_ALG); + + given().param("proof", proof) + .when().post("/vaults/{vaultId}/claim-ownership", "7E57C0DE-0000-4000-8000-000100009999") + .then().statusCode(403); + } + @Test @Order(2) @DisplayName("POST /vaults/7E57C0DE-0000-4000-8000-000100009999/claim-ownership returns 200") From 2124f368f09cc6161f3ca0763157f252a5da8eee Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Fri, 11 Oct 2024 08:10:40 +0200 Subject: [PATCH 02/10] =?UTF-8?q?changed=20`isAdmin()`=20=E2=86=92=20`hasR?= =?UTF-8?q?ole('admin')`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- frontend/src/common/auth.ts | 8 ++------ frontend/src/components/NavigationBar.vue | 2 +- frontend/src/components/VaultDetails.vue | 2 +- frontend/src/components/VaultList.vue | 2 +- frontend/src/router/index.ts | 2 +- 5 files changed, 6 insertions(+), 10 deletions(-) diff --git a/frontend/src/common/auth.ts b/frontend/src/common/auth.ts index 1f8f9e14..f0b9d0b9 100644 --- a/frontend/src/common/auth.ts +++ b/frontend/src/common/auth.ts @@ -45,12 +45,8 @@ class Auth { return this.keycloak.token; } - public isAdmin(): boolean { - return this.keycloak.tokenParsed?.realm_access?.roles.includes('admin') ?? false; - } - - public isUser(): boolean { - return this.keycloak.tokenParsed?.realm_access?.roles.includes('user') ?? false; + public hasRole(role: string): boolean { + return this.keycloak.tokenParsed?.realm_access?.roles.includes(role) ?? false; } } diff --git a/frontend/src/components/NavigationBar.vue b/frontend/src/components/NavigationBar.vue index d0225299..0948fa67 100644 --- a/frontend/src/components/NavigationBar.vue +++ b/frontend/src/components/NavigationBar.vue @@ -107,7 +107,7 @@ const props = defineProps<{ }>(); onMounted(async () => { - if ((await auth).isAdmin()) { + if ((await auth).hasRole('admin')) { profileDropdown.value = [profileDropdownSections.infoSection, profileDropdownSections.adminSection, profileDropdownSections.hubSection]; } else { profileDropdown.value = [profileDropdownSections.infoSection, profileDropdownSections.hubSection]; diff --git a/frontend/src/components/VaultDetails.vue b/frontend/src/components/VaultDetails.vue index 2d944e1f..ac1c21fe 100644 --- a/frontend/src/components/VaultDetails.vue +++ b/frontend/src/components/VaultDetails.vue @@ -283,7 +283,7 @@ onMounted(fetchData); async function fetchData() { onFetchError.value = null; try { - isAdmin.value = (await auth).isAdmin(); + isAdmin.value = (await auth).hasRole('admin'); vault.value = await backend.vaults.get(props.vaultId); me.value = await userdata.me; license.value = await backend.license.getUserInfo(); diff --git a/frontend/src/components/VaultList.vue b/frontend/src/components/VaultList.vue index a57981d6..39d1a591 100644 --- a/frontend/src/components/VaultList.vue +++ b/frontend/src/components/VaultList.vue @@ -177,7 +177,7 @@ onMounted(fetchData); async function fetchData() { onFetchError.value = null; try { - isAdmin.value = (await auth).isAdmin(); + isAdmin.value = (await auth).hasRole('admin'); if (isAdmin.value) { filterOptions.value['allVaults'] = t('vaultList.filter.entry.allVaults'); diff --git a/frontend/src/router/index.ts b/frontend/src/router/index.ts index 8522fe71..7a1f5be4 100644 --- a/frontend/src/router/index.ts +++ b/frontend/src/router/index.ts @@ -72,7 +72,7 @@ const routes: RouteRecordRaw[] = [ path: 'admin', beforeEnter: async () => { const auth = await authPromise; - return auth.isAdmin(); //TODO: reroute to NotFound Screen/ AccessDeniedScreen? + return auth.hasRole('admin'); //TODO: reroute to NotFound Screen/ AccessDeniedScreen? }, children: [ { From 192f6f81f6b18da93b85ec9d93c9bfce135d14c9 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Fri, 11 Oct 2024 08:11:43 +0200 Subject: [PATCH 03/10] require `create-vaults` role for: * `/app/vaults/create` * `/app/vaults/recover` --- frontend/src/components/Forbidden.vue | 18 ++++++++++++++++ frontend/src/router/index.ts | 30 ++++++++++++++++++++------- 2 files changed, 41 insertions(+), 7 deletions(-) create mode 100644 frontend/src/components/Forbidden.vue diff --git a/frontend/src/components/Forbidden.vue b/frontend/src/components/Forbidden.vue new file mode 100644 index 00000000..ae3cd0f8 --- /dev/null +++ b/frontend/src/components/Forbidden.vue @@ -0,0 +1,18 @@ + diff --git a/frontend/src/router/index.ts b/frontend/src/router/index.ts index 7a1f5be4..7393afde 100644 --- a/frontend/src/router/index.ts +++ b/frontend/src/router/index.ts @@ -1,4 +1,4 @@ -import { createRouter, createWebHistory, RouteLocationRaw, RouteRecordRaw } from 'vue-router'; +import { createRouter, createWebHistory, NavigationGuardWithThis, RouteLocationRaw, RouteRecordRaw } from 'vue-router'; import authPromise from '../common/auth'; import backend from '../common/backend'; import { baseURL } from '../common/config'; @@ -14,6 +14,18 @@ import UnlockSuccess from '../components/UnlockSuccess.vue'; import UserProfile from '../components/UserProfile.vue'; import VaultDetails from '../components/VaultDetails.vue'; import VaultList from '../components/VaultList.vue'; +import Forbidden from '../components/Forbidden.vue'; + +function checkRole(role: string): NavigationGuardWithThis { + return async () => { + const auth = await authPromise; + if (auth.hasRole(role)) { + return true; + } else { + return { path: '/app/forbidden', replace: true }; + } + }; +} const routes: RouteRecordRaw[] = [ { @@ -52,12 +64,14 @@ const routes: RouteRecordRaw[] = [ { path: 'vaults/create', component: CreateVault, - props: () => ({ recover: false }) + props: () => ({ recover: false }), + beforeEnter: checkRole('create-vaults'), }, { path: 'vaults/recover', component: CreateVault, - props: () => ({ recover: true }) + props: () => ({ recover: true }), + beforeEnter: checkRole('create-vaults'), }, { path: 'vaults/:id', @@ -70,10 +84,7 @@ const routes: RouteRecordRaw[] = [ }, { path: 'admin', - beforeEnter: async () => { - const auth = await authPromise; - return auth.hasRole('admin'); //TODO: reroute to NotFound Screen/ AccessDeniedScreen? - }, + beforeEnter: checkRole('admin'), children: [ { path: '', @@ -114,6 +125,11 @@ const routes: RouteRecordRaw[] = [ component: NotFound, meta: { skipAuth: true, skipSetup: true } }, + { + path: '/app/forbidden', + component: Forbidden, + meta: { skipAuth: true, skipSetup: true } + }, ]; const router = createRouter({ From e6c2d7426ba98615eb08a802cb97048c8c27a820 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Fri, 11 Oct 2024 08:12:00 +0200 Subject: [PATCH 04/10] disable "add vault" button if role is missing --- frontend/src/components/VaultList.vue | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frontend/src/components/VaultList.vue b/frontend/src/components/VaultList.vue index 39d1a591..b316b0f6 100644 --- a/frontend/src/components/VaultList.vue +++ b/frontend/src/components/VaultList.vue @@ -40,7 +40,7 @@
- + {{ t('vaultList.addVault') }} @@ -148,6 +148,7 @@ const roleOfSelectedVault = computed(() => { }); const isAdmin = ref(); +const canCreateVaults = ref(); const licenseStatus = ref(); const isLicenseViolated = computed(() => { if (licenseStatus.value) { @@ -178,6 +179,7 @@ async function fetchData() { onFetchError.value = null; try { isAdmin.value = (await auth).hasRole('admin'); + canCreateVaults.value = (await auth).hasRole('create-vaults'); if (isAdmin.value) { filterOptions.value['allVaults'] = t('vaultList.filter.entry.allVaults'); From 8aee7355fd28bde2ca7d811a8d7ba73f64757e5e Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Fri, 11 Oct 2024 08:21:07 +0200 Subject: [PATCH 05/10] updated changelog [ci skip] --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b647e5b..06a8add5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - This CHANGELOG file - WoT: Users will now have an ECDH as well as ECDSA key (#282) - WoT: Users can now mutually verify their identity, hardening Hub against injection of malicious public keys (#281) +- Permission to create new vaults can now be controlled via the `create-vaults` role in Keycloak (#206) ### Changed From 460b3faf5934ccd6421b8869f871d764ffc772e6 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Fri, 11 Oct 2024 10:39:10 +0200 Subject: [PATCH 06/10] minor tweaks after code review --- frontend/src/components/VaultList.vue | 6 +++--- frontend/src/router/index.ts | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/frontend/src/components/VaultList.vue b/frontend/src/components/VaultList.vue index b316b0f6..21e5ec23 100644 --- a/frontend/src/components/VaultList.vue +++ b/frontend/src/components/VaultList.vue @@ -40,7 +40,7 @@
- + {{ t('vaultList.addVault') }} @@ -147,8 +147,8 @@ const roleOfSelectedVault = computed(() => { } }); -const isAdmin = ref(); -const canCreateVaults = ref(); +const isAdmin = ref(false); +const canCreateVaults = ref(false); const licenseStatus = ref(); const isLicenseViolated = computed(() => { if (licenseStatus.value) { diff --git a/frontend/src/router/index.ts b/frontend/src/router/index.ts index 7393afde..fefc4511 100644 --- a/frontend/src/router/index.ts +++ b/frontend/src/router/index.ts @@ -17,11 +17,12 @@ import VaultList from '../components/VaultList.vue'; import Forbidden from '../components/Forbidden.vue'; function checkRole(role: string): NavigationGuardWithThis { - return async () => { + return async (to, _) => { const auth = await authPromise; if (auth.hasRole(role)) { return true; } else { + console.warn(`Access denied: User requires role ${role} to access ${to.fullPath}`); return { path: '/app/forbidden', replace: true }; } }; From f9eda5cd68690378a36d2fd5680d53fef252583e Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Thu, 17 Oct 2024 13:48:47 +0200 Subject: [PATCH 07/10] conditional tooltip & label for missing permission --- frontend/src/components/VaultList.vue | 4 ++-- frontend/src/i18n/de-DE.json | 2 ++ frontend/src/i18n/en-US.json | 2 ++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/frontend/src/components/VaultList.vue b/frontend/src/components/VaultList.vue index 21e5ec23..36afe773 100644 --- a/frontend/src/components/VaultList.vue +++ b/frontend/src/components/VaultList.vue @@ -40,7 +40,7 @@
- + {{ t('vaultList.addVault') }} @@ -98,7 +98,7 @@

{{ t('vaultList.empty.title') }}

-

{{ t('vaultList.empty.description') }}

+

{{ t('vaultList.empty.description') }}

diff --git a/frontend/src/i18n/de-DE.json b/frontend/src/i18n/de-DE.json index bf4c366e..6b0115ba 100644 --- a/frontend/src/i18n/de-DE.json +++ b/frontend/src/i18n/de-DE.json @@ -244,6 +244,8 @@ "vaultList.empty.title": "Keine Tresore", "vaultList.empty.description": "Beginne mit der Erstellung eines neuen Tresors.", "vaultList.addVault": "Hinzufügen", + "vaultList.addVault.disabled.licenseViolation": "Lizenz überschritten.", + "vaultList.addVault.disabled.missingPermission": "Die fehlt die Berechtigung zur Erstellung von Tresoren.", "vaultList.addVault.create": "Neu erstellen", "vaultList.addVault.recover": "Wiederherstellen", "vaultList.filter": "Filter", diff --git a/frontend/src/i18n/en-US.json b/frontend/src/i18n/en-US.json index 9537ecd7..768294af 100644 --- a/frontend/src/i18n/en-US.json +++ b/frontend/src/i18n/en-US.json @@ -245,6 +245,8 @@ "vaultList.empty.title": "No vaults", "vaultList.empty.description": "Get started by creating a new vault.", "vaultList.addVault": "Add", + "vaultList.addVault.disabled.licenseViolation": "License limit exceeded.", + "vaultList.addVault.disabled.missingPermission": "You don't have permission to create a vault.", "vaultList.addVault.create": "Create New", "vaultList.addVault.recover": "Recover Existing", "vaultList.filter": "Filter", From f5ac75233412ab7ae836c909935d232411ea17f2 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Thu, 17 Oct 2024 15:45:41 +0200 Subject: [PATCH 08/10] add fallback realm role to `@VaultRole` annotation The fallback is (currently) only used in case of `onMissingVault=REQUIRE_REALM_ROLE`, when no vault role exists yet --- .../cryptomator/hub/filters/VaultRole.java | 10 +++++- .../hub/filters/VaultRoleFilter.java | 6 ++++ .../hub/filters/VaultRoleFilterTest.java | 34 +++++++++++++++++++ 3 files changed, 49 insertions(+), 1 deletion(-) 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 0dad890d..39e4098a 100644 --- a/backend/src/main/java/org/cryptomator/hub/filters/VaultRole.java +++ b/backend/src/main/java/org/cryptomator/hub/filters/VaultRole.java @@ -31,5 +31,13 @@ * @return How to treat the case when a vault does not exist. */ OnMissingVault onMissingVault() default OnMissingVault.FORBIDDEN; - enum OnMissingVault { FORBIDDEN, NOT_FOUND, PASS } + enum OnMissingVault { FORBIDDEN, NOT_FOUND, PASS, REQUIRE_REALM_ROLE } + + /** + * Which additional realm role is required to access the annotated resource. + * + * Only relevant if {@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 6cb24d8a..ca94d360 100644 --- a/backend/src/main/java/org/cryptomator/hub/filters/VaultRoleFilter.java +++ b/backend/src/main/java/org/cryptomator/hub/filters/VaultRoleFilter.java @@ -32,6 +32,7 @@ public class VaultRoleFilter implements ContainerRequestFilter { @Inject EffectiveVaultAccess.Repository effectiveVaultAccessRepo; + @Inject Vault.Repository vaultRepo; @@ -67,6 +68,11 @@ public void filter(ContainerRequestContext requestContext) throws NotFoundExcept case FORBIDDEN -> throw new ForbiddenException(forbiddenMsg); case NOT_FOUND -> throw new NotFoundException("Vault not found"); case PASS -> {} + case REQUIRE_REALM_ROLE -> { + if (!requestContext.getSecurityContext().isUserInRole(annotation.realmRole())) { + throw new ForbiddenException("Missing role " + annotation.realmRole()); + } + } } } } 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 b6a2368a..34049a81 100644 --- a/backend/src/test/java/org/cryptomator/hub/filters/VaultRoleFilterTest.java +++ b/backend/src/test/java/org/cryptomator/hub/filters/VaultRoleFilterTest.java @@ -6,6 +6,7 @@ import jakarta.ws.rs.container.ContainerRequestContext; import jakarta.ws.rs.container.ResourceInfo; import jakarta.ws.rs.core.MultivaluedHashMap; +import jakarta.ws.rs.core.SecurityContext; import jakarta.ws.rs.core.UriInfo; import org.cryptomator.hub.entities.EffectiveVaultAccess; import org.cryptomator.hub.entities.Vault; @@ -28,6 +29,7 @@ public class VaultRoleFilterTest { private final ResourceInfo resourceInfo = Mockito.mock(ResourceInfo.class); private final UriInfo uriInfo = Mockito.mock(UriInfo.class); private final ContainerRequestContext context = Mockito.mock(ContainerRequestContext.class); + private final SecurityContext securityContext = Mockito.mock(SecurityContext.class); private final JsonWebToken jwt = Mockito.mock(JsonWebToken.class); private final EffectiveVaultAccess.Repository effectiveVaultAccessRepo = Mockito.mock(EffectiveVaultAccess.Repository.class); private final Vault.Repository vaultRepo = Mockito.mock(Vault.Repository.class); @@ -41,6 +43,7 @@ public void setup() { filter.vaultRepo = vaultRepo; Mockito.doReturn(uriInfo).when(context).getUriInfo(); + Mockito.doReturn(securityContext).when(context).getSecurityContext(); } @Test @@ -173,6 +176,34 @@ public void testPass() throws NoSuchMethodException { Assertions.assertDoesNotThrow(() -> filter.filter(context)); } + @Nested + @DisplayName("if @VaultRole(onMissingVault = OnMissingVault.REQUIRE_REALM_ROLE)") + public class RequireRealmRole { + + @BeforeEach + public void setup() throws NoSuchMethodException { + Mockito.doReturn(NonExistingVault.class.getMethod("requireRealmRole")).when(resourceInfo).getResourceMethod(); + } + + @Test + @DisplayName("error 403 if user lacks realm role required by @VaultRole(realmRole = \"foobar\")") + public void testMissesRole() { + Mockito.doReturn(false).when(securityContext).isUserInRole("foobar"); + + Assertions.assertThrows(ForbiddenException.class, () -> filter.filter(context)); + } + + + @Test + @DisplayName("pass if user has realm role required by @VaultRole(realmRole = \"foobar\")") + public void testHasRole() { + Mockito.doReturn(true).when(securityContext).isUserInRole("foobar"); + + Assertions.assertDoesNotThrow(() -> filter.filter(context)); + } + + } + } /* @@ -194,6 +225,9 @@ public void notFound() {} @VaultRole(value = {VaultAccess.Role.OWNER}, onMissingVault = VaultRole.OnMissingVault.PASS) public void pass() {} + + @VaultRole(value = {VaultAccess.Role.OWNER}, onMissingVault = VaultRole.OnMissingVault.REQUIRE_REALM_ROLE, realmRole = "foobar") + public void requireRealmRole() {} } From 508c6515fc6457ae5e51b0f451655906dcc0ef14 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Thu, 17 Oct 2024 15:47:50 +0200 Subject: [PATCH 09/10] apply new `@VaultRole(realmRole=...)` property * allowing any user to modify a owned vault * allowing only users with `create-vaults` role to create vault * allowing any user to claim ownership (if they can prove worthy) --- .../main/java/org/cryptomator/hub/api/VaultResource.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 e760f723..bab3998f 100644 --- a/backend/src/main/java/org/cryptomator/hub/api/VaultResource.java +++ b/backend/src/main/java/org/cryptomator/hub/api/VaultResource.java @@ -388,8 +388,8 @@ public VaultDto get(@PathParam("vaultId") UUID vaultId) { @PUT @Path("/{vaultId}") - @RolesAllowed("create-vaults") - @VaultRole(value = VaultAccess.Role.OWNER, onMissingVault = VaultRole.OnMissingVault.PASS) + @RolesAllowed("user") // general authentication. VaultRole filter will check for specific access rights + @VaultRole(value = VaultAccess.Role.OWNER, onMissingVault = VaultRole.OnMissingVault.REQUIRE_REALM_ROLE, realmRole = "create-vaults") @Consumes(MediaType.APPLICATION_JSON) @Produces(MediaType.APPLICATION_JSON) @Transactional @@ -439,7 +439,7 @@ public Response createOrUpdate(@PathParam("vaultId") UUID vaultId, @Valid @NotNu @POST @Path("/{vaultId}/claim-ownership") - @RolesAllowed("create-vaults") + @RolesAllowed("user") @Consumes(MediaType.APPLICATION_FORM_URLENCODED) @Transactional @Operation(summary = "claims ownership of a vault", From 7ec3ad06fcf66853a6f392adf33f35c7a8a34be4 Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Thu, 17 Oct 2024 17:20:20 +0200 Subject: [PATCH 10/10] adjust tests --- .../cryptomator/hub/api/VaultResourceIT.java | 44 ++++++------------- 1 file changed, 13 insertions(+), 31 deletions(-) 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 d2511338..24f6d86c 100644 --- a/backend/src/test/java/org/cryptomator/hub/api/VaultResourceIT.java +++ b/backend/src/test/java/org/cryptomator/hub/api/VaultResourceIT.java @@ -241,22 +241,10 @@ public void testUnlock() { when().get("/vaults/{vaultId}/access-token", "7E57C0DE-0000-4000-8000-000100001111") .then().statusCode(449); } - } - - @Nested - @DisplayName("As vault admin user1") - @TestSecurity(user = "User Name 1", roles = {"create-vaults"}) - @OidcSecurity(claims = { - @Claim(key = "sub", value = "user1") - }) - @TestMethodOrder(MethodOrderer.OrderAnnotation.class) - public class CreateVaults { @Test - @Order(1) - @TestSecurity(user = "User Name 1", roles = {"user"}) @DisplayName("PUT /vaults/7E57C0DE-0000-4000-8000-000100003333 returns 403 for missing role") - public void testCreteVaultWithMissingRole() { + public void testCreateVaultWithMissingRole() { var uuid = UUID.fromString("7E57C0DE-0000-4000-8000-000100003333"); var vaultDto = new VaultResource.VaultDto(uuid, "My Vault", "Test vault 3", false, Instant.parse("2112-12-21T21:12:21Z"), "masterkey3", 42, "NaCl", "authPubKey3", "authPrvKey3"); @@ -265,6 +253,17 @@ public void testCreteVaultWithMissingRole() { .then().statusCode(403); } + } + + @Nested + @DisplayName("As vault admin user1") + @TestSecurity(user = "User Name 1", roles = {"user", "create-vaults"}) + @OidcSecurity(claims = { + @Claim(key = "sub", value = "user1") + }) + @TestMethodOrder(MethodOrderer.OrderAnnotation.class) + public class CreateVaults { + @Test @Order(1) @DisplayName("PUT /vaults/7E57C0DE-0000-4000-8000-000100003333 returns 201") @@ -845,7 +844,7 @@ public void reset() throws SQLException { @Nested @DisplayName("Claim Ownership") - @TestSecurity(user = "User Name 1", roles = {"create-vaults"}) + @TestSecurity(user = "User Name 1", roles = {"user"}) @OidcSecurity(claims = { @Claim(key = "sub", value = "user1") }) @@ -1007,23 +1006,6 @@ public void testClaimOwnershipNoSuchVault() { .then().statusCode(404); } - @Test - @Order(1) - @TestSecurity(user = "User Name 1", roles = {"user"}) - @DisplayName("POST /vaults/7E57C0DE-0000-4000-8000-000100009999/claim-ownership returns 403 for missing role") - public void testClaimOwnershipWithMissingRole() { - var proof = JWT.create() - .withNotBefore(Instant.now().minusSeconds(10)) - .withExpiresAt(Instant.now().plusSeconds(10)) - .withSubject("user1") - .withClaim("vaultId", "7E57C0DE-0000-4000-8000-000100009999".toLowerCase()) - .sign(JWT_ALG); - - given().param("proof", proof) - .when().post("/vaults/{vaultId}/claim-ownership", "7E57C0DE-0000-4000-8000-000100009999") - .then().statusCode(403); - } - @Test @Order(2) @DisplayName("POST /vaults/7E57C0DE-0000-4000-8000-000100009999/claim-ownership returns 200")