Compare commits
14 Commits
6b39c1209f
...
1d059db083
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
1d059db083 | ||
|
|
68f481095a | ||
|
|
d1cd4c3132 | ||
|
|
4af7d67bf3 | ||
|
|
96664f954c | ||
|
|
fad3e5187e | ||
|
|
54f3e300a5 | ||
|
|
2e98155b5f | ||
|
|
119a567a49 | ||
|
|
749cec8b37 | ||
|
|
506787f8fc | ||
|
|
c2f51a0920 | ||
|
|
8adee23a69 | ||
|
|
9e5df8d9bb |
3
.gitignore
vendored
3
.gitignore
vendored
@@ -1 +1,4 @@
|
||||
__pycache__/
|
||||
|
||||
# Upstream EasyRSA checkout, kept alongside for reference — not part of this project
|
||||
easy-rsa/
|
||||
|
||||
@@ -13,6 +13,7 @@ pip install -r requirements.txt # just: cryptography>=41
|
||||
python3 openvpncertupdate.py # interactive TUI
|
||||
python3 openvpncertupdate.py --create CN --email user@example.com
|
||||
python3 openvpncertupdate.py --reissue CN [--email user@example.com]
|
||||
python3 openvpncertupdate.py --create CN --email user@example.com --days 90
|
||||
python3 openvpncertupdate.py --revoke CN
|
||||
python3 openvpncertupdate.py --gen-crl
|
||||
python3 openvpncertupdate.py --list
|
||||
@@ -30,8 +31,9 @@ Edit the `SETTINGS` block at the top of `openvpncertupdate.py` before first run.
|
||||
| `--revoke CN` | Revoke cert and regenerate CRL |
|
||||
| `--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 |
|
||||
| `--list-all` | List all CNs with email; read-only, no CA passphrase needed. Both `--list`/`--list-all` 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 |
|
||||
| `--days N` | Certificate lifetime in days for `--create`/`--reissue`; overrides `CERT_DAYS` for that run |
|
||||
| `--send-email` | Force email delivery |
|
||||
| `--no-send-email` | Skip email; print URL to stdout |
|
||||
| `--show-eml` | Print base64-encoded `.eml` to stdout (implies `--no-send-email` unless `--send-email` also given) |
|
||||
@@ -53,7 +55,7 @@ python3 -m pytest tests/test_pki.py::test_sorted_ascending -v # single test
|
||||
| SETTINGS OVERRIDE | `ConfigError`, `load_settings_overrides()`, `_OVERRIDABLE_SETTINGS` |
|
||||
| PKI | `CertInfo`, `_load_current_certs()`, `load_expiring_certs()`, `load_all_certs()`, `_parse_index_line()`, `get_email()` |
|
||||
| PASSWORD | `generate_password()` |
|
||||
| EASYRSA | `EasyRSAError`, `revoke_issued()`, `build_client_full()`, `gen_crl()`, `copy_crl()`, `is_ca_key_encrypted()`, `resolve_ca_passphrase()` |
|
||||
| EASYRSA | `EasyRSAError`, `_easyrsa_diagnostics()`, `issued_cert_path()`, `has_issued_cert()`, `revoke_issued()`, `build_client_full()`, `gen_crl()`, `copy_crl()`, `is_ca_key_encrypted()`, `resolve_ca_passphrase()`, `resolve_cert_days()` |
|
||||
| CONFIG | `build_ovpn()` → `vpn-configs/<CN>_<YYYY-MM-DD>_<NN>/CONFIG_NAME` |
|
||||
| CRYPTGEON | `CryptgeonError`, `create_note()` |
|
||||
| MAILER | `build_mime_message()`, `send_email()` |
|
||||
@@ -66,6 +68,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. 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`. Two guards bound the skip so it can't silently do the wrong thing: (a) on the CLI, the skip only fires for a CN that `_load_current_certs()` actually knows about — a typo'd/nonexistent `--reissue CN` is *not* a migration gap and is rejected with `error: unknown CN ...` before anything is built (the TUI can't hit this: renewal always opens on an existing row, so the CN is never freeform there); (b) whichever entry point takes the skip, it first checks for leftover `pki/reqs/<CN>.req` / `pki/private/<CN>.key` — normally `revoke-issued` archives both into `pki/revoked/`, but skipping it leaves them in place, and EasyRSA's `build-client-full` aborts outright rather than overwrite them. Either leftover fails the workflow fast with the exact path(s), before the CA passphrase prompt — this tool never moves or deletes a private key itself
|
||||
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
|
||||
@@ -86,7 +89,9 @@ python3 -m pytest tests/test_pki.py::test_sorted_ascending -v # single test
|
||||
- External config file (`load_settings_overrides()`, run once in `main()` right after arg parsing, before dispatch): resolution order is `--config PATH` > `CONFIG_PATH` setting > `<this-script-path>.conf` next to the script. The CLI flag or `CONFIG_PATH` make the path explicit — a missing file there is a fatal `ConfigError`; the default `<script>.conf` path is optional and silently skipped if absent. The file is executed as Python (same syntax as the `SETTINGS` block, so only run trusted files) and only names listed in `_OVERRIDABLE_SETTINGS` are applied — `CONFIG_PATH` itself is deliberately not overridable this way
|
||||
- Email: set `SMTP_HOST` to use smtplib (SMTP_TLS: `"starttls"`/`"ssl"`/`""`); leave empty to use `MAIL_BINARY`. Auth skipped when `SMTP_USER=""`
|
||||
- EasyRSA called with `--batch`; `--passin=pass:<passphrase>` omitted when the resolved passphrase is empty
|
||||
- EasyRSA error text arrives on **stdout**, not stderr: its `print()` is `printf '%s\n'`, and both `die()` and `user_error()` route through it. stderr only carries output from the tools EasyRSA shells out to (openssl), and even that is silenced under `-S/--silent-ssl` (not passed here). `_easyrsa_diagnostics()` therefore merges both streams — building an error from stderr alone reports failures as blank
|
||||
- CA passphrase resolution (`resolve_ca_passphrase()`, run once in `main()` right after arg parsing, before dispatch): `CA_PASSPHRASE=""` → auto-detect via `is_ca_key_encrypted()` (checks `<PKI_DIR>/private/ca.key` PEM header for `ENCRYPTED`) and prompt only if encrypted; `"!empty"` → never check/prompt, passphrase is `""`; `"!ask"` → always prompt, skip detection; any other value → used literally. `--list`/`--list-all` skip this resolution entirely since they only read `index.txt` and never touch the CA
|
||||
- Certificate lifetime (`resolve_cert_days()`, run once in `main()` right after the config overrides are applied — before the `--list` early return, since it validates config and never prompts): `CERT_DAYS = "default"` or `""` → omit `--days` entirely and let EasyRSA's own `EASYRSA_CERT_EXPIRE` (from `vars`) decide; a positive number → passed as `--days=N`, normalised so `"090"` becomes `"90"`. `0`, negatives and non-numeric values raise `ConfigError` — a fast-fail mirror of EasyRSA's own gate (`Cannot use --days=0 for command build-client-full`), not a substitute for it. `--days N` overrides `CERT_DAYS` for one CLI run; the TUI's Days field overrides it per certificate. The flag is added in `build_client_full()` only, never in `_base_cmd()` — `gen-crl` reads `--days` as CRL validity
|
||||
- Cryptgeon: matches the `occulto` browser client — `key=os.urandom(32)` used directly (no derivation) for AES-256-GCM; `contents` = `base64(b"AES-GCM") + "--" + base64(nonce) + "--" + base64(ciphertext)`; `meta` = JSON string `{"type": "text"}`; URL = `<base>/note/<id>#<key.hex()>`
|
||||
- `copy_crl()` does `chmod 644` after copy, then runs `RESTORECON_BINARY` (default `restorecon`) on the copied file — best-effort like `is_ca_key_encrypted()`: a missing/misconfigured binary is swallowed, not fatal. Set `RESTORECON_BINARY=""` to disable on non-SELinux systems
|
||||
- Password: pos 1=uppercase, pos 2=lowercase (no j), pos 3-27=alphanumeric, pos 28=lowercase (no j); `oO01lIQ5S2Z8B` banned everywhere
|
||||
|
||||
940
docs/superpowers/plans/2026-08-15-cert-days.md
Normal file
940
docs/superpowers/plans/2026-08-15-cert-days.md
Normal file
@@ -0,0 +1,940 @@
|
||||
# Configurable Certificate Lifetime Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Let the operator choose how long an issued certificate is valid, via a `CERT_DAYS` setting, a `--days` CLI flag, and a Days field in the TUI cert form — while leaving today's behaviour unchanged for anyone who doesn't set it.
|
||||
|
||||
**Architecture:** One resolver, `resolve_cert_days()`, turns every input path (SETTINGS constant, external `.conf`, CLI flag, TUI field) into either `""` (omit the flag, let EasyRSA's own `EASYRSA_CERT_EXPIRE` win) or a normalised decimal string. `build_client_full()` appends `--days=N` to its own argument list when that string is non-empty. The CLI resolves once in `main()` and overwrites the module global, exactly as `CA_PASSPHRASE` already does; the TUI carries a per-cert value out of the form instead.
|
||||
|
||||
**Tech Stack:** Python 3.9, stdlib `argparse` and `curses`, existing `ConfigError` / `_base_cmd()` / `InputField` / `show_cert_form()` in `openvpncertupdate.py`. Tests via `pytest` + `unittest.mock`, following the mock-curses patterns already in `tests/test_dialogs.py` and `tests/test_widgets.py`.
|
||||
|
||||
**Design doc:** `docs/superpowers/specs/2026-08-15-cert-days-design.md`
|
||||
|
||||
## Global Constraints
|
||||
|
||||
- Single-file script: all production code stays in `openvpncertupdate.py`; no new modules.
|
||||
- `"default"` and `""` both mean *inherit from EasyRSA* — deploying this must never silently shorten certificates on an existing PKI. Do not change the shipped value to a number.
|
||||
- `--days` goes in `build_client_full()`'s own argument list. **Never** in `_base_cmd()` — that helper is shared with `gen-crl`, where `--days` sets CRL validity instead (`easyrsa:7270`).
|
||||
- Rejecting `0` locally is a fast-fail mirror of EasyRSA's own gate (`easyrsa:5701`), not a substitute for it. Do not add a `--days=0` code path expecting EasyRSA to accept it.
|
||||
- Leading zeros are normalised (`"090"` → `"90"`) because EasyRSA's own validation pattern `*[!1234567890]*|0*` (`easyrsa:7178`) rejects them with a message that makes no sense to someone who typed only digits.
|
||||
- Follow existing conventions: `CliRunner` reads module globals directly (`EASYRSA_DIR`, `EASYRSA_PKI_DIR`, `CA_PASSPHRASE`) rather than threading them as parameters.
|
||||
- Every task ends with the full suite green: `python3 -m pytest tests/ -q`.
|
||||
|
||||
**Two deliberate deviations from the spec:**
|
||||
|
||||
1. The spec's data-flow sketch has `CliRunner.create()`/`.reissue()` take a `days` parameter. This plan instead resolves into the module global `CERT_DAYS` in `main()`. Reason: it matches the established `CA_PASSPHRASE` pattern exactly, avoids threading one value through three call layers, and leaves the existing `test_main_dispatches_*` signature assertions intact. Behaviour is identical.
|
||||
2. The spec says the TUI shows "the `resolve_cert_days()` message" on the hint line. The form window is 62 columns wide, leaving 58 for the hint, and the resolver's full sentence is longer than that — it would be truncated mid-word. The form still *validates* with `resolve_cert_days()`, so what counts as valid never diverges, but renders its own short message.
|
||||
|
||||
---
|
||||
|
||||
## File Structure
|
||||
|
||||
- **Modify `openvpncertupdate.py`:**
|
||||
- SETTINGS: add `CERT_DAYS` constant; add it to `_OVERRIDABLE_SETTINGS`.
|
||||
- EASYRSA section: add `resolve_cert_days()` beside `resolve_ca_passphrase()`; add a `days` parameter to `build_client_full()`.
|
||||
- TUI WIDGETS: add an `allowed` charset parameter to `InputField`.
|
||||
- TUI DIALOGS: add a `days` field to `CertFormResult` and `show_cert_form()`; grow the window; validate on submit.
|
||||
- APP: `_process_cert()` seeds the form from `CERT_DAYS` and forwards `form.days`.
|
||||
- CLI: add `--days` to `_build_parser()`; resolve in `main()`; `_issue()` passes the global.
|
||||
- **Modify `tests/test_easyrsa.py`:** `resolve_cert_days()` table; `build_client_full` flag presence/absence; `--days` absent from revoke/gen-crl commands.
|
||||
- **Modify `tests/test_widgets.py`:** `InputField` charset filtering.
|
||||
- **Modify `tests/test_dialogs.py`:** Days field behaviour; **fix two existing tests whose keypress counts assume three fields**.
|
||||
- **Modify `tests/test_cli.py`:** `--days` parsing and dispatch.
|
||||
- **Modify `tests/test_config_file.py`:** `CERT_DAYS` survives a `.conf` override.
|
||||
- **Modify `CLAUDE.md`:** flags table, SETTINGS mention, key constraints.
|
||||
|
||||
---
|
||||
|
||||
### Task 1: `CERT_DAYS` setting and `resolve_cert_days()`
|
||||
|
||||
**Files:**
|
||||
- Modify: `openvpncertupdate.py` (SETTINGS block ~line 75; `_OVERRIDABLE_SETTINGS` ~line 81; EASYRSA section, after `resolve_ca_passphrase()` ~line 380)
|
||||
- Test: `tests/test_easyrsa.py`, `tests/test_config_file.py`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: `ConfigError` (defined in the SETTINGS OVERRIDE section).
|
||||
- Produces: `resolve_cert_days(configured, name: str = "CERT_DAYS") -> str` — returns `""` or a normalised decimal string; raises `ConfigError`. Module constant `CERT_DAYS: str`.
|
||||
|
||||
- [ ] **Step 1: Write the failing tests**
|
||||
|
||||
Add to `tests/test_easyrsa.py`, importing `resolve_cert_days` and `ConfigError` alongside the existing imports:
|
||||
|
||||
```python
|
||||
# ---------------------------------------------------------------------------
|
||||
# resolve_cert_days
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@pytest.mark.parametrize("configured", ["default", "", " ", " default "])
|
||||
def test_resolve_cert_days_sentinels_mean_inherit(configured):
|
||||
# "" and "default" leave the lifetime to EasyRSA's own EASYRSA_CERT_EXPIRE,
|
||||
# so deploying this cannot silently shorten certs on an existing PKI.
|
||||
assert resolve_cert_days(configured) == ""
|
||||
|
||||
|
||||
@pytest.mark.parametrize("configured,expected", [
|
||||
(90, "90"), ("90", "90"), (" 90 ", "90"), (1, "1"), (3650, "3650"),
|
||||
])
|
||||
def test_resolve_cert_days_accepts_positive_numbers(configured, expected):
|
||||
assert resolve_cert_days(configured) == expected
|
||||
|
||||
|
||||
def test_resolve_cert_days_normalises_leading_zeros():
|
||||
# EasyRSA's own pattern rejects "090" with "Number expected", which reads
|
||||
# as nonsense to someone who typed only digits.
|
||||
assert resolve_cert_days("090") == "90"
|
||||
|
||||
|
||||
def test_resolve_cert_days_rejects_zero():
|
||||
with pytest.raises(ConfigError, match="positive"):
|
||||
resolve_cert_days("0")
|
||||
|
||||
|
||||
@pytest.mark.parametrize("configured", ["-5", -5, "abc", "9 0", "90.5", 90.5, None])
|
||||
def test_resolve_cert_days_rejects_junk(configured):
|
||||
with pytest.raises(ConfigError):
|
||||
resolve_cert_days(configured)
|
||||
|
||||
|
||||
def test_resolve_cert_days_error_names_the_source():
|
||||
# The same resolver serves CERT_DAYS, --days and the TUI field; the message
|
||||
# must say which one the user actually touched.
|
||||
with pytest.raises(ConfigError, match="--days"):
|
||||
resolve_cert_days("0", "--days")
|
||||
```
|
||||
|
||||
Add to `tests/test_config_file.py`:
|
||||
|
||||
```python
|
||||
def test_cert_days_is_overridable(tmp_path):
|
||||
conf = tmp_path / "app.conf"
|
||||
conf.write_text('CERT_DAYS = 90\n')
|
||||
out = load_settings_overrides(str(conf), "", str(tmp_path / "app.py"))
|
||||
assert out["CERT_DAYS"] == 90
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run tests to verify they fail**
|
||||
|
||||
Run: `python3 -m pytest tests/test_easyrsa.py -k cert_days tests/test_config_file.py -q`
|
||||
Expected: FAIL — `ImportError: cannot import name 'resolve_cert_days'`, and `KeyError: 'CERT_DAYS'`.
|
||||
|
||||
- [ ] **Step 3: Add the setting**
|
||||
|
||||
In the SETTINGS block, directly after `DAYS_AHEAD = 14`:
|
||||
|
||||
```python
|
||||
CERT_DAYS = "default" # lifetime of certs this tool issues, in days.
|
||||
# "default" or "" = leave it to EasyRSA (your vars'
|
||||
# EASYRSA_CERT_EXPIRE). A positive integer overrides
|
||||
# it — e.g. 90.
|
||||
```
|
||||
|
||||
In `_OVERRIDABLE_SETTINGS`, change the `DAYS_PAST` line to:
|
||||
|
||||
```python
|
||||
"DAYS_PAST", "DAYS_AHEAD", "CERT_DAYS",
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Add the resolver**
|
||||
|
||||
In the EASYRSA section, directly after `resolve_ca_passphrase()`:
|
||||
|
||||
```python
|
||||
def resolve_cert_days(configured, name: str = "CERT_DAYS") -> str:
|
||||
"""Resolve a configured certificate lifetime into an EasyRSA --days value.
|
||||
|
||||
"default" / "" → "" (omit --days; EasyRSA's EASYRSA_CERT_EXPIRE wins)
|
||||
positive number → that many days, normalised ("090" → "90")
|
||||
|
||||
Rejects 0, negatives and non-numeric input. EasyRSA rejects those itself,
|
||||
but failing here keeps the error next to the input the user actually typed
|
||||
— in the TUI that means the form is still open, and on the CLI it means no
|
||||
CA passphrase prompt and no subprocess round-trip first.
|
||||
|
||||
`name` names the source ("CERT_DAYS", "--days", "Days") for the message.
|
||||
"""
|
||||
text = str(configured).strip()
|
||||
if text in ("", "default"):
|
||||
return ""
|
||||
try:
|
||||
days = int(text)
|
||||
except ValueError:
|
||||
raise ConfigError(
|
||||
f'{name} must be a positive number of days, "default" or "" '
|
||||
f'(inherit from EasyRSA); got {configured!r}'
|
||||
)
|
||||
if days <= 0:
|
||||
raise ConfigError(
|
||||
f"{name} must be a positive number of days "
|
||||
f"(EasyRSA rejects 0); got {configured!r}"
|
||||
)
|
||||
return str(days)
|
||||
```
|
||||
|
||||
Note `str(configured)` first: it makes `int`, `str` and `float` inputs behave uniformly, and `str(90.5)` → `"90.5"` → `ValueError`, so a float is rejected rather than silently truncated. `str(None)` → `"None"` → `ValueError`, which is also what we want.
|
||||
|
||||
- [ ] **Step 5: Run tests to verify they pass**
|
||||
|
||||
Run: `python3 -m pytest tests/test_easyrsa.py -k cert_days tests/test_config_file.py -q`
|
||||
Expected: PASS
|
||||
|
||||
- [ ] **Step 6: Run the full suite**
|
||||
|
||||
Run: `python3 -m pytest tests/ -q`
|
||||
Expected: PASS, no regressions.
|
||||
|
||||
- [ ] **Step 7: Commit**
|
||||
|
||||
```bash
|
||||
git add openvpncertupdate.py tests/test_easyrsa.py tests/test_config_file.py
|
||||
git commit -m "Add CERT_DAYS setting and resolve_cert_days()"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 2: Pass `--days` to `build-client-full`
|
||||
|
||||
**Files:**
|
||||
- Modify: `openvpncertupdate.py` — `build_client_full()` (EASYRSA section, ~line 315)
|
||||
- Test: `tests/test_easyrsa.py`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: `_base_cmd()`, `_run_easyrsa()`, `resolve_cert_days()` output format from Task 1.
|
||||
- Produces: `build_client_full(easyrsa_dir, pki_dir, cn, key_passphrase, ca_passphrase, email="", days="") -> None`.
|
||||
|
||||
- [ ] **Step 1: Write the failing tests**
|
||||
|
||||
Add to `tests/test_easyrsa.py`:
|
||||
|
||||
```python
|
||||
@patch("openvpncertupdate.subprocess.run")
|
||||
def test_build_client_full_includes_days_when_set(mock_run):
|
||||
mock_run.return_value = ok_result()
|
||||
build_client_full("/er", "/pki", "bob", "keypass", "capass", days="90")
|
||||
assert "--days=90" in mock_run.call_args[0][0]
|
||||
|
||||
|
||||
@patch("openvpncertupdate.subprocess.run")
|
||||
def test_build_client_full_omits_days_when_inheriting(mock_run):
|
||||
mock_run.return_value = ok_result()
|
||||
build_client_full("/er", "/pki", "bob", "keypass", "capass", days="")
|
||||
assert not any(a.startswith("--days") for a in mock_run.call_args[0][0])
|
||||
|
||||
|
||||
@patch("openvpncertupdate.subprocess.run")
|
||||
def test_build_client_full_days_precedes_the_verb(mock_run):
|
||||
# --days is a global option: EasyRSA parses it before the command word.
|
||||
mock_run.return_value = ok_result()
|
||||
build_client_full("/er", "/pki", "bob", "keypass", "capass", days="90")
|
||||
args = mock_run.call_args[0][0]
|
||||
assert args.index("--days=90") < args.index("build-client-full")
|
||||
|
||||
|
||||
@patch("openvpncertupdate.subprocess.run")
|
||||
def test_revoke_and_gen_crl_never_carry_days(mock_run):
|
||||
# _base_cmd() is shared with gen-crl, where --days means CRL validity.
|
||||
# Putting the flag there would quietly change how long CRLs are valid.
|
||||
mock_run.return_value = ok_result()
|
||||
revoke_issued("/er", "/pki", "alice", "capass")
|
||||
assert not any(a.startswith("--days") for a in mock_run.call_args[0][0])
|
||||
gen_crl("/er", "/pki", "capass")
|
||||
assert not any(a.startswith("--days") for a in mock_run.call_args[0][0])
|
||||
|
||||
|
||||
@patch("openvpncertupdate.subprocess.run")
|
||||
def test_error_verb_detection_survives_days_flag(mock_run):
|
||||
# _run_easyrsa picks the verb as the first non-flag arg; "--days=90"
|
||||
# must not be mistaken for it.
|
||||
mock_run.return_value = err_result(stdout="Error\nboom", stderr="")
|
||||
with pytest.raises(EasyRSAError, match="build-client-full"):
|
||||
build_client_full("/er", "/pki", "bob", "keypass", "capass", days="90")
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run tests to verify they fail**
|
||||
|
||||
Run: `python3 -m pytest tests/test_easyrsa.py -k days -q`
|
||||
Expected: FAIL — `TypeError: build_client_full() got an unexpected keyword argument 'days'`.
|
||||
|
||||
- [ ] **Step 3: Add the parameter**
|
||||
|
||||
Replace `build_client_full()` entirely:
|
||||
|
||||
```python
|
||||
def build_client_full(
|
||||
easyrsa_dir: str, pki_dir: str, cn: str,
|
||||
key_passphrase: str, ca_passphrase: str, email: str = "", days: str = "",
|
||||
) -> None:
|
||||
extra_env = {"EASYRSA_REQ_EMAIL": email} if email else None
|
||||
_run_easyrsa(
|
||||
_base_cmd(easyrsa_dir, pki_dir, ca_passphrase)
|
||||
+ [f"--passout=pass:{key_passphrase}"]
|
||||
# Global option, so it must precede the verb. Deliberately not in
|
||||
# _base_cmd(): gen-crl reads --days as CRL validity instead.
|
||||
+ ([f"--days={days}"] if days else [])
|
||||
+ ["build-client-full", cn],
|
||||
cwd=easyrsa_dir,
|
||||
extra_env=extra_env,
|
||||
)
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run tests to verify they pass**
|
||||
|
||||
Run: `python3 -m pytest tests/test_easyrsa.py -q`
|
||||
Expected: PASS
|
||||
|
||||
- [ ] **Step 5: Run the full suite**
|
||||
|
||||
Run: `python3 -m pytest tests/ -q`
|
||||
Expected: PASS
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add openvpncertupdate.py tests/test_easyrsa.py
|
||||
git commit -m "Pass --days to build-client-full when a lifetime is set"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 3: `--days` CLI flag
|
||||
|
||||
**Files:**
|
||||
- Modify: `openvpncertupdate.py` — `_build_parser()` (~line 1559), `main()` (~line 1593), `CliRunner._issue()` (~line 1425)
|
||||
- Test: `tests/test_cli.py`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: `resolve_cert_days()` (Task 1), `build_client_full(days=...)` (Task 2).
|
||||
- Produces: `args.days` (`Optional[str]`, default `None`); module global `CERT_DAYS` holding the resolved value by the time any `CliRunner` method runs.
|
||||
|
||||
- [ ] **Step 1: Write the failing tests**
|
||||
|
||||
Add to `tests/test_cli.py`:
|
||||
|
||||
```python
|
||||
# ---------------------------------------------------------------------------
|
||||
# --days
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_days_flag_parses():
|
||||
args = _build_parser().parse_args(["--create", "alice", "--email", "a@b.c",
|
||||
"--days", "90"])
|
||||
assert args.days == "90"
|
||||
|
||||
|
||||
def test_days_defaults_to_none_when_absent():
|
||||
args = _build_parser().parse_args(["--create", "alice", "--email", "a@b.c"])
|
||||
assert args.days is None
|
||||
|
||||
|
||||
def test_issue_passes_resolved_days_to_build_client_full(monkeypatch, capsys):
|
||||
mocks = _patch_issue(monkeypatch)
|
||||
monkeypatch.setattr("openvpncertupdate.CERT_DAYS", "90")
|
||||
CliRunner().create("alice", "alice@example.com")
|
||||
assert mocks["build_client_full"].call_args.kwargs["days"] == "90"
|
||||
|
||||
|
||||
def test_issue_passes_empty_days_when_inheriting(monkeypatch, capsys):
|
||||
mocks = _patch_issue(monkeypatch)
|
||||
monkeypatch.setattr("openvpncertupdate.CERT_DAYS", "")
|
||||
CliRunner().create("alice", "alice@example.com")
|
||||
assert mocks["build_client_full"].call_args.kwargs["days"] == ""
|
||||
|
||||
|
||||
def test_main_days_flag_overrides_cert_days(monkeypatch):
|
||||
import openvpncertupdate
|
||||
monkeypatch.setattr(sys, "argv",
|
||||
["prog", "--create", "alice", "--email", "a@b.c", "--days", "30"])
|
||||
monkeypatch.setattr("openvpncertupdate.CERT_DAYS", "default")
|
||||
monkeypatch.setattr("openvpncertupdate.resolve_ca_passphrase", MagicMock(return_value=""))
|
||||
runner = MagicMock()
|
||||
monkeypatch.setattr("openvpncertupdate.CliRunner", MagicMock(return_value=runner))
|
||||
main()
|
||||
assert openvpncertupdate.CERT_DAYS == "30"
|
||||
|
||||
|
||||
def test_main_resolves_cert_days_when_no_flag(monkeypatch):
|
||||
import openvpncertupdate
|
||||
monkeypatch.setattr(sys, "argv", ["prog", "--create", "alice", "--email", "a@b.c"])
|
||||
monkeypatch.setattr("openvpncertupdate.CERT_DAYS", "090")
|
||||
monkeypatch.setattr("openvpncertupdate.resolve_ca_passphrase", MagicMock(return_value=""))
|
||||
monkeypatch.setattr("openvpncertupdate.CliRunner", MagicMock(return_value=MagicMock()))
|
||||
main()
|
||||
assert openvpncertupdate.CERT_DAYS == "90"
|
||||
|
||||
|
||||
def test_main_rejects_bad_days_flag(monkeypatch, capsys):
|
||||
monkeypatch.setattr(sys, "argv",
|
||||
["prog", "--create", "alice", "--email", "a@b.c", "--days", "0"])
|
||||
monkeypatch.setattr("openvpncertupdate.resolve_ca_passphrase", MagicMock(return_value=""))
|
||||
with pytest.raises(SystemExit):
|
||||
main()
|
||||
assert "--days" in capsys.readouterr().err
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run tests to verify they fail**
|
||||
|
||||
Run: `python3 -m pytest tests/test_cli.py -k days -q`
|
||||
Expected: FAIL — `AttributeError: 'Namespace' object has no attribute 'days'`.
|
||||
|
||||
- [ ] **Step 3: Add the flag**
|
||||
|
||||
In `_build_parser()`, directly after the `--email` argument:
|
||||
|
||||
```python
|
||||
parser.add_argument("--days", metavar="N",
|
||||
help="Certificate lifetime in days for --create/--reissue "
|
||||
"(overrides CERT_DAYS; omit to use CERT_DAYS)")
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Resolve it in `main()`**
|
||||
|
||||
In `main()`, directly after `globals().update(overrides)` and **before** the `if args.list:` early return — this validates the config on every path, and unlike the CA passphrase it never prompts:
|
||||
|
||||
```python
|
||||
global CERT_DAYS
|
||||
try:
|
||||
CERT_DAYS = (resolve_cert_days(args.days, "--days") if args.days is not None
|
||||
else resolve_cert_days(CERT_DAYS))
|
||||
except ConfigError as exc:
|
||||
print(f"error: {exc}", file=sys.stderr)
|
||||
sys.exit(1)
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Forward it from `_issue()`**
|
||||
|
||||
In `CliRunner._issue()`, change the `build_client_full` call to:
|
||||
|
||||
```python
|
||||
build_client_full(EASYRSA_DIR, EASYRSA_PKI_DIR, cn, password, CA_PASSPHRASE,
|
||||
email=email_addr, days=CERT_DAYS)
|
||||
```
|
||||
|
||||
- [ ] **Step 6: Run tests to verify they pass**
|
||||
|
||||
Run: `python3 -m pytest tests/test_cli.py -q`
|
||||
Expected: PASS
|
||||
|
||||
- [ ] **Step 7: Run the full suite**
|
||||
|
||||
Run: `python3 -m pytest tests/ -q`
|
||||
Expected: PASS
|
||||
|
||||
- [ ] **Step 8: Commit**
|
||||
|
||||
```bash
|
||||
git add openvpncertupdate.py tests/test_cli.py
|
||||
git commit -m "Add --days CLI flag overriding CERT_DAYS"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 4: `InputField` charset restriction
|
||||
|
||||
**Files:**
|
||||
- Modify: `openvpncertupdate.py` — `InputField.__init__()` and `.handle_key()` (TUI WIDGETS, ~line 647)
|
||||
- Test: `tests/test_widgets.py`
|
||||
|
||||
**Interfaces:**
|
||||
- Produces: `InputField(win, y, x, width, initial="", mask=False, allowed=None)`. When `allowed` is a string, `handle_key()` inserts only characters found in it; when `None`, behaviour is unchanged.
|
||||
|
||||
- [ ] **Step 1: Write the failing tests**
|
||||
|
||||
Add to `tests/test_widgets.py`, using whatever mock-window helper that file already defines for `InputField`:
|
||||
|
||||
```python
|
||||
def test_allowed_charset_accepts_listed_characters():
|
||||
f = InputField(MagicMock(), 0, 0, 10, initial="", allowed="0123456789")
|
||||
for ch in "90":
|
||||
f.handle_key(ord(ch))
|
||||
assert f.value == "90"
|
||||
|
||||
|
||||
def test_allowed_charset_drops_other_characters():
|
||||
f = InputField(MagicMock(), 0, 0, 10, initial="", allowed="0123456789")
|
||||
for ch in "9a0-!":
|
||||
f.handle_key(ord(ch))
|
||||
assert f.value == "90"
|
||||
|
||||
|
||||
def test_allowed_none_accepts_everything():
|
||||
f = InputField(MagicMock(), 0, 0, 10, initial="")
|
||||
for ch in "a-1!":
|
||||
f.handle_key(ord(ch))
|
||||
assert f.value == "a-1!"
|
||||
|
||||
|
||||
def test_allowed_charset_still_supports_editing_keys():
|
||||
f = InputField(MagicMock(), 0, 0, 10, initial="90", allowed="0123456789")
|
||||
f.handle_key(_curses.KEY_BACKSPACE)
|
||||
assert f.value == "9"
|
||||
```
|
||||
|
||||
If `tests/test_widgets.py` does not already import `MagicMock` or bind `_curses`, add those imports to match the file's existing style.
|
||||
|
||||
- [ ] **Step 2: Run tests to verify they fail**
|
||||
|
||||
Run: `python3 -m pytest tests/test_widgets.py -k allowed -q`
|
||||
Expected: FAIL — `TypeError: __init__() got an unexpected keyword argument 'allowed'`.
|
||||
|
||||
- [ ] **Step 3: Add the parameter**
|
||||
|
||||
Change `InputField.__init__()`'s signature and add one attribute:
|
||||
|
||||
```python
|
||||
def __init__(
|
||||
self, win, y: int, x: int, width: int,
|
||||
initial: str = "", mask: bool = False, allowed: Optional[str] = None,
|
||||
) -> None:
|
||||
self._win = win
|
||||
self._y = y
|
||||
self._x = x
|
||||
self._width = width
|
||||
self._mask = mask
|
||||
self._allowed = allowed
|
||||
self._buf = list(initial)
|
||||
self._cur = len(self._buf)
|
||||
```
|
||||
|
||||
Change the printable-character branch at the end of `handle_key()`:
|
||||
|
||||
```python
|
||||
elif 32 <= key <= 126:
|
||||
ch = chr(key)
|
||||
if self._allowed is not None and ch not in self._allowed:
|
||||
return
|
||||
self._buf.insert(self._cur, ch)
|
||||
self._cur += 1
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run tests to verify they pass**
|
||||
|
||||
Run: `python3 -m pytest tests/test_widgets.py -q`
|
||||
Expected: PASS
|
||||
|
||||
- [ ] **Step 5: Run the full suite**
|
||||
|
||||
Run: `python3 -m pytest tests/ -q`
|
||||
Expected: PASS
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add openvpncertupdate.py tests/test_widgets.py
|
||||
git commit -m "Add optional charset restriction to InputField"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 5: Days field in the cert form
|
||||
|
||||
**Files:**
|
||||
- Modify: `openvpncertupdate.py` — `CertFormResult`, `_FORM_FIELDS`, `_FORM_LABELS`, `_FORM_FIELD_Y`, `show_cert_form()` (TUI DIALOGS, ~line 738-903)
|
||||
- Test: `tests/test_dialogs.py`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: `InputField(allowed=...)` (Task 4), `resolve_cert_days()` (Task 1), `ConfigError`.
|
||||
- Produces: `CertFormResult(cn, email, password, confirmed, days="")`; `show_cert_form(stdscr, cn="", email="", password="", days="", cn_readonly=False)`.
|
||||
|
||||
**Watch out:** two existing tests hard-code keypress counts that assume three fields. Both must change in this task or the suite breaks.
|
||||
|
||||
- [ ] **Step 1: Fix the two existing tests for a fourth field**
|
||||
|
||||
In `tests/test_dialogs.py`, `test_show_cert_form_confirm` — the field list becomes `cn, email, days, password`, so reaching Continue takes one more Enter:
|
||||
|
||||
```python
|
||||
def test_show_cert_form_confirm():
|
||||
"""Enter advances through the 4 fields to the Continue button; Enter on
|
||||
Continue confirms. That's 5 Enter keypresses total."""
|
||||
stdscr = _make_stdscr()
|
||||
win = _make_win(rows=20, cols=70)
|
||||
win.getch.side_effect = [10, 10, 10, 10, 10]
|
||||
with patch("curses.newwin", return_value=win):
|
||||
result = show_cert_form(stdscr, cn="bob", email="bob@example.com")
|
||||
assert isinstance(result, CertFormResult)
|
||||
assert result.confirmed is True
|
||||
assert result.cn == "bob"
|
||||
assert result.email == "bob@example.com"
|
||||
assert len(result.password) > 0
|
||||
```
|
||||
|
||||
And `test_show_cert_form_cancel_button`:
|
||||
|
||||
```python
|
||||
def test_show_cert_form_cancel_button():
|
||||
"""Tab to the Cancel button (5 Tabs from start) and press Enter cancels."""
|
||||
stdscr = _make_stdscr()
|
||||
win = _make_win(rows=20, cols=70)
|
||||
# Tab×5: cn→email→days→password→Continue→Cancel, then Enter
|
||||
win.getch.side_effect = [9, 9, 9, 9, 9, 10]
|
||||
with patch("curses.newwin", return_value=win):
|
||||
result = show_cert_form(stdscr, cn="alice", email="alice@example.com")
|
||||
assert isinstance(result, CertFormResult)
|
||||
assert result.confirmed is False
|
||||
```
|
||||
|
||||
In `test_show_cert_form_clamps_to_narrow_screen`, update the docstring only — `13x62` becomes `15x62`. The assertions still hold.
|
||||
|
||||
- [ ] **Step 2: Write the failing tests for the new behaviour**
|
||||
|
||||
Add to `tests/test_dialogs.py`:
|
||||
|
||||
```python
|
||||
def test_show_cert_form_returns_days():
|
||||
stdscr = _make_stdscr()
|
||||
win = _make_win(rows=20, cols=70)
|
||||
win.getch.side_effect = [10, 10, 10, 10, 10]
|
||||
with patch("curses.newwin", return_value=win):
|
||||
result = show_cert_form(stdscr, cn="bob", email="b@c.d", days="90")
|
||||
assert result.confirmed is True
|
||||
assert result.days == "90"
|
||||
|
||||
|
||||
def test_show_cert_form_blank_days_means_inherit():
|
||||
stdscr = _make_stdscr()
|
||||
win = _make_win(rows=20, cols=70)
|
||||
win.getch.side_effect = [10, 10, 10, 10, 10]
|
||||
with patch("curses.newwin", return_value=win):
|
||||
result = show_cert_form(stdscr, cn="bob", email="b@c.d", days="")
|
||||
assert result.confirmed is True
|
||||
assert result.days == ""
|
||||
|
||||
|
||||
def test_show_cert_form_days_field_is_digits_only():
|
||||
# cn is read-only here, so focus starts on email: Tab once to reach days.
|
||||
stdscr = _make_stdscr()
|
||||
win = _make_win(rows=20, cols=70)
|
||||
win.getch.side_effect = [9] + [ord(c) for c in "9a0!"] + [10, 10, 10]
|
||||
with patch("curses.newwin", return_value=win):
|
||||
result = show_cert_form(stdscr, cn="bob", email="b@c.d", days="",
|
||||
cn_readonly=True)
|
||||
assert result.confirmed is True
|
||||
assert result.days == "90"
|
||||
|
||||
|
||||
def test_show_cert_form_refuses_zero_days():
|
||||
# Submitting "0" must keep the form open rather than closing and failing
|
||||
# later inside EasyRSA. Enter×5 tries to submit, Esc then cancels.
|
||||
stdscr = _make_stdscr()
|
||||
win = _make_win(rows=20, cols=70)
|
||||
win.getch.side_effect = [10, 10, 10, 10, 10, 27]
|
||||
with patch("curses.newwin", return_value=win):
|
||||
result = show_cert_form(stdscr, cn="bob", email="b@c.d", days="0")
|
||||
assert result.confirmed is False
|
||||
|
||||
|
||||
def test_show_cert_form_normalises_leading_zero_days():
|
||||
stdscr = _make_stdscr()
|
||||
win = _make_win(rows=20, cols=70)
|
||||
win.getch.side_effect = [10, 10, 10, 10, 10]
|
||||
with patch("curses.newwin", return_value=win):
|
||||
result = show_cert_form(stdscr, cn="bob", email="b@c.d", days="090")
|
||||
assert result.days == "90"
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Run tests to verify they fail**
|
||||
|
||||
Run: `python3 -m pytest tests/test_dialogs.py -q`
|
||||
Expected: FAIL — `TypeError: show_cert_form() got an unexpected keyword argument 'days'`, plus the two rewritten tests failing on keypress counts.
|
||||
|
||||
- [ ] **Step 4: Add the field to the dataclass and layout constants**
|
||||
|
||||
```python
|
||||
@dataclass
|
||||
class CertFormResult:
|
||||
cn: str
|
||||
email: str
|
||||
password: str
|
||||
confirmed: bool
|
||||
days: str = ""
|
||||
|
||||
|
||||
_FORM_FIELDS = ("cn", "email", "days", "password")
|
||||
_FORM_LABELS = {
|
||||
"cn": "CN",
|
||||
"email": "Email",
|
||||
"days": "Days (blank = EasyRSA default)",
|
||||
"password": "Password",
|
||||
}
|
||||
_FORM_FIELD_Y = {"cn": 3, "email": 5, "days": 7, "password": 9}
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Grow the window and move the buttons**
|
||||
|
||||
In `show_cert_form()`, change the signature, the window height and the button row:
|
||||
|
||||
```python
|
||||
def show_cert_form(
|
||||
stdscr,
|
||||
cn: str = "",
|
||||
email: str = "",
|
||||
password: str = "",
|
||||
days: str = "",
|
||||
cn_readonly: bool = False,
|
||||
) -> CertFormResult:
|
||||
```
|
||||
|
||||
```python
|
||||
h, w = min(15, sh), min(62, sw)
|
||||
```
|
||||
|
||||
```python
|
||||
_BTN_Y = 11
|
||||
```
|
||||
|
||||
The `curses.error` fallback near the top returns a cancelled result; leave it as is — `days` defaults to `""`.
|
||||
|
||||
- [ ] **Step 6: Add the field and submit validation**
|
||||
|
||||
Add the days field to the `fields` dict:
|
||||
|
||||
```python
|
||||
fields: Dict[str, InputField] = {
|
||||
"cn": InputField(win, _FORM_FIELD_Y["cn"], 3, fw, initial=cn),
|
||||
"email": InputField(win, _FORM_FIELD_Y["email"], 3, fw, initial=email),
|
||||
"days": InputField(win, _FORM_FIELD_Y["days"], 3, fw, initial=days,
|
||||
allowed=string.digits),
|
||||
"password": InputField(win, _FORM_FIELD_Y["password"], 3, fw,
|
||||
initial=password if password else generate_password(),
|
||||
mask=True),
|
||||
}
|
||||
focus = 0
|
||||
error = ""
|
||||
```
|
||||
|
||||
Replace `_submit()`. It now returns `None` when the input is bad, leaving the form open:
|
||||
|
||||
```python
|
||||
def _submit() -> Optional[CertFormResult]:
|
||||
nonlocal error
|
||||
try:
|
||||
# Validate with the same resolver the CLI and .conf use, so what
|
||||
# counts as valid never diverges between entry points.
|
||||
days_value = resolve_cert_days(fields["days"].value, "Days")
|
||||
except ConfigError:
|
||||
# The resolver's full sentence does not fit a 62-column window.
|
||||
error = "Days must be a positive number, or blank to inherit"
|
||||
return None
|
||||
error = ""
|
||||
final_cn = cn if cn_readonly else fields["cn"].value
|
||||
return CertFormResult(
|
||||
cn=final_cn,
|
||||
email=fields["email"].value,
|
||||
password=fields["password"].value,
|
||||
days=days_value,
|
||||
confirmed=True,
|
||||
)
|
||||
```
|
||||
|
||||
`string` is already imported at the top of the script (the password generator uses it); confirm with `grep -n "^import string" openvpncertupdate.py` and add it if absent.
|
||||
|
||||
- [ ] **Step 7: Show the error and handle the two submit sites**
|
||||
|
||||
Replace the hint block:
|
||||
|
||||
```python
|
||||
hint = "Tab=next F5=regen pwd Ctrl-G=confirm Esc=cancel"
|
||||
hint_attr = curses.color_pair(COLOR_DISABLED)
|
||||
if error:
|
||||
hint, hint_attr = error, curses.color_pair(COLOR_ERROR)
|
||||
try:
|
||||
win.addstr(h - 2, 2, hint[:w - 4], hint_attr)
|
||||
except curses.error:
|
||||
pass
|
||||
```
|
||||
|
||||
Replace the Ctrl-G handler:
|
||||
|
||||
```python
|
||||
if key == 0x07: # Ctrl-G: immediate submit
|
||||
result = _submit()
|
||||
if result is not None:
|
||||
curses.curs_set(0)
|
||||
return result
|
||||
continue
|
||||
```
|
||||
|
||||
And the Continue-button branch:
|
||||
|
||||
```python
|
||||
if key in (10, 13, curses.KEY_ENTER, ord(" ")):
|
||||
if current == _BTN_CONTINUE:
|
||||
result = _submit()
|
||||
if result is not None:
|
||||
curses.curs_set(0)
|
||||
return result
|
||||
continue
|
||||
curses.curs_set(0)
|
||||
return CertFormResult(cn=cn, email="", password="", confirmed=False)
|
||||
```
|
||||
|
||||
- [ ] **Step 8: Run tests to verify they pass**
|
||||
|
||||
Run: `python3 -m pytest tests/test_dialogs.py -q`
|
||||
Expected: PASS
|
||||
|
||||
- [ ] **Step 9: Run the full suite**
|
||||
|
||||
Run: `python3 -m pytest tests/ -q`
|
||||
Expected: PASS
|
||||
|
||||
- [ ] **Step 10: Commit**
|
||||
|
||||
```bash
|
||||
git add openvpncertupdate.py tests/test_dialogs.py
|
||||
git commit -m "Add Days field to the TUI cert form"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 6: Wire the TUI form through to EasyRSA
|
||||
|
||||
**Files:**
|
||||
- Modify: `openvpncertupdate.py` — `CursesApp._process_cert()` (APP section, ~line 1165-1210)
|
||||
- Test: `tests/test_app_reissue.py`
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: `show_cert_form(days=...)` and `CertFormResult.days` (Task 5), `build_client_full(days=...)` (Task 2), module global `CERT_DAYS` (Task 1).
|
||||
|
||||
- [ ] **Step 1: Write the failing tests**
|
||||
|
||||
Add to `tests/test_app_reissue.py`:
|
||||
|
||||
```python
|
||||
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_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"
|
||||
```
|
||||
|
||||
`_patch_workflow` already patches `show_cert_form`; the second test replaces that patch with one returning a `days` value. Import `CertFormResult` at the top of the file alongside `CursesApp`.
|
||||
|
||||
- [ ] **Step 2: Run tests to verify they fail**
|
||||
|
||||
Run: `python3 -m pytest tests/test_app_reissue.py -k days -q`
|
||||
Expected: FAIL — `KeyError: 'days'` on the `show_cert_form` kwargs, and on `build_client_full`'s.
|
||||
|
||||
- [ ] **Step 3: Seed the form and forward the result**
|
||||
|
||||
In `_process_cert()`, change the form call:
|
||||
|
||||
```python
|
||||
form = show_cert_form(stdscr, cn=cn, email=email, days=CERT_DAYS,
|
||||
cn_readonly=is_renewal)
|
||||
```
|
||||
|
||||
And the build call:
|
||||
|
||||
```python
|
||||
build_client_full(EASYRSA_DIR, EASYRSA_PKI_DIR,
|
||||
final_cn, form.password, CA_PASSPHRASE,
|
||||
email=form.email, days=form.days)
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run tests to verify they pass**
|
||||
|
||||
Run: `python3 -m pytest tests/test_app_reissue.py -q`
|
||||
Expected: PASS
|
||||
|
||||
- [ ] **Step 5: Run the full suite**
|
||||
|
||||
Run: `python3 -m pytest tests/ -q`
|
||||
Expected: PASS
|
||||
|
||||
- [ ] **Step 6: Verify by mutation**
|
||||
|
||||
Revert each of the two edits in turn (restore the original call without `days=`), confirm the matching test goes red, then restore.
|
||||
|
||||
Run: `python3 -m pytest tests/ -q`
|
||||
Expected: PASS after restoring.
|
||||
|
||||
- [ ] **Step 7: Commit**
|
||||
|
||||
```bash
|
||||
git add openvpncertupdate.py tests/test_app_reissue.py
|
||||
git commit -m "Wire the TUI Days field through to build-client-full"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 7: Documentation
|
||||
|
||||
**Files:**
|
||||
- Modify: `CLAUDE.md` — CLI flags table (~line 32), Running section (~line 20), Key constraints (~line 88)
|
||||
|
||||
**Interfaces:**
|
||||
- Consumes: everything above. No code changes.
|
||||
|
||||
- [ ] **Step 1: Add the flag to the CLI flags table**
|
||||
|
||||
After the `--email EMAIL` row:
|
||||
|
||||
```markdown
|
||||
| `--days N` | Certificate lifetime in days for `--create`/`--reissue`; overrides `CERT_DAYS` for that run |
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Add an example to the Running section**
|
||||
|
||||
After the existing `--reissue` example:
|
||||
|
||||
```bash
|
||||
python3 openvpncertupdate.py --create CN --email user@example.com --days 90
|
||||
```
|
||||
|
||||
- [ ] **Step 3: Add a key constraint**
|
||||
|
||||
After the CA-passphrase bullet:
|
||||
|
||||
```markdown
|
||||
- Certificate lifetime (`resolve_cert_days()`, run once in `main()` right after the config overrides are applied — before the `--list` early return, since it validates config and never prompts): `CERT_DAYS = "default"` or `""` → omit `--days` entirely and let EasyRSA's own `EASYRSA_CERT_EXPIRE` (from `vars`) decide; a positive number → passed as `--days=N`, normalised so `"090"` becomes `"90"`. `0`, negatives and non-numeric values raise `ConfigError` — a fast-fail mirror of EasyRSA's own gate (`Cannot use --days=0 for command build-client-full`), not a substitute for it. `--days N` overrides `CERT_DAYS` for one CLI run; the TUI's Days field overrides it per certificate. The flag is added in `build_client_full()` only, never in `_base_cmd()` — `gen-crl` reads `--days` as CRL validity
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Update the section symbol table**
|
||||
|
||||
In the "File layout" table, replace the EASYRSA row with:
|
||||
|
||||
```markdown
|
||||
| EASYRSA | `EasyRSAError`, `_easyrsa_diagnostics()`, `issued_cert_path()`, `has_issued_cert()`, `revoke_issued()`, `build_client_full()`, `gen_crl()`, `copy_crl()`, `is_ca_key_encrypted()`, `resolve_ca_passphrase()`, `resolve_cert_days()` |
|
||||
```
|
||||
|
||||
Leave the TUI WIDGETS row alone — `InputField` is already listed and its new `allowed` parameter needs no separate entry.
|
||||
|
||||
- [ ] **Step 5: Run the full suite one last time**
|
||||
|
||||
Run: `python3 -m pytest tests/ -q`
|
||||
Expected: PASS
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add CLAUDE.md
|
||||
git commit -m "Document CERT_DAYS and --days"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Manual verification after Task 7
|
||||
|
||||
The test suite never invokes EasyRSA. Confirm the flag actually reaches it, on a machine with the PKI:
|
||||
|
||||
```bash
|
||||
python3 openvpncertupdate.py --create testcert --email you@example.com --days 90 --no-send-email
|
||||
openssl x509 -in <PKI_DIR>/issued/testcert.crt -noout -dates
|
||||
```
|
||||
|
||||
`notAfter` minus `notBefore` should be 90 days. Then revoke the test cert:
|
||||
|
||||
```bash
|
||||
python3 openvpncertupdate.py --revoke testcert
|
||||
```
|
||||
122
docs/superpowers/specs/2026-08-15-cert-days-design.md
Normal file
122
docs/superpowers/specs/2026-08-15-cert-days-design.md
Normal file
@@ -0,0 +1,122 @@
|
||||
# Configurable Certificate Lifetime — Design
|
||||
|
||||
**Goal:** Let the operator choose how long an issued certificate is valid — as a setting (`CERT_DAYS`), a CLI flag (`--days`), and a field in the TUI cert form — while leaving today's behaviour untouched for anyone who doesn't set it.
|
||||
|
||||
**Status:** approved 2026-08-15, ready for an implementation plan.
|
||||
|
||||
## Background
|
||||
|
||||
`build_client_full()` passes nothing about validity to EasyRSA today, so every certificate inherits `EASYRSA_CERT_EXPIRE` from the `vars` file (365 days on the deployment that prompted this). There is no way to issue a shorter-lived cert without editing `vars`, which changes it for every tool that touches the PKI.
|
||||
|
||||
## Decisions
|
||||
|
||||
### The setting
|
||||
|
||||
```python
|
||||
CERT_DAYS = "default" # lifetime of certs this tool issues, in days.
|
||||
# "default" or "" = leave it to EasyRSA (your vars'
|
||||
# EASYRSA_CERT_EXPIRE). A positive integer overrides
|
||||
# it — e.g. 90.
|
||||
```
|
||||
|
||||
`"default"` and `""` both mean *inherit*. This is deliberate: deploying the new version must not silently shorten certificates on an existing PKI. Anyone who wants a fixed lifetime sets a number.
|
||||
|
||||
`CERT_DAYS` joins `_OVERRIDABLE_SETTINGS`, so an external `.conf` can set it like any other setting. The repo ships no sample `.conf` (`CONFIG_PATH` defaults to `<script>.conf` next to the script), so "in the external config" means *overridable and documented*, not a new file.
|
||||
|
||||
### `resolve_cert_days()`
|
||||
|
||||
Lives in the EASYRSA section beside `resolve_ca_passphrase()` and follows the same sentinel-resolution shape. Called once in `main()` after argument parsing.
|
||||
|
||||
| Input | Result |
|
||||
|---|---|
|
||||
| `"default"`, `""` | `""` — omit the flag, EasyRSA decides |
|
||||
| `90`, `"90"` | `"90"` |
|
||||
| `"090"` | `"90"` — normalised, see below |
|
||||
| `0`, `"0"` | `ConfigError` |
|
||||
| `-5`, `"abc"`, `"9 0"` | `ConfigError` |
|
||||
|
||||
It accepts both `int` and `str` because people write either in a `.conf`.
|
||||
|
||||
Two behaviours need justifying:
|
||||
|
||||
**Rejecting `0` is a fast-fail mirror of EasyRSA's own rule, not a substitute for it.** EasyRSA already refuses: `--days=0` passes option parsing (`zero_allowed=1`, `easyrsa:6955`) and then hits an explicit gate at `easyrsa:5701` — `Cannot use --days=0 for command build-client-full`. Checking locally only moves the failure somewhere cheaper: in the TUI, to the still-open form instead of after it closes; on the CLI, to before the CA passphrase prompt and a subprocess round-trip. The message says why:
|
||||
|
||||
```
|
||||
CERT_DAYS must be a positive number of days (EasyRSA rejects 0); got "0"
|
||||
```
|
||||
|
||||
**Normalising leading zeros avoids a baffling error.** EasyRSA's validation pattern is `*[!1234567890]*|0*` (`easyrsa:7178`), so `--days=090` fails with `Number expected: '090'` — an unhelpful thing to read when you typed nothing but digits. `int()` then `str()` sidesteps it.
|
||||
|
||||
### Passing it to EasyRSA
|
||||
|
||||
Use the **`--days=N` global option**, appended in `build_client_full()`'s own argument list:
|
||||
|
||||
```python
|
||||
_base_cmd(...) + [f"--passout=pass:{key_passphrase}"]
|
||||
+ ([f"--days={days}"] if days else [])
|
||||
+ ["build-client-full", cn]
|
||||
```
|
||||
|
||||
Rejected alternative: exporting `EASYRSA_CERT_EXPIRE` through `extra_env`, which `build_client_full()` already uses for the email. `set_var` is `export X="${X-$default}"` (`easyrsa:6170`), so it does respect an existing environment value — but a `vars` file that uses a plain `EASYRSA_CERT_EXPIRE=365` assignment instead of `set_var` overwrites it silently. `--days` has no such failure mode: EasyRSA exports it at command dispatch, after `vars` is sourced. It is also the guarded path — `EASYRSA_CERT_EXPIRE=0` reaches `openssl ca -days 0` with no gate (`easyrsa:2738`), while `--days=0` is refused.
|
||||
|
||||
**The flag must not go in `_base_cmd()`.** That helper is shared with `gen-crl`, where `--days` sets CRL validity instead (`easyrsa:7270`).
|
||||
|
||||
`_run_easyrsa()`'s verb detection (`next(c for c in cmd[1:] if not c.startswith("-"))`) is unaffected: `--days=90` starts with `-`, so `build-client-full` is still found.
|
||||
|
||||
### CLI
|
||||
|
||||
`--days N`, a non-exclusive argument alongside `--email`. Overrides `CERT_DAYS` for that run of `--create` or `--reissue`. Parsed through `resolve_cert_days()` so a bad value reads the same however it arrives.
|
||||
|
||||
Like `--email`, it is accepted-but-unused with `--revoke`, `--gen-crl` and `--list*` rather than being an error — consistent with the existing parser, which does not police flag combinations beyond its one mutually-exclusive group.
|
||||
|
||||
`CliRunner.create()` and `.reissue()` gain a `days: str = ""` parameter, forwarded to `_issue()` and on to `build_client_full()`.
|
||||
|
||||
### TUI
|
||||
|
||||
The cert form grows a fourth field, in order **CN, Email, Days, Password**. Fields move to y=3,5,7,9; buttons to y=11; the window grows from 13 to 15 rows, which still fits an 80x24 terminal. `CertFormResult` gains `days: str`.
|
||||
|
||||
The Days field is **digits-only**: `InputField` gains an optional `allowed: Optional[str] = None` charset that filters insertions in `handle_key()` — pure logic, directly testable, no curses needed. Empty means inherit, so `"default"` never has to be typed.
|
||||
|
||||
That leaves `0` (and `00`, …) as the only reachable invalid input. Submitting it shows the `resolve_cert_days()` message on the hint line in `COLOR_ERROR` and keeps the form open, rather than closing the form and failing later.
|
||||
|
||||
The field is seeded from the resolved `CERT_DAYS`: blank when inheriting, otherwise the number.
|
||||
|
||||
## Data flow
|
||||
|
||||
```
|
||||
CERT_DAYS (SETTINGS) ──┐
|
||||
.conf override ──┤
|
||||
├─► resolve_cert_days() ─► "" | "90"
|
||||
--days N (CLI) ──┘ │
|
||||
├─► CliRunner._issue()
|
||||
TUI form "Days" field ─► resolve_cert_days() ──────────┤
|
||||
└─► build_client_full(days=…)
|
||||
│
|
||||
--days=90 ──┘ (omitted when "")
|
||||
```
|
||||
|
||||
## Error handling
|
||||
|
||||
| Failure | Where it surfaces |
|
||||
|---|---|
|
||||
| Bad `CERT_DAYS` in `.conf` or SETTINGS | `ConfigError` in `main()`, printed and exit 1 — same path as other config errors |
|
||||
| Bad `--days` on the CLI | `ConfigError` at parse time, printed and exit 1 |
|
||||
| `0` typed in the TUI form | Hint line in `COLOR_ERROR`, form stays open |
|
||||
| Anything EasyRSA still rejects | `EasyRSAError` with EasyRSA's own text, now legible via `_easyrsa_diagnostics()` |
|
||||
|
||||
## Testing
|
||||
|
||||
- `resolve_cert_days()` table: valid, both sentinels, `0`, negative, junk, `int` vs `str`, leading zeros.
|
||||
- `build_client_full()` includes `--days=90` when given, omits it when `""`, and keeps `--days` out of `_base_cmd()` (assert `revoke_issued`/`gen_crl` commands carry no `--days`).
|
||||
- `--days` parses and reaches `create`/`reissue`; a bad value exits 1.
|
||||
- `CERT_DAYS` survives an external `.conf` override (extend the existing `_OVERRIDABLE_SETTINGS` test).
|
||||
- `InputField` drops characters outside `allowed` and keeps existing behaviour when `allowed is None`.
|
||||
- `show_cert_form()` returns `days`, seeds the field from the setting, and refuses to submit `0`.
|
||||
|
||||
Each verified by mutation — revert the logic, confirm the intended tests go red — as with the two preceding changes on this branch.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Changing the lifetime of an existing certificate (that is `easyrsa renew`, a different workflow).
|
||||
- CA lifetime (`EASYRSA_CA_EXPIRE`) and CRL validity (`--days` for `gen-crl`).
|
||||
- Warning when `CERT_DAYS` is shorter than `DAYS_AHEAD`, which would make every new cert appear in the expiring list immediately. Worth revisiting if it bites in practice.
|
||||
@@ -75,6 +75,10 @@ SMTP_TLS = "starttls" # "starttls" | "ssl" | "" (plain)
|
||||
|
||||
DAYS_PAST = 30
|
||||
DAYS_AHEAD = 14
|
||||
CERT_DAYS = "default" # lifetime of certs this tool issues, in days.
|
||||
# "default" or "" = leave it to EasyRSA (your vars'
|
||||
# EASYRSA_CERT_EXPIRE). A positive integer overrides
|
||||
# it — e.g. 90.
|
||||
|
||||
# Names an external .conf file is allowed to override — keep in sync with the
|
||||
# SETTINGS block above. Deliberately excludes CONFIG_PATH itself.
|
||||
@@ -85,7 +89,7 @@ _OVERRIDABLE_SETTINGS = frozenset({
|
||||
"CRYPTGEON_URL",
|
||||
"MAIL_FROM", "MAIL_SUBJECT", "EMAIL_TEMPLATE_PATH", "MAIL_BINARY",
|
||||
"SMTP_HOST", "SMTP_PORT", "SMTP_USER", "SMTP_PASSWORD", "SMTP_TLS",
|
||||
"DAYS_PAST", "DAYS_AHEAD",
|
||||
"DAYS_PAST", "DAYS_AHEAD", "CERT_DAYS",
|
||||
})
|
||||
|
||||
|
||||
@@ -133,6 +137,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 +202,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(
|
||||
@@ -261,6 +273,33 @@ class EasyRSAError(Exception):
|
||||
pass
|
||||
|
||||
|
||||
def _easyrsa_diagnostics(stdout: str, stderr: str) -> str:
|
||||
"""Merge both streams of a failed EasyRSA run into displayable error text.
|
||||
|
||||
EasyRSA's print() is `printf '%s\\n'`, so die() and user_error() write their
|
||||
diagnostics to *stdout*; stderr only carries output from the tools EasyRSA
|
||||
shells out to (openssl). Reporting stderr alone therefore leaves most
|
||||
failures with an empty message. Blank padding and the version banner around
|
||||
EasyRSA's error block are dropped so the text fits the TUI error dialog.
|
||||
|
||||
stderr comes first: when EasyRSA dies at the openssl step its stdout also
|
||||
carries the warn() banner and show_host() footer, so the specific cause
|
||||
(openssl's own message) must lead or it is what the dialog clips.
|
||||
"""
|
||||
lines = []
|
||||
for stream in (stderr, stdout):
|
||||
for raw in (stream or "").splitlines():
|
||||
line = raw.rstrip()
|
||||
if not line.strip():
|
||||
continue
|
||||
if line.startswith("EasyRSA version ") or set(line.strip()) in (
|
||||
{"-"}, {"="},
|
||||
):
|
||||
continue
|
||||
lines.append(line)
|
||||
return "\n".join(lines)
|
||||
|
||||
|
||||
def _run_easyrsa(cmd: List[str], cwd: str,
|
||||
extra_env: Optional[Dict[str, str]] = None) -> None:
|
||||
env = None
|
||||
@@ -274,8 +313,9 @@ def _run_easyrsa(cmd: List[str], cwd: str,
|
||||
if result.returncode != 0:
|
||||
# Find the first non-flag argument after the binary (the actual easyrsa verb)
|
||||
verb = next((c for c in cmd[1:] if not c.startswith("-")), "unknown")
|
||||
detail = _easyrsa_diagnostics(result.stdout, result.stderr)
|
||||
raise EasyRSAError(
|
||||
f"{verb} failed (exit {result.returncode}): {result.stderr.strip()}"
|
||||
f"{verb} failed (exit {result.returncode}):\n{detail}"
|
||||
)
|
||||
|
||||
|
||||
@@ -286,6 +326,36 @@ def _base_cmd(easyrsa_dir: str, pki_dir: str, ca_passphrase: str) -> List[str]:
|
||||
return cmd
|
||||
|
||||
|
||||
def issued_cert_path(pki_dir: str, cn: str) -> str:
|
||||
return f"{pki_dir}/issued/{cn}.crt"
|
||||
|
||||
|
||||
def has_issued_cert(pki_dir: str, cn: str) -> bool:
|
||||
"""Whether EasyRSA still holds the signed certificate for this CN.
|
||||
|
||||
index.txt can carry a V-status line whose .crt is gone — e.g. an index
|
||||
copied over from an older EasyRSA install without the issued/ files.
|
||||
EasyRSA reads the serial out of the .crt itself, so `revoke-issued` fails
|
||||
outright on those CNs; the re-issue workflow skips the revoke step instead.
|
||||
"""
|
||||
return os.path.isfile(issued_cert_path(pki_dir, cn))
|
||||
|
||||
|
||||
def _leftover_build_paths(pki_dir: str, cn: str) -> List[str]:
|
||||
"""Which of pki/reqs/<CN>.req and pki/private/<CN>.key still exist.
|
||||
|
||||
`build-client-full` aborts outright if either is present — EasyRSA
|
||||
refuses to overwrite them. Normally `revoke-issued` clears both by
|
||||
archiving them into pki/revoked/, but that step is skipped when there is
|
||||
no issued cert to revoke (see has_issued_cert()), so a re-issue that
|
||||
takes the skip path must check for these leftovers itself before calling
|
||||
build-client-full, rather than let the confusing EasyRSA abort surface
|
||||
after the CA passphrase prompt.
|
||||
"""
|
||||
candidates = [f"{pki_dir}/reqs/{cn}.req", f"{pki_dir}/private/{cn}.key"]
|
||||
return [p for p in candidates if os.path.isfile(p)]
|
||||
|
||||
|
||||
def revoke_issued(
|
||||
easyrsa_dir: str, pki_dir: str, cn: str, ca_passphrase: str
|
||||
) -> None:
|
||||
@@ -297,12 +367,16 @@ def revoke_issued(
|
||||
|
||||
def build_client_full(
|
||||
easyrsa_dir: str, pki_dir: str, cn: str,
|
||||
key_passphrase: str, ca_passphrase: str, email: str = "",
|
||||
key_passphrase: str, ca_passphrase: str, email: str = "", days: str = "",
|
||||
) -> None:
|
||||
extra_env = {"EASYRSA_REQ_EMAIL": email} if email else None
|
||||
_run_easyrsa(
|
||||
_base_cmd(easyrsa_dir, pki_dir, ca_passphrase)
|
||||
+ [f"--passout=pass:{key_passphrase}", "build-client-full", cn],
|
||||
+ [f"--passout=pass:{key_passphrase}"]
|
||||
# Global option, so it must precede the verb. Deliberately not in
|
||||
# _base_cmd(): gen-crl reads --days as CRL validity instead.
|
||||
+ ([f"--days={days}"] if days else [])
|
||||
+ ["build-client-full", cn],
|
||||
cwd=easyrsa_dir,
|
||||
extra_env=extra_env,
|
||||
)
|
||||
@@ -362,6 +436,37 @@ def resolve_ca_passphrase(
|
||||
return configured
|
||||
|
||||
|
||||
def resolve_cert_days(configured, name: str = "CERT_DAYS") -> str:
|
||||
"""Resolve a configured certificate lifetime into an EasyRSA --days value.
|
||||
|
||||
"default" / "" → "" (omit --days; EasyRSA's EASYRSA_CERT_EXPIRE wins)
|
||||
positive number → that many days, normalised ("090" → "90")
|
||||
|
||||
Rejects 0, negatives and non-numeric input. EasyRSA rejects those itself,
|
||||
but failing here keeps the error next to the input the user actually typed
|
||||
— in the TUI that means the form is still open, and on the CLI it means no
|
||||
CA passphrase prompt and no subprocess round-trip first.
|
||||
|
||||
`name` names the source ("CERT_DAYS", "--days", "Days") for the message.
|
||||
"""
|
||||
text = str(configured).strip()
|
||||
if text in ("", "default"):
|
||||
return ""
|
||||
try:
|
||||
days = int(text)
|
||||
except ValueError:
|
||||
raise ConfigError(
|
||||
f'{name} must be a positive number of days, "default" or "" '
|
||||
f'(inherit from EasyRSA); got {configured!r}'
|
||||
)
|
||||
if days <= 0:
|
||||
raise ConfigError(
|
||||
f"{name} must be a positive number of days "
|
||||
f"(EasyRSA rejects 0); got {configured!r}"
|
||||
)
|
||||
return str(days)
|
||||
|
||||
|
||||
# ============================================================
|
||||
# === CONFIG ===
|
||||
# ============================================================
|
||||
@@ -599,15 +704,16 @@ class InputField:
|
||||
|
||||
def __init__(
|
||||
self, win, y: int, x: int, width: int,
|
||||
initial: str = "", mask: bool = False,
|
||||
initial: str = "", mask: bool = False, allowed: Optional[str] = None,
|
||||
) -> None:
|
||||
self._win = win
|
||||
self._y = y
|
||||
self._x = x
|
||||
self._width = width
|
||||
self._mask = mask
|
||||
self._buf = list(initial)
|
||||
self._cur = len(self._buf)
|
||||
self._win = win
|
||||
self._y = y
|
||||
self._x = x
|
||||
self._width = width
|
||||
self._mask = mask
|
||||
self._allowed = allowed
|
||||
self._buf = list(initial)
|
||||
self._cur = len(self._buf)
|
||||
|
||||
@property
|
||||
def value(self) -> str:
|
||||
@@ -647,7 +753,10 @@ class InputField:
|
||||
if self._cur < len(self._buf):
|
||||
del self._buf[self._cur]
|
||||
elif 32 <= key <= 126:
|
||||
self._buf.insert(self._cur, chr(key))
|
||||
ch = chr(key)
|
||||
if self._allowed is not None and ch not in self._allowed:
|
||||
return
|
||||
self._buf.insert(self._cur, ch)
|
||||
self._cur += 1
|
||||
|
||||
|
||||
@@ -697,15 +806,17 @@ class CertFormResult:
|
||||
email: str
|
||||
password: str
|
||||
confirmed: bool
|
||||
days: str = ""
|
||||
|
||||
|
||||
_FORM_FIELDS = ("cn", "email", "password")
|
||||
_FORM_FIELDS = ("cn", "email", "days", "password")
|
||||
_FORM_LABELS = {
|
||||
"cn": "CN",
|
||||
"email": "Email",
|
||||
"days": "Days (blank = EasyRSA default)",
|
||||
"password": "Password",
|
||||
}
|
||||
_FORM_FIELD_Y = {"cn": 3, "email": 5, "password": 7}
|
||||
_FORM_FIELD_Y = {"cn": 3, "email": 5, "days": 7, "password": 9}
|
||||
|
||||
|
||||
def show_cert_form(
|
||||
@@ -713,6 +824,7 @@ def show_cert_form(
|
||||
cn: str = "",
|
||||
email: str = "",
|
||||
password: str = "",
|
||||
days: str = "",
|
||||
cn_readonly: bool = False,
|
||||
) -> CertFormResult:
|
||||
"""Modal cert-detail form.
|
||||
@@ -722,7 +834,7 @@ def show_cert_form(
|
||||
# Clamp to the screen so curses.newwin() can't raise "curses function
|
||||
# returned NULL" outright on a terminal narrower/shorter than the usual
|
||||
# 80x24 (e.g. a serial console with an unusually small viewport).
|
||||
h, w = min(13, sh), min(62, sw)
|
||||
h, w = min(15, sh), min(62, sw)
|
||||
try:
|
||||
win = curses.newwin(h, w, max(0, (sh - h) // 2), max(0, (sw - w) // 2))
|
||||
except curses.error:
|
||||
@@ -732,7 +844,7 @@ def show_cert_form(
|
||||
|
||||
_BTN_CONTINUE = "_continue"
|
||||
_BTN_CANCEL = "_cancel"
|
||||
_BTN_Y = 9
|
||||
_BTN_Y = 11
|
||||
_btn_x_cont = (w - 26) // 2 # "[ Continue ]"=12 + gap=4 + "[ Cancel ]"=10 = 26
|
||||
_btn_x_canc = _btn_x_cont + 16
|
||||
|
||||
@@ -741,18 +853,32 @@ def show_cert_form(
|
||||
fields: Dict[str, InputField] = {
|
||||
"cn": InputField(win, _FORM_FIELD_Y["cn"], 3, fw, initial=cn),
|
||||
"email": InputField(win, _FORM_FIELD_Y["email"], 3, fw, initial=email),
|
||||
"days": InputField(win, _FORM_FIELD_Y["days"], 3, fw, initial=days,
|
||||
allowed=string.digits),
|
||||
"password": InputField(win, _FORM_FIELD_Y["password"], 3, fw,
|
||||
initial=password if password else generate_password(),
|
||||
mask=True),
|
||||
}
|
||||
focus = 0
|
||||
error = ""
|
||||
|
||||
def _submit() -> CertFormResult:
|
||||
def _submit() -> Optional[CertFormResult]:
|
||||
nonlocal error
|
||||
try:
|
||||
# Validate with the same resolver the CLI and .conf use, so what
|
||||
# counts as valid never diverges between entry points.
|
||||
days_value = resolve_cert_days(fields["days"].value, "Days")
|
||||
except ConfigError:
|
||||
# The resolver's full sentence does not fit a 62-column window.
|
||||
error = "Days must be a positive number, or blank to inherit"
|
||||
return None
|
||||
error = ""
|
||||
final_cn = cn if cn_readonly else fields["cn"].value
|
||||
return CertFormResult(
|
||||
cn=final_cn,
|
||||
email=fields["email"].value,
|
||||
password=fields["password"].value,
|
||||
days=days_value,
|
||||
confirmed=True,
|
||||
)
|
||||
|
||||
@@ -793,8 +919,11 @@ def show_cert_form(
|
||||
pass
|
||||
|
||||
hint = "Tab=next F5=regen pwd Ctrl-G=confirm Esc=cancel"
|
||||
hint_attr = curses.color_pair(COLOR_DISABLED)
|
||||
if error:
|
||||
hint, hint_attr = error, curses.color_pair(COLOR_ERROR)
|
||||
try:
|
||||
win.addstr(h - 2, 2, hint[:w - 4], curses.color_pair(COLOR_DISABLED))
|
||||
win.addstr(h - 2, 2, hint[:w - 4], hint_attr)
|
||||
except curses.error:
|
||||
pass
|
||||
|
||||
@@ -819,8 +948,11 @@ def show_cert_form(
|
||||
curses.curs_set(0)
|
||||
return CertFormResult(cn=cn, email="", password="", confirmed=False)
|
||||
if key == 0x07: # Ctrl-G: immediate submit
|
||||
curses.curs_set(0)
|
||||
return _submit()
|
||||
result = _submit()
|
||||
if result is not None:
|
||||
curses.curs_set(0)
|
||||
return result
|
||||
continue
|
||||
if key in (9, curses.KEY_DOWN): # Tab / Down: next
|
||||
focus = (focus + 1) % len(active)
|
||||
continue
|
||||
@@ -840,9 +972,13 @@ def show_cert_form(
|
||||
)
|
||||
continue
|
||||
if key in (10, 13, curses.KEY_ENTER, ord(" ")):
|
||||
curses.curs_set(0)
|
||||
if current == _BTN_CONTINUE:
|
||||
return _submit()
|
||||
result = _submit()
|
||||
if result is not None:
|
||||
curses.curs_set(0)
|
||||
return result
|
||||
continue
|
||||
curses.curs_set(0)
|
||||
return CertFormResult(cn=cn, email="", password="", confirmed=False)
|
||||
else:
|
||||
if key in (10, 13, curses.KEY_ENTER):
|
||||
@@ -951,7 +1087,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:
|
||||
@@ -1119,19 +1259,49 @@ class CursesApp:
|
||||
self, stdscr, cn: str, email: str, is_renewal: bool,
|
||||
) -> bool:
|
||||
"""Form → generate → deliver. Returns False if user cancelled."""
|
||||
form = show_cert_form(stdscr, cn=cn, email=email, cn_readonly=is_renewal)
|
||||
form = show_cert_form(stdscr, cn=cn, email=email, days=CERT_DAYS,
|
||||
cn_readonly=is_renewal)
|
||||
if not form.confirmed:
|
||||
return False
|
||||
|
||||
final_cn = form.cn
|
||||
self._msg(stdscr, f"Generating certificate for {final_cn}…")
|
||||
|
||||
if is_renewal:
|
||||
revoked = False
|
||||
if is_renewal and not has_issued_cert(EASYRSA_PKI_DIR, final_cn):
|
||||
# No unknown-CN check here (unlike CliRunner._issue): final_cn is
|
||||
# cn_readonly in this form, always the CN the row was opened
|
||||
# for, which is only ever populated from _load_current_certs()
|
||||
# (see show_main_screen()/RENEW_SELECTED) — a typo'd CN can't
|
||||
# reach this branch through the TUI.
|
||||
leftovers = _leftover_build_paths(EASYRSA_PKI_DIR, final_cn)
|
||||
if leftovers:
|
||||
self._error(
|
||||
stdscr,
|
||||
f"Cannot skip revoke for {final_cn} — build-client-full "
|
||||
f"would abort:\n" + "\n".join(leftovers) + "\n\n"
|
||||
f"revoke-issued normally archives these; since there is "
|
||||
f"no issued cert to revoke, move or remove them manually "
|
||||
f"(this tool won't touch a private key), then retry.",
|
||||
)
|
||||
return True
|
||||
# index.txt lists the cert but the .crt is gone, so there is
|
||||
# nothing EasyRSA can revoke. Issue the replacement anyway.
|
||||
warning = (
|
||||
f"No certificate file for {final_cn} at\n"
|
||||
f"{issued_cert_path(EASYRSA_PKI_DIR, final_cn)}\n\n"
|
||||
f"Nothing to revoke — skipping revoke and CRL update.\n"
|
||||
f"Issuing a new certificate only."
|
||||
)
|
||||
self._log(f"{final_cn}: no issued cert file, revoke skipped")
|
||||
self._msg(stdscr, warning, wait=True)
|
||||
elif is_renewal:
|
||||
try:
|
||||
revoke_issued(EASYRSA_DIR, EASYRSA_PKI_DIR, final_cn, CA_PASSPHRASE)
|
||||
except EasyRSAError as exc:
|
||||
self._error(stdscr, f"EasyRSA error during revoke:\n{exc}")
|
||||
return True
|
||||
revoked = True
|
||||
# The old cert is now revoked, so the published CRL is stale
|
||||
# until regenerated — do that now rather than leaving it to a
|
||||
# separate manual "Regenerate CRL" step. Not fatal: the new
|
||||
@@ -1150,9 +1320,9 @@ class CursesApp:
|
||||
try:
|
||||
build_client_full(EASYRSA_DIR, EASYRSA_PKI_DIR,
|
||||
final_cn, form.password, CA_PASSPHRASE,
|
||||
email=form.email)
|
||||
email=form.email, days=form.days)
|
||||
except EasyRSAError as exc:
|
||||
if is_renewal:
|
||||
if revoked:
|
||||
self._error(
|
||||
stdscr,
|
||||
f"EasyRSA error during build-client-full:\n{exc}\n\n"
|
||||
@@ -1309,18 +1479,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)
|
||||
|
||||
|
||||
@@ -1370,13 +1555,53 @@ class CliRunner:
|
||||
send_email_flag: bool = True, show_eml: bool = False) -> None:
|
||||
password = generate_password()
|
||||
|
||||
if is_renewal:
|
||||
revoked = False
|
||||
if is_renewal and not has_issued_cert(EASYRSA_PKI_DIR, cn):
|
||||
# A CN with no issued .crt only takes the skip path (below) if
|
||||
# index.txt still knows it — that's the migration-gap case this
|
||||
# branch exists for. Anything else (a typo'd/nonexistent CN) must
|
||||
# not fall through to issuing a certificate nobody asked for.
|
||||
try:
|
||||
known_cns = {c.cn for c in _load_current_certs(EASYRSA_PKI_DIR)}
|
||||
except FileNotFoundError as exc:
|
||||
print(f"error: cannot read PKI index.txt: {exc}", file=sys.stderr)
|
||||
sys.exit(1)
|
||||
if cn not in known_cns:
|
||||
print(
|
||||
f"error: unknown CN {cn!r} — no entry in index.txt and no "
|
||||
f"issued certificate; nothing to reissue.",
|
||||
file=sys.stderr,
|
||||
)
|
||||
sys.exit(1)
|
||||
leftovers = _leftover_build_paths(EASYRSA_PKI_DIR, cn)
|
||||
if leftovers:
|
||||
print(
|
||||
f"error: cannot skip revoke for {cn} — "
|
||||
f"build-client-full would abort: " + " and ".join(leftovers) +
|
||||
" already exist. revoke-issued normally archives these; "
|
||||
"since there is no issued cert to revoke, move or remove "
|
||||
"them manually (this tool won't touch a private key), "
|
||||
"then retry.",
|
||||
file=sys.stderr,
|
||||
)
|
||||
sys.exit(1)
|
||||
# index.txt lists the cert but the .crt is gone, so there is
|
||||
# nothing EasyRSA can revoke. Issue the replacement anyway.
|
||||
print(
|
||||
f"warning: no certificate file at "
|
||||
f"{issued_cert_path(EASYRSA_PKI_DIR, cn)}\n"
|
||||
f"warning: nothing to revoke for {cn} — skipping revoke and CRL "
|
||||
f"update, issuing a new certificate only.",
|
||||
file=sys.stderr,
|
||||
)
|
||||
elif is_renewal:
|
||||
print(f"Revoking existing cert for {cn}…", file=sys.stderr)
|
||||
try:
|
||||
revoke_issued(EASYRSA_DIR, EASYRSA_PKI_DIR, cn, CA_PASSPHRASE)
|
||||
except EasyRSAError as exc:
|
||||
print(f"error during revoke: {exc}", file=sys.stderr)
|
||||
sys.exit(1)
|
||||
revoked = True
|
||||
# The old cert is now revoked, so the published CRL is stale
|
||||
# until regenerated — do that now rather than leaving it to a
|
||||
# separate --gen-crl run. Not fatal: the new cert is more
|
||||
@@ -1396,9 +1621,9 @@ class CliRunner:
|
||||
print(f"Building certificate for {cn}…", file=sys.stderr)
|
||||
try:
|
||||
build_client_full(EASYRSA_DIR, EASYRSA_PKI_DIR, cn, password, CA_PASSPHRASE,
|
||||
email=email_addr)
|
||||
email=email_addr, days=CERT_DAYS)
|
||||
except EasyRSAError as exc:
|
||||
if is_renewal:
|
||||
if revoked:
|
||||
print(
|
||||
f"error during build-client-full: {exc}\n"
|
||||
f"WARNING: {cn} has been revoked but no new cert was built.\n"
|
||||
@@ -1477,6 +1702,9 @@ def _build_parser() -> argparse.ArgumentParser:
|
||||
group.add_argument("--list-all", action="store_true", help="List all CNs with email")
|
||||
parser.add_argument("--email", metavar="EMAIL",
|
||||
help="Email address (required for --create; optional for --reissue)")
|
||||
parser.add_argument("--days", metavar="N",
|
||||
help="Certificate lifetime in days for --create/--reissue "
|
||||
"(overrides CERT_DAYS; omit to use CERT_DAYS)")
|
||||
parser.add_argument("--send-email", dest="send_email", action="store_const", const=True,
|
||||
default=None, help="Send email after issuing cert (default unless --show-eml)")
|
||||
parser.add_argument("--no-send-email", dest="send_email", action="store_const", const=False,
|
||||
@@ -1506,6 +1734,14 @@ def main() -> None:
|
||||
sys.exit(1)
|
||||
globals().update(overrides)
|
||||
|
||||
global CERT_DAYS
|
||||
try:
|
||||
CERT_DAYS = (resolve_cert_days(args.days, "--days") if args.days is not None
|
||||
else resolve_cert_days(CERT_DAYS))
|
||||
except ConfigError as exc:
|
||||
print(f"error: {exc}", file=sys.stderr)
|
||||
sys.exit(1)
|
||||
|
||||
if args.list:
|
||||
CliRunner().list_certs(show_all=False)
|
||||
return
|
||||
|
||||
148
tests/test_app_reissue.py
Normal file
148
tests/test_app_reissue.py
Normal file
@@ -0,0 +1,148 @@
|
||||
"""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"
|
||||
@@ -4,6 +4,9 @@ from unittest.mock import MagicMock, patch, call
|
||||
import pytest
|
||||
|
||||
from openvpncertupdate import CliRunner, _build_parser, main
|
||||
# Bound before _patch_issue() rebinds the module attribute, so tests can put
|
||||
# the real filesystem check back and exercise the wiring end to end.
|
||||
from openvpncertupdate import has_issued_cert as real_has_issued_cert
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -13,6 +16,9 @@ from openvpncertupdate import CliRunner, _build_parser, main
|
||||
def _patch_issue(monkeypatch, ovpn_path="/out/cn_2026-01-01/client.ovpn",
|
||||
one_time_url="https://cg.example.com/#/note/abc/xyz"):
|
||||
"""Patch all side-effectful callables used by CliRunner._issue."""
|
||||
# Default to "the issued .crt is there", the normal case; the tests that
|
||||
# care about a missing cert file override this.
|
||||
monkeypatch.setattr("openvpncertupdate.has_issued_cert", MagicMock(return_value=True))
|
||||
monkeypatch.setattr("openvpncertupdate.revoke_issued", MagicMock())
|
||||
monkeypatch.setattr("openvpncertupdate.gen_crl", MagicMock())
|
||||
monkeypatch.setattr("openvpncertupdate.copy_crl", MagicMock())
|
||||
@@ -108,6 +114,164 @@ def test_create_does_not_touch_crl(monkeypatch, capsys):
|
||||
mocks["copy_crl"].assert_not_called()
|
||||
|
||||
|
||||
def _write_index(pki_dir, cns):
|
||||
"""Write an index.txt with one far-future V-status line per CN, so
|
||||
_load_current_certs()/has_issued_cert()'s "is this CN known?" check has
|
||||
something real to read. Mirrors make_pki() in test_pki.py."""
|
||||
lines = "".join(
|
||||
f"V\t350101000000Z\t\t01\tunknown\t/CN={cn}/emailAddress={cn}@example.com\n"
|
||||
for cn in cns
|
||||
)
|
||||
(pki_dir / "index.txt").write_text(lines)
|
||||
|
||||
|
||||
def test_reissue_skips_revoke_when_cert_file_missing(monkeypatch, capsys, tmp_path):
|
||||
# An index.txt copied from an older EasyRSA install lists V-status certs
|
||||
# whose .crt was never carried over. EasyRSA reads the serial out of the
|
||||
# .crt, so revoke-issued can only fail — issue the replacement instead.
|
||||
_write_index(tmp_path, ["y.kuts"])
|
||||
monkeypatch.setattr("openvpncertupdate.EASYRSA_PKI_DIR", str(tmp_path))
|
||||
mocks = _patch_issue(monkeypatch)
|
||||
monkeypatch.setattr("openvpncertupdate.has_issued_cert", MagicMock(return_value=False))
|
||||
CliRunner().reissue("y.kuts", "y.kuts@example.com")
|
||||
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_reissue_warns_when_revoke_skipped(monkeypatch, capsys, tmp_path):
|
||||
_write_index(tmp_path, ["y.kuts"])
|
||||
monkeypatch.setattr("openvpncertupdate.EASYRSA_PKI_DIR", str(tmp_path))
|
||||
mocks = _patch_issue(monkeypatch)
|
||||
monkeypatch.setattr("openvpncertupdate.has_issued_cert", MagicMock(return_value=False))
|
||||
CliRunner().reissue("y.kuts", "y.kuts@example.com")
|
||||
err = capsys.readouterr().err
|
||||
assert "nothing to revoke for y.kuts" in err
|
||||
assert "issued/y.kuts.crt" in err
|
||||
|
||||
|
||||
def test_reissue_skipped_revoke_build_failure_does_not_claim_revocation(monkeypatch, capsys, tmp_path):
|
||||
# Nothing was revoked, so the "has been revoked but no new cert" warning
|
||||
# would be a lie here.
|
||||
import openvpncertupdate
|
||||
_write_index(tmp_path, ["y.kuts"])
|
||||
monkeypatch.setattr("openvpncertupdate.EASYRSA_PKI_DIR", str(tmp_path))
|
||||
_patch_issue(monkeypatch)
|
||||
monkeypatch.setattr("openvpncertupdate.has_issued_cert", MagicMock(return_value=False))
|
||||
monkeypatch.setattr("openvpncertupdate.build_client_full",
|
||||
MagicMock(side_effect=openvpncertupdate.EasyRSAError("boom")))
|
||||
with pytest.raises(SystemExit):
|
||||
CliRunner().reissue("y.kuts", "y.kuts@example.com")
|
||||
assert "has been revoked" not in capsys.readouterr().err
|
||||
|
||||
|
||||
def test_reissue_unknown_cn_exits_without_issuing(monkeypatch, capsys, tmp_path):
|
||||
# A typo'd/nonexistent CN must not fall into the "migration gap" skip
|
||||
# path: before this fix it warned, skipped the revoke, and issued (and
|
||||
# could email) a brand-new certificate for a CN nobody asked to renew.
|
||||
_write_index(tmp_path, ["someone.else"])
|
||||
monkeypatch.setattr("openvpncertupdate.EASYRSA_PKI_DIR", str(tmp_path))
|
||||
mocks = _patch_issue(monkeypatch)
|
||||
monkeypatch.setattr("openvpncertupdate.has_issued_cert", MagicMock(return_value=False))
|
||||
with pytest.raises(SystemExit) as exc_info:
|
||||
CliRunner().reissue("totally-made-up-cn", "ghost@example.com")
|
||||
assert exc_info.value.code == 1
|
||||
err = capsys.readouterr().err
|
||||
assert "unknown" in err.lower()
|
||||
assert "totally-made-up-cn" in err
|
||||
mocks["build_client_full"].assert_not_called()
|
||||
mocks["build_ovpn"].assert_not_called()
|
||||
mocks["create_note"].assert_not_called()
|
||||
mocks["send_email"].assert_not_called()
|
||||
|
||||
|
||||
def test_reissue_leftover_req_blocks_skip(monkeypatch, capsys, 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
|
||||
# before the CA passphrase prompt with an actionable message instead.
|
||||
_write_index(tmp_path, ["y.kuts"])
|
||||
(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_issue(monkeypatch)
|
||||
monkeypatch.setattr("openvpncertupdate.has_issued_cert", MagicMock(return_value=False))
|
||||
with pytest.raises(SystemExit) as exc_info:
|
||||
CliRunner().reissue("y.kuts", "y.kuts@example.com")
|
||||
assert exc_info.value.code == 1
|
||||
err = capsys.readouterr().err
|
||||
assert str(req_path) in err
|
||||
mocks["build_client_full"].assert_not_called()
|
||||
|
||||
|
||||
def test_reissue_leftover_key_blocks_skip(monkeypatch, capsys, tmp_path):
|
||||
_write_index(tmp_path, ["y.kuts"])
|
||||
(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_issue(monkeypatch)
|
||||
monkeypatch.setattr("openvpncertupdate.has_issued_cert", MagicMock(return_value=False))
|
||||
with pytest.raises(SystemExit) as exc_info:
|
||||
CliRunner().reissue("y.kuts", "y.kuts@example.com")
|
||||
assert exc_info.value.code == 1
|
||||
err = capsys.readouterr().err
|
||||
assert str(key_path) in err
|
||||
mocks["build_client_full"].assert_not_called()
|
||||
|
||||
|
||||
def test_reissue_missing_index_txt_exits_cleanly(monkeypatch, capsys, tmp_path):
|
||||
# Covers only the _load_current_certs() call added inside _issue()'s new
|
||||
# unknown-CN check: with an --email supplied, reissue()'s own
|
||||
# get_email(EASYRSA_PKI_DIR, cn) fallback lookup is short-circuited
|
||||
# (`final_email = email_addr or get_email(...)`) and never runs, so this
|
||||
# does not exercise (or claim to fix) get_email()'s own bare open() on a
|
||||
# missing index.txt when no --email is given — that gap predates this
|
||||
# branch and is not one of the findings in scope here.
|
||||
pki = tmp_path / "no-such-pki"
|
||||
monkeypatch.setattr("openvpncertupdate.EASYRSA_PKI_DIR", str(pki))
|
||||
mocks = _patch_issue(monkeypatch)
|
||||
monkeypatch.setattr("openvpncertupdate.has_issued_cert", MagicMock(return_value=False))
|
||||
with pytest.raises(SystemExit) as exc_info:
|
||||
CliRunner().reissue("y.kuts", "y.kuts@example.com")
|
||||
assert exc_info.value.code == 1
|
||||
assert "index.txt" in capsys.readouterr().err
|
||||
mocks["build_client_full"].assert_not_called()
|
||||
|
||||
|
||||
def _reissue_against_real_pki(monkeypatch, tmp_path, cert_files, index_cns=("y.kuts",)):
|
||||
"""Run --reissue with the real has_issued_cert against a temp PKI layout."""
|
||||
issued = tmp_path / "issued"
|
||||
issued.mkdir()
|
||||
for name in cert_files:
|
||||
(issued / name).write_text("-----BEGIN CERTIFICATE-----\n")
|
||||
if index_cns:
|
||||
_write_index(tmp_path, index_cns)
|
||||
mocks = _patch_issue(monkeypatch)
|
||||
monkeypatch.setattr("openvpncertupdate.has_issued_cert", real_has_issued_cert)
|
||||
monkeypatch.setattr("openvpncertupdate.EASYRSA_PKI_DIR", str(tmp_path))
|
||||
CliRunner().reissue("y.kuts", "y.kuts@example.com")
|
||||
return mocks
|
||||
|
||||
|
||||
def test_reissue_reads_pki_dir_and_skips_revoke_for_missing_crt(monkeypatch, capsys, tmp_path):
|
||||
# issued/ exists but holds other people's certs — exactly the server state.
|
||||
mocks = _reissue_against_real_pki(
|
||||
monkeypatch, tmp_path, ["ivan.radchenko.crt", "s.krasota.crt"])
|
||||
mocks["revoke_issued"].assert_not_called()
|
||||
mocks["build_client_full"].assert_called_once()
|
||||
|
||||
|
||||
def test_reissue_reads_pki_dir_and_revokes_when_crt_present(monkeypatch, capsys, tmp_path):
|
||||
mocks = _reissue_against_real_pki(monkeypatch, tmp_path, ["y.kuts.crt"])
|
||||
mocks["revoke_issued"].assert_called_once()
|
||||
mocks["gen_crl"].assert_called_once()
|
||||
mocks["build_client_full"].assert_called_once()
|
||||
|
||||
|
||||
def test_reissue_continues_building_when_crl_regen_fails(monkeypatch, capsys):
|
||||
import openvpncertupdate
|
||||
mocks = _patch_issue(monkeypatch)
|
||||
@@ -238,6 +402,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
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -522,3 +727,70 @@ def test_main_show_eml_send_email_override(monkeypatch):
|
||||
runner.create.assert_called_once_with(
|
||||
"alice", "a@b.com", send_email_flag=True, show_eml=True
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# --days
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_days_flag_parses():
|
||||
args = _build_parser().parse_args(["--create", "alice", "--email", "a@b.c",
|
||||
"--days", "90"])
|
||||
assert args.days == "90"
|
||||
|
||||
|
||||
def test_days_defaults_to_none_when_absent():
|
||||
args = _build_parser().parse_args(["--create", "alice", "--email", "a@b.c"])
|
||||
assert args.days is None
|
||||
|
||||
|
||||
def test_issue_passes_resolved_days_to_build_client_full(monkeypatch, capsys):
|
||||
mocks = _patch_issue(monkeypatch)
|
||||
monkeypatch.setattr("openvpncertupdate.CERT_DAYS", "90")
|
||||
CliRunner().create("alice", "alice@example.com")
|
||||
assert mocks["build_client_full"].call_args.kwargs["days"] == "90"
|
||||
|
||||
|
||||
def test_issue_passes_empty_days_when_inheriting(monkeypatch, capsys):
|
||||
mocks = _patch_issue(monkeypatch)
|
||||
monkeypatch.setattr("openvpncertupdate.CERT_DAYS", "")
|
||||
CliRunner().create("alice", "alice@example.com")
|
||||
assert mocks["build_client_full"].call_args.kwargs["days"] == ""
|
||||
|
||||
|
||||
def test_main_days_flag_overrides_cert_days(monkeypatch):
|
||||
import openvpncertupdate
|
||||
monkeypatch.setattr(sys, "argv",
|
||||
["prog", "--create", "alice", "--email", "a@b.c", "--days", "30"])
|
||||
monkeypatch.setattr("openvpncertupdate.CERT_DAYS", "default")
|
||||
monkeypatch.setattr("openvpncertupdate.resolve_ca_passphrase", MagicMock(return_value=""))
|
||||
runner = MagicMock()
|
||||
monkeypatch.setattr("openvpncertupdate.CliRunner", MagicMock(return_value=runner))
|
||||
main()
|
||||
assert openvpncertupdate.CERT_DAYS == "30"
|
||||
|
||||
|
||||
def test_main_resolves_cert_days_when_no_flag(monkeypatch):
|
||||
import openvpncertupdate
|
||||
monkeypatch.setattr(sys, "argv", ["prog", "--create", "alice", "--email", "a@b.c"])
|
||||
monkeypatch.setattr("openvpncertupdate.CERT_DAYS", "090")
|
||||
monkeypatch.setattr("openvpncertupdate.resolve_ca_passphrase", MagicMock(return_value=""))
|
||||
monkeypatch.setattr("openvpncertupdate.CliRunner", MagicMock(return_value=MagicMock()))
|
||||
main()
|
||||
assert openvpncertupdate.CERT_DAYS == "90"
|
||||
|
||||
|
||||
def test_main_rejects_bad_days_flag(monkeypatch, capsys):
|
||||
monkeypatch.setattr(sys, "argv",
|
||||
["prog", "--create", "alice", "--email", "a@b.c", "--days", "0"])
|
||||
monkeypatch.setattr("openvpncertupdate.resolve_ca_passphrase", MagicMock(return_value=""))
|
||||
with pytest.raises(SystemExit):
|
||||
main()
|
||||
err = capsys.readouterr().err
|
||||
# Must come from resolve_cert_days()'s own message, not just from
|
||||
# argparse's "unrecognized arguments: --days 0" (which also contains the
|
||||
# substring "--days" and would pass even if the --days flag were removed
|
||||
# entirely — see resolve_cert_days()).
|
||||
assert "--days" in err
|
||||
assert "must be a positive number of days" in err
|
||||
assert "EasyRSA rejects 0" in err
|
||||
|
||||
@@ -60,3 +60,10 @@ def test_comments_and_non_string_types_supported(tmp_path):
|
||||
)
|
||||
overrides = load_settings_overrides(None, "", _script_path(tmp_path))
|
||||
assert overrides == {"SMTP_PORT": 2525, "CA_PASSPHRASE": ""}
|
||||
|
||||
|
||||
def test_cert_days_is_overridable(tmp_path):
|
||||
conf = tmp_path / "app.conf"
|
||||
conf.write_text('CERT_DAYS = 90\n')
|
||||
out = load_settings_overrides(str(conf), "", str(tmp_path / "app.py"))
|
||||
assert out["CERT_DAYS"] == 90
|
||||
|
||||
@@ -115,26 +115,38 @@ def test_show_cert_form_uses_erase_not_clear():
|
||||
|
||||
|
||||
def test_show_cert_form_confirm():
|
||||
"""Enter advances through the 3 fields to the Continue button; Enter on
|
||||
Continue confirms. That's 4 Enter keypresses total."""
|
||||
"""Enter advances through the 4 fields to the Continue button; Enter on
|
||||
Continue confirms. That's 5 Enter keypresses total.
|
||||
|
||||
days="90" (non-blank) is used deliberately: a blank default would still
|
||||
read back as "" even from a reverted three-field form (CertFormResult.days
|
||||
defaults to ""), so it would not actually prove the days field was
|
||||
traversed. A non-blank value only round-trips if the days field exists,
|
||||
was reached by the Enter sequence, and was carried into the result."""
|
||||
stdscr = _make_stdscr()
|
||||
win = _make_win(rows=20, cols=70)
|
||||
win.getch.side_effect = [10, 10, 10, 10]
|
||||
win.getch.side_effect = [10, 10, 10, 10, 10]
|
||||
with patch("curses.newwin", return_value=win):
|
||||
result = show_cert_form(stdscr, cn="bob", email="bob@example.com")
|
||||
result = show_cert_form(stdscr, cn="bob", email="bob@example.com", days="90")
|
||||
assert isinstance(result, CertFormResult)
|
||||
assert result.confirmed is True
|
||||
assert result.cn == "bob"
|
||||
assert result.email == "bob@example.com"
|
||||
assert result.days == "90"
|
||||
assert len(result.password) > 0
|
||||
# Pins the field count itself: a partial reversion that drops "days"
|
||||
# from _FORM_FIELDS while CertFormResult/_submit() keep it would still
|
||||
# round-trip days="90" above (it's passed straight through, untraversed),
|
||||
# but would only consume 4 Enters, not 5.
|
||||
assert win.getch.call_count == 5
|
||||
|
||||
|
||||
def test_show_cert_form_cancel_button():
|
||||
"""Tab to the Cancel button (4 Tabs from start) and press Enter cancels."""
|
||||
"""Tab to the Cancel button (5 Tabs from start) and press Enter cancels."""
|
||||
stdscr = _make_stdscr()
|
||||
win = _make_win(rows=20, cols=70)
|
||||
# Tab×4: cn→email→password→Continue→Cancel, then Enter
|
||||
win.getch.side_effect = [9, 9, 9, 9, 10]
|
||||
# Tab×5: cn→email→days→password→Continue→Cancel, then Enter
|
||||
win.getch.side_effect = [9, 9, 9, 9, 9, 10]
|
||||
with patch("curses.newwin", return_value=win):
|
||||
result = show_cert_form(stdscr, cn="alice", email="alice@example.com")
|
||||
assert isinstance(result, CertFormResult)
|
||||
@@ -142,7 +154,7 @@ def test_show_cert_form_cancel_button():
|
||||
|
||||
|
||||
def test_show_cert_form_clamps_to_narrow_screen():
|
||||
"""The form's fixed 13x62 size must not exceed a smaller-than-usual
|
||||
"""The form's fixed 15x62 size must not exceed a smaller-than-usual
|
||||
screen, which would make curses.newwin() raise."""
|
||||
stdscr = _make_stdscr(rows=10, cols=40)
|
||||
win = _make_win()
|
||||
@@ -156,6 +168,58 @@ def test_show_cert_form_clamps_to_narrow_screen():
|
||||
assert x >= 0
|
||||
|
||||
|
||||
def test_show_cert_form_returns_days():
|
||||
stdscr = _make_stdscr()
|
||||
win = _make_win(rows=20, cols=70)
|
||||
win.getch.side_effect = [10, 10, 10, 10, 10]
|
||||
with patch("curses.newwin", return_value=win):
|
||||
result = show_cert_form(stdscr, cn="bob", email="b@c.d", days="90")
|
||||
assert result.confirmed is True
|
||||
assert result.days == "90"
|
||||
|
||||
|
||||
def test_show_cert_form_blank_days_means_inherit():
|
||||
stdscr = _make_stdscr()
|
||||
win = _make_win(rows=20, cols=70)
|
||||
win.getch.side_effect = [10, 10, 10, 10, 10]
|
||||
with patch("curses.newwin", return_value=win):
|
||||
result = show_cert_form(stdscr, cn="bob", email="b@c.d", days="")
|
||||
assert result.confirmed is True
|
||||
assert result.days == ""
|
||||
|
||||
|
||||
def test_show_cert_form_days_field_is_digits_only():
|
||||
# cn is read-only here, so focus starts on email: Tab once to reach days.
|
||||
stdscr = _make_stdscr()
|
||||
win = _make_win(rows=20, cols=70)
|
||||
win.getch.side_effect = [9] + [ord(c) for c in "9a0!"] + [10, 10, 10]
|
||||
with patch("curses.newwin", return_value=win):
|
||||
result = show_cert_form(stdscr, cn="bob", email="b@c.d", days="",
|
||||
cn_readonly=True)
|
||||
assert result.confirmed is True
|
||||
assert result.days == "90"
|
||||
|
||||
|
||||
def test_show_cert_form_refuses_zero_days():
|
||||
# Submitting "0" must keep the form open rather than closing and failing
|
||||
# later inside EasyRSA. Enter×5 tries to submit, Esc then cancels.
|
||||
stdscr = _make_stdscr()
|
||||
win = _make_win(rows=20, cols=70)
|
||||
win.getch.side_effect = [10, 10, 10, 10, 10, 27]
|
||||
with patch("curses.newwin", return_value=win):
|
||||
result = show_cert_form(stdscr, cn="bob", email="b@c.d", days="0")
|
||||
assert result.confirmed is False
|
||||
|
||||
|
||||
def test_show_cert_form_normalises_leading_zero_days():
|
||||
stdscr = _make_stdscr()
|
||||
win = _make_win(rows=20, cols=70)
|
||||
win.getch.side_effect = [10, 10, 10, 10, 10]
|
||||
with patch("curses.newwin", return_value=win):
|
||||
result = show_cert_form(stdscr, cn="bob", email="b@c.d", days="090")
|
||||
assert result.days == "90"
|
||||
|
||||
|
||||
def test_show_cert_form_newwin_failure_returns_cancelled():
|
||||
"""If curses.newwin() still fails despite clamping, show_cert_form must
|
||||
degrade to an unconfirmed result rather than crash the whole app."""
|
||||
|
||||
@@ -4,15 +4,29 @@ import pytest
|
||||
from unittest.mock import patch, MagicMock
|
||||
from openvpncertupdate import (
|
||||
revoke_issued, build_client_full, gen_crl, copy_crl, EasyRSAError,
|
||||
is_ca_key_encrypted, resolve_ca_passphrase,
|
||||
is_ca_key_encrypted, resolve_ca_passphrase, resolve_cert_days, ConfigError,
|
||||
has_issued_cert, issued_cert_path,
|
||||
)
|
||||
|
||||
|
||||
def ok_result():
|
||||
r = MagicMock(); r.returncode = 0; r.stderr = ""; return r
|
||||
r = MagicMock(); r.returncode = 0; r.stdout = ""; r.stderr = ""; return r
|
||||
|
||||
def err_result():
|
||||
r = MagicMock(); r.returncode = 1; r.stderr = "oops"; return r
|
||||
def err_result(stdout="", stderr="oops"):
|
||||
r = MagicMock(); r.returncode = 1; r.stdout = stdout; r.stderr = stderr; return r
|
||||
|
||||
|
||||
# EasyRSA's print() is `printf '%s\n'` -> stdout, and both user_error() and
|
||||
# die() route through it, so this is what a real failure looks like on the wire.
|
||||
EASYRSA_USER_ERROR = """
|
||||
EasyRSA version 3.2.6
|
||||
|
||||
Error
|
||||
-----
|
||||
Unable to revoke as no certificate was found.
|
||||
Certificate was expected at:
|
||||
* /etc/easy-rsa/pki/issued/alice.crt
|
||||
"""
|
||||
|
||||
|
||||
@patch("openvpncertupdate.subprocess.run")
|
||||
@@ -43,6 +57,43 @@ def test_revoke_raises_on_failure(mock_run):
|
||||
revoke_issued("/er", "/pki", "alice", "")
|
||||
|
||||
|
||||
@patch("openvpncertupdate.subprocess.run")
|
||||
def test_error_reports_easyrsa_stdout_diagnostics(mock_run):
|
||||
# EasyRSA writes its diagnostics to stdout; reporting stderr alone left the
|
||||
# user with a blank error box.
|
||||
mock_run.return_value = err_result(stdout=EASYRSA_USER_ERROR, stderr="")
|
||||
with pytest.raises(EasyRSAError) as exc:
|
||||
revoke_issued("/er", "/pki", "alice", "")
|
||||
msg = str(exc.value)
|
||||
assert "Unable to revoke as no certificate was found." in msg
|
||||
assert "* /etc/easy-rsa/pki/issued/alice.crt" in msg
|
||||
|
||||
|
||||
@patch("openvpncertupdate.subprocess.run")
|
||||
def test_error_strips_banner_and_blank_padding(mock_run):
|
||||
mock_run.return_value = err_result(stdout=EASYRSA_USER_ERROR, stderr="")
|
||||
with pytest.raises(EasyRSAError) as exc:
|
||||
revoke_issued("/er", "/pki", "alice", "")
|
||||
lines = str(exc.value).splitlines()
|
||||
assert "" not in lines # no blank padding
|
||||
assert not any(l.startswith("EasyRSA version") for l in lines)
|
||||
assert "-----" not in lines # no rule under "Error"
|
||||
|
||||
|
||||
@patch("openvpncertupdate.subprocess.run")
|
||||
def test_error_keeps_both_streams(mock_run):
|
||||
# openssl failures land on stderr while EasyRSA's die() text lands on stdout.
|
||||
mock_run.return_value = err_result(
|
||||
stdout="Easy-RSA error:\n\nFailed to revoke certificate.",
|
||||
stderr="unable to load CA private key",
|
||||
)
|
||||
with pytest.raises(EasyRSAError) as exc:
|
||||
revoke_issued("/er", "/pki", "alice", "")
|
||||
msg = str(exc.value)
|
||||
assert "Failed to revoke certificate." in msg
|
||||
assert "unable to load CA private key" in msg
|
||||
|
||||
|
||||
@patch("openvpncertupdate.subprocess.run")
|
||||
def test_build_client_full_args(mock_run):
|
||||
mock_run.return_value = ok_result()
|
||||
@@ -99,6 +150,30 @@ def test_copy_crl_restorecon_failure_is_non_fatal(mock_chmod, mock_copy, mock_ru
|
||||
mock_chmod.assert_called_once()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# has_issued_cert
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_issued_cert_path(tmp_path):
|
||||
assert issued_cert_path("/pki", "y.kuts") == "/pki/issued/y.kuts.crt"
|
||||
|
||||
|
||||
def test_has_issued_cert_true_when_file_present(tmp_path):
|
||||
issued = tmp_path / "issued"
|
||||
issued.mkdir()
|
||||
(issued / "y.kuts.crt").write_text("-----BEGIN CERTIFICATE-----\n")
|
||||
assert has_issued_cert(str(tmp_path), "y.kuts") is True
|
||||
|
||||
|
||||
def test_has_issued_cert_false_when_index_lists_cert_but_file_is_gone(tmp_path):
|
||||
(tmp_path / "issued").mkdir()
|
||||
assert has_issued_cert(str(tmp_path), "y.kuts") is False
|
||||
|
||||
|
||||
def test_has_issued_cert_false_when_issued_dir_missing(tmp_path):
|
||||
assert has_issued_cert(str(tmp_path), "y.kuts") is False
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# is_ca_key_encrypted
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -169,3 +244,92 @@ def test_resolve_literal_passphrase_passed_through_unchanged(tmp_path):
|
||||
result = resolve_ca_passphrase("mysecret", str(tmp_path), prompt=prompt)
|
||||
assert result == "mysecret"
|
||||
prompt.assert_not_called()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# resolve_cert_days
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@pytest.mark.parametrize("configured", ["default", "", " ", " default "])
|
||||
def test_resolve_cert_days_sentinels_mean_inherit(configured):
|
||||
# "" and "default" leave the lifetime to EasyRSA's own EASYRSA_CERT_EXPIRE,
|
||||
# so deploying this cannot silently shorten certs on an existing PKI.
|
||||
assert resolve_cert_days(configured) == ""
|
||||
|
||||
|
||||
@pytest.mark.parametrize("configured,expected", [
|
||||
(90, "90"), ("90", "90"), (" 90 ", "90"), (1, "1"), (3650, "3650"),
|
||||
])
|
||||
def test_resolve_cert_days_accepts_positive_numbers(configured, expected):
|
||||
assert resolve_cert_days(configured) == expected
|
||||
|
||||
|
||||
def test_resolve_cert_days_normalises_leading_zeros():
|
||||
# EasyRSA's own pattern rejects "090" with "Number expected", which reads
|
||||
# as nonsense to someone who typed only digits.
|
||||
assert resolve_cert_days("090") == "90"
|
||||
|
||||
|
||||
def test_resolve_cert_days_rejects_zero():
|
||||
with pytest.raises(ConfigError, match="positive"):
|
||||
resolve_cert_days("0")
|
||||
|
||||
|
||||
@pytest.mark.parametrize("configured", ["-5", -5, "abc", "9 0", "90.5", 90.5, None])
|
||||
def test_resolve_cert_days_rejects_junk(configured):
|
||||
with pytest.raises(ConfigError):
|
||||
resolve_cert_days(configured)
|
||||
|
||||
|
||||
def test_resolve_cert_days_error_names_the_source():
|
||||
# The same resolver serves CERT_DAYS, --days and the TUI field; the message
|
||||
# must say which one the user actually touched.
|
||||
with pytest.raises(ConfigError, match="--days"):
|
||||
resolve_cert_days("0", "--days")
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# build_client_full with days parameter
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@patch("openvpncertupdate.subprocess.run")
|
||||
def test_build_client_full_includes_days_when_set(mock_run):
|
||||
mock_run.return_value = ok_result()
|
||||
build_client_full("/er", "/pki", "bob", "keypass", "capass", days="90")
|
||||
assert "--days=90" in mock_run.call_args[0][0]
|
||||
|
||||
|
||||
@patch("openvpncertupdate.subprocess.run")
|
||||
def test_build_client_full_omits_days_when_inheriting(mock_run):
|
||||
mock_run.return_value = ok_result()
|
||||
build_client_full("/er", "/pki", "bob", "keypass", "capass", days="")
|
||||
assert not any(a.startswith("--days") for a in mock_run.call_args[0][0])
|
||||
|
||||
|
||||
@patch("openvpncertupdate.subprocess.run")
|
||||
def test_build_client_full_days_precedes_the_verb(mock_run):
|
||||
# --days is a global option: EasyRSA parses it before the command word.
|
||||
mock_run.return_value = ok_result()
|
||||
build_client_full("/er", "/pki", "bob", "keypass", "capass", days="90")
|
||||
args = mock_run.call_args[0][0]
|
||||
assert args.index("--days=90") < args.index("build-client-full")
|
||||
|
||||
|
||||
@patch("openvpncertupdate.subprocess.run")
|
||||
def test_revoke_and_gen_crl_never_carry_days(mock_run):
|
||||
# _base_cmd() is shared with gen-crl, where --days means CRL validity.
|
||||
# Putting the flag there would quietly change how long CRLs are valid.
|
||||
mock_run.return_value = ok_result()
|
||||
revoke_issued("/er", "/pki", "alice", "capass")
|
||||
assert not any(a.startswith("--days") for a in mock_run.call_args[0][0])
|
||||
gen_crl("/er", "/pki", "capass")
|
||||
assert not any(a.startswith("--days") for a in mock_run.call_args[0][0])
|
||||
|
||||
|
||||
@patch("openvpncertupdate.subprocess.run")
|
||||
def test_error_verb_detection_survives_days_flag(mock_run):
|
||||
# _run_easyrsa picks the verb as the first non-flag arg; "--days=90"
|
||||
# must not be mistaken for it.
|
||||
mock_run.return_value = err_result(stdout="Error\nboom", stderr="")
|
||||
with pytest.raises(EasyRSAError, match="build-client-full"):
|
||||
build_client_full("/er", "/pki", "bob", "keypass", "capass", days="90")
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -94,3 +94,30 @@ def test_init_colors_survives_init_pair_rejecting_every_pair():
|
||||
side_effect=_curses.error("init_pair() returned ERR")), \
|
||||
patch.object(_curses, "COLORS", 256, create=True):
|
||||
init_colors() # must not raise
|
||||
|
||||
|
||||
def test_allowed_charset_accepts_listed_characters():
|
||||
f = InputField(MagicMock(), 0, 0, 10, initial="", allowed="0123456789")
|
||||
for ch in "90":
|
||||
f.handle_key(ord(ch))
|
||||
assert f.value == "90"
|
||||
|
||||
|
||||
def test_allowed_charset_drops_other_characters():
|
||||
f = InputField(MagicMock(), 0, 0, 10, initial="", allowed="0123456789")
|
||||
for ch in "9a0-!":
|
||||
f.handle_key(ord(ch))
|
||||
assert f.value == "90"
|
||||
|
||||
|
||||
def test_allowed_none_accepts_everything():
|
||||
f = InputField(MagicMock(), 0, 0, 10, initial="")
|
||||
for ch in "a-1!":
|
||||
f.handle_key(ord(ch))
|
||||
assert f.value == "a-1!"
|
||||
|
||||
|
||||
def test_allowed_charset_still_supports_editing_keys():
|
||||
f = InputField(MagicMock(), 0, 0, 10, initial="90", allowed="0123456789")
|
||||
f.handle_key(_curses.KEY_BACKSPACE)
|
||||
assert f.value == "9"
|
||||
|
||||
Reference in New Issue
Block a user