diff --git a/CHANGELOG.md b/CHANGELOG.md index e185ed43ee..5b7f3ca37c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,8 +2,9 @@ ## Current (in progress) -- Nothing yet - +- Add inactive users notification and deletion jobs [#3274](https://github.com/opendatateam/udata/pull/3274) + - these jobs can be scheduled daily for example + - `YEARS_OF_INACTIVITY_BEFORE_DELETION` setting must be configured at least to activate it ## 10.1.1 (2025-03-03) - Allow temporal coverage with only a start date [#3192](https://github.com/opendatateam/udata/pull/3192) diff --git a/docs/adapting-settings.md b/docs/adapting-settings.md index 0cbdd37e49..67c17447bf 100644 --- a/docs/adapting-settings.md +++ b/docs/adapting-settings.md @@ -443,6 +443,30 @@ If set to `True`, the new passwords will need to contain at least one uppercase If set to `True`, the new passwords will need to contain at least one symbol. +## Inactive users deletion options + +### YEARS_OF_INACTIVITY_BEFORE_DELETION + +**default**: `None` + +Set this setting to an int value to activate inactive users account notification and deletion. +It will filter on users with an account inactive for longer than this number of years. + +#### DAYS_BEFORE_ACCOUNT_INACTIVITY_NOTIFY_DELAY + +**default**: 30 + +The delay of days between inactive user notification and its account deletion. + +### MAX_NUMBER_OF_USER_INACTIVITY_NOTIFICATIONS + +**default**: 200 + +The maximal number of notifications to send per `notify-inactive-users` job. +If activating `YEARS_OF_INACTIVITY_BEFORE_DELETION`, you can slowly increase this configuration +batch after batch. The limitation is made to prevent sending too many mail notifications at once +resulting in your mail domain being flagged as spam. + ## Flask-Cache options udata uses Flask-Cache to handle cache and use Redis by default. diff --git a/udata/core/user/api.py b/udata/core/user/api.py index ff44edefd4..d69fe723cb 100644 --- a/udata/core/user/api.py +++ b/udata/core/user/api.py @@ -275,6 +275,13 @@ def post(self, user): location="args", default=False, ) +delete_parser.add_argument( + "delete_comments", + type=bool, + help="Delete comments posted by the user upon user deletion", + location="args", + default=False, +) @ns.route("//", endpoint="user") @@ -317,7 +324,7 @@ def delete(self, user): 403, "You cannot delete yourself with this API. " + 'Use the "me" API instead.' ) - user.mark_as_deleted(notify=not args["no_mail"]) + user.mark_as_deleted(notify=not args["no_mail"], delete_comments=args["delete_comments"]) return "", 204 diff --git a/udata/core/user/models.py b/udata/core/user/models.py index 1cab00ee58..7edbd40795 100644 --- a/udata/core/user/models.py +++ b/udata/core/user/models.py @@ -82,6 +82,10 @@ class User(WithMetrics, UserMixin, db.Document): ext = db.MapField(db.GenericEmbeddedDocumentField()) extras = db.ExtrasField() + # Used to track notification for automatic inactive users deletion + # when YEARS_OF_INACTIVITY_BEFORE_DEACTIVATION is set + inactive_deletion_notified_at = db.DateTimeField() + before_save = Signal() after_save = Signal() on_create = Signal() @@ -237,7 +241,7 @@ def delete(self, *args, **kwargs): raise NotImplementedError("""This method should not be using directly. Use `mark_as_deleted` (or `_delete` if you know what you're doing)""") - def mark_as_deleted(self, notify: bool = True): + def mark_as_deleted(self, notify: bool = True, delete_comments: bool = False): if self.avatar.filename is not None: storage = storages.avatars storage.delete(self.avatar.filename) @@ -265,16 +269,17 @@ def mark_as_deleted(self, notify: bool = True): member for member in organization.members if member.user != self ] organization.save() - for discussion in Discussion.objects(discussion__posted_by=self): - # Remove all discussions with current user as only participant - if all(message.posted_by == self for message in discussion.discussion): - discussion.delete() - continue - - for message in discussion.discussion: - if message.posted_by == self: - message.content = "DELETED" - discussion.save() + if delete_comments: + for discussion in Discussion.objects(discussion__posted_by=self): + # Remove all discussions with current user as only participant + if all(message.posted_by == self for message in discussion.discussion): + discussion.delete() + continue + + for message in discussion.discussion: + if message.posted_by == self: + message.content = "DELETED" + discussion.save() Follow.objects(follower=self).delete() Follow.objects(following=self).delete() diff --git a/udata/core/user/tasks.py b/udata/core/user/tasks.py index 7df5b36151..425b0acb04 100644 --- a/udata/core/user/tasks.py +++ b/udata/core/user/tasks.py @@ -1,10 +1,14 @@ import logging +from copy import copy +from datetime import datetime, timedelta + +from flask import current_app from udata import mail from udata.i18n import lazy_gettext as _ -from udata.tasks import task +from udata.tasks import job, task -from .models import datastore +from .models import User, datastore log = logging.getLogger(__name__) @@ -13,3 +17,78 @@ def send_test_mail(email): user = datastore.find_user(email=email) mail.send(_("Test mail"), user, "test") + + +@job("notify-inactive-users") +def notify_inactive_users(self): + if not current_app.config["YEARS_OF_INACTIVITY_BEFORE_DEACTIVATION"]: + logging.warning( + "YEARS_OF_INACTIVITY_BEFORE_DEACTIVATION setting is not set, no deletion planned" + ) + return + notification_comparison_date = ( + datetime.utcnow() + - timedelta(days=current_app.config["YEARS_OF_INACTIVITY_BEFORE_DEACTIVATION"] * 365) + + timedelta(days=current_app.config["DAYS_BEFORE_ACCOUNT_INACTIVITY_NOTIFY_DELAY"]) + ) + + users_to_notify = User.objects( + deleted=None, + inactive_deletion_notified_at=None, + current_login_at__lte=notification_comparison_date, + ) + for i, user in enumerate(users_to_notify): + if i >= current_app.config["MAX_NUMBER_OF_USER_INACTIVITY_NOTIFICATIONS"]: + logging.warning("MAX_NUMBER_OF_USER_INACTIVITY_NOTIFICATIONS reached, stopping here.") + return + mail.send( + _("Inactivity of your {site} account").format(site=current_app.config["SITE_TITLE"]), + user, + "account_inactivity", + user=user, + ) + logging.debug(f"Notified {user.email} of account inactivity") + user.inactive_deletion_notified_at = datetime.utcnow() + user.save() + + logging.info(f"Notified {users_to_notify.count()} inactive users") + + +@job("delete-inactive-users") +def delete_inactive_users(self): + if not current_app.config["YEARS_OF_INACTIVITY_BEFORE_DEACTIVATION"]: + logging.warning( + "YEARS_OF_INACTIVITY_BEFORE_DEACTIVATION setting is not set, no deletion planned" + ) + return + + # Clear inactive_deletion_notified_at field if user has logged in since notification + for user in User.objects(deleted=None, inactive_deletion_notified_at__exists=True): + if user.current_login_at > user.inactive_deletion_notified_at: + user.inactive_deletion_notified_at = None + user.save() + + # Delete inactive users upon notification delay if user still hasn't logged in + deletion_comparison_date = datetime.utcnow() - timedelta( + days=current_app.config["YEARS_OF_INACTIVITY_BEFORE_DEACTIVATION"] * 365 + ) + notified_at = datetime.utcnow() - timedelta( + days=current_app.config["DAYS_BEFORE_ACCOUNT_INACTIVITY_NOTIFY_DELAY"] + ) + users_to_delete = User.objects( + deleted=None, + current_login_at__lte=deletion_comparison_date, + inactive_deletion_notified_at__lte=notified_at, + ) + for user in users_to_delete: + copied_user = copy(user) + user.mark_as_deleted(notify=False, delete_comments=False) + logging.warning(f"Deleted user {copied_user.email} due to account inactivity") + mail.send( + _("Deletion of your inactive {site} account").format( + site=current_app.config["SITE_TITLE"] + ), + copied_user, + "inactive_account_deleted", + ) + logging.info(f"Deleted {users_to_delete.count()} inactive users") diff --git a/udata/core/user/tests/test_user_model.py b/udata/core/user/tests/test_user_model.py index f579a720fc..cfbcb1634a 100644 --- a/udata/core/user/tests/test_user_model.py +++ b/udata/core/user/tests/test_user_model.py @@ -18,7 +18,7 @@ def test_mark_as_deleted(self): user = UserFactory() other_user = UserFactory() org = OrganizationFactory(editors=[user]) - discussion_only_user = DiscussionFactory( + discussion = DiscussionFactory( user=user, subject=org, discussion=[ @@ -26,14 +26,6 @@ def test_mark_as_deleted(self): MessageDiscussionFactory(posted_by=user), ], ) - discussion_with_other = DiscussionFactory( - user=other_user, - subject=org, - discussion=[ - MessageDiscussionFactory(posted_by=other_user), - MessageDiscussionFactory(posted_by=user), - ], - ) user_follow_org = Follow.objects.create(follower=user, following=org) user_followed = Follow.objects.create(follower=other_user, following=user) @@ -42,15 +34,40 @@ def test_mark_as_deleted(self): org.reload() assert len(org.members) == 0 - assert Discussion.objects(id=discussion_only_user.id).first() is None - discussion_with_other.reload() - assert discussion_with_other.discussion[1].content == "DELETED" + # discussions are kept by default + discussion.reload() + assert len(discussion.discussion) == 2 + assert discussion.discussion[1].content != "DELETED" assert Follow.objects(id=user_follow_org.id).first() is None assert Follow.objects(id=user_followed.id).first() is None assert user.slug == "deleted" + def test_mark_as_deleted_with_comments_deletion(self): + user = UserFactory() + other_user = UserFactory() + discussion_only_user = DiscussionFactory( + user=user, + discussion=[ + MessageDiscussionFactory(posted_by=user), + MessageDiscussionFactory(posted_by=user), + ], + ) + discussion_with_other = DiscussionFactory( + user=other_user, + discussion=[ + MessageDiscussionFactory(posted_by=other_user), + MessageDiscussionFactory(posted_by=user), + ], + ) + + user.mark_as_deleted(delete_comments=True) + + assert Discussion.objects(id=discussion_only_user.id).first() is None + discussion_with_other.reload() + assert discussion_with_other.discussion[1].content == "DELETED" + def test_mark_as_deleted_slug_multiple(self): user = UserFactory() other_user = UserFactory() diff --git a/udata/settings.py b/udata/settings.py index 404f01453b..cb2d9ddb6f 100644 --- a/udata/settings.py +++ b/udata/settings.py @@ -115,6 +115,11 @@ class Defaults(object): SECURITY_RETURN_GENERIC_RESPONSES = False + # Inactive users settings + YEARS_OF_INACTIVITY_BEFORE_DELETION = None + DAYS_BEFORE_ACCOUNT_INACTIVITY_NOTIFY_DELAY = 30 + MAX_NUMBER_OF_USER_INACTIVITY_NOTIFICATIONS = 200 + # Sentry configuration SENTRY_DSN = None SENTRY_TAGS = {} diff --git a/udata/templates/mail/account_inactivity.html b/udata/templates/mail/account_inactivity.html new file mode 100644 index 0000000000..8e604aa9fb --- /dev/null +++ b/udata/templates/mail/account_inactivity.html @@ -0,0 +1,29 @@ +{% extends 'mail/base.html' %} +{% from 'mail/button.html' import mail_button %} + +{% block body %} +

+ {{ _( + 'Your account (%(user_email)s) has been inactive for %(inactivity_years)d years or more.', + user_email=user.email, + inactivity_years=config.YEARS_OF_INACTIVITY_BEFORE_DEACTIVATION + ) + }} +

+
+

+ {{ _( + 'If you want to keep your account, please log in with your account on %(site)s.', + site=config.SITE_TITLE + ) + }} +

+
+

+ {{ _( + 'Without logging in, your account will be deleted within %(notify_delay)d days.', + notify_delay=config.DAYS_BEFORE_ACCOUNT_INACTIVITY_NOTIFY_DELAY + ) + }} +

+{% endblock %} diff --git a/udata/templates/mail/account_inactivity.txt b/udata/templates/mail/account_inactivity.txt new file mode 100644 index 0000000000..b1bae32d09 --- /dev/null +++ b/udata/templates/mail/account_inactivity.txt @@ -0,0 +1,22 @@ +{% extends 'mail/base.txt' %} + +{% block body %} +{{ _( + 'Your account (%(user_email)s) has been inactive for %(inactivity_years)d years or more.', + user_email=user.email, + inactivity_years=config.YEARS_OF_INACTIVITY_BEFORE_DEACTIVATION + ) +}} + +{{ _( + 'If you want to keep your account, please log in with your account on %(site)s.', + site=config.SITE_TITLE + ) +}} + +{{ _( + 'Without logging in, your account will be deleted within %(notify_delay)d days.', + notify_delay=config.DAYS_BEFORE_ACCOUNT_INACTIVITY_NOTIFY_DELAY + ) +}} +{% endblock %} diff --git a/udata/templates/mail/inactive_account_deleted.html b/udata/templates/mail/inactive_account_deleted.html new file mode 100644 index 0000000000..c15f18f7bc --- /dev/null +++ b/udata/templates/mail/inactive_account_deleted.html @@ -0,0 +1,5 @@ +{% extends 'mail/base.html' %} + +{% block body %} +

{{ _('Your account on %(site)s has been deleted due to inactivity', site=config.SITE_TITLE) }}

+{% endblock %} diff --git a/udata/templates/mail/inactive_account_deleted.txt b/udata/templates/mail/inactive_account_deleted.txt new file mode 100644 index 0000000000..4ac3cc3326 --- /dev/null +++ b/udata/templates/mail/inactive_account_deleted.txt @@ -0,0 +1,6 @@ +{% extends 'mail/base.txt' %} + +{% block body %} +{{ _('Your account on %(site)s has been deleted due to inactivity', site=config.SITE_TITLE) }}. + +{% endblock %} diff --git a/udata/tests/api/test_me_api.py b/udata/tests/api/test_me_api.py index 18973c2219..c4b17cdeef 100644 --- a/udata/tests/api/test_me_api.py +++ b/udata/tests/api/test_me_api.py @@ -332,7 +332,7 @@ def test_delete(self): # The discussions are kept but the messages are anonymized self.assertEqual(len(discussion.discussion), 2) - self.assertEqual(discussion.discussion[0].content, "DELETED") + self.assertEqual(discussion.discussion[0].posted_by.fullname, "DELETED DELETED") self.assertEqual(discussion.discussion[1].content, other_disc_msg_content) # The datasets are unchanged diff --git a/udata/tests/api/test_user_api.py b/udata/tests/api/test_user_api.py index 6cc2f961f5..3d76f384b1 100644 --- a/udata/tests/api/test_user_api.py +++ b/udata/tests/api/test_user_api.py @@ -1,8 +1,9 @@ from flask import url_for from udata.core import storages +from udata.core.discussions.factories import DiscussionFactory, MessageDiscussionFactory from udata.core.user.factories import AdminFactory, UserFactory -from udata.models import Follow +from udata.models import Discussion, Follow from udata.tests.helpers import capture_mails, create_test_image from udata.utils import faker @@ -352,36 +353,74 @@ def test_user_roles(self): def test_delete_user(self): user = AdminFactory() self.login(user) - other_user = UserFactory() + user_to_delete = UserFactory() file = create_test_image() + discussion = DiscussionFactory( + user=user_to_delete, + discussion=[ + MessageDiscussionFactory(posted_by=user_to_delete), + MessageDiscussionFactory(posted_by=user_to_delete), + ], + ) response = self.post( - url_for("api.user_avatar", user=other_user), + url_for("api.user_avatar", user=user_to_delete), {"file": (file, "test.png")}, json=False, ) with capture_mails() as mails: - response = self.delete(url_for("api.user", user=other_user)) + response = self.delete(url_for("api.user", user=user_to_delete)) self.assertEqual(list(storages.avatars.list_files()), []) self.assert204(response) self.assertEquals(len(mails), 1) - other_user.reload() - response = self.delete(url_for("api.user", user=other_user)) + user_to_delete.reload() + response = self.delete(url_for("api.user", user=user_to_delete)) self.assert410(response) response = self.delete(url_for("api.user", user=user)) self.assert403(response) + # discussions are kept by default + discussion.reload() + assert len(discussion.discussion) == 2 + assert discussion.discussion[1].content != "DELETED" + def test_delete_user_without_notify(self): user = AdminFactory() self.login(user) - other_user = UserFactory() + user_to_delete = UserFactory() with capture_mails() as mails: - response = self.delete(url_for("api.user", user=other_user, no_mail=True)) + response = self.delete(url_for("api.user", user=user_to_delete, no_mail=True)) self.assert204(response) self.assertEqual(len(mails), 0) + def test_delete_user_with_comments_deletion(self): + user = AdminFactory() + self.login(user) + user_to_delete = UserFactory() + discussion_only_user = DiscussionFactory( + user=user_to_delete, + discussion=[ + MessageDiscussionFactory(posted_by=user_to_delete), + MessageDiscussionFactory(posted_by=user_to_delete), + ], + ) + discussion_with_other = DiscussionFactory( + user=user, + discussion=[ + MessageDiscussionFactory(posted_by=user), + MessageDiscussionFactory(posted_by=user_to_delete), + ], + ) + + response = self.delete(url_for("api.user", user=user_to_delete, delete_comments=True)) + self.assert204(response) + + assert Discussion.objects(id=discussion_only_user.id).first() is None + discussion_with_other.reload() + assert discussion_with_other.discussion[1].content == "DELETED" + def test_contact_points(self): user = AdminFactory() self.login(user) diff --git a/udata/tests/user/test_user_tasks.py b/udata/tests/user/test_user_tasks.py new file mode 100644 index 0000000000..5eba901789 --- /dev/null +++ b/udata/tests/user/test_user_tasks.py @@ -0,0 +1,144 @@ +from datetime import datetime, timedelta + +import pytest +from flask import current_app + +from udata.core.discussions.factories import DiscussionFactory +from udata.core.user import tasks +from udata.core.user.factories import UserFactory +from udata.core.user.models import User +from udata.i18n import gettext as _ +from udata.tests.api import APITestCase +from udata.tests.helpers import capture_mails + + +class UserTasksTest(APITestCase): + @pytest.mark.options(YEARS_OF_INACTIVITY_BEFORE_DEACTIVATION=3) + def test_notify_inactive_users(self): + notification_comparison_date = ( + datetime.utcnow() + - timedelta(days=current_app.config["YEARS_OF_INACTIVITY_BEFORE_DEACTIVATION"] * 365) + + timedelta(days=current_app.config["DAYS_BEFORE_ACCOUNT_INACTIVITY_NOTIFY_DELAY"]) + - timedelta(days=1) # add margin + ) + + inactive_user = UserFactory(current_login_at=notification_comparison_date) + UserFactory(current_login_at=datetime.utcnow()) # Active user + + with capture_mails() as mails: + tasks.notify_inactive_users() + + # Assert (only one) mail has been sent + self.assertEqual(len(mails), 1) + self.assertEqual(mails[0].send_to, set([inactive_user.email])) + self.assertEqual( + mails[0].subject, + _("Inactivity of your {site} account").format(site=current_app.config["SITE_TITLE"]), + ) + + # We shouldn't notify users twice + with capture_mails() as mails: + tasks.notify_inactive_users() + + self.assertEqual(len(mails), 0) + + @pytest.mark.options(YEARS_OF_INACTIVITY_BEFORE_DEACTIVATION=3) + @pytest.mark.options(MAX_NUMBER_OF_USER_INACTIVITY_NOTIFICATIONS=10) + def test_notify_inactive_users_max_notifications(self): + notification_comparison_date = ( + datetime.utcnow() + - timedelta(days=current_app.config["YEARS_OF_INACTIVITY_BEFORE_DEACTIVATION"] * 365) + + timedelta(days=current_app.config["DAYS_BEFORE_ACCOUNT_INACTIVITY_NOTIFY_DELAY"]) + - timedelta(days=1) # add margin + ) + + NB_USERS_TO_NOTIFY = 15 + + [UserFactory(current_login_at=notification_comparison_date) for _ in range(15)] + UserFactory(current_login_at=datetime.utcnow()) # Active user + + with capture_mails() as mails: + tasks.notify_inactive_users() + + # Assert MAX_NUMBER_OF_USER_INACTIVITY_NOTIFICATIONS mails have been sent + self.assertEqual( + len(mails), current_app.config["MAX_NUMBER_OF_USER_INACTIVITY_NOTIFICATIONS"] + ) + + # Second batch + with capture_mails() as mails: + tasks.notify_inactive_users() + + # Assert what's left have been sent + self.assertEqual( + len(mails), + NB_USERS_TO_NOTIFY - current_app.config["MAX_NUMBER_OF_USER_INACTIVITY_NOTIFICATIONS"], + ) + + @pytest.mark.options(YEARS_OF_INACTIVITY_BEFORE_DEACTIVATION=3) + def test_delete_inactive_users(self): + deletion_comparison_date = ( + datetime.utcnow() + - timedelta(days=current_app.config["YEARS_OF_INACTIVITY_BEFORE_DEACTIVATION"] * 365) + - timedelta(days=1) # add margin + ) + + notification_comparison_date = ( + datetime.utcnow() + - timedelta(days=current_app.config["DAYS_BEFORE_ACCOUNT_INACTIVITY_NOTIFY_DELAY"]) + - timedelta(days=1) # add margin + ) + + inactive_user_to_delete = UserFactory( + current_login_at=deletion_comparison_date, + inactive_deletion_notified_at=notification_comparison_date, + ) + UserFactory(current_login_at=datetime.utcnow()) # Active user + discussion = DiscussionFactory(user=inactive_user_to_delete) + discussion_title = discussion.title + + with capture_mails() as mails: + tasks.delete_inactive_users() + + # Assert (only one) mail has been sent + self.assertEqual(len(mails), 1) + self.assertEqual(mails[0].send_to, set([inactive_user_to_delete.email])) + self.assertEqual( + mails[0].subject, + _( + _("Deletion of your inactive {site} account").format( + site=current_app.config["SITE_TITLE"] + ) + ), + ) + + # Assert user has been deleted but not its discussion + inactive_user_to_delete.reload() + discussion.reload() + self.assertEqual(inactive_user_to_delete.fullname, "DELETED DELETED") + self.assertEqual(discussion.title, discussion_title) + + @pytest.mark.options(YEARS_OF_INACTIVITY_BEFORE_DEACTIVATION=3) + def test_keep_inactive_users_that_logged_in(self): + notification_comparison_date = ( + datetime.utcnow() + - timedelta(days=current_app.config["DAYS_BEFORE_ACCOUNT_INACTIVITY_NOTIFY_DELAY"]) + - timedelta(days=1) # add margin + ) + + inactive_user_that_logged_in_since_notification = UserFactory( + current_login_at=datetime.utcnow(), + inactive_deletion_notified_at=notification_comparison_date, + ) + + with capture_mails() as mails: + tasks.delete_inactive_users() + + # Assert no mail has been sent + self.assertEqual(len(mails), 0) + + # Assert user hasn't been deleted and won't be deleted + self.assertEqual(User.objects().count(), 1) + user = User.objects().first() + self.assertEqual(user, inactive_user_that_logged_in_since_notification) + self.assertIsNone(user.inactive_deletion_notified_at)