Closed
Bug 591118
Opened 14 years ago
Closed 14 years ago
Generate Sync Key
Categories
(Firefox :: Sync, defect)
Firefox
Sync
Tracking
()
VERIFIED
FIXED
People
(Reporter: philikon, Assigned: philikon)
References
Details
(Whiteboard: [strings])
Attachments
(3 files, 4 obsolete files)
8.00 KB,
patch
|
mconnor
:
review+
|
Details | Diff | Splinter Review |
13.61 KB,
patch
|
mconnor
:
review+
|
Details | Diff | Splinter Review |
19.56 KB,
patch
|
Details | Diff | Splinter Review |
Instead of requiring the user to enter a passphrase/Sync Key, we want to auto-generate it. a-z is mobile friendly, 20 char length gives us 94 bits.
Assignee | ||
Updated•14 years ago
|
Component: Sync → Firefox UI
QA Contact: sync → firefox
Comment 1•14 years ago
|
||
Note that hyphens need to be ignored when evaluating a sync key. In the mockup they are displayed for readability, but users may or may not decide to enter them on additional machines.
Comment 2•14 years ago
|
||
Similar interfaces provide the hyphens automatically, but we can't really do that since the user can provide their own. But either way people have been trained to not type them, so we should expect that a certain number of people won't.
Would taking the same approach with spaces mess up our entropy too much for user created sync keys? (in case they enter spaces instead of hyphens?)
Comment 3•14 years ago
|
||
A few related questions:
1. If the user opts to have their Sync Key emailed or copies it to clipboard, will it include the hyphens or not?
2. When they are setting up Sync on Android (or Firefox Home), will they have to enter the hyphen or not?
3. Can we gather aggregate information about how many users prefer to go with an auto generated Sync Key vs. creating their own?
Comment 4•14 years ago
|
||
>1. If the user opts to have their Sync Key emailed or copies it to clipboard,
>will it include the hyphens or not?
yes, they are important for readability as the user is doing re-entry
>2. When they are setting up Sync on Android (or Firefox Home), will they have
>to enter the hyphen or not?
they may or may not enter them, so we should assume that a certain percentage are not going to, and evaluate what is entered without hyphens (and potentially without spaces if possible).
Assignee | ||
Comment 5•14 years ago
|
||
This ambiguity worries me a bit: "Here's your Sync key nicely hyphenated into 4 groups. But you don't have to enter it that way. Best is you don't worry about the hyphens because they're hard to enter on mobile. In fact, we don't care about the hyphens at all and when you enter your own key, there will be no hyphens. They're not what you think they are."
I think we should either not show the hyphens at all OR make this clearer in the UI. For instance by separating the autogenerated Sync Key case from the custom passphrase case. The initial view of the setup screen would present the Sync Key, but not in an editable textbox. There's a button next to it that says "I want to provide my own". When clicked that makes the Sync Key and the Email/Print/Save buttons disappear and just shows the regular textbox that allows you to enter your own passphrase. For input we would do the same: present four textboxes a la Windows Product Key with a "I have my custom passphrase" button next to it.
Assignee | ||
Comment 6•14 years ago
|
||
Btw, are the Email/Print/Save options final? Are we dropping Copy To Clipboard?
Assignee | ||
Comment 7•14 years ago
|
||
Work in progress.
Done:
* Generate 20 character (a-z) passphrase
* Implement Email/Print/Save backup options with preliminary text. Print and Save use an HTML page that needs to be styled (bug 591533).
Todo:
* Add hyphens, do not focus passphrase textbox, select all when focusing
* Fix up other parts of the wizard and pref UI where the passphrase can be entered/set.
Assignee: nobody → philipp
Assignee | ||
Comment 8•14 years ago
|
||
Splitting the patch into parts. Part 1 contains the common methods for generating as well as backing up the passphrase/Sync Key.
Attachment #470134 -
Attachment is obsolete: true
Attachment #470665 -
Flags: review?(mconnor)
Assignee | ||
Comment 9•14 years ago
|
||
This makes the wizard generate the Sync Key (also changed the terminology from passphrase/secret phrase to Sync Key everywhere). You can't continue until you either choose one of the backup methods or provide your own passphrase.
Attachment #470666 -
Flags: review?(mconnor)
Attachment #470666 -
Flags: feedback?(faaborg)
Assignee | ||
Comment 10•14 years ago
|
||
Fix up the Change/Update Passphrase dialog in the preference pane. The user can open this dialog manually when they want to change their passphrase. Since we now display the current passphrase/Sync Key in clear text, it's also handy to see what the Sync Key is. The three backup methods are available here as well, so this dialog can also be used to redo the backup. The user can also either enter a new passphrase or have a new Sync Key generated.
Attachment #470668 -
Flags: review?(mconnor)
Attachment #470668 -
Flags: feedback?(faaborg)
Assignee | ||
Updated•14 years ago
|
Whiteboard: [strings]
Assignee | ||
Comment 11•14 years ago
|
||
Incorporate faaborg's feedback:
* Move error msg for custom passphrases into the box
* Fix up remaining error messages that still mention the "secret phrase"
Attachment #470666 -
Attachment is obsolete: true
Attachment #471870 -
Flags: review?(mconnor)
Attachment #470666 -
Flags: review?(mconnor)
Attachment #470666 -
Flags: feedback?(faaborg)
Assignee | ||
Comment 12•14 years ago
|
||
Incorporate faaborg's feedback:
* Rename the "Change Secret Phrase" button to "My Sync Key" as it now allows you to review the key and back it up again, not just change it.
Attachment #470668 -
Attachment is obsolete: true
Attachment #471873 -
Flags: review?(mconnor)
Attachment #470668 -
Flags: review?(mconnor)
Attachment #470668 -
Flags: feedback?(faaborg)
Updated•14 years ago
|
Attachment #470665 -
Flags: review?(mconnor) → review+
Comment 13•14 years ago
|
||
Comment on attachment 471870 [details] [diff] [review]
Part 2 (v2): Generate the Sync Key in the setup wizard
* please just remove the "deprecated" strings. Cruft is bad.
>+<!ENTITY setup.newSyncKeyPage.description.label "To ensure your total privacy, all of your data is encrypted prior to being uploaded. The key to decrypt your data is not uploaded.">
not quite technically true, at this point. We generate insanely powerful keys to encrypt data, and wrap those keys with the key we don't update... we really don't want to get into a "well, the UI claims X, but X is a lie" state.
Attachment #471870 -
Flags: review?(mconnor) → review+
Assignee | ||
Comment 14•14 years ago
|
||
(In reply to comment #13)
> * please just remove the "deprecated" strings. Cruft is bad.
Not a good idea since that will break Fx's integrated UI on the next merge. We should remove them only once the integrated UI has caught up with this. Filed bug 593427 as a follow-up.
> >+<!ENTITY setup.newSyncKeyPage.description.label "To ensure your total privacy, all of your data is encrypted prior to being uploaded. The key to decrypt your data is not uploaded.">
>
> not quite technically true, at this point. We generate insanely powerful keys
> to encrypt data, and wrap those keys with the key we don't update... we really
> don't want to get into a "well, the UI claims X, but X is a lie" state.
Right. I think what Faaborg means is "The Sync Key which is necessary to decrypt your data is not uploaded." Would that be ok?
Comment 15•14 years ago
|
||
Right I understand how it works (so that we only have to reencrpyt the actual key, not the whole chunk of data) bit ultimately it's the sync key that is used to encpyt access to your data. Text seems acurate enough for me, but we can tweak it.
Comment 16•14 years ago
|
||
That's the historical reason that it works that way, though we no longer do key-rewrapping in the general case. We can massage the wording, but saying something that isn't true feels like lighting a fuse.
cc-ing johnath, in the hopes that he has a better suggestion.
Comment 17•14 years ago
|
||
Comment on attachment 471873 [details] [diff] [review]
Part 3 (v2): Fix up Change Passphrase dialog
>+ doGeneratePassphrase: function () {
>+ let passphrase = gWeaveCommon.generatePassphrase();
>+ let el = document.getElementById("passphraseBox");
>+ el.value = gWeaveCommon.hyphenatePassphrase(passphrase);
>+ this._dialog.getButton("accept").disabled = false;
>+ },
I'm actually now wondering if we should move generatePassphrase and friends into Utils. File a followup to discuss?
>- validate: function (event) {
>+ validate: function() {
uber-nit: space before ()
Attachment #471873 -
Flags: review?(mconnor) → review+
Assignee | ||
Comment 18•14 years ago
|
||
(In reply to comment #17)
> I'm actually now wondering if we should move generatePassphrase and friends
> into Utils. File a followup to discuss?
Filed bug 593820.
Assignee | ||
Comment 19•14 years ago
|
||
Small rev to address review comment:
Changed setup.newSyncKeyPage.description.label to "The Sync Key which is necessary to decrypt your data is not uploaded." as proposed by myself.
Attachment #471870 -
Attachment is obsolete: true
Assignee | ||
Comment 20•14 years ago
|
||
Part 1: http://hg.mozilla.org/services/fx-sync/rev/0e2776059306
Part 2: http://hg.mozilla.org/services/fx-sync/rev/a5ee66c16761
Part 3: http://hg.mozilla.org/services/fx-sync/rev/6a615de6c55f
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Comment 21•14 years ago
|
||
When I enter my keyphrase/key I see what I type in the field. Before, when it was called a passphrase, it wasn't like that.
Assignee | ||
Comment 22•14 years ago
|
||
(In reply to comment #21)
> When I enter my keyphrase/key I see what I type in the field. Before, when it
> was called a passphrase, it wasn't like that.
This is intended. We want to encourage users to back up their Sync Key in a safe manner. Copy'n'paste is a common option which wouldn't work with a password field.
Comment 23•14 years ago
|
||
Verified fix on Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b7pre) Gecko/20100918 Firefox/4.0b7pre
Status: RESOLVED → VERIFIED
Updated•6 years ago
|
Component: Firefox Sync: UI → Sync
Product: Cloud Services → Firefox
You need to log in
before you can comment on or make changes to this bug.
Description
•