Skip to content

refactor(settings): use typed properties in SensitiveDataClient - #21228

Merged
vpomerleau merged 1 commit into
mainfrom
fxa-10929
Sep 15, 2026
Merged

vpomerleau merged 1 commit into
mainfrom
fxa-10929

Conversation

@vbudhram

Copy link
Copy Markdown
Contributor

Because

  • SensitiveDataClient kept every value in one private object behind setDataType(key, value) and getDataType(key).
  • The generic accessors hid the type of each value behind the SensitiveData.Key enum and the SensitiveData.DataMap lookup type. A reader had to follow both to learn what a call returns.
  • The file already carried a TODO(FXA-10929) on KeyStretchUpgradeData that showed the wanted shape.

This pull request

  • Removes SensitiveData.Key, SensitiveData.DataMap, the private storage object, the constructor and both accessors from sensitive-data-client.ts.
  • Adds one typed public property per former key: AuthData, AccountResetData, NewRecoveryKeyData, Password and DecryptedRecoveryKeyData.
  • Keeps the SensitiveData data types. Four prop types still intersect SensitiveData.AuthData.
  • Updates the call sites to assign and read the properties directly.
  • Updates the specs to set the property on a real client instead of replacing a getter with jest.fn(). Two specs now use the shared mockSensitiveDataClient helper in place of an ad-hoc {getDataType, setDataType} object.

Storage semantics do not change. The client stores the same values, and it clears them at the same points.

Issue that this pull request solves

Closes: https://mozilla-hub.atlassian.net/browse/FXA-10929

Checklist

Put an x in the boxes that apply

  • My commit is GPG signed.
  • If applicable, I have modified or added tests which pass locally.
  • I have added necessary documentation (if appropriate).
  • I have verified that my changes render correctly in RTL (if appropriate).
  • I have manually reviewed all AI generated code.

How to review (Optional)

  • Key files/areas to focus on: packages/fxa-settings/src/lib/sensitive-data-client.ts holds the whole API change. The rest follows from it.
  • Suggested review order: the client first, then the containers, then the specs.
  • Risky or complex parts: SigninUnblock/container.tsx is the one non-mechanical spot. It keeps the previous non-null assertion, now on sensitiveDataClient.Password!. Every other edit is a direct swap of an accessor call for a property.

Screenshots (Optional)

There is no user interface change.

Other information (Optional)

29 files changed, 89 insertions and 206 deletions, all under packages/fxa-settings/src.

Local checks:

  • 13 touched spec files: 246 passed, 0 failed.
  • npx tsc -p packages/fxa-settings/tsconfig.json --noEmit: passes. This is the real gate. The change deletes an API, so a missed call site fails the compile.
  • npx nx lint fxa-settings: exits 0.

Conflict warning for FXA-13151 (passwordless Sync): PRs #21223 and #21224 add a PasskeyWrap key to this same file. After this pull request lands, that work must add PasskeyWrap as a property, not as an enum member. Expect a merge conflict.

## Because

- `SensitiveDataClient` kept every value in one private object behind `setDataType(key, value)` and `getDataType(key)`.
- The generic accessors hid the type of each value behind the `SensitiveData.Key` enum and the `SensitiveData.DataMap` lookup type. A reader had to follow both to learn what a call returns.
- The file already carried a `TODO(FXA-10929)` on `KeyStretchUpgradeData` that showed the wanted shape.

## This pull request

- Removes `SensitiveData.Key`, `SensitiveData.DataMap`, the private storage object, the constructor and both accessors from `sensitive-data-client.ts`.
- Adds one typed public property per former key: `AuthData`, `AccountResetData`, `NewRecoveryKeyData`, `Password` and `DecryptedRecoveryKeyData`.
- Keeps the `SensitiveData` data types. Four prop types still intersect `SensitiveData.AuthData`.
- Updates the call sites to assign and read the properties directly.
- Updates the specs to set the property on a real client instead of replacing a getter with `jest.fn()`. Two specs now use the shared `mockSensitiveDataClient` helper in place of an ad-hoc `{getDataType, setDataType}` object.

Storage semantics do not change. The client stores the same values, and it clears them at the same points.

## Issue that this pull request solves

Closes: https://mozilla-hub.atlassian.net/browse/FXA-10929
Copilot AI balanced review requested due to automatic review settings September 15, 2026 20:22
@vbudhram vbudhram added the auto label Sep 15, 2026
@vbudhram
vbudhram requested a review from a team as a code owner September 15, 2026 20:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No unresolved review comments remain, and all supplied assessments indicate approval readiness.

Pull request overview

Refactors SensitiveDataClient to use typed public properties while preserving existing storage behavior.

Changes:

  • Replaces generic getters/setters with typed properties.
  • Migrates signup, signin, recovery, and reset-password call sites.
  • Updates affected tests and mocks.
File summaries
File Change
packages/fxa-settings/src/pages/Signup/index.tsx Stores signup auth data directly.
packages/fxa-settings/src/pages/Signup/index.test.tsx Updates signup storage assertions.
packages/fxa-settings/src/pages/Signup/ConfirmSignupCode/container.tsx Reads typed auth data.
packages/fxa-settings/src/pages/Signup/ConfirmSignupCode/container.test.tsx Updates auth-data mocks.
packages/fxa-settings/src/pages/Signin/SigninUnblock/container.tsx Reads password and stores auth data.
packages/fxa-settings/src/pages/Signin/SigninUnblock/container.test.tsx Updates password mocks.
packages/fxa-settings/src/pages/Signin/SigninTotpCode/container.tsx Reads typed auth data.
packages/fxa-settings/src/pages/Signin/SigninTotpCode/container.test.tsx Resets typed mock state.
packages/fxa-settings/src/pages/Signin/SigninTokenCode/container.tsx Reads typed auth data.
packages/fxa-settings/src/pages/Signin/SigninTokenCode/container.test.tsx Updates auth-data mocks.
packages/fxa-settings/src/pages/Signin/SigninRecoveryPhone/container.tsx Reads typed auth data.
packages/fxa-settings/src/pages/Signin/SigninRecoveryCode/container.tsx Reads typed auth data.
packages/fxa-settings/src/pages/Signin/SigninRecoveryCode/container.test.tsx Updates recovery-code assertions.
packages/fxa-settings/src/pages/Signin/index.tsx Stores passwords directly.
packages/fxa-settings/src/pages/Signin/index.test.tsx Updates password assertions.
packages/fxa-settings/src/pages/Signin/container.tsx Stores sign-in auth data directly.
packages/fxa-settings/src/pages/Signin/container.test.tsx Updates sign-in assertions.
packages/fxa-settings/src/pages/Signin/components/SigninDecider/index.test.tsx Uses the shared client mock.
packages/fxa-settings/src/pages/ResetPassword/ResetPasswordWithRecoveryKeyVerified/container.tsx Reads reset and recovery-key data.
packages/fxa-settings/src/pages/ResetPassword/ResetPasswordConfirmed/container.tsx Reads reset data directly.
packages/fxa-settings/src/pages/ResetPassword/CompleteResetPassword/container.tsx Reads and stores reset data directly.
packages/fxa-settings/src/pages/ResetPassword/CompleteResetPassword/container.test.tsx Uses the shared client mock.
packages/fxa-settings/src/pages/ResetPassword/AccountRecoveryConfirmKey/container.tsx Stores decrypted recovery-key data.
packages/fxa-settings/src/pages/PostVerify/SetPassword/container.test.tsx Removes obsolete setter mocking.
packages/fxa-settings/src/pages/InlineRecoverySetupFlow/container.tsx Reads typed auth data.
packages/fxa-settings/src/pages/InlineRecoverySetupFlow/container.test.tsx Updates auth-data tests.
packages/fxa-settings/src/pages/InlineRecoveryKeySetup/container.tsx Reads typed auth data.
packages/fxa-settings/src/pages/InlineRecoveryKeySetup/container.test.tsx Updates recovery-key mocks.
packages/fxa-settings/src/lib/sensitive-data-client.ts Defines typed sensitive-data properties.
Review details
  • Files reviewed: 29/29 changed files
  • Comments generated: 0
  • Review effort level: Lite (auto)

Note

Copilot is running an experiment and ran this review at Lite.


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@vpomerleau
vpomerleau merged commit d4f3f35 into main Sep 15, 2026
22 checks passed
@vpomerleau
vpomerleau deleted the fxa-10929 branch September 15, 2026 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants