SL abuser system improvements (#2494)

* IDTEAM-4828: Allow to search abuser by user id not just by email.

* IDTEAM-4827: Add abuser admin logs to abuser page.

* IDTEAM-4826: Run the abuser bundle generation in a job with low prio.

app/abuser.py

@@ -0,0 +1,22 @@
+from typing import Optional
+
+from app.abuser_audit_log_utils import emit_abuser_audit_log, AbuserAuditLogAction
+from app.db import Session
+from app.jobs.mark_abuser_job import MarkAbuserJob
+from app.models import User
+
+
+def mark_user_as_abuser(
+    abuse_user: User, note: str, admin_id: Optional[int] = None
+) -> None:
+    abuse_user.disabled = True
+
+    emit_abuser_audit_log(
+        user_id=abuse_user.id,
+        action=AbuserAuditLogAction.MarkAbuser,
+        message=note,
+        admin_id=admin_id,
+    )
+    job = MarkAbuserJob(user=abuse_user)
+    job.store_job_in_db()
+    Session.commit()

app/abuser_utils.py

@@ -51,22 +51,7 @@ def check_if_abuser_email(new_address: str) -> Optional[AbuserLookup]:
     )
 
 
-def mark_user_as_abuser(
-    abuse_user: User, note: str, admin_id: Optional[int] = None
-) -> None:
-    abuse_user.disabled = True
-
-    emit_abuser_audit_log(
-        user_id=abuse_user.id,
-        action=AbuserAuditLogAction.MarkAbuser,
-        message=note,
-        admin_id=admin_id,
-    )
-    Session.commit()
-    _store_abuse_data(abuse_user)
-
-
-def _store_abuse_data(user: User) -> None:
+def store_abuse_data(user: User) -> None:
     """
     Archive the given abusive user's data and update blocklist/lookup tables.
     """
@@ -133,7 +118,7 @@ def _store_abuse_data(user: User) -> None:
         mac_key_bytes = config.MAC_KEY
         master_key_bytes = config.MASTER_ENC_KEY
 
-        for raw_identifier_address in all_identifiers_raw:
+        for idx, raw_identifier_address in enumerate(all_identifiers_raw):
             if not raw_identifier_address:
                 continue
 
@@ -168,6 +153,9 @@ def _store_abuse_data(user: User) -> None:
 
             Session.add(abuser_lookup_entry)
 
+            if idx % 1000 == 0:
+                Session.flush()
+
         Session.commit()
     except Exception:
         Session.rollback()

app/admin_model.py

@@ -1,5 +1,7 @@
 from __future__ import annotations
 
+import json
+from datetime import datetime
 from typing import Optional, List, Dict
 
 import arrow
@@ -15,8 +17,9 @@ from flask_login import current_user
 from markupsafe import Markup
 
 from app import models, s3, config
+from app.abuser_audit_log_utils import AbuserAuditLog
+from app.abuser import mark_user_as_abuser
 from app.abuser_utils import (
-    mark_user_as_abuser,
     unmark_as_abusive_user,
     get_abuser_bundles_for_address,
 )
@@ -56,8 +59,6 @@ from app.newsletter_utils import send_newsletter_to_user, send_newsletter_to_add
 from app.proton.proton_unlink import perform_proton_account_unlink
 from app.user_audit_log_utils import emit_user_audit_log, UserAuditLogAction
 from app.utils import sanitize_email
-from datetime import datetime
-import json
 
 
 def _admin_action_formatter(view, context, model, name):
@@ -1114,19 +1115,42 @@ class CustomDomainSearchAdmin(BaseView):
 class AbuserLookupResult:
     def __init__(self):
         self.no_match: bool = False
-        self.email: Optional[str] = None
+        self.query: Optional[str | int] = None
         self.bundles: Optional[List[Dict]] = None
+        self.audit_log: Optional[List[Dict]] = None
 
     @staticmethod
-    def from_email(email: Optional[str]) -> AbuserLookupResult:
+    def from_email_or_user_id(query: str) -> AbuserLookupResult:
         out = AbuserLookupResult()
+        email: str
+        audit_log: List[AbuserAuditLog] = []
 
-        if email is None or email == "":
+        if query is None or query == "":
             out.no_match = True
 
             return out
 
-        out.email = email
+        if query.isnumeric():
+            user_id = int(query)
+            user = User.get(user_id)
+
+            if not user:
+                out.no_match = True
+
+                return out
+
+            email = user.email
+            audit_log = AbuserAuditLog.filter(AbuserAuditLog.user_id == user.id).all()
+        else:
+            email = sanitize_email(query)
+            user = User.get_by(email=email)
+
+            if user:
+                audit_log = AbuserAuditLog.filter(
+                    AbuserAuditLog.user_id == user.id
+                ).all()
+
+        out.query = query
         bundles = get_abuser_bundles_for_address(
             target_address=email,
             admin_id=current_user.id,
@@ -1153,6 +1177,15 @@ class AbuserLookupResult:
                 AbuserLookupResult.convert_dt(alias_item)
 
         out.bundles = bundles
+        out.audit_log = [
+            {
+                "admin_id": alog.admin_id,
+                "action": alog.action,
+                "message": alog.message,
+                "created_at": alog.created_at,
+            }
+            for alog in audit_log
+        ]
 
         return out
 
@@ -1174,13 +1207,12 @@ class AbuserLookupAdmin(BaseView):
 
     @expose("/", methods=["GET", "POST"])
     def index(self):
-        query = request.args.get("email")
+        query: Optional[str] = request.args.get("search")
 
         if query is None:
             result = AbuserLookupResult()
         else:
-            email = sanitize_email(query)
-            result = AbuserLookupResult.from_email(email)
+            result = AbuserLookupResult.from_email_or_user_id(query)
 
         return self.render(
             "admin/abuser_lookup.html",

app/constants.py

@@ -19,3 +19,4 @@ class JobType(enum.Enum):
     SEND_ALIAS_CREATION_EVENTS = "send-alias-creation-events"
     SEND_EVENT_TO_WEBHOOK = "send-event-to-webhook"
     SYNC_SUBSCRIPTION = "sync-subscription"
+    ABUSER_MARK = "abuser-mark"

app/jobs/mark_abuser_job.py

@@ -0,0 +1,38 @@
+from __future__ import annotations
+
+from typing import Optional
+
+import arrow
+
+from app.abuser_utils import store_abuse_data
+from app.constants import JobType
+from app.models import (
+    User,
+    Job,
+    JobPriority,
+)
+
+
+class MarkAbuserJob:
+    def __init__(self, user: User):
+        self._user: User = user
+
+    def run(self) -> None:
+        store_abuse_data(user=self._user)
+
+    @staticmethod
+    def create_from_job(job: Job) -> Optional[MarkAbuserJob]:
+        user = User.get(job.payload["user_id"])
+        if not user:
+            return None
+
+        return MarkAbuserJob(user)
+
+    def store_job_in_db(self) -> Job:
+        return Job.create(
+            name=JobType.ABUSER_MARK.value,
+            payload={"user_id": self._user.id},
+            priority=JobPriority.Low,
+            run_at=arrow.now(),
+            commit=True,
+        )

job_runner.py

@@ -23,6 +23,7 @@ from app.events.event_dispatcher import PostgresDispatcher
 from app.import_utils import handle_batch_import
 from app.jobs.event_jobs import send_alias_creation_events_for_user
 from app.jobs.export_user_data_job import ExportUserDataJob
+from app.jobs.mark_abuser_job import MarkAbuserJob
 from app.jobs.send_event_job import SendEventToWebhookJob
 from app.jobs.sync_subscription_job import SyncSubscriptionJob
 from app.log import LOG
@@ -291,6 +292,10 @@ def process_job(job: Job):
         sync_job = SyncSubscriptionJob.create_from_job(job)
         if sync_job:
             sync_job.run(HttpEventSink())
+    elif job.name == JobType.ABUSER_MARK.value:
+        mark_abuser_job = MarkAbuserJob.create_from_job(job)
+        if mark_abuser_job:
+            mark_abuser_job.run()
     else:
         LOG.e("Unknown job name %s", job.name)
 

oneshot/backfill_abuser_data.py

@@ -4,7 +4,7 @@ from typing import List
 
 from sqlalchemy import func
 
-from app.abuser_utils import mark_user_as_abuser
+from app.abuser import mark_user_as_abuser
 from app.db import Session
 from app.models import User, AbuserData
 

templates/admin/abuser_lookup.html

@@ -4,50 +4,50 @@
     <h5>Overview</h5>
     <table class="table table-striped table-bordered table-hover">
         <thead>
-            <tr>
-                <th scope="col">User ID</th>
-                <th scope="col">Primary email address</th>
-                <th scope="col">User created</th>
-            </tr>
+        <tr>
+            <th scope="col">User ID</th>
+            <th scope="col">Primary email address</th>
+            <th scope="col">User created</th>
+        </tr>
         </thead>
         <tbody>
-            <tr>
-                <td>
-                    {% if bundle.get('user', None) %}
-                        <a href="/admin/user/?search={{ bundle.get('user').id }}">{{ bundle.get('user').id }}</a>
-                    {% else %}
-                        {{ bundle.get('account_id') }}
-                    {% endif %}
-                </td>
-                <td>
-                    {% if bundle.get('user', None) %}
-                        <a href="/admin/admin.email_search/?query={{ bundle.get('user').email }}">{{ bundle.get('user').email }}</a>
-                    {% else %}
-                        {{ bundle.get('email', '') }}
-                    {% endif %}
-                </td>
-                <td>{{ bundle.get('user_created_at', '').strftime('%B %d, %Y %I:%M %p') }}</td>
-            </tr>
+        <tr>
+            <td>
+                {% if bundle.get('user', None) %}
+                    <a href="/admin/user/?search={{ bundle.get('user').id }}">{{ bundle.get('user').id }}</a>
+                {% else %}
+                    {{ bundle.get('account_id') }}
+                {% endif %}
+            </td>
+            <td>
+                {% if bundle.get('user', None) %}
+                    <a href="/admin/admin.email_search/?query={{ bundle.get('user').email }}">{{ bundle.get('user').email }}</a>
+                {% else %}
+                    {{ bundle.get('email', '') }}
+                {% endif %}
+            </td>
+            <td>{{ bundle.get('user_created_at', '').strftime('%B %d, %Y %I:%M %p') }}</td>
+        </tr>
         </tbody>
     </table>
 {%- endmacro %}
 {% macro show_emails_table(emails) -%}
     <table class="table table-striped table-bordered table-hover">
         <thead>
-            <tr>
-                <th scope="col">#</th>
-                <th scope="col">Email address</th>
-                <th scope="col">Date created</th>
-            </tr>
+        <tr>
+            <th scope="col">#</th>
+            <th scope="col">Email address</th>
+            <th scope="col">Date created</th>
+        </tr>
         </thead>
         <tbody>
-            {% for idx, email in emails|enumerate %}
-                <tr>
-                    <th scope="row">{{ idx + 1 }}</th>
-                    <td>{{ email.get('address', '') }}</td>
-                    <td>{{ email.get('created_at', '').strftime('%B %d, %Y %I:%M %p') }}</td>
-                </tr>
-            {% endfor %}
+        {% for idx, email in emails|enumerate %}
+            <tr>
+                <th scope="row">{{ idx + 1 }}</th>
+                <td>{{ email.get('address', '') }}</td>
+                <td>{{ email.get('created_at', '').strftime('%B %d, %Y %I:%M %p') }}</td>
+            </tr>
+        {% endfor %}
         </tbody>
     </table>
 {%- endmacro %}
@@ -63,7 +63,10 @@
         {{ show_emails_table(bundle.get('mailboxes', [])) }}
     {% endif %}
     <div>
-        <button type="button" class="btn btn-secondary" onclick="navigator.clipboard.writeText('{{ bundle.get('json', '') }}');alert('The bundle data has been copied to your clipboard.');">Copy bundle</button>
+        <button type="button" class="btn btn-secondary"
+                onclick="navigator.clipboard.writeText('{{ bundle.get('json', '') }}');alert('The bundle data has been copied to your clipboard.');">
+            Copy bundle
+        </button>
     </div>
     <hr>
 {%- endmacro %}
@@ -76,31 +79,74 @@
         {% endfor %}
     </div>
 {%- endmacro %}
+{% macro show_audit_log(audit_log) -%}
+    {% if audit_log and audit_log|length > 0 %}
+        <p>
+            <a class="btn btn-primary" data-toggle="collapse" href="#multiCollapseExample1" role="button"
+               aria-expanded="false" aria-controls="multiCollapseExample1">Toggle audit log</a>
+        </p>
+        <div class="row">
+            <div class="col">
+                <div class="collapse multi-collapse" id="multiCollapseExample1">
+                    <div class="card card-body">
+                        <table class="table table-striped table-bordered table-hover">
+                            <thead>
+                            <tr>
+                                <th scope="col">#</th>
+                                <th scope="col">Action</th>
+                                <th scope="col">Message</th>
+                                <th scope="col">Date created</th>
+                                <th scope="col">Admin User ID</th>
+                            </tr>
+                            </thead>
+                            <tbody>
+                            {% for idx, log in audit_log|enumerate %}
+                                <tr>
+                                    <th scope="row">{{ idx + 1 }}</th>
+                                    <td>
+                                        <code>{{ log.get('action', '') }}</code>
+                                    </td>
+                                    <td>{{ log.get('message', '') }}</td>
+                                    <td>{{ log.get('created_at', '').strftime('%B %d, %Y %I:%M %p') }}</td>
+                                    <td>
+                                        <a href="/admin/user/?search={{ log.admin_id }}">{{ log.admin_id }}</a>
+                                    </td>
+                                </tr>
+                            {% endfor %}
+                            </tbody>
+                        </table>
+                    </div>
+                    <br>
+                </div>
+            </div>
+        </div>
+    {% endif %}
+{%- endmacro %}
 {% block body %}
 
-  <div class="border border-dark border-2 mt-1 mb-2 p-3">
-    <form method="get">
-      <div class="form-group">
-        <label for="email">Email address:</label>
-        <input type="email"
-               class="form-control"
-               name="email"
-               value="{{ query or '' }}" />
-      </div>
-      <button type="submit" class="btn btn-primary">Look up</button>
-    </form>
-  </div>
-  {% if data.no_match and query %}
-
-    <div class="border border-dark border-2 mt-1 mb-2 p-3 alert alert-warning"
-         role="alert">No abuser data was found for the provided email address.</div>
-  {% endif %}
-  {% if data.bundles %}
-
     <div class="border border-dark border-2 mt-1 mb-2 p-3">
-      <h3 class="mb-3">Found abuser data for <code>{{ data.email }}</code></h3>
-      {{ show_bundles(data.bundles) }}
+        <form method="get">
+            <div class="form-group">
+                <label for="search">UserID or Email to search::</label>
+                <input type="text"
+                       class="form-control"
+                       name="search"
+                       value="{{ query or '' }}"/>
+            </div>
+            <button type="submit" class="btn btn-primary">Look up</button>
+        </form>
+    </div>
+    {% if data.no_match and query %}
+        <div class="border border-dark border-2 mt-1 mb-2 p-3 alert alert-warning"
+             role="alert">No abuser data was found for the provided email address.
+        </div>
+    {% endif %}
+    {% if data.bundles %}
+        <div class="border border-dark border-2 mt-1 mb-2 p-3">
+            <h3 class="mb-3">Found abuser data for <code>{{ data.query }}</code></h3>
+            {{ show_audit_log(data.audit_log) }}
+            {{ show_bundles(data.bundles) }}
+        </div>
+    {% endif %}
     </div>
-  {% endif %}
-</div>
 {% endblock %}

tests/test_abuser_utils.py

@@ -7,14 +7,14 @@ from cryptography.hazmat.backends import default_backend
 from cryptography.hazmat.primitives import hashes as crypto_hashes
 from cryptography.hazmat.primitives.kdf.hkdf import HKDF
 
+from app import constants
 from app.abuser_utils import (
-    mark_user_as_abuser,
     check_if_abuser_email,
     get_abuser_bundles_for_address,
     unmark_as_abusive_user,
     _derive_key_for_identifier,
+    store_abuse_data,
 )
-from app import constants
 from app.db import Session
 from app.models import AbuserLookup, AbuserData, Alias, Mailbox, User
 from tests.utils import random_email, create_new_user
@@ -136,7 +136,7 @@ def test_archive_basic_user(flask_client, monkeypatch):
 
     mailbox1_email_normalized = mailbox1.email.lower()
     Session.commit()
-    mark_user_as_abuser(user, "")
+    store_abuse_data(user)
     ab_data = AbuserData.filter_by(user_id=user.id).first()
 
     assert ab_data is not None
@@ -180,7 +180,7 @@ def test_archive_user_with_no_aliases_or_mailboxes(flask_client, monkeypatch):
     user_primary_email_normalized = user.email.lower()
     Alias.filter_by(user_id=user.id).delete(synchronize_session=False)
     Session.commit()
-    mark_user_as_abuser(user, "")
+    store_abuse_data(user)
     ab_data = AbuserData.filter_by(user_id=user.id).first()
 
     assert ab_data is not None
@@ -224,7 +224,7 @@ def test_duplicate_addresses_do_not_create_duplicate_lookups(flask_client, monke
     default_mb.email = duplicate_email_normalized
     Session.add(default_mb)
     Session.commit()
-    mark_user_as_abuser(user, "")
+    store_abuse_data(user)
     identifier_hmac_duplicate = calculate_hmac(duplicate_email_normalized)
     ab_data = AbuserData.filter_by(user_id=user.id).first()
 
@@ -253,7 +253,7 @@ def test_invalid_user_or_identifier_fails_gracefully(flask_client, monkeypatch):
     with pytest.raises(
         ValueError, match=f"User ID {user_obj_no_email.id} must have a primary email"
     ):
-        mark_user_as_abuser(user_obj_no_email, "")
+        store_abuse_data(user_obj_no_email)
 
 
 def test_can_decrypt_bundle_for_all_valid_identifiers(flask_client, monkeypatch):
@@ -283,7 +283,7 @@ def test_can_decrypt_bundle_for_all_valid_identifiers(flask_client, monkeypatch)
 
     mailbox1_email_normalized = mailbox1.email.lower()
     Session.commit()
-    mark_user_as_abuser(user, "")
+    store_abuse_data(user)
     ab_data = AbuserData.filter_by(user_id=user.id).first()
 
     assert ab_data is not None
@@ -318,7 +318,7 @@ def test_db_rollback_on_error(monkeypatch, flask_client):
     monkeypatch.setattr(Session, "commit", mock_commit_failure)
 
     with pytest.raises(RuntimeError, match="Simulated DB failure during commit"):
-        mark_user_as_abuser(user, "")
+        store_abuse_data(user)
 
     monkeypatch.setattr(Session, "commit", original_commit)  # Restore
     Session.rollback()
@@ -333,7 +333,7 @@ def test_db_rollback_on_error(monkeypatch, flask_client):
 def test_unarchive_abusive_user_removes_data(flask_client, monkeypatch):
     user = create_new_user()
     email_normalized = user.email.lower()
-    mark_user_as_abuser(user, "")
+    store_abuse_data(user)
 
     assert AbuserData.filter_by(user_id=user.id).first() is not None
     assert get_lookup_count_for_address(email_normalized) > 0
@@ -357,7 +357,7 @@ def test_unarchive_idempotent_on_missing_data(flask_client, monkeypatch):
 
 def test_abuser_data_deletion_cascades_to_lookup(flask_client, monkeypatch):
     user = create_new_user()
-    mark_user_as_abuser(user, "")
+    store_abuse_data(user)
     ab_data = AbuserData.filter_by(user_id=user.id).first()
 
     assert ab_data is not None
@@ -374,7 +374,7 @@ def test_abuser_data_deletion_cascades_to_lookup(flask_client, monkeypatch):
 
 def test_archive_then_unarchive_then_rearchive_is_consistent(flask_client, monkeypatch):
     user = create_new_user()
-    mark_user_as_abuser(user, "")
+    store_abuse_data(user)
     ab_data1 = AbuserData.filter_by(user_id=user.id).first()
 
     assert ab_data1 is not None
@@ -383,7 +383,7 @@ def test_archive_then_unarchive_then_rearchive_is_consistent(flask_client, monke
 
     assert AbuserData.filter_by(user_id=user.id).first() is None
 
-    mark_user_as_abuser(user, "")
+    store_abuse_data(user)
     ab_data2 = AbuserData.filter_by(user_id=user.id).first()
 
     assert ab_data2 is not None
@@ -393,7 +393,7 @@ def test_archive_then_unarchive_then_rearchive_is_consistent(flask_client, monke
 def test_get_abuser_bundles_returns_bundle_for_primary_email(flask_client, monkeypatch):
     user = create_new_user()
     email_normalized = user.email.lower()
-    mark_user_as_abuser(user, "")
+    store_abuse_data(user)
     bundles = get_abuser_bundles_for_address(email_normalized, -1)
 
     assert len(bundles) == 1
@@ -429,7 +429,7 @@ def test_get_abuser_bundles_from_alias_address(flask_client, monkeypatch):
         mailbox_id=user.default_mailbox_id,
         commit=True,
     )
-    mark_user_as_abuser(user, "")
+    store_abuse_data(user)
     results = get_abuser_bundles_for_address(alias_email_normalized, -1)
 
     assert len(results) == 1
@@ -469,7 +469,7 @@ def test_get_abuser_bundles_from_mailbox_address(flask_client, monkeypatch):
         Session.commit()
 
     current_mailbox_email_normalized = mailbox.email.lower()
-    mark_user_as_abuser(user, "")
+    store_abuse_data(user)
 
     results = get_abuser_bundles_for_address(current_mailbox_email_normalized, -1)
 
@@ -488,7 +488,7 @@ def test_get_abuser_bundles_with_corrupt_encrypted_k_bundle_is_skipped(
     flask_client, monkeypatch
 ):
     user = create_new_user()
-    mark_user_as_abuser(user, "")
+    store_abuse_data(user)
     identifier_hmac = calculate_hmac(user.email)
     lookup_entry = AbuserLookup.filter_by(hashed_address=identifier_hmac).first()
 
@@ -512,7 +512,7 @@ def test_get_abuser_bundles_with_corrupt_main_bundle_is_skipped(
     flask_client, monkeypatch
 ):
     user = create_new_user()
-    mark_user_as_abuser(user, "")
+    store_abuse_data(user)
     ab_data = AbuserData.filter_by(user_id=user.id).first()
 
     assert ab_data is not None
@@ -550,7 +550,7 @@ def test_archive_and_fetch_flow_end_to_end(flask_client, monkeypatch):
         Session.commit()
 
     current_mailbox_email_normalized = mailbox.email.lower()
-    mark_user_as_abuser(user, "")
+    store_abuse_data(user)
     bundles = get_abuser_bundles_for_address(user.email, -1)
 
     assert len(bundles) == 1