Closed Bug 774300 Opened 12 years ago Closed 12 years ago

Sync authentication errors if passwords contain non-ASCII characters

Categories

(Firefox for Android Graveyard :: Android Sync, defect, P1)

ARM
Android
defect

Tracking

(firefox15+ fixed, firefox16+ verified)

VERIFIED FIXED
mozilla17
Tracking Status
firefox15 + fixed
firefox16 + verified

People

(Reporter: u363675, Assigned: rnewman)

References

Details

(Whiteboard: [qa+])

Attachments

(2 files, 1 obsolete file)

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
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
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
JSON decoding issue in J-PAKE transfer?
Hardware: All → ARM
(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.
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
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: nobody → rnewman
Status: NEW → ASSIGNED
Attached patch Proposed patch. v1 (obsolete) — Splinter Review
Reviewed by nalexander. Testing now, then I'll land and request uplift.
Attachment #648444 - Flags: review+
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".
Flags: in-litmus?
Whiteboard: [qa+]
Target Milestone: --- → mozilla17
And remove a test that I checked in to m-i by accident:

https://hg.mozilla.org/integration/mozilla-inbound/rev/6ecf2a75d1e5
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
Closed: 12 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.
(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
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+
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.
Product: Mozilla Services → Android Background Services
Product: Android Background Services → Firefox for Android
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: