diff --git a/routstr/core/admin.py b/routstr/core/admin.py index 71a6e348..7823fd99 100644 --- a/routstr/core/admin.py +++ b/routstr/core/admin.py @@ -20,6 +20,7 @@ from ..wallet import ( send_token, slow_filter_spend_proofs, ) +from . import vault from .db import ( ApiKey, CashuTransaction, @@ -28,6 +29,9 @@ from .db import ( ModelRow, UpstreamProviderRow, create_session, + get_secret, + set_admin_password, + set_nsec, ) from .db import ( store_cashu_transaction_with_retry as store_cashu_transaction, @@ -35,7 +39,7 @@ from .db import ( from .log_manager import log_manager from .logging import get_logger from .provider_slugs import allocate_unique_provider_slug -from .settings import SettingsService, settings +from .settings import SettingsService, derive_npub_from_nsec, settings logger = get_logger(__name__) @@ -206,8 +210,6 @@ async def get_settings(request: Request) -> dict: data = settings.dict() if "upstream_api_key" in data: data["upstream_api_key"] = "[REDACTED]" if data["upstream_api_key"] else "" - if "admin_password" in data: - data["admin_password"] = "[REDACTED]" if data["admin_password"] else "" if "nsec" in data: data["nsec"] = "[REDACTED]" if data["nsec"] else "" return data @@ -224,9 +226,10 @@ class PasswordUpdate(BaseModel): @admin_router.patch("/api/settings", dependencies=[Depends(require_admin_api)]) async def update_settings(request: Request, update: SettingsUpdate) -> dict: - # Remove sensitive fields from general settings update + # Secrets are not editable through the general settings endpoint; they have + # dedicated rotation paths and never reach the settings blob. settings_data = update.root.copy() - sensitive_fields = ["admin_password", "upstream_api_key", "nsec"] + sensitive_fields = ["upstream_api_key", "nsec"] for field in sensitive_fields: if field in settings_data: del settings_data[field] @@ -241,8 +244,6 @@ async def update_settings(request: Request, update: SettingsUpdate) -> dict: data = new_settings.dict() if "upstream_api_key" in data: data["upstream_api_key"] = "[REDACTED]" if data["upstream_api_key"] else "" - if "admin_password" in data: - data["admin_password"] = "[REDACTED]" if data["admin_password"] else "" if "nsec" in data: data["nsec"] = "[REDACTED]" if data["nsec"] else "" return data @@ -250,43 +251,85 @@ async def update_settings(request: Request, update: SettingsUpdate) -> dict: @admin_router.patch("/api/password", dependencies=[Depends(require_admin_api)]) async def update_password(request: Request, password_update: PasswordUpdate) -> dict: - current_password = settings.admin_password - - if not current_password: - raise HTTPException(status_code=500, detail="Admin password not configured") - - if password_update.current_password != current_password: - raise HTTPException(status_code=401, detail="Current password is incorrect") - - # Validate new password - new_password = password_update.new_password.strip() - if len(new_password) < 6: - raise HTTPException( - status_code=400, detail="New password must be at least 6 characters" - ) - - # Update password async with create_session() as session: - await SettingsService.update({"admin_password": new_password}, session) + secret = await get_secret(session) + + if not secret.admin_password_hash: + raise HTTPException( + status_code=500, detail="Admin password not configured" + ) + + if not vault.verify_password( + password_update.current_password, secret.admin_password_hash + ): + raise HTTPException( + status_code=401, detail="Current password is incorrect" + ) + + # Validate new password + new_password = password_update.new_password.strip() + if len(new_password) < vault.MIN_PASSWORD_LENGTH: + raise HTTPException( + status_code=400, + detail=( + "New password must be at least " + f"{vault.MIN_PASSWORD_LENGTH} characters" + ), + ) + + await set_admin_password(session, new_password) return {"ok": True, "message": "Password updated successfully"} +class NsecUpdate(BaseModel): + nsec: str + + +@admin_router.patch("/api/nsec", dependencies=[Depends(require_admin_api)]) +async def update_nsec(request: Request, payload: NsecUpdate) -> dict[str, object]: + # The node's Nostr identity is a secret: it is stored encrypted in the + # Secret store, never in the settings blob, so it gets its own endpoint + # rather than riding the general settings PATCH (which strips it). An empty + # nsec clears the identity. + nsec = payload.nsec.strip() + npub = "" + if nsec: + derived = derive_npub_from_nsec(nsec) + if not derived: + raise HTTPException(status_code=400, detail="Invalid nsec") + npub = derived + + async with create_session() as session: + await set_nsec(session, nsec) + + # Reflect the change in the live runtime so Nostr signing/announcements pick + # it up without a restart (mirrors what bootstrap_secrets sets at boot). + settings.nsec = nsec + settings.npub = npub + return {"ok": True, "npub": npub} + + class SetupRequest(BaseModel): password: str @admin_router.post("/api/setup") async def initial_setup(request: Request, payload: SetupRequest) -> dict[str, object]: - if settings.admin_password: - raise HTTPException(status_code=409, detail="Admin password already set") - pw = (payload.password or "").strip() - if len(pw) < 8: - raise HTTPException( - status_code=400, detail="Password must be at least 8 characters" - ) async with create_session() as session: - await SettingsService.update({"admin_password": pw}, session) + secret = await get_secret(session) + if secret.admin_password_hash: + raise HTTPException(status_code=409, detail="Admin password already set") + pw = (payload.password or "").strip() + if len(pw) < vault.MIN_PASSWORD_LENGTH: + raise HTTPException( + status_code=400, + detail=( + "Password must be at least " + f"{vault.MIN_PASSWORD_LENGTH} characters" + ), + ) + await set_admin_password(session, pw) return {"ok": True} @@ -298,12 +341,13 @@ class AdminLoginRequest(BaseModel): async def admin_login( request: Request, payload: AdminLoginRequest ) -> dict[str, object]: - admin_pw = settings.admin_password + async with create_session() as session: + secret = await get_secret(session) - if not admin_pw: + if not secret.admin_password_hash: raise HTTPException(status_code=500, detail="Admin password not configured") - if payload.password != admin_pw: + if not vault.verify_password(payload.password, secret.admin_password_hash): raise HTTPException(status_code=401, detail="Invalid password") token = secrets.token_urlsafe(32) diff --git a/tests/integration/test_admin_auth.py b/tests/integration/test_admin_auth.py new file mode 100644 index 00000000..e7743d45 --- /dev/null +++ b/tests/integration/test_admin_auth.py @@ -0,0 +1,141 @@ +"""Tests for admin password auth backed by the hashed Secret store (issue #553). + +Login, password change and first-run setup verify against the one-way +``Secret.admin_password_hash`` (scrypt) instead of a plaintext settings field, +which also closes the old ``!=`` timing-attack comparison. The flow is exercised +end-to-end through the public admin endpoints: setup writes the first hash, +login checks against it, and a password change re-hashes so the old password +stops working and the new one starts. +""" + +from __future__ import annotations + +import pytest +from httpx import AsyncClient, Response + + +async def _setup_password(client: AsyncClient, password: str) -> None: + resp = await client.post("/admin/api/setup", json={"password": password}) + assert resp.status_code == 200, resp.text + + +async def _login(client: AsyncClient, password: str) -> Response: + return await client.post("/admin/api/login", json={"password": password}) + + +# --- login ----------------------------------------------------------------- + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_login_500_when_no_password_configured( + integration_client: AsyncClient, +) -> None: + resp = await _login(integration_client, "anything") + assert resp.status_code == 500 + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_login_succeeds_with_correct_password( + integration_client: AsyncClient, +) -> None: + await _setup_password(integration_client, "correct horse") + resp = await _login(integration_client, "correct horse") + assert resp.status_code == 200 + body = resp.json() + assert body["ok"] is True + assert isinstance(body["token"], str) and body["token"] + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_login_rejects_wrong_password( + integration_client: AsyncClient, +) -> None: + await _setup_password(integration_client, "correct horse") + resp = await _login(integration_client, "wrong horse") + assert resp.status_code == 401 + + +# --- first-run setup ------------------------------------------------------- + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_setup_rejects_short_password( + integration_client: AsyncClient, +) -> None: + resp = await integration_client.post("/admin/api/setup", json={"password": "short"}) + assert resp.status_code == 400 + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_setup_409_when_password_already_set( + integration_client: AsyncClient, +) -> None: + await _setup_password(integration_client, "first password") + resp = await integration_client.post( + "/admin/api/setup", json={"password": "second password"} + ) + assert resp.status_code == 409 + + +# --- password change ------------------------------------------------------- + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_update_password_rehashes_so_only_new_works( + integration_client: AsyncClient, +) -> None: + await _setup_password(integration_client, "old password") + login = await _login(integration_client, "old password") + token = login.json()["token"] + integration_client.headers["Authorization"] = f"Bearer {token}" + + resp = await integration_client.patch( + "/admin/api/password", + json={"current_password": "old password", "new_password": "new password"}, + ) + assert resp.status_code == 200, resp.text + + # Drop admin auth so the login calls aren't treated as authenticated noise. + integration_client.headers.pop("Authorization", None) + assert (await _login(integration_client, "old password")).status_code == 401 + assert (await _login(integration_client, "new password")).status_code == 200 + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_update_password_rejects_wrong_current( + integration_client: AsyncClient, +) -> None: + await _setup_password(integration_client, "old password") + login = await _login(integration_client, "old password") + token = login.json()["token"] + integration_client.headers["Authorization"] = f"Bearer {token}" + + resp = await integration_client.patch( + "/admin/api/password", + json={"current_password": "not the password", "new_password": "new password"}, + ) + assert resp.status_code == 401 + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_update_password_rejects_short_new( + integration_client: AsyncClient, +) -> None: + await _setup_password(integration_client, "old password") + login = await _login(integration_client, "old password") + token = login.json()["token"] + integration_client.headers["Authorization"] = f"Bearer {token}" + + resp = await integration_client.patch( + "/admin/api/password", + json={"current_password": "old password", "new_password": "x"}, + ) + assert resp.status_code == 400 diff --git a/tests/integration/test_admin_nsec_endpoint.py b/tests/integration/test_admin_nsec_endpoint.py new file mode 100644 index 00000000..23d11c36 --- /dev/null +++ b/tests/integration/test_admin_nsec_endpoint.py @@ -0,0 +1,113 @@ +"""Tests for the admin nsec rotation endpoint (issue #553). + +The Nostr identity is a secret: it lives encrypted in the Secret store, never in +the settings blob, so it cannot be set through the general settings PATCH. This +dedicated endpoint is the supported way to set/rotate/clear it — it encrypts the +key at rest, updates the live runtime identity (so signing picks it up without a +restart), and derives the npub. Invalid keys are rejected. +""" + +from __future__ import annotations + +import secrets +import time +from collections.abc import AsyncGenerator + +import pytest +import pytest_asyncio +from httpx import AsyncClient + +from routstr.core import vault +from routstr.core.admin import admin_sessions +from routstr.core.db import AsyncSession, get_secret +from routstr.core.settings import derive_npub_from_nsec, settings + +# A valid 64-char hex private key (accepted by nsec_to_keypair, as in bootstrap). +NSEC_HEX = "1" * 64 + + +@pytest_asyncio.fixture +async def admin_client( + integration_client: AsyncClient, +) -> AsyncGenerator[AsyncClient, None]: + """An integration_client pre-authenticated with an admin session token.""" + token = secrets.token_urlsafe(24) + admin_sessions[token] = int(time.time()) + 3600 + integration_client.headers["Authorization"] = f"Bearer {token}" + yield integration_client + admin_sessions.pop(token, None) + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_update_nsec_stores_encrypted_and_derives_npub( + admin_client: AsyncClient, + integration_session: AsyncSession, + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(settings, "nsec", "") + monkeypatch.setattr(settings, "npub", "") + + resp = await admin_client.patch("/admin/api/nsec", json={"nsec": NSEC_HEX}) + assert resp.status_code == 200 + + expected_npub = derive_npub_from_nsec(NSEC_HEX) + assert resp.json() == {"ok": True, "npub": expected_npub} + + # Stored encrypted at rest, decryptable back to the original key. + integration_session.expunge_all() + secret = await get_secret(integration_session) + assert secret.encrypted_nsec is not None + assert vault.is_encrypted(secret.encrypted_nsec) + assert vault.decrypt(secret.encrypted_nsec) == NSEC_HEX + + # Live runtime identity updated so Nostr signing reflects it without restart. + assert settings.nsec == NSEC_HEX + assert settings.npub == expected_npub + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_update_nsec_rejects_invalid_key( + admin_client: AsyncClient, + integration_session: AsyncSession, + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(settings, "nsec", "") + monkeypatch.setattr(settings, "npub", "") + + resp = await admin_client.patch( + "/admin/api/nsec", json={"nsec": "not-a-real-nsec"} + ) + assert resp.status_code == 400 + + # Nothing stored, live identity untouched. + integration_session.expunge_all() + secret = await get_secret(integration_session) + assert secret.encrypted_nsec is None + assert settings.nsec == "" + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_update_nsec_clears_identity_with_empty_value( + admin_client: AsyncClient, + integration_session: AsyncSession, + monkeypatch: pytest.MonkeyPatch, +) -> None: + # Start from a node that has an identity... + monkeypatch.setattr(settings, "nsec", "") + monkeypatch.setattr(settings, "npub", "") + set_resp = await admin_client.patch("/admin/api/nsec", json={"nsec": NSEC_HEX}) + assert set_resp.status_code == 200 + + # ...then clear it. + clear_resp = await admin_client.patch("/admin/api/nsec", json={"nsec": ""}) + assert clear_resp.status_code == 200 + assert clear_resp.json() == {"ok": True, "npub": ""} + + integration_session.expunge_all() + secret = await get_secret(integration_session) + assert secret.encrypted_nsec is None + assert settings.nsec == "" + assert settings.npub == "" diff --git a/tests/integration/test_admin_settings_endpoint.py b/tests/integration/test_admin_settings_endpoint.py new file mode 100644 index 00000000..c97a9a97 --- /dev/null +++ b/tests/integration/test_admin_settings_endpoint.py @@ -0,0 +1,82 @@ +"""Tests for the admin settings endpoint's handling of secrets (issue #553). + +``admin_password`` is no longer a settings field (it lives only as a one-way +hash in the Secret store), so it must never appear in the GET/PATCH payloads. +``nsec`` and ``upstream_api_key`` remain live in-memory runtime values but are +redacted on read and ignored on write — they cannot be set through the general +settings endpoint, only through their dedicated rotation paths. +""" + +from __future__ import annotations + +import secrets +import time +from collections.abc import AsyncGenerator + +import pytest +import pytest_asyncio +from httpx import AsyncClient + +from routstr.core.admin import admin_sessions +from routstr.core.db import AsyncSession +from routstr.core.settings import SettingsService, settings + + +@pytest_asyncio.fixture +async def admin_client( + integration_client: AsyncClient, +) -> AsyncGenerator[AsyncClient, None]: + """An integration_client pre-authenticated with an admin session token.""" + token = secrets.token_urlsafe(24) + admin_sessions[token] = int(time.time()) + 3600 + integration_client.headers["Authorization"] = f"Bearer {token}" + yield integration_client + admin_sessions.pop(token, None) + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_get_settings_omits_admin_password_and_redacts_secrets( + admin_client: AsyncClient, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.setattr(settings, "nsec", "nsec-secret") + monkeypatch.setattr(settings, "upstream_api_key", "sk-secret") + + resp = await admin_client.get("/admin/api/settings") + assert resp.status_code == 200 + + data = resp.json() + assert "admin_password" not in data + assert data["nsec"] == "[REDACTED]" + assert data["upstream_api_key"] == "[REDACTED]" + + +@pytest.mark.integration +@pytest.mark.asyncio +async def test_patch_settings_ignores_secret_fields( + admin_client: AsyncClient, + integration_session: AsyncSession, + monkeypatch: pytest.MonkeyPatch, +) -> None: + # The PATCH path persists through SettingsService, which needs an + # initialized current snapshot and a settings row in the shared test DB. + await SettingsService.initialize(integration_session) + monkeypatch.setattr(settings, "nsec", "original-nsec") + + resp = await admin_client.patch( + "/admin/api/settings", + json={ + "name": "Renamed", + "nsec": "attacker-nsec", + "upstream_api_key": "attacker-key", + "admin_password": "attacker-pw", + }, + ) + assert resp.status_code == 200 + + data = resp.json() + assert data["name"] == "Renamed" + assert "admin_password" not in data + assert data["nsec"] == "[REDACTED]" + # The live secret was not overwritten through the general settings endpoint. + assert settings.nsec == "original-nsec" diff --git a/ui/components/settings/admin-settings.tsx b/ui/components/settings/admin-settings.tsx index f5ee51d9..68045b3f 100644 --- a/ui/components/settings/admin-settings.tsx +++ b/ui/components/settings/admin-settings.tsx @@ -116,9 +116,24 @@ export function AdminSettings() { setSaving(true); setError(''); - const updatedData = await AdminService.updateSettings(settings); - setSettings(updatedData as SettingsData); - setInitialSettings(updatedData as SettingsData); + // The nsec is a secret with its own endpoint (the general settings PATCH + // strips it); only send it when the operator actually changed it, so an + // untouched redacted value is never written back. The npub is derived + // server-side from the new nsec — fold it into the settings payload so the + // persisted blob stays consistent with the stored key. + let settingsPayload = settings; + if (hasFieldChanged('nsec')) { + const result = await AdminService.updateNsec( + (settings.nsec as string) || '' + ); + settingsPayload = { ...settings, npub: result.npub }; + } + + const updatedData = (await AdminService.updateSettings( + settingsPayload + )) as SettingsData; + setSettings(updatedData); + setInitialSettings(updatedData); toast.success('Settings saved successfully'); } catch (err) { const message = @@ -140,8 +155,8 @@ export function AdminSettings() { return; } - if (passwordData.new_password.length < 6) { - setPasswordError('New password must be at least 6 characters'); + if (passwordData.new_password.length < 8) { + setPasswordError('New password must be at least 8 characters'); return; } @@ -944,7 +959,7 @@ export function AdminSettings() { new_password: e.target.value, })) } - placeholder='Enter new password (min 6 characters)' + placeholder='Enter new password (min 8 characters)' /> diff --git a/ui/lib/api/services/admin.ts b/ui/lib/api/services/admin.ts index 5fa168cf..13c4366d 100644 --- a/ui/lib/api/services/admin.ts +++ b/ui/lib/api/services/admin.ts @@ -801,6 +801,15 @@ export class AdminService { ); } + static async updateNsec( + nsec: string + ): Promise<{ ok: boolean; npub: string }> { + return await apiClient.patch<{ ok: boolean; npub: string }>( + '/admin/api/nsec', + { nsec } + ); + } + static async login(password: string): Promise<{ ok: boolean; token: string;