Skip to content

fix(realunit): confirm existing-shareholder registrations without an email that never arrives#4283

Merged
TaprootFreak merged 2 commits into
developfrom
fix/realunit-confirm-existing-shareholder
Jul 20, 2026
Merged

fix(realunit): confirm existing-shareholder registrations without an email that never arrives#4283
TaprootFreak merged 2 commits into
developfrom
fix/realunit-confirm-existing-shareholder

Conversation

@Blume1977

Copy link
Copy Markdown
Collaborator

Problem

Since #4215 buy quotes are gated on a confirmed registration email. That gate assumes Aktionariat sends a confirmation email on every registerUser forward. It does not: when the signed email already belongs to an existing share-register shareholder, Aktionariat answers

Existing user found, updated your address.

updates that shareholder's wallet in place, and sends no confirmation email. A newly registered email instead gets Confirmation email sent to <email>.

Those existing-shareholder registrations were persisted COMPLETED with requiresEmailConfirmation = true, so they hang forever on a confirmation link that never arrives and the customer cannot buy.

Prod evidence (registration logs since 11.07): every one of the 6 Existing user found responses is stuck (regs 116, 119, 125, 129, 130, 141), every Confirmation email sent response confirmed within minutes, zero exceptions, ~1-2 new stuck cases/day.

Fix

  • Forward path: detect the Existing user found marker in the registerUser response and persist the completed registration with requiresEmailConfirmation = false. Aktionariat's authoritative match of an existing shareholder is the confirmation, so the row is un-gated exactly like a grandfathered one (emailConfirmed = requiresEmailConfirmation === false || confirmedDate != null). A normal Confirmation email sent response stays gated.
  • Backfill migration: un-gate the rows already stuck before this shipped, keyed on the durable Aktionariat/Registration log evidence (response.message = "Existing user found"). Only touches active, COMPLETED, still-gated, unconfirmed rows. Idempotent; down() is an intentional no-op (forward-only data correction).

Verified against prod: the migration predicate matches exactly the 6 existing-shareholder rows and correctly leaves reg 143 (a genuine Confirmation email sent case the customer simply hasn't confirmed yet) gated.

Scope / not included

  • No API-contract change (the emailConfirmed / confirmedDate read-back DTO is unchanged) → no app or services pair-PR needed.
  • Frohs (reg 125) additionally needs identification (KYC level 20); that is separate from this gate.

Test

realunit.service.spec.ts (246 pass) incl. two new cases: existing-shareholder response → un-gated; Confirmation email sent response → stays gated.

…email that never arrives

Aktionariat's registerUser answers "Existing user found, updated your address." when the
signed email already belongs to a share-register shareholder: it updates that shareholder's
wallet in place and sends NO confirmation email (a newly registered email instead gets
"Confirmation email sent to ..."). Since #4215 gates buy quotes on a confirmed registration
email, those registrations were persisted COMPLETED with requiresEmailConfirmation=true and
hang forever on a confirmation link that is never sent.

Persist an existing-shareholder registration with requiresEmailConfirmation=false (Aktionariat's
match IS the confirmation), and backfill the rows already stuck before the fix, keyed on the
durable Aktionariat/Registration log evidence.
@github-actions

Copy link
Copy Markdown

⚠️ Unverified Commits (1)

The following commits are not signed/verified:

  • ef10eee fix(realunit): confirm existing-shareholder registrations without an email that never arrives (Blume1977)
How to sign commits
# SSH signing (recommended)
git config --global gpg.format ssh
git config --global user.signingkey ~/.ssh/id_ed25519.pub
git config --global commit.gpgsign true

# Re-sign last commit
git commit --amend -S --no-edit
git push --force-with-lease

@Blume1977
Blume1977 requested a review from TaprootFreak July 20, 2026 08:18
The isExistingShareholderResponse guard matched "existing user found" as a
free substring of Aktionariat's registerUser reply. Because the alternative
"Confirmation email sent to <email>" reply echoes the raw registrant email, an
address embedding those words (e.g. a quoted local part) could spoof the marker
and un-gate a registration whose confirmation email was in fact sent and never
confirmed. Match it as a start-anchored prefix instead (the real reply is
exactly "Existing user found, updated your address.") and add a regression test
pinning the substring-spoof case as still gated.
@TaprootFreak

Copy link
Copy Markdown
Collaborator

Reviewed this end-to-end (incl. a read-only check against prod + the RealUnit ponder) and pushed one hardening follow-up commit (e8784c7). The core fix and the backfill migration are sound — the migration predicate matches exactly the 6 stuck rows as claimed, and all 6 are on-chain REALU holders in the ponder that held shares before they registered, so the "existing shareholder = confirmation" premise holds for them (reg 143, correctly not matched, holds no REALU).

What the follow-up changes and why

isExistingShareholderResponse matched the marker as a free substring (.includes('existing user found')). The problem: the alternative reply Confirmation email sent to <email> echoes the raw registrant email into the message. A valid, lowercase RFC-5321 quoted-local-part address such as "existing user found"@example.com passes the registration DTO (@IsEmail + @IsLowercase), and its echoed message lowercased contains the marker — so includes would un-gate a registration whose confirmation email was in fact sent and never confirmed, bypassing the #4215 buy gate.

The commit anchors the match to the message start (.startsWith(...)). The real reply is exactly Existing user found, updated your address. (begins with the marker → still un-gates the 6 legitimate cases), while Confirmation email sent to … begins with a fixed literal prefix and can no longer be spoofed by the echoed address. Added a regression test that pins the substring-spoof case as still gated — verified red against includes, green against startsWith. Full realunit spec 247/247, type-check + lint clean.

One related case this commit deliberately does NOT fix (for a separate decision)

startsWith closes the substring-spoof and the silent-reword regression, but not the concurrent double-submit of a genuinely new registration: if two requests race, the first POST creates the user at Aktionariat (Confirmation email sent) and the second POST can get Aktionariat's genuine Existing user found, updated your address. (the user now exists) — the marker is real, not spoofed, so no anchoring helps. Whichever caller wins the advisory-lock persist writes its own flag, so the row can end up un-gated even though a confirmation email was sent. This is currently latent (0 occurrences on prod: no wallet has both reply types across the registration logs) and Aktionariat's own payAndAllocate stays the hard backstop, so I left it out of scope here rather than expand this PR.

Options if we want to close it (suggest a follow-up issue, not blocking this PR):

  • bind the existingShareholder un-gating to an additional proof — e.g. a ponder holder check on the signing wallet, or only un-gate when no Confirmation email sent was logged for the same registration event;
  • or accept it, given the reactive backstop + KYC-30 requirement + zero prod incidence.

Not blocking; the migration and the anchored marker are safe to ship as-is.

@TaprootFreak
TaprootFreak marked this pull request as ready for review July 20, 2026 09:04
@TaprootFreak
TaprootFreak merged commit 1cc0729 into develop Jul 20, 2026
8 checks passed
@TaprootFreak
TaprootFreak deleted the fix/realunit-confirm-existing-shareholder branch July 20, 2026 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants