Skip to content

fix: increase password length to 128 chars#3141

Merged
CharlVS merged 4 commits intodevfrom
cursor/fix-password-validation-bug-1868
Sep 16, 2025
Merged

fix: increase password length to 128 chars#3141
CharlVS merged 4 commits intodevfrom
cursor/fix-password-validation-bug-1868

Conversation

@CharlVS
Copy link
Copy Markdown
Collaborator

@CharlVS CharlVS commented Sep 12, 2025

Remove 40-character input limits from password fields to prevent truncation and ensure consistent validation of long passwords.


Open in Cursor Open in Web

Co-authored-by: charl <charl@vanstaden.info>
@cursor
Copy link
Copy Markdown

cursor bot commented Sep 12, 2025

Cursor Agent can help with this pull request. Just @cursor in comments and I'll start working on changes in this branch.
Learn more about Cursor Agents

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Sep 12, 2025

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

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.

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

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch cursor/fix-password-validation-bug-1868

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.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Sep 12, 2025

Visit the preview URL for this PR (updated for commit e8306b6):

https://walletrc--pull-3141-merge-8xgadwgu.web.app

(expires Tue, 23 Sep 2025 13:40:48 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: f66a4ff03faa546f12f0ae5a841bd9eff2714dcc

@CharlVS CharlVS marked this pull request as ready for review September 12, 2025 21:06
@smk762
Copy link
Copy Markdown
Collaborator

smk762 commented Sep 14, 2025

Unrestricted length inputs frighten me. Suggest a permissive, yet sane limit of 1024.

I consulted an agent for recommendations, pasted below:


I pulled guidance from RFC 9106 and standards (NIST, OWASP) to answer precisely.

  • Short answer: Argon2id itself accepts passwords up to 2^32−1 bytes (~4.29 GB). For production services, standards recommend allowing at least 64 characters without truncation; a practical cap of 128 characters is commonly recommended and aligns with best practice.

Why:

  • Algorithmic max (not a practical limit): Argon2 per RFC 9106 supports inputs up to 2^32−1 bytes.
  • Usability/security guidance: Verifiers should permit at least 64 characters and not truncate; many orgs choose 128 as a safe, user-friendly cap.
  • DoS considerations: Extremely long inputs can be abused; a sane upper bound (e.g., 128–256) mitigates risk while not weakening security.

Recommendation for this app (after removing the 40-char limit in the PR):

  • Accept and hash passwords up to 128 characters (no truncation), with client and server-side validation consistent.

References:

I reviewed the standards and propose setting a 128-character cap with no truncation to align with NIST/OWASP while preventing DoS from excessively long inputs.


@CharlVS
Copy link
Copy Markdown
Collaborator Author

CharlVS commented Sep 15, 2025

@cursoragent, increase the max password character limit to 128 throughout the app. Include a TODO about adding the same changes to the SDK.

@cursor
Copy link
Copy Markdown

cursor bot commented Sep 15, 2025

I standardized the maximum password length across the application by introducing a new constant and applying it to all relevant password input fields. This ensures consistent validation and user experience for password entry throughout the app.

  • I added a const int passwordMaxLength = 128; to lib/shared/constants.dart to define a global maximum password length.
  • I updated numerous TextFormField widgets across the application to enforce this new passwordMaxLength. This includes password fields in TrezorDialogSelectWallet, PasswordDialogContent, PasswordUpdatePage, CreationPasswordFields, WalletDeleting, WalletImportByFile, and WalletLogIn.
  • For all updated TextFormField instances, I also set counterText: '' to hide the default character counter, providing a cleaner UI.

4 tasks completed

  • Add passwordMaxLength=128 to shared constants
  • Apply maxLength to all password entry fields across app
  • Add TODO note to mirror changes in SDK
  • Run linter on edited files and fix issues
Open in Cursor Open in Web

Learn more about Cursor Agents

Co-authored-by: charl <charl@vanstaden.info>
@CharlVS CharlVS requested a review from smk762 September 15, 2025 10:54
@CharlVS CharlVS changed the title Fix password validation bug fix: increase password length to 128 chars Sep 15, 2025
Copy link
Copy Markdown
Collaborator

@smk762 smk762 left a comment

Choose a reason for hiding this comment

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

Confirm has hard limit of 128 char ✔️
An edge case exists, where if user enters a longer password, there is no visual feedback to indicate that the limit has been exceeded. This may result in users with excessively long passwords to be unaware that the excess chars were not included.

This is P2: low probability occurrence with mid-level UX impact. It shouldn't lead to loss of funds - input on login in similarly constrained, and could be resolved with a simple support query if needed.

Content to approve in current state and delegate the above comment to an issue if resolution would further delay release.

@CharlVS CharlVS requested a review from Copilot September 16, 2025 12:50
@CharlVS
Copy link
Copy Markdown
Collaborator Author

CharlVS commented Sep 16, 2025

@cursor review

@CharlVS CharlVS self-assigned this Sep 16, 2025
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR increases the maximum password length from 40 to 128 characters across all password input fields in the application to prevent truncation and ensure consistent validation of long passwords.

  • Removes LengthLimitingTextInputFormatter(40) from password fields
  • Adds a centralized passwordMaxLength constant set to 128 characters
  • Updates all password input fields to use the new length limit with hidden counter text

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
lib/shared/constants.dart Adds new passwordMaxLength constant set to 128 characters
lib/views/wallets_manager/widgets/wallet_login.dart Updates password field to use new length limit
lib/views/wallets_manager/widgets/wallet_import_by_file.dart Updates password field to use new length limit
lib/views/wallets_manager/widgets/wallet_deleting.dart Updates password field to use new length limit
lib/views/wallets_manager/widgets/creation_password_fields.dart Removes old 40-character limit and applies new 128-character limit
lib/views/settings/widgets/security_settings/password_update_page.dart Removes old 40-character limit and applies new 128-character limit
lib/views/common/wallet_password_dialog/password_dialog_content.dart Removes old 40-character limit and applies new 128-character limit
lib/views/common/hw_wallet_dialog/trezor_steps/trezor_dialog_select_wallet.dart Updates passphrase field to use new length limit

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

cursor[bot]

This comment was marked as outdated.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@CharlVS
Copy link
Copy Markdown
Collaborator Author

CharlVS commented Sep 16, 2025

Confirm has hard limit of 128 char ✔️ An edge case exists, where if user enters a longer password, there is no visual feedback to indicate that the limit has been exceeded. This may result in users with excessively long passwords to be unaware that the excess chars were not included.

This is P2: low probability occurrence with mid-level UX impact. It shouldn't lead to loss of funds - input on login in similarly constrained, and could be resolved with a simple support query if needed.

Content to approve in current state and delegate the above comment to an issue if resolution would further delay release.

@smk762 Please do create an issue.

@CharlVS CharlVS merged commit fed5f3a into dev Sep 16, 2025
7 of 13 checks passed
@CharlVS CharlVS deleted the cursor/fix-password-validation-bug-1868 branch September 16, 2025 13:37
@CharlVS CharlVS mentioned this pull request Oct 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants