Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make auth failure during device registration drive FxA to "separated". #5756

Conversation

rfk
Copy link
Contributor

@rfk rfk commented Nov 13, 2019

Fixes #5755. Sorry, I don't have my build environment set up in order to test this, but I figured I'd at least put up a draft PR to communicate the intent of that issue.

Previously, an auth failure during device registration would drive the
FxA state machine to "doghouse", which is a special terminal state for
when the FxA server tells the client that it needs to upgrade itself
before it can continue using the FxA APIs.

The appropriate state for auth failures is "separated", which prompts
the user to re-enter their password to reconnect to the account.

Previously, an auth failure during device registration would drive the
FxA state machine to "doghouse", which is a special terminal state for
when the FxA server tells the client that it needs to upgrade itself
before it can continue using the FxA APIs.

The appropriate state for auth failures is "separated", which prompts
the user to re-enter their password to reconnect to the account.
@garvankeeley
Copy link
Contributor

Thanks, I can certainly land this (the code is fine), what should we do about testing this change?

@rfk
Copy link
Contributor Author

rfk commented Nov 15, 2019

I'm not familiar enough with this codebase to be able to say much about testing in practice, I'm afraid :-(

IIUC, this device-registration function gets called by Account.updateDeviceName. One way to test by hand might be to:

  • Sign in to Firefox for iOS with your Firefox Account, sync things, generally get to a stable stable.
  • Open the settings page in Firefox for iOS to see the device name
  • Visit https://accounts.firefox.com/settings/clients on another device and disconnect Firefox for iOS, destroying its sessionToken
  • Quickly go back and change the device name in Firefox for iOS

I think this should trigger the codepath above. Before the change, I'm not entirely sure what the user would see but I'd expect some sort of "you need to update your browser" messaging. After the change I'd expect to see "Enter your password to reconnect" messaging.

(Sorry I don't currently have an iOS device capable of running the latest Firefox for iOS in order to test this out myself)

@garvankeeley
Copy link
Contributor

I see no difference with or without the changed code. The main problem is that without the device being registered for push notifications, we never show the device as disconnected (yikes!).

@garvankeeley
Copy link
Contributor

This is still marked as a draft, do you want to remove that label and I can land this change?

@rfk rfk marked this pull request as ready for review November 25, 2019 01:11
@rfk
Copy link
Contributor Author

rfk commented Nov 25, 2019

Thanks, done 👍

@garvankeeley garvankeeley merged commit ca3b34d into mozilla-mobile:master Nov 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Auth failures during device registration should drive FxA to "separated", not "doghouse".
2 participants