ruimarinho / authy-client

A complete Authy client with support for TOTP, OneTouch, Phone Verification and Phone Intelligence APIs
122 stars 27 forks source link

Phone Verification: is_ported property not always present in Authy response #19

Closed torfjor closed 7 years ago

torfjor commented 8 years ago

First of all, thanks for a fantastic library!

Seems like the beforementioned property is missing from some Authy responses, making assert throw:

$ curl 'https://api.authy.com/protected/json/phones/verification/start?' 
-d via='sms' 
-d phone_number='9XXXXXXX' 
-d country_code=47 
-d locale='nb' 
-H "X-Authy-API-Key: XXX"

Response:

{
"carrier":"Telenor Norge AS",
"is_cellphone":true,
"message":"SMS sendt til +47 9-XXX-XXXX.",
"seconds_to_expire":599,
"uuid":"557c6b40-6941-0134-ffdf-0ece2b9a2925",
"success":true
}
ruimarinho commented 8 years ago

Hi @torors, thanks for the feedback. While I assume you're correct, given that you've received a response without is_ported, Authy's API documentation states it is always returned. I have seen such inconsistencies before, so I have sent a support request for some feedback from them.

Until then, I ask for your patience and comprehension until I get an official confirmation.

holm commented 7 years ago

We just had our OneTouch support broken, because Authy slightly changed their callback request to have an int instead of a string. Can I ask why you do all this validation of the requests from Authy? In my eyes it doesn't really add any value, and only causes issues.

ruimarinho commented 7 years ago

Authy cannot simply change response types and expect everyone to be ok with it. That's a breaking change independently of how you look at that problem - the issue is not in the extensiveness of validation of this lib.

In your case, perhaps that change wasn't that dramatic, but imagine you were doing a date calculation using the returned integer value:

new Date(response.sent_at) // 1482712457663
// Mon Dec 26 2016 00:34:17 GMT+0000 (WET)

And you use this time to validate something important to the end user (number of attempts during a certain period?). The field is now returned as a string:

new Date(response.sent_at) // '1482712457663'
// Invalid Date

Would you prefer the lib to silently fail and let your application logic error out? From my experience, I prefer to be alerted early on.

ruimarinho commented 7 years ago

I still haven't received a word from Authy support on this, but looks like their decision is permanent. Added a PR to fix this.