infrastructure/eid-wallet/src/routes/(auth)/verify/steps/selfie.svelte (1)

65-67: Progression via SSE/WebSocket only — add a fallback

Good to defer step advancement to server events. Consider a timeout fallback (e.g., show “Retry” or re-enable capture) if no event arrives to avoid UI dead-ends.

Confirm the event that sets verifStep=3 is always emitted after PATCH; if not, add a fallback timer.

infrastructure/eid-wallet/src/routes/(auth)/verify/store.ts (1)

11-11: documentType union looks good; consider centralizing the type

Optional: export a DocumentType type and reuse it across components to avoid union drift.

export type DocumentType = "passport" | "id" | "permit" | "dl";
export const documentType = writable<DocumentType | null>(null);

infrastructure/eid-wallet/src/routes/(auth)/verify/steps/passport.svelte (2)

51-94: Camera selection complexity (torch probing) likely unnecessary

You detect torch support but never enable it; iterating all devices with getUserMedia can be heavy and prompt repeatedly on some browsers. Consider simplifying to facingMode: "environment" and avoid per-device streams. If torch is needed, use applyConstraints when available.

// Simplified approach
async function getMainCameraStream() {
  return navigator.mediaDevices.getUserMedia({ video: { facingMode: "environment" } });
}

Ensure iOS/Safari behavior meets your UX with the simpler constraint; otherwise, add torch via track.applyConstraints({ advanced: [{ torch: true }] }) when low-light is detected.

---

141-141: Remove debug logs or guard behind dev flag

console.log in production UI can be noisy.

- console.log("Switched to back image capture, image =", image);
+ if (import.meta.env?.DEV) console.debug("Switched to back image capture", { image });

Also applies to: 173-173

infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte (2)

272-279: Encode w3id when building the resolve URL

Use URLSearchParams to avoid malformed queries if w3id contains special characters.

-                const response = await axios.get(
-                    new URL(
-                        `resolve?w3id=${existingW3id}`,
-                        PUBLIC_REGISTRY_URL,
-                    ).toString(),
-                );
+                const url = new URL("resolve", PUBLIC_REGISTRY_URL);
+                url.searchParams.set("w3id", existingW3id);
+                const response = await axios.get(url.toString());

---

118-145: Close the EventSource on component destroy to prevent leaks

Persist the EventSource and close it in onDestroy.

-    import { getContext, onMount } from "svelte";
+    import { getContext, onMount, onDestroy } from "svelte";

+    let verificationEventSource: EventSource | null = null;

-        const eventSource = new EventSource(sseUrl);
+        verificationEventSource = new EventSource(sseUrl);

