1581709 - Stop using BrowserID for OAuth requests (to help the server emit accurate sync activity metrics)

Status: RESOLVED FIXED

(Firefox :: Firefox Accounts, enhancement)

Description

[Ryan Kelly [:rfkelly]]

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

[Shane Tomlinson [:stomlinson]]

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

Comment 2

[Edouard Oger [:eoger]]

Attachment: Bug 1581709 - Use sessionTokens for OAuth requests.

Comment 3

[Ryan Kelly [:rfkelly]]

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.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6852bdb8d8c5
Use sessionTokens for OAuth requests. r=vladikoff

Comment 5

[Razvan Maries]

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

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b7f97ca6620e
Use sessionTokens for OAuth requests. r=vladikoff

Comment 7

[Edouard Oger [:eoger]]

FWIW the lint failure was not caused by my patch.

Comment 8

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/b7f97ca6620e