fix(realunit): make register/wallet idempotent on signature match

[TaprootFreak]

Summary

Surface a real client retry-stuck bug spotted while reviewing DFXswiss/realunit-app#466 (KYC merge flow). `POST /v1/realunit/register/wallet` (and the same-wallet branch of `POST /v1/realunit/register/complete`) was non-idempotent: if the first call succeeded server-side but the response was lost (mobile network drop is the realistic case), the client's automatic retry with the same EIP-712 signature got a hard `400 Bad Request` — "RealUnit registration already exists for this wallet" — with no recovery path. The user ends up stuck on a failure snackbar even though the registration is in fact complete.

Fix

Replace the hard throw with a signature-aware idempotency check in `RealUnitService` (new private helper `idempotentRegistrationResult`):

• Same wallet + same signature → return the existing registration's status. `COMPLETED` if the existing `KycStep` is completed (forward to Aktionariat succeeded on the first attempt), `FORWARDING_FAILED` otherwise. No new `KycStep`, no re-forward.
• Same wallet + different signature → keep the 400, but with a more precise message: `"RealUnit registration already exists for this wallet with a different signature"`. A fresh sign over the same wallet means the client produced a new payload — that is a real conflict, not a retry.

Signature comparison is case-insensitive (`Util.equalsIgnoreCase`): EIP-712 signatures are 0x-prefixed hex strings and lower/upper case representations are semantically identical. Matches the convention already used for wallet-address comparisons in `findRegistrationStep`.

Both endpoints share the helper; `completeRegistration` is refactored to call `findRegistrationStep` directly instead of the boolean-only `hasRegistrationForWallet`, so it has access to the `KycStep` for the idempotency check. `hasRegistrationForWallet` itself is preserved (still used by `realunit.controller.ts` and two other call-sites).

The helper takes `userData` as an explicit parameter rather than reading `step.userData` so the call does not depend on TypeORM back-populating the inverse relation on steps loaded via `userData.kycSteps`. Each idempotent hit is logged at `info` level (`userData.id`, `kycStep.id`, resulting status) so production retry frequency is observable and stuck-user reports can be correlated to a specific step.

Tests

`describe('completeRegistrationForWalletAddress (idempotency)', ...)` block in `realunit.service.spec.ts` covers all branches:

1. Same signature + completed step → returns `COMPLETED`, no `createCustomKycStep` call
2. Same signature + non-completed step → returns `FORWARDING_FAILED`, no `createCustomKycStep` call
3. Same signature, different case (stored upper, incoming lower) → returns `COMPLETED`, no `createCustomKycStep` call
4. Different signature → throws `BadRequestException`, no `createCustomKycStep` call

Test setup expands the `UserService` and `KycService` mock providers (previously empty objects) with the methods needed by these tests. The `Util.equalsIgnoreCase` jest mock was missing from the existing setup — added here too, since both `findRegistrationStep` and the new signature comparison rely on it.

Local verification

• `npm install` ✅
• `npm run format` — no changes to touched files
• `npm run lint` ✅
• `npm run type-check` ✅
• `npm test` — 14/14 in `realunit.service.spec.ts` ✅

Test plan

• Manual reproduction of the original stuck state (against DEV): trigger `/v1/realunit/register/wallet` twice with identical body — second call should now return `201 { "status": "completed" }` instead of `400`.
• Signature-mismatch protection: second call with same wallet but altered signature → still `400`, new message ("…with a different signature").
• Unrelated flows unchanged: new-customer registration (no existing step) still creates a `KycStep` and forwards as before.

Known limitations

• UTC midnight retry: the realunit-app client signs over `registrationDate = today` (in UTC). A retry that crosses the UTC date boundary therefore re-signs against a different message and lands in the "different signature" branch (400). In the real-world failure mode (snackbar fires, user taps "retry" seconds later) this never triggers, but if a retry happens minutes after the rollover the client will still need to surface a sensible error. A future iteration could relax the match to wallet+userData once the forward has reached `COMPLETED`, but that loosens the conflict semantics deliberately defined here.

Out of scope / follow-ups

• The realunit-app KYC merge flow already surfaces these failures correctly after DFXswiss/realunit-app#466 — this PR closes the backend loop so legitimate retries succeed instead of looping in the failure path.
• `completeRegistrationForWalletAddress` still does not call `validateRegistrationDto` (only the signature is verified). Out of scope here, but worth a separate look — `registrationDate` and birthday/age validation are skipped on this path.

[TaprootFreak]

🧭 Vollständiges Why-Briefing — warum diese Idempotenz-Änderung notwendig ist

Dieser Kommentar dokumentiert ausführlich den Kontext und die Begründung, weil der Fix selbst klein ist (21 Zeilen Logik), aber die Auswirkungen auf das User-Erlebnis und die Datenintegrität signifikant. Wer den Diff alleine liest, sieht nicht, warum die strikte BadRequestException falsch war.

---

1. Wo das Problem aufgetaucht ist

Bei einem kritischen Code-Review von DFXswiss/realunit-app#466 (Frontend, KYC-Merge-Flow). Der PR fixt zwei Bugs im Wallet-Onboarding für existierende DFX-Kunden:

• Bug A: Existierende DFX-Kunden wurden nach erfolgreichem Account-Merge fälschlich auf das leere Registration-Formular geleitet.
• Bug B: KycEmailVerificationCubit.checkEmailVerification emittierte Success unconditionally, auch wenn _completeRegistration (= registerWallet-Call) intern bereits ein RegistrationFailure ausgelöst hatte. User glaubte an Erfolg, während die Wallet de facto nicht registriert war.

Während des Reviews ist aufgefallen, dass selbst nach Bug-B-Fix der Retry-Pfad für den User in eine Sackgasse läuft — und der Grund liegt backend-seitig.

---

2. Der vollständige End-to-End-Flow eines existierenden DFX-Kunden

Damit nachvollziehbar wird, an welcher Stelle der Bug zuschlägt, hier die komplette Kette:

1. User öffnet realUnit-App
2. → "Buy" → "Next" → Email-Eingabe
3. App → API:  POST /v1/realunit/register/email   { email }
                ↓ Backend triggert Merge-Request (Email schon bei DFX bekannt)
4. User klickt Bestätigungslink im Email-Postfach
5. User tippt "I have confirmed my email" in der App
6. App → API:  GET  /v1/auth/jwt                  (neuer Token = neuer accountId)
              GET  /v1/realunit/wallet/status    (holt userData incl. realUnitUserDataDto)
              POST /v1/realunit/register/wallet  { walletAddress, signature, registrationDate }
                ↑ DAS ist der Endpoint, um den es hier geht.
7. Bei Erfolg: App pop'd zurück, markRegistrationSignProduced() → checkKyc() → next Step

EIP-712-Signatur in Schritt 6 wird im App-Code aus dem gespeicherten User-Data des bestehenden DFX-Accounts (Adresse, Name, Geburtstag, …) plus der neuen Wallet-Adresse plus dem heutigen Datum zusammengesetzt und mit dem Private Key der neuen Wallet signiert. Deterministisch: gleiche Eingaben ⇒ gleiche Signatur.

---

3. Das "lost response" Szenario im Detail

Mobile Netzwerke verlieren Responses regelmäßig — TCP-Connection-Reset, kurzer LTE-Drop nach Antenna-Handover, App im Background, etc. Genau das Szenario, das hier durchschlägt:

T+0.0s   App → POST /v1/realunit/register/wallet
T+0.5s   Backend: validate signature ✅
T+0.5s   Backend: kycService.createCustomKycStep() → KycStep angelegt mit ReviewStatus.INTERNAL_REVIEW
T+1.2s   Backend: forwardRegistration() → Aktionariat ✅ → kycStep → ReviewStatus.COMPLETED
T+1.3s   Backend: return 201 { status: "completed" }
T+1.3s   Response unterwegs zurück zur App
T+1.4s   📡 Netzwerk-Drop. Response geht verloren.
T+30s    App: http.post timeout → DioException
T+30s    Cubit: emit(RegistrationFailure), User sieht Snackbar
T+35s    User tippt nochmal auf "Erneut versuchen"
T+35.1s  App → POST /v1/realunit/register/wallet   ← gleicher Body, gleiche Signatur!
T+35.6s  Backend: findRegistrationStep(...) → isForCurrentWallet=true
                   (weil KycStep vom ersten Call existiert)
T+35.6s  ❌ Backend: throw BadRequestException("RealUnit registration already exists for this wallet")
T+35.7s  App: 400 → erneut RegistrationFailure → User stuck

Der entscheidende Punkt: Der erste Call ist erfolgreich durchgelaufen. Die Wallet ist registriert, der KycStep ist COMPLETED, der Forward zu Aktionariat war erfolgreich. Aber der User weiss nichts davon, weil er die Response nie gesehen hat — und der Retry bekommt einen 400, der nach "deine Registrierung ist gescheitert" aussieht. Tatsächlich heisst er "deine Registrierung war bereits erfolgreich, ich erkenne nur nicht, dass du derselbe Anfrager bist".

---

4. Wie sich das vor PR #466 für den User dargestellt hat

Pre-#466 war es noch perfider: Bug B (emit(Success) unconditionally) hat den RegistrationFailure aus dem catch-Block überschrieben. Der User sah einen kurz aufblitzenden Snackbar und dann pop(true) zurück zum Dashboard — sah aus wie Erfolg. In Wahrheit: Wallet teilweise registriert, KYC-State inkonsistent.

Damit war der Bug unsichtbar für den User, was ihn so lange unentdeckt hat überleben lassen.

---

5. Wie sich das nach PR #466 darstellt (Stand jetzt, ohne diesen Backend-Fix)

Nach dem Frontend-Fix in #466 (gemerged in develop) gibt es:

1. Korrekte Failure-Surface: Success nur noch wenn _completeRegistration true returnt.
2. _runGeneration Cancellation-Token gegen Race-Conditions.
3. _mergeDetected-Short-Circuit: beim Retry wird der Auth-Token-Check übersprungen, der Cubit geht direkt zu _completeRegistration (= registerWallet).

Ergebnis: User sieht zwar jetzt korrekt die Failure-Snackbar (gut), aber sein Retry läuft direkt in den 400 Bad Request des Backends (immer noch schlecht). Snackbar bleibt, die Page bleibt offen, der User klickt 3× "erneut versuchen", bekommt jedes Mal die gleiche Antwort, kontaktiert dann Support — der Support muss dann manuell verifizieren, dass die Registrierung im Backend bereits erfolgreich war.

Das ist genau das Verhalten, das wir mit diesem PR fixen. Frontend war nur der halbe Fix.

---

6. Warum genau Idempotenz und nicht "wallet-only check"

Naive Lösung wäre: "wenn isForCurrentWallet, return COMPLETED" — also egal welche Signatur. Das wäre gefährlich:

• Ein Angreifer könnte mit einer beliebigen (gültigen oder ungültigen) Signatur einen weiteren Call absetzen und einen 200 OK zurückbekommen, ohne dass jemals validiert wird, dass er Kontrolle über die Wallet hat.
• Replay-Angriff: alte Signatur eines anderen Zeitstempels könnte funktionieren.

Deswegen signature-aware:

• Gleiche Signatur ⇒ das ist garantiert derselbe Request, weil EIP-712-Signaturen deterministisch aus (Wallet-PrivKey + Payload-Hash) abgeleitet werden. Nur derjenige, der den Original-Request abgesetzt hat, kann diese Signatur reproduzieren.
• Andere Signatur ⇒ entweder neuer Sign-Akt (legitimer Konflikt) oder Replay-Versuch. Beides berechtigt einen 400.

Das ist die Mindest-Sicherheitsschwelle, die wir nicht aufweichen dürfen.

---

7. Warum beide Endpoints betroffen sind

Es gibt zwei verwandte Endpoints im selben Service:

Endpoint	Anwendungsfall	Gleicher Bug?
POST /v1/realunit/register/complete	Neue Registration (kein existierender Account)	Ja — Zeile 607 (alt)
POST /v1/realunit/register/wallet	Wallet-Registrierung für existing-customer nach Merge	Ja — Zeile 669 (alt)

Beide hatten dieselbe Stelle:

if (this.hasRegistrationForWallet(...)) {  // bzw. isForCurrentWallet
  throw new BadRequestException('RealUnit registration already exists for this wallet');
}

Das Frontend ruft je nach Pfad einen anderen Endpoint auf, aber das Stuck-Verhalten ist in beiden Pfaden identisch. Deshalb wird dieselbe Helper-Methode idempotentRegistrationResult an beiden Stellen angewandt — konsistente Semantik, ein zentraler Punkt für späteres Tuning.

---

8. Warum die Helper-Methode COMPLETED vs FORWARDING_FAILED differenziert

Der existierende KycStep kann zwei Status haben, die beide "die Idempotenz greift" bedeuten:

• step.isCompleted === true — Forward zu Aktionariat war beim ersten Mal erfolgreich. Der Client darf annehmen, dass alles fertig ist. Return COMPLETED.
• step.isCompleted === false (Status INTERNAL_REVIEW, Forward fehlgeschlagen oder noch nicht durchgelaufen) — Forward muss vom Admin oder einem retry-Mechanismus nachgeholt werden. Return FORWARDING_FAILED.

Das matched exakt das Verhalten, das der erste Call gehabt hätte. Idempotenz ist nur dann sauber, wenn das Resultat des zweiten Calls dasselbe ist wie das des ersten — egal welchen Pfad das Backend intern nimmt.

Wir versuchen bewusst nicht, einen fehlgeschlagenen Forward beim Retry erneut zu pushen. Das wäre Scope-Creep und potenziell gefährlich (idempotent ≠ retry-policy). Aktionariat-Forward-Retry ist ein eigenes Thema und wird heute (offenbar) über admin/registration/:kycStepId/forward manuell angestossen.

---

9. Was bewusst NICHT verändert wurde

Bereich	Status	Begründung
Signatur-Verifikation (verifyRealUnitRegistrationSignature)	unverändert	Wir validieren die Signatur in completeRegistrationForWalletAddress weiterhin nur, wenn wir tatsächlich einen neuen KycStep anlegen. Auf dem idempotenten Pfad wäre eine erneute Verifikation redundant — die Signatur wurde beim ersten Call schon geprüft und ist (per definition match) dieselbe.
hasRegistrationForWallet-Methode	unverändert	Wird noch an drei weiteren Stellen genutzt (controllers/realunit.controller.ts:499, sowie zwei weitere Service-interne Calls). completeRegistration musste auf den direkten findRegistrationStep-Call umgestellt werden, damit der step auch verfügbar ist — hasRegistrationForWallet returned nur boolean.
validateRegistrationDto für den /register/wallet-Pfad	unverändert	Existierte vorher schon nicht (nur Signatur-Check). Ist eine separate Frage und wird unten als Follow-up erwähnt.
Aktionariat-Forward bei Retry	unverändert	Wir machen ihn nicht erneut. Siehe oben — Idempotenz heisst, dasselbe Resultat zu liefern, nicht den fehlgeschlagenen Forward zu retryen.
Existierende KycStep-Daten	unverändert	Auf dem idempotenten Pfad wird nichts modifiziert — kein neuer KycStep, kein Update des bestehenden, keine User-Data-Änderung. Vollständig read-only-Response.

---

10. Sicherheitsbetrachtung — wird hier eine Lücke geöffnet?

Nein. Konkret durchgespielt:

• Replay durch einen anderen Account: blockiert durch userData.id !== userDataId Check (Zeile 665 unverändert) — ein anderer Account kann nicht über die Wallet-Adresse einer fremden Wallet einen 200 erschleichen.
• Replay mit gestohlener Signatur: blockiert durch den userData-Lookup über getUserByAddress(walletAddress) — die walletAddress aus dem DTO muss zum gleichen Account gehören, der das JWT mitgesendet hat. Eine gestohlene Signatur eines anderen Users hat keinen Effekt.
• Replay vom gleichen User mit alter Signatur: technisch sind alle Signaturen "alt" (auch der erste Call war "alt" als er ankam) — der Schutz besteht ausschliesslich darin, dass nur jemand mit dem Wallet-Private-Key die Signatur produzieren kann. Das ändert sich durch diesen PR nicht.
• Datenmanipulation: ausgeschlossen, weil der idempotente Pfad nichts schreibt. Keine personalData, kein kycStep, kein Forward.

