Add password in preferences check to Code Validation Tool

RESOLVED FIXED in 5.4

Status

addons.mozilla.org Graveyard
Admin/Editor Tools
P2
normal
RESOLVED FIXED
9 years ago
2 years ago

People

(Reporter: jorgev, Assigned: rjwalsh)

Tracking

unspecified

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

9 years ago
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
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.
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 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 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 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
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.