Naprawa: ekran kont tłumaczy awarię magazynu zamiast dawać 500 #71
@@ -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)
|
||||
|
||||
@@ -677,8 +677,15 @@ def timezone_lookup(lat: float, lon: float, date: str = "", time: str = "12:00")
|
||||
# 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,
|
||||
@@ -708,7 +715,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()}”.")
|
||||
|
||||
@@ -718,7 +725,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}”.")
|
||||
@@ -728,7 +735,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}”.")
|
||||
|
||||
|
||||
@@ -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"
|
||||
|
||||
Reference in New Issue
Block a user