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.

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

ACCOUNTS_APPROVAL_REQUIRED bypasses ACCOUNTS_VERIFICATION_REQUIRED #691

Closed
lingthio opened this issue Jun 6, 2013 · 5 comments
Closed

Comments

@lingthio
Copy link
Contributor

lingthio commented Jun 6, 2013

Hi, Thanks for a GREAT product! I'm using Mezzanine 1.4.7

With ACCOUNTS_VERIFICATION_REQUIRED=True and ACCOUNTS_APPROVAL_REQUIRED=False, Mezzanine nicely forces new users to verify their email addresses -- So far so good.

With ACCOUNTS_VERIFICATION_REQUIRED=True and ACCOUNTS_APPROVAL_REQUIRED=True, the admin receives an email and can activate the new account manually -- Nice!

However, once activated, the user can login regardless of whether their email address is valid or not. The new user won't be notified of the approval, but they can try to log in every day until their account has been approved. This effectively bypasses the ACCOUNTS_VERIFICATION_REQUIRED=True setting.

Is it possible to I have approval AND verification? Perhaps by modifying the "Your account has been activated" email to a "Your account has been approved" email containing a "/account/verify/..." link instead of a "/account/login/" link and keeping track of "has_been_approved" and "has_been_verified" states separately?

Thanks,
Ling

@stephenmcd
Copy link
Owner

Yeah it's a good idea - I like the approach of using the verification email when the account gets approved.

Would you be interested in working on this?

@lingthio
Copy link
Contributor Author

lingthio commented Jun 9, 2013

@stephenmcd and I had a quick discussion around this (Thanks Steve!) and here's our summary:

Currently the accounts approval process works like this:

  • User signs up
  • Admin receives an 'account needs approval' email with a link to the 'Change user' admin page (/admin/auth/user/<user_id>/)
  • Admin checks the user's 'Active' field (user.is_active=True) to approve
  • When the user object is saved, mezzanine.accounts.admin.UserProfileAdmin.save_model() detects this state change, effectively sets 'is_active' to True, and sends the account approval email.

When ACCOUNTS_VERIFICATION_REQUIRED is also set, we'd like to leave the 'is_active' as False and send the account verification email.

Here are some options we discussed:

Option 1) Continue to use the 'is_active' state change in save_model(), but forcefully reset the is_active state to False and send the verification email instead. Beginner admins may notice that the state did not change and retry, which will result in duplicate verification email messages being sent.

Option 2) Similar to option 1) but the Ui now uses a Approve this account button that the admin can click. This is more intuitive for beginner admins, reduces the chance for retry, but the button will continue to show after an approval, so this still can result in email duplication.

Option 3) Similar to option 2) except that the we keep track of the new 'is_approved' state through the use of a UserGroup "Waiting for approval". Users are added to this group on sign-up and removed when the admin clicks the `Approve this account' button.

Steve's note regarding option 3): "That's a pretty good idea, but I just don't think we need to make it any more complicated. There'd still be concerns around creating that group, ensuring it doesn't get deleted, and what do we do if the settings defined don't even need it."

I'm not familiar with the Django admin, and less with the Mezzanine adapted Django admin, so I hope someone else offers to send me a snippet of what it takes to create this Approve this account button.

Thanks,
@lingthio

@lingthio
Copy link
Contributor Author

I implemented Option 1) as described in this issue.

There was a slight gotcha in accounts.admin.UserProfileAdmin(): The user object needed to be saved before calling send_verification_mail() to generate the correct token that will match with the token verification in accounts.views.signup_verify().

I have tested the code against all four combinations of VERIFICATION=True/False and APPROVAL=True/False.

See https://github.com/lingthio/mezzanine/commit/e5e173e19a89cd4c6d196204d7cfa9e5e2714f88

@lingthio

@stephenmcd
Copy link
Owner

Nice. Thanks a lot for this.

@lingthio
Copy link
Contributor Author

Glad I could contribute Steve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants