Closed
Bug 1142596
Opened 8 years ago
Closed 8 years ago
Cache reading list oauth tokens
Categories
(Android Background Services Graveyard :: Reading List Sync, defect)
Tracking
(firefox38 fixed, firefox39 fixed)
RESOLVED
FIXED
Firefox 39
People
(Reporter: nalexander, Assigned: nalexander)
References
Details
Attachments
(1 file)
Right now we: 1) ensure the Firefox Account is in a healthy state; 2) generate an FxA assertion; 3) exchange the assertion for an oauth token. All before syncing the RL! Oauth tokens are long lived. We should invert the flow of control to just "ask for an oauth token", and push the FxA mangling out of the RL sync flow.
Assignee | ||
Comment 1•8 years ago
|
||
rnewman: over to you. I elected to use the Android framework for this, mostly 'cuz I wanted to see how it worked in the case it was intended to handle. It's fine, although having to maintain the oauth token in order to invalidate it is irritating. Pay attention to the two layers of token invalidation (one at the oauth layer, one moving the account state backwards). It's challenging to test the latter because "obviously bogus" certificates (like those produced by the debug helper I added) trigger a 400 from the oauth endpoint, not a 401. (This is wrong, and rfkelly agrees, but c'est la vie for now.) This yields a nice simplification of the RL Sync Adapter which suggests the token approach is reasonable. The complete absence of automated tests is a function of the compressed schedule and the difficulty of testing the interactions across the full stack. Manual testing with the debug utilities gives me some confidence in the mechanism, however; and it will get more testing as I implement the remaining follow-ups.
Comment 2•8 years ago
|
||
Comment on attachment 8582918 [details] [review] Link to Github pull-request: https://github.com/mozilla-services/android-sync/pull/540 See GitHub comments.
Attachment #8582918 -
Flags: review?(rnewman) → review+
Assignee | ||
Comment 3•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/6c3fd5d5aa40
Comment 4•8 years ago
|
||
Backed out for bustage. https://hg.mozilla.org/integration/fx-team/rev/cb309087f978 https://treeherder.mozilla.org/logviewer.html#?job_id=2452077&repo=fx-team
Assignee | ||
Comment 6•8 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/91e52cfb333a
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox39:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 39
Comment 7•8 years ago
|
||
Comment on attachment 8582918 [details] [review] Link to Github pull-request: https://github.com/mozilla-services/android-sync/pull/540 Batch uplift of Android RL to 38.
Attachment #8582918 -
Flags: approval-mozilla-beta?
Updated•8 years ago
|
status-firefox38:
--- → affected
Updated•8 years ago
|
Attachment #8582918 -
Flags: approval-mozilla-beta?
You need to log in
before you can comment on or make changes to this bug.
Description
•