From c4fcebd9c7bc7782e04210ef0c9962e21d31f223 Mon Sep 17 00:00:00 2001
From: Afonso Coutinho <afonso@omelhorsite.pt>
Date: Wed, 3 Jun 2026 05:23:50 +0100
Subject: [PATCH] fix: disabling auth wipes all users' preferences on next pref
 save (#1764)

---
 routes/prefs_routes.py                     | 13 +++++-
 tests/test_prefs_single_user_no_clobber.py | 53 ++++++++++++++++++++++
 2 files changed, 65 insertions(+), 1 deletion(-)
 create mode 100644 tests/test_prefs_single_user_no_clobber.py

diff --git a/routes/prefs_routes.py b/routes/prefs_routes.py
index 65f56a7..8839d17 100644
--- a/routes/prefs_routes.py
+++ b/routes/prefs_routes.py
@@ -40,7 +40,18 @@ def _save_for_user(user: Optional[str], prefs: dict):
     """Save preferences for a specific user."""
     all_prefs = _load()
     if user is None:
-        # Auth disabled — save flat
+        # Auth disabled. If the store is already multi-user (e.g. auth was
+        # turned off on a deployment that previously ran multi-user), writing
+        # `prefs` flat would overwrite the whole `_users` map and destroy every
+        # other user's preferences. Instead write back into the same (first)
+        # slot _load_for_user(None) reads from, preserving the others.
+        if "_users" in all_prefs:
+            users = all_prefs["_users"]
+            first_key = next(iter(users), None)
+            if first_key is not None:
+                users[first_key] = prefs
+                _save(all_prefs)
+                return
         _save(prefs)
         return
     if "_users" not in all_prefs:
diff --git a/tests/test_prefs_single_user_no_clobber.py b/tests/test_prefs_single_user_no_clobber.py
new file mode 100644
index 0000000..7bd2c61
--- /dev/null
+++ b/tests/test_prefs_single_user_no_clobber.py
@@ -0,0 +1,53 @@
+"""Saving prefs with auth disabled must not wipe a multi-user store.
+
+When auth is disabled get_current_user returns None. _save_for_user(None,...)
+wrote prefs flat, overwriting the entire {"_users": {...}} map and destroying
+every other user's preferences (a realistic ops transition: auth turned off
+on a deployment that previously ran multi-user). It must preserve the other
+users and round-trip the change into the same (first) slot _load_for_user
+reads from.
+"""
+import json
+
+import routes.prefs_routes as pr
+
+
+def test_single_user_save_preserves_other_users(tmp_path, monkeypatch):
+    f = tmp_path / "user_prefs.json"
+    f.write_text(json.dumps({"_users": {
+        "alice": {"theme": "light"},
+        "bob": {"theme": "paper"},
+    }}), encoding="utf-8")
+    monkeypatch.setattr(pr, "PREFS_FILE", str(f))
+
+    # auth disabled: load (first user) -> modify -> save
+    current = pr._load_for_user(None)
+    current["theme"] = "dark"
+    pr._save_for_user(None, current)
+
+    data = json.loads(f.read_text())
+    assert "_users" in data, "multi-user store was clobbered"
+    assert "bob" in data["_users"] and data["_users"]["bob"] == {"theme": "paper"}
+    # the change round-tripped into the first user's slot
+    assert data["_users"]["alice"]["theme"] == "dark"
+
+
+def test_legacy_flat_store_still_saved_flat(tmp_path, monkeypatch):
+    f = tmp_path / "user_prefs.json"
+    f.write_text(json.dumps({"theme": "light"}), encoding="utf-8")
+    monkeypatch.setattr(pr, "PREFS_FILE", str(f))
+
+    pr._save_for_user(None, {"theme": "dark"})
+    data = json.loads(f.read_text())
+    assert data == {"theme": "dark"}
+
+
+def test_named_user_save_unaffected(tmp_path, monkeypatch):
+    f = tmp_path / "user_prefs.json"
+    f.write_text(json.dumps({"_users": {"alice": {"theme": "light"}}}), encoding="utf-8")
+    monkeypatch.setattr(pr, "PREFS_FILE", str(f))
+
+    pr._save_for_user("bob", {"theme": "dark"})
+    data = json.loads(f.read_text())
+    assert data["_users"]["alice"] == {"theme": "light"}
+    assert data["_users"]["bob"] == {"theme": "dark"}