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.

Sign up for GitHub

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

Jump to bottom

Support displaying a message in case of a Twilio exception #315

Open
yoandyshyno opened this issue Sep 29, 2019 · 2 comments · May be fixed by #316
Open

Support displaying a message in case of a Twilio exception #315

yoandyshyno opened this issue Sep 29, 2019 · 2 comments · May be fixed by #316

Comments

@yoandyshyno
Copy link

yoandyshyno commented Sep 29, 2019 •
edited

Expected Behavior

I would like to propose to give a change to developers to display a configured message in the settings, in case a Twilio exception happens. Like for example "Unable to create record: The 'To' number +4900000000000 is not a valid phone number.".

It may ocurre if a phone number is incorrectly setup.

Current Behavior

An exception is raise by the Twilio Client, meaning we get an internal error response.

Possible Solution

In case a settings variable like for instance:

TWILIO_ERROR_MESSAGE = 'Some error message'

We display the message using Django's message framework. Otherwise we keep the current behavior.

Steps to Reproduce (for bugs)

  1. Enable two factor for a user by configuring a false number for a user Ex: +4900000000000
  2. Try to login with the user's username and password.
  3. You should get an Internal Error response.

Context

I would like to show users a custom error message, instead of just a 500 or internal error page.

I would gladly make a pull request for this issue.
yoandyshyno added a commit to yoandyshyno/django-two-factor-auth that referenced this issue Sep 29, 2019
@yoandyshyno
jazzband#315 display a message in case of a Twilio exception
e85115d 
if TWILIO_ERROR_MESSAGE is configured in settings, then an error message is added to the messages instance with this text.
yoandyshyno added a commit to yoandyshyno/django-two-factor-auth that referenced this issue Sep 29, 2019
@yoandyshyno
jazzband#315 Update documentation in Twilio gateway class
5a1018e
yoandyshyno added a commit to yoandyshyno/django-two-factor-auth that referenced this issue Sep 30, 2019
@yoandyshyno
jazzband#315 display a message in case of a Twilio exception
b66fe18 
if TWILIO_ERROR_MESSAGE is configured in settings, then an error message is added to the messages instance with this text.
@yoandyshyno yoandyshyno linked a pull request Sep 30, 2019 that will close this issue
Open
8 tasks
@moggers87
Copy link
Collaborator

I think we need a stacktrace - in most instance if generating a challenge fails the view catches it and sets challenge_succeeded in the template context. I would imagine we've just missed a place where this happens.

yoandyshyno added a commit to yoandyshyno/django-two-factor-auth that referenced this issue Oct 1, 2019
@yoandyshyno
Update unittest with changes of jazzband#315
9eaea76
yoandyshyno added a commit to yoandyshyno/django-two-factor-auth that referenced this issue Oct 1, 2019
@yoandyshyno
Update unittest with changes of jazzband#315
02c09a1
@yoandyshyno
Copy link
Author

Also, for the Twilio gateway we can add the possibility to add a status callback url. This way, for instance, admins can track users that have devices unable to receive an SMS. This could be described in a new issue.

@yoandyshyno yoandyshyno reopened this Oct 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add message if send_sms fail with an Exception yoandyshyno/django-two-factor-auth
2 participants
@moggers87 @yoandyshyno