570180 - Setup wizard sets passphrase='foo'

Status: RESOLVED FIXED

(Firefox :: Sync, defect)

Description

[Philipp von Weitershausen [:philikon]]

The setup wizard calls Weave.Service.login() to check whether a person's username + password are correct. login() calls _checkSetup() which wants a non-empty passphrase, otherwise it returns a failure code. So the wizard simply sets passphrase='foo' for the time being.

We should investigate whether there are cases where this can be a problem. Ideally, checking username + password should be possible independently from verifying the passphrase.

Comment 1

[Ed Lee :Mardak]

So if there's no private key to check the passphrase, login will succeed with any passphrase and 2nd computer setup will continue to the merge page:

http://hg.mozilla.org/labs/weave/annotate/710dec717f8d/source/chrome/content/preferences/fx-setup.js#l344

Comment 2

[Zandr Milewski [:zandr]]

I ended up with an account created with a passphrase of foo. Unfortunately, I was trying to test migration with the iPhone client, so by the time I figured out what was going on, I didn't have the logs.

We recently had some LDAP issues, both with the LDAP servers denying account creation and then with replication lag due to some restarts of the master.

Thinking over this, I think what happened was the following:

Created an account, which then couldn't log in due to replication lag.

Went off to troubleshoot the LDAP problems, including hitting https://auth.services.mozilla.com/user/1.0/[username]/node/weave with a browser instead of the client. (thus node assignment happened when the client wasn't looking)

Came back, got everything set up, synced a couple of times with no issues.

It was when I was trying to set up the iPhone on this account that I kept getting incorrect passphrase errors, and had to look in FF to see if I typo'ed something, that I saw it was set to 'foo'

Comment 3

[Philipp von Weitershausen [:philikon]]

Attachment: v1

Promote _verifyLogin to a public method so we have a way to query login status even with a non-existent or invalid passphrase.

Comment 4

[Mike Connor [:mconnor]]

Comment on attachment 450712 [details] [diff] [review]
v1

>diff --git a/ui/firefox/content/setup.js b/ui/firefox/content/setup.js
>--- a/ui/firefox/content/setup.js
>+++ b/ui/firefox/content/setup.js
>@@ -344,22 +344,20 @@
>       case EXISTING_ACCOUNT_LOGIN_PAGE:
>         Weave.Service.username = document.getElementById("existingUsername").value;
>         Weave.Service.password = document.getElementById("existingPassword").value;
>-        Weave.Service.passphrase = document.getElementById("existingPassphrase").value || "foo";
>-        if (Weave.Service.login()) {
>+        Weave.Service.passphrase = document.getElementById("existingPassphrase").value;
>+        // verifyLogin() will likely return false because we probably don't
>+        // have a passphrase yet (unless the user already entered it
>+        // and hit the back button).
>+        if (Weave.Service.verifyLogin()
>+            || Weave.Status.login == Weave.LOGIN_FAILED_NO_PASSPHRASE
>+            || Weave.Status.login == Weave.LOGIN_FAILED_INVALID_PASSPHRASE) {
>           // jump to merge screen
>-          this.wizard.pageIndex = EXISTING_ACCOUNT_MERGE_PAGE;
>+          this.wizard.pageIndex = EXISTING_ACCOUNT_PP_PAGE;
>           return false;

there's no need to set pageIndex + return false now, since that's the next page if we return true.  You can just invert this (if !login && != PP_ERROR) and do the actions in the else statement, falling through otherwise.

Comment 5

[Philipp von Weitershausen [:philikon]]

Attachment: v1.1

Simplify logic as suggested by mconnor. Also switch login observers to listen for verify-login, making the "Connecting" throbber work again.

Comment 6

[Ed Lee :Mardak]

http://hg.mozilla.org/services/fx-sync/rev/b0691477a740
Promote _verifyLogin to a public method so we have a way to query login status even with a non-existent or invalid passphrase.