search

google / exposure-notifications-verification-server

Verification component for COVID-19 Exposure Notifications.
Apache License 2.0
233 stars 83 forks source link

SMS sending of verification codes #117

Closed mikehelmick closed 4 years ago

mikehelmick commented 4 years ago

TL;DR

Add the ability to send verification codes over SMS

https://www.twilio.com/messaging as likely baked in provider. Must be configurable so operator can bring their own credentials.

/kind enhancement /priority important-soon

google-oss-robot commented 4 years ago

@mikehelmick: The label(s) priority/important-soon cannot be applied, because the repository doesn't have them

In response to [this](https://github.com/google/exposure-notifications-verification-server/issues/117): >### TL;DR > >Add the ability to send verification codes over SMS > >https://www.twilio.com/messaging as likely baked in provider. Must be configurable so operator can bring their own credentials. > >/kind enhancement >/priority important-soon Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
mikehelmick commented 4 years ago

One other note - best if the phone numbers don't get stored in the database.

sethvargo commented 4 years ago

I have some experience with the Twilio API, so I can take this

sethvargo commented 4 years ago

/assign

sherifkozman commented 4 years ago

Twilio has a some considerations to be noted if it is used for an impelemntation that is not in the US. i.e : Creating a Numeric or Alpha Numeric Sender ID would be a requirement in some destinations and verifying that the server would use those values when posting a payload. https://support.twilio.com/hc/en-us/articles/223133767-International-support-for-Alphanumeric-Sender-ID

sethvargo commented 4 years ago

I think that's fine. The server operator would need to take those steps, but I don't think it would affect our code. Can you share more about your concerns?

sherifkozman commented 4 years ago

I am only noting that the Sender ID (i.e numeric or alpha numeric) is a value that should be factored in when integrating with twilio to ensure portability of the service outside of the US. So it would be set as an ENV variable for example the same way you add Twilio API Key and the application SID. Twilio mostly doesnt handle overriding that value on behalf of the sender.

mikehelmick commented 4 years ago

The SMS config will need to be in the database and configured on a per-local basis. See #104

mikehelmick commented 4 years ago

Follow up from previous comment

SMS provider and use should be database driven. If there is no provider configured, then the dialog shouldn't be shown.

As we go multi-tenant, this is important since it is possible that different "realms" will use different SMS config and some might not use SMS at all.

sethvargo commented 4 years ago

No no, bad GitHub. That was only part 1 of 1000.

mikehelmick commented 4 years ago

/assign

sethvargo commented 4 years ago

/assign