search

macropin / django-registration

Django-registration (redux) provides user registration functionality for Django websites.
http://django-registration-redux.readthedocs.org
Other
974 stars 350 forks source link

`cleanupregistration` deletes manually-activated users #340

Closed Xyene closed 5 years ago

Xyene commented 5 years ago

We recently ran cleanupregistration on a production instance for the first time in several years, and were unpleasantly surprised when the command dropped several thousand users which had been manually activated (or programatically created).

The check on https://github.com/macropin/django-registration/blob/master/registration/models.py#L274 doesn't check whether the user's is_active field is set to true, which has been the standard practice our staff has been using to manually activate accounts where the user did not receive the confirmation email.

Is there a particular use-case where adding a check for the user's is_active field is undesirable? If not, we would be happy to send in a PR to address this -- certainly dropping thousands of "active" users was not ideal in our case.

wdoekes commented 5 years ago

Heh. I did the same, about 21 hours go. Hilarious coincidence. I only dropped 5 users or so though.

I wholeheartedly agree that the command looks "safe" to run, but apparently wasn't.

joshblum commented 5 years ago

@Xyene @wdoekes really sorry about the data loss. It seems like we dropped in the user.is_active check in an oversight in https://github.com/macropin/django-registration/issues/233 and https://github.com/macropin/django-registration/pull/308. Originally we dropped the check in favor having an inactive registration profile, however from your report i think it's clear we should check all three values. will put up a PR and release to pypi shortly, really sorry about this!

joshblum commented 5 years ago

released versions 2.5 and 1.11 to pypi, going to close this for now!

https://pypi.org/project/django-registration-redux/1.11/ https://pypi.org/project/django-registration-redux/2.5

wdoekes commented 5 years ago

Thanks for the quick fix :)