619013 - Update Sync Key dialog wording needs to be more clear

Status: RESOLVED FIXED

(Firefox :: Sync, defect)

Description

[Tony Chung [:tchung]]

Attachment: Cannot Update Sync Key screenshot

After updating to sync 1.6 on fx4b7 nightly, sync fails because the HMAC is mismatched.  however, following the update notification steps takes me to the sync UI setup path, telling me the keys are outdated.  

The sync UI pops up a dialog that seems like i can customize my own sync key, which is untrue.   Please update the sync UI to reflect generating a sync key, or allow the user to support creating their own sync key.   It continues to fail as well.

* See screenshot

Repro:
1) login to a sync v5 account on an old minefield build (4.0b7 nightly)
2) you'll be asked to upgrade your extension.  do it.
3) login in and try to sync with 1.6 installed
4) notification complains about hmac keys mismatched.  Launch the sync UI, click My sync key, and click Update
5) Verify UI asks to Update sync key.   Type in a customized key, and click update.  Error updating.

2010-12-13 22:19:07	Service.Main         INFO	Logging in user 2ws5dmu6tpatkaicmzi4ny76azxhgcrh
2010-12-13 22:19:07	Net.Resource         DEBUG	GET success 200 https://phx-sync474.services.mozilla.com/1.0/2ws5dmu6tpatkaicmzi4ny76azxhgcrh/info/collections
2010-12-13 22:19:07	Service.Main         DEBUG	Fetching global metadata record
2010-12-13 22:19:07	Service.Main         DEBUG	Weave Version: 1.6b4 Local Storage: 5 Remote Storage: 5
2010-12-13 22:19:07	Service.Main         INFO	Upgrading sync key...
2010-12-13 22:19:07	Service.Main         INFO	Saving upgraded sync key...
2010-12-13 22:19:07	Service.Main         INFO	Done saving.
2010-12-13 22:19:07	Service.Main         DEBUG	Fetching and verifying -- or generating -- symmetric keys.
2010-12-13 22:19:07	Net.Resource         DEBUG	GET success 200 https://phx-sync474.services.mozilla.com/1.0/2ws5dmu6tpatkaicmzi4ny76azxhgcrh/info/collections
2010-12-13 22:19:07	Service.Main         INFO	Testing info/collections: {"meta":1292220774.77,"crypto":1292220754.33,"clients":1292305527.05,"bookmarks":1292290541.68,"forms":1292298581.73,"history":1292298642.18,"prefs":1292220774.48,"tabs":1292305827.78}
2010-12-13 22:19:07	CollectionKeys       INFO	Testing for updateNeeded. Last modified: 0
2010-12-13 22:19:07	Service.Main         INFO	CollectionKeys reports that a key update is needed.
2010-12-13 22:19:07	Net.Resource         DEBUG	GET success 200 https://phx-sync474.services.mozilla.com/1.0/2ws5dmu6tpatkaicmzi4ny76azxhgcrh/storage/crypto/keys
2010-12-13 22:19:07	CollectionKeys       INFO	Updating collection keys...
2010-12-13 22:19:07	CollectionKeys       WARN	Got exception "Record SHA256 HMAC mismatch: should be cbf2549ea8c9344b2d699d7602a1dab642d678363aef06721cfdd7d2f1a45a4c, is b9e25c3381cb6e3191bdf79778fac2bcdb79b32e9ec87a9d42cf7b27f8289b2b" decrypting storage keys with sync key.
2010-12-13 22:19:07	CollectionKeys       INFO	Aborting updateContents. Rethrowing.
2010-12-13 22:19:07	Service.Main         WARN	Got exception "Record SHA256 HMAC mismatch: should be cbf2549ea8c9344b2d699d7602a1dab642d678363aef06721cfdd7d2f1a45a4c, is b9e25c3381cb6e3191bdf79778fac2bcdb79b32e9ec87a9d42cf7b27f8289b2b" fetching cryptoKeys.
2010-12-13 22:19:07	Service.Main         WARN	Failed to fetch symmetric keys. Failing remote setup.
2010-12-13 22:19:07	Service.Main         WARN	Remote setup failed.
2010-12-13 22:19:07	Service.Main         DEBUG	Exception: Login failed: error.login.reason.synckey No traceback available

Expected:
- Update sync key successful

Acutal:
- login failed.  if a user cannot update their own sync key with their own key, dont make the UI so ambiguous that it looks like they can.

Comment 1

[Richard Newman [:rnewman]]

(In reply to comment #0)
> The sync UI pops up a dialog that seems like i can customize my own sync key,
> which is untrue.   Please update the sync UI to reflect generating a sync key,
> or allow the user to support creating their own sync key.   It continues to
> fail as well.

This could happen because your sync key is wrong. You need to type in the correct sync key from another device. The UI can't (shouldn't!) just be for generation, because at some point you need to enter the key elsewhere.

Arguably the existing "stop using", "reset sync", or "my sync key" UI address key creation. You should only see this dialog if you're already using sync elsewhere, and *this* device has the wrong key.
 
> * See screenshot
> 
> Repro:
> 1) login to a sync v5 account on an old minefield build (4.0b7 nightly)
> 2) you'll be asked to upgrade your extension.  do it.
> 3) login in and try to sync with 1.6 installed
> 4) notification complains about hmac keys mismatched.  Launch the sync UI,
> click My sync key, and click Update
> 5) Verify UI asks to Update sync key.   Type in a customized key, and click
> update.  Error updating.

For this to succeed, two things need to be true:

* The sync ID on the server must be the same as it was when you first had your key upgraded from a passphrase. This will not be the case if you migrated v3 => v4 => v5.

* The string you entered must be either your new 26-char sync key, or your correct original passphrase.

If either of these is false, the generated key will be different to the one used to encrypt the data on the server, and Sync will report "Wrong Sync Key".

So: the "v5 account" you're using -- did you ever use it with v4? Or is it brand new?

If it's brand new, you cannot expect a typed passphrase (not 26 chars) to work... and if you used it with v4, you cannot expect key upgrade to work (known issue).

> Storage: 5 Remote Storage: 5
> 2010-12-13 22:19:07    Service.Main         INFO    Upgrading sync key...

OK, so we're v5, but the local passphrase is a passphrase, not a sync key...

> 2010-12-13 22:19:07    CollectionKeys       WARN    Got exception "Record
> SHA256 HMAC mismatch: should be
> cbf2549ea8c9344b2d699d7602a1dab642d678363aef06721cfdd7d2f1a45a4c, is
> b9e25c3381cb6e3191bdf79778fac2bcdb79b32e9ec87a9d42cf7b27f8289b2b" decrypting
> storage keys with sync key.

... and the generated sync key is not correct (for one of the two reasons above).

> - login failed.  if a user cannot update their own sync key with their own key,
> dont make the UI so ambiguous that it looks like they can.

Most users will not be upgrading twice. This is a known issue.

If you did the first upgrade straight from v3 to v5, then this might be a bug.

Could you please clarify?

Comment 2

[Mike Connor [:mconnor]]

Looking at the screenshot, I don't know how ambiguous this really is.  This is the same as "your password has changed, enter your new password", which I don't think creates the expectation that you can change your password in that dialog.

Maybe because it's a sync key, this wording is less clear?

We can get some better strings, for sure.  Blocking b9 on sanding off that rough edge.

Comment 3

[Alex Faaborg [:faaborg] (Firefox UX)]

If the user only reads "Please enter your new sync key" they might not get the broader context that it already exists, and incorrectly believe that they are creating a new one right now (this compounded by the fact that during the transition, we created a new one for them).  Perhaps change the first string to:

Your Sync Key was changed using another device, please enter your updated Sync Key.

(with that being the only string in the dialog, so they are less likely to only read the text field label and get confused with the cropping).

I'll create a quick mockup.

Comment 4

[Alex Faaborg [:faaborg] (Firefox UX)]

