Code review findings on the "skip revoke when .crt is missing" migration path: - CliRunner._issue() took the skip path whenever has_issued_cert() was False, which is also true for a typo'd/nonexistent CN — it would warn, skip the revoke, and go on to build, package, and email a brand-new certificate for a CN nobody asked to renew. The skip now only fires when the CN has a current index.txt entry (via _load_current_certs()); an unknown CN prints an error and exits 1 with nothing built. The TUI's _process_cert() doesn't need the same guard — renewal there always opens on an existing row (cn_readonly pins the CN), so a typo'd CN can't reach the branch. - Skipping the revoke leaves pki/reqs/<CN>.req and pki/private/<CN>.key in place (normally revoke-issued archives both), which makes EasyRSA's build-client-full abort. Both CliRunner._issue() and CursesApp._process_cert() now check for those leftovers before building and fail fast with the exact paths, rather than surfacing EasyRSA's confusing error after the CA passphrase prompt. Neither path touches the files itself. Also: strengthened two under-specified tests (test_main_rejects_bad_days_flag now checks the resolver's message text, not just "--days", which also appears in argparse's unrelated error; test_show_cert_form_confirm now pins the Enter-keypress count so a partial "days" field reversion is caught), and folded a malformed CLAUDE.md table row into its neighbor. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
149 lines
6.3 KiB
Python
149 lines
6.3 KiB
Python
"""Tests for the TUI re-issue workflow (CursesApp._process_cert).
|
|
|
|
Uses the same mock-curses approach as test_dialogs.py — no real terminal
|
|
needed. _msg()/_error() swallow curses.error, so a MagicMock stdscr is enough
|
|
as long as getmaxyx() returns real ints.
|
|
"""
|
|
from unittest.mock import MagicMock, patch
|
|
import curses as _curses
|
|
|
|
# Stub curses constants/callables before importing the module under test.
|
|
_curses.color_pair = lambda x: 0
|
|
_curses.A_BOLD = 0
|
|
_curses.A_UNDERLINE = 0
|
|
_curses.curs_set = lambda x: None
|
|
|
|
from openvpncertupdate import CursesApp, CertFormResult
|
|
|
|
|
|
def _stdscr():
|
|
s = MagicMock()
|
|
s.getmaxyx.return_value = (24, 80) # `sh - 2` needs a real int
|
|
s.getch.return_value = ord("q")
|
|
return s
|
|
|
|
|
|
def _patch_workflow(monkeypatch, cert_file_present):
|
|
"""Patch everything _process_cert touches after the form is confirmed."""
|
|
monkeypatch.setattr(
|
|
"openvpncertupdate.show_cert_form",
|
|
MagicMock(return_value=CertFormResult(
|
|
cn="y.kuts", email="", password="Testpass1234567890abcdefgh",
|
|
confirmed=True)))
|
|
monkeypatch.setattr("openvpncertupdate.has_issued_cert",
|
|
MagicMock(return_value=cert_file_present))
|
|
for name, retval in (
|
|
("revoke_issued", None),
|
|
("gen_crl", None),
|
|
("copy_crl", None),
|
|
("build_client_full", None),
|
|
("build_ovpn", "/out/y.kuts_2026-08-15_01/client.ovpn"),
|
|
("create_note", "https://cg.example.com/note/abc#deadbeef"),
|
|
):
|
|
monkeypatch.setattr(f"openvpncertupdate.{name}",
|
|
MagicMock(return_value=retval))
|
|
import openvpncertupdate as m
|
|
return {n: getattr(m, n) for n in (
|
|
"revoke_issued", "gen_crl", "copy_crl", "build_client_full")}
|
|
|
|
|
|
def test_tui_reissue_skips_revoke_when_cert_file_missing(monkeypatch):
|
|
# The bug as reported: the TUI offered renewal for a CN whose .crt was
|
|
# never carried over from the older EasyRSA install, and revoke-issued
|
|
# failed. There is nothing to revoke — build the replacement instead.
|
|
mocks = _patch_workflow(monkeypatch, cert_file_present=False)
|
|
app = CursesApp()
|
|
assert app._process_cert(_stdscr(), "y.kuts", "", is_renewal=True) is True
|
|
mocks["revoke_issued"].assert_not_called()
|
|
mocks["gen_crl"].assert_not_called()
|
|
mocks["copy_crl"].assert_not_called()
|
|
mocks["build_client_full"].assert_called_once()
|
|
assert mocks["build_client_full"].call_args.args[2] == "y.kuts"
|
|
|
|
|
|
def test_tui_reissue_logs_the_skip(monkeypatch):
|
|
_patch_workflow(monkeypatch, cert_file_present=False)
|
|
app = CursesApp()
|
|
app._process_cert(_stdscr(), "y.kuts", "", is_renewal=True)
|
|
assert any("revoke skipped" in e for e in app._session_log)
|
|
|
|
|
|
def test_tui_reissue_revokes_when_cert_file_present(monkeypatch):
|
|
mocks = _patch_workflow(monkeypatch, cert_file_present=True)
|
|
app = CursesApp()
|
|
assert app._process_cert(_stdscr(), "y.kuts", "", is_renewal=True) is True
|
|
mocks["revoke_issued"].assert_called_once()
|
|
mocks["gen_crl"].assert_called_once()
|
|
mocks["copy_crl"].assert_called_once()
|
|
mocks["build_client_full"].assert_called_once()
|
|
|
|
|
|
def test_tui_skipped_revoke_build_failure_does_not_claim_revocation(monkeypatch):
|
|
# Nothing was revoked, so the "has been revoked" warning would be a lie.
|
|
import openvpncertupdate as m
|
|
_patch_workflow(monkeypatch, cert_file_present=False)
|
|
monkeypatch.setattr("openvpncertupdate.build_client_full",
|
|
MagicMock(side_effect=m.EasyRSAError("boom")))
|
|
app = CursesApp()
|
|
shown = []
|
|
monkeypatch.setattr(CursesApp, "_error",
|
|
lambda self, stdscr, text: shown.append(text))
|
|
app._process_cert(_stdscr(), "y.kuts", "", is_renewal=True)
|
|
assert shown and "has been revoked" not in shown[0]
|
|
|
|
|
|
def test_tui_seeds_days_field_from_cert_days(monkeypatch):
|
|
import openvpncertupdate as m
|
|
_patch_workflow(monkeypatch, cert_file_present=True)
|
|
monkeypatch.setattr("openvpncertupdate.CERT_DAYS", "90")
|
|
CursesApp()._process_cert(_stdscr(), "y.kuts", "", is_renewal=True)
|
|
assert m.show_cert_form.call_args.kwargs["days"] == "90"
|
|
|
|
|
|
def test_tui_reissue_blocks_skip_when_leftover_req_exists(monkeypatch, tmp_path):
|
|
# revoke-issued normally archives pki/reqs/<CN>.req and
|
|
# pki/private/<CN>.key into pki/revoked/; when it's skipped (no issued
|
|
# .crt to revoke) those leftovers make build-client-full abort. Catch it
|
|
# with an error dialog instead of letting the confusing EasyRSA error
|
|
# surface after the CA passphrase prompt.
|
|
(tmp_path / "reqs").mkdir()
|
|
req_path = tmp_path / "reqs" / "y.kuts.req"
|
|
req_path.write_text("leftover request")
|
|
monkeypatch.setattr("openvpncertupdate.EASYRSA_PKI_DIR", str(tmp_path))
|
|
mocks = _patch_workflow(monkeypatch, cert_file_present=False)
|
|
app = CursesApp()
|
|
shown = []
|
|
monkeypatch.setattr(CursesApp, "_error",
|
|
lambda self, stdscr, text: shown.append(text))
|
|
result = app._process_cert(_stdscr(), "y.kuts", "", is_renewal=True)
|
|
assert result is True
|
|
assert shown and str(req_path) in shown[0]
|
|
mocks["build_client_full"].assert_not_called()
|
|
|
|
|
|
def test_tui_reissue_blocks_skip_when_leftover_key_exists(monkeypatch, tmp_path):
|
|
(tmp_path / "private").mkdir()
|
|
key_path = tmp_path / "private" / "y.kuts.key"
|
|
key_path.write_text("leftover key")
|
|
monkeypatch.setattr("openvpncertupdate.EASYRSA_PKI_DIR", str(tmp_path))
|
|
mocks = _patch_workflow(monkeypatch, cert_file_present=False)
|
|
app = CursesApp()
|
|
shown = []
|
|
monkeypatch.setattr(CursesApp, "_error",
|
|
lambda self, stdscr, text: shown.append(text))
|
|
result = app._process_cert(_stdscr(), "y.kuts", "", is_renewal=True)
|
|
assert result is True
|
|
assert shown and str(key_path) in shown[0]
|
|
mocks["build_client_full"].assert_not_called()
|
|
|
|
|
|
def test_tui_forwards_form_days_to_build_client_full(monkeypatch):
|
|
mocks = _patch_workflow(monkeypatch, cert_file_present=True)
|
|
monkeypatch.setattr(
|
|
"openvpncertupdate.show_cert_form",
|
|
MagicMock(return_value=CertFormResult(
|
|
cn="y.kuts", email="", password="Testpass1234567890abcdefgh",
|
|
days="30", confirmed=True)))
|
|
CursesApp()._process_cert(_stdscr(), "y.kuts", "", is_renewal=True)
|
|
assert mocks["build_client_full"].call_args.kwargs["days"] == "30"
|