Skip to content

Mobile: Fixes #14975: Add missing validations for setting master password and encryption passwords#14983

Draft
mrjo118 wants to merge 25 commits intolaurent22:devfrom
mrjo118:encryption-issues-mobile
Draft

Mobile: Fixes #14975: Add missing validations for setting master password and encryption passwords#14983
mrjo118 wants to merge 25 commits intolaurent22:devfrom
mrjo118:encryption-issues-mobile

Conversation

@mrjo118
Copy link
Copy Markdown
Contributor

@mrjo118 mrjo118 commented Apr 2, 2026

As per #14975, it is possible to face issues with items being unable to be decrypted when entering incorrect encryption passwords on the mobile app, due to lack of necessary validations which exist on the desktop app.

This PR addresses these issues by adding the relevant validations. This includes:

  • Ensuring password entry for individual keys will validate against the master password, when the master password has not been set
  • Ensuring password entry for the master password is validated against the active master key on the sync target
  • Ensuring when enabling encryption, there is password entry and confirmation when creating a master password, but there is just one password input when matching to an existing master password, and this will validate against the existing master password when sync has been setup for the client

Other issues observed during testing, which are not addressed in this PR:

  • When multiple master keys exist upon syncing with a sync target for initial device setup, entering the encryption key password or master password does not automatically enable encryption. I've seen this happen on the desktop app as well after changing the master password and incorrectly entering the new password on the first attempt, but this is comparitively a minor issue, which is fixed by manually re-enabling encryption
  • If the user enables encryption with a different encryption password prior to syncing with an existing target, they will be prompted for the master password on the encryption config screen. The validation now works correctly when entering this password, however when entering the correct password for the encryption on the sync target, the items still cannot be decrypted, unless disabling encryption (can then be re-enabled if desired) and then choosing the retry all option via the link in the some items cannot be decrypted banner. This however results in all items needing to be re-synced. I will raise a separate issue for this.

This fixes #14975

Testing

See video demonstrating the main flows (creating a master key locally, entering master password for syncing with an existing target, and validating password is validated when disabling and re-enabling encryption after a profile has been synced):

1NdPaKxKmp.mp4

Part 2 - testing the scenario where a user has setup encryption with an incorrect password before setting up sync:
https://github.com/user-attachments/assets/f80c522b-6d99-4e63-944b-b6d41e98d8da

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 2, 2026

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 99b393f5-ed62-4fd3-b713-ab02f03399af

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Updated the mobile encryption configuration to validate entered master passwords against the active master key (via masterPasswordIsValid) and to conditionally require confirm-password only when creating a new password (shouldCreatePassword derived from localSyncInfo().ppk), preventing invalid password saves.

Changes

Cohort / File(s) Summary
Encryption Configuration
packages/app-mobile/components/screens/encryption-config.tsx
Replaced usage of SyncInfo with localSyncInfo().ppk. Added masterPasswordIsValid validation in the master-key Save handler and in the "Enable encryption" flow. Introduced shouldCreatePassword branch: when true, require and validate confirm-password; when false, skip confirm and validate the entered password against the active master key. Confirm-password inputs are conditionally rendered only when creating a new password.

Possibly related issues

Suggested reviewers

  • Kaushalendra-Marcus
🚥 Pre-merge checks | ✅ 6
✅ Passed checks (6 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding missing validations for master password and encryption passwords on mobile to fix issue #14975.
Linked Issues check ✅ Passed The PR successfully implements all primary coding objectives: validating individual key passwords against master passwords, validating master password entry against active master keys, and conditionally requiring password confirmation based on whether a master password is being created or matched.
Out of Scope Changes check ✅ Passed All changes in encryption-config.tsx are directly scoped to the linked issue requirements; no unrelated modifications were introduced outside the password validation objectives.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Pr Description Must Follow Guidelines ✅ Passed The pull request description includes all three required sections: a clear problem statement explaining user impact, a high-level solution explanation detailing validations implemented, and testing information.
Description check ✅ Passed The pull request description clearly relates to the changeset, explaining the purpose of adding password validations to the encryption configuration on the mobile app to prevent items from becoming undecryptable.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai bot added bug It's a bug mobile All mobile platforms labels Apr 2, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
packages/app-mobile/components/screens/encryption-config.tsx (1)

177-178: Consider handling the case where the active master key is not found.

props.masterKeys.find(...) may return undefined if activeMasterKeyId doesn't match any key in the array. Whilst masterPasswordIsValid falls back to getDefaultMasterKey() internally, this silent fallback could validate against a different key than the user expects, potentially masking a misconfiguration.

Suggestion: explicit handling
-} else if (!(await masterPasswordIsValid(password, props.masterKeys.find(mk => mk.id === props.activeMasterKeyId)))) {
+} else {
+	const activeMasterKey = props.masterKeys.find(mk => mk.id === props.activeMasterKeyId);
+	if (!activeMasterKey) {
+		// Falls back to getDefaultMasterKey() inside masterPasswordIsValid - this is expected behaviour
+	}
+	if (!(await masterPasswordIsValid(password, activeMasterKey))) {
+		throw new Error(_('Invalid password. Please try again. If you have forgotten your password you will need to reset it.'));
+	}
+}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/app-mobile/components/screens/encryption-config.tsx` around lines
177 - 178, The code calls masterPasswordIsValid(..., props.masterKeys.find(mk =>
mk.id === props.activeMasterKeyId)) which can pass undefined when the active
master key isn't found; instead explicitly resolve the active master key first
(e.g., const activeKey = props.masterKeys.find(...)), and if activeKey is
undefined throw a clear, specific error (or surface a UI message) before calling
masterPasswordIsValid so you don't silently fall back to getDefaultMasterKey();
update the block around masterPasswordIsValid and
props.activeMasterKeyId/props.masterKeys to perform this explicit check and
error path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/app-mobile/components/screens/encryption-config.tsx`:
- Around line 107-113: The onSave handler calls masterPasswordIsValid(password,
mk) which throws Error('Password is empty') when password is falsy, so clicking
Save with an empty field causes an unhandled rejection; fix by guarding the
empty-password case or catching exceptions: in the onSave function (the handler
that currently awaits masterPasswordIsValid) first validate that password is
non-empty and show the alert, or wrap the await masterPasswordIsValid(...) call
in a try/catch and show the same _('Password is invalid. Please try again.')
alert on error, then only call onSavePasswordClick(mk, inputPasswords) when
validation succeeds.

---

Nitpick comments:
In `@packages/app-mobile/components/screens/encryption-config.tsx`:
- Around line 177-178: The code calls masterPasswordIsValid(...,
props.masterKeys.find(mk => mk.id === props.activeMasterKeyId)) which can pass
undefined when the active master key isn't found; instead explicitly resolve the
active master key first (e.g., const activeKey = props.masterKeys.find(...)),
and if activeKey is undefined throw a clear, specific error (or surface a UI
message) before calling masterPasswordIsValid so you don't silently fall back to
getDefaultMasterKey(); update the block around masterPasswordIsValid and
props.activeMasterKeyId/props.masterKeys to perform this explicit check and
error path.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 0ba2f231-1804-49c9-b817-efe2c7842e1e

📥 Commits

Reviewing files that changed from the base of the PR and between fd9f6c1 and fde26ea.

📒 Files selected for processing (1)
  • packages/app-mobile/components/screens/encryption-config.tsx

@mrjo118 mrjo118 marked this pull request as draft April 5, 2026 00:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug It's a bug mobile All mobile platforms

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Mobile: Entering the wrong password in encryption config causes local notes to be encryted with a corrupt or non-existent encryption key

1 participant