diff --git a/desktop/src-tauri/src/commands/admin/mod.rs b/desktop/src-tauri/src/commands/admin/mod.rs index 010f9760b..cd69bb60d 100644 --- a/desktop/src-tauri/src/commands/admin/mod.rs +++ b/desktop/src-tauri/src/commands/admin/mod.rs @@ -172,14 +172,16 @@ async fn admin_probe_inner( return Ok(AdminProbeResult::NetworkOrIntercepted); } - // Step 3: success without auth → disabled mode (if body is a valid probe). + // Step 3: success without auth → disabled mode (only when the body is a + // coherent disabled-mode probe: status ok, authMode disabled, no + // principal, no capabilities). A 200 in any other shape or mode is a + // contract violation (token/nip98 must 401 an unauthenticated caller); + // classify defensively. if resp.status().is_success() { let content_type = response_content_type(&resp); let bytes = read_bounded(resp, PROBE_JSON_CAP).await?; return Ok(match parse_probe(&content_type, &bytes) { - Some(p) if p.auth_mode == "disabled" => AdminProbeResult::Disabled, - // A 200 in any other mode is a contract violation (token/nip98 - // must 401 an unauthenticated caller); classify defensively. + Some(p) if p.is_coherent_disabled() => AdminProbeResult::Disabled, _ => AdminProbeResult::NotAdminApi, }); } @@ -223,11 +225,16 @@ async fn admin_probe_inner( let content_type = response_content_type(&auth_resp); let bytes = read_bounded(auth_resp, PROBE_JSON_CAP).await?; return Ok(match parse_probe(&content_type, &bytes) { - // Relay resolves role/source for the authenticated - // principal; carry them through for the staffing tab. - Some(p) => AdminProbeResult::Nip98Authorized { - role: p.role, - source: p.source, + // Trust the 2xx only when the full NIP-98 invariant holds; + // carry the relay-resolved role/source for the staffing tab. + Some(p) => match p.authorized_principal() { + Some((role, source)) => AdminProbeResult::Nip98Authorized { + role: Some(role.as_str().to_string()), + source: Some(source.as_str().to_string()), + }, + // Structurally a probe body but not a coherent + // authorized NIP-98 response: fail closed. + None => AdminProbeResult::NotAdminApi, }, // 2xx but not a probe shape: endpoint exists but isn't // the admin API. @@ -296,23 +303,109 @@ async fn read_bounded(resp: reqwest::Response, cap: u64) -> Result, Stri /// The relay's `/probe` response contract (`ProbeResponse` in the relay's /// `api/admin/mod.rs`, serialised `rename_all = "camelCase"`). /// -/// Only the fields the desktop consumes are typed. `role`/`source` are -/// present (non-null) only in nip98 mode with a resolved principal; they are -/// `null` in token/disabled modes. +/// All six fields are required and typed; deserialisation rejects a body that +/// omits any field or carries a wrong-typed one, so an unrelated JSON endpoint +/// cannot be mistaken for the admin API. Unknown fields are tolerated for +/// forward compatibility. Structural validity alone does NOT authorize: a +/// deserialised `ProbeWire` still has to pass [`ProbeWire::authorized_principal`] +/// or [`ProbeWire::is_coherent_disabled`] before its state is trusted. #[derive(serde::Deserialize)] #[serde(rename_all = "camelCase")] struct ProbeWire { - #[allow(dead_code)] status: String, auth_mode: String, role: Option, source: Option, - #[allow(dead_code)] can_act: bool, - #[allow(dead_code)] can_staff: bool, } +/// The relay's resolved principal role. Closed vocabulary — an unknown string +/// fails to parse and denies authorization. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum ProbeRole { + Operator, + Moderator, +} + +impl ProbeRole { + fn parse(s: &str) -> Option { + match s { + "operator" => Some(Self::Operator), + "moderator" => Some(Self::Moderator), + _ => None, + } + } + + fn as_str(self) -> &'static str { + match self { + Self::Operator => "operator", + Self::Moderator => "moderator", + } + } +} + +/// How the relay established the principal's role. Closed vocabulary. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum ProbeSource { + Config, + OwnerFallback, + Db, +} + +impl ProbeSource { + fn parse(s: &str) -> Option { + match s { + "config" => Some(Self::Config), + "owner_fallback" => Some(Self::OwnerFallback), + "db" => Some(Self::Db), + _ => None, + } + } + + fn as_str(self) -> &'static str { + match self { + Self::Config => "config", + Self::OwnerFallback => "owner_fallback", + Self::Db => "db", + } + } +} + +impl ProbeWire { + /// Validate the complete NIP-98 authorization invariant and return the + /// typed principal only when every field is coherent with the relay + /// contract: `status == "ok"`, `authMode == "nip98"`, a recognised + /// non-null `role`/`source`, `canAct == true`, and + /// `canStaff == (role == operator)`. Any deviation yields `None`, so the + /// caller classifies the response `NotAdminApi` rather than trusting a + /// fail-open 2xx. + fn authorized_principal(&self) -> Option<(ProbeRole, ProbeSource)> { + if self.status != "ok" || self.auth_mode != "nip98" { + return None; + } + let role = ProbeRole::parse(self.role.as_deref()?)?; + let source = ProbeSource::parse(self.source.as_deref()?)?; + if !self.can_act || self.can_staff != (role == ProbeRole::Operator) { + return None; + } + Some((role, source)) + } + + /// Validate the disabled-mode invariant for an unauthenticated 200: + /// `status == "ok"`, `authMode == "disabled"`, no principal, no + /// capabilities. Any deviation is a contract violation (token/nip98 must + /// 401 an unauthenticated caller). + fn is_coherent_disabled(&self) -> bool { + self.status == "ok" + && self.auth_mode == "disabled" + && self.role.is_none() + && self.source.is_none() + && !self.can_act + && !self.can_staff + } +} + /// Parse a `/probe` response body into a [`ProbeWire`], returning `None` when /// the Content-Type is not JSON or the body does not match the probe contract. /// diff --git a/desktop/src-tauri/src/commands/admin/mod_tests.rs b/desktop/src-tauri/src/commands/admin/mod_tests.rs index 845780b20..92950c231 100644 --- a/desktop/src-tauri/src/commands/admin/mod_tests.rs +++ b/desktop/src-tauri/src/commands/admin/mod_tests.rs @@ -153,6 +153,91 @@ fn parse_probe_rejects_non_object() { assert!(parse_probe("application/json", b"not json").is_none()); } +// ── authorized_principal / is_coherent_disabled invariants ──────────────── +// +// Structural deserialisation (parse_probe) is necessary but not sufficient: +// a 2xx body must also satisfy the full relay contract before its state is +// trusted. These pin every branch of that invariant. + +/// Parse a body known to be structurally valid, then validate it. +fn authorized(body: &str) -> Option<(ProbeRole, ProbeSource)> { + parse_probe("application/json", body.as_bytes()) + .expect("structurally valid probe") + .authorized_principal() +} + +#[test] +fn authorized_principal_accepts_every_coherent_shape() { + // Operator (canStaff true) and moderator (canStaff false) across all three + // recognised sources — each must yield the typed principal. + let cases: &[(String, ProbeRole, ProbeSource)] = &[ + ( + probe_json("nip98", r#""operator""#, r#""config""#, true, true), + ProbeRole::Operator, + ProbeSource::Config, + ), + ( + probe_json("nip98", r#""operator""#, r#""owner_fallback""#, true, true), + ProbeRole::Operator, + ProbeSource::OwnerFallback, + ), + ( + probe_json("nip98", r#""moderator""#, r#""db""#, true, false), + ProbeRole::Moderator, + ProbeSource::Db, + ), + ]; + for (body, role, source) in cases { + assert_eq!(authorized(body), Some((*role, *source))); + } +} + +#[test] +fn authorized_principal_rejects_every_incoherent_shape() { + // Structurally valid probe bodies the relay never emits under nip98; + // accepting any is fail-open. One invariant broken per row, top to bottom: + // wrong status, non-nip98 authMode, missing role, unknown role, missing + // source, unknown source, false canAct, operator lacking canStaff, + // moderator carrying canStaff. + let pj = probe_json; + let cases = [ + pj("nip98", r#""operator""#, r#""config""#, true, true) + .replace(r#""status":"ok""#, r#""status":"error""#), + pj("token", "null", "null", false, false), + pj("nip98", "null", r#""config""#, true, true), + pj("nip98", r#""superuser""#, r#""config""#, true, true), + pj("nip98", r#""operator""#, "null", true, true), + pj("nip98", r#""operator""#, r#""ldap""#, true, true), + pj("nip98", r#""operator""#, r#""config""#, false, true), + pj("nip98", r#""operator""#, r#""config""#, true, false), + pj("nip98", r#""moderator""#, r#""config""#, true, true), + ]; + for (i, body) in cases.iter().enumerate() { + assert_eq!(authorized(body), None, "row {i} must be rejected"); + } +} + +#[test] +fn is_coherent_disabled_accepts_only_canonical_disabled() { + // Canonical disabled authorizes; nip98 mode and any disabled body claiming + // a role/source or a capability is incoherent and must be rejected. + let disabled = probe_json("disabled", "null", "null", false, false); + assert!(parse_probe("application/json", disabled.as_bytes()) + .unwrap() + .is_coherent_disabled()); + + let incoherent = [ + probe_json("nip98", r#""operator""#, r#""config""#, true, true), + probe_json("disabled", r#""operator""#, "null", false, false), + probe_json("disabled", "null", "null", true, false), + ]; + for body in &incoherent { + assert!(!parse_probe("application/json", body.as_bytes()) + .unwrap() + .is_coherent_disabled()); + } +} + // ── Storage core through production code ───────────────────────────────── // // All tests call `get_admin_origin_core` / `set_admin_origin_core` directly diff --git a/desktop/src/features/admin-console/AdminConsoleFeedbackTab.tsx b/desktop/src/features/admin-console/AdminConsoleFeedbackTab.tsx index ed9015f61..bef695799 100644 --- a/desktop/src/features/admin-console/AdminConsoleFeedbackTab.tsx +++ b/desktop/src/features/admin-console/AdminConsoleFeedbackTab.tsx @@ -25,6 +25,7 @@ import { import { type AsyncState, type AttachmentMeta, + adminErrorMessage, CommunityGroupedList, DetailRow, ErrorMessage, @@ -331,20 +332,18 @@ function FeedbackStatusControl({ onStatusChanged: (newStatus: AdminFeedbackStatus) => void; }) { const [isWorking, setIsWorking] = useState(false); - const [error, setError] = useState(null); const statuses: AdminFeedbackStatus[] = ["new", "reviewed", "archived"]; const handleStatusChange = async (newStatus: AdminFeedbackStatus) => { if (newStatus === currentStatus) return; - setError(null); setIsWorking(true); try { await patchAdminFeedback(origin, feedbackId, newStatus); toast.success(`Feedback marked ${newStatus}`); onStatusChanged(newStatus); } catch (e) { - setError(e instanceof Error ? e.message : String(e)); + toast.error(adminErrorMessage(e)); } finally { setIsWorking(false); } @@ -375,7 +374,6 @@ function FeedbackStatusControl({ ))} - {error &&

{error}

} ); } diff --git a/desktop/src/features/admin-console/adminConsolePanelEvents.jsdom-test.mjs b/desktop/src/features/admin-console/adminConsolePanelEvents.jsdom-test.mjs index d118759cf..e9bdda6da 100644 --- a/desktop/src/features/admin-console/adminConsolePanelEvents.jsdom-test.mjs +++ b/desktop/src/features/admin-console/adminConsolePanelEvents.jsdom-test.mjs @@ -3099,3 +3099,92 @@ test("feedback-list-refetches-on-back-after-mutation: changing status then navig await unmount(); }); + +test("resolve-rejection-surfaces-parsed-message: a rejected resolve toasts the relay message, not raw JSON", async () => { + // A resolve mutation that the relay rejects (e.g. invalid_action_for_target) + // must surface the envelope's human message via toast.error — never the raw + // JSON envelope and never a success toast. + // + // Mutation evidence: replace `toast.error(adminErrorMessage(e))` in + // handleSubmit with `toast.error(String(e))` → the raw-JSON assertion goes + // red because the envelope leaks verbatim. + + const origin = "https://admin.example.com"; + const pubkey = "f7".repeat(32); + + const openItem = { + id: "00000000-0000-0000-0000-0000000000f7", + communityId: "comm-1", + communityHost: "alpha.example.com", + reportEventId: "aa", + reporterPubkey: "bb", + targetKind: "event", + target: "cc", + reportType: "spam", + status: "open", + createdAt: "2024-06-01T12:00:00Z", + }; + const openDetail = { + ...openItem, + channelId: "00000000-0000-0000-0000-0000000000ff", + note: null, + resolvedBy: null, + resolvedAt: null, + actionId: null, + message: null, + }; + + const humanMessage = + "action kick requires the report to have an associated channel"; + // The native command rejects with `admin API error: {envelope}` — exactly the + // shape adminErrorMessage strips down to the envelope's `message`. + const rawError = `admin API error: {"error":{"code":"invalid_action_for_target","message":"${humanMessage}","requestId":"00000000-0000-0000-0000-0000000000e7"}}`; + + setIpcHandler("admin_list_reports", () => Promise.resolve([openItem])); + setIpcHandler("admin_get_report", () => Promise.resolve(openDetail)); + setIpcHandler("admin_list_feedback", () => Promise.resolve([])); + setIpcHandler("admin_resolve_report", () => + Promise.reject(new Error(rawError)), + ); + + const { container, doRender, unmount } = mountPanel({ origin, pubkey }); + await doRender(); + await settle(30); + await openFirstReportDetail(container); + await settle(20); + + // Select the kick action, then submit — the relay rejects it. + const kickBtn = container.querySelector("[data-testid='action-btn-kick']"); + assert.ok(kickBtn, "kick action must be present (channel is set)"); + await act(async () => { + fireEvent.click(kickBtn); + await new Promise((r) => setTimeout(r, 10)); + }); + const submit = container.querySelector("[data-testid='resolve-submit-btn']"); + assert.ok(submit, "resolve submit button must appear after selecting kick"); + await act(async () => { + fireEvent.click(submit); + await new Promise((r) => setTimeout(r, 30)); + }); + await settle(20); + + // The parsed human message reaches toast.error. + assert.ok( + capturedErrorToasts.some((m) => m.includes(humanMessage)), + `the parsed relay message must surface via toast.error; got: ${JSON.stringify(capturedErrorToasts)}`, + ); + // The raw JSON envelope must NOT leak into any error toast. + assert.ok( + !capturedErrorToasts.some( + (m) => m.includes('{"error"') || m.includes("admin API error:"), + ), + `the raw JSON envelope must not appear in an error toast; got: ${JSON.stringify(capturedErrorToasts)}`, + ); + // No success toast on a rejected resolve. + assert.ok( + !capturedToasts.some((m) => m.toLowerCase().includes("resolved")), + `no success toast on a rejected resolve; got: ${JSON.stringify(capturedToasts)}`, + ); + + await unmount(); +});