Problem sending SMS for OPT in Twillio

[dmorfav]

Bug report

Problem sending SMS for OPT verification with twillio service

Describe the bug

When the service is activated to authenticate with Twilio activating the OPT verification will generate an error saying: Error sending confirmation sms: json: cannot unmarshal number into Go struct field SmsStatus.status of type string

To Reproduce

1. Configure Twillio Integration in Settings
2. Disable Enable phone confirmations
3. Try to register with phone and password
4. Go to Settings -> Authentication -> User and send OTP for user registered in previous step
5. Get the error: "code": 400, "msg": "Error sending confirmation sms: json: cannot unmarshal number into Go struct field SmsStatus.status of type string"

Expected behavior

This should allow the confirmation to be sent, even if the "Enable phone confirmations" option is activated, it does not allow registration, throwing the same error

Screenshots

System information

Ionic CLI : 6.19.0 (/usr/lib/node_modules/@ionic/cli) Ionic Framework : @ionic/angular 6.1.3 @angular-devkit/build-angular : 13.2.6 @angular-devkit/schematics : 13.2.6 @angular/cli : 13.2.6 @ionic/angular-toolkit : 6.1.0

Capacitor:

Capacitor CLI : 3.5.0 @capacitor/android : not installed @capacitor/core : 3.5.0 @capacitor/ios : not installed

Utility:

cordova-res : not installed globally native-run : 1.5.0

System:

NodeJS : v16.14.2 (/usr/bin/node) npm : 8.7.0 OS : Linux 5.10

[w0wka91]

I will have a look at this one if thats okay

[J0]

@w0wka91

Go for it! Feel free to let us know if you need any help :)

[w0wka91]

Is there any way to test or debug this locally?

[dmorfav]

Hi, i test the send a SMS with curl and this is the structure of response:

{"sid": "SM83c95095a2464122a90418b9765f65fc", "date_created": "Sat, 07 May 2022 04:45:11 +0000", "date_updated": "Sat, 07 May 2022 04:45:11 +0000", "date_sent": null, "account_sid": "$ACCOUNT_SID", "to": "$TO_NUMBER", "from": "$FROM_NUMBER", "messaging_service_sid": null, "body": "Sent from your Twilio trial account - This will be the body of the new message!", "status": "queued", "num_segments": "1", "num_media": "0", "direction": "outbound-api", "api_version": "2010-04-01", "price": null, "price_unit": "USD", "error_code": null, "error_message": null, "uri": "URL.json", "subresource_uris": {"media": "URL.json"}}

I change the personal values with $VAR

I follow this example

[w0wka91]

I was able to reproduce the error message the following way:

1. Configure Twillio Integration with INVALID credentials in Settings
2. Disable Enable phone confirmations
3. Register with phone and password
4. Settings -> Authentication -> User - Send OTP for user registered in previous step

This will trigger an HTTP-Call to a non existent Twilio Endpoint and according to the Twilio Docs the API returns exceptions in the HTTP response body when something goes wrong. https://www.twilio.com/docs/usage/twilios-response#response-formats-exceptions. At the moment we are only handling 400 responses: https://github.com/supabase/gotrue/blob/master/api/sms_provider/twilio.go#L76 What are your thoughts about that?

@dmorfav Are you sure you supplied correct credentials?

[dmorfav]

@w0wka91 I have recreated the credentials in twilio and if everything has worked correctly, I apologize for the inconvenience and thanks for the collaboration.

[w0wka91]

I think we shouldnt just close this bug but rather deal with these exceptions correctly. Maybe we could display an error if we get an 404: "Cannot find Twilio Endpoint. Please check credentials." What do you think @J0 ?

[J0]

Hey @w0wka91,

Apologies for the delayed reply -- missed this. I'm open to the idea of supporting 404 errors but would like to consider other options as well. Let me check with the team and get back to you. Will reopen the issue for now

cc @kangmingtay @thorwebdev

[kangmingtay]

@w0wka91 i think we can just treat any non-2xx status codes as an error and return the error message that twilio sends to the end user rather than coming up with our own error messages for each error code. But yeah, the fix here should be simple, we can just change it to if res.StatusCode/100 != 2

[J0]

Fixed by #515