Centralize disabling contacts to ensure the logs are consistent

app/api/views/alias.py

@@ -19,6 +19,7 @@ from app.api.serializer import (
     get_alias_info_v2,
     get_alias_infos_with_pagination_v3,
 )
+from app.contact_utils import contact_toggle_block
 from app.dashboard.views.alias_contact_manager import create_contact
 from app.dashboard.views.alias_log import get_alias_log
 from app.db import Session
@@ -485,13 +486,6 @@ def toggle_contact(contact_id):
 
     if not contact or contact.alias.user_id != user.id:
         return jsonify(error="Forbidden"), 403
-
-    contact.block_forward = not contact.block_forward
-    emit_alias_audit_log(
-        alias=contact.alias,
-        action=AliasAuditLogAction.UpdateContact,
-        message=f"Set contact state {contact.id} {contact.email} -> {contact.website_email} to blocked {contact.block_forward}",
-    )
-    Session.commit()
+    contact_toggle_block(contact)
 
     return jsonify(block_forward=contact.block_forward), 200

app/contact_utils.py

@@ -136,3 +136,14 @@ def create_contact(
             return ContactCreateResult(
                 None, created=False, error=ContactCreateError.Unknown
             )
+
+
+def contact_toggle_block(contact: Contact) -> Contact:
+    contact.block_forward = not contact.block_forward
+    emit_alias_audit_log(
+        alias=contact.alias,
+        action=AliasAuditLogAction.UpdateContact,
+        message=f"Set contact state {contact.id} {contact.email} -> {contact.website_email} to blocked {contact.block_forward}",
+    )
+    Session.commit()
+    LOG.i(f"Updated contact {contact} blocked state to {contact.block_forward}")

app/dashboard/views/index.py

@@ -6,6 +6,7 @@ from flask_login import login_required, current_user
 from app import alias_utils, parallel_limiter, alias_delete
 from app.api.serializer import get_alias_infos_with_pagination_v3, get_alias_info_v3
 from app.config import ALIAS_LIMIT, PAGE_LIMIT
+from app.contact_utils import contact_toggle_block
 from app.dashboard.base import dashboard_bp
 from app.db import Session
 from app.extensions import limiter
@@ -253,9 +254,7 @@ def toggle_contact(contact_id):
     if not contact or contact.alias.user_id != current_user.id:
         return "Forbidden", 403
 
-    contact.block_forward = not contact.block_forward
-    Session.commit()
-
+    contact_toggle_block(contact)
     if contact.block_forward:
         toast_msg = f"{contact.website_email} can no longer send emails to {contact.alias.email}"
     else:

app/dashboard/views/unsubscribe.py

@@ -1,15 +1,13 @@
 """
 Allow user to disable an alias or block a contact via the one click unsubscribe
 """
-
-from app.db import Session
-
-
 from flask import redirect, url_for, flash, request, render_template
 from flask_login import login_required, current_user
 
 from app import alias_utils
+from app.contact_utils import contact_toggle_block
 from app.dashboard.base import dashboard_bp
+from app.db import Session
 from app.handler.unsubscribe_encoder import UnsubscribeAction
 from app.handler.unsubscribe_handler import UnsubscribeHandler
 from app.models import Alias, Contact
@@ -60,9 +58,11 @@ def block_contact(contact_id):
 
     # automatic unsubscribe, according to https://tools.ietf.org/html/rfc8058
     if request.method == "POST":
-        contact.block_forward = True
-        flash(f"Emails sent from {contact.website_email} are now blocked", "success")
-        Session.commit()
+        if contact.block_forward is False:
+            contact_toggle_block(contact)
+            flash(
+                f"Emails sent from {contact.website_email} are now blocked", "success"
+            )
 
         return redirect(
             url_for(

app/fake_data.py

@@ -181,7 +181,7 @@ def fake_data():
     custom_domain1 = CustomDomain.create(user_id=user.id, domain="ab.cd", verified=True)
     Session.commit()
 
-    Alias.create(
+    alias = Alias.create(
         user_id=user.id,
         email="first@ab.cd",
         mailbox_id=user.default_mailbox_id,
@@ -189,6 +189,22 @@ def fake_data():
         commit=True,
     )
 
+    contact = Contact.create(
+        user_id=user.id,
+        alias_id=alias.id,
+        website_email="firstcontact@sl.lan",
+        reply_email="nobody@nowhere.net",
+        commit=True,
+    )
+    EmailLog.create(
+        user_id=user.id,
+        contact_id=contact.id,
+        alias_id=contact.alias_id,
+        refused_email_id=None,
+        bounced=False,
+        commit=True,
+    )
+
     Alias.create(
         user_id=user.id,
         email="second@ab.cd",

app/handler/unsubscribe_handler.py

@@ -4,8 +4,9 @@ from typing import Optional
 
 from aiosmtpd.smtp import Envelope
 
-from app import config
 from app import alias_utils
+from app import config
+from app.contact_utils import contact_toggle_block
 from app.db import Session
 from app.email import headers, status
 from app.email_utils import (
@@ -143,7 +144,8 @@ class UnsubscribeHandler:
         ):
             return status.E509
         alias = contact.alias
-        contact.block_forward = True
+        if contact.block_forward is False:
+            contact_toggle_block(contact)
         Session.commit()
         unblock_contact_url = (
             config.URL

tests/test_contact_utils.py

@@ -3,12 +3,14 @@ from typing import Optional
 import pytest
 
 from app import config
-from app.contact_utils import create_contact, ContactCreateError
+from app.alias_audit_log_utils import AliasAuditLogAction
+from app.contact_utils import create_contact, ContactCreateError, contact_toggle_block
 from app.db import Session
 from app.models import (
     Alias,
     Contact,
     User,
+    AliasAuditLog,
 )
 from tests.utils import create_new_user, random_email, random_token
 
@@ -194,3 +196,40 @@ def test_update_mail_from_for_existing():
     assert not contact_result.created
     assert contact_result.contact is not None
     assert contact_result.contact.mail_from == mail_from
+
+
+def test_toggle_contact_block():
+    user = create_new_user()
+    alias = Alias.create_new_random(user)
+    Session.commit()
+    email = random_email()
+    contact = create_contact(email, alias).contact
+    last_log_id = (
+        AliasAuditLog.filter_by(alias_id=alias.id)
+        .order_by(AliasAuditLog.id.desc())
+        .first()
+        .id
+    )
+    assert contact is not None
+    assert not contact.block_forward
+    # First toggle
+    contact_toggle_block(contact)
+    audit_log = (
+        AliasAuditLog.filter_by(alias_id=alias.id)
+        .order_by(AliasAuditLog.id.desc())
+        .first()
+    )
+    assert audit_log.action == AliasAuditLogAction.UpdateContact.value
+    assert audit_log.id > last_log_id
+    assert contact.block_forward
+    last_log_id = audit_log.id
+    # Second toggle
+    contact_toggle_block(contact)
+    audit_log = (
+        AliasAuditLog.filter_by(alias_id=alias.id)
+        .order_by(AliasAuditLog.id.desc())
+        .first()
+    )
+    assert audit_log.action == AliasAuditLogAction.UpdateContact.value
+    assert audit_log.id > last_log_id
+    assert not contact.block_forward