diff --git a/services/presentation/app/accounts.py b/services/presentation/app/accounts.py index ee67d39..6eed557 100644 --- a/services/presentation/app/accounts.py +++ b/services/presentation/app/accounts.py @@ -28,6 +28,16 @@ from app import features _lock = threading.Lock() +class AccountsUnavailable(RuntimeError): + """Pliku kont nie da się odczytać albo zapisać — problem z magazynem. + + Osobny wyjątek, bo to NIE JEST błąd danych: konta mogą być w porządku, tylko + katalog jest niedostępny (złe prawa na udziale, nieprzemontowany wolumen, + brak miejsca). Ekran ma wtedy POWIEDZIEĆ, co i gdzie jest nie tak — gołe 500 + zostawia administratora z niczym, a to jedyny ekran, z którego może to + naprawić.""" + + def store_path() -> str: """Ścieżka pliku kont. @@ -38,27 +48,62 @@ def store_path() -> str: def _read() -> dict: + path = store_path() try: - with open(store_path(), encoding="utf-8") as fh: + with open(path, encoding="utf-8") as fh: data = json.load(fh) - except (FileNotFoundError, json.JSONDecodeError): + except FileNotFoundError: + # Pierwsze uruchomienie: pliku jeszcze nie ma i to jest normalne. return {"users": {}} + except json.JSONDecodeError: + # Plik jest, ale nieczytelny. NIE nadpisujemy go pustym zbiorem — + # to skasowałoby wszystkie konta przy pierwszym zapisie. + raise AccountsUnavailable( + f"Plik kont {path} jest uszkodzony (niepoprawny JSON). " + f"Nic nie zostało zmienione — napraw albo usuń plik ręcznie.") from None + except OSError as e: + raise AccountsUnavailable( + f"Nie mogę odczytać pliku kont {path}: {e.strerror or e}. " + f"Najczęstsza przyczyna: prawa do katalogu na udziale albo " + f"niezamontowany wolumen.") from e users = data.get("users") return {"users": users if isinstance(users, dict) else {}} def _write(data: dict) -> None: path = store_path() - os.makedirs(os.path.dirname(path) or ".", exist_ok=True) + try: + os.makedirs(os.path.dirname(path) or ".", exist_ok=True) + except OSError as e: + raise AccountsUnavailable( + f"Nie mogę utworzyć katalogu na plik kont ({os.path.dirname(path)}): " + f"{e.strerror or e}.") from e # Plik tymczasowy MUSI powstać w tym samym katalogu — os.replace jest # niepodzielne tylko w obrębie jednego systemu plików. - fd, tmp = tempfile.mkstemp(dir=os.path.dirname(path) or ".", suffix=".tmp") + # mkstemp też musi być w klamrze: przy katalogu tylko do odczytu wywala się + # ONO pierwsze, jeszcze zanim dojdzie do zapisu i podmiany. + try: + fd, tmp = tempfile.mkstemp(dir=os.path.dirname(path) or ".", suffix=".tmp") + except OSError as e: + raise AccountsUnavailable( + f"Nie mogę zapisać pliku kont {path}: {e.strerror or e}. " + f"Najczęstsza przyczyna: udział zamontowany tylko do odczytu albo " + f"prawa katalogu. Nic nie zostało zmienione.") from e try: with os.fdopen(fd, "w", encoding="utf-8") as fh: json.dump(data, fh, ensure_ascii=False, indent=1, sort_keys=True) fh.flush() os.fsync(fh.fileno()) os.replace(tmp, path) + except OSError as e: + try: + os.unlink(tmp) + except OSError: + pass + raise AccountsUnavailable( + f"Nie mogę zapisać pliku kont {path}: {e.strerror or e}. " + f"Najczęstsza przyczyna: udział zamontowany tylko do odczytu albo " + f"prawa katalogu. Nic nie zostało zmienione.") from e except BaseException: try: os.unlink(tmp) diff --git a/services/presentation/app/main.py b/services/presentation/app/main.py index fece586..1ec7e53 100644 --- a/services/presentation/app/main.py +++ b/services/presentation/app/main.py @@ -816,8 +816,15 @@ def _http_detail(e: httpx.HTTPStatusError) -> str: # dla całej aplikacji, sprawdzana testem, który przechodzi po WSZYSTKICH trasach. def _accounts_context(request: Request, error: str = "", done: str = "") -> dict: + # Problem z magazynem NIE MOŻE dawać gołego 500: to jedyny ekran, z którego + # administrator może go naprawić, więc musi na nim przeczytać, co i gdzie + # jest nie tak. + try: + users = accounts_store.all_users() + except accounts_store.AccountsUnavailable as e: + users, error = {}, str(e) return { - "users": accounts_store.all_users(), + "users": users, "catalog": features.ALL, "screens": features.SCREENS, "extras": features.EXTRAS, @@ -847,7 +854,7 @@ def accounts_create(request: Request, login: str = Form(""), password: str = For note: str = Form(""), granted: list[str] = Form([])): try: accounts_store.create(login, password, granted, note) - except ValueError as e: + except (ValueError, accounts_store.AccountsUnavailable) as e: return _accounts_redirect(error=str(e)) return _accounts_redirect(done=f"Założono konto „{login.strip()}”.") @@ -857,7 +864,7 @@ def accounts_update(request: Request, login: str = Form(...), password: str = Fo note: str = Form(""), granted: list[str] = Form([])): try: accounts_store.update(login, granted=granted, password=password, note=note) - except ValueError as e: + except (ValueError, accounts_store.AccountsUnavailable) as e: return _accounts_redirect(error=str(e)) changed = "uprawnienia i hasło" if password.strip() else "uprawnienia" return _accounts_redirect(done=f"Zapisano {changed} konta „{login}”.") @@ -867,7 +874,7 @@ def accounts_update(request: Request, login: str = Form(...), password: str = Fo def accounts_delete(request: Request, login: str = Form(...)): try: accounts_store.delete(login) - except ValueError as e: + except (ValueError, accounts_store.AccountsUnavailable) as e: return _accounts_redirect(error=str(e)) return _accounts_redirect(done=f"Skasowano konto „{login}”.") diff --git a/services/presentation/tests/test_kontrola_dostepu.py b/services/presentation/tests/test_kontrola_dostepu.py index 3c528d4..f9fbb2b 100644 --- a/services/presentation/tests/test_kontrola_dostepu.py +++ b/services/presentation/tests/test_kontrola_dostepu.py @@ -309,3 +309,62 @@ def test_deleting_an_account_cannot_touch_the_administrator(env, monkeypatch): assert r.status_code == 303 assert store.exists("ala"), "kasowanie nieistniejącego konta ruszyło inne" assert c.get("/", headers=admin).status_code == 200 + + +# ── awaria magazynu kont ───────────────────────────────────────────────── +# Ekran kont to JEDYNE miejsce, z którego administrator może naprawić problem +# z magazynem — więc musi na nim przeczytać, co i gdzie jest nie tak. Gołe 500 +# (tak było na pierwszym wdrożeniu) zostawia go z niczym. + +def _unreadable(tmp_path, monkeypatch, mode): + d = tmp_path / "stan" + d.mkdir() + monkeypatch.setenv("ACCOUNTS_FILE", str(d / "accounts.json")) + d.chmod(mode) + return d + + +def test_unreadable_store_explains_itself_instead_of_500(env, tmp_path, monkeypatch): + d = _unreadable(tmp_path, monkeypatch, 0o000) + try: + c = _client(monkeypatch) + r = c.get("/accounts", headers=_auth("szef", "tajne-szefa")) + assert r.status_code == 200, "problem z magazynem nie może wywalać strony" + assert "Nie mogę odczytać pliku kont" in r.text + assert str(d / "accounts.json") in r.text, "komunikat ma podać ŚCIEŻKĘ" + finally: + d.chmod(0o755) + + +def test_read_only_store_refuses_to_save_with_a_reason(env, tmp_path, monkeypatch): + d = _unreadable(tmp_path, monkeypatch, 0o555) + try: + c = _client(monkeypatch) + r = c.post("/accounts/create", headers=_auth("szef", "tajne-szefa"), + follow_redirects=False, + data={"login": "ala", "password": "x", "granted": ["chart"]}) + assert r.status_code == 303 + # Komunikat jedzie w parametrze zapytania, więc jest zakodowany — + # porównanie na surowym nagłówku sprawdzałoby procenty, nie treść. + from urllib.parse import unquote_plus + + assert "Nie mogę zapisać pliku kont" in unquote_plus(r.headers["location"]) + finally: + d.chmod(0o755) + + +def test_a_corrupt_file_is_never_silently_overwritten(env, tmp_path, monkeypatch): + """Nadpisanie uszkodzonego pliku pustym zbiorem skasowałoby WSZYSTKIE konta. + Lepiej odmówić i powiedzieć, co jest nie tak.""" + path = tmp_path / "accounts.json" + path.write_text("{to nie jest json", encoding="utf-8") + monkeypatch.setenv("ACCOUNTS_FILE", str(path)) + + c = _client(monkeypatch) + r = c.get("/accounts", headers=_auth("szef", "tajne-szefa")) + assert r.status_code == 200 and "uszkodzony" in r.text + + c.post("/accounts/create", headers=_auth("szef", "tajne-szefa"), + follow_redirects=False, data={"login": "ala", "password": "x"}) + assert path.read_text(encoding="utf-8") == "{to nie jest json", \ + "uszkodzony plik został nadpisany — konta by zniknęły"