Repository navigation
Change password - #134
Change password#134bordalix wants to merge 4 commits into
Conversation
WalkthroughThe changes introduce a "Change Password" feature in the wallet application's settings. This includes a new settings option and UI for changing the password, validation of the old password, and support for biometric password registration. Supporting code includes new logic for password validation, updates to settings options, and UI adjustments for icons and text. Minor refactoring and renaming were performed for clarity in the service worker. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SettingsScreen
participant PasswordComponent
participant PrivateKeyLib
participant WalletContext
participant BiometricLib
User->>SettingsScreen: Select "Change Password"
SettingsScreen->>PasswordComponent: Render Password UI
User->>PasswordComponent: Enter old password
PasswordComponent->>PrivateKeyLib: isValidPassword(oldPassword)
PrivateKeyLib-->>PasswordComponent: true/false
alt Valid password
PasswordComponent->>User: Show new password input
User->>PasswordComponent: Enter new password
PasswordComponent->>PrivateKeyLib: getPrivateKey(oldPassword)
PrivateKeyLib-->>PasswordComponent: privateKey
PasswordComponent->>PrivateKeyLib: setPrivateKey(newPassword, privateKey)
PrivateKeyLib-->>PasswordComponent: success/failure
PasswordComponent->>WalletContext: Update password state
PasswordComponent->>User: Show success
opt Biometric supported and selected
PasswordComponent->>BiometricLib: registerBiometric()
BiometricLib-->>PasswordComponent: passkeyId
PasswordComponent->>WalletContext: Save biometric info
end
else Invalid password
PasswordComponent->>User: Show error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches
🧪 Generate 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
public/wallet-service-worker.mjs(53 hunks)src/components/NeedsPassword.tsx(2 hunks)src/icons/Passkey.tsx(0 hunks)src/lib/privateKey.ts(1 hunks)src/lib/types.ts(1 hunks)src/providers/options.tsx(1 hunks)src/screens/Init/Password.tsx(2 hunks)src/screens/Settings/Index.tsx(2 hunks)src/screens/Settings/Password.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- src/icons/Passkey.tsx
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: bordalix
PR: arkade-os/wallet#114
File: src/lib/asp.ts:0-0
Timestamp: 2025-06-30T18:33:29.839Z
Learning: In src/lib/asp.ts, only the collaborativeExit function should accept both IWallet and ServiceWorkerWallet types. Other wallet functions (getBalance, getTxHistory, getReceivingAddresses, redeemNotes, sendOnChain, settleVtxos) should maintain their original IWallet-only signatures and not be updated for consistency.
📚 Learning: in src/screens/init/restore.tsx, the input component for private key restoration intentionally omits...
Learnt from: bordalix
PR: arkade-os/wallet#121
File: src/screens/Init/Restore.tsx:58-58
Timestamp: 2025-07-21T18:25:36.391Z
Learning: In src/screens/Init/Restore.tsx, the Input component for private key restoration intentionally omits the `value` prop to make it uncontrolled. This is a security feature to prevent the private key from being displayed if users navigate away and return to the screen, avoiding potential exposure of sensitive data in the UI.
Applied to files:
src/screens/Init/Password.tsxsrc/components/NeedsPassword.tsxsrc/screens/Settings/Index.tsxsrc/screens/Settings/Password.tsxsrc/providers/options.tsx
📚 Learning: in src/lib/asp.ts, only the collaborativeexit function should accept both iwallet and serviceworkerw...
Learnt from: bordalix
PR: arkade-os/wallet#114
File: src/lib/asp.ts:0-0
Timestamp: 2025-06-30T18:33:29.839Z
Learning: In src/lib/asp.ts, only the collaborativeExit function should accept both IWallet and ServiceWorkerWallet types. Other wallet functions (getBalance, getTxHistory, getReceivingAddresses, redeemNotes, sendOnChain, settleVtxos) should maintain their original IWallet-only signatures and not be updated for consistency.
Applied to files:
src/components/NeedsPassword.tsxsrc/lib/privateKey.tspublic/wallet-service-worker.mjs
📚 Learning: do not review `public/wallet-service-worker.mjs` as it is a built file generated by vite....
Learnt from: bordalix
PR: arkade-os/wallet#114
File: public/wallet-service-worker.mjs:6350-6353
Timestamp: 2025-06-30T18:23:34.163Z
Learning: Do not review `public/wallet-service-worker.mjs` as it is a built file generated by Vite.
Applied to files:
public/wallet-service-worker.mjs
📚 Learning: in src/lib/bolt11.ts, the code is copied from the original bitcoinjs/bolt11 library (https://github....
Learnt from: bordalix
PR: arkade-os/wallet#106
File: src/lib/bolt11.ts:153-155
Timestamp: 2025-07-07T14:41:35.991Z
Learning: In src/lib/bolt11.ts, the code is copied from the original bitcoinjs/bolt11 library (https://github.com/bitcoinjs/bolt11/blob/master/payreq.js) and should be kept unchanged to maintain compatibility with the upstream source, even if there are potential improvements or syntax issues.
Applied to files:
public/wallet-service-worker.mjs
🪛 Biome (2.1.2)
src/screens/Settings/Password.tsx
[error] 2-2: Do not shadow the global "Error" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🪛 ESLint
public/wallet-service-worker.mjs
[error] 1084-1084: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 1087-1087: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 1122-1122: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 1124-1124: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 1126-1126: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 1135-1135: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 1135-1135: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 1135-1135: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 1144-1144: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 1146-1146: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 1150-1150: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 1154-1154: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 1233-1233: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 1253-1253: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 1293-1293: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 9447-9447: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 9992-9992: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 9992-9992: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 11076-11076: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 11413-11413: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 11413-11413: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 11426-11426: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 11431-11431: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 11435-11435: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 11435-11435: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 11435-11435: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 11439-11439: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 11840-11840: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 11840-11840: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 11841-11841: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 11841-11841: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 11841-11841: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 11843-11843: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 11893-11893: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 11902-11902: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 11902-11902: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 11902-11902: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 11902-11902: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 11902-11902: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 12057-12057: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 12057-12057: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 12058-12058: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 12337-12337: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 12518-12518: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 12518-12518: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 12525-12525: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 12554-12554: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 12585-12585: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 12585-12585: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 12585-12585: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 12587-12587: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 12587-12587: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 12587-12587: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 12587-12587: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 12630-12630: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 12630-12630: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 12631-12631: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 12631-12631: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 12632-12632: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 12633-12633: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 12635-12635: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 12808-12808: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 12820-12820: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 12823-12823: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 12828-12828: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 12830-12830: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 12835-12835: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 12984-12984: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 12991-12991: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 12991-12991: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 12992-12992: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 12993-12993: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 13213-13213: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 13213-13213: Strings must use singlequote.
(@typescript-eslint/quotes)
[error] 13213-13213: Strings must use singlequote.
(@typescript-eslint/quotes)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cloudflare Pages: arkade-wallet
🔇 Additional comments (15)
src/lib/privateKey.ts (2)
48-48: LGTM!The success logging is appropriate and doesn't expose sensitive information.
54-61: LGTM! Well-implemented password validation function.The
isValidPasswordfunction correctly validates passwords by attempting to decrypt the private key and returns a boolean result without exposing sensitive information. The implementation properly handles errors by returning false instead of throwing.src/lib/types.ts (1)
52-52: LGTM! Improved enum value clarity.The change from
'password'to'change password'makes the settings option more descriptive and user-friendly.src/providers/options.tsx (1)
92-96: LGTM! Consistent option addition.The new Password option follows the established pattern with an empty icon (consistent with other config options) and appropriate placement in the Advanced section.
src/screens/Init/Password.tsx (3)
15-15: LGTM! Icon import update for consistency.The change from PasskeyIcon to LockIcon provides better visual consistency across the application.
55-55: LGTM! Consistent icon usage.Using LockIcon with the
bigprop maintains visual consistency with the updated import.
60-60: LGTM! Improved text clarity.Removing "username or" from the description accurately reflects that only password authentication is bypassed with biometrics, making the text more precise.
src/screens/Settings/Index.tsx (2)
20-20: LGTM! Clean component import.The Password component import follows the established pattern for settings components.
43-43: LGTM! Consistent conditional rendering.The conditional rendering for the Password component follows the same pattern as other settings options.
src/components/NeedsPassword.tsx (1)
14-14: LGTM! Clean icon standardization.The replacement of
PasskeyIconwithLockIconand text simplification maintains the same functionality while standardizing the biometric unlock UI across the application.Also applies to: 38-39
src/screens/Settings/Password.tsx (4)
26-32: Good password validation pattern with proper error handling.The useEffect hook properly validates the old password asynchronously and updates the authentication state. The approach of using
isValidPasswordto verify the old password before allowing changes is a solid security practice.
41-48: Good biometric registration flow with proper wallet state updates.The biometric registration properly updates the wallet state and chains the password saving operation. The error handling with
consoleLogis appropriate for biometric failures.
55-55: Clean conditional rendering logic.The early return pattern for unauthenticated state is clean and follows good React practices. The condition properly handles both authentication and success states.
70-77: Smart conditional button rendering.The button logic correctly:
- Hides buttons when showing success message
- Disables continue button when no new password is entered
- Conditionally shows biometric option based on support and current state
public/wallet-service-worker.mjs (1)
1-13216: Skip review of generated build fileThis file appears to be a minified/built output generated by Vite. According to established practices, built files should not be manually edited or reviewed. Any necessary changes should be made to the source files instead.
|
I thin biometrics should be a dedicated row in settings. Or you did it this way to preserve the "either passowed or biomtric loging, not both" ? I guess we can do an iteration with design after riga to be more consistent UX |
Deploying arkade-wallet with
|
| Latest commit: |
4eb3e23
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://cecee983.arkade-wallet.pages.dev |
| Branch Preview URL: | https://change-password.arkade-wallet.pages.dev |
Deploying wallet-for-riga with
|
| Latest commit: |
4eb3e23
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://3ba14502.wallet-23u.pages.dev |
| Branch Preview URL: | https://change-password.wallet-23u.pages.dev |
Closes #132 and #133
@theDavidCoen please test and review
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Refactor
Chores