Closed Bug 1568336 Opened 5 years ago Closed 5 years ago

Share authentication information in the Cohabiting state

Categories

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

Unspecified
Android
defect

Tracking

(firefox-esr60 wontfix, firefox-esr6869+ fixed, firefox68 wontfix, firefox69 fixed, firefox70 fixed)

RESOLVED FIXED
Firefox 70
Tracking Status
firefox-esr60 --- wontfix
firefox-esr68 69+ fixed
firefox68 --- wontfix
firefox69 --- fixed
firefox70 --- fixed

People

(Reporter: Grisha, Assigned: Grisha)

References

Details

(Whiteboard: [fennec68.1])

Attachments

(2 files)

AuthStateProvider's current behaviour is that it only publishes sessionToken, kSync and kXCS values if it finds the account in the "Married" state. Unfortunately, this isn't quite correct. Cohabiting state has all of the necessary bits of information, and we could easily find the account in that state.

In Fennec, as far as I can tell, there are two main reasons we can find ourselves in a Cohabiting state:

  • certificate is stale
  • we have bad credentials

Both of these scenarios may happen:

  • as we're obtaining a token from the token server using a BrowserID assertion - this happens during every sync. Or...
  • as we're obtaining a scoped auth token using a BrowserID assertion - I can't recall if we actually use this code; I don't think we do. This is part of Android's AbstractAccountAuthenticator.

In both cases, BrowserID assertion is generated as part of the Married state, and it relies on a valid certificate, which is generated while we're in the Cohabitating state.

Maximum value for the certificate duration is 24hrs, according to https://github.com/mozilla/fxa-auth-server/blob/master/docs/api.md#request-body-27.
We request for a certificate to last 12hrs:
https://dxr.mozilla.org/mozilla-central/source/mobile/android/services/src/main/java/org/mozilla/gecko/fxa/authenticator/FxAccountAuthenticator.java?q=FxAccountAuthenticator.java&redirect_type=direct#139

However, we do not obtain a certificate every time we need to generate an assertion. We "cache" the certificate as part of data bound to the state.

Once we get to the Married state (meaning that we were able to obtain a valid certificate, with good credentials around good), we stay in it - certificate is now part of the persisted state data. Next time we sync maybe after our certificate has expired, in which case we'll transition into a Cohabiting state:
https://dxr.mozilla.org/mozilla-central/source/mobile/android/services/src/main/java/org/mozilla/gecko/fxa/sync/FxAccountSyncAdapter.java?q=FxAccountSyncAdapter.java&redirect_type=direct#437

At that point, if the AuthStateProvider is queried, it'll find the (perfectly fine) account in a Cohabiting state, and currently will refuse to provide credentials.

If another sync is performed, we will obtain a fresh certificate in the Cohabiting state, transition into Married state, succeed in obtaining a token from the token server, and then sync using that token.

If the AuthStateProvider is queried again then, it'll see the account is in Married state.

So in essence, the current behaviour of AuthStateProvider works fine if the browser is frequently used. In that scenario, stale certificates are regenerated as needed as part of regular syncing. This is why we didn't uncover this bug during initial testing, and this is subtle enough to slip in the review.

Another angle here is that a 401 from the token server may, in theory, also mean that we have bad credentials - not just a stale certificate. In practice, I don't believe that to be the case, but need to confirm.

It also doesn't matter very much - clients of AuthStateProvider will quickly learn that the credentials they've obtained are no longer any good, and they're expected to handle this scenario gracefully.

Blocks: 1545232

Another angle here is that a 401 from the token server may, in theory, also mean that we have bad credentials

  • not just a stale certificate. In practice, I don't believe that to be the case, but need to confirm.

This is rare but it can happen, e.g. if you change password on another device, then tokenserver might reject an otherwise-valid certificate because it can tell it was generated using a now-invalid sessionToken.

because [tokenserver] can tell [certificate] was generated using a now-invalid sessionToken

This is the bit that I wasn't certain about, thanks for clarifying.
IIRC the storage server may accept token server's token that was obtained using a now-invalid sessionToken, but that's one layer removed from this so not immediately important.

Pushed by gkruglov@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6edd17669d3f
Share credentials in Cohabiting as well as Married states r=nalexander
Assignee: nobody → gkruglov
Status: NEW → ASSIGNED
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 70