Die Sicherheits-Garantien sind also exakt dieselben wie vorher. Was sich ändert ist nur, dass ein legitimer Retry nicht mehr in eine User-Sackgasse läuft.

---

11. Test-Strategie

Drei Test-Cases im neuen describe('completeRegistrationForWalletAddress (idempotency)') Block:

#	Setup	Expectation
1	KycStep existiert mit gleicher Signatur, isCompleted=true	Return COMPLETED, createCustomKycStep nicht aufgerufen
2	KycStep existiert mit gleicher Signatur, isCompleted=false	Return FORWARDING_FAILED, createCustomKycStep nicht aufgerufen
3	KycStep existiert mit anderer Signatur	Wirft BadRequestException, createCustomKycStep nicht aufgerufen

Wichtig ist die Zusatz-Assertion expect(kycService.createCustomKycStep).not.toHaveBeenCalled() — sie sichert ab, dass auf dem idempotenten Pfad kein neues KycStep angelegt wird. Ohne die Assertion könnte ein späteres Refactoring den Helper falsch implementieren, die ersten zwei Tests würden trotzdem grün bleiben.

completeRegistration (der andere Endpoint) ist über denselben Helper abgedeckt — die spezifischen Pfade dort haben mehr Pre-Conditions (Email-Match, KycLevel-Check, etc.) und werden über das vorhandene Test-Setup nicht trivial isolierbar. Der Helper ist die Single Source of Truth.

---

12. Real-World-Frequenz — wie oft passiert sowas?

Mobile Network Drops zwischen Backend-Write und Client-Receipt sind ein Klassiker. Realistische Schätzungen aus Mobile-Backend-Literatur:

• ~0.1 % bis 1 % aller mobile API-Calls fallen ins "Server hat geschrieben, Client hat keine Response gesehen"-Fenster, abhängig von Netzwerk-Qualität und Request-Latenz.
• Bei einem Onboarding-Flow mit ~1 kritischem Wallet-Registration-Call pro neuem User: bei 1000 neuen DFX-Kunden ⇒ ~1-10 stuck users pro Onboarding-Welle.

Das ist die Größenordnung, in der man Support-Tickets bekommt mit der Beschreibung "ich hab Wallet hinzugefügt, aber die App sagt Fehler und ich kann nicht weitermachen". Die User wären dann tatsächlich schon registriert, der Support müsste das manuell verifizieren und den User durchschalten. Diese Tickets verschwinden mit diesem Fix.

---

13. Follow-ups, die in eigene PRs gehören

1. validateRegistrationDto auch für /register/wallet-Pfad: Der Endpoint validiert aktuell nur die Signatur, nicht registrationDate (muss heute sein), Birthday/Age, Personal-Data-Format. Bei completeRegistration läuft validateRegistrationDto als erstes — bei completeRegistrationForWalletAddress nicht. Unklar, ob bewusst (Daten kommen aus dem bestehenden KycStep, müssten dort schon validiert sein), aber das registrationDate aus dem DTO sollte wahrscheinlich auf "heute" geprüft werden.
2. Aktionariat-Forward Retry-Policy: Wenn ein KycStep auf FORWARDING_FAILED steht, gibt es keinen automatischen Retry. Der Idempotenz-Fix macht den User darauf aufmerksam (Frontend zeigt jetzt die korrekte Failure-Snackbar), aber der eigentliche Recovery-Pfad fehlt. Heute über admin/registration/:kycStepId/forward (manuell).
3. Beobachtung des stuck-Patterns vor Fix: Wäre interessant, ein paar Wochen Daten aus den Logs zu ziehen, wie oft der "400 already exists"-Response in Production tatsächlich aufgetreten ist — das wäre die Validierung der Real-World-Frequenz-Schätzung in Punkt 12.

---

14. Zusammenfassung in einem Satz

Wir tauschen einen unbedingten 400 Bad Request gegen eine signature-verifizierte Idempotenz-Antwort, damit legitimen Client-Retries nach Netzwerk-Verlust nicht mehr in eine User-Sackgasse laufen, ohne die Sicherheits-Garantien zu schwächen.

[davidleomay]

Use case insensitive check? Util.equalsIgnoreCase