235360 - Password Manager API changes to facilitate import/export

Status: RESOLVED FIXED

(SeaMonkey :: Passwords & Permissions, defect, P1)

Description

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

We need to be able to read in password entries from an arbitrary file to support
import/export in Firefox. This requires changes to nsIPasswordManagerInternal.

Comment 1

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

Attachment: patch to add functions to read/write from specific files

Comment 2

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

taking, targeting, setting dependencies

Comment 3

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

Attachment: this one actually compiles!

Comment 4

[Daniel Veditz [:dveditz]]

Comment on attachment 142106 [details] [diff] [review]
this one actually compiles!

r=dveditz (or sr= if you need Darin's r= for the netwerk code)

Comment 5

[Darin Fisher]

Comment on attachment 142106 [details] [diff] [review]
this one actually compiles!

so, do you need the WritePasswords function?  i ask because you determined that
you don't need it for cookies.

also, i might rename |aHost| to something more accurate, like |aKey| since
aHost is not a simple hostname.  it is a password entry key ;-)

i know, i know... all the singsign code refers to it as the host field.  that's
just unfortunate.

Comment 6

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

Attachment: newer patch

Yeah, I didn't turn out to need writePasswords after all. I suppose symmetry
isn't so important. Also add a "encrypt vs. obscure" param at darin's
suggestion in case this is implemented for Single Signon

Comment 7

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

Attachment: a newer patch

This one doesn't clutter up the API for (what I can tell) no reason with
Seamonkey implementation details. Use "key" instead of "host" in terminology as
well. 

Darin, you choose.

Comment 8

[Michiel van Leeuwen (email: mvl+moz@)]

in the newer patch, is aPassword plaintext, obscured or encrypted? Can you
document that?

Comment 9

[Darin Fisher]

Comment on attachment 142697 [details] [diff] [review]
a newer patch

>Index: netwerk/base/public/nsIPasswordManagerInternal.idl

>+   * @param aUser The username portion of the login
>+   * @param aPassword The password portion of the login
...
>+  void addUserFull(in AUTF8String aKey, 
>+                   in AString aUser, 
>+                   in AString aPassword,

yeah, i agree with mvl.  please document the fact that the username and
password must be passed in plain text.	it's not obvious in the context
of importing usernames and passwords.


>+  /**
>+   * Reads logins from a Mozilla Password Manager file, augmenting the current 
>+   * in-memory set.

augmenting is a vague term.  can you be more specific?	like, perhaps
we want to mention what happens when a conflict occurs.  e.g., suppose
we already have a password for a given key+username combo, but suppose
the new password is different.	we should document this behavior.


sr=darin with those things documented.

Comment 10

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

Fix checked in. 

Comment 11

[Boris 'pi' Piwinger]

Is there a bug for a UI to use this? Bug 256406 asks for the export 
funtionality. Bug 250160 asks for import. They could serve this purpose.

pi