Session - COOKIE_SECURE=false no longer persists the owner key for ten years. services/session.cookie_kwargs() drops max_age in that mode, so the browser holds the key in memory and the session ends with the window. Everything still persists server-side; only the browser link is temporary. base.html shows a warning banner (FR/EN) and an extra paragraph in the onboarding modal, and the README explains the trade-off and the backup-key escape hatch. - Both cookie writers (middleware, POST /session/restore) go through cookie_kwargs() so the policy cannot drift between them. - The CSRF guard on /session/restore compared request.url.scheme against the Origin header. Behind a TLS-terminating proxy uvicorn sees http while the browser sends https, so every legitimate restore was rejected with 403. It now compares hosts only, including X-Forwarded-Host. - /static/*, /favicon.ico and /robots.txt skip the middleware. Each cookieless hit was inserting an Owner row no browser could ever use. Reliability - Malformed printer-form FK fields no longer escape as HTTP 500: a non-numeric client_id/driver_id raised ValueError and an unknown driver_id hit a FOREIGN KEY constraint. Both are now 400/404 HTMX fragments, and the duplicated field checks moved into _validate_fields(). - Package exports stream. build_intunewin() encrypts the inner ZIP in 1 MB chunks against temp files with a streaming HMAC and SHA256, and both endpoints serve the result with FileResponse plus a background cleanup task. A 100 MB driver used to be held in memory three or four times over per concurrent download. The byte layout is unchanged. - FileResponse also escapes the download filename, which was previously interpolated raw into Content-Disposition. - python-multipart >= 0.0.18 (CVE-2024-53981, reachable from /drivers/upload) and Pillow >= 10.3 (CVE-2024-28219, reachable from icon upload). - icons.py reads cfg.ICONS_DIR instead of re-deriving the path from DATA_DIR, matching the .intunewin export. UI - Sidebar/topbar shell, inline SVG icon macros (partials/icons.html), card and data-table components, grouped printer list, and the dedicated /printers/new page replacing partials/printer_form.html. Tests - 194 pass with a bare `pytest tests/`: tests/conftest.py now forces cfg.COOKIE_SECURE = False like the e2e conftest already did, so the Secure cookie is no longer dropped over http://testserver. - New coverage for the malformed-FK guards, the chunk-boundary cases in the encrypt loop (every residue mod _CHUNK plus a multi-megabyte payload), temp-dir cleanup after both exports, and the whole COOKIE_SECURE matrix. - test_printer_edit.py located the Edit button by its translated label, so it only passed on English-locale machines. It now targets the showModal() hook, which also cuts the e2e run from 84s to 15s. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
271 lines
9.0 KiB
Python
271 lines
9.0 KiB
Python
"""Tests for per-owner cookie-scoped session — first visit, isolation, restore."""
|
|
from fastapi.testclient import TestClient
|
|
|
|
from imptune.services.session import COOKIE_NAME
|
|
|
|
|
|
def test_first_visit_sets_cookie_and_new_owner_flag(client):
|
|
resp = client.get("/")
|
|
assert resp.status_code == 200
|
|
assert COOKIE_NAME in client.cookies
|
|
assert "session-choice-modal" in resp.text
|
|
|
|
# Second visit — cookie already present, modal must not reappear
|
|
resp2 = client.get("/")
|
|
assert "session-choice-modal" not in resp2.text
|
|
|
|
|
|
def test_printers_are_isolated_between_owners(tmp_data_dir):
|
|
from imptune.main import app
|
|
|
|
with TestClient(app) as client_a, TestClient(app) as client_b:
|
|
client_a.post(
|
|
"/printers",
|
|
data={"name": "Owner A Printer", "ip_address": "10.0.0.1", "port_name": "IP_A"},
|
|
follow_redirects=False,
|
|
)
|
|
|
|
page_a = client_a.get("/printers")
|
|
page_b = client_b.get("/printers")
|
|
|
|
assert "Owner A Printer" in page_a.text
|
|
assert "Owner A Printer" not in page_b.text
|
|
|
|
|
|
def test_owner_b_cannot_reach_owner_a_printer_by_id(tmp_data_dir):
|
|
from imptune.main import app
|
|
|
|
with TestClient(app) as client_a, TestClient(app) as client_b:
|
|
client_a.post(
|
|
"/printers",
|
|
data={"name": "Private Printer", "ip_address": "10.0.0.2", "port_name": "IP_B"},
|
|
follow_redirects=False,
|
|
)
|
|
from imptune.db.models import Printer
|
|
|
|
printer_id = Printer.get(Printer.name == "Private Printer").id
|
|
|
|
assert client_b.get(f"/printers/{printer_id}").status_code == 404
|
|
assert client_b.patch(f"/printers/{printer_id}", data={"name": "Hijacked"}).status_code == 404
|
|
assert client_b.delete(f"/printers/{printer_id}").status_code == 404
|
|
assert client_b.get(f"/printers/{printer_id}/scripts/install").status_code == 404
|
|
assert client_b.get(f"/printers/{printer_id}/packages/ninja").status_code == 404
|
|
|
|
|
|
def test_download_key_marks_permanent_and_returns_key(client):
|
|
from imptune.db.models import Owner
|
|
|
|
client.get("/")
|
|
resp = client.get("/session/key/download")
|
|
assert resp.status_code == 200
|
|
assert "attachment" in resp.headers["content-disposition"]
|
|
assert "imptune-backup-key.txt" in resp.headers["content-disposition"]
|
|
|
|
key = resp.text
|
|
assert len(key) > 20
|
|
|
|
owner = Owner.get(Owner.key == key)
|
|
assert owner.is_permanent is True
|
|
|
|
|
|
def test_restore_with_valid_key_reattaches_owner_data(tmp_data_dir):
|
|
from imptune.main import app
|
|
|
|
with TestClient(app) as client_a:
|
|
client_a.get("/")
|
|
client_a.post(
|
|
"/printers",
|
|
data={"name": "Backed Up Printer", "ip_address": "10.0.0.3", "port_name": "IP_C"},
|
|
follow_redirects=False,
|
|
)
|
|
key = client_a.get("/session/key/download").text
|
|
|
|
with TestClient(app) as client_new:
|
|
restore_resp = client_new.post(
|
|
"/session/restore",
|
|
data={"key": key},
|
|
headers={"origin": "http://testserver"},
|
|
follow_redirects=False,
|
|
)
|
|
assert restore_resp.status_code == 303
|
|
|
|
page = client_new.get("/printers")
|
|
assert "Backed Up Printer" in page.text
|
|
|
|
|
|
def test_restore_with_invalid_key_shows_error(client):
|
|
resp = client.post(
|
|
"/session/restore",
|
|
data={"key": "not-a-real-key"},
|
|
headers={"origin": "http://testserver"},
|
|
)
|
|
assert resp.status_code == 404
|
|
assert "Key not found" in resp.text
|
|
|
|
|
|
def test_restore_rejects_cross_origin_post(client):
|
|
"""CSRF guard: a forged cross-site form POST must not be able to re-point
|
|
the victim's cookie at an attacker-known key (login-CSRF / session fixation)."""
|
|
resp = client.post(
|
|
"/session/restore",
|
|
data={"key": "irrelevant"},
|
|
headers={"origin": "https://attacker.example"},
|
|
)
|
|
assert resp.status_code == 403
|
|
|
|
resp_no_header = client.post("/session/restore", data={"key": "irrelevant"})
|
|
assert resp_no_header.status_code == 403
|
|
|
|
|
|
def test_health_endpoint_does_not_create_owner_rows(client):
|
|
from imptune.db.models import Owner
|
|
|
|
before = Owner.select().count()
|
|
for _ in range(5):
|
|
client.get("/health")
|
|
after = Owner.select().count()
|
|
assert after == before
|
|
|
|
|
|
def test_static_assets_do_not_create_owner_rows(client):
|
|
"""Asset fetches are unscoped — they must not mint an Owner row each time."""
|
|
from imptune.db.models import Owner
|
|
|
|
client.get("/") # first visit provisions exactly one Owner
|
|
before = Owner.select().count()
|
|
for _ in range(5):
|
|
assert client.get("/static/app.css").status_code == 200
|
|
client.get("/favicon.ico")
|
|
assert Owner.select().count() == before
|
|
|
|
|
|
def test_restore_accepts_https_origin_behind_tls_proxy(tmp_data_dir):
|
|
"""A TLS-terminating proxy leaves uvicorn seeing http:// while the browser
|
|
sends Origin: https://host. Comparing schemes rejected every real restore."""
|
|
from imptune.main import app
|
|
|
|
with TestClient(app) as client_a:
|
|
client_a.get("/")
|
|
client_a.post(
|
|
"/printers",
|
|
data={"name": "Proxied Printer", "ip_address": "10.0.0.9", "port_name": "IP_P"},
|
|
follow_redirects=False,
|
|
)
|
|
key = client_a.get("/session/key/download").text
|
|
|
|
with TestClient(app) as client_new:
|
|
resp = client_new.post(
|
|
"/session/restore",
|
|
data={"key": key},
|
|
headers={"origin": "https://testserver"},
|
|
follow_redirects=False,
|
|
)
|
|
assert resp.status_code == 303
|
|
assert "Proxied Printer" in client_new.get("/printers").text
|
|
|
|
|
|
def test_restore_still_rejects_foreign_host_origin(client):
|
|
"""Host is what's checked — a same-scheme attacker host must still fail."""
|
|
resp = client.post(
|
|
"/session/restore",
|
|
data={"key": "irrelevant"},
|
|
headers={"origin": "http://attacker.example"},
|
|
)
|
|
assert resp.status_code == 403
|
|
|
|
|
|
# ---------------------------------------------------------------------------
|
|
# COOKIE_SECURE=false — usable, remembered, but memory-only + warned about
|
|
# ---------------------------------------------------------------------------
|
|
|
|
|
|
def _set_cookie_header(resp):
|
|
return next(
|
|
v for k, v in resp.headers.multi_items() if k.lower() == "set-cookie"
|
|
)
|
|
|
|
|
|
def test_insecure_mode_issues_a_browser_session_cookie(client, monkeypatch):
|
|
"""No Max-Age/Expires: the browser keeps the key in memory and drops it on
|
|
exit, instead of writing a 10-year plaintext bearer credential to disk."""
|
|
import imptune.config as cfg
|
|
|
|
monkeypatch.setattr(cfg, "COOKIE_SECURE", False)
|
|
resp = client.get("/")
|
|
|
|
header = _set_cookie_header(resp)
|
|
assert COOKIE_NAME in header
|
|
assert "Max-Age" not in header
|
|
assert "Expires" not in header
|
|
assert "Secure" not in header
|
|
|
|
|
|
def test_secure_mode_still_issues_a_persistent_cookie(tmp_data_dir, monkeypatch):
|
|
from imptune.main import app
|
|
|
|
import imptune.config as cfg
|
|
|
|
monkeypatch.setattr(cfg, "COOKIE_SECURE", True)
|
|
with TestClient(app) as fresh:
|
|
header = _set_cookie_header(fresh.get("/"))
|
|
|
|
assert "Max-Age=" in header
|
|
assert "Secure" in header
|
|
|
|
|
|
def test_insecure_mode_still_remembers_configs_within_the_session(client, monkeypatch):
|
|
"""The whole point: memory-only ≠ stateless. Same browser, same data."""
|
|
import imptune.config as cfg
|
|
|
|
monkeypatch.setattr(cfg, "COOKIE_SECURE", False)
|
|
client.post(
|
|
"/printers",
|
|
data={"name": "Ephemeral Printer", "ip_address": "10.0.6.1", "port_name": "IP_E"},
|
|
follow_redirects=False,
|
|
)
|
|
assert "Ephemeral Printer" in client.get("/printers").text
|
|
|
|
|
|
def test_insecure_mode_warns_the_user(client, monkeypatch):
|
|
import imptune.config as cfg
|
|
|
|
monkeypatch.setattr(cfg, "COOKIE_SECURE", False)
|
|
html = client.get("/printers").text
|
|
|
|
assert "ephemeral-session-warning" in html
|
|
assert "only while this browser stays open" in html
|
|
assert "/session/key/download" in html
|
|
|
|
|
|
def test_secure_mode_shows_no_warning(tmp_data_dir, monkeypatch):
|
|
from imptune.main import app
|
|
|
|
import imptune.config as cfg
|
|
|
|
monkeypatch.setattr(cfg, "COOKIE_SECURE", True)
|
|
with TestClient(app) as fresh:
|
|
assert "ephemeral-session-warning" not in fresh.get("/").text
|
|
|
|
|
|
def test_restore_also_issues_a_session_cookie_when_insecure(tmp_data_dir, monkeypatch):
|
|
"""The restore route sets its own cookie — it must honour the same policy."""
|
|
from imptune.main import app
|
|
|
|
import imptune.config as cfg
|
|
|
|
monkeypatch.setattr(cfg, "COOKIE_SECURE", False)
|
|
with TestClient(app) as client_a:
|
|
client_a.get("/")
|
|
key = client_a.get("/session/key/download").text
|
|
|
|
with TestClient(app) as client_new:
|
|
resp = client_new.post(
|
|
"/session/restore",
|
|
data={"key": key},
|
|
headers={"origin": "http://testserver"},
|
|
follow_redirects=False,
|
|
)
|
|
assert resp.status_code == 303
|
|
header = _set_cookie_header(resp)
|
|
assert "Max-Age" not in header and "Expires" not in header
|