Skip to content

Commit

Permalink
Merge branch 'develop' into feature/uvf
Browse files Browse the repository at this point in the history
# Conflicts:
#	frontend/package-lock.json
#	frontend/package.json
#	frontend/src/common/jwe.ts
#	frontend/src/components/AdminSettings.vue
#	frontend/src/components/GrantPermissionDialog.vue
#	frontend/src/components/UserProfile.vue
#	frontend/test/common/crypto.spec.ts
#	frontend/test/common/jwe.spec.ts
  • Loading branch information
overheadhunter committed Nov 3, 2024
2 parents e77ca7a + 06c4d28 commit 3e30a00
Show file tree
Hide file tree
Showing 59 changed files with 3,072 additions and 3,702 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/keycloak.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ jobs:
username: ${{ github.actor }}
password: ${{ secrets.GITHUB_TOKEN }}
- name: Build and Push Container Image
uses: docker/build-push-action@v5
uses: docker/build-push-action@v6
with:
context: keycloak
platforms: linux/amd64,linux/arm64/v8
Expand Down
38 changes: 38 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# Changelog

All notable changes to this project will be documented in this file.

The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased](https://github.com/cryptomator/hub/compare/1.3.4...HEAD)

### Added

- 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.6
- Updated to Java 21 (#272)
- Updated to Quarkus 3.8.x LTS (#272)
- Bumped build time dependencies
- Migrated remaining commonjs modules in frontend build to ESM (#291)
- Memoize infrequently changing data, reducing XHR roundtrips
- Switched to JWK thumbprint format in user profile
- Switched to Repository Pattern (#273)

### Fixed

- Fixed incorrect ARIA roles improving accessibility
- Fixed incorrect `Content-Type` header for `/api/vaults/{vaultId}/access-token` (#284)

### Security

- CVE-2023-45133: Babel vulnerable to arbitrary code execution when compiling specifically crafted malicious code
- CVE-2024-4068: Uncontrolled resource consumption in braces
- CVE-2024-39338: Server-Side Request Forgery in axios

20 changes: 10 additions & 10 deletions backend/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,14 @@
<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.2</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>
<dependency-plugin.version>3.6.1</dependency-plugin.version>
<surefire-plugin.version>3.2.5</surefire-plugin.version>
<failsafe-plugin.version>3.2.5</failsafe-plugin.version>
<junit-tree-reporter.version>1.2.1</junit-tree-reporter.version>
<dependency-plugin.version>3.8.0</dependency-plugin.version>
<surefire-plugin.version>3.5.0</surefire-plugin.version>
<failsafe-plugin.version>3.5.0</failsafe-plugin.version>
<junit-tree-reporter.version>1.3.0</junit-tree-reporter.version>
</properties>

<dependencyManagement>
Expand Down Expand Up @@ -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>
Expand All @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}

Expand Down Expand Up @@ -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);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 6 additions & 5 deletions backend/src/main/java/org/cryptomator/hub/api/VaultResource.java
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@
import org.eclipse.microprofile.jwt.JsonWebToken;
import org.eclipse.microprofile.openapi.annotations.Operation;
import org.eclipse.microprofile.openapi.annotations.enums.ParameterIn;
import org.eclipse.microprofile.openapi.annotations.media.Content;
import org.eclipse.microprofile.openapi.annotations.media.Schema;
import org.eclipse.microprofile.openapi.annotations.parameters.Parameter;
import org.eclipse.microprofile.openapi.annotations.responses.APIResponse;

Expand Down Expand Up @@ -299,9 +301,8 @@ public Response legacyUnlock(@PathParam("vaultId") UUID vaultId, @PathParam("dev
@RolesAllowed("user")
@VaultRole({VaultAccess.Role.MEMBER, VaultAccess.Role.OWNER}) // may throw 403
@Transactional
@Produces(MediaType.TEXT_PLAIN)
@Operation(summary = "get the user-specific vault key", description = "retrieves a jwe containing the vault key, encrypted for the current user")
@APIResponse(responseCode = "200")
@APIResponse(responseCode = "200", content = {@Content(mediaType = MediaType.TEXT_PLAIN, schema = @Schema(implementation = String.class))})
@APIResponse(responseCode = "402", description = "license expired or number of effective vault users that have a token exceeds available license seats")
@APIResponse(responseCode = "403", description = "not a vault member")
@APIResponse(responseCode = "404", description = "unknown vault")
Expand Down Expand Up @@ -329,7 +330,7 @@ public Response unlock(@PathParam("vaultId") UUID vaultId, @QueryParam("evenIfAr
eventLogger.logVaultKeyRetrieved(jwt.getSubject(), vaultId, VaultKeyRetrievedEvent.Result.SUCCESS);
var subscriptionStateHeaderName = "Hub-Subscription-State";
var subscriptionStateHeaderValue = license.isSet() ? "ACTIVE" : "INACTIVE"; // license expiration is not checked here, because it is checked in the ActiveLicense filter
return Response.ok(access.getVaultKey()).header(subscriptionStateHeaderName, subscriptionStateHeaderValue).build();
return Response.ok(access.getVaultKey(), MediaType.TEXT_PLAIN_TYPE).header(subscriptionStateHeaderName, subscriptionStateHeaderValue).build();
} else if (vaultRepo.findById(vaultId) == null) {
throw new NotFoundException("No such vault.");
} else {
Expand Down Expand Up @@ -426,8 +427,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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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")
Expand Down
8 changes: 2 additions & 6 deletions backend/src/main/java/org/cryptomator/hub/entities/User.java
Original file line number Diff line number Diff line change
Expand Up @@ -129,16 +129,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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 "";
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ public class VaultRoleFilter implements ContainerRequestFilter {

@Inject
EffectiveVaultAccess.Repository effectiveVaultAccessRepo;

@Inject
Vault.Repository vaultRepo;

Expand Down Expand Up @@ -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());
}
}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion backend/src/main/resources/application.properties
Original file line number Diff line number Diff line change
Expand Up @@ -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.1
%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
Expand Down
12 changes: 9 additions & 3 deletions backend/src/main/resources/dev-realm.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": [
Expand Down Expand Up @@ -73,7 +79,7 @@
"email": "alice@localhost",
"enabled": true,
"credentials": [{"type": "password", "value": "asd"}],
"realmRoles": ["user"]
"realmRoles": ["user", "create-vaults"]
},
{
"username": "bob",
Expand All @@ -82,7 +88,7 @@
"email": "bob@localhost",
"enabled": true,
"credentials": [{"type": "password", "value": "asd"}],
"realmRoles": ["user"]
"realmRoles": ["user", "create-vaults"]
},
{
"username": "carol",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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
Expand Down Expand Up @@ -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));
}
}

Expand Down Expand Up @@ -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);
}
}
}
Expand Down
16 changes: 14 additions & 2 deletions backend/src/test/java/org/cryptomator/hub/api/VaultResourceIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -243,11 +243,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")
})
Expand Down Expand Up @@ -649,7 +661,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")
})
Expand Down
Loading

0 comments on commit 3e30a00

Please sign in to comment.