If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Ask for password/passphrase before changing it

RESOLVED FIXED in 0.6

Status

Cloud Services
Firefox Sync: UI
P3
normal
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: anant, Assigned: anant)

Tracking

unspecified
x86
All
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

8 years ago
The current change password / passphrase dialogs do not ask for your old password / passphrase before changing them. We should change them to ask and verify them before change.
(Assignee)

Updated

8 years ago
OS: Mac OS X → All
Priority: -- → P3
Target Milestone: --- → 0.6

Comment 1

8 years ago
Does the current dialog delete current data, or does it re-encrypt the private key?

Comment 2

8 years ago
What will we do about 'forgot passphrase'? I suppose that could be put behind a password check.

How about in the future when it's just a passphrase with no password?
(Assignee)

Comment 3

8 years ago
(In reply to comment #1)
> Does the current dialog delete current data, or does it re-encrypt the private
> key?

It re-encrypts the private key.
(Assignee)

Comment 4

8 years ago
(In reply to comment #2)
> What will we do about 'forgot passphrase'? I suppose that could be put behind a
> password check.

'Forgot passphrase' is already behind a password check.

> How about in the future when it's just a passphrase with no password?

This is a slightly harder problem that we need to think about. One suggestion I had was to send an email to the user linking to a web page which has Javascript that generates the new password (from the new passphrase), updates the server and uploads the new private key.
(Assignee)

Comment 5

8 years ago
Created attachment 391808 [details] [diff] [review]
Ask for current password/passphrase

Ask the user for the current password/passphrase before changing it.
Attachment #391808 - Flags: review?(edilee)

Comment 6

8 years ago
Comment on attachment 391808 [details] [diff] [review]
Ask for current password/passphrase

>       alert(this._stringBundle.getString("noPassphrase.alert"));
>     } else if (this._firstBox.value != this._secondBox.value) {
>       alert(this._stringBundle.getString("passphraseNoMatch.alert"));
>+    } else if (this._oldBox.value != Weave.Service.passphrase) {
>+      alert(this._stringBundle.getString("incorrectPassphrase.alert"));
How does the alert message look right now? I believe the change * dialogs are a modal dialog, so alerting from the modal either causes the current modal to temporarily close and show the alert then reshow the change after dismissing the alert. Or another modal tries to open from the existing modal.

Is it possible to display an error (red/bold?) message like on the login dialog?
(Assignee)

Comment 7

8 years ago
(In reply to comment #6)
> How does the alert message look right now? I believe the change * dialogs are a
> modal dialog, so alerting from the modal either causes the current modal to
> temporarily close and show the alert then reshow the change after dismissing
> the alert. Or another modal tries to open from the existing modal.

On my mac, the current modal temporarily closes and shows the alert; which when dismissed, goes back to the original modal.

> Is it possible to display an error (red/bold?) message like on the login
> dialog?

Sure, but I don't think it's a major improvement. Even on the current login dialog, if you don't enter a username, password or passphrase, you get such an alert. If we change it to display the red error message here, we should probably do it there too :)

Comment 8

8 years ago
Oh. Indeed we do. Yet we also have the red/bold error message for "wrong username or password... or passphrase!"
(Assignee)

Comment 9

8 years ago
Yes, we're pretty inconsistent, probably because several folks have worked on the login dialog / wizard over time :)

Hopefully, most of this should go away with the shiny new about:weave!

Comment 10

8 years ago
I've encountered this problem when weave wouldn't login (bug 504803). "Forgot passphrase" apparently allowed me to change it, but login was still unsuccessful. Validating on second PC showed that passphrase was still the original one.

So: passphrase update without first specifying correct passphrase seems to fail silently.

Comment 11

8 years ago
Comment on attachment 391808 [details] [diff] [review]
Ask for current password/passphrase

>+++ b/source/chrome/content/generic-change.js	Fri Jul 31 00:16:42 2009 -0700
>+  get _oldBox() {
>+    delete this._oldBox;
>+    return this._oldBox = document.getElementById("cBoxText");
What's "cBoxText"? Could you give it a more useful name? "c" for "current"? "change"? a, b, "c" ? Might as well make the getter name related to the element id.

>+    let cboxlabel = document.getElementById("cBoxLabel");
>     let box1label = document.getElementById("textBox1Label");
>     let box2label = document.getElementById("textBox2Label");
I suppose it's a separate bug to fix those "box1" ids.. Just fix cbox for now I guess.

>@@ -75,6 +81,9 @@
>         this._title.value = this._stringBundle.getString(
>           "change.passphrase.title"
>         );
>+        cboxlabel.value = this._stringBundle.getString(
>+          "new.passphrase.old"
>+        );
>         box1label.value = this._stringBundle.getString(
>           "new.passphrase.label"
>         );
Could you make sure to attach patches with context 8 and function headers?

>@@ -85,11 +94,15 @@
>           "ondialogaccept",
>           "return Change.doChangePassphrase();"
>         );
>+        document.getElementById("oldBox").setAttribute("hidden", "false");
Why not use the getter you defined earlier for this? Also, put this next to where you're setting the string for the label. Same for ChangePassword.

>+++ b/source/chrome/locale/en-US/generic-change.properties	Fri Jul 31 00:16:42 2009 -0700
>@@ -3,6 +3,9 @@
>+incorrectPassword.alert = Your current password was incorrect!
>+incorrectPassphrase.alert = Your current passphrase was incorrect!
Make these present tense with "is"... Or maybe just "Incorrect current password."

>@@ -22,7 +25,9 @@
>+new.passphrase.old = Enter your current passphrase
> new.passphrase.label = Enter your new passphrase
> new.passphrase.confirm = Confirm your new passphrase
>+new.password.old = Enter your current password
> new.password.label = Enter your new password
>-new.password.confirm = Confirm your new password
>\ No newline at end of file
>+new.password.confirm = Confirm your new password
These all seem kinda wordy for a simple dialog but should be fine for now.. "Current passphrase" "New passphrase" "Confirm passphrase"

r=Mardak w/ fixes
Attachment #391808 - Flags: review?(edilee) → review+
(Assignee)

Comment 12

8 years ago
(In reply to comment #11)
> What's "cBoxText"? Could you give it a more useful name? "c" for "current"?
> "change"? a, b, "c" ? Might as well make the getter name related to the element
> id.

I changed the element IDs to "currentBoxText" and "currentBoxLabel" and changed the getter to reflect that.

> >@@ -85,11 +94,15 @@
> >           "ondialogaccept",
> >           "return Change.doChangePassphrase();"
> >         );
> >+        document.getElementById("oldBox").setAttribute("hidden", "false");
> Why not use the getter you defined earlier for this? Also, put this next to
> where you're setting the string for the label. Same for ChangePassword.

The earlier getter was for the textbox, not the whole row. This line un-hides the row that displayed current password/passphrase. I changed the row ID to oldBoxRow and created a getter for it.

> r=Mardak w/ fixes

http://hg.mozilla.org/labs/weave/rev/302e4fb27114
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.