From a5efdf22a144220913e0039ada37bf455824b199 Mon Sep 17 00:00:00 2001 From: Padreug Date: Thu, 18 Jun 2026 18:51:54 +0200 Subject: [PATCH] feat(pairing): optional token TTL + revoke endpoint (#9/#12, #22) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Builds on the seed-URL pairing in #21 (stacked). (b) TTL — PairMachineData.duration_hours (validated > 0) threads through pair_spire -> create_new_token (lnbits#55). None = non-expiring. (c) Revoke — POST /machines/{id}/revoke -> revoke_spire -> admin_client.revoke_key_user(spire-). Per spirekeeper#22, revoke MUST go through KeyUser.revokedAt (revoke_key_user), NOT token revoke: lnbits eager-binds (redeems) the connect token at provision, so nsecbunkerd has materialised the policy into per-KeyUser grants its ACL checks BEFORE the Token.revokedAt filter -> token revoke is a silent no-op. Returns RevokeResult{revoked_count}: >=1 = cut, 0 = never bound. set_machine_unpaired clears paired_at (keeps npub + bunker_spire_key_name for audit / re-pair). 7 new tests (duration threading + default-None; revoke routes to revoke_key_user and never token-revoke + error mapping; endpoint wiring revoke happy/zero/502). 210 green; new code black/ruff-clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- crud.py | 18 +++++++++++ models.py | 10 ++++++- pairing.py | 47 ++++++++++++++++++++++++++++- tests/test_pair_endpoint.py | 49 +++++++++++++++++++++++++++++- tests/test_pairing.py | 60 +++++++++++++++++++++++++++++++++++-- views_api.py | 49 +++++++++++++++++++++++++++--- 6 files changed, 223 insertions(+), 10 deletions(-) diff --git a/crud.py b/crud.py index bf51ecd..6c4ef53 100644 --- a/crud.py +++ b/crud.py @@ -233,6 +233,24 @@ async def set_machine_pairing( return await get_machine(machine_id) +async def set_machine_unpaired(machine_id: str) -> Machine | None: + """Mark a machine unpaired after revoking its spire's bunker access + (POST /revoke). Clears `paired_at`; keeps `machine_npub` + + `bunker_spire_key_name` for audit / re-pair. The bunker-side + `KeyUser.revokedAt` (set by `revoke_spire`) is what actually stops the + spire signing — this just records the operator-visible state.""" + await db.execute( + """ + UPDATE spirekeeper.dca_machines + SET paired_at = NULL, + updated_at = :updated_at + WHERE id = :id + """, + {"updated_at": datetime.now(), "id": machine_id}, + ) + return await get_machine(machine_id) + + async def delete_machine(machine_id: str) -> None: await db.execute( "DELETE FROM spirekeeper.dca_machines WHERE id = :id", diff --git a/models.py b/models.py index 68ffdd2..f56cbcd 100644 --- a/models.py +++ b/models.py @@ -85,9 +85,17 @@ class PairMachineData(BaseModel): """Body for POST /machines/{id}/pair (S0 / #9). `relays` are the relays the spire will use for its own events (kind-21000/30078) — typically the operator's nostrrelay; the bunker connection relay is added separately - from the lnbits bunker settings.""" + from the lnbits bunker settings. `duration_hours` optionally time-bounds + the spire's connect token (None = non-expiring).""" relays: list[str] + duration_hours: int | None = None + + @validator("duration_hours") + def _positive_duration(cls, v): + if v is not None and v <= 0: + raise ValueError("duration_hours must be positive when set") + return v # ============================================================================= diff --git a/pairing.py b/pairing.py index 833de3a..b65b760 100644 --- a/pairing.py +++ b/pairing.py @@ -97,6 +97,14 @@ class PairResult(BaseModel): seed_url: str +class RevokeResult(BaseModel): + """Output of revoke. `revoked_count` >= 1 = the spire's signing access + is cut (KeyUser.revokedAt set); 0 = nothing was bound (token minted but + the spire never connected).""" + + revoked_count: int + + def spire_key_name(machine_id: str) -> str: """The spire's key name in the bunker keystore. Stable across re-pairs so re-issuing a token reuses the same underlying key (create_new_key @@ -147,10 +155,16 @@ async def pair_spire( admin_client: NsecBunkerAdminClient, bunker_relay: str | None = None, keystore_passphrase: str | None = None, + duration_hours: int | None = None, ) -> PairResult: """Mint a bunker-held key + scoped connect token for `machine` and return the seed URL the spire redeems at first boot. + `duration_hours` (optional, aiolabs/lnbits#54 item 2) sets a TTL on the + spire's connect token — the bunker stamps `expiresAt` and rejects the + token once it lapses, forcing a re-pair. None = non-expiring (the only + invalidation path is then revoke, `revoke_spire`). + `admin_client` must already be connected (the caller owns the `async with NsecBunkerAdminClient.from_settings()` context) — keeps connection lifecycle out of the orchestration so this is unit-testable @@ -196,7 +210,9 @@ async def pair_spire( rules=SPIRE_POLICY_RULES, methods_no_kind=SPIRE_POLICY_METHODS_NO_KIND, ) - await admin_client.create_new_token(key_name, client_name, policy_id) + await admin_client.create_new_token( + key_name, client_name, policy_id, duration_hours=duration_hours + ) tokens = await admin_client.get_key_tokens(key_name) except NsecBunkerNotConfiguredError as exc: raise PairingError(f"nsecbunkerd is not configured: {exc}") from exc @@ -223,3 +239,32 @@ async def pair_spire( bunker_url=bunker_url, seed_url=seed_url, ) + + +async def revoke_spire( + machine: Machine, *, admin_client: NsecBunkerAdminClient +) -> int: + """Revoke a spire's bunker access (the "Revoke spire access" UX, + aiolabs/spirekeeper#9/#12; security model per #22). + + Calls `revoke_key_user` — NOT `revoke_token` / `revoke_key_token`. + lnbits eager-binds (redeems) the connect token at provision time + (aiolabs/lnbits#32), so nsecbunkerd has already materialised the + token's policy into standing per-`KeyUser` `SigningCondition` grants; + its sign-time ACL checks those *before* the `Token.revokedAt` filter, + so revoking the token is a silent no-op (the spire keeps signing). + Only `KeyUser.revokedAt` — set by `revoke_user` / `revoke_key_user` — + actually cuts off signing (verified live 2026-06-18, #22). + + Returns the number of KeyUsers revoked: >= 1 means the spire's signing + access is now cut; 0 means nothing was bound (token minted but the + spire never connected). Raises PairingError on any bunker failure. + """ + try: + return await admin_client.revoke_key_user(spire_key_name(machine.id)) + except NsecBunkerNotConfiguredError as exc: + raise PairingError(f"nsecbunkerd is not configured: {exc}") from exc + except NsecBunkerError as exc: + raise PairingError( + f"bunker admin RPC failed during revoke: {exc}" + ) from exc diff --git a/tests/test_pair_endpoint.py b/tests/test_pair_endpoint.py index d816c2f..0d50d95 100644 --- a/tests/test_pair_endpoint.py +++ b/tests/test_pair_endpoint.py @@ -66,7 +66,7 @@ def _wire(monkeypatch, *, pair="ok"): async def fake_owned(machine_id, user_id): return _machine() - async def fake_pair(machine, *, relays, admin_client): + async def fake_pair(machine, *, relays, admin_client, duration_hours=None): if pair == "error": raise PairingError("boom") return _result() @@ -118,3 +118,50 @@ def test_pair_failure_maps_to_bad_gateway(monkeypatch): assert ei.value.status_code == 502 # nothing persisted on failure assert state["persisted"] is None + + +def _wire_revoke(monkeypatch, *, revoke="ok", count=2): + state = {"unpaired": None} + + async def fake_owned(machine_id, user_id): + return _machine() + + async def fake_revoke(machine, *, admin_client): + if revoke == "error": + raise PairingError("boom") + return count + + async def fake_unpaired(machine_id): + state["unpaired"] = machine_id + return _machine() + + monkeypatch.setattr(views_api, "_machine_owned_by", fake_owned) + monkeypatch.setattr(views_api, "NsecBunkerAdminClient", _FakeAdmin) + monkeypatch.setattr(views_api, "revoke_spire", fake_revoke) + monkeypatch.setattr(views_api, "set_machine_unpaired", fake_unpaired) + return state + + +def _call_revoke(): + user = SimpleNamespace(id="op1") + return asyncio.run(views_api.api_revoke_machine("m1", user)) + + +def test_revoke_cuts_access_and_marks_unpaired(monkeypatch): + state = _wire_revoke(monkeypatch, count=2) + result = _call_revoke() + assert result.revoked_count == 2 + assert state["unpaired"] == "m1" + + +def test_revoke_zero_when_nothing_bound(monkeypatch): + _wire_revoke(monkeypatch, count=0) + assert _call_revoke().revoked_count == 0 + + +def test_revoke_failure_maps_to_bad_gateway(monkeypatch): + state = _wire_revoke(monkeypatch, revoke="error") + with pytest.raises(HTTPException) as ei: + _call_revoke() + assert ei.value.status_code == 502 + assert state["unpaired"] is None # not persisted on failure diff --git a/tests/test_pairing.py b/tests/test_pairing.py index 82eb4a2..4deb5a0 100644 --- a/tests/test_pairing.py +++ b/tests/test_pairing.py @@ -16,6 +16,7 @@ import json from datetime import datetime, timezone import pytest +from lnbits.core.services.nsec_bunker import NsecBunkerError from lnbits.utils.nostr import hex_to_npub from ..models import Machine @@ -27,6 +28,7 @@ from ..pairing import ( PairingError, build_seed_url, pair_spire, + revoke_spire, spire_key_name, ) @@ -70,9 +72,10 @@ class FakeBunker: admin_pubkey = "fake-admin-pubkey" # pragma: allowlist secret - def __init__(self, *, policies=None, token_secret="s3cr3t"): + def __init__(self, *, policies=None, token_secret="s3cr3t", revoke_count=1): self._policies = policies or [] self._token_secret = token_secret + self._revoke_count = revoke_count self.calls: list[tuple] = [] self._next_policy_id = 7 @@ -93,8 +96,16 @@ class FakeBunker: async def add_policy_rule(self, policy_id, rule): self.calls.append(("add_policy_rule", policy_id, rule)) - async def create_new_token(self, key_name, client_name, policy_id): - self.calls.append(("create_new_token", key_name, client_name, policy_id)) + async def create_new_token( + self, key_name, client_name, policy_id, duration_hours=None + ): + self.calls.append( + ("create_new_token", key_name, client_name, policy_id, duration_hours) + ) + + async def revoke_key_user(self, key_name): + self.calls.append(("revoke_key_user", key_name)) + return self._revoke_count async def get_key_tokens(self, key_name): self.calls.append(("get_key_tokens", key_name)) @@ -251,3 +262,46 @@ def test_build_seed_url_roundtrip(): payload = json.loads(base64.urlsafe_b64decode(blob + "=" * (-len(blob) % 4))) assert payload["spire_pubkey"] == _SPIRE_HEX assert payload["relays"] == _RELAYS + + +def test_pair_threads_duration_hours(): + bunker = FakeBunker() + asyncio.run( + pair_spire( + _machine(), + relays=_RELAYS, + admin_client=bunker, + bunker_relay=_BUNKER_RELAY, + keystore_passphrase=_PASSPHRASE, + duration_hours=720, + ) + ) + # create_new_token tuple is (name, key, client, policy_id, duration_hours) + assert bunker.named("create_new_token")[0][4] == 720 + + +def test_pair_default_duration_is_none(): + bunker = FakeBunker() + _pair(bunker) # no duration_hours + assert bunker.named("create_new_token")[0][4] is None + + +def test_revoke_spire_calls_revoke_key_user(): + # revoke MUST go through revoke_key_user (KeyUser.revokedAt), not token + # revoke — token revoke is a no-op once redeemed (spirekeeper#22). + bunker = FakeBunker(revoke_count=2) + count = asyncio.run(revoke_spire(_machine(), admin_client=bunker)) + assert count == 2 + assert bunker.named("revoke_key_user") == [("revoke_key_user", "spire-m1")] + assert not bunker.named("revoke_token") # never token-revoke + + +def test_revoke_spire_maps_bunker_error(): + bunker = FakeBunker() + + async def _boom(key_name): + raise NsecBunkerError("nope") + + bunker.revoke_key_user = _boom + with pytest.raises(PairingError, match="revoke"): + asyncio.run(revoke_spire(_machine(), admin_client=bunker)) diff --git a/views_api.py b/views_api.py index a37df74..8dff07e 100644 --- a/views_api.py +++ b/views_api.py @@ -29,7 +29,13 @@ from .cassette_transport import ( publish_to_atm, ) from .fee_transport import publish_fee_config -from .pairing import PairResult, PairingError, pair_spire +from .pairing import ( + PairResult, + PairingError, + RevokeResult, + pair_spire, + revoke_spire, +) from .crud import ( append_settlement_note, count_completed_legs_for_settlement, @@ -63,6 +69,7 @@ from .crud import ( replace_commission_splits, reset_settlement_for_retry, set_machine_pairing, + set_machine_unpaired, update_cassette_config, update_dca_client, update_deposit, @@ -297,15 +304,21 @@ async def api_pair_machine( npub, so lnbits' path-B roster routes the spire's cash-out RPCs to this operator's wallet — no nsec ever lands on the spire. - Re-pair is supported (re-issues a token for the same spire key). Token - revocation + expiry are gated on aiolabs/lnbits#54 (admin-client gaps).""" + Re-pair is supported (re-issues a token for the same spire key). + `duration_hours` (optional) time-bounds the token; revoke via the + sibling `POST .../revoke` endpoint.""" machine = await _machine_owned_by(machine_id, user.id) if not data.relays: raise HTTPException(HTTPStatus.BAD_REQUEST, "at least one relay is required") try: async with NsecBunkerAdminClient.from_settings() as client: - result = await pair_spire(machine, relays=data.relays, admin_client=client) + result = await pair_spire( + machine, + relays=data.relays, + admin_client=client, + duration_hours=data.duration_hours, + ) except NsecBunkerNotConfiguredError as exc: raise HTTPException( HTTPStatus.SERVICE_UNAVAILABLE, @@ -327,6 +340,34 @@ async def api_pair_machine( return result +@spirekeeper_api_router.post( + "/api/v1/dca/machines/{machine_id}/revoke", response_model=RevokeResult +) +async def api_revoke_machine( + machine_id: str, + user: User = Depends(check_user_exists), +) -> RevokeResult: + """Revoke a spire's bunker access — the "Revoke spire access" UX + (#9/#12). Cuts the spire's signing ability at the bunker + (`KeyUser.revokedAt` via `revoke_key_user`; token-revoke alone is a + no-op once the token is redeemed — see #22), then marks the machine + unpaired. `revoked_count` >= 1 = access cut; 0 = nothing was bound.""" + machine = await _machine_owned_by(machine_id, user.id) + try: + async with NsecBunkerAdminClient.from_settings() as client: + revoked_count = await revoke_spire(machine, admin_client=client) + except NsecBunkerNotConfiguredError as exc: + raise HTTPException( + HTTPStatus.SERVICE_UNAVAILABLE, + f"nsecbunkerd is not configured on this LNbits instance: {exc}", + ) from exc + except (PairingError, NsecBunkerError) as exc: + raise HTTPException(HTTPStatus.BAD_GATEWAY, f"revoke failed: {exc}") from exc + + await set_machine_unpaired(machine_id) + return RevokeResult(revoked_count=revoked_count) + + @spirekeeper_api_router.get("/api/v1/dca/machines", response_model=list[Machine]) async def api_list_machines( user: User = Depends(check_user_exists),