Opened 3 years ago

Closed 3 years ago

#20866 closed New feature (invalid)

Password Reset does not catch exception from send_mail

Reported by: ezra Owned by: ezra
Component: contrib.auth Version: 1.5
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: yes
Easy pickings: yes UI/UX: no

Description method calls send_mail to send the password reset token to the user. Per the documentation, send_mail may raise smtplib.SMTPException. PasswordResetForm class does not handle that exception, and neither the views.password_reset method.

It should be straight forward to catch this exception, and report an appropriate error message to the user.

Attachments (1)

django_20866.patch (1.2 KB) - added by ezra 3 years ago.
small patch to catch the excpetion

Download all attachments as: .zip

Change History (7)

Changed 3 years ago by ezra

small patch to catch the excpetion

comment:1 Changed 3 years ago by ezra

  • Needs documentation unset
  • Needs tests unset
  • Owner changed from nobody to ezra
  • Patch needs improvement unset
  • Status changed from new to assigned

Attached a potential patch.

comment:2 Changed 3 years ago by charettes

  • Has patch set
  • Needs tests set
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to New feature

comment:3 Changed 3 years ago by timo

  • Patch needs improvement set

Thanks for the patch, however, I think it's a bad idea to introduce a dependency of contrib.messages in contrib.auth. This would also require a test for the new feature.

comment:4 Changed 3 years ago by charettes

Thinking of it I'm not sure we should fix this at the form level.

I think that returning a 500 and triggering an admin report (mail_admin, sentry, ...) is the correct way to handle this.

If we choose to display the user a message we must make sure the exception is still triggering an exception report.

comment:5 Changed 3 years ago by ezra

I don't know Django core code well enough to recommend the correct way to fix this, however I'd love to inform my users if something goes wrong while they were trying to do something.

comment:6 Changed 3 years ago by charettes

  • Resolution set to invalid
  • Status changed from assigned to closed

@ezra in this case I suggest you read the documentation regarding handling of runtime exceptions. You should be able to inform your users something went wrong this way.

Note: See TracTickets for help on using tickets.
Back to Top