Schedule deletion of users (#1872)

* Accounts to be scheduled to be deleted cannot receive emails or login

* Create model and create migration for user

* Add test for the cron function

* Move logic to one place

* Use the class name to call the static delete method

app/admin_model.py

@@ -256,6 +256,17 @@ class UserAdmin(SLModelView):
 
         Session.commit()
 
+    @action(
+        "clear_delete_on",
+        "Remove scheduled deletion of user",
+        "This will remove the scheduled deletion for this users",
+    )
+    def clean_delete_on(self, ids):
+        for user in User.filter(User.id.in_(ids)):
+            user.delete_on = None
+
+        Session.commit()
+
     # @action(
     #     "login_as",
     #     "Login as this user",

app/api/views/auth.py

@@ -63,6 +63,11 @@ def auth_login():
     elif user.disabled:
         LoginEvent(LoginEvent.ActionType.disabled_login, LoginEvent.Source.api).send()
         return jsonify(error="Account disabled"), 400
+    elif user.delete_on is not None:
+        LoginEvent(
+            LoginEvent.ActionType.scheduled_to_be_deleted, LoginEvent.Source.api
+        ).send()
+        return jsonify(error="Account scheduled for deletion"), 400
     elif not user.activated:
         LoginEvent(LoginEvent.ActionType.not_activated, LoginEvent.Source.api).send()
         return jsonify(error="Account not activated"), 422

app/auth/views/login.py

@@ -54,6 +54,12 @@ def login():
                 "error",
             )
             LoginEvent(LoginEvent.ActionType.disabled_login).send()
+        elif user.delete_on is not None:
+            flash(
+                f"Your account is scheduled to be deleted on {user.delete_on}",
+                "error",
+            )
+            LoginEvent(LoginEvent.ActionType.scheduled_to_be_deleted).send()
         elif not user.activated:
             show_resend_activation = True
             flash(

app/events/auth_event.py

@@ -9,6 +9,7 @@ class LoginEvent:
         failed = 1
         disabled_login = 2
         not_activated = 3
+        scheduled_to_be_deleted = 4
 
     class Source(EnumE):
         web = 0

app/models.py

@@ -539,10 +539,14 @@ class User(Base, ModelMixin, UserMixin, PasswordOracle):
         nullable=False,
     )
 
+    # Trigger hard deletion of the account at this time
+    delete_on = sa.Column(ArrowType, default=None)
+
     __table_args__ = (
         sa.Index(
             "ix_users_activated_trial_end_lifetime", activated, trial_end, lifetime
         ),
+        sa.Index("ix_users_delete_on", delete_on),
     )
 
     @property
@@ -833,6 +837,17 @@ class User(Base, ModelMixin, UserMixin, PasswordOracle):
                 < self.max_alias_for_free_account()
             )
 
+    def can_send_or_receive(self) -> bool:
+        if self.disabled:
+            LOG.i(f"User {self} is disabled. Cannot receive or send emails")
+            return False
+        if self.delete_on is not None:
+            LOG.i(
+                f"User {self} is scheduled to be deleted. Cannot receive or send emails"
+            )
+            return False
+        return True
+
     def profile_picture_url(self):
         if self.profile_picture_id:
             return self.profile_picture.get_url()

cron.py

@@ -5,7 +5,7 @@ from typing import List, Tuple
 
 import arrow
 import requests
-from sqlalchemy import func, desc, or_
+from sqlalchemy import func, desc, or_, and_
 from sqlalchemy.ext.compiler import compiles
 from sqlalchemy.orm import joinedload
 from sqlalchemy.orm.exc import ObjectDeletedError
@@ -1106,6 +1106,18 @@ def notify_hibp():
         Session.commit()
 
 
+def clear_users_scheduled_to_be_deleted():
+    users = User.filter(
+        and_(User.delete_on.isnot(None), User.delete_on < arrow.now())
+    ).all()
+    for user in users:
+        LOG.i(
+            f"Scheduled deletion of user {user} with scheduled delete on {user.delete_on}"
+        )
+        User.delete(user.id)
+        Session.commit()
+
+
 if __name__ == "__main__":
     LOG.d("Start running cronjob")
     parser = argparse.ArgumentParser()
@@ -1172,3 +1184,6 @@ if __name__ == "__main__":
         elif args.job == "send_undelivered_mails":
             LOG.d("Sending undelivered emails")
             load_unsent_mails_from_fs_and_resend()
+        elif args.job == "delete_scheduled_users":
+            LOG.d("Deleting users scheduled to be deleted")
+            clear_users_scheduled_to_be_deleted()

crontab.yml

@@ -61,7 +61,12 @@ jobs:
     schedule: "15 10 * * *"
     captureStderr: true
 
-
+  - name: SimpleLogin delete users scheduled to be deleted
+    command: echo disabled_user_deletion #python /code/cron.py -j delete_scheduled_users
+    shell: /bin/bash
+    schedule: "15 11 * * *"
+    captureStderr: true
+    concurrencyPolicy: Forbid
 
   - name: SimpleLogin send unsent emails
     command: python /code/cron.py -j send_undelivered_mails

email_handler.py

@@ -637,8 +637,8 @@ def handle_forward(envelope, msg: Message, rcpt_to: str) -> List[Tuple[bool, str
 
     user = alias.user
 
-    if user.disabled:
-        LOG.w("User %s disabled, disable forwarding emails for %s", user, alias)
+    if not user.can_send_or_receive():
+        LOG.i(f"User {user} cannot receive emails")
         if should_ignore_bounce(envelope.mail_from):
             return [(True, status.E207)]
         else:
@@ -1070,13 +1070,8 @@ def handle_reply(envelope, msg: Message, rcpt_to: str) -> (bool, str):
     user = alias.user
     mail_from = envelope.mail_from
 
-    if user.disabled:
-        LOG.e(
-            "User %s disabled, disable sending emails from %s to %s",
-            user,
-            alias,
-            contact,
-        )
+    if not user.can_send_or_receive():
+        LOG.i(f"User {user} cannot send emails")
         return False, status.E504
 
     # Check if we need to reject or quarantine based on dmarc

migrations/versions/2023_090715_0a5701a4f5e4_.py

@@ -0,0 +1,33 @@
+"""empty message
+
+Revision ID: 0a5701a4f5e4
+Revises: 01827104004b
+Create Date: 2023-09-07 15:28:10.122756
+
+"""
+import sqlalchemy_utils
+from alembic import op
+import sqlalchemy as sa
+
+
+# revision identifiers, used by Alembic.
+revision = '0a5701a4f5e4'
+down_revision = '01827104004b'
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.add_column('users', sa.Column('delete_on', sqlalchemy_utils.types.arrow.ArrowType(), nullable=True))
+    with op.get_context().autocommit_block():
+        op.create_index('ix_users_delete_on', 'users', ['delete_on'], unique=False, postgresql_concurrently=True)
+    # ### end Alembic commands ###
+
+
+def downgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    with op.get_context().autocommit_block():
+        op.drop_index('ix_users_delete_on', table_name='users', postgresql_concurrently=True)
+    op.drop_column('users', 'delete_on')
+    # ### end Alembic commands ###

tests/test_cron.py

@@ -1,18 +1,17 @@
 import arrow
 
-from app.models import CoinbaseSubscription, ApiToCookieToken, ApiKey
-from cron import notify_manual_sub_end, delete_expired_tokens
+import cron
+from app.db import Session
+from app.models import CoinbaseSubscription, ApiToCookieToken, ApiKey, User
 from tests.utils import create_new_user
 
 
 def test_notify_manual_sub_end(flask_client):
     user = create_new_user()
-
     CoinbaseSubscription.create(
         user_id=user.id, end_at=arrow.now().shift(days=13, hours=2), commit=True
     )
-
-    notify_manual_sub_end()
+    cron.notify_manual_sub_end()
 
 
 def test_cleanup_tokens(flask_client):
@@ -33,6 +32,22 @@ def test_cleanup_tokens(flask_client):
         api_key_id=api_key.id,
         commit=True,
     ).id
-    delete_expired_tokens()
+    cron.delete_expired_tokens()
     assert ApiToCookieToken.get(id_to_clean) is None
     assert ApiToCookieToken.get(id_to_keep) is not None
+
+
+def test_cleanup_users():
+    u_delete_none_id = create_new_user().id
+    u_delete_after = create_new_user()
+    u_delete_after_id = u_delete_after.id
+    u_delete_before = create_new_user()
+    u_delete_before_id = u_delete_before.id
+    now = arrow.now()
+    u_delete_after.delete_on = now.shift(minutes=1)
+    u_delete_before.delete_on = now.shift(minutes=-1)
+    Session.flush()
+    cron.clear_users_scheduled_to_be_deleted()
+    assert User.get(u_delete_none_id) is not None
+    assert User.get(u_delete_after_id) is not None
+    assert User.get(u_delete_before_id) is None

tests/test_models.py

@@ -315,3 +315,13 @@ def test_create_contact_for_noreply(flask_client):
         reply_email=generate_reply_email(NOREPLY, alias),
     )
     assert contact.website_email == NOREPLY
+
+
+def test_user_can_send_receive():
+    user = create_new_user()
+    assert user.can_send_or_receive()
+    user.disabled = True
+    assert not user.can_send_or_receive()
+    user.disabled = False
+    user.delete_on = arrow.now()
+    assert not user.can_send_or_receive()