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

Schedule deletion of users #1872

Merged
merged 5 commits into from
Sep 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions app/admin_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,17 @@ def stop_paddle_sub(self, ids):

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",
Expand Down
5 changes: 5 additions & 0 deletions app/api/views/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions app/auth/views/login.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
1 change: 1 addition & 0 deletions app/events/auth_event.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ class ActionType(EnumE):
failed = 1
disabled_login = 2
not_activated = 3
scheduled_to_be_deleted = 4

class Source(EnumE):
web = 0
Expand Down
15 changes: 15 additions & 0 deletions app/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -833,6 +837,17 @@ def can_create_new_alias(self) -> bool:
< 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()
Expand Down
17 changes: 16 additions & 1 deletion cron.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -1172,3 +1184,6 @@ def notify_hibp():
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()
7 changes: 6 additions & 1 deletion crontab.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 4 additions & 9 deletions email_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down
33 changes: 33 additions & 0 deletions migrations/versions/2023_090715_0a5701a4f5e4_.py
Original file line number Diff line number Diff line change
@@ -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 ###
27 changes: 21 additions & 6 deletions tests/test_cron.py
Original file line number Diff line number Diff line change
@@ -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):
Expand All @@ -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
10 changes: 10 additions & 0 deletions tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Loading