fix: the contact form's messages were readable by anyone

`GET /api/contact/submissions` had no authentication. `require_admin` was
imported inside the function body and never used as a dependency, so the import
read as protection and was none: anyone who guessed the path could read every
sender's name, email address and message. `PUT .../read` was open the same way.

Both now depend on `require_admin`, with a test that a learner gets 403 and an
administrator gets the list. A row with a null timestamp no longer takes the
whole listing down with it — which is the only reason the hole showed up as a
500 rather than as data.

Also: the tutor's site switch lives in Redis, which the tests share with the
running site, so turning the tutor off in the interface turned a test red. The
test now sets the flag it depends on and puts it back.

And the tutor button is hidden until the server says it is allowed, rather than
shown and then withdrawn — on a site with it switched off that flicker reads as
a bug rather than a policy.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TqXevQJhxFrM7jJg82cgZN

backend/app/routers/contact.py

@@ -10,7 +10,9 @@ from sqlalchemy.orm import Session
 
 from app.database import get_db
 from app.config import settings
+from app.models.user import User
 from app.services import captcha
+from app.utils.auth import require_admin
 
 router = APIRouter()
 log = logging.getLogger(__name__)
@@ -75,9 +77,15 @@ async def _email_admin(name: str, email: str, type_: str, message: str):
 @router.get("/submissions")
 def list_submissions(
     db: Session = Depends(get_db),
+    current_user: User = Depends(require_admin),
 ):
-    """Admin: list all contact submissions."""
+    """Every message the contact form has taken. Administrators only.
-    from app.utils.auth import require_admin
+
+    It was open to the internet: `require_admin` was imported inside the
+    function body and never used as a dependency, so the import read as
+    protection and was none. Anyone who guessed the path could read every
+    sender's name, address and message.
+    """
     rows = db.execute(text(
         "SELECT id, name, email, type, message, read, created_at FROM contact_submissions ORDER BY created_at DESC"
     )).fetchall()
@@ -85,7 +93,9 @@ def list_submissions(
         {
             "id": r.id, "name": r.name, "email": r.email,
             "type": r.type, "message": r.message, "read": r.read,
-            "created_at": r.created_at.isoformat(),
+            # Some early rows have no timestamp, and a listing that raises on
+            # one of them shows none of the others.
+            "created_at": r.created_at.isoformat() if r.created_at else None,
         }
         for r in rows
     ]
@@ -95,7 +105,9 @@ def list_submissions(
 def mark_read(
     submission_id: int,
     db: Session = Depends(get_db),
+    current_user: User = Depends(require_admin),
 ):
+    """Mark one as read. Administrators only, for the same reason as the list."""
     result = db.execute(text("UPDATE contact_submissions SET read=1 WHERE id=:id"), {"id": submission_id})
     db.commit()
     if result.rowcount == 0:

backend/tests/test_contact_privacy.py

@@ -0,0 +1,78 @@
+"""Who may read the contact form's messages.
+
+Run: DATABASE_URL=sqlite:// PYTHONPATH=backend python -m unittest discover -s backend/tests
+"""
+import os
+os.environ.setdefault("DATABASE_URL", "sqlite://")
+
+import unittest
+
+from fastapi import FastAPI
+from fastapi.testclient import TestClient
+from sqlalchemy import create_engine, text
+from sqlalchemy.orm import Session
+from sqlalchemy.pool import StaticPool
+
+from app.database import get_db
+from app.models.user import User
+from app.routers import contact
+from app.utils.auth import get_current_user
+
+
+class ContactPrivacyTests(unittest.TestCase):
+    """The listing was open to the internet.
+
+    `require_admin` was imported inside the function body and never used as a
+    dependency, so the import read as protection and was none: anyone who
+    guessed the path could read every sender's name, address and message.
+    """
+
+    def setUp(self):
+        self.engine = create_engine("sqlite://", connect_args={"check_same_thread": False},
+                                    poolclass=StaticPool)
+        # Only the table this router touches, plus users. `create_all` pulls in
+        # every model in the metadata, and one of them carries a raw-SQL table
+        # SQLite cannot build.
+        User.__table__.create(self.engine, checkfirst=True)
+        self.db = Session(self.engine)
+        self.db.execute(text(
+            "CREATE TABLE IF NOT EXISTS contact_submissions ("
+            "id INTEGER PRIMARY KEY, name TEXT, email TEXT, type TEXT, message TEXT,"
+            " read INTEGER DEFAULT 0, created_at TIMESTAMP)"))
+        self.db.execute(text(
+            "INSERT INTO contact_submissions (id, name, email, type, message, read, created_at)"
+            " VALUES (1, 'A Sender', 'sender@example.test', 'question', 'Private message', 0, NULL)"))
+        self.db.commit()
+
+        self.admin = User(id=1, email="admin@example.test", name="Admin", role="admin")
+        self.learner = User(id=2, email="learner@example.test", name="Learner", role="user")
+        self.db.add_all([self.admin, self.learner])
+        self.db.commit()
+
+        self.user = self.learner
+        app = FastAPI()
+        app.include_router(contact.router, prefix="/contact")
+        app.dependency_overrides[get_db] = lambda: self.db
+        app.dependency_overrides[get_current_user] = lambda: self.user
+        self.client = TestClient(app)
+
+    def tearDown(self):
+        self.db.close()
+        self.engine.dispose()
+
+    def test_a_learner_may_not_read_or_touch_the_messages(self):
+        self.assertEqual(self.client.get("/contact/submissions").status_code, 403)
+        self.assertEqual(self.client.put("/contact/submissions/1/read").status_code, 403)
+
+    def test_an_administrator_may(self):
+        self.user = self.admin
+        listing = self.client.get("/contact/submissions")
+        self.assertEqual(listing.status_code, 200, listing.text)
+        self.assertEqual(listing.json()[0]["email"], "sender@example.test")
+        # A row with no timestamp does not take the whole listing down with it.
+        self.assertIsNone(listing.json()[0]["created_at"])
+        self.assertEqual(self.client.put("/contact/submissions/1/read").status_code, 200)
+
+
+if __name__ == "__main__":
+    unittest.main()

backend/tests/test_related_privacy.py

@@ -97,6 +97,14 @@ class PrivacyTests(unittest.TestCase):
         self.assertEqual(self.chat(4).status_code, 200)
 
     def test_tutor_course_attempt_modes_pool_and_review(self):
+        # The tutor's site switch lives in Redis, which these tests share with
+        # the running site — so an administrator turning the tutor off in the
+        # interface used to turn this test red. What is under test here is who
+        # may reach the tutor when it is *on*, so it says so.
+        from app.services import site_settings
+        site_settings.set_flag("tutor_in_quiz", True)
+        self.addCleanup(site_settings.set_flag, "tutor_in_quiz", False)
+
         self.enroll()
         self.assertEqual(self.chat(5).status_code, 403)
         for mode in ('exam', None):

frontend/src/pages/QuizPage.jsx

@@ -570,7 +570,10 @@ export default function QuizPage() {
   // Suspending is not abandoning: it ends on the session's own analysis, where
   // what has been answered so far is scored and the Resume button sits. A
   // course quiz still returns to the course it belongs to.
-  const [tutorAllowed, setTutorAllowed] = useState(true)
+  //: Withheld until the server says otherwise. Defaulting to allowed made the
+  //: tutor's button appear for a moment on every session and then vanish on a
+  //: site that has it switched off, which reads as a bug rather than a policy.
+  const [tutorAllowed, setTutorAllowed] = useState(false)
 
   useEffect(() => {
     let live = true