RestComm / Restcomm-Connect

The Open Source Cloud Communications Platform
http://www.restcomm.com/
GNU Affero General Public License v3.0
242 stars 215 forks source link

When buying an SMS Only Nexmo DID the capabilities stored in DB are incorrect #631

Open deruelle opened 8 years ago

deruelle commented 8 years ago

Example here curl https://cloud.restcomm.com/restcomm/2012-04-24/Accounts/XXXX/IncomingPhoneNumbers.json [ { "sid": "PN276b1f326beb40a6ae4ea6b1cd50aaca", "account_sid": "XXXX", "friendly_name": "+14156537711", "phone_number": "14156537711", "voice_url": "/restcomm-rvd/services/apps/Nanda\u0027s%20App/controller", "voice_method": "POST", "voice_fallback_url": null, "voice_fallback_method": "POST", "status_callback": null, "status_callback_method": "POST", "voice_caller_id_lookup": false, "voice_application_sid": null, "date_created": "Mon, 9 Nov 2015 16:36:49 +0000", "date_updated": "Mon, 9 Nov 2015 16:36:49 +0000", "sms_url": null, "sms_method": "POST", "sms_fallback_url": null, "sms_fallback_method": "POST", "sms_application_sid": null, "capabilities": { "voice_capable": false, "sms_capable": false, "mms_capable": false, "fax_capable": false }, "api_version": "2012-04-24", "uri": "/restcomm/2012-04-24/Accounts/XXXX/IncomingPhoneNumbers/PN276b1f326beb40a6ae4ea6b1cd50aaca.json" }

otsakir commented 8 years ago

I confirm the issue after a test on staging.

With this request:

https://staging.restcomm.com/restcomm/2012-04-24/Accounts/.../AvailablePhoneNumbers/US/Local.json?
AreaCode=209&
FaxEnabled=true&
RangeIndex=1&
RangeSize=10&
SmsEnabled=true&
VoiceEnabled=true

i got the following:

{
  "sid": "...",
  "account_sid": "...",
  "friendly_name": "+12092693528",
  "phone_number": "12092693528",
  "voice_url": null,
  "voice_method": "POST",
  "voice_fallback_url": null,
  "voice_fallback_method": "POST",
  "status_callback": null,
  "status_callback_method": "POST",
  "voice_caller_id_lookup": false,
  "voice_application_sid": null,
  "date_created": "Mon, 18 Jan 2016 13:33:16 +0000",
  "date_updated": "Mon, 18 Jan 2016 13:33:16 +0000",
  "sms_url": null,
  "sms_method": "POST",
  "sms_fallback_url": null,
  "sms_fallback_method": "POST",
  "sms_application_sid": null,
  "capabilities": {
    "voice_capable": false,
    "sms_capable": false,
    "mms_capable": false,
    "fax_capable": false
  },
  "api_version": "2012-04-24",
  "uri": "/restcomm/2012-04-24/Accounts/.../IncomingPhoneNumbers/....json"
}

It looks like the capabilities are ignored alltogether.

otsakir commented 8 years ago

Did some research on how we 'buy' numbers:

  1. AdminUI searches for available numbers based on capabilities: GET .../AvailablePhoneNumbers/US/Local.json? AreaCode=209& FaxEnabled=true& RangeIndex=1& RangeSize=10& SmsEnabled=true& VoiceEnabled=true

and retrieves a list of available numbers like this: [ { "friendlyName": "12345", "phoneNumber": "12345", "isoCountry": "US", "cost": "0.67", "voiceCapable": true, "smsCapable": true } ... ]

  1. AdminUI post the properties of the number:POST .../IncomingPhoneNumbers.json PhoneNumber=12345 FriendlyName=12345
  2. Restcomm PhoneNumberProvisioningManager actually buys the number returning boolean. The actual number capabilities are not know at this step.

I can see two alternatives:

a. Before restcomm buys a number, make a search request with the exact number in it. So it gets the capabilities. When the purchase is complete store them in the database.

b. In step 2, AdminUI sends the capabilities it already knows (from step 1) along with the exact number to buy. So the request becomes like this:

POST .../IncomingPhoneNumbers.json PhoneNumber=12345 FriendlyName=12345 SmsEnabled=true VoiceEnabled=true

I vote for (a). @gvagenas, @deruelle thoughts ?

gvagenas commented 8 years ago

@otsakir to me option b is more efficient since it doesn't add one more http request to the provider as the option a does.

deruelle commented 8 years ago

http://docs.telestax.com/restcomm-api-incomingphonenumbers/ is supposed to return the capabilities when a number is bought so we should return them correctly too.

PhoneNumberProvisioningManager buy Number should probably return a PhoneNumber instead of a boolean. The trouble with b) @gvagenas https://github.com/gvagenas is that if people don't use the Admin UI but the API directly, they may mess up/tamper with the SmsEnabled=true and VoiceEnabled=true in which case information stored would be wrong. I would say ideally the provider should return the exact capability of the number bought in the JSON response similarly to how http://docs.telestax.com/restcomm-api-incomingphonenumbers/ is returning the capabilities when a phone number is bought. If the provider doesn't do that, we should go with a) to avoid any friction with developers and avoid any issues at the expense of one more HTTP call. I would avoid complexifying the API for a bad API Design choice from one provider.

On Thu, Jan 21, 2016 at 1:36 PM, George Vagenas notifications@github.com wrote:

@otsakir https://github.com/otsakir to me option b is more efficient since it doesn't add one more http request to the provider as the option a does.

— Reply to this email directly or view it on GitHub https://github.com/RestComm/RestComm-Core/issues/631#issuecomment-173558040 .

otsakir commented 8 years ago

Agree with jean. Allthough (b) is a quick patch, it will polute the API and grant the user of it the permission to set capabilities arbitrarily.

I haven't work with the providers apis, but it seems cleaner to handle that on the PhoneNumberProvisioningManager and either make a second call or to get number caps or if the provider returns them when buying the number.

deruelle commented 8 years ago

@otsakir since SSO work is paused until #748, would you be able to work on that one and #630 ?

We got another occurrence of the problem from a new person joining the Restcomm cloud.