From 6e1189e31977c09cd11afa55119f58a7b95b38c9 Mon Sep 17 00:00:00 2001
From: hermes <ridgway_infrastructure@gmail.com>
Date: Mon, 9 Nov 2026 10:31:03 +1000
Subject: [PATCH] fix: log a failed password migration instead of swallowing it

The round-2 review noted that `_upgrade_password` catches `PyMongoError`
without logging it. A failed upgrade is harmless to the login itself - the
user still gets in - but it leaves the password stored in plain text, and
that is exactly the kind of thing an operator needs to know about. It is now
logged as a warning with the username and the traceback.

A test covers the path: the migration write is forced to fail, the login
still succeeds, and the warning is emitted.
---
 src/flask/mongo/repository.py | 14 ++++++++++++--
 tests/test_app.py             | 23 +++++++++++++++++++++++
 2 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/src/flask/mongo/repository.py b/src/flask/mongo/repository.py
index baa7bbd..7cdd880 100644
--- a/src/flask/mongo/repository.py
+++ b/src/flask/mongo/repository.py
@@ -17,6 +17,7 @@ Two collections are used:
 
 from __future__ import annotations
 
+import logging
 import os
 from typing import Any
 
@@ -28,6 +29,8 @@ from brewing import Run
 
 __all__ = ["BeerRepository", "get_repository"]
 
+logger = logging.getLogger(__name__)
+
 
 class BeerRepository:
     """Thin, testable wrapper around the two Mongo collections."""
@@ -89,8 +92,15 @@ class BeerRepository:
                 }},
             )
         except PyMongoError:
-            # A failed upgrade must not stop the login succeeding.
-            pass
+            # A failed upgrade must not stop the login succeeding — but the
+            # operator needs to know the password is still stored in plain
+            # text, so log it rather than swallowing the error silently.
+            logger.warning(
+                "Could not upgrade the password for user %r to a hashed value; "
+                "it is still stored in plain text",
+                doc.get("username"),
+                exc_info=True,
+            )
 
     # -- readings ----------------------------------------------------------- #
 
diff --git a/tests/test_app.py b/tests/test_app.py
index dd73020..cb5746f 100644
--- a/tests/test_app.py
+++ b/tests/test_app.py
@@ -215,6 +215,29 @@ def test_legacy_plaintext_password_is_accepted_and_migrated(repo):
     assert "password_migrated_at" in doc
 
 
+def test_a_failed_password_migration_still_lets_the_login_succeed(repo, caplog):
+    """If the upgrade write fails, the login must still work — but the
+    failure is logged so the operator knows the password is still plaintext."""
+    import logging
+
+    from pymongo.errors import PyMongoError
+
+    repo.users.insert_one({"username": "beer", "password": "beer_pass"})
+
+    def boom(*args, **kwargs):
+        raise PyMongoError("write failed")
+
+    original = repo.users.update_one
+    repo.users.update_one = boom
+
+    with caplog.at_level(logging.WARNING):
+        assert repo.user_check("beer", "beer_pass") is True
+
+    repo.users.update_one = original
+
+    assert any("plain text" in rec.message for rec in caplog.records)
+
+
 def test_hashed_password_verifies(repo):
     from mongo.repository import hash_password