Attachment: Update sync key

New string, also added the same help message from our Jpake flow, just in case the user has no idea what they are being asked.

Comment 5

[Philipp von Weitershausen [:philikon]]

Thanks, Alex!

(In reply to comment #4)
> New string,

Added [strings] to whiteboard.

> also added the same help message from our Jpake flow, just in case
> the user has no idea what they are being asked.

That's a good idea. I see that the dialog now looks like the wizard. Shouldnt' we then give the other dialogs we have (change password, update password, change Sync Key) the same treatment?

Comment 6

[Alex Faaborg [:faaborg] (Firefox UX)]

>Shouldnt' we then give the other dialogs we have (change password, update >password, change Sync Key) the same treatment?

Yeah, I kind of like this treatment since it separates the message from the buttons.

Comment 7

[Philipp von Weitershausen [:philikon]]

(In reply to comment #6)
> Yeah, I kind of like this treatment since it separates the message from the
> buttons.

Ok, that makes it easier because underneath they're all the same dialog and we just dynamically change what we display in the page.

Comment 8

[LegNeato]

As per today's meeting, beta 9 will be a time-based release. Marking these all betaN+. Please move it back to beta9+ if  you believe it  MUST be in the next beta (ie: trunk is in an unshippable state without this)

Comment 9

[LegNeato]

As per today's meeting, beta 9 will be a time-based release. Marking these all betaN+. Please move it back to beta9+ if  you believe it  MUST be in the next beta (ie: trunk is in an unshippable state without this)

Comment 10

[LegNeato]

(whoops, sorry for the double comment...script gone awry)

Comment 11

[Philipp von Weitershausen [:philikon]]

I have a WIP for this.

Comment 12

[Axel Hecht [:Pike]]

Can this land today? I really don't want to see any strings land after this week.

Comment 13

[Philipp von Weitershausen [:philikon]]

(In reply to comment #12)
> Can this land today? I really don't want to see any strings land after this
> week.

I'm afraid not. We were and still are prioritizing hardblockers, so even if my somehow manage to turn my WIP into a patch today, it probably won't be able to land today. I can work on this over the weekend. Good enough?

Comment 14

[Philipp von Weitershausen [:philikon]]

Maybe we can land the string change now and the rest later. Will try to split the patch apart that way.

Comment 15

[Axel Hecht [:Pike]]

Going into the business of pre-landing strings doesn't put us into a much better position in terms of getting to RC, IMHO. 

Sounds like we should triage this.

Comment 16

[Philipp von Weitershausen [:philikon]]

(In reply to comment #15)
> Going into the business of pre-landing strings doesn't put us into a much
> better position in terms of getting to RC, IMHO. 

I wasn't talking so much about pre-landing strings but about reducing the focus of this bug to primarily making the wording clearer. If we also get the time to make the sucky UI look better, great. If not, at least the words will make sense. Does that sound ok?

Comment 17

[Axel Hecht [:Pike]]

Yes, much better.

Comment 18

[Philipp von Weitershausen [:philikon]]

Attachment: Part 1 (v1): String change

This gets us just the string change faaborg suggests in the mockup. It doesn't get us the helpful blue box borrowed from the setup wizard that explains how you get to the Sync Key, nor the pretty styling (those will be addressed in part 2).

Comment 19

[Philipp von Weitershausen [:philikon]]

Attachment: Part 2 (wip v1): Style generic change dialogs like other wizards

WIP for part 2, not done yet.

Comment 20

[Philipp von Weitershausen [:philikon]]

Split off bug 626949 for changing the dialog styling so that we can land the string now.

Comment 21

[Philipp von Weitershausen [:philikon]]

http://hg.mozilla.org/mozilla-central/rev/f861419e8e45

Comment 22

[Philipp von Weitershausen [:philikon]]

Comment on attachment 504047 [details] [diff] [review]
Part 2 (wip v1): Style generic change dialogs like other wizards

The restyling is happening in bug 626949.