Allow to soft-delete users (#2034)

* Allow the possibility of soft-deleting users

* Unschedule for delete after link

* Add dry run to the cron

app/account_linking.py

@@ -168,6 +168,8 @@ class NewUserStrategy(ClientMergeStrategy):
 
 class ExistingUnlinkedUserStrategy(ClientMergeStrategy):
     def process(self) -> LinkResult:
+        # IF it was scheduled to be deleted. Unschedule it.
+        self.user.delete_on = None
         partner_user = ensure_partner_user_exists_for_user(
             self.link_request, self.user, self.partner
         )
@@ -246,6 +248,8 @@ def link_user(
 ) -> LinkResult:
     # Sanitize email just in case
     link_request.email = sanitize_email(link_request.email)
+    # If it was scheduled to be deleted. Unschedule it.
+    current_user.delete_on = None
     partner_user = ensure_partner_user_exists_for_user(
         link_request, current_user, partner
     )

app/api/base.py

@@ -33,6 +33,9 @@ def authorize_request() -> Optional[Tuple[str, int]]:
     if g.user.disabled:
         return jsonify(error="Disabled account"), 403
 
+    if not g.user.is_active():
+        return jsonify(error="Account does not exist"), 401
+
     g.api_key = api_key
     return None
 

app/models.py

@@ -727,6 +727,11 @@ class User(Base, ModelMixin, UserMixin, PasswordOracle):
 
         return True
 
+    def is_active(self) -> bool:
+        if self.delete_on is None:
+            return True
+        return self.delete_on < arrow.now()
+
     def in_trial(self):
         """return True if user does not have lifetime licence or an active subscription AND is in trial period"""
         if self.lifetime_or_active_subscription():
@@ -828,6 +833,9 @@ class User(Base, ModelMixin, UserMixin, PasswordOracle):
         Whether user can create a new alias. User can't create a new alias if
         - has more than 15 aliases in the free plan, *even in the free trial*
         """
+        if not self.is_active():
+            return False
+
         if self.disabled:
             return False
 

app/oauth/views/authorize.py

@@ -140,7 +140,7 @@ def authorize():
                 Scope=Scope,
             )
     else:  # POST - user allows or denies
-        if not current_user.is_authenticated or not current_user.is_active:
+        if not current_user.is_authenticated or not current_user.is_active():
             LOG.i(
                 "Attempt to validate a OAUth allow request by an unauthenticated user"
             )

cron.py

@@ -62,6 +62,8 @@ from app.proton.utils import get_proton_partner
 from app.utils import sanitize_email
 from server import create_light_app
 
+DELETE_GRACE_DAYS = 30
+
 
 def notify_trial_end():
     for user in User.filter(
@@ -1126,14 +1128,19 @@ def notify_hibp():
         Session.commit()
 
 
-def clear_users_scheduled_to_be_deleted():
+def clear_users_scheduled_to_be_deleted(dry_run=False):
     users = User.filter(
-        and_(User.delete_on.isnot(None), User.delete_on < arrow.now())
+        and_(
+            User.delete_on.isnot(None),
+            User.delete_on <= arrow.now().shift(days=-DELETE_GRACE_DAYS),
+        )
     ).all()
     for user in users:
         LOG.i(
             f"Scheduled deletion of user {user} with scheduled delete on {user.delete_on}"
         )
+        if dry_run:
+            continue
         User.delete(user.id)
         Session.commit()
 
@@ -1206,4 +1213,4 @@ if __name__ == "__main__":
             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()
+            clear_users_scheduled_to_be_deleted(dry_run=True)

crontab.yml

@@ -62,7 +62,7 @@ jobs:
     captureStderr: true
 
   - name: SimpleLogin delete users scheduled to be deleted
-    command: echo disabled_user_deletion #python /code/cron.py -j delete_scheduled_users
+    command: python /code/cron.py -j delete_scheduled_users
     shell: /bin/bash
     schedule: "15 11 * * *"
     captureStderr: true

email_handler.py

@@ -636,6 +636,10 @@ def handle_forward(envelope, msg: Message, rcpt_to: str) -> List[Tuple[bool, str
 
     user = alias.user
 
+    if not user.is_active():
+        LOG.w(f"User {user} has been soft deleted")
+        return False, status.E502
+
     if not user.can_send_or_receive():
         LOG.i(f"User {user} cannot receive emails")
         if should_ignore_bounce(envelope.mail_from):
@@ -1055,6 +1059,9 @@ def handle_reply(envelope, msg: Message, rcpt_to: str) -> (bool, str):
     if not contact:
         LOG.w(f"No contact with {reply_email} as reverse alias")
         return False, status.E502
+    if not contact.user.is_active():
+        LOG.w(f"User {contact.user} has been soft deleted")
+        return False, status.E502
 
     alias = contact.alias
     alias_address: str = contact.alias.email
@@ -1921,6 +1928,9 @@ def handle_bounce(envelope, email_log: EmailLog, msg: Message) -> str:
         contact,
         alias,
     )
+    if not email_log.user.is_active():
+        LOG.d(f"User {email_log.user} is not active")
+        return status.E510
 
     if email_log.is_reply:
         content_type = msg.get_content_type().lower()
@@ -1982,6 +1992,9 @@ def send_no_reply_response(mail_from: str, msg: Message):
     if not mailbox:
         LOG.d("Unknown sender. Skipping reply from {}".format(NOREPLY))
         return
+    if not mailbox.user.is_active():
+        LOG.d(f"User {mailbox.user} is soft-deleted. Skipping sending reply response")
+        return
     send_email_at_most_times(
         mailbox.user,
         ALERT_TO_NOREPLY,

server.py

@@ -228,6 +228,8 @@ def load_user(alternative_id):
         sentry_sdk.set_user({"email": user.email, "id": user.id})
         if user.disabled:
             return None
+        if not user.is_active():
+            return None
 
     return user
 

tests/test_cron.py

@@ -39,15 +39,17 @@ def test_cleanup_tokens(flask_client):
 
 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
+    u_delete_grace_has_expired = create_new_user()
+    u_delete_grace_has_expired_id = u_delete_grace_has_expired.id
+    u_delete_grace_has_not_expired = create_new_user()
+    u_delete_grace_has_not_expired_id = u_delete_grace_has_not_expired.id
     now = arrow.now()
-    u_delete_after.delete_on = now.shift(minutes=1)
-    u_delete_before.delete_on = now.shift(minutes=-1)
+    u_delete_grace_has_expired.delete_on = now.shift(days=-(cron.DELETE_GRACE_DAYS + 1))
+    u_delete_grace_has_not_expired.delete_on = now.shift(
+        days=-(cron.DELETE_GRACE_DAYS - 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
+    assert User.get(u_delete_grace_has_not_expired_id) is not None
+    assert User.get(u_delete_grace_has_expired_id) is None