diff --git a/CLAUDE.md b/CLAUDE.md index beafbcd..f579058 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -31,6 +31,7 @@ Edit the `SETTINGS` block at the top of `openvpncertupdate.py` before first run. | `--gen-crl` | Regenerate and copy CRL only | | `--list` | List recently-expired/soon-to-expire CNs (per `DAYS_PAST`/`DAYS_AHEAD`) with email; read-only, no CA passphrase needed | | `--list-all` | List all CNs with email; read-only, no CA passphrase needed | +| | Both grow a trailing `CERT` column marking `MISSING` CNs — see `CertInfo.has_cert_file`. The column is omitted entirely when every CN has its `.crt` | | `--email EMAIL` | Recipient address | | `--send-email` | Force email delivery | | `--no-send-email` | Skip email; print URL to stdout | @@ -66,7 +67,7 @@ python3 -m pytest tests/test_pki.py::test_sorted_ascending -v # single test ## Re-issue workflow -0. `has_issued_cert()` gates steps 1–2: if `/issued/.crt` is absent, both are **skipped** with a warning and the workflow goes straight to step 3. EasyRSA reads the serial out of the `.crt` itself, so `revoke-issued` can only fail on such a CN — and there is nothing to add to the CRL either. This happens when an `index.txt` is carried over from an older EasyRSA install without the `issued/` files: the index still lists V-status certs whose `.crt` never came along. `--revoke` / the TUI `r` key deliberately do *not* skip — an explicit revoke request should fail loudly rather than silently no-op +0. `has_issued_cert()` gates steps 1–2: if `/issued/.crt` is absent, both are **skipped** with a warning and the workflow goes straight to step 3. EasyRSA reads the serial out of the `.crt` itself, so `revoke-issued` can only fail on such a CN — and there is nothing to add to the CRL either. This happens when an `index.txt` is carried over from an older EasyRSA install without the `issued/` files: the index still lists V-status certs whose `.crt` never came along. `--revoke` / the TUI `r` key deliberately do *not* skip — an explicit revoke request should fail loudly rather than silently no-op. Such CNs are flagged before the user picks one: `_load_current_certs()` sets `CertInfo.has_cert_file` (one stat per CN, after the dedup), rendered as `(no cert)` before the email in the TUI list and as a `MISSING` cell in the `CERT` column of `--list`/`--list-all` 1. `revoke-issued ` — archives old key + CSR to `pki/revoked/` 2. CRL regenerated and copied to `CRL_DEST_PATH` immediately after the revoke succeeds — the old cert is already revoked at this point, so the published CRL would otherwise be stale until a separate manual regen. Not fatal: a failure here is reported but the workflow continues to step 3 (a new cert is more urgent than a fresh CRL, and "Regenerate CRL" / `--gen-crl` remain available to retry) 3. `build-client-full --passout=pass:` — generates new key + cert diff --git a/openvpncertupdate.py b/openvpncertupdate.py index 89196f7..b96e836 100644 --- a/openvpncertupdate.py +++ b/openvpncertupdate.py @@ -133,6 +133,10 @@ class CertInfo: expires: datetime # UTC days_left: int # negative = already expired email: str = "" + # False when index.txt lists the CN but /issued/.crt is gone + # — see has_issued_cert(). Such a CN can still be re-issued, but nothing + # can be revoked for it, so the list marks it. + has_cert_file: bool = True def _parse_date(raw: str) -> datetime: @@ -194,7 +198,11 @@ def _load_current_certs(pki_dir: str) -> List[CertInfo]: days_left=(expiry - now).days, email=email, ) - return list(by_cn.values()) + certs = list(by_cn.values()) + # After the dedup, so a CN with several V-lines is stat'ed once. + for c in certs: + c.has_cert_file = has_issued_cert(pki_dir, c.cn) + return certs def load_expiring_certs( @@ -994,7 +1002,11 @@ def show_main_screen( cert = certs[i] box = "[X]" if cert.cn in checked else "[ ]" lbl = _expiry_label(cert) - line = f" {box} {lbl:<18} {cert.cn:<20} {cert.email}" + # Sits before the email so a long address truncating at the + # right edge can't push the marker off screen, and kept + # short so a typical address still fits at 80 columns. + note = "" if cert.has_cert_file else "(no cert) " + line = f" {box} {lbl:<18} {cert.cn:<20} {note}{cert.email}" try: stdscr.addstr(row_y, 0, line.ljust(sw - 1)[:sw - 1], sel_attr) except curses.error: @@ -1365,18 +1377,33 @@ class CursesApp: def _format_cert_table(certs: List[CertInfo]) -> str: - """Render CN / expiry / email as an aligned table, like `ls -l`.""" + """Render CN / expiry / email as an aligned table, like `ls -l`. + + A trailing CERT column appears only when at least one CN has lost its + issued .crt — it is pure noise on a healthy PKI. + """ if not certs: return "(no certificates)" rows = [ - (c.cn, _expiry_label(c).strip(), get_email(EASYRSA_PKI_DIR, c.cn) or "(none)") + (c.cn, _expiry_label(c).strip(), get_email(EASYRSA_PKI_DIR, c.cn) or "(none)", + "" if c.has_cert_file else "MISSING") for c in certs ] + show_cert_col = any(r[3] for r in rows) cn_w = max(len("CN"), max(len(r[0]) for r in rows)) exp_w = max(len("EXPIRES"), max(len(r[1]) for r in rows)) - lines = [f"{'CN':<{cn_w}} {'EXPIRES':<{exp_w}} EMAIL"] - for cn, exp, email in rows: - lines.append(f"{cn:<{cn_w}} {exp:<{exp_w}} {email}") + if not show_cert_col: + lines = [f"{'CN':<{cn_w}} {'EXPIRES':<{exp_w}} EMAIL"] + for cn, exp, email, _ in rows: + lines.append(f"{cn:<{cn_w}} {exp:<{exp_w}} {email}") + return "\n".join(lines) + + mail_w = max(len("EMAIL"), max(len(r[2]) for r in rows)) + lines = [f"{'CN':<{cn_w}} {'EXPIRES':<{exp_w}} {'EMAIL':<{mail_w}} CERT"] + for cn, exp, email, cert in rows: + lines.append( + f"{cn:<{cn_w}} {exp:<{exp_w}} {email:<{mail_w}} {cert}".rstrip() + ) return "\n".join(lines) diff --git a/tests/test_cli.py b/tests/test_cli.py index 86a6c1b..72043d4 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -308,6 +308,47 @@ def test_format_cert_table_columns_are_aligned(monkeypatch): assert line.index("(none)") == email_col +def test_format_cert_table_has_no_cert_column_when_all_files_present(monkeypatch): + # The extra column is noise when nothing is missing, so it only appears + # when it has something to say. + import openvpncertupdate + certs = [openvpncertupdate.CertInfo(cn="alice", expires=None, days_left=24)] + monkeypatch.setattr("openvpncertupdate.get_email", MagicMock(return_value="")) + table = openvpncertupdate._format_cert_table(certs) + assert "CERT" not in table + assert "MISSING" not in table + + +def test_format_cert_table_marks_missing_cert_file(monkeypatch): + import openvpncertupdate + certs = [ + openvpncertupdate.CertInfo(cn="alice", expires=None, days_left=24), + openvpncertupdate.CertInfo(cn="y.kuts", expires=None, days_left=-2, + has_cert_file=False), + ] + monkeypatch.setattr("openvpncertupdate.get_email", MagicMock(return_value="")) + table = openvpncertupdate._format_cert_table(certs) + lines = table.splitlines() + assert lines[0].split() == ["CN", "EXPIRES", "EMAIL", "CERT"] + alice = next(l for l in lines if l.startswith("alice")) + kuts = next(l for l in lines if l.startswith("y.kuts")) + assert "MISSING" in kuts + assert "MISSING" not in alice + + +def test_format_cert_table_stays_aligned_with_cert_column(monkeypatch): + import openvpncertupdate + certs = [ + openvpncertupdate.CertInfo(cn="a", expires=None, days_left=24), + openvpncertupdate.CertInfo(cn="a-much-longer", expires=None, days_left=5, + has_cert_file=False), + ] + monkeypatch.setattr("openvpncertupdate.get_email", MagicMock(return_value="")) + lines = openvpncertupdate._format_cert_table(certs).splitlines() + cert_col = lines[0].index("CERT") + assert lines[2].index("MISSING") == cert_col + + # --------------------------------------------------------------------------- # CliRunner.list_certs # --------------------------------------------------------------------------- diff --git a/tests/test_main_screen.py b/tests/test_main_screen.py index b9cc0ab..15a083e 100644 --- a/tests/test_main_screen.py +++ b/tests/test_main_screen.py @@ -28,13 +28,15 @@ from openvpncertupdate import ( # Helpers # --------------------------------------------------------------------------- -def _cert(cn: str, days_left: int, email: str = "") -> CertInfo: +def _cert(cn: str, days_left: int, email: str = "", + has_cert_file: bool = True) -> CertInfo: """Build a minimal CertInfo; expires value is a plausible UTC datetime.""" return CertInfo( cn=cn, expires=datetime(2030, 1, 1, tzinfo=timezone.utc), days_left=days_left, email=email, + has_cert_file=has_cert_file, ) @@ -156,6 +158,31 @@ def test_cert_row_shows_email(): assert "alice@example.com" in alice_calls[0].args[2] +def test_cert_row_marks_missing_cert_file(): + # index.txt lists the CN but pki/issued/.crt is gone — renewing it + # cannot revoke anything, so say so on the row rather than at the point + # of failure. + certs = [_cert("y.kuts", -2, email="y@example.com", has_cert_file=False)] + stdscr = _make_stdscr() + stdscr.getch.side_effect = [ord("q")] + with patch("openvpncertupdate.init_colors"): + show_main_screen(stdscr, certs) + calls = [c for c in stdscr.addstr.call_args_list if "y.kuts" in str(c.args[2])] + assert calls + assert "(no cert)" in calls[0].args[2] + + +def test_cert_row_unmarked_when_cert_file_present(): + certs = [_cert("alice", 10, email="alice@example.com")] + stdscr = _make_stdscr() + stdscr.getch.side_effect = [ord("q")] + with patch("openvpncertupdate.init_colors"): + show_main_screen(stdscr, certs) + calls = [c for c in stdscr.addstr.call_args_list if "alice" in str(c.args[2])] + assert calls + assert "(no cert)" not in calls[0].args[2] + + def test_cert_row_blank_when_no_email(): certs = [_cert("bob", 3)] # email defaults to "" stdscr = _make_stdscr() diff --git a/tests/test_pki.py b/tests/test_pki.py index 4d3e413..85722fd 100644 --- a/tests/test_pki.py +++ b/tests/test_pki.py @@ -22,6 +22,40 @@ def make_pki(tmp_path, now): return str(pki) +# --------------------------------------------------------------------------- +# has_cert_file — index.txt can list a CN whose pki/issued/.crt is gone +# (index carried over from an older EasyRSA install). Those CNs cannot be +# revoked, so the list marks them. +# --------------------------------------------------------------------------- + +def test_has_cert_file_false_when_issued_dir_missing(tmp_path): + now = datetime.now(tz=timezone.utc) + pki = make_pki(tmp_path, now) + assert all(c.has_cert_file is False for c in load_all_certs(pki)) + + +def test_has_cert_file_tracks_issued_dir(tmp_path): + now = datetime.now(tz=timezone.utc) + pki = make_pki(tmp_path, now) + issued = tmp_path / "pki" / "issued" + issued.mkdir() + (issued / "soon.crt").write_text("-----BEGIN CERTIFICATE-----\n") + by_cn = {c.cn: c for c in load_all_certs(pki)} + assert by_cn["soon"].has_cert_file is True + assert by_cn["later"].has_cert_file is False + + +def test_has_cert_file_set_on_expiring_view_too(tmp_path): + now = datetime.now(tz=timezone.utc) + pki = make_pki(tmp_path, now) + issued = tmp_path / "pki" / "issued" + issued.mkdir() + (issued / "past15.crt").write_text("-----BEGIN CERTIFICATE-----\n") + by_cn = {c.cn: c for c in load_expiring_certs(pki, days_past=30, days_ahead=14)} + assert by_cn["past15"].has_cert_file is True + assert by_cn["soon"].has_cert_file is False + + def test_filters_within_window(tmp_path): now = datetime.now(tz=timezone.utc) pki = make_pki(tmp_path, now)