Stop using BrowserID for OAuth requests (to help the server emit accurate sync activity metrics)
Categories
(Firefox :: Firefox Accounts, enhancement)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox71 | --- | fixed |
People
(Reporter: rfkelly, Assigned: eoger)
References
Details
Attachments
(1 file)
As described in Bug 1577690 Comment 7, we'd like to signal to the FxA backend whether or not the browser is using Sync. The initial proposal was to send a ?service=sync query param in requests to /certificate/sign, but from ensuing slack conversation I think there's a better way.
We eventually want to move away from BrowserID assertions, but can't boil that ocean all at once. What we can do is make it so that they're only used for sync. Then the server can safely assume that any call to /certificate/sign is in service of something that's connected to sync -- either an older browser for which being signed in means you're always syncing, or a new browser where the user has deliberately opted in to sync. That will give us enough info on the backend to continue emitting appropriate metrics in the short term.
AFAICT there are only two other places in the browser where we make BrowserID assertions, and both of them can be changed not to do that:
- This code currently calls oauth.accounts.firefox.com/v1/authorization, but should instead call api.accounts.firefox.com/v1/oauth/token using
grant_type=fxa-credentials. - This code currently calls oauth.accounts.firefox.com/v1/authorization, but should instead call api.accounts.firefox.com/v1/oauth/authorization.
We should do this as part of the decoupling work, to help FxA emit accurate sync activity metrics.
Shane, could you please gut-check whether this proposal makes sense?
|
||
Comment 1•5 years ago
|
||
Shane, could you please gut-check whether this proposal makes sense?
This proposal makes sense from the Desktop P.O.V. Does Fennec still use /certificate/sign? I can't find any obvious references to it, but am not particularly well versed at searching Fennec code.
There are however definite uses of assertions to create OAuth tokens in Firefox for iOS. See https://github.com/mozilla-mobile/firefox-ios/blob/de66de9f664a5dd0282961297fd2e15fac0ca5fc/Account/FxAClient10.swift#L601
|
Assignee | |
Comment 2•5 years ago
|
||
|
Reporter | |
Comment 3•5 years ago
|
||
This proposal makes sense from the Desktop P.O.V. Does Fennec still use /certificate/sign?
Yes it does; however, Fennec is also not getting the "decouple" treatment, so I think it's fine to assume its uses of that endpoint are "sync-related" for metrics purposes.
|
|
Reporter | |
Updated•5 years ago
|
Pushed by eoger@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/6852bdb8d8c5 Use sessionTokens for OAuth requests. r=vladikoff
|
||
Comment 5•5 years ago
|
||
Backed out for lint failure.
Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=6852bdb8d8c5e1a09434c052e3d046a9fc7d5443
Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=270301450&repo=autoland&lineNumber=864
Backout: https://hg.mozilla.org/integration/autoland/rev/14648bab1262dad0a388db6ab58d909e1ce3104e
Pushed by eoger@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b7f97ca6620e Use sessionTokens for OAuth requests. r=vladikoff
|
|
Assignee | |
Comment 7•5 years ago
•
|
||
FWIW the lint failure was not caused by my patch.
|
||
Comment 8•5 years ago
|
||
| bugherder | ||
Description
•