Mark CNs with no issued cert file in the TUI and CLI lists
Renewing such a CN now works (it skips the revoke), but the list gave no hint that it was a special case until the workflow printed its warning. Flag it at selection time instead. _load_current_certs() sets CertInfo.has_cert_file, so every view built on it — the TUI list, --list and --list-all — gets the flag for free. The stat happens after the per-CN dedup, so a CN with several V-lines in index.txt is checked once. TUI rows render "(no cert file)" between the CN and the email, placed before the email so a long address truncating at the right edge cannot push the marker off screen. --list/--list-all grow a trailing CERT column holding MISSING; the column is omitted entirely when every CN has its .crt, since it is pure noise on a healthy PKI. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -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 `<PKI_DIR>/issued/<CN>.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 `<PKI_DIR>/issued/<CN>.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 <CN>` — 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 <CN> --passout=pass:<pw>` — generates new key + cert
|
||||
|
||||
@@ -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 <PKI_DIR>/issued/<CN>.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)
|
||||
|
||||
|
||||
|
||||
@@ -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
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -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/<CN>.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()
|
||||
|
||||
@@ -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/<CN>.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)
|
||||
|
||||
Reference in New Issue
Block a user