search

OriginProtocol / origin-bridge

We've moved to a monorepo: https://github.com/OriginProtocol/origin/tree/master/infra/bridge
MIT License
15 stars 7 forks source link

Appropriately Handle International Phone Numbers #95

Closed micahalcorn closed 6 years ago

micahalcorn commented 6 years ago

The bridge server uses Twilio for phone attestations. We currently only attempt to send an SMS message and otherwise raise an exception. Twilio does not support SMS in all countries. The attestation service should account for the availability of services for the phone number and handle voice-based verification where possible.

Related: https://github.com/OriginProtocol/origin-dapp/issues/287 & https://github.com/OriginProtocol/origin-dapp/issues/286

tomlinton commented 6 years ago

Twilio has an API for phone verification that handles voice verification including different languages. It doesn't look like it has the limits on SMS availability (at least I can't find anything). Any thoughts on changing the bridge server over to using that?

It seems like a good idea to then offer a choice of voice or SMS in the dapp.

micahalcorn commented 6 years ago

Thanks, @tomlinton. I think that we probably need to detect the reason for the failure, fall back to the voice-based verification, and then handle additional failures appropriately. Let me know if you decide to take this on.

tomlinton commented 6 years ago

Sorry @micahalcorn I don't think I explained that very well. The Twilio Verify API does SMS and voice verification (you specify in the API request). Because you aren't using your own Twilio number the limitations on SMS sending from certain countries don't exist. So this exception won't exist anymore. Of course there are a bunch of other failures to handle like bad API keys.

We then don't have to consider the availability of SMS or voice for the phone number because the verify API can handle either in every case. The only disadvantage is the Verify API costs more than sending an SMS from your own Twilio number.

micahalcorn commented 6 years ago

@tomlinton ah, got it. 👍 @wanderingstan ?

wanderingstan commented 6 years ago

Thanks @tomlinton , didn't know about the dedicated purpose verification API. That's perfect.

@micahalcorn can we make an issue to switch over to that? I don't see it being a priority right away, but something we can put out to the community (a slightly advanced "good first issue"?), and/or pick it up ourselves in a few weeks.

tomlinton commented 6 years ago

I'm happy to take it on, I've been testing it out to make sure it'd work so I've done a fair bit of the work implementing it already :joy:

micahalcorn commented 6 years ago

@tomlinton I think we're on board but let me confirm and get back to you.

wanderingstan commented 6 years ago

That's awesome @tomlinton that you've already tried it out. Want to take it on officially?

@joshfraser also gave the okay. Let us know what changes need to be made on the Twilio admin side by us.

tomlinton commented 6 years ago

@wanderingstan yep I'll take it on and also put together some instructions for the Twilio changes.

tomlinton commented 6 years ago

Done by #113 :heavy_check_mark: