macropin / django-registration

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

Problem with "manage.py cleanupregistration" command in 3-step registration mode #274

Closed davidfdezc closed 6 years ago

davidfdezc commented 7 years ago

Hi,

We have developed an application that uses the three step registration workflow and that executes the cleanupregistration command daily as a cron job at midnight.

The problem we have detected is that whenever the user registers (step 1) and activates (step 2) his account before midnight, but the admin approval (step 3) is made after midnight, the approval fails, as the execution of "cleanupregistration" procedure has deleted the registration and user profile.

The problem seems to be located in function "delete_expired_users" of models.py, which deletes the users whose activation key has expired and they are not active, which is the case of users that have reached step 2 but not step 3.

Those users are deleted because their activation key is considered expired for the fact that they have activated their account in "activation_key_expired" function of models.py:

    expiration_date = datetime.timedelta(
        days=settings.ACCOUNT_ACTIVATION_DAYS)
    return (self.activated or (self.user.date_joined + expiration_date <= datetime_now()))

Maybe the first term of the return line should be eliminated and only delete the users that have not completed the whole registration process in the period of time (ACCOUNT_ACTIVATION_DAYS).

By the way, looking at the code at the end of "activation_key_expired" function, there is a sentence that seems to be of no use:

activation_key_expired.boolean = True

Any help is appreciated. Thanks in advance and best regards, David

joshblum commented 7 years ago

Hi @davidfdezc! Thanks for reporting the issue.

I think that the crux of the problem is that the 3 step process relies on the activated flag being set on the profile (which is then accidentally cleaned up by the cron job). Would you be able to submit a PR to address the issue? It seems if we don't set the flag during _activate and don't require it during the admin approval, things will work out. Let me know if you can submit a patch unless @sergafts has any thoughts.

As for the erroneous line activation_key_expired.boolean = True, I think it can be removed :) Git blame points it to coming into the library during the initial import but I don't see any purpose for it.

safts commented 7 years ago

Hello everyone,

I'm not sure if not setting the flag would work. I will take a look provide suggestions on how we could address the issue. Just a little busy atm.

I'll let you know once I have given a thorough look.

joshblum commented 7 years ago

@sergafts any update on how we can address the issue?