From eae1866992b30e5aa7f31f2561a9121d21ce087d Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Thu, 10 Nov 2022 13:40:54 -0800 Subject: [PATCH] Allow users to temporarily deactivate their accounts (#2324) --- bookwyrm/forms/landing.py | 16 +++++ .../migrations/0160_auto_20221101_2251.py | 52 ++++++++++++++++ ...to_20221101_2251_0162_importjob_task_id.py | 13 ++++ bookwyrm/models/base_model.py | 1 + bookwyrm/models/user.py | 20 +++++- bookwyrm/settings.py | 2 +- bookwyrm/templates/landing/login.html | 2 +- bookwyrm/templates/landing/reactivate.html | 60 ++++++++++++++++++ .../templates/preferences/delete_user.html | 41 +++++++++---- bookwyrm/tests/views/landing/test_login.py | 3 +- .../views/preferences/test_delete_user.py | 61 ++++++++++++++++++- bookwyrm/urls.py | 10 +++ bookwyrm/views/__init__.py | 2 +- bookwyrm/views/landing/login.py | 32 +++++----- bookwyrm/views/preferences/delete_user.py | 54 +++++++++++++++- bookwyrm/views/preferences/two_factor_auth.py | 3 + 16 files changed, 333 insertions(+), 39 deletions(-) create mode 100644 bookwyrm/migrations/0160_auto_20221101_2251.py create mode 100644 bookwyrm/migrations/0163_merge_0160_auto_20221101_2251_0162_importjob_task_id.py create mode 100644 bookwyrm/templates/landing/reactivate.html diff --git a/bookwyrm/forms/landing.py b/bookwyrm/forms/landing.py index d5f1b196f..bd9884bc3 100644 --- a/bookwyrm/forms/landing.py +++ b/bookwyrm/forms/landing.py @@ -7,6 +7,7 @@ from django.utils.translation import gettext_lazy as _ import pyotp from bookwyrm import models +from bookwyrm.settings import DOMAIN from .custom_form import CustomForm @@ -20,6 +21,21 @@ class LoginForm(CustomForm): "password": forms.PasswordInput(), } + def infer_username(self): + """Users may enter their localname, username, or email""" + localname = self.data.get("localname") + if "@" in localname: # looks like an email address to me + try: + return models.User.objects.get(email=localname).username + except models.User.DoesNotExist: # maybe it's a full username? + return localname + return f"{localname}@{DOMAIN}" + + def add_invalid_password_error(self): + """We don't want to be too specific about this""" + # pylint: disable=attribute-defined-outside-init + self.non_field_errors = _("Username or password are incorrect") + class RegisterForm(CustomForm): class Meta: diff --git a/bookwyrm/migrations/0160_auto_20221101_2251.py b/bookwyrm/migrations/0160_auto_20221101_2251.py new file mode 100644 index 000000000..5c3c1d09e --- /dev/null +++ b/bookwyrm/migrations/0160_auto_20221101_2251.py @@ -0,0 +1,52 @@ +# Generated by Django 3.2.15 on 2022-11-01 22:51 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("bookwyrm", "0159_auto_20220924_0634"), + ] + + operations = [ + migrations.AddField( + model_name="user", + name="allow_reactivation", + field=models.BooleanField(default=False), + ), + migrations.AlterField( + model_name="connector", + name="deactivation_reason", + field=models.CharField( + blank=True, + choices=[ + ("pending", "Pending"), + ("self_deletion", "Self deletion"), + ("self_deactivation", "Self deactivation"), + ("moderator_suspension", "Moderator suspension"), + ("moderator_deletion", "Moderator deletion"), + ("domain_block", "Domain block"), + ], + max_length=255, + null=True, + ), + ), + migrations.AlterField( + model_name="user", + name="deactivation_reason", + field=models.CharField( + blank=True, + choices=[ + ("pending", "Pending"), + ("self_deletion", "Self deletion"), + ("self_deactivation", "Self deactivation"), + ("moderator_suspension", "Moderator suspension"), + ("moderator_deletion", "Moderator deletion"), + ("domain_block", "Domain block"), + ], + max_length=255, + null=True, + ), + ), + ] diff --git a/bookwyrm/migrations/0163_merge_0160_auto_20221101_2251_0162_importjob_task_id.py b/bookwyrm/migrations/0163_merge_0160_auto_20221101_2251_0162_importjob_task_id.py new file mode 100644 index 000000000..a76f19b00 --- /dev/null +++ b/bookwyrm/migrations/0163_merge_0160_auto_20221101_2251_0162_importjob_task_id.py @@ -0,0 +1,13 @@ +# Generated by Django 3.2.15 on 2022-11-10 20:34 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ("bookwyrm", "0160_auto_20221101_2251"), + ("bookwyrm", "0162_importjob_task_id"), + ] + + operations = [] diff --git a/bookwyrm/models/base_model.py b/bookwyrm/models/base_model.py index 3ac220bc4..2d39e2a6f 100644 --- a/bookwyrm/models/base_model.py +++ b/bookwyrm/models/base_model.py @@ -17,6 +17,7 @@ from .fields import RemoteIdField DeactivationReason = [ ("pending", _("Pending")), ("self_deletion", _("Self deletion")), + ("self_deactivation", _("Self deactivation")), ("moderator_suspension", _("Moderator suspension")), ("moderator_deletion", _("Moderator deletion")), ("domain_block", _("Domain block")), diff --git a/bookwyrm/models/user.py b/bookwyrm/models/user.py index 79f6fafbd..257b6c93c 100644 --- a/bookwyrm/models/user.py +++ b/bookwyrm/models/user.py @@ -47,6 +47,7 @@ def site_link(): return f"{protocol}://{DOMAIN}" +# pylint: disable=too-many-public-methods class User(OrderedCollectionPageMixin, AbstractUser): """a user who wants to read books""" @@ -169,6 +170,7 @@ class User(OrderedCollectionPageMixin, AbstractUser): max_length=255, choices=DeactivationReason, null=True, blank=True ) deactivation_date = models.DateTimeField(null=True, blank=True) + allow_reactivation = models.BooleanField(default=False) confirmation_code = models.CharField(max_length=32, default=new_access_code) name_field = "username" @@ -367,12 +369,28 @@ class User(OrderedCollectionPageMixin, AbstractUser): self.create_shelves() def delete(self, *args, **kwargs): - """deactivate rather than delete a user""" + """We don't actually delete the database entry""" # pylint: disable=attribute-defined-outside-init self.is_active = False # skip the logic in this class's save() super().save(*args, **kwargs) + def deactivate(self): + """Disable the user but allow them to reactivate""" + # pylint: disable=attribute-defined-outside-init + self.is_active = False + self.deactivation_reason = "self_deactivation" + self.allow_reactivation = True + super().save(broadcast=False) + + def reactivate(self): + """Now you want to come back, huh?""" + # pylint: disable=attribute-defined-outside-init + self.is_active = True + self.deactivation_reason = None + self.allow_reactivation = False + super().save(broadcast=False) + @property def local_path(self): """this model doesn't inherit bookwyrm model, so here we are""" diff --git a/bookwyrm/settings.py b/bookwyrm/settings.py index 893669694..de898609f 100644 --- a/bookwyrm/settings.py +++ b/bookwyrm/settings.py @@ -21,7 +21,7 @@ RELEASE_API = env( PAGE_LENGTH = env("PAGE_LENGTH", 15) DEFAULT_LANGUAGE = env("DEFAULT_LANGUAGE", "English") -JS_CACHE = "e678183b" +JS_CACHE = "e678183c" # email EMAIL_BACKEND = env("EMAIL_BACKEND", "django.core.mail.backends.smtp.EmailBackend") diff --git a/bookwyrm/templates/landing/login.html b/bookwyrm/templates/landing/login.html index c9ac25261..369a72bd2 100644 --- a/bookwyrm/templates/landing/login.html +++ b/bookwyrm/templates/landing/login.html @@ -14,7 +14,7 @@ {% if show_confirmed_email %}

{% trans "Success! Email address confirmed." %}

{% endif %} -
+ {% csrf_token %} {% if show_confirmed_email %}{% endif %}
diff --git a/bookwyrm/templates/landing/reactivate.html b/bookwyrm/templates/landing/reactivate.html new file mode 100644 index 000000000..da9e0b050 --- /dev/null +++ b/bookwyrm/templates/landing/reactivate.html @@ -0,0 +1,60 @@ +{% extends 'layout.html' %} +{% load i18n %} + +{% block title %}{% trans "Reactivate Account" %}{% endblock %} + +{% block content %} +

{% trans "Reactivate Account" %}

+
+
+ {% if login_form.non_field_errors %} +

{{ login_form.non_field_errors }}

+ {% endif %} + + + {% csrf_token %} +
+ +
+ +
+
+
+ +
+ +
+ + {% include 'snippets/form_errors.html' with errors_list=login_form.password.errors id="desc_password" %} +
+
+
+ +
+
+ +
+ + {% if site.allow_registration %} +
+
+

{% trans "Create an Account" %}

+
+ {% include 'snippets/register_form.html' %} +
+
+
+ {% endif %} + +
+
+ {% include 'snippets/about.html' %} + +

+ {% trans "More about this site" %} +

+
+
+
+ +{% endblock %} diff --git a/bookwyrm/templates/preferences/delete_user.html b/bookwyrm/templates/preferences/delete_user.html index b009230c5..20ca8eac1 100644 --- a/bookwyrm/templates/preferences/delete_user.html +++ b/bookwyrm/templates/preferences/delete_user.html @@ -8,22 +8,37 @@ {% endblock %} {% block panel %} +
+

{% trans "Deactivate account" %}

+
+

+ {% trans "Your account will be hidden. You can log back in at any time to re-activate your account." %} +

+ +
+ {% csrf_token %} + +
+
+
+

{% trans "Permanently delete account" %}

-

- {% trans "Deleting your account cannot be undone. The username will not be available to register in the future." %} -

+
+

+ {% trans "Deleting your account cannot be undone. The username will not be available to register in the future." %} +

-
- {% csrf_token %} -
- - + + {% csrf_token %} +
+ + - {% include 'snippets/form_errors.html' with errors_list=form.password.errors id="desc_password" %} -
- - + {% include 'snippets/form_errors.html' with errors_list=form.password.errors id="desc_password" %} +
+ + +
{% endblock %} - diff --git a/bookwyrm/tests/views/landing/test_login.py b/bookwyrm/tests/views/landing/test_login.py index 6c1188b09..d76e9a55f 100644 --- a/bookwyrm/tests/views/landing/test_login.py +++ b/bookwyrm/tests/views/landing/test_login.py @@ -17,6 +17,7 @@ from bookwyrm.tests.validate_html import validate_html class LoginViews(TestCase): """login and password management""" + # pylint: disable=invalid-name def setUp(self): """we need basic test data and mocks""" self.factory = RequestFactory() @@ -81,7 +82,7 @@ class LoginViews(TestCase): self.assertEqual(result.status_code, 302) def test_login_post_username(self, *_): - """there are so many views, this just makes sure it LOADS""" + """valid login where the user provides their user@domain.com username""" view = views.Login.as_view() form = forms.LoginForm() form.data["localname"] = "mouse@your.domain.here" diff --git a/bookwyrm/tests/views/preferences/test_delete_user.py b/bookwyrm/tests/views/preferences/test_delete_user.py index 95bfa4df3..151b9ab2e 100644 --- a/bookwyrm/tests/views/preferences/test_delete_user.py +++ b/bookwyrm/tests/views/preferences/test_delete_user.py @@ -2,6 +2,7 @@ import json from unittest.mock import patch +from django.contrib.auth.models import AnonymousUser from django.contrib.sessions.middleware import SessionMiddleware from django.template.response import TemplateResponse from django.test import TestCase @@ -15,6 +16,7 @@ from bookwyrm.tests.validate_html import validate_html class DeleteUserViews(TestCase): """view user and edit profile""" + # pylint: disable=invalid-name def setUp(self): """we need basic test data and mocks""" self.factory = RequestFactory() @@ -22,14 +24,18 @@ class DeleteUserViews(TestCase): "bookwyrm.activitystreams.populate_stream_task.delay" ), patch("bookwyrm.lists_stream.populate_lists_task.delay"): self.local_user = models.User.objects.create_user( - "mouse@local.com", + "mouse@your.domain.here", "mouse@mouse.mouse", "password", local=True, localname="mouse", ) self.rat = models.User.objects.create_user( - "rat@local.com", "rat@rat.rat", "password", local=True, localname="rat" + "rat@your.domain.here", + "rat@rat.rat", + "password", + local=True, + localname="rat", ) self.book = models.Edition.objects.create( @@ -44,6 +50,8 @@ class DeleteUserViews(TestCase): shelf=self.local_user.shelf_set.first(), ) + self.anonymous_user = AnonymousUser + self.anonymous_user.is_authenticated = False models.SiteSettings.objects.create() def test_delete_user_page(self, _): @@ -84,3 +92,52 @@ class DeleteUserViews(TestCase): self.local_user.refresh_from_db() self.assertFalse(self.local_user.is_active) self.assertEqual(self.local_user.deactivation_reason, "self_deletion") + + def test_deactivate_user(self, _): + """Impermanent deletion""" + self.assertTrue(self.local_user.is_active) + view = views.DeactivateUser.as_view() + request = self.factory.post("") + request.user = self.local_user + middleware = SessionMiddleware() + middleware.process_request(request) + request.session.save() + + view(request) + + self.local_user.refresh_from_db() + self.assertFalse(self.local_user.is_active) + self.assertEqual(self.local_user.deactivation_reason, "self_deactivation") + + def test_reactivate_user_get(self, _): + """Reactication page""" + view = views.ReactivateUser.as_view() + request = self.factory.get("") + request.user = self.anonymous_user + + result = view(request) + self.assertIsInstance(result, TemplateResponse) + validate_html(result.render()) + self.assertEqual(result.status_code, 200) + + def test_reactivate_user_post(self, _): + """Reactivate action""" + self.local_user.deactivate() + self.local_user.refresh_from_db() + + view = views.ReactivateUser.as_view() + form = forms.LoginForm() + form.data["localname"] = "mouse" + form.data["password"] = "password" + request = self.factory.post("", form.data) + request.user = self.local_user + middleware = SessionMiddleware() + middleware.process_request(request) + request.session.save() + + with patch("bookwyrm.views.preferences.delete_user.login"): + view(request) + + self.local_user.refresh_from_db() + self.assertTrue(self.local_user.is_active) + self.assertIsNone(self.local_user.deactivation_reason) diff --git a/bookwyrm/urls.py b/bookwyrm/urls.py index 16683e52d..7af123016 100644 --- a/bookwyrm/urls.py +++ b/bookwyrm/urls.py @@ -526,6 +526,16 @@ urlpatterns = [ ), re_path(r"^preferences/export/?$", views.Export.as_view(), name="prefs-export"), re_path(r"^preferences/delete/?$", views.DeleteUser.as_view(), name="prefs-delete"), + re_path( + r"^preferences/deactivate/?$", + views.DeactivateUser.as_view(), + name="prefs-deactivate", + ), + re_path( + r"^preferences/reactivate/?$", + views.ReactivateUser.as_view(), + name="prefs-reactivate", + ), re_path(r"^preferences/block/?$", views.Block.as_view(), name="prefs-block"), re_path(r"^block/(?P\d+)/?$", views.Block.as_view()), re_path(r"^unblock/(?P\d+)/?$", views.unblock), diff --git a/bookwyrm/views/__init__.py b/bookwyrm/views/__init__.py index c900ab175..e5b772136 100644 --- a/bookwyrm/views/__init__.py +++ b/bookwyrm/views/__init__.py @@ -31,7 +31,7 @@ from .admin.user_admin import UserAdmin, UserAdminList from .preferences.change_password import ChangePassword from .preferences.edit_user import EditUser from .preferences.export import Export -from .preferences.delete_user import DeleteUser +from .preferences.delete_user import DeleteUser, DeactivateUser, ReactivateUser from .preferences.block import Block, unblock from .preferences.two_factor_auth import ( Edit2FA, diff --git a/bookwyrm/views/landing/login.py b/bookwyrm/views/landing/login.py index 3b7b10f0d..322afb768 100644 --- a/bookwyrm/views/landing/login.py +++ b/bookwyrm/views/landing/login.py @@ -6,12 +6,10 @@ from django.contrib.auth.decorators import login_required from django.shortcuts import redirect from django.template.response import TemplateResponse from django.utils.decorators import method_decorator -from django.utils.translation import gettext_lazy as _ from django.views import View from django.views.decorators.debug import sensitive_variables, sensitive_post_parameters from bookwyrm import forms, models -from bookwyrm.settings import DOMAIN from bookwyrm.views.helpers import set_language @@ -40,19 +38,13 @@ class Login(View): return redirect("/") login_form = forms.LoginForm(request.POST) - localname = login_form.data.get("localname") - - if "@" in localname: # looks like an email address to me - try: - username = models.User.objects.get(email=localname).username - except models.User.DoesNotExist: # maybe it's a full username? - username = localname - else: - username = f"{localname}@{DOMAIN}" + # who do we think is trying to log in + username = login_form.infer_username() password = login_form.data.get("password") # perform authentication user = authenticate(request, username=username, password=password) + if user is not None: # if 2fa is set, don't log them in until they enter the right code if user.two_factor_auth: @@ -76,14 +68,22 @@ class Login(View): return set_language(user, redirect("/")) - # maybe the user is pending email confirmation - if models.User.objects.filter( - username=username, is_active=False, deactivation_reason="pending" - ).exists(): + user_attempt = models.User.objects.filter( + username=username, is_active=False + ).first() + if user_attempt and user_attempt.deactivation_reason == "pending": + # maybe the user is pending email confirmation return redirect("confirm-email") + if ( + user_attempt + and user_attempt.allow_reactivation + and user_attempt.check_password(password) + ): + # maybe we want to reactivate an account? + return redirect("prefs-reactivate") # login errors - login_form.non_field_errors = _("Username or password are incorrect") + login_form.add_invalid_password_error() register_form = forms.RegisterForm() data = {"login_form": login_form, "register_form": register_form} return TemplateResponse(request, "landing/login.html", data) diff --git a/bookwyrm/views/preferences/delete_user.py b/bookwyrm/views/preferences/delete_user.py index 17daf0216..c8b19ec09 100644 --- a/bookwyrm/views/preferences/delete_user.py +++ b/bookwyrm/views/preferences/delete_user.py @@ -1,7 +1,9 @@ """ edit your own account """ -from django.contrib.auth import logout +import time + +from django.contrib.auth import login, logout from django.contrib.auth.decorators import login_required -from django.shortcuts import redirect +from django.shortcuts import get_object_or_404, redirect from django.template.response import TemplateResponse from django.utils.decorators import method_decorator from django.views import View @@ -23,7 +25,7 @@ class DeleteUser(View): return TemplateResponse(request, "preferences/delete_user.html", data) def post(self, request): - """les get fancy with images""" + """There's no going back from this""" form = forms.DeleteUserForm(request.POST, instance=request.user) # idk why but I couldn't get check_password to work on request.user user = models.User.objects.get(id=request.user.id) @@ -36,3 +38,49 @@ class DeleteUser(View): form.errors["password"] = ["Invalid password"] data = {"form": form, "user": request.user} return TemplateResponse(request, "preferences/delete_user.html", data) + + +@method_decorator(login_required, name="dispatch") +class DeactivateUser(View): + """deactivate user view""" + + def post(self, request): + """You can reactivate""" + request.user.deactivate() + logout(request) + return redirect("/") + + +class ReactivateUser(View): + """now reactivate the user""" + + def get(self, request): + """so you want to rejoin?""" + if request.user.is_authenticated: + return redirect("/") + data = {"login_form": forms.LoginForm()} + return TemplateResponse(request, "landing/reactivate.html", data) + + def post(self, request): + """reactivate that baby""" + login_form = forms.LoginForm(request.POST) + + username = login_form.infer_username() + password = login_form.data.get("password") + user = get_object_or_404(models.User, username=username) + + # we can't use "authenticate" because that requires an active user + if not user.check_password(password): + login_form.add_invalid_password_error() + data = {"login_form": login_form} + return TemplateResponse(request, "landing/reactivate.html", data) + + # Correct password, do you need 2fa too? + if user.two_factor_auth: + request.session["2fa_user"] = user.username + request.session["2fa_auth_time"] = time.time() + return redirect("login-with-2fa") + + user.reactivate() + login(request, user) + return redirect("/") diff --git a/bookwyrm/views/preferences/two_factor_auth.py b/bookwyrm/views/preferences/two_factor_auth.py index 61003777f..f3b04eb3c 100644 --- a/bookwyrm/views/preferences/two_factor_auth.py +++ b/bookwyrm/views/preferences/two_factor_auth.py @@ -133,6 +133,9 @@ class LoginWith2FA(View): request, "two_factor_auth/two_factor_login.html", data ) + # is this a reactivate? let's go for it + if not user.is_active and user.allow_reactivation: + user.reactivate() # log the user in - we are bypassing standard login login(request, user) user.update_active_date()