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

Integrate the Mozilla China phone number verification screens behind a feature flag #583

Closed
shane-tomlinson opened this issue Dec 7, 2018 · 6 comments

Comments

@shane-tomlinson
Copy link
Member

@shane-tomlinson shane-tomlinson commented Dec 7, 2018

The Mozilla China stack has phone number verification screens, we should add, behind a feature flag, so that team can stop worrying about doing it every time we update the content server.

This feature is only enabled for OAuth reliers.

┆Issue is synchronized with this Jira Task
┆Issue Number: FXA-730

@shane-tomlinson
Copy link
Member Author

@shane-tomlinson shane-tomlinson commented Dec 7, 2018

@l-hedgehog
Copy link
Member

@l-hedgehog l-hedgehog commented Jan 30, 2019

Hi, just to clarify, since we're currently using a fork of mozilla-services/msisdn-gateway but the instance at https://msisdn.services.mozilla.com/ is long gone, when preparing my PR, I should propose any necessary changes directly in fxa-auth-server instead of assuming that service could be revived, right?

@dannycoates dannycoates transferred this issue from mozilla/fxa-content-server Apr 3, 2019
@davismtl
Copy link
Contributor

@davismtl davismtl commented Jun 4, 2019

I recommend we look into re-prioritizing this and even enabling it for a lot of markets.

I see this having many benefits:

  • could help reduce spammy accounts since phone numbers are harder to burn through (think AMO problems with spammy webextension submissions)
  • could help for login if the primary email is lost (we could send a link / code by SMS instead)
  • could be an option for 2FA, instead of email and TOTP
  • combined with other data, it could lead to a path forward for TOTP recovery (though alone, it's not secure enough to recover TOTP)
@shane-tomlinson
Copy link
Member Author

@shane-tomlinson shane-tomlinson commented Jun 5, 2019

TL;DR - Yes, we need to expand SMS capabilities, however, @l-hedgehog implemented a phone number verification system that is tailored to Mozilla China's needs, including where verifying control of a phone number takes place in the flow. I would be extremely hesitant to use it for the rest of the world in its current form.

As Hector points out above, their logic is is based using the no-longer maintained mozilla-services/msisdn-gateway to send SMS and verify control of a particular phone number.

There are some things that stand out about this approach:

  1. The msisdn-gateway hasn't been updated in 4 years.
  2. The msisdn-gateway has a totally different use case than our needs, in FxA terms, it is the auth server, it authenticates users by sending SMS's. Since it's an authentication system, it has to return a proof of identity. The proof it uses is a BrowserID assertion, which we are actively trying to remove all traces of. Because msisdn-gateway is a full authentication system, it involves a lot of complexity we just don't need.
  3. We already know how to send SMS. We send lots of them.
    • While we've talked about extracting an SMS sending service from the auth-server, msisdn-gateway is not the generic SMS sending service we hoped for.
  4. It has been a long while since looking at the code, but I remember seeing some client side logic I wished were on the server.
@davismtl
Copy link
Contributor

@davismtl davismtl commented Jun 5, 2019

@shane-tomlinson
Am I understanding correctly that I should file a separate bug since the China integration doesn't seem ideal for the use cases I described?

@shane-tomlinson
Copy link
Member Author

@shane-tomlinson shane-tomlinson commented Jun 6, 2019

@shane-tomlinson
Am I understanding correctly that I should file a separate bug since the China integration doesn't seem ideal for the use cases I described?

Ya.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants