Closed (fixed)
Project:
Twilio SMS Integration
Version:
6.x-1.0
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
3 Dec 2010 at 16:45 UTC
Updated:
23 Oct 2011 at 05:20 UTC
Jump to comment: Most recent file
Comments
Comment #1
univate commentedI don't have any immediate plans to work on this feature at the moment.
smsframework has some support for handling incoming messages so it should probably integrate with that.
Comment #2
nmcclain commentedHi univate, Thanks for this module - it works great!
Hi that0n3guy, This patch provides incoming SMS handling for Twilio via the SMS Framework. You'll need to apply the patch to sms_twilio, then configure your Twilio phone number (via the Twilio web site) to POST SMS messages to: http://YOURSITE/sms/twilio/incoming
Let me know if you run into problems!!
Cheers! Ned.
Comment #3
nmcclain commentedTo be secure, for the moment this path either to be protected by HTTP basic auth (which Twilio supports).
The right answer in the long run is to implement the "Validating Requests are coming from Twilio" protocol described here:
http://www.twilio.com/docs/security-reliability/security
Be careful implementing the above patch in the meantime... someone could easily forge incoming SMSs!
Comment #4
notasheep commentedsubscribing
thanks for writing this -- exactly what I needed and just in time!
Comment #5
djdevinTried the patch and two-way works successfully, thanks! Should add the security first tho
Comment #6
univate commentedI agree it would be best to make this secure, the PHP code required looks like its there above in that link, just need someone to copy it in and test.
Comment #7
nmcclain commentedUnivate, et al,
Okay, here's an updated patch against 6.x-1.0 that does security "the Twilio way" by validating the HTTP_X_TWILIO_SIGNATURE.
I also added a new admin config field so you can customize the incoming URI (someday, we'll have to support multiple incoming phone numbers!). I ran this through Coder module's code review and removed some extra spaces from other parts of sms_twilio.module.
Please don't be shy with feedback!! I'm happy to make whatever changes are necessary to get this committed to a dev version of sms_twilio.
Comment #8
univate commentedWhy do we need to make the incoming path configurable? I presume that the menu cache would need to be cleared when changing that path. I just think it would be best to set the path.
People can override the menu if they really want through a menu_alter as well.
Comment #9
nmcclain commentedI added the configurable path hoping to lay the groundwork for multiple incoming numbers, but I agree this is definitely not important. I have removed that feature in this latest attached patch against 6.x-1.0.
Is there anything else I can do to get this patch committed?
Comment #10
univate commentedYou could support multiple incoming paths through a variable:
I was confused with that last patch, until I realised that the patch included a copy of the patch inside it as well
Comment #11
nmcclain commentedThanks! As I thought through multiple incoming numbers and Twilio specifically, I realized we only need one incoming URI. Twilio passes both "to" and "from" SMS numbers in the POST. So currently it supports receiving SMS's from multiple Twilio numbers (just set the same URL in the Twilio.com admin page), but it does NOT provide a way to differentiate which of those numbers received the SMS on the Drupal side. I don't believe this is currently supported in the SMS Framework module.
So I think the only question is: is there a way to pass the SMS destination number to the SMS Framework module(s) such that it is actually useful somehow? SMS Framework would need to provide support for this before it could be handled in this module.
Regardless of the answer: Would you be up for going ahead and committing this fix, which resolves this issue? If someone opens a new issue for differentiating between multiple incoming SMS numbers in the SMS Framework issue queue, I'd be willing to look into implementing it...
--- I apologize for the recursive patch!!! (I'm embarrassed!!) If you'd like a "clean" patch, just let me know! ---
Comment #12
nmcclain commentedOkay, attached is a "clean" patch against 6.x-1.0. I believe this patch resolves this issue. Please let me know if you would like any additional changes before committing it.
I would be happy to tackle differentiating between incoming numbers in a different issue after we get this feature added.
Comment #13
univate commentedcommitted