Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New SMS integration #48

Closed
Bashar opened this issue May 8, 2014 · 7 comments
Closed

New SMS integration #48

Bashar opened this issue May 8, 2014 · 7 comments
Labels

Comments

@Bashar
Copy link

Bashar commented May 8, 2014

Hello @Bouke ,
I'm traveling to DjangoCon[1] Europe next week and thought of working on new SMS gateway integration for django-two-factor-auth after working on the translation.

Can you please let me know from where I need to start (keeping in mind i haven't worked on code for almost a decade since i have my own team 😄)

Best Regards
Bashar
P.S: are you coming to DjangoCon?
[1] http://2014.djangocon.eu/

@Bouke Bouke added the question label May 8, 2014
@Bouke
Copy link
Collaborator

Bouke commented May 9, 2014

Thanks for the translation, it's great to see everyone's involvement from all over the world!

Regarding implementing a new gateway, have a look at the Twilio gateway, and most notably the interface of the gateway class. If you implement a class with the signature shown below, it's enough. Depending on how phone calls are made, you might need to wire some Django views and urls, as is also shown in the Twilio gateway. Finally, you can also have a look at the "fake gateway", which also shows an implementation of the gateway.

class MyGateway(object):
    def make_call(self, device, token):
        raise NotImplementedError()

    def send_sms(self, device, token):
        raise NotImplementedError()

Sadly I'm not traveling to DjangoCon this round, but would love to visit some year in the future. Have a great week!

@Bouke
Copy link
Collaborator

Bouke commented May 13, 2014

If you have any additional questions, please let me know :).

@Bouke Bouke closed this as completed May 13, 2014
@Bashar
Copy link
Author

Bashar commented Oct 2, 2014

Hello again @Bouke :)
I've worked on fake.py and sms is now getting sent and working perfectly.

How do i show you my work? paste here or add you to the fork i did to review before submitting pull request?

I based fake.py on another django app which made it work within minutes!

@Bashar
Copy link
Author

Bashar commented Oct 2, 2014

and I believe i need help in testing what i did?

@Bouke
Copy link
Collaborator

Bouke commented Oct 4, 2014

For reviewing your code, you could post it here. I'm not very inclined to add extra gateways, as the burden of supporting it and updating it with API changes would also be mine.

@Bashar
Copy link
Author

Bashar commented Oct 4, 2014

@Bouke actually its a complete ready app so maintaining it is not going to be handled by django-two-factor-auth but django-nexmo

What I did is installed django-nexmo and updated fake.py to the following:

from nexmo import send_message

class Fake(object):
    @staticmethod
    def send_sms(device, token):
        send_message(device.number,'Your token is: %s' % token)

and worked, my next addition is to have django-nexmo supports voice then would add another staticmedhod for make_call to call since Nexmo supports voice too.

@Bouke
Copy link
Collaborator

Bouke commented Oct 11, 2014

That looks like a nice and simple solution. However you could consider renaming the class to NexmoGateway instead of Fake, but looks great otherwise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants