aaronn / django-rest-framework-passwordless

Passwordless Auth for Django REST Framework
MIT License
706 stars 153 forks source link

Twilio bug #48

Open vladtyum opened 4 years ago

vladtyum commented 4 years ago

hey, thanks for package @aaronn

bug in send_sms_with_callback_token()

 twilio_client.messages.create(
                body=base_string % mobile_token.key,
                to=to_number,
                from_=api_settings.PASSWORDLESS_MOBILE_NOREPLY_NUMBER
            )

to_number should have "+" otherwise twilio client doesn't work and it was hard to catch the error, since passwordless doesn't pass Twilio's error

cheers!

crj1001 commented 4 years ago

Sometimes the "+" prefix is there, sometimes it's not. It depends on what you pass in, as it seems there is nothing in the codebase to enforce the prefix. What's even more problematic is that if you follow the README.md instructions, you will end up without it, and getting the error you mentioned:

curl -X POST -d "mobile=+15552143912" localhost:8000/auth/mobile/

should be revised to:

curl -X POST -d "mobile=%2B15552143912" localhost:8000/auth/mobile/

Otherwise the plus symbol will be dropped for me. I agree the lack of errors being passed is also not ideal.

jaymes15 commented 2 years ago

it doesn't work till. @crj1001 even after using %2B instead of + any help would be appreciated!

Screen Shot 2021-09-16 at 11 24 13 AM
jaymes15 commented 2 years ago

there is actually no bug. got it to work

This my config in my settings.py

`PASSWORDLESS_AUTH = {

'PASSWORDLESS_AUTH_TYPES': ['EMAIL', 'MOBILE'],
'PASSWORDLESS_EMAIL_NOREPLY_ADDRESS': DEFAULT_FROM_EMAIL,
'PASSWORDLESS_USER_MOBILE_FIELD_NAME': 'mobile',
'PASSWORDLESS_MOBILE_NOREPLY_NUMBER': \
    os.environ.get('TWILIO_PHONE_NUMBER'),
'PASSWORDLESS_EMAIL_TOKEN_HTML_TEMPLATE_NAME': "core/auth.html"

}`

Screen Shot 2021-09-17 at 7 28 57 AM