feat(accounts): ID verification Phase 0 — evidence, reviewer role, retention - #500
Open
islandbitcoin wants to merge 2 commits into
Open
feat(accounts): ID verification Phase 0 — evidence, reviewer role, retention#500islandbitcoin wants to merge 2 commits into
islandbitcoin wants to merge 2 commits into
Conversation
…tention Backend half of the ID-verification tool (docs/id-verification.md). - Upgrade mutations accept optional structured `evidence` ([UpgradeEvidenceInput!] with UpgradeEvidenceType); legacy `idDocument` keeps working unchanged and is folded in as an ID_FRONT row. Pure domain normalize/validate in src/domain/accounts/upgrade-evidence.ts; strict identity requirement (ID_FRONT+SELFIE or approved BRIDGE_KYC for L2/L3) behind UPGRADE_EVIDENCE_STRICT (default false). - Bridge KYC: BRIDGE_KYC rows require an approved Bridge KYC; the Bridge customer is snapshotted (id/status/updated_at/endorsements only), never blocking the request. - ERPNext: companion "ID Verification" document POSTed after the Account Upgrade Request (ErpNext.postIdVerification). Failure is warn-logged and non-fatal; id_document still carries the first ID_FRONT key. - Admin authz: "Flash Admin" recognised and allowed only on accountDetailsByUserPhone, idDocumentReadUrl and accountUpdateLevel via an explicit allowlist (src/servers/authorization/admin-permissions.ts); everything else keeps System Manager / Accounts Manager. - PII: drop the console.log dumping upgrade requests from the public accountUpgradeRequest query; postUpgradeRequest error log no longer spreads the request body. - Retention: deleteIdDocument/listIdDocuments on storage (id_documents/ only); pure policy in src/domain/accounts/evidence-retention.ts (7y default, EVIDENCE_RETENTION_YEARS; Approved → closed-at + N years, Rejected/Closed → decided-at + N years; closed-at derived from statusHistory); cron task runEvidenceRetention, dry-run by default (EVIDENCE_RETENTION_DRY_RUN=true). - Env booleans for the new flags parse "false"/"0"/"no"/"off" as false. - SDL regenerated (public schema + supergraph: additions only). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01W5e4H8MF7oX2LPUxBRW3Pf
…ip check Code review on PR #500: - upgrade-evidence.ts: `startsWith(id_documents/<username>_)` had no delimiter boundary, so a user whose username is an exact prefix of another user's (`alice` vs. `alice_smith`) could pass the ownership check for the other account's uploaded file (usernames may contain `_`). Switch the username/filename separator to `/`, which UsernameRegex forbids in usernames, so the prefix can no longer be satisfied by a longer username. Updated the matching key generation in services/storage and the format documented in docs/id-verification.md. - IdVerification.fromErpnext: log a warning (doc name, upgrade request, row, raw value) when an unrecognized evidence_type falls back to id_front, instead of silently reclassifying it. - Added regression tests pinning the `alice` vs `alice_smith` prefix- collision shape in both upgrade-evidence.spec.ts and upgrade-request-evidence.spec.ts, and tests for the new warning log (and its absence on a recognized type) in IdVerification.spec.ts. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EtQzyzQ38thfC2F8BLK4u8
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 0 (backend side) of the Level 2 / Level 3 ID-verification tool. Companion ERPNext PR:
feat/id-verification-phase0in lnflash/frappe-flash-admin — deploy that first so theID Verificationdoctype exists before this starts posting to it (the POST is non-fatal if it doesn't, so ordering is about data, not outages).What this adds
Flash Adminwere rejected by the admin shield (System Manager / Accounts Manager only), soaccountUpdateLevelfailed for them. Newsrc/servers/authorization/admin-permissions.tsbuilds the shield from an explicit allowlist:Flash Adminis accepted onaccountDetailsByUserPhone,idDocumentReadUrlandaccountUpdateLevelonly; every other admin field keeps the existing rule. Tests evaluate the real graphql-shield rules for allowed and denied fields.evidence: [UpgradeEvidenceInput!](ID_FRONT,ID_BACK,SELFIE,LIVENESS_FRAME,BUSINESS_REGISTRATION,TRN,PROOF_OF_ADDRESS,BRIDGE_KYC). LegacyidDocumentkeeps working and is normalised to oneID_FRONTrow. Strict rule (L2/L3 need ID front + selfie, or approved Bridge KYC) sits behindUPGRADE_EVIDENCE_STRICT(default false) until the new mobile capture ships. Pure domain logic insrc/domain/accounts/upgrade-evidence.ts.BRIDGE_KYCrow is accepted (or synthesised when no evidence is sent) and a minimal customer snapshot (id, status, updated_at, endorsements) is recorded; snapshot failure never blocks the request.postUpgradeRequest,ErpNext.postIdVerificationcreates theID Verificationdoc with evidence child rows. Failure → warn log, user request still succeeds; the legacyid_documentkey is still written so nothing regresses.console.logof the full upgrade request in the public query, and a second leak inpostUpgradeRequest's error path that spread name/phone/email/address into the log.deleteIdDocument/listIdDocumentsin the storage service (refuse anything outsideid_documents/).src/domain/accounts/evidence-retention.ts: 7-year policy (EVIDENCE_RETENTION_YEARS), two clocks — approved: 7y after account closure (derived fromstatusHistory; reopened accounts restart the clock;lockedis not closure), rejected/closed: 7y afterreviewed_at. CronevidenceRetentionJobinsrc/servers/cron.ts,EVIDENCE_RETENTION_DRY_RUNdefault true for the first release."false"→ false); the existingz.coerce.boolean()pattern was left untouched elsewhere.docs/id-verification.md.Tests
yarn tsc-check,yarn eslint-checkclean.yarn write-sdl: additions only (UpgradeEvidenceType,UpgradeEvidenceInput,evidenceon both inputs); admin schema unchanged.Not in this PR
Mobile capture (document picker, front/back, selfie) — next. When it sends
documentType/issuingCountry, they must be the ERPNext registry code and the Frappe Country name, since both are Link fields.🤖 Generated with Claude Code
https://claude.ai/code/session_01W5e4H8MF7oX2LPUxBRW3Pf