218694 - implement master password UI

Status: RESOLVED FIXED

(Toolkit :: Password Manager, defect, P2)

Description

[Asa Dotzler [:asa]]

This bug is for tracking the design and implementation of a master password UI
for Mozilla Firebird.

Comment 1

[Steffen Wilberg]

I already filed bug 216539: we need a way to choose/edit/remove the master
password in Options->Privacy.

Comment 2

[David P James]

Moving all active Password Manager bugs to Autocomplete component with
[pwd-mngr] in summary for querying. Sorry for the bugspam.

Comment 3

[Steffen Wilberg]

Reassigning to the default assignee.

Comment 4

[Darin Fisher]

i wanted to put in a plug for improving the user-discovery of the master
password feature.  for example, a dialog shown to the user the first time they
save a password might want to include a comment and a button directing the user
to the master password feature (in addition to a warning about what saving
passwords means, etc).

Comment 5

[Ben Goodger (use ben at mozilla dot org for email)]

Targeting. This needs to go into Options... need to discuss with brian what's
needed here. 

Comment 6

[Ben Goodger (use ben at mozilla dot org for email)]

*** Bug 216539 has been marked as a duplicate of this bug. ***

Comment 7

[chris hofmann]

blake says some basic seamonkey ui coming soon...

Comment 8

[Mike Connor [:mconnor]]

patch coming up

Comment 9

[Mike Connor [:mconnor]]

Attachment: lots of stolen and cleaned up code from changepassword.xul

Comment 10

[Mike Connor [:mconnor]]

Comment on attachment 155464 [details] [diff] [review]
lots of stolen and cleaned up code from changepassword.xul

blake, hopefully you didn't put much work into this already

Comment 11

[Mike Connor [:mconnor]]

hmm, fun, I didn't implement the "ask me for this when X" pref stuff, do we want
that too? that's much more trivial :)

Comment 12

[Ben Goodger (use ben at mozilla dot org for email)]

Mike, can you show some screenshots? A quick glance at the code makes it seem
like the presentation is relatively low key which is probably the best thing. 

Comment 13

[Mike Connor [:mconnor]]

will post screenshots in a couple hours, not at my dev machine

Comment 14

[Mike Connor [:mconnor]]

with no password set
http://www.steelgryphon.com/stuff/masterpass1.png

setting a password
http://www.steelgryphon.com/stuff/masterpass2.png

with a password set
http://www.steelgryphon.com/stuff/masterpass3.png

left undecided by this patch is bug 222408, which is UI for setting the master
password frequency prefs.  I'm inclined to think that the current "once per
session" is sufficient for most users.

Comment 15

[Daniel Küstner]

Looks fine. But I think normal user doesn't know what a master password is and
that the passwords are encrypted with MP and saved plaintext without. Thus, a
hint like „Master password increases security“ would be nice.

Comment 16

[Steffen Wilberg]

Please also add ellipses ("...") to the labels of all buttons which open a
dialog, i.e. View Saved Passwords, Set/Change Master Password, Exceptions, View
Cookies.

This is for consistency with buttons everywhere else (including Web Features,
after I've checked in my patch for bug 250543).

Comment 17

[Mike Connor [:mconnor]]

Set Master Password should have an ellipsis, but View etc should not.

Note that in the privacy pane, I've actually removed all of the bogus usage of
the ellipsis already with my patch for the cookies UI.

Comment 18

[Christian :Biesinger (don't email me, ping me on IRC)]

(In reply to comment #16)
> Please also add ellipses ("...") to the labels of all buttons which open a
> dialog, i.e. View Saved Passwords, Set/Change Master Password, Exceptions, View
> Cookies.

mconnor is right. see also:
http://developer.gnome.org/projects/gup/hig/2.0/menus-design.html
http://java.sun.com/products/jlf/ed2/book/HIG.Controls.html#43232
http://developer.apple.com/documentation/UserExperience/Conceptual/OSXHIGuidelines/XHIGText/chapter_4_section_3.html#//apple_ref/doc/uid/TP30000365/TPXREF126

Comment 19

[Michael Lefevre]

And seeing as I already looked it up for the same conversation in bug 250543:
http://msdn.microsoft.com/library/default.asp?url=/library/en-us/dnwue/html/ch14d.asp

Comment 20

[Ben Goodger (use ben at mozilla dot org for email)]

Mike, looks good, except I'd add a message to the top of the "Set Master
Password" window that explains briefly what it's doing.

"A Master Password is used to protect sensitive information like site passwords.
If you create a Master Password you will be asked to enter it once per session
when you log into a site that &brandShortName; has saved login information for."

and, in bolded text below:

"Please be sure to remember your Master Password. If you forget it, you will not
be able to access any of your stored login information."

or some such.

Once per session sounds fine to start with. 

Comment 21

[Mike Connor [:mconnor]]

Attachment: patch with descriptive text

screenshot of the new dialog at
http://www.steelgryphon.com/stuff/masterpass4.png

Comment 22

[Ben Goodger (use ben at mozilla dot org for email)]

Comment on attachment 155783 [details] [diff] [review]
patch with descriptive text

r+a=[e-mail]

Comment 23

[Steffen Wilberg]

Neither setting nor changing the master password work on Linux. There is no
effect even after clicking OK in the main Options dialog.

It works fine in Advanced->Certificates->Manage Security Devices->Software
Security Device->Change Password.

Comment 24

[Mike Connor [:mconnor]]

Steffen, I grabbed a tinderbox build since I'm on Windows atm, it works fine
there.  Note that after setting the password, you won't be prompted until you
restart Firefox.

Comment 25

[Steffen Wilberg]

Doesn't work with a Sweetlou build either. Not even after restarting Firefox.
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.2) Gecko/20040814
Firefox/0.9.1+.

Comment 26

[Mike Connor [:mconnor]]

fix in, missed an accept case, should work fine using Enter instead of clicking OK.

Comment 27

[Volkmar W. Pogatzki]

What about "Master Password Timeout" from the old UI
chrome://pippki/content/pref-masterpass.xul ? Shouldn't it be available too?

Comment 28

[Mike Connor [:mconnor]]

it was considered and rejected in favour of the simpler implementation.  The
prefs are still in existence, but we're not going to provide UI at this time. 
Given that banks and other secure sites are always advising users to close all
browser windows to ensure security of sessions, I believe that there isn't a
real need to have timeout functionality etc for the vast majority of users.

Comment 29

[Asa Dotzler [:asa]]

Do we have a bug on removing the show passwords button or will that be done as
part of this bug? I think bryner's on the hook for the button removal.

Comment 30

[Peter van der Woude [:Peter6]]

I looked for other bugs about "edit password" but they were all duped to this one.

how about the option to double leftclick on an entry in PW manager giving you
the option to  edit both the login name and password ?

Fixed-aviary1.0 is not totally fixed but will have to do till the next milestone ?

Comment 31

[Peter van der Woude [:Peter6]]

*** Bug 264186 has been marked as a duplicate of this bug. ***

Comment 32

[Steffen Wilberg]

Peter, please file a new bug on that, if there is none yet.
This one is about the *master* password.

Comment 33

[Peter van der Woude [:Peter6]]

(In reply to comment #32)
> Peter, please file a new bug on that, if there is none yet.
> This one is about the *master* password.

filed Bug 264201 for Edit passwords