From f55ab58d0c082d13db17237e2a20ae7d666852b9 Mon Sep 17 00:00:00 2001
From: Carlos Quintana <74399022+cquintana92@users.noreply.github.com>
Date: Wed, 23 Oct 2024 10:24:15 +0200
Subject: [PATCH] fix: prevent mailbox disclosure (#2284)

---
 app/dashboard/views/mailbox.py |  6 ++++++
 app/mailbox_utils.py           | 10 +++++-----
 tests/test_mailbox_utils.py    |  9 +++++++++
 3 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/app/dashboard/views/mailbox.py b/app/dashboard/views/mailbox.py
index e7124716..ee436c52 100644
--- a/app/dashboard/views/mailbox.py
+++ b/app/dashboard/views/mailbox.py
@@ -121,10 +121,16 @@ def mailbox_route():
 @login_required
 def mailbox_verify():
     mailbox_id = request.args.get("mailbox_id")
+    if not mailbox_id:
+        LOG.i("Missing mailbox_id")
+        flash("You followed an invalid link", "error")
+        return redirect(url_for("dashboard.mailbox_route"))
+
     code = request.args.get("code")
     if not code:
         # Old way
         return verify_with_signed_secret(mailbox_id)
+
     try:
         mailbox = mailbox_utils.verify_mailbox_code(current_user, mailbox_id, code)
     except mailbox_utils.MailboxError as e:
diff --git a/app/mailbox_utils.py b/app/mailbox_utils.py
index 42182671..19499567 100644
--- a/app/mailbox_utils.py
+++ b/app/mailbox_utils.py
@@ -171,17 +171,17 @@ def verify_mailbox_code(user: User, mailbox_id: int, code: str) -> Mailbox:
             f"User {user} failed to verify mailbox {mailbox_id} because it does not exist"
         )
         raise MailboxError("Invalid mailbox")
+    if mailbox.user_id != user.id:
+        LOG.i(
+            f"User {user} failed to verify mailbox {mailbox_id} because it's owned by another user"
+        )
+        raise MailboxError("Invalid mailbox")
     if mailbox.verified:
         LOG.i(
             f"User {user} failed to verify mailbox {mailbox_id} because it's already verified"
         )
         clear_activation_codes_for_mailbox(mailbox)
         return mailbox
-    if mailbox.user_id != user.id:
-        LOG.i(
-            f"User {user} failed to verify mailbox {mailbox_id} because it's owned by another user"
-        )
-        raise MailboxError("Invalid mailbox")
 
     activation = (
         MailboxActivation.filter(MailboxActivation.mailbox_id == mailbox_id)
diff --git a/tests/test_mailbox_utils.py b/tests/test_mailbox_utils.py
index 51aabb53..030bf130 100644
--- a/tests/test_mailbox_utils.py
+++ b/tests/test_mailbox_utils.py
@@ -286,6 +286,15 @@ def test_verify_other_users_mailbox():
         mailbox_utils.verify_mailbox_code(user, mailbox.id, "9999999")
 
 
+def test_verify_other_users_already_verified_mailbox():
+    other = create_new_user()
+    mailbox = Mailbox.create(
+        user_id=other.id, email=random_email(), verified=True, commit=True
+    )
+    with pytest.raises(mailbox_utils.MailboxError):
+        mailbox_utils.verify_mailbox_code(user, mailbox.id, "9999999")
+
+
 @mail_sender.store_emails_test_decorator
 def test_verify_fail():
     output = mailbox_utils.create_mailbox(user, random_email())