Sync authentication errors if passwords contain non-ASCII characters

VERIFIED FIXED in Firefox 15

Status

Android Background Services
Android Sync
P1
normal
VERIFIED FIXED
5 years ago
4 years ago

People

(Reporter: Alexandre Provencio, Assigned: rnewman)

Tracking

(Depends on: 1 bug)

unspecified
mozilla17
ARM
Android
Dependency tree / graph
Bug Flags:
in-litmus ?

Firefox Tracking Flags

(firefox15+ fixed, firefox16+ verified)

Details

(Whiteboard: [qa+])

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

5 years ago
Created attachment 642601 [details]
trying to sync with a fresh firefox profile

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:13.0) Gecko/20100101 Firefox/13.0.1
Build ID: 20120615112143

Steps to reproduce:

- confirmed wether sync service was ok on https://services.mozilla.com/status/
- confirmed that I'm able to sync the firefox on my laptop (ubuntu) with another firefox on a deskop (mac)
- have never installed other android firefox versions
- have already hit 'clear your sync data' on https://account.services.mozilla.com/
- tried to take the addons off the sync
- tried the recover key method
- tried reinstalling android firefox
- tried to set up sync under two scenarios:
    1) my usual firefox account on my laptop
    2) a fresh new firefox profile on my laptop (logfile attached)


Actual results:

1) Sync seems to be working ok under settings->sync->Firefox and hitting 'sync now' generates no errors, but nothing is really synced
2) Sync generates a syncing problem message when doing the same test above and also nothing is synced


Expected results:

The tabs, bookmarks, history, etc. on my laptop should show on android's firefox

Updated

5 years ago
Component: General → Android Sync
Product: Firefox for Android → Mozilla Services
Version: Firefox 14 → unspecified
I see

W/GlobalSession( 4266): Aborting sync: User password has changed.

Are you sure you have the right password?
OS: Linux → Android
Priority: -- → P1
Hardware: x86_64 → All
(Reporter)

Comment 2

5 years ago
This is very odd because the android device never asks me to type a password, I only do that on the laptop. In addiction firefox on laptop was syncing ok with other desktop versions.

However I took you suggestion, and remembered my password had a 'ã' character on it, so I manage to change my password to a less secure but less error prone one, and... it's working! Even with my fully loaded profile.

Maybe you can explain better what really happened here, but my guess really is this accented letter. Hope this can help other users, and thanks a lot for your time :)
Alexandre, sounds good, glad to help.

Nick, is there client work to be done here to improve this?
Status: UNCONFIRMED → NEW
Ever confirmed: true
(In reply to Aaron Train [:aaronmt] from comment #3)
> Alexandre, sounds good, glad to help.
> 
> Nick, is there client work to be done here to improve this?

I can think of a few things:

1. we shouldn't create an Account with null credentials. Ever.
2. we could be surfacing Account errors better: prompting for credentials, or at least showing red sync failure.
3. we could QA that usernames/passwords/URLs with "exotic" characters work.

We can probably test problems with 3. in our test code.
I think Bug 709393 is most relevant.
Depends on: 709393, 709395
(Assignee)

Comment 6

5 years ago
JSON decoding issue in J-PAKE transfer?
Hardware: All → ARM
(Assignee)

Comment 7

5 years ago
(In reply to Nick Alexander :nalexander from comment #5)
> I think Bug 709393 is most relevant.

If the user set up with J-PAKE, we should have the correct password. That's why Bug 709393 has not been a high priority: incorrect credentials should be rare.
(Assignee)

Comment 8

5 years ago
Desktop does some really crappy encoding, and I'm sure it's tripping us up along the way. Writing a test now.
Summary: Sync problems → Sync authentication errors if passwords contain non-ASCII characters
(Assignee)

Updated

5 years ago
Duplicate of this bug: 779190
(Assignee)

Comment 10

5 years ago
Firstly, we need to use UTF-8 as the encoding in BasicScheme. I don't know why that would be the case.

Secondly, we need to get the un-mangled desktop password.

*sigh*
(Assignee)

Updated

5 years ago
Assignee: nobody → rnewman
Status: NEW → ASSIGNED
(Assignee)

Comment 11

5 years ago
Created attachment 648444 [details] [diff] [review]
Proposed patch. v1

Reviewed by nalexander. Testing now, then I'll land and request uplift.
Attachment #648444 - Flags: review+
(Assignee)

Comment 12

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/3f0334d53fd2

I verified this with J-PAKE pairing an inbound build with a non-ASCII password.

QA, please verify this when it lands, and ensure that our test suite includes passwords like "éfoöb£ar".
status-firefox15: --- → affected
status-firefox16: --- → affected
Flags: in-litmus?
Whiteboard: [qa+]
Target Milestone: --- → mozilla17
(Assignee)

Comment 13

5 years ago
And remove a test that I checked in to m-i by accident:

https://hg.mozilla.org/integration/mozilla-inbound/rev/6ecf2a75d1e5
(Assignee)

Comment 14

5 years ago
Created attachment 648614 [details] [diff] [review]
Patch for Aurora. v1

Patch for future uplift.
Attachment #648444 - Attachment is obsolete: true
Attachment #648614 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/3f0334d53fd2
https://hg.mozilla.org/mozilla-central/rev/6ecf2a75d1e5
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Just so I am clear, the STR's are:

1) Create an account with a non-ASCII password such as "éfoöb£ar"
2) Pair mobile device, via J-PAKE, to that account

Pairing and sync should just work.
(Assignee)

Comment 17

5 years ago
(In reply to Tracy Walker [:tracy] from comment #16)
> Just so I am clear, the STR's are:
> 
> 1) Create an account with a non-ASCII password such as "éfoöb£ar"
> 2) Pair mobile device, via J-PAKE, to that account
> 
> Pairing and sync should just work.

Exactly that.
looks good on latest m-c build
Status: RESOLVED → VERIFIED
(Assignee)

Comment 19

5 years ago
Comment on attachment 648614 [details] [diff] [review]
Patch for Aurora. v1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
  Oooold code.

User impact if declined: 
  Users with non-ASCII passwords won't be able to successfully set up Sync.

Testing completed (on m-c, etc.): 
  QA verified on m-c (thanks tracy!). Covered by unit tests.

Risk to taking this patch (and alternatives if risky): 
  Slim. Should have blown up catastrophically if there were any problems.

String or UUID changes made by this patch: 
  None.
Attachment #648614 - Flags: approval-mozilla-beta?
Attachment #648614 - Flags: approval-mozilla-aurora?
Comment on attachment 648614 [details] [diff] [review]
Patch for Aurora. v1

letting users use non-ASCII chars in their passwords is pretty important, approving for branches.
Attachment #648614 - Flags: approval-mozilla-beta?
Attachment #648614 - Flags: approval-mozilla-beta+
Attachment #648614 - Flags: approval-mozilla-aurora?
Attachment #648614 - Flags: approval-mozilla-aurora+
tracking-firefox15: --- → +
tracking-firefox16: --- → +
(Assignee)

Comment 21

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/2454d9517b7b
https://hg.mozilla.org/releases/mozilla-beta/rev/bd0b4c2ac18e
status-firefox15: affected → fixed
status-firefox16: affected → fixed
(Assignee)

Comment 22

5 years ago
Tracy, would you be so kind as to verify Aurora and Beta when these changes filter through?
looks good Aurora (desktop) to Aurora (mobile)

Had a little scare with Beta 'til rnewman reminded me beta won't have the fix 'til tomorrow Fx15b5.
status-firefox16: fixed → verified
Component: Android Sync → Android Sync
Product: Mozilla Services → Android Background Services
You need to log in before you can comment on or make changes to this bug.