449810 - legacy storage module should throw on canceled master password entry in getAllLogins

Status: RESOLVED FIXED

(Toolkit :: Password Manager, defect)

Description

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

In order for the new mozStorage module to handle importing correctly, the legacy module needs to throw if the master password was canceled. This allows the mozStorage module to gracefully fail, and prompt later.

Comment 1

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Attachment: Patch v0.1

patch to storage-Legacy.js

Comment 2

[Justin Dolske [:Dolske]]

Comment on attachment 332978 [details] [diff] [review]
Patch v0.1

Yeah, I think there also needs to be a change to LoadSignons() in toolkit/components/passwordmgr/content/passwordManager.js... The call to getAllLogins() should be wrapped in a try/catch, and if an exception is thrown |signons| should end up as an empty array (as would have happened before this change).

Comment 3

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Attachment: Patch v0.2

Updated to include the content fixes - the one mentioned above, and the other one we discussed

Comment 4

[Justin Dolske [:Dolske]]

Comment on attachment 332986 [details] [diff] [review]
Patch v0.2

>+
>+        // We want to throw in this case, so that LoginManagerStorage-mozStorage
>+        // will halt importing.
>+        if (userCanceled)
>+            throw "User canceled Master Password entry";
> 
>         // decrypt entries for caller.
>         [result, userCanceled] = this._decryptLogins(result);

I think you probably want to add that *after* userCanceled is assigned a value. :) Also, no need for the comment.

Comment 5

[Paul O'Shannessy [:zpao] (not bugmail, email directly)]

Attachment: Patch v0.3

I'm not even sure how that happened...

Comment 6

[Justin Dolske [:Dolske]]

Pushed changeset 29f2c16ef6e2.