Status
People
(Reporter: jorgev, Assigned: rjwalsh)
Tracking
Details
Attachments
(1 attachment, 2 obsolete attachments)
4.58 KB,
patch
|
clouserw
:
review+
|
Details | Diff | Splinter Review |
We need a validation for extensions that store passwords in the preferences. It's not that big of a deal, but a warning would be nice. We could match the string "password", but only on preference files. So this should only be run on defaults/preferences/*.js The Help text should be something like "Extensions should use the Password Manager instead of storing passwords in the preferences. Users should be able to manage their passwords in the Password Manager and protect all of them with a Master Password if they choose to."
Updated•9 years ago
|
Assignee: nobody → jorge
Comment 1•9 years ago
|
||
Jorge, Wil can show you how to do this. Probably good to learn since you'll be the person who knows best regarding what the validator should do.
Comment 2•9 years ago
|
||
RJ - do you have time for this? Otherwise, I'll do it. Let me know when you know. :)
Assignee: jorge → rjbuild1088
(Assignee) | ||
Comment 3•9 years ago
|
||
Created attachment 414625 [details] [diff] [review] Fix
Attachment #414625 -
Flags: review?(clouserw)
(Assignee) | ||
Comment 4•9 years ago
|
||
Created attachment 414626 [details] [diff] [review] Migration and sql changes
Attachment #414626 -
Flags: review?(clouserw)
Comment 5•9 years ago
|
||
Comment on attachment 414626 [details] [diff] [review] Migration and sql changes 25 already exists. Make it 26 and I'll r+. And I already ran the 26 version on preview.amo so it should just start working when the next patch goes in.
Attachment #414626 -
Flags: review?(clouserw) → review-
Comment 6•9 years ago
|
||
Comment on attachment 414625 [details] [diff] [review] Fix This is great stuff, but I think Jorge's explanation is way too long for that spot. In the actual test we should return "A preferences file references a password." The longer explanation Jorge gave should go on https://addons.mozilla.org/en-US/firefox/pages/validation Also, use ___() for the string in validation.php Thanks RJ!
Attachment #414625 -
Flags: review?(clouserw) → review-
(Reporter) | ||
Comment 7•9 years ago
|
||
How about "Possible insecure password storage in preferences".
(Assignee) | ||
Comment 8•9 years ago
|
||
Created attachment 414805 [details] [diff] [review] v2 It would probably be a good idea to get test #25 into remora.sql
Attachment #414625 -
Attachment is obsolete: true
Attachment #414626 -
Attachment is obsolete: true
Attachment #414805 -
Flags: review?(clouserw)
Comment 9•9 years ago
|
||
Comment on attachment 414805 [details] [diff] [review] v2 > How about "Possible insecure password storage in preferences". That does sound better than mine. RJ, mind changing it to that before you commit? > It would probably be a good idea to get test #25 into remora.sql That's a good idea too. I'll do it. Thanks for the patch!
Attachment #414805 -
Flags: review?(clouserw) → review+
(Assignee) | ||
Comment 10•9 years ago
|
||
Fixed in r56920 and migration in r56921. Hope everyone had a happy Thanksgiving!
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Updated•2 years ago
|
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•