search

ubernostrum / django-registration

An extensible user-registration app for Django.
https://django-registration.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
924 stars 241 forks source link

Two-step `ActivationView` runs `activate()` on HTTP GET #242

Closed djramones closed 1 week ago

djramones commented 1 year ago

Altering database state on GET requests is frowned upon. (Django, for example, is already deprecating logout via GET.)

Besides being a matter of HTTP spec compliance, there is a known issue with email services preprocessing/crawling links in the email body, causing one-time links to be invalidated before the user gets to click them. For django-registration this has been reported in a comment on closed issue #104.

My workaround for this currently is to simply tell the user in activation_complete.html, if ActivationError code == "already_activated", that their email service could have done this and that they can already log in. At the infra level perhaps folks could also run some bot/crawler countermeasures on the activation URL path, though that's a bit more involved.

The proper fix I think is to run activate() only on POST, like what Django's DeleteView does. GET should only present a form/button, and the user needs to click to confirm activation. This also presents an opportunity for customizations like requiring a captcha solve, though I guess it's also a backwards-incompatible change.

Sorry if this is a duplicate issue, the closest report I found is #104.

ubernostrum commented 1 year ago

This is a thing I've been thinking about for a while, but any solution has to wait for a major version bump due to being backwards-incompatible, so I'm going to leave this open as a reminder for when the 4.0 release is being planned.

MSP-Stev commented 10 months ago

I have the same problem, but its caused by security software. The mail will be checked and the url is scanned for potential risks.

PS: I have also received registration emails where you have to enter a code, so nothing new :)

ubernostrum commented 2 weeks ago

in progress and backwards-incompatible refactor:

https://github.com/ubernostrum/django-registration/tree/activation-refactor

ubernostrum commented 1 week ago

Fixed in https://github.com/ubernostrum/django-registration/commit/5efeaa16d192fa6bace8c1d29375a1a546f7219f

There's still work to do to polish things up and get a release ready, though, so will be a bit more of a wait to get it up on PyPI.