-
Notifications
You must be signed in to change notification settings - Fork 50
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix send password handling #493
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #493 +/- ##
==========================================
+ Coverage 49.34% 49.51% +0.17%
==========================================
Files 154 154
Lines 7373 7398 +25
==========================================
+ Hits 3638 3663 +25
Misses 3735 3735 ☔ View full report in Codecov by Sentry. |
Hmm, this works but I think it would be clearer if we separated the password hashing to a different function and left the encrypt/decrypt to just do encryption. We can even have a
|
@dani-garcia There are some more nuances to this. The server will also hash the password. The server response for the password field is actually If on encrypt we respond with My desire with the current approach was to completely eliminate the possibility of calling |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah fair enough, I forgot about the server side hashing, maybe there's something we could do with an enum like enum SendPassword { NoPassword, AlreadySet, New(String) }
, but maybe that won't translate very nicely to the mobile bindings.
Type of change
Objective
We should hash send passwords appropriately using pbkdf. Also changed how SendView handles passwords. It no longer provides the password but rather a boolean field
has_password
to prevent accidentally overriding the password when doingsend.decrypt().encrypt()
.Before you submit