Fixed Saved Passwords section in settings is empty #5656
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
simonhong
force-pushed
the
fix_password_not_shown_in_settings
branch
2 times, most recently
from
5450740
to
02809ee
Compare
|
Filed issue to crbug.com - https://bugs.chromium.org/p/chromium/issues/detail?id=1086348 |
simonhong
force-pushed
the
fix_password_not_shown_in_settings
branch
from
02809ee
to
65dcaef
Compare
bridiver
reviewed
May 28, 2020
| return ENCRYPTION_RESULT_SUCCESS; | ||
|
|
||
| - return OSCrypt::IsEncryptionAvailable() && IsUsingCleanupMechanism() | ||
| + return OSCrypt::IsEncryptionAvailable() /* && IsUsingCleanupMechanism() */ |
There was a problem hiding this comment.
why aren't we using kDeleteCorruptedPasswords to fix this?
There was a problem hiding this comment.
ok - Using kDeleteCorruptedPasswords could work on macOS.
There was a problem hiding this comment.
Hmm, I think using kDeleteCorruptedPasswords by default seems dangerous.
Brave will clear all passwords if decryption is failed temporarily.
How about using my initial way?
simonhong
force-pushed
the
fix_password_not_shown_in_settings
branch
2 times, most recently
from
f707dcc
to
c957228
Compare
bridiver
reviewed
Jun 2, 2020
| EncryptionResult result = InitPasswordFormFromStatement( | ||
| *statement, /*decrypt_and_fill_password_value=*/true, &primary_key, | ||
| new_form.get()); | ||
| + if (result == ENCRYPTION_RESULT_SERVICE_FAILURE) result = ENCRYPTION_RESULT_ITEM_FAILURE; |
There was a problem hiding this comment.
please convert to standard patch format BRAVE_STATEMENT_TO_FORMS
Some users reported that Saved Passwords sections is empty but passwords are autofilled to website's password form on linux and macOS. I think this can happen if user insert/update new password entries after login db encryption is corrupted. However, currently user can't anything even if current login db has decryptable entries. The reason is LoginDatabase::StatementToForms() gives empty password entry when any entry in login db has decryption failure on linux and macOS. To fix this, ENCRYPTION_RESULT_ITEM_FAILURE is use for individual decryption failure instead of ENCRYPTION_RESULT_SERVICE_FAILURE. If ENCRYPTION_RESULT_SERVICE_FAILURE is returned, LoginDatabase::StatementToForms() assumes all other entries will have decrypt failure. However, db could have decryptable entries even if current entry is failed to decrypt.
simonhong
force-pushed
the
fix_password_not_shown_in_settings
branch
from
c957228
to
ba2de6f
Compare
bridiver
approved these changes
Jun 2, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Some users reported that Saved Passwords sections is empty but passwords are autofilled
to website's password form on linux and macOS.
I think this can happen if user insert/update new password entries after login db encryption
is corrupted.
However, currently user can't anything even if current login db has decryptable entries.
The reason is LoginDatabase::StatementToForms() gives empty password entry when any entry in
login db has decryption failure on linux and macOS.
To fix this, ENCRYPTION_RESULT_ITEM_FAILURE is used for individual decryption failure
instead of ENCRYPTION_RESULT_SERVICE_FAILURE.
If ENCRYPTION_RESULT_SERVICE_FAILURE is returned, LoginDatabase::StatementToForms() assumes
all other entries will have decrypt failure. However, db could have decryptable entries
even if current entry is failed to decrypt.
Resolves brave/brave-browser#3196
Submitter Checklist:
npm run lint)git rebase master(if needed).git rebase -ito squash commits (if needed).Test Plan:
Login Datain user's profile folder to Brave's folderReviewer Checklist:
After-merge Checklist:
changes has landed on.