-        eventSource.onopen = () => {
+        verificationEventSource.onopen = () => {
@@
-        eventSource.onmessage = (e) => {
+        verificationEventSource.onmessage = (e) => {

+    onDestroy(() => {
+        verificationEventSource?.close();
+        verificationEventSource = null;
+    });

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

infrastructure/eid-wallet/src/routes/(auth)/verify/steps/passport.svelte (1)

241-242: Instruction text logic is still incorrect (duplicate issue).

This issue was flagged in the previous review but remains unfixed. The instruction text incorrectly uses $DocFront ? "back" : "photo", which:

• Shows "photo" initially (when $DocFront is null), which is wrong for non-passport documents
• Shows "back" after front capture for all documents, which is wrong for passports

For passports, it should always show "photo" (single-page document). For other documents, it should show "front" initially, then "back" after the front is captured.

Apply this diff to fix the instruction text:

                 <p>
                     Please place your document's
-                    {$DocFront ? "back" : "photo"}
+                    {#if $documentType === "passport"}
+                        photo
+                    {:else}
+                        {image === 1 ? "front" : "back"}
+                    {/if}
                     page within the rectangle and press the take photo button
                 </p>

infrastructure/eid-wallet/src/routes/(auth)/verify/steps/passport.svelte (1)

22-23: Optional: Consider using plain reactive variables for local state.

image1Captured and image2Captured are only used within this component and don't need to be writable stores. Plain reactive variables would be simpler and more performant for local-only state.

Apply this diff to simplify the state management:

-let image1Captured = writable(false);
-let image2Captured = writable(false);
+let image1Captured = false;
+let image2Captured = false;

Then update the usages:

-            image1Captured.set(true);
+            image1Captured = true;

-            image2Captured.set(true);
+            image2Captured = true;

-    image1Captured.set(false);
-    image2Captured.set(false);
+    image1Captured = false;
+    image2Captured = false;

And in the template:

-{#if ($documentType !== "passport" && $image1Captured && $image2Captured) || ($documentType === "passport" && $image1Captured)}
+{#if ($documentType !== "passport" && image1Captured && image2Captured) || ($documentType === "passport" && image1Captured)}

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

infrastructure/eid-wallet/src/routes/(auth)/verify/steps/passport.svelte (6)

130-130: Remove debug logging for production.

The console.error statement should be removed or replaced with a production-safe logging mechanism.

---

141-141: Remove debug logging for production.

The console.log statement should be removed to avoid debug output in production.

---

166-166: Remove debug logging for production.

The console.error statement should be removed or replaced with a production-safe logging mechanism.

---

173-173: Remove debug logging for production.

The console.log statement should be removed to avoid debug output in production.

---

264-264: Remove commented-out code.

The commented-out HTML comment should be removed to keep the codebase clean.

---

241-242: Instruction text logic is still incorrect.

Despite the past review comment being marked as addressed, the instruction text still uses {$DocFront ? "back" : "photo"} which is incorrect. For passports, it should always show "photo" (single page). For other documents, it should show "front" or "back" based on the current capture state (the image variable), not based on whether $DocFront exists.

Apply this diff to fix the logic:

                <p>
-                    Please place your document's {$DocFront ? "back" : "photo"} page within the rectangle
+                    Please place your document's
+                    {#if $documentType === "passport"}
+                        photo
+                    {:else}
+                        {image === 1 ? "front" : "back"}
+                    {/if}
+                    page within the rectangle
                    and press the take photo button
                </p>

infrastructure/eid-wallet/src/routes/(auth)/verify/steps/passport.svelte (2)

241-242: Redundant instruction text in two locations.

The component has instruction text at both lines 241-242 (brief) and lines 263-289 (detailed). The detailed version (lines 263-289) provides better context by showing the current capture state (front/back). Consider removing or simplifying the instruction at lines 241-242 to avoid confusion, or consolidate both into a single, clear instruction.

Also applies to: 263-289

---

1-1: Consider renaming the component file.

The component is named passport.svelte but now handles multiple document types (passport, ID card, driver's license, residence permit). Consider renaming it to something more generic like document-capture.svelte to better reflect its purpose.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte (2)

17-28: Import documentType from the store.

The documentType store is still not imported despite the past review comment. This store is needed to make user-facing labels document-type aware (as suggested in the previous review for lines 252-258).

Add the import:

 import {
     DocFront,
     DocBack,
     Selfie as SelfiePic,
     reason,
     status,
     verifStep,
     verificaitonId,
+    documentType,
 } from "./store";

---

247-258: Use documentType store for consistent labeling.

The code now handles different document types from the backend response (document.type.value), but it doesn't use the documentType store that captures the user's selection. This creates a potential inconsistency if the backend classification differs from what the user selected.

As suggested in the previous review, import and use the documentType store:

 "ID submitted":
-    document.type.value === "passport"
-        ? `Passport - ${document.country.value}`
-        : document.type.value === "drivers_license"
-          ? `Driving License - ${document.country.value}`
-          : `ID Card - ${document.country.value}`,
+    (() => {
+        const docLabel = {
+            passport: "Passport",
+            id: "National ID",
+            permit: "Residence Permit",
+            dl: "Driver's License",
+        } as const;
+        return `${docLabel[$documentType ?? "passport"]} - ${document.country.value}`;
+    })(),

infrastructure/eid-wallet/src/lib/fragments/IdentityCard/IdentityCard.svelte (1)

94-94: Terminology update aligns with multi-document support.

The change from "DEMO PASSPORT"/"VERIFIED PASSPORT" to "DEMO ID"/"VERIFIED ID" appropriately generalizes the badge text to support multiple document types beyond passports.

Consider extracting these strings to an i18n/localization file if the application supports multiple languages.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

infrastructure/evault-provisioner/test-real-notifications.js (1)

12-12: Consider a stronger default secret for testing.

Similar to test-notification.js, the default value 'your-secret-here' is weak. While this is a test script, consider using a more complex default or requiring the environment variable to be set.

-const SHARED_SECRET = process.env.NOTIFICATION_SHARED_SECRET || 'your-secret-here';
+const SHARED_SECRET = process.env.NOTIFICATION_SHARED_SECRET || (() => { throw new Error('NOTIFICATION_SHARED_SECRET must be set'); })();

infrastructure/evault-provisioner/test-notification.js (1)

15-15: Consider a stronger default secret for testing.

The default value 'your-secret-here' is weak. While this is a test script, consider using a more complex default or requiring the environment variable to be set to avoid accidental misuse.

-const SHARED_SECRET = process.env.NOTIFICATION_SHARED_SECRET || 'your-secret-here';
+const SHARED_SECRET = process.env.NOTIFICATION_SHARED_SECRET || (() => { throw new Error('NOTIFICATION_SHARED_SECRET must be set'); })();

infrastructure/evault-provisioner/src/migrations/1759924736417-migration.ts (1)

6-11: Consider adding indexes for device queries.

The new device-related columns are queried in NotificationService (e.g., getDevicesByEName at lines 75-78 queries by linkedEName and deviceActive). Adding indexes would improve query performance as device registrations scale.

Apply this diff to add a composite index:

     public async up(queryRunner: QueryRunner): Promise<void> {
         await queryRunner.query(`ALTER TABLE "verification" ADD "deviceId" character varying`);
         await queryRunner.query(`ALTER TABLE "verification" ADD "platform" character varying`);
         await queryRunner.query(`ALTER TABLE "verification" ADD "fcmToken" character varying`);
         await queryRunner.query(`ALTER TABLE "verification" ADD "deviceActive" boolean NOT NULL DEFAULT true`);
+        await queryRunner.query(`CREATE INDEX "IDX_verification_linkedename_deviceactive" ON "verification" ("linkedEName", "deviceActive")`);
     }

And update the down migration:

     public async down(queryRunner: QueryRunner): Promise<void> {
+        await queryRunner.query(`DROP INDEX "IDX_verification_linkedename_deviceactive"`);
         await queryRunner.query(`ALTER TABLE "verification" DROP COLUMN "deviceActive"`);
         await queryRunner.query(`ALTER TABLE "verification" DROP COLUMN "fcmToken"`);
         await queryRunner.query(`ALTER TABLE "verification" DROP COLUMN "platform"`);
         await queryRunner.query(`ALTER TABLE "verification" DROP COLUMN "deviceId"`);
     }

infrastructure/evault-provisioner/src/controllers/NotificationController.ts (3)

9-14: Use entity classes instead of string literals for type safety.

Using string literals "Verification" and "Notification" to retrieve repositories is less type-safe than using the entity classes directly. This makes the code more prone to typos and refactoring errors.

Apply this diff:

+import { Verification } from "../entities/Verification";
+
 export class NotificationController {
     private notificationService: NotificationService;

     constructor() {
         this.notificationService = new NotificationService(
-            AppDataSource.getRepository("Verification"),
-            AppDataSource.getRepository("Notification")
+            AppDataSource.getRepository(Verification),
+            AppDataSource.getRepository(Notification)
         );
     }

---

142-145: Optimize notification marking with bulk update.

Marking notifications as delivered sequentially in a loop is inefficient and creates multiple database round-trips. Consider using a bulk update operation.

Update NotificationService to add a bulk marking method:

// In NotificationService.ts
async markNotificationsAsDelivered(notificationIds: string[]): Promise<void> {
    await this.notificationRepository.update(
        { id: In(notificationIds) },
        { delivered: true, deliveredAt: new Date() }
    );
}

Then update the controller:

             // Get undelivered notifications for this eName
             const notifications = await this.notificationService.getUndeliveredNotifications(eName);
             
             // Mark notifications as delivered
-            for (const notification of notifications) {
-                await this.notificationService.markNotificationAsDelivered(notification.id);
-            }
+            if (notifications.length > 0) {
+                await this.notificationService.markNotificationsAsDelivered(
+                    notifications.map(n => n.id)
+                );
+            }

---

128-167: Consider adding pagination and rate limiting.

The checkNotifications endpoint could return a large number of notifications without pagination, and the notification endpoints lack rate limiting, which could lead to performance issues or abuse.

Consider:

1. Adding pagination parameters (limit/offset or cursor-based) to checkNotifications
2. Implementing rate limiting middleware (e.g., using express-rate-limit) on all notification endpoints
3. Adding request size limits to prevent large payload attacks

Example for rate limiting:

import rateLimit from 'express-rate-limit';

const notificationLimiter = rateLimit({
    windowMs: 15 * 60 * 1000, // 15 minutes
    max: 100, // limit each IP to 100 requests per windowMs
    message: 'Too many requests from this IP'
});

registerRoutes(app: any) {
    app.post("/api/notifications/check", notificationLimiter, this.checkNotifications.bind(this));
    // ... other routes
}

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro