feat: collect phone number at registration to activate phone_hash anti-abuse controls - #319
Conversation
ericmt-98
left a comment
There was a problem hiding this comment.
Thanks — the client-side hashing utility, the optional phone field, and the privacy notice are the right approach and the code is clean. A few things to address before merge:
1. The "server cannot reverse the number" claim is overstated
phoneHash.ts and the UI note say the hash can't be reversed. But an unsalted SHA-256 of a phone number isn't meaningfully irreversible: the phone-number space is small (~10-13 digits), so anyone with DB access can brute-force the whole space and recover every user's number in seconds.
There's a real constraint here that makes this hard: the hash has to be deterministic and identical across all clients so the assertNotRelatedAccounts equality check works — which rules out a per-user salt. A client-side hash therefore can't give both equality-matching and true irreversibility (a "pepper" shipped in the app bundle isn't secret). So this is partly inherent to the client-side approach.
Minimum ask: correct the wording so it doesn't claim irreversibility — say the raw number isn't transmitted and isn't stored in plaintext, which is true and still a real improvement. Bigger question worth a comment thread (not necessarily this PR): whether to move hashing server-side with a real secret pepper (the server has a secret the client doesn't) at the cost of sending the raw number over TLS. Let's decide that before calling this "privacy-preserving" in user-facing copy.
2. Remove TODO.md
TODO.md at the repo root is a working scratchpad that got committed — please drop it from the PR. (It also notes tsc wasn't run — please run cd micopay/frontend && npx tsc --noEmit and confirm before pushing.)
3. Revert the unrelated whitespace reformatting in services/api.ts
The diff reindents ~20 functions unrelated to this change (createTrade, lockTrade, getTrade, buyCETES, …) from 4-space to 2-space continuation. That balloons the diff, makes review harder, and — concretely — will collide with #321, which also edits api.ts. Please keep the diff scoped to just registerUser() + the new import.
The core change (collect phone → hash → send phone_hash) is good; these are about scoping and not overclaiming. Thanks!
|
Hi @AgilityB — light check-in, no rush. The changes I asked for are small: drop |
Okay. I will fix them |
…i-abuse controls Rebased AgilityB's PR Micopay#319 cleanly onto main: dropped stray merge artifacts from their branch (api_main.ts/api_main_clean.ts/api_original.ts/ "tash pop") and reapplied the real change (client-side SHA-256 phone hash, never sent in plaintext) on top of current main's api.ts/Register.tsx. Co-Authored-By: AgilityB <noreply@anthropic.com>
d938ee2 to
ddf5af3
Compare
Context
abuse.service.ts already reads users.phone_hash to block related-account abuse (assertNotRelatedAccounts — two accounts sharing the same phone_hash can't trade with each other). POST /users/register (routes/users.ts) already accepts an optional phone_hash and stores it if provided.
The problem: no frontend flow ever sends one. registerUser() in services/api.ts only posts { username, stellar_address } — no phone number, no hash. Register.tsx never asks for a phone number at all. So this anti-abuse control has been dormant since it was built — every user registers with phone_hash = NULL.
What this PR does
Adds a phone number step to registration and hashes it client-side before sending, so phone_hash actually gets populated going forward.
Three files changed:
micopay/frontend/src/lib/phoneHash.ts (new) — Client-side hashing utility using Web Crypto API (crypto.subtle.digest('SHA-256', ...)). Provides normalizePhone() (canonical digit form, preserves leading +) and hashPhone() (returns hex-encoded SHA-256).
micopay/frontend/src/services/api.ts (modified) — registerUser() now accepts an optional second parameter phoneHash?: string. When provided, it's included as phone_hash in the POST body. No backend changes needed — the schema already accepts this field.
micopay/frontend/src/pages/Register.tsx (modified) — Added an optional phone number input (type="tel") with a privacy notice explaining the hash is local. Before calling registerUser(), if the phone field is non-empty, it's hashed via hashPhone() — only the hash is sent over the wire.
Key design decisions
Phone field is optional (making it mandatory is a product/legal decision beyond this PR's scope — but even optional collection is strictly better than the current zero)
Raw number never leaves the device — the SHA-256 hash is computed client-side before any network call
No SMS/OTP verification (out of scope — this PR just collects + hashes; verifying ownership is a separate, larger feature)
No backend changes needed — the backend plumbing has existed since #82
Acceptance criteria
✅ Register.tsx collects a phone number during registration (optional)
✅ Phone number is hashed client-side before any network call — raw number never sent to backend
✅ registerUser() passes the computed phone_hash through to POST /users/register
✅ Existing assertNotRelatedAccounts abuse check starts seeing non-null phone_hash values for new registrations
✅ No raw phone number is logged or stored anywhere server-side
⬜ tsc --noEmit passes (dependencies not installed in this environment, but code follows existing patterns exactly)
Related
Activates the dormant anti-abuse control from abuse.service.ts's assertNotRelatedAccounts
Referenced as an open gap in #314's closing notes and in docs/GRANTFOX_KYC_QUEUE_2026-07.md
closes #318