Sync authentication errors if passwords contain non-ASCII characters

VERIFIED FIXED in Firefox 15

Status

()

defect
P1
normal
VERIFIED FIXED
7 years ago
2 years ago

People

(Reporter: alexandreprovencio, Assigned: rnewman)

Tracking

(Depends on 1 bug)

unspecified
mozilla17
ARM
Android
Points:
---
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

7 years ago
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
Reporter

Comment 2

7 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

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

Comment 7

7 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

7 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

7 years ago
Duplicate of this bug: 779190
Assignee

Comment 10

7 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

7 years ago
Assignee: nobody → rnewman
Status: NEW → ASSIGNED
Assignee

Comment 11

7 years ago
Posted patch Proposed patch. v1 (obsolete) — Splinter Review
Reviewed by nalexander. Testing now, then I'll land and request uplift.
Attachment #648444 - Flags: review+
Assignee

Comment 12

7 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".
Flags: in-litmus?
Whiteboard: [qa+]
Target Milestone: --- → mozilla17
Assignee

Comment 13

7 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

7 years ago
Patch for future uplift.
Attachment #648444 - Attachment is obsolete: true
Attachment #648614 - Flags: review+
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

7 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

7 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+
Assignee

Comment 22

7 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.
Product: Mozilla Services → Android Background Services

Updated

2 years ago
Product: Android Background Services → Firefox for Android
You need to log in before you can comment on or make changes to this bug.