Beta/Release Uplift Approval Request

  • User impact if declined: Fenix (and Lockwise) applications that use auto-login functionality will only be able to read authenticated data from Fennec if it was recently synchronized.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This expands when Fennec is allowed to share the authenticated state, from just a single state ("married") to two states (adding "cohabitating"). Interaction between those two states are well-understood and documented in this bug. There should be no risk either to Fennec or to consuming clients.
  • String changes made/needed: n/a
Attachment #9081809 - Flags: approval-mozilla-beta?
Attachment #9080192 - Flags: approval-mozilla-beta?

Grisha, is your FxA fix only needed for Fenix? Or do we need to uplift your fix for Fennec (ESR 68), too?

ESR 68 is the last major version of Fennec, so we need to uplift all Fennec fixes to mozilla-esr68. We build Fennec in mozilla-central and mozilla-beta, but we won't ship Fennec versions >= 69 to users.

Flags: needinfo?(gkruglov)
Priority: -- → P1

Let's uplift this to ESR68. This fix lives in Fennec, but it's needed for Fenix and Lockwise (since they're consumers of the Fennec-hosted AuthStateProvider).

Flags: needinfo?(gkruglov)
Attachment #9080192 - Flags: approval-mozilla-beta?
Comment on attachment 9081809 [details] [diff] [review]
auth_state_cohabitating.patch

Improves Fennec and other mobile apps' ability to share the authenticated state. Approved for GV69 and Fennec 68.1b5.
Attachment #9081809 - Flags: approval-mozilla-esr68+
Attachment #9081809 - Flags: approval-mozilla-beta?
Attachment #9081809 - Flags: approval-mozilla-beta+

Grisha says this fix is a good ride-along candidate for a Fennec ESR 68.0.z dot release, if we release another one in August before 68.1.

Adding [fennec68.1] whiteboard tag because this fix will ship in Fennec ESR 68.1 (September 3) unless we have the opportunity to ship it as a ridealong in a Fennec ESR 68.0.z dot release.

OS: Unspecified → Android
Whiteboard: [fennec68.1]

Hi, can you provide some STR for this issue? Or can you check the provided ones from below and clarify if it's the case? The information would help in having the testing done thoroughly.
As per my understanding after going through the ticket details.

Prerequisites:

  • Have a Firefox account

Steps to reproduce:

  1. Open Fennec and login into twitter.
  2. Save the credentials.
  3. Wait for 24h.(Maximum value for the certificate duration)
  4. Make a sync when logging into account.
  5. Open Lockwise application.
  6. Login with fennec account.
  7. Refresh the "All logins" list in order to make a sync.

Expected Result: After refresh the saved credentials from Fennec are present in the "All logins" list from Lockwise application.

Note:

  • The Twitter application serves as an example.

Thanks

Flags: needinfo?(gkruglov)

Hi Diana,

This bug is related to supporting the FxA auto-login functionality in the Firefox Preview browser (Fenix). If a user has Fennec installed that's logged into FxA and they install and launch Fenix, as part of Fenix onboarding they will see an auto-login prompt message, something like this: "would you like to sign-in into your grisha@mozilla.com account from another Firefox browser on this device?"

This bug fixed a flaw in Fennec:

  • if Fennec wasn't frequently used, it will fail to provide FxA information to Fenix

So steps to reproduce for this are:

  • install Fennec, make sure it's logged into FxA
  • install Fenix, launch it, make sure you see an auto-login prompt in the onboarding, and that you're able to log into FxA following this prompt
  • don't use Fennec for 24hours
  • re-install Fenix (onboarding is only displayed once, I think) launch it, make sure you see an auto-login prompt in the onboarding, and that you're able to log into FxA following this prompt
Flags: needinfo?(gkruglov)

Hi, I checked this issue now, with Sony Xperia Z5 (Android 7) i was able to log in on Firefox Preview Nightly 200320 (Build #208006605).
Fennec/Firefox Release 68.6.0 was not used for a day, Fenix/Firefox Preview Nightly has been installed with the latest build, the account with which i logged in Fennec was present on the onboarding screen right after the Welcome to Firefox message.

Note: I will do a check on Lockwise as well, will comeback with the result.

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

Created:
Updated:
Size: