Tech Spec: Backup code and account recovery - #3843
Conversation
Covers four work areas: change backup code, backup code recovery via email OTP, periodic reminders, and email modification flow changes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add source tracking to Area 1, missing recovery-from-reminder event to Area 2, and a full analytics section for Area 4. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fragment hierarchy and nav graph flow diagrams for both backup code and phone verification refactors. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe design document defines PersonalID backup-code management across profile, signup, recovery, reminder, and email-change flows. It adds shared backup-code and phone-verification fragment contracts, email OTP handling, activity-scoped profile state, a backup-code update API, reminder scheduling, recovery behavior, verification gates, analytics events, and updated signup messaging. Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@docs/superpowers/specs/2026-07-28-personalid-backup-code-management-design.md`:
- Line 15: Update the Markdown in the design document by adding the text
language identifier to the ASCII diagram fences at the referenced locations and
removing the extra space in the heading near the later diagram. Preserve the
diagram contents and heading text while resolving MD040 and MD019.
- Around line 175-180: Update PersonalIdProfileActivityViewModel to define
currentCode lifecycle rules: bind it to the active user or session, clear it
immediately after a successful authenticated operation, and also clear it on
cancellation, lockout, logout, and account changes. Ensure stale backup
credentials cannot be reused across users or sessions.
- Around line 165-173: Expand the PersonalIdSendEmailOtpFragment specification
and its send_email_otp contract to define server-enforced OTP expiration,
single-use consumption, resend throttling, verification-attempt limits, and
binding to the account, email, and EmailWorkFlow. Document these controls as
authoritative server behavior, including the failure responses clients must
handle; do not rely on the client-side resend timer.
- Around line 165-173: Update the PersonalIdSendEmailOtpFragment navigation
contract so EmailWorkFlow is passed to PersonalIdEmailVerificationFragment
alongside the existing OTP navigation. Define the destination argument and
ensure the workflow is restored through the established navigation or
shared-state mechanism after recreation, allowing verification to select the
correct completion path.
- Around line 40-62: Update the email-change flow specification around the
phone-verification branch and the backup-code-gated branch to define and retain
pendingEmail with the newly entered address before navigation begins, ensuring
the no-email path can use it during the later email OTP step.
- Around line 83-94: Update the backup-code fragment hierarchy documentation and
related workflow contracts so each workflow/navigation graph has one unambiguous
authentication contract. Separate credential type from credential source for
changes, profile recovery, and session recovery, avoid using ambiguous
ProvidedAuth(userId, currentCode), and make the documented
/users/set_backup_code contract match the PersonalID API’s TokenAuth behavior
for CONFIRM_RECOVERY.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c0426239-226a-416a-82c0-bfbe1e3e11a6
📒 Files selected for processing (1)
docs/superpowers/specs/2026-07-28-personalid-backup-code-management-design.md
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3843 +/- ##
============================================
- Coverage 27.29% 26.84% -0.46%
+ Complexity 4773 4704 -69
============================================
Files 991 988 -3
Lines 59019 58856 -163
Branches 7025 7016 -9
============================================
- Hits 16111 15797 -314
- Misses 40974 41146 +172
+ Partials 1934 1913 -21 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…design spec Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
conroy-ricketts
left a comment
There was a problem hiding this comment.
Thanks for clearly separating out the implementation reference for Claude! That made this a lot easier to review and is a good model for specs going forward
| All CommCare launching activities (e.g. `ConnectActivity`, `LoginActivity`, `StandardHomeActivity`) check for due reminders on resume. | ||
| If due, show the reminder dialog. |
There was a problem hiding this comment.
My main concern here is that we keep triggering these dialog prompts directly inside the Activity. This could lead to multiple dialogs popping up at the same time—for example, if a user doesn't have an email set, they get prompted to add one, but they might simultaneously trigger a prompt to update their backup code.
We should consider establishing a priority queue or hierarchy for these dialogs so only one is presented to the user at a time.
There was a problem hiding this comment.
Agree. can you remind me if we have the email pop-up already on launch screens ?
…ackupCodeWorkflow enum Both email-change paths (has email and no email) now start with Confirm Backup Code. For the add-email path, Forgot? routes to Phone OTP rather than blocking the user. CONFIRM_BACKUP_CODE split into three distinct workflow values to make routing explicit. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
calellowitz
left a comment
There was a problem hiding this comment.
One real concern about the flow, and a few smaller questions
|
|
||
| ### API Changes | ||
|
|
||
| #### New: `POST /users/set_backup_code` |
There was a problem hiding this comment.
Existing workflows require confirmation of newly set backup codes to ensure the user entered the backup code they intended (and know what they put in). Are we intentionally removing that step?
There was a problem hiding this comment.
we are not, the UI on this page presents users with 2 input fields for user to verify the backup code before we make the API call.
| │ Email OTP entry ◄ Profile (success) | ||
| │ │ | ||
| │ ▼ | ||
| │ Set new backup code |
There was a problem hiding this comment.
why do we allow setting a new backup code before biometric/PIN unlock? That allows them to change the profile info, before they are fully authenticated. Setting a backup code isn't really part of the authentication process in this flow, but is happening part way through. If we moved it to the end, after the success, we would only need to allow logged in users to change the code. Rather than users part way through a recovery and would therefore only need to support one authentication method (and one view that already exists).
There was a problem hiding this comment.
Ahh the flow diagram makes it a bit confusing but the Biometric unlock is the pop on the same page that happens before we make the API call. So the order is more like this -
- User enters and confirms backup code on "Set New Backup Code Page"
- We show the biometric unlock pop on the same page. If failed, the only option user have is to press back from here.
- If success, we make the API call to set the backup code
Rather than users part way through a recovery and would therefore only need to support one authentication method (and one view that already exists).
We do need to support setting a new back up code during Account recovery /configuration flow otherwise how will we let users signing on a device but who have forgotten their backup code recover their accounts ?
There was a problem hiding this comment.
how will we let users signing on a device but who have forgotten their backup code recover their accounts
We will set it after they successfully recover their account. If they have recovered their account by other means, they will have a password for basic auth. If they have not finished recovering, it is not safe to allow them to modify the authentication methods for the account.
…gn-in - Account configuration forgot flow: email OTP now signs user in and then prompts to set a new backup code, rather than setting backup code without authentication - set_recovery_pin (existing endpoint, ProvidedAuth only) replaces the planned new set_backup_code endpoint; TokenAuth removed from SET_NEW_CODE - Split EmailWorkFlow.BACKUP_CODE_RECOVERY into BACKUP_CODE_RECOVERY_SIGN_IN (account config graph) and BACKUP_CODE_RECOVERY_SET_CODE (profile graph) - Add explicit server team request: account recovery via email OTP endpoint and email surfacing in start_configuration vs check_name Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
eeeef49
Jira
Design Doc
Reviewer Notes: