Emergency Access

[overheadhunter]

This pull request introduces comprehensive support for emergency access and recovery features in the backend API, including new endpoints, event logging, and configuration options. The changes span multiple files to add new resource classes, extend DTOs, update audit logging, and enhance settings management.

Emergency Access and Recovery Features

• Added the new EmergencyAccessResource REST API, which enables users to start, approve, complete, abort, and delete emergency recovery processes, as well as manage recovered key shares. This includes new DTOs for recovery processes and key shares.
• Enhanced the audit logging system to support emergency access events by adding new event types and corresponding DTOs, updating the event conversion logic, and extending the valid event types set. [1] [2] [3] [4]

Settings and Configuration

• Expanded the SettingsResource and SettingsDto to allow configuration of emergency access options, such as enabling/disabling emergency access, council member IDs, required key shares, minimum members, and council selection. Also added audit logging for emergency access settings changes.

User Account Management

• Updated the user account reset logic in UsersResource to remove emergency key shares and recovery process key shares for the user, ensuring proper cleanup during account reset.

General Improvements

• Marked AuthorityDto and its subclasses with @JsonInclude(JsonInclude.Include.NON_NULL) to avoid serializing null fields in API responses.

These changes collectively provide robust emergency access and recovery capabilities, improve auditability, and enhance configuration flexibility.

[coderabbitai]

Actionable comments posted: 6

🤖 Fix all issues with AI agents

In `@backend/src/main/java/org/cryptomator/hub/api/EmergencyAccessResource.java`:
- Around line 125-139: The complete(...) method currently logs an audit event
for any processId without verifying existence; update
EmergencyAccessResource.complete to first look up the recovery process (e.g.,
via EmergencyAccessRepository/EmergencyAccessService or the existing entity
manager) by processId, return Response.status(404).build() if not found, and
only call eventLogger.logEmergencyAccessRecoveryCompleted(processId,
currentUserId, ip) and return noContent() when the process is present; ensure
you reference the same JWT subject and request.remoteAddress logic already in
the method and keep the method transactional.
- Around line 186-194: The annotations on RecoveryProcessDto use `@NotBlank` for
non-CharSequence fields (id, vaultId, type) which causes
UnexpectedTypeException; replace `@NotBlank` with `@NotNull` on the UUID fields id
and vaultId and on the EmergencyRecoveryProcess.Type field type in the
RecoveryProcessDto record, keeping the other annotations (e.g., `@RawJson`, `@Min`,
`@ValidJWE`, `@NotEmpty`) unchanged so validation semantics remain correct.

In `@backend/src/main/java/org/cryptomator/hub/api/SettingsResource.java`:
- Around line 88-99: Add a nullability constraint to the DTO: annotate the
emergencyCouncilMemberIds component of the SettingsDto record with `@NotNull`
(import javax.validation.constraints.NotNull) so deserialization/validation
fails instead of passing null into Settings.setEmergencyCouncilMemberIds (which
calls addAll(null) and NPEs); update any validation setup if needed to ensure
bean validation runs on incoming SettingsDto instances.
- Around line 76-84: The audit string creation in SettingsResource incorrectly
serializes dto.emergencyCouncilMemberIds (producing [""] for empty sets) and can
NPE if the list is null; update the change-detection and logging block to guard
null by treating dto.emergencyCouncilMemberIds as an empty collection when null
and serialize it with a proper JSON serializer or explicitly produce "[]" for
empty (instead of using the current String.join trick), then pass that
well-formed JSON string into eventLogger.logEmergencyAccessSettingsUpdated;
additionally, add `@NotNull` to the emergencyCouncilMemberIds field on the
SettingsDto record to enforce non-null payloads at validation time (or keep the
null-guard if you add `@NotNull` as defense-in-depth).

In
`@backend/src/main/java/org/cryptomator/hub/entities/events/EmergencyAccessSettingsUpdatedEvent.java`:
- Around line 83-97: In EmergencyAccessSettingsUpdatedEvent, update the
equals(Object o) and hashCode() implementations to include the
enableEmergencyAccess and minMembers fields so events differing only by those
values are not treated as equal; specifically, in equals after comparing
councilMemberIds and other fields also compare
Objects.equals(enableEmergencyAccess, other.enableEmergencyAccess) (or direct
boolean compare) and compare minMembers, and in hashCode add
enableEmergencyAccess and minMembers to the Objects.hash(...) call alongside the
existing super.hashCode(), adminId, councilMemberIds, requiredKeyShares, and
allowChoosingCouncil.

In `@backend/src/main/java/org/cryptomator/hub/entities/Settings.java`:
- Line 149: The toString in class Settings incorrectly appends a stray
single-quote after the boolean enableEmergencyAccess, resulting in output like
enableEmergencyAccess=true'; remove the extra concatenation (+ '\'' ) for the
enableEmergencyAccess portion in the Settings.toString method so the boolean is
appended without string quotes (keep the single-quote wrappers only for actual
string fields like hubId/licenseKey).

[coderabbitai]

Actionable comments posted: 3

🤖 Fix all issues with AI agents

In `@backend/src/main/java/org/cryptomator/hub/api/EmergencyAccessResource.java`:
- Line 103: The `@APIResponse` annotation on the EmergencyAccessResource endpoint
currently has the wrong description ("process created"); update that annotation
on the method in class EmergencyAccessResource to a correct, specific
description such as "recovered key share added" (or equivalent that reflects
adding a recovered key share) so the API docs match the endpoint's behavior.
- Around line 147-160: The delete(`@PathParam`("processId") UUID processId) method
removes a recovery process without audit logging; mirror the abort flow by
calling the event logger before/after successful deletion so deletions are
auditable: after recoverProcessRepo.deleteById(processId) returns true, invoke
an EventLogger method (e.g., logEmergencyAccessRecoveryDeleted) with the
processId and current user/context (same principals used in abort) and then
return Response.noContent(); if such a logger method doesn’t exist, add
logEmergencyAccessRecoveryDeleted to EventLogger (or reuse an appropriate
existing event name) and ensure it records actor, processId and timestamp.
- Line 66: The controller methods are not triggering bean validation because the
request DTO parameters (RecoveryProcessDto and RecoveredKeyShareDto) are missing
the `@Valid` annotation; add the jakarta.validation.Valid import and annotate the
method parameters (e.g., the DTO parameter in startRecovery(UUID processId,
RecoveryProcessDto dto) and the analogous parameter at the other method around
line 106) with `@Valid` so RESTEasy/Quarkus will perform bean validation for
fields like `@NotNull`, `@Min`, `@ValidJWE/`@ValidJWS/@NotEmpty.

🧹 Nitpick comments (8)

backend/src/main/java/org/cryptomator/hub/entities/Vault.java (2)

320-327: deleteEmergencyKeySharesForUser — stream usage looks correct but has a subtle consideration.

findRecoverable(userId) returns vaults where the user is in emergencyKeyShares or in recoveredEmergencyKeyShares. For vaults where the user only appears in recoveredEmergencyKeyShares, emergencyKeyShares.remove(userId) is a no-op — the vault is still persisted (dirtied for no reason). This is harmless but will trigger unnecessary SQL UPDATEs for those vaults. If the recovery-process shares are cleaned up separately (e.g., in UsersResource), you could tighten the query to only match vaults where the user has an entry in emergency_key_shares to avoid unnecessary writes.

---

270-272: Objects.equals on primitive fields causes unnecessary autoboxing.

archived (boolean) and requiredEmergencyKeyShares (int) are primitives. Using Objects.equals() autoboxes them. Direct == is more idiomatic and avoids the allocation.

Proposed fix

-				&& Objects.equals(archived, vault.archived)
-				&& Objects.equals(requiredEmergencyKeyShares, vault.requiredEmergencyKeyShares)
+				&& archived == vault.archived
+				&& requiredEmergencyKeyShares == vault.requiredEmergencyKeyShares

backend/src/main/java/org/cryptomator/hub/api/SettingsResource.java (2)

83-84: Manual JSON serialization is fragile — council member IDs containing " will produce malformed output.

Line 83 wraps each ID in literal double quotes without escaping. If a council member ID ever contains ", \, or other JSON-special characters, the resulting councilMemberIds string stored in the audit event will be invalid JSON. Consider using Jackson's ObjectMapper (already on the classpath) for safe serialization:

Proposed fix using ObjectMapper

Inject or create an ObjectMapper and replace the manual construction:

-			var councilMemberIds = "[" + dto.emergencyCouncilMemberIds.stream().map(s -> "\"" + s + "\"").collect(Collectors.joining(", ")) + "]";
+			var councilMemberIds = new com.fasterxml.jackson.databind.ObjectMapper().writeValueAsString(dto.emergencyCouncilMemberIds);

You may want to inject the ObjectMapper as a field or handle the checked JsonProcessingException.

---

77-82: Set comparison can be simplified with Set.equals().

The two-way containsAll check is functionally equivalent to !oldEmergencyCouncilMemberIds.equals(dto.emergencyCouncilMemberIds) for Set types (which define equality as mutual containment of equal elements). This would reduce noise and improve readability.

Proposed simplification

-		if (!oldEmergencyCouncilMemberIds.containsAll(dto.emergencyCouncilMemberIds) 
-			|| !dto.emergencyCouncilMemberIds.containsAll(oldEmergencyCouncilMemberIds)
-			|| oldRequiredEmergencyKeyShares != dto.defaultRequiredEmergencyKeyShares 
+		if (!oldEmergencyCouncilMemberIds.equals(dto.emergencyCouncilMemberIds)
+			|| oldRequiredEmergencyKeyShares != dto.defaultRequiredEmergencyKeyShares 

backend/src/main/resources/org/cryptomator/hub/flyway/V24__Emergency_Access.sql (2)

8-25: Consider adding an index on council_member_id for CASCADE delete performance.

PostgreSQL does not automatically create indexes on FK columns. The ON DELETE CASCADE from user_details to emergency_key_shares (on council_member_id) and to default_emergency_council (on member_id) will require a sequential scan of these tables when a user is deleted. For small deployments this is negligible, but as the number of vaults and council members grows, these cascade deletes could hold locks and degrade performance.

Proposed indexes

CREATE INDEX "IDX_EMERGENCY_KEY_SHARES_MEMBER" ON "emergency_key_shares" ("council_member_id");

default_emergency_council has its PK on member_id so it's already indexed there.

---

41-52: Same index consideration for recovered_emergency_key_shares.council_member_id.

This FK also cascades on user deletion but council_member_id is the second column of the composite PK, so a leading-column index lookup won't cover it in PostgreSQL. An additional index would help cascade-delete performance.

Proposed index

CREATE INDEX "IDX_RECOVERED_EMERG_KEY_SHARES_MEMBER" ON "recovered_emergency_key_shares" ("council_member_id");

backend/src/main/java/org/cryptomator/hub/api/EmergencyAccessResource.java (2)

180-189: Ambiguous path semantics: /{vaultId} vs /{processId} on the same base path.

GET /{vaultId} interprets the path parameter as a vault ID, while PUT/DELETE /{processId} interpret the same path segment as a process ID. Although JAX-RS resolves routing by HTTP method (so no technical conflict), this is confusing for API consumers and makes it impossible to later add, e.g., a GET-by-processId endpoint. Consider distinguishing the paths:

-	`@Path`("/{vaultId}")
+	`@Path`("/vault/{vaultId}")

---

191-213: Nested validation won't cascade into map values.

Even after adding @Valid on the method parameter (per the earlier comment), validation won't cascade into the RecoveredKeyShareDto values inside the map unless the map's value type is annotated with @Valid:

-			`@JsonProperty`("recoveredKeyShares") `@NotEmpty` Map<String, RecoveredKeyShareDto> recoveredKeyShares) {
+			`@JsonProperty`("recoveredKeyShares") `@NotEmpty` Map<String, `@Valid` RecoveredKeyShareDto> recoveredKeyShares) {

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@backend/src/main/java/org/cryptomator/hub/api/EmergencyAccessResource.java`:
- Line 207: The method parameter annotated with `@ValidJWE` for processPublicKey
in EmergencyAccessResource is wrong because the frontend sends a JWK JSON
string, not a JWE; replace the `@ValidJWE` annotation on the processPublicKey
parameter with a validator that checks JWK JSON format (e.g., `@ValidJWK` or a
custom `@Pattern/`@Json-validating annotation) or remove the annotation entirely
so the controller accepts the JWK string and let downstream code (where
recoveryProcessPublicKey is parsed) perform validation; update any
import/validator references accordingly and run tests to ensure bean validation
no longer rejects the incoming JWK.

🧹 Nitpick comments (2)

backend/src/main/java/org/cryptomator/hub/api/EmergencyAccessResource.java (2)

68-77: dto.id is validated but never used — potential inconsistency with processId.

RecoveryProcessDto.id is annotated @NotNull and validated via @Valid, but line 76 uses the @PathParam("processId") value instead. A client could submit a body where dto.id != processId and it would silently succeed using the path value. Consider either:

1. Removing id from the DTO (since it comes from the path), or
2. Validating that dto.id.equals(processId) to prevent ambiguity.

Option 2: Validate consistency

 public Response startRecovery(`@PathParam`("processId") UUID processId, `@Valid` RecoveryProcessDto dto) {
+	if (!processId.equals(dto.id)) {
+		throw new BadRequestException("Path processId does not match body id");
+	}
 	var currentUser = jwt.getSubject();

---

142-153: complete only logs an audit event without changing process state.

The fetched process entity (line 145) is used only for the existence check — no status field is updated, no state transition occurs. Any user can call this endpoint repeatedly, generating duplicate "completed" audit events for the same process. If the design intent is purely audit logging, consider documenting that, or adding idempotency (e.g., a completedAt timestamp or status field to prevent repeated completion).

[coderabbitai]

Actionable comments posted: 8

🤖 Fix all issues with AI agents

In `@frontend/src/components/emergencyaccess/EmergencyAccessVaultList.vue`:
- Around line 267-275: fetchData is causing two identical settings fetches
because loadDefaultSettings() triggers backend.settings.get() fire-and-forget
and fetchData then calls backend.settings.get() again; fix by consolidating to a
single fetch (either await loadDefaultSettings() and return its result into
settings.value, or remove the loadDefaultSettings() call and rely on the
explicit settings.value = await backend.settings.get() in fetchData) and then
delete the now-unused loadDefaultSettings function (or remove its internal
backend.settings.get() call) if it is not referenced elsewhere; update
references so only one backend.settings.get() is invoked per refresh.
- Around line 329-336: In the isProcessAboutToComplete function remove the two
debug console.log calls that print 'recovered' and 'proc.requiredKeyShares' so
no debug output is left in production; locate the function by name
(isProcessAboutToComplete) and delete the console.log('recovered:' + recovered)
and console.log('proc.requiredKeyShares:' + proc.requiredKeyShares) lines,
leaving the computed recovered value and the final return expression intact.
- Line 68: The v-for list key currently uses vault.masterkey which is
semantically wrong and risky; update the list rendering in
EmergencyAccessVaultList.vue so the :key uses vault.id instead of
vault.masterkey (change the iteration line that references filteredVaults and
:key), and ensure any related references that assumed masterkey as unique remain
using vault.id (e.g., where filteredVaults, vault.id are used elsewhere in this
component like in methods or bindings).
- Around line 456-459: The function isEmergencyKeyShareHolder currently accesses
vault.emergencyKeyShares[me.value.id] directly which can throw if
emergencyKeyShares is null/undefined; update isEmergencyKeyShareHolder to guard
that property (e.g., use optional chaining or a null check) before indexing —
specifically change the access in isEmergencyKeyShareHolder to read
vault.emergencyKeyShares?.[me.value.id] !== undefined or equivalent null-safe
check so it matches other usages (lines referencing emergencyKeyShares
elsewhere) and avoids a TypeError when emergencyKeyShares is missing.
- Around line 2-8: The template's v-if checks for vaults == null but vaults is
initialized as ref<VaultDto[]>([]) so loading/error states are never shown;
either initialize vaults as null (change the ref<VaultDto[]>([]) to
ref<VaultDto[] | null>(null)) and update usages like filterVaults, any computed
or methods that assume vaults is non-null to handle null, or keep vaults as []
and change the template to use a dedicated isLoading ref (update fetchData to
set isLoading and onFetchError accordingly and replace the v-if guard with
isLoading) — update references to vaults, filterVaults, fetchData, and
onFetchError to remain null-safe after the change.
- Around line 181-187: The v-else-if condition block is wrong: inside the v-else
branch entitlements.emergencyAccessEnabled is already true so the
`!entitlements.emergencyAccessEnabled || !settings?.enableEmergencyAccess` check
contains dead code and also causes "none found" to show prematurely because
filteredVaults is initialized to [] before fetchData finishes. Replace that
condition to only check the live setting (e.g.,
`!settings?.enableEmergencyAccess`), and change the following
`v-else-if="filteredVaults && filteredVaults.length == 0"` to guard against
loading (e.g., check a loading flag or that fetchData has completed) so
filteredVaults being an empty array during fetch doesn't render the "none found"
message; update references to filteredVaults, fetchData, and any loading boolean
you introduce accordingly.

In `@frontend/src/i18n/en-US.json`:
- Around line 252-253: The title string "emergencyAccess.badge.broken.title"
uses the unclear abbreviation "EA"; update its value to spell out the term
(e.g., "Broken Emergency Access" or another concise full phrase) so end users
understand it, leaving "emergencyAccess.badge.broken.message" unchanged; ensure
the new title remains concise and consistent with other badge titles.
- Line 232: The translation string
emergencyAccess.validation.selectMoreCouncilMembers uses singular "council
member" but {0} can be >1; change the message to use the plural form (e.g.,
"Select at least {0} more council members.") or replace this key with a
locale-aware pluralized message using your i18n pluralization API (so the code
that calls emergencyAccess.validation.selectMoreCouncilMembers can pass the
count and the i18n layer will select "member" vs "members").

🧹 Nitpick comments (9)

frontend/src/i18n/en-US.json (2)

135-140: Inconsistent key naming: emergencyaccess vs. dot-separated/camelCase siblings.

Existing audit log detail keys use dot-separated segments (device.register, vault.create, vaultMember.add, vaultKey.retrieve), but the new keys use the lowercased compound emergencyaccess. Consider emergencyAccess.setup (camelCase) or emergency.access.setup (dot-separated) for consistency.

---

287-289: Hardcoded "2" in validation messages.

Lines 287-288 embed a literal 2 in the error strings. If the minimum requirement is ever configurable or changes, these strings won't reflect it. Consider using a placeholder like {0} and injecting the value at runtime.

Proposed fix

-  "grantEmergencyAccessDialog.error.keySharesRequired": "At least 2 Emergency Key Shares are required.",
-  "grantEmergencyAccessDialog.error.councilMembersRequired": "At least 2 Council Members are required.",
+  "grantEmergencyAccessDialog.error.keySharesRequired": "At least {0} Emergency Key Shares are required.",
+  "grantEmergencyAccessDialog.error.councilMembersRequired": "At least {0} Council Members are required.",

backend/src/main/java/org/cryptomator/hub/api/EmergencyAccessResource.java (2)

189-198: GET /{vaultId} shares the same path template as PUT /{processId} — confusing API surface.

Both endpoints resolve to @Path("/{uuid}") with only the HTTP method differentiating them. Since vault IDs and process IDs are both UUIDs, a client accidentally issuing GET /emergency-access/{processId} will silently receive an empty list instead of a meaningful error. This also produces a confusing OpenAPI spec where the same path has parameters with different semantic names.

Consider using a distinct path, e.g., @Path("/vaults/{vaultId}") or @Path("/by-vault/{vaultId}"), to make the intent unambiguous.

---

224-225: Long single-line record declaration reduces readability.

The RecoveredKeyShareDto record packs all four annotated fields onto one line (~220 chars). Consider breaking it across multiple lines for consistency with RecoveryProcessDto.

Suggested formatting

-	public record RecoveredKeyShareDto(`@JsonProperty`("processPrivateKey") `@ValidJWE` String processPrivateKey, `@JsonProperty`("unrecoveredKeyShare") `@ValidJWE` String unrecoveredKeyShare, `@JsonProperty`("recoveredKeyShare") `@ValidJWE` String recoveredKeyShare, `@JsonProperty`("signedProcessInfo") `@ValidJWS` String signedProcessInfo) {
+	public record RecoveredKeyShareDto(
+			`@JsonProperty`("processPrivateKey") `@ValidJWE` String processPrivateKey,
+			`@JsonProperty`("unrecoveredKeyShare") `@ValidJWE` String unrecoveredKeyShare,
+			`@JsonProperty`("recoveredKeyShare") `@ValidJWE` String recoveredKeyShare,
+			`@JsonProperty`("signedProcessInfo") `@ValidJWS` String signedProcessInfo) {

frontend/src/components/emergencyaccess/EmergencyAccessVaultList.vue (5)

281-284: Sequential API calls in a loop — consider parallelizing with Promise.all.

Each vault triggers a separate await in sequence. For N vaults this means N serial round-trips.

Proposed fix

-    for (const vault of vaults.value) {
-      const processes = await backend.emergencyAccess.findProcessesForVault(vault.id);
-      vaultRecoveryProcesses.value[vault.id] = processes;
-    }
+    const processResults = await Promise.all(
+      vaults.value.map(v => backend.emergencyAccess.findProcessesForVault(v.id))
+    );
+    for (let i = 0; i < vaults.value.length; i++) {
+      vaultRecoveryProcesses.value[vaults.value[i].id] = processResults[i];
+    }

---

386-396: Avoid as any casts — use proper type narrowing for AuthorityDto.

Lines 390–391 cast to any to access name and pictureUrl. If AuthorityDto is a discriminated union (user vs. group), use a type guard or check a discriminant property. If these fields are common to all authority types, update the AuthorityDto type definition instead.

-    if (a && (a as any).name) {
-      return { id: a.id, name: (a as any).name, pictureUrl: (a as any).pictureUrl };
+    if (a && 'name' in a && typeof a.name === 'string') {
+      return { id: a.id, name: a.name, pictureUrl: 'pictureUrl' in a ? (a.pictureUrl as string) : undefined };

---

489-494: TODO: activeProcessForVault always returns the first process regardless of type.

This is acknowledged in the comment. As the component supports multiple process types (CHANGE_PERMISSIONS, COUNCIL_CHANGE), callers like hasSubmittedEmergencyKeyShare (line 450) and the approvable filter (line 409) may behave incorrectly when multiple processes exist for a vault.

Would you like me to open an issue to track this, or would you like a suggested implementation that filters by type or returns all active processes?

---

207-214: Consolidate duplicate imports from ../../common/backend.

UserDto on line 214 can be merged into the existing import on line 207.

-import backend, { LicenseUserInfoDto, VaultDto, RecoveryProcessDto, AuthorityDto, SettingsDto } from '../../common/backend';
+import backend, { LicenseUserInfoDto, VaultDto, RecoveryProcessDto, AuthorityDto, SettingsDto, UserDto } from '../../common/backend';
 ...
-import { UserDto } from '../../common/backend';

---

461-464: Use strict equality (===) instead of loose equality (==).

Line 463 uses == to compare numbers. In TypeScript, prefer === for type-safe comparisons.

-  return vault.requiredEmergencyKeyShares == members;
+  return vault.requiredEmergencyKeyShares === members;

[coderabbitai]

Actionable comments posted: 3

🤖 Fix all issues with AI agents

In `@frontend/src/components/emergencyaccess/EmergencyAccessDialog.vue`:
- Line 103: Rename the misspelled template ref "concilMembersSelect" to
"councilMembersSelect" and update all usages accordingly: change the ref
attribute in the template (ref="concilMembersSelect") to
ref="councilMembersSelect" and replace any runtime references like
this.$refs.concilMembersSelect (or refs.concilMembersSelect) to
this.$refs.councilMembersSelect so all three occurrences are consistent.
- Around line 850-856: The membersWithRole construction can silently assign
MEMBER over OWNER when a user ID appears in both selectedNewOwners and
selectedNewmembers; update the logic in the CHANGE_PERMISSIONS branch so owners
take precedence: either deduplicate/validate that selectedNewOwners and
selectedNewmembers are disjoint (throw or log/assert if overlap) or build the
record with owners applied last (ensure selectedNewOwners wins) before assigning
to membersWithRole; reference the selectedNewOwners, selectedNewmembers and
membersWithRole identifiers in your fix.

In `@frontend/src/components/emergencyaccess/EmergencyAccessVaultList.vue`:
- Around line 372-382: The function getCurrentCouncilMembers uses unsafe "as
any" casts; replace them by treating authoritiesById.value[id] as an
AuthorityDto | undefined (no "as any"), use optional chaining or explicit type
guards to check for existence, then access a.name and a.pictureUrl directly
(e.g., const a = authoritiesById.value[id] as AuthorityDto | undefined; if (a &&
a.name) return { id: a.id, name: a.name, pictureUrl: a.pictureUrl }; else return
fallback), keeping the current fallback branch; reference
getCurrentCouncilMembers and authoritiesById and ensure AuthorityDto's
name/pictureUrl are used without casting.

🧹 Nitpick comments (8)

frontend/src/components/emergencyaccess/EmergencyAccessDialog.vue (5)

536-540: No-op filter masquerading as a type guard.

filter((a): a is AuthorityDto => true) always returns true — it doesn't actually filter anything. If the intent is just to narrow the type (the search API already returns AuthorityDto[]), the cast is unnecessary. If the intent is to filter out non-authorities, the predicate needs a real check.

Proposed simplification

 async function searchUsers(query: string): Promise<AuthorityDto[]> {
   const authorities = await backend.authorities.search(query, true);
-  return authorities
-    .filter((a): a is AuthorityDto => true)
-    .sort((a, b) => a.name.localeCompare(b.name));
+  return authorities.sort((a, b) => a.name.localeCompare(b.name));
 }

---

891-900: JWT claims (iss, sub, iat) placed in the header instead of the payload.

Per RFC 7519, iss, sub, and iat are registered claims that belong in the JWT payload, not the header. Here they're passed as part of the first (header) argument to JWT.build, while the second argument contains only processInfo. The verification on line 930 reads them back from header, so it's internally consistent — but this is non-standard and may confuse future maintainers or break interop with standard JWT libraries.

---

947-966: Error handling silently swallows failure and leaves defaults at zero.

The TODO on line 961 acknowledges this. On failure, defaultRequiredEmergencyKeyShares is set to 0 and defaultEmergencyCouncilMembers stays empty, which could allow the user to proceed with invalid configuration (e.g., canStartRecovery for COUNCIL_CHANGE checks newCouncilMembers.length >= newRequiredKeyShares — both could be 0).

Would you like me to open an issue to track the TODO for proper error handling here?

---

876-877: Non-atomic complete + delete: a failure on delete leaves an orphaned completed process.

If backend.emergencyAccess.complete(process.id) succeeds but backend.emergencyAccess.delete(process.id) throws, the process is completed but never cleaned up. The UI will show success since the error would be caught, but the process lingers on the backend.

Consider whether delete should be handled separately (e.g., with a retry or a warning rather than a hard error), or whether the backend should delete on complete.

---

160-160: Typo: selectedNewmembers should be selectedNewMembers for naming consistency.

The computed on line 483 is named selectedNewmembers (lowercase 'm') while the corresponding selectedNewOwners uses PascalCase for "Owners". This inconsistency appears in the template (line 160) and the completeRecovery function (line 855).

frontend/src/components/emergencyaccess/EmergencyAccessVaultList.vue (3)

280-283: Sequential API calls in a loop — N+1 request pattern.

Each vault's recovery processes are fetched sequentially in a for loop, causing a waterfall of network requests. For users with many vaults, this will noticeably delay data loading.

Proposed fix: parallelize with Promise.all

-      for (const vault of vaults.value) {
-        const processes = await backend.emergencyAccess.findProcessesForVault(vault.id);
-        vaultRecoveryProcesses.value[vault.id] = processes;
-      }
+      const processResults = await Promise.all(
+        vaults.value.map(async (vault) => {
+          const processes = await backend.emergencyAccess.findProcessesForVault(vault.id);
+          return [vault.id, processes] as const;
+        })
+      );
+      vaultRecoveryProcesses.value = Object.fromEntries(processResults);

---

473-478: activeProcessForVault arbitrarily returns the first process — the TODO should be addressed.

This function is used by hasSubmittedEmergencyKeyShare (line 437) and the approvable filter (line 396). With multiple process types (CHANGE_PERMISSIONS, COUNCIL_CHANGE) potentially active simultaneously, always returning list[0] means the filter results depend on arbitrary server ordering. A vault could appear "approved" for one process type but "approvable" for another, yet only one state is reflected.

Would you like me to open an issue to track fixing this, or propose an implementation that checks all active processes?

---

219-225: Exported Item type appears to duplicate AuthorityDto.

Item has id, name, pictureUrl?, type?, memberSize? which overlaps significantly with AuthorityDto. Consider reusing AuthorityDto (or Pick<AuthorityDto, ...>) to avoid maintaining a parallel type. If this type is used by child components that shouldn't depend on the full DTO, it may be intentional — but worth noting.

[coderabbitai]

Actionable comments posted: 7

🤖 Fix all issues with AI agents

In `@frontend/src/components/emergencyaccess/EmergencyAccessDialog.vue`:
- Around line 893-900: The JWT.build call is currently putting standard claims
(iss, sub, iat) into the header instead of the payload; update the call to pass
only header params (alg, typ, b64) as the first argument and move iss, sub, iat
plus the existing processInfo into the payload (second) argument when calling
JWT.build so the token payload contains those claims; keep signing key
userKeys.ecdsaKeyPair.privateKey as the third argument and use
Math.floor(Date.now()/1000) for iat, referencing the JWT.build invocation,
props.me.id, process.id, processInfo, and userKeys.ecdsaKeyPair.privateKey to
locate the change.

In `@frontend/src/components/emergencyaccess/EmergencyAccessVaultList.vue`:
- Around line 416-418: getCouncilMembersForProcess is mapping
proc.recoveredKeyShares keys to usersById.value but can emit undefined for
missing users; update the function (getCouncilMembersForProcess) to mirror
getCurrentCouncilMembers by filtering out falsy/undefined entries after mapping
so it only returns UserDto items that exist (so EmergencyProcessButton won't
receive undefined entries).

In `@frontend/src/components/emergencyaccess/EmergencyScenarioVisualization.vue`:
- Around line 114-116: The component starts an interval in
startRandomCouncilInterval but onBeforeUnmount only removes the resize listener;
store the interval id in a scoped variable (e.g., councilInterval) when calling
setInterval and ensure you call clearInterval(councilInterval) in
onBeforeUnmount (and whenever you explicitly stop the randomization) so the
interval is cleared on unmount; update startRandomCouncilInterval and any stop
logic to set/clear that same variable to avoid leaks.
- Line 35: displayItems is a computed that currently returns the computed ref
randomCouncilSelectionWithPluses, causing a double-wrapped ref and preventing
template rendering; change the displayItems computed to return
randomCouncilSelectionWithPluses.value (i.e., return the raw array) so it yields
the underlying array instead of a ComputedRef, and update the template v-for
usages (including the one at the other occurrence) to iterate over displayItems
(remove `.value` in the template) so Vue's auto-unwrap works correctly.
- Around line 85-88: The component is missing a declared prop for the externally
passed grant-button-disabled, so either remove the prop from the parent or
expose it here; to fix, add an optional boolean prop (e.g. grantButtonDisabled?:
boolean) to the defineProps in EmergencyScenarioVisualization.vue and update the
internal computed isGrantButtonDisabled to prefer the incoming prop when defined
(use the prop value if not undefined, otherwise fall back to the existing
internal computation), ensuring the prop name maps correctly
(grantButtonDisabled in camelCase) so the parent's :grant-button-disabled binds
successfully.
- Line 168: The code casts props.selectedUsers to an undefined generic T (const
available = props.selectedUsers as T[]), causing the build error; change this to
use the actual DTO type or no cast: set const available = props.selectedUsers or
const available = props.selectedUsers as UserDto[] and ensure UserDto is
imported/available in EmergencyScenarioVisualization.vue so the value uses the
concrete UserDto[] type instead of T.

In `@frontend/src/components/MultiUserSelectInputGroup.vue`:
- Around line 182-196: The file registers the same watcher twice (watch(query,
...)) causing duplicate props.onSearch calls; remove the redundant watcher block
so only a single watch(query, async (newQuery) => { ... }) remains, ensuring
searchResults.value is set once per keystroke and the existing logic using
query, searchResults, and props.onSearch is preserved unchanged.

🧹 Nitpick comments (3)

frontend/src/components/emergencyaccess/EmergencyAccessDialog.vue (1)

959-965: TODO: Error handling falls back silently instead of hard-failing.

The catch block on line 959 sets default values instead of surfacing the error. The inline TODO acknowledges this should be a hard failure with an error message instead.

Would you like me to generate an implementation that propagates the error to the user via onError instead of silently falling back?

frontend/src/components/emergencyaccess/EmergencyAccessVaultList.vue (1)

272-275: N+1 request pattern: processes fetched sequentially per vault.

Each vault triggers a separate findProcessesForVault API call in a for...of loop, making this sequential and O(n) in network round-trips. For users with many vaults, this will be noticeably slow.

Consider batching with Promise.all or introducing a backend endpoint that returns processes for multiple vaults at once.

♻️ Quick improvement using Promise.all

-      for (const vault of vaults.value) {
-        const processes = await backend.emergencyAccess.findProcessesForVault(vault.id);
-        vaultRecoveryProcesses.value[vault.id] = processes;
-      }
+      const processEntries = await Promise.all(
+        vaults.value.map(async vault => {
+          const processes = await backend.emergencyAccess.findProcessesForVault(vault.id);
+          return [vault.id, processes] as const;
+        })
+      );
+      vaultRecoveryProcesses.value = Object.fromEntries(processEntries);

frontend/src/components/MultiUserSelectInputGroup.vue (1)

133-133: Redundant computed shadows the prop name.

inputVisible is declared as both a prop (line 121) and a computed (line 133). The computed props.inputVisible !== false is equivalent to the boolean prop value. This shadowing is confusing — the template and script both refer to the computed rather than the prop. If the intent is just to provide a default, the prop's default value mechanism or withDefaults would be clearer.