Merge branch 'develop' into release/1.4.0

CHANGELOG.md

@@ -12,10 +12,11 @@ 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
 
-- Updated Keycloak to 25.0.4
+- Updated Keycloak to 25.0.6
 - Updated to Java 21 (#272)
 - Updated to Quarkus 3.8.x LTS (#272)
 - Bumped build time dependencies

backend/pom.xml

@@ -12,7 +12,7 @@
     <project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
     <quarkus.container-image.group>cryptomator</quarkus.container-image.group>
     <quarkus.container-image.name>hub</quarkus.container-image.name>
-    <quarkus.platform.version>3.8.6</quarkus.platform.version>
+    <quarkus.platform.version>3.15.1</quarkus.platform.version>
     <quarkus.jib.base-jvm-image>eclipse-temurin:21-jre</quarkus.jib.base-jvm-image> <!-- irrelevant for -Pnative -->
     <jwt.version>4.4.0</jwt.version>
     <compiler-plugin.version>3.13.0</compiler-plugin.version>
@@ -57,11 +57,11 @@
     </dependency>
     <dependency>
       <groupId>io.quarkus</groupId>
-      <artifactId>quarkus-resteasy-reactive</artifactId>
+      <artifactId>quarkus-rest</artifactId>
     </dependency>
     <dependency>
       <groupId>io.quarkus</groupId>
-      <artifactId>quarkus-resteasy-reactive-jackson</artifactId>
+      <artifactId>quarkus-rest-jackson</artifactId>
     </dependency>
     <dependency>
       <groupId>io.quarkus</groupId>
@@ -81,15 +81,15 @@
     </dependency>
     <dependency>
       <groupId>io.quarkus</groupId>
-      <artifactId>quarkus-rest-client-reactive-jackson</artifactId>
+      <artifactId>quarkus-rest-client-jackson</artifactId>
     </dependency>
     <dependency>
       <groupId>io.quarkus</groupId>
-      <artifactId>quarkus-rest-client-reactive</artifactId>
+      <artifactId>quarkus-rest-client</artifactId>
     </dependency>
     <dependency>
       <groupId>io.quarkus</groupId>
-      <artifactId>quarkus-keycloak-admin-client-reactive</artifactId>
+      <artifactId>quarkus-keycloak-admin-rest-client</artifactId>
     </dependency>
     <dependency>
       <groupId>io.quarkus</groupId>

backend/src/main/java/org/cryptomator/hub/RemoteUserPuller.java

@@ -54,7 +54,7 @@ void sync(Map<String, Group> keycloakGroups, Map<String, User> keycloakUsers, Ma
 	<T extends Authority> void syncAddedAuthorities(Map<String, T> keycloakAuthorities, Map<String, T> databaseAuthorities) {
 		var addedAuthority = diff(keycloakAuthorities.keySet(), databaseAuthorities.keySet());
 		for (var id : addedAuthority) {
-			authorityRepo.persist(keycloakAuthorities.get(id));
+			authorityRepo.persistAndFlush(keycloakAuthorities.get(id));
 		}
 	}
 
@@ -91,7 +91,7 @@ void syncUpdatedGroups(Map<String, Group> keycloakGroups, Map<String, Group> dat
 
 			dbGroup.getMembers().addAll(diff(kcGroup.getMembers(), dbGroup.getMembers()));
 			dbGroup.getMembers().removeAll(diff(dbGroup.getMembers(), kcGroup.getMembers()));
-			// TODO why don't we run dbGroup.persist()?
+			groupRepo.persist(dbGroup);
 		}
 	}
 

backend/src/main/java/org/cryptomator/hub/api/ConfigResource.java

@@ -49,7 +49,7 @@ public ConfigDto getConfig() {
 		var authUri = replacePrefix(oidcConfData.getAuthorizationUri(), trimTrailingSlash(internalRealmUrl), publicRealmUri);
 		var tokenUri = replacePrefix(oidcConfData.getTokenUri(), trimTrailingSlash(internalRealmUrl), publicRealmUri);
 
-		return new ConfigDto(keycloakPublicUrl, keycloakRealm, keycloakClientIdHub, keycloakClientIdCryptomator, authUri, tokenUri, Instant.now().truncatedTo(ChronoUnit.MILLIS), 3);
+		return new ConfigDto(keycloakPublicUrl, keycloakRealm, keycloakClientIdHub, keycloakClientIdCryptomator, authUri, tokenUri, Instant.now().truncatedTo(ChronoUnit.MILLIS), 4);
 	}
 
 	//visible for testing

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("user")
-	@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

backend/src/main/java/org/cryptomator/hub/entities/Group.java

@@ -2,6 +2,7 @@
 
 import io.quarkus.hibernate.orm.panache.PanacheRepositoryBase;
 import jakarta.enterprise.context.ApplicationScoped;
+import jakarta.persistence.CascadeType;
 import jakarta.persistence.DiscriminatorValue;
 import jakarta.persistence.Entity;
 import jakarta.persistence.JoinColumn;
@@ -17,7 +18,7 @@
 @DiscriminatorValue("GROUP")
 public class Group extends Authority {
 
-	@ManyToMany
+	@ManyToMany(cascade = {CascadeType.REMOVE})
 	@JoinTable(name = "group_membership",
 			joinColumns = @JoinColumn(name = "group_id", referencedColumnName = "id"),
 			inverseJoinColumns = @JoinColumn(name = "member_id", referencedColumnName = "id")

backend/src/main/java/org/cryptomator/hub/entities/User.java

@@ -138,16 +138,12 @@ public boolean equals(Object o) {
 		User that = (User) o;
 		return super.equals(that) //
 				&& Objects.equals(pictureUrl, that.pictureUrl) //
-				&& Objects.equals(email, that.email) //
-				&& Objects.equals(ecdhPublicKey, that.ecdhPublicKey) //
-				&& Objects.equals(ecdsaPublicKey, that.ecdsaPublicKey) //
-				&& Objects.equals(privateKeys, that.privateKeys) //
-				&& Objects.equals(setupCode, that.setupCode);
+				&& Objects.equals(email, that.email);
 	}
 
 	@Override
 	public int hashCode() {
-		return Objects.hash(super.getId(), pictureUrl, email, ecdhPublicKey, privateKeys, setupCode);
+		return Objects.hash(super.getId(), pictureUrl, email);
 	}
 
 	@ApplicationScoped

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 "";
 }

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());
+					}
+				}
 			}
 		}
 	}

backend/src/main/resources/application.properties

@@ -33,7 +33,7 @@ hub.keycloak.oidc.cryptomator-client-id=cryptomator
 %dev.quarkus.keycloak.devservices.start-command=start-dev
 %dev.quarkus.keycloak.devservices.port=8180
 %dev.quarkus.keycloak.devservices.service-name=quarkus-cryptomator-hub
-%dev.quarkus.keycloak.devservices.image-name=ghcr.io/cryptomator/keycloak:25.0.4
+%dev.quarkus.keycloak.devservices.image-name=ghcr.io/cryptomator/keycloak:25.0.6
 %dev.quarkus.oidc.devui.grant.type=code
 # OIDC will be mocked during unit tests. Use fake auth url to prevent dev services to start:
 %test.quarkus.oidc.auth-server-url=http://localhost:43210/dev/null

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",

backend/src/test/java/org/cryptomator/hub/RemoteUserPullerTest.java

@@ -28,6 +28,7 @@ class RemoteUserPullerTest {
 	private final User user = Mockito.mock(User.class);
 	private final Authority.Repository authorityRepo = Mockito.mock(Authority.Repository.class);
 	private final User.Repository userRepo = Mockito.mock(User.Repository.class);
+	private final Group.Repository groupRepo = Mockito.mock(Group.Repository.class);
 
 	private RemoteUserPuller remoteUserPuller;
 
@@ -37,8 +38,10 @@ void setUp() {
 		remoteUserPuller.remoteUserProvider = remoteUserProvider;
 		remoteUserPuller.authorityRepo = authorityRepo;
 		remoteUserPuller.userRepo = userRepo;
-		Mockito.doNothing().when(authorityRepo).persist((Authority) Mockito.any());
+		remoteUserPuller.groupRepo = groupRepo;
+		Mockito.doNothing().when(authorityRepo).persistAndFlush(Mockito.any());
 		Mockito.doNothing().when(userRepo).persist((User) Mockito.any());
+		Mockito.doNothing().when(groupRepo).persist((Group) Mockito.any());
 	}
 
 	@Nested
@@ -75,7 +78,7 @@ public void testAddAuthorities(@ConvertWith(StringArrayConverter.class) String[]
 			remoteUserPuller.syncAddedAuthorities(keycloakAuthorities, databaseAuthorities);
 
 			for (String authorityId : addedAuthorityIds) {
-				Mockito.verify(authorityRepo).persist(keycloakAuthorities.get(authorityId));
+				Mockito.verify(authorityRepo).persistAndFlush(keycloakAuthorities.get(authorityId));
 			}
 		}
 
@@ -202,6 +205,7 @@ public void testUpdateGroups(@ConvertWith(StringArrayConverter.class) String[] k
 				var dbGroup = databaseGroups.get(groupId);
 				Mockito.verify(dbGroup).setName(String.format("name %s", groupId));
 				MatcherAssert.assertThat(dbGroupMembers, Matchers.containsInAnyOrder(user, otherKCUser));
+				Mockito.verify(groupRepo).persist(dbGroup);
 			}
 		}
 	}

backend/src/test/java/org/cryptomator/hub/api/VaultResourceIT.java

@@ -241,11 +241,23 @@ public void testUnlock() {
 			when().get("/vaults/{vaultId}/access-token", "7E57C0DE-0000-4000-8000-000100001111")
 					.then().statusCode(449);
 		}
+
+		@Test
+		@DisplayName("PUT /vaults/7E57C0DE-0000-4000-8000-000100003333 returns 403 for missing role")
+		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");
+
+			given().contentType(ContentType.JSON).body(vaultDto)
+					.when().put("/vaults/{vaultId}", "7E57C0DE-0000-4000-8000-000100003333")
+					.then().statusCode(403);
+		}
+
 	}
 
 	@Nested
 	@DisplayName("As vault admin user1")
-	@TestSecurity(user = "User Name 1", roles = {"user"})
+	@TestSecurity(user = "User Name 1", roles = {"user", "create-vaults"})
 	@OidcSecurity(claims = {
 			@Claim(key = "sub", value = "user1")
 	})
@@ -647,7 +659,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")
 	})

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() {}
 	}