feat(dashboard): drop token auth, trust oauth2-proxy X-Forwarded-User

tests/pilot/test_dashboard.py

@@ -1,8 +1,9 @@
 """Tests for pilot/dashboard.py — stdlib HTTP server rendering dashboard HTML.
 
 We spin up the server on an ephemeral port in setUp, drive it with
-http.client, and tear it down in tearDown. Tests are split between
-no-auth (default — env unset) and auth-on (PRAGENT_DASHBOARD_TOKEN set).
+http.client, and tear it down in tearDown. Auth is now performed by
+oauth2-proxy: the dashboard trusts `X-Forwarded-User` set by the proxy
+and returns 401 (with a Basic challenge) when the header is missing.
 
 The dashboard reads `PRAGENT_FEEDBACK_DB` and renders views via
 `dashboard_data`. We seed an in-memory SQLite at `tmp_path` for each
@@ -69,15 +70,72 @@ def _post(port: int, path: str, body: bytes, headers: dict | None = None) -> tup
     return r.status, h, body_b
 
 
-class TestDashboardNoAuth(unittest.TestCase):
-    """PRAGENT_DASHBOARD_TOKEN unset → no cookie check."""
+class TestDashboardAuth(unittest.TestCase):
+    """Auth gate: require X-Forwarded-User (set by oauth2-proxy).
+
+    When the header is missing every non-static route returns 401 with a
+    Basic challenge, which lets oauth2-proxy redirect the browser to
+    Logto. Static is exempt so the unauthenticated probe traffic doesn't
+    loop the proxy through the auth flow.
+    """
 
     def setUp(self):
         self.tmp = tempfile.TemporaryDirectory()
         self.db = f"{self.tmp.name}/f.db"
         os.environ["PRAGENT_FEEDBACK_DB"] = self.db
-        os.environ["PRAGENT_DASHBOARD_TOKEN"] = ""
         os.environ["DASHBOARD_PORT"] = str(0)  # we override below
+
+        self.port = _free_port()
+        from http.server import ThreadingHTTPServer
+        self.srv = ThreadingHTTPServer(("127.0.0.1", self.port), dash.Handler)
+        self.thread = threading.Thread(target=self.srv.serve_forever, daemon=True)
+        self.thread.start()
+
+    def tearDown(self):
+        self.srv.shutdown()
+        self.srv.server_close()
+        self.thread.join(timeout=2)
+        for k in ("PRAGENT_FEEDBACK_DB", "DASHBOARD_PORT"):
+            os.environ.pop(k, None)
+        self.tmp.cleanup()
+
+    def test_anonymous_overview_returns_401_with_basic_challenge(self):
+        status, h, body = _get(self.port, "/")
+        self.assertEqual(status, 401)
+        self.assertEqual(h.get("WWW-Authenticate"), 'Basic realm="pragent-dashboard"')
+        self.assertEqual(body, b"unauthorized\n")
+
+    def test_anonymous_repo_returns_401(self):
+        status, _h, _body = _get(self.port, "/r/alpha/one")
+        self.assertEqual(status, 401)
+
+    def test_anonymous_post_returns_401(self):
+        status, _h, _body = _post(self.port, "/r/alpha/one/edit", b"x=1")
+        self.assertEqual(status, 401)
+
+    def test_authenticated_overview_succeeds(self):
+        status, h, body = _get(self.port, "/", headers={"X-Forwarded-User": "marcos@example.com"})
+        self.assertEqual(status, 200)
+        self.assertIn(b"Overview", body)
+
+    def test_static_does_not_require_auth(self):
+        status, h, body = _get(self.port, "/static/style.css")
+        self.assertEqual(status, 200)
+        self.assertIn("text/css", h.get("Content-Type", ""))
+        self.assertGreater(len(body), 50)
+
+    def test_empty_x_forwarded_user_treated_as_anonymous(self):
+        status, _h, _body = _get(self.port, "/", headers={"X-Forwarded-User": "   "})
+        self.assertEqual(status, 401)
+
+
+class TestDashboardRender(unittest.TestCase):
+    """Render-only tests — X-Forwarded-User set, real seeded data."""
+
+    def setUp(self):
+        self.tmp = tempfile.TemporaryDirectory()
+        self.db = f"{self.tmp.name}/f.db"
+        os.environ["PRAGENT_FEEDBACK_DB"] = self.db
         # Seed: 2 repos, a couple of reviews + findings each.
         conn = feedback.init(self.db)
         for repo, n_prs in (("alpha/one", 2), ("beta/two", 1)):
@@ -99,22 +157,21 @@ class TestDashboardNoAuth(unittest.TestCase):
         conn.close()
 
         self.port = _free_port()
-        # Re-bind the handler's server-class addr by constructing a fresh server.
         from http.server import ThreadingHTTPServer
         self.srv = ThreadingHTTPServer(("127.0.0.1", self.port), dash.Handler)
         self.thread = threading.Thread(target=self.srv.serve_forever, daemon=True)
         self.thread.start()
+        self.auth_hdr = {"X-Forwarded-User": "marcos@example.com"}
 
     def tearDown(self):
         self.srv.shutdown()
         self.srv.server_close()
         self.thread.join(timeout=2)
-        for k in ("PRAGENT_FEEDBACK_DB", "PRAGENT_DASHBOARD_TOKEN", "DASHBOARD_PORT"):
-            os.environ.pop(k, None)
+        os.environ.pop("PRAGENT_FEEDBACK_DB", None)
         self.tmp.cleanup()
 
     def test_overview_200_contains_repo_names(self):
-        status, _h, body = _get(self.port, "/")
+        status, _h, body = _get(self.port, "/", headers=self.auth_hdr)
         self.assertEqual(status, 200)
         text = body.decode()
         self.assertIn("Overview", text)
@@ -122,7 +179,7 @@ class TestDashboardNoAuth(unittest.TestCase):
         self.assertIn("beta/two", text)
 
     def test_repo_page_200(self):
-        status, _h, body = _get(self.port, "/r/alpha/one")
+        status, _h, body = _get(self.port, "/r/alpha/one", headers=self.auth_hdr)
         self.assertEqual(status, 200)
         text = body.decode()
         self.assertIn("alpha/one", text)
@@ -130,102 +187,17 @@ class TestDashboardNoAuth(unittest.TestCase):
         self.assertIn("src/file_0.py", text)
 
     def test_pr_page_200(self):
-        status, _h, body = _get(self.port, "/r/alpha/one/1")
+        status, _h, body = _get(self.port, "/r/alpha/one/1", headers=self.auth_hdr)
         self.assertEqual(status, 200)
         text = body.decode()
         self.assertIn("alpha/one", text)
         self.assertIn("#1", text)
         self.assertIn("src/file_0.py", text)
 
-    def test_static_css_200(self):
-        status, h, body = _get(self.port, "/static/style.css")
-        self.assertEqual(status, 200)
-        self.assertIn("text/css", h.get("Content-Type", ""))
-        self.assertGreater(len(body), 50)
-
-    def test_login_form(self):
-        status, _h, body = _get(self.port, "/login")
-        self.assertEqual(status, 200)
-        self.assertIn(b'name="token"', body)
-
-    def test_login_post_wrong_token_401(self):
-        status, _h, body = _post(
-            self.port, "/login", b"token=wrong",
-        )
-        self.assertEqual(status, 401)
-
     def test_unknown_route_404(self):
-        status, _h, _body = _get(self.port, "/no/such/route")
+        status, _h, _body = _get(self.port, "/no/such/route", headers=self.auth_hdr)
         self.assertEqual(status, 404)
 
 
-class TestDashboardWithAuth(unittest.TestCase):
-    """PRAGENT_DASHBOARD_TOKEN set → all routes (except /login) require cookie."""
-
-    def setUp(self):
-        self.tmp = tempfile.TemporaryDirectory()
-        self.db = f"{self.tmp.name}/f.db"
-        os.environ["PRAGENT_FEEDBACK_DB"] = self.db
-        os.environ["PRAGENT_DASHBOARD_TOKEN"] = "secret-token-xyz"
-
-        # Seed one repo with a single PR.
-        conn = feedback.init(self.db)
-        rid = feedback.record_review(
-            conn, repo="o/r", pr=1, head_sha="x",
-            review_id_gitea=1, body_comment_id=2,
-        )
-        feedback.record_inline_finding(
-            conn, review_id=rid, repo="o/r", pr=1,
-            path="a.py", line=1, severity="low", problem="x",
-        )
-        conn.close()
-
-        self.port = _free_port()
-        from http.server import ThreadingHTTPServer
-        self.srv = ThreadingHTTPServer(("127.0.0.1", self.port), dash.Handler)
-        self.thread = threading.Thread(target=self.srv.serve_forever, daemon=True)
-        self.thread.start()
-
-    def tearDown(self):
-        self.srv.shutdown()
-        self.srv.server_close()
-        self.thread.join(timeout=2)
-        for k in ("PRAGENT_FEEDBACK_DB", "PRAGENT_DASHBOARD_TOKEN"):
-            os.environ.pop(k, None)
-        self.tmp.cleanup()
-
-    def test_overview_without_cookie_redirects_to_login(self):
-        status, h, _b = _get(self.port, "/")
-        self.assertEqual(status, 302)
-        self.assertIn("/login", h.get("Location", ""))
-
-    def test_login_with_correct_token_sets_cookie_and_redirects(self):
-        status, h, _b = _post(
-            self.port, "/login", b"token=secret-token-xyz",
-        )
-        self.assertEqual(status, 302)
-        self.assertIn("/", h.get("Location", ""))
-        sc = h.get("Set-Cookie", "")
-        self.assertIn("pragent_dash=secret-token-xyz", sc)
-        self.assertIn("HttpOnly", sc)
-        self.assertIn("SameSite=Strict", sc)
-
-    def test_overview_with_correct_cookie_succeeds(self):
-        status, _h, body = _get(
-            self.port, "/",
-            headers={"Cookie": "pragent_dash=secret-token-xyz"},
-        )
-        self.assertEqual(status, 200)
-        self.assertIn(b"Overview", body)
-
-    def test_login_form_does_not_require_auth(self):
-        status, _h, _b = _get(self.port, "/login")
-        self.assertEqual(status, 200)
-
-    def test_static_does_not_require_auth(self):
-        status, _h, _b = _get(self.port, "/static/style.css")
-        self.assertEqual(status, 200)
-
-
 if __name__ == "__main__":
-    unittest.main()
+    unittest.main()

tests/pilot/test_dashboard_edit.py

@@ -2,7 +2,7 @@
 
 We mock the Gitea HTTP layer (urllib.request.urlopen) so the test never
 touches the network. The dashboard handler is responsible for:
-  * auth + CSRF
+  * auth (X-Forwarded-User set by oauth2-proxy) + CSRF
   * read .pr-review.json via GET (404 → start from {})
   * validate model against cost_model.PRICES
   * PUT the updated file back, with sha + base64 content
@@ -35,12 +35,12 @@ def _free_port() -> int:
     return port
 
 
-def _post(port: int, path: str, body: bytes, *, cookie: str | None = None) -> tuple[int, dict, bytes]:
+def _post(port: int, path: str, body: bytes, *, headers: dict | None = None) -> tuple[int, dict, bytes]:
     conn = http.client.HTTPConnection("127.0.0.1", port, timeout=5)
-    headers = {"Content-Type": "application/x-www-form-urlencoded"}
-    if cookie:
-        headers["Cookie"] = cookie
-    conn.request("POST", path, body=body, headers=headers)
+    hdrs = {"Content-Type": "application/x-www-form-urlencoded"}
+    if headers:
+        hdrs.update(headers)
+    conn.request("POST", path, body=body, headers=hdrs)
     r = conn.getresponse()
     body_b = r.read()
     h = dict(r.getheaders())
@@ -68,7 +68,6 @@ class TestDashboardEdit(unittest.TestCase):
         self.tmp = tempfile.TemporaryDirectory()
         self.db = f"{self.tmp.name}/f.db"
         os.environ["PRAGENT_FEEDBACK_DB"] = self.db
-        os.environ["PRAGENT_DASHBOARD_TOKEN"] = "secret-edit-token"
         os.environ["PRAGENT_BOT_TOKEN"] = "bot-token"
         # Seed a row so the repo page is meaningful.
         conn = feedback.init(self.db)
@@ -82,7 +81,10 @@ class TestDashboardEdit(unittest.TestCase):
         self.srv = ThreadingHTTPServer(("127.0.0.1", self.port), dash.Handler)
         self.thread = threading.Thread(target=self.srv.serve_forever, daemon=True)
         self.thread.start()
-        self.cookie = "pragent_dash=secret-edit-token"
+        # Pull the per-process CSRF secret from the rendered repo page — the
+        # edit form embeds the same token as a hidden input.
+        self.csrf = dash._CSRF_SECRET
+        self.auth_hdr = {"X-Forwarded-User": "marcos@example.com"}
 
         # Records of HTTP calls made by the handler.
         self.calls: list[tuple[str, str, dict | None, bytes | None]] = []
@@ -91,7 +93,7 @@ class TestDashboardEdit(unittest.TestCase):
         self.srv.shutdown()
         self.srv.server_close()
         self.thread.join(timeout=2)
-        for k in ("PRAGENT_FEEDBACK_DB", "PRAGENT_DASHBOARD_TOKEN", "PRAGENT_BOT_TOKEN"):
+        for k in ("PRAGENT_FEEDBACK_DB", "PRAGENT_BOT_TOKEN"):
             os.environ.pop(k, None)
         self.tmp.cleanup()
 
@@ -114,12 +116,12 @@ class TestDashboardEdit(unittest.TestCase):
 
     def test_edit_updates_static_message_and_model(self):
         form = (
-            b"_csrf=secret-edit-token"
-            b"&static_message=Hello%20world"
-            b"&model=claude-sonnet-5"
-        )
+            f"_csrf={self.csrf}"
+            f"&static_message=Hello%20world"
+            f"&model=claude-sonnet-5"
+        ).encode()
         with patch.object(dash.urllib.request, "urlopen", side_effect=self._urlopen):
-            status, h, _b = _post(self.port, "/r/o/r/edit", form, cookie=self.cookie)
+            status, h, _b = _post(self.port, "/r/o/r/edit", form, headers=self.auth_hdr)
         self.assertEqual(status, 302)
         self.assertEqual(h.get("Location"), "/r/o/r")
 
@@ -139,24 +141,24 @@ class TestDashboardEdit(unittest.TestCase):
     def test_edit_strips_static_message_to_400(self):
         long_msg = "x" * 600
         form = (
-            f"_csrf=secret-edit-token"
+            f"_csrf={self.csrf}"
             f"&static_message={long_msg}"
             f"&model=claude-haiku-4-5"
         ).encode()
         with patch.object(dash.urllib.request, "urlopen", side_effect=self._urlopen):
-            _post(self.port, "/r/o/r/edit", form, cookie=self.cookie)
+            _post(self.port, "/r/o/r/edit", form, headers=self.auth_hdr)
         put = next(c for c in self.calls if c[0] == "PUT")
         cfg = json.loads(base64.b64decode(json.loads(put[3])["content"]))
         self.assertEqual(len(cfg["static_message"]), 400)
 
     def test_edit_rejects_unknown_model_with_flash(self):
         form = (
-            b"_csrf=secret-edit-token"
-            b"&static_message=hi"
-            b"&model=does-not-exist"
-        )
+            f"_csrf={self.csrf}"
+            f"&static_message=hi"
+            f"&model=does-not-exist"
+        ).encode()
         with patch.object(dash.urllib.request, "urlopen", side_effect=self._urlopen):
-            status, h, _b = _post(self.port, "/r/o/r/edit", form, cookie=self.cookie)
+            status, h, _b = _post(self.port, "/r/o/r/edit", form, headers=self.auth_hdr)
         self.assertEqual(status, 302)
         self.assertIn("flash=", h.get("Location", ""))
         # No PUT should have been issued.
@@ -164,16 +166,18 @@ class TestDashboardEdit(unittest.TestCase):
         self.assertEqual(put_calls, [])
 
     def test_edit_requires_auth(self):
-        form = b"_csrf=secret-edit-token&static_message=x&model=claude-haiku-4-5"
+        form = f"_csrf={self.csrf}&static_message=x&model=claude-haiku-4-5".encode()
         with patch.object(dash.urllib.request, "urlopen", side_effect=self._urlopen):
             status, h, _b = _post(self.port, "/r/o/r/edit", form)
-        self.assertEqual(status, 302)
-        self.assertEqual(h.get("Location"), "/login")
+        self.assertEqual(status, 401)
+        self.assertEqual(h.get("WWW-Authenticate"), 'Basic realm="pragent-dashboard"')
+        # No Gitea calls at all — auth gate fires first.
+        self.assertEqual(self.calls, [])
 
     def test_edit_csrf_mismatch_redirects_without_save(self):
-        form = b"_csrf=wrong&static_message=x&model=claude-haiku-4-5"
+        form = f"_csrf=wrong&static_message=x&model=claude-haiku-4-5".encode()
         with patch.object(dash.urllib.request, "urlopen", side_effect=self._urlopen):
-            status, h, _b = _post(self.port, "/r/o/r/edit", form, cookie=self.cookie)
+            status, h, _b = _post(self.port, "/r/o/r/edit", form, headers=self.auth_hdr)
         self.assertEqual(status, 302)
         self.assertEqual(h.get("Location"), "/r/o/r")
         put_calls = [c for c in self.calls if c[0] == "PUT"]
@@ -192,9 +196,9 @@ class TestDashboardEdit(unittest.TestCase):
                 return _FakeResp(201, b"{}")
             return _FakeResp(404, b"")
 
-        form = b"_csrf=secret-edit-token&static_message=hi&model=claude-haiku-4-5"
+        form = f"_csrf={self.csrf}&static_message=hi&model=claude-haiku-4-5".encode()
         with patch.object(dash.urllib.request, "urlopen", side_effect=_route):
-            status, h, _b = _post(self.port, "/r/o/r/edit", form, cookie=self.cookie)
+            status, h, _b = _post(self.port, "/r/o/r/edit", form, headers=self.auth_hdr)
         self.assertEqual(status, 302)
         put = next(c for c in self.calls if c[0] == "PUT")
         payload = json.loads(put[3])
@@ -202,4 +206,4 @@ class TestDashboardEdit(unittest.TestCase):
 
 
 if __name__ == "__main__":
-    unittest.main()
+    unittest.main()

tests/pilot/test_dashboard_select.py

@@ -38,7 +38,6 @@ class TestRepoEditSelect(unittest.TestCase):
         self.tmp = tempfile.TemporaryDirectory()
         self.db = f"{self.tmp.name}/f.db"
         os.environ["PRAGENT_FEEDBACK_DB"] = self.db
-        os.environ["PRAGENT_DASHBOARD_TOKEN"] = ""
         os.environ["PRAGENT_BOT_TOKEN"] = ""
         conn = feedback.init(self.db)
         feedback.record_review(conn, repo="o/r", pr=1, head_sha="x")
@@ -49,17 +48,18 @@ class TestRepoEditSelect(unittest.TestCase):
         self.srv = ThreadingHTTPServer(("127.0.0.1", self.port), dash.Handler)
         self.thread = threading.Thread(target=self.srv.serve_forever, daemon=True)
         self.thread.start()
+        self.auth_hdr = {"X-Forwarded-User": "marcos@example.com"}
 
     def tearDown(self):
         self.srv.shutdown()
         self.srv.server_close()
         self.thread.join(timeout=2)
-        for k in ("PRAGENT_FEEDBACK_DB", "PRAGENT_DASHBOARD_TOKEN", "PRAGENT_BOT_TOKEN"):
+        for k in ("PRAGENT_FEEDBACK_DB", "PRAGENT_BOT_TOKEN"):
             os.environ.pop(k, None)
         self.tmp.cleanup()
 
     def test_repo_page_renders_select_with_one_option_per_price(self):
-        status, _h, body = _get(self.port, "/r/o/r")
+        status, _h, body = _get(self.port, "/r/o/r", headers=self.auth_hdr)
         self.assertEqual(status, 200)
         text = body.decode()
         self.assertIn('<select id="model" name="model">', text)