From ee8edd370979db0cb48b1258eff274a6b41b00a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Tue, 25 Aug 2026 16:51:31 +0000 Subject: [PATCH] the settings form, and the branch that never ran MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit the plugin could not be configured. /_config had GET, PUT, DELETE and a test that validates without storing, and nothing rendered any of it — the connection had to be inserted by hand. ConnectionPanel is both the wizard and the settings page, one component, the same choice jellyfin's ConnectionSection makes and for the same reason: a connection edited months later goes through exactly the path that stored the first one. HostsPanel shows it when nothing is configured and on ?config=1, which is in the URL so it survives a refresh. test comes before save and stores nothing, so a wrong password cannot replace a working one. the secret field starts empty even when one is stored, and empty means keep — that is what lets a typo in the URL be fixed without going to find the password. and while wiring the unconfigured case, it turned out that case never fired. both panels read `(error as { body?: Failure }).body` to decide whether a failure was unconfigured, unauthenticated or unreachable. useClient throws ApiError(status, message) and has no `.body` at all, so that was always undefined: every failure fell through to the generic message, and the careful explanation of NPM's 404-means-credential quirk was written, shipped and unreachable. asFailure now reads the STATUS, which the router already chooses deliberately — 409 nothing stored, 503 no answer, 502 answered and refused. owner-only is explained rather than reported. a member hitting 403 here reads why: it is an admin login and NPM has no read-only role. still not installed, still no db:push. Co-Authored-By: Claude Opus 5 Officer-Session: 656f5c85-e49a-4f1f-b9f3-042a04b4d043 --- web/CertificatesPanel.tsx | 5 +- web/ConnectionPanel.tsx | 323 ++++++++++++++++++++++++++++++++++++++ web/HostsPanel.tsx | 60 ++++++- 3 files changed, 379 insertions(+), 9 deletions(-) create mode 100644 web/ConnectionPanel.tsx diff --git a/web/CertificatesPanel.tsx b/web/CertificatesPanel.tsx index 6848b81..f3df7ec 100644 --- a/web/CertificatesPanel.tsx +++ b/web/CertificatesPanel.tsx @@ -36,8 +36,9 @@ export const CertificatesPanel = () => { setCertificates(await clientRef.current.get("/proxy-manager/certificates")); setError(null); } catch (caught) { - const body = (caught as { body?: { error?: string } })?.body; - setError(body?.error ?? "Could not load certificates."); + // Not `.body` — `useClient` throws `ApiError(status, message)` and has no body. See `asFailure` in + // HostsPanel.tsx for the full account; this panel had the same dead read. + setError(caught instanceof Error ? caught.message : "Could not load certificates."); setCertificates(null); } }, []); diff --git a/web/ConnectionPanel.tsx b/web/ConnectionPanel.tsx new file mode 100644 index 0000000..4c65983 --- /dev/null +++ b/web/ConnectionPanel.tsx @@ -0,0 +1,323 @@ +import { useCallback, useEffect, useRef, useState } from 'react'; +import { CheckCircle2, Loader2, Plug, ShieldAlert, Trash2, TriangleAlert } from 'lucide-react'; +import { useClient } from 'hooks/useClient'; +import { Button } from '@/components/ui/button'; +import { Input } from '@/components/ui/input'; + +/** + * The connection to Nginx Proxy Manager — both the setup wizard and the permanent settings page. + * + * One component for both, the same choice Jellyfin's `ConnectionSection` makes and for the same reason: a + * connection edited months later goes through exactly the code path that stored the first one. `HostsPanel` + * renders this in place of itself while nothing is configured, and `?config=1` renders it for good. + * + * ── Owner only, and it says so rather than failing oddly ── + * + * The manifest marks `/_config` `ownerOnly`, so every request from this component 403s for anybody else — + * including the test button, deliberately, because that request carries the admin secret in its body. A + * member reaching this screen should read a sentence explaining that, not a raw refusal. + * + * ── The secret is never loaded ── + * + * `GET /_config` returns `hasSecret`, never the secret. So the field starts empty even when one is stored, + * and an empty field on save means "keep the stored one" — which is what lets a typo in the URL be fixed + * without going to find the password again. + */ + +type ConnectionView = { + configured: boolean; + url: string | null; + identity: string | null; + hasSecret: boolean; + version: string | null; + lastProbeAt: string | null; + lastProbeOk: boolean | null; +}; + +type TestResult = { + reachable: boolean; + authenticated: boolean; + version?: string; + setup?: boolean; + error?: string; + likelyAuth?: boolean; +}; + +const HINT = 'text-[11px] leading-relaxed text-muted-foreground'; + +/** + * NPM's admin UI port. Officer reaches it from the SERVER, so localhost here is the machine Officer runs + * on — and on a host where NPM is a container, its address on the docker network works whether or not the + * port is published. `docs/npm-api.md` has the measurements. + */ +const DEFAULT_URL = 'http://127.0.0.1:81'; + +const URL_HINT = + 'Where NPM answers, reached from the server rather than from this browser. A trailing slash is trimmed. ' + + 'If NPM runs as a container beside Officer, its address on the docker network works even with no ' + + 'published port.'; + +const SECRET_HINT = + 'The NPM admin password. Stored encrypted under this plugin’s own key and never returned by the ' + + 'API — not to a member, not to you. Leave it empty to keep the one already stored.'; + +type FieldProps = { + label: string; + hint?: string; + value: string; + onChange: (value: string) => void; + placeholder: string; + type?: string; + autoFocus?: boolean; +}; + +const Field = ({ label, hint, value, onChange, placeholder, type, autoFocus }: FieldProps) => ( + +); + +/** A 403 here means the account is not the owner — the one refusal worth explaining rather than reporting. */ +const isForbidden = (error: unknown): boolean => (error as { status?: number })?.status === 403; + +/** + * `useClient` throws `ApiError(status, message)` and nothing else — there is no `.body` to read, which is + * the bug `HostsPanel`'s `asFailure` documents at length. The message is already the server's own text: + * since 2026-08-25 the platform answers `{ error }` and the client unwraps it before throwing. + */ +const errorText = (error: unknown): string => + error instanceof Error ? error.message : 'Something went wrong.'; + +export const ConnectionPanel = () => { + const client = useClient(); + // useClient() rebuilds its verbs every render, so it must never sit in a dependency list. + const clientRef = useRef(client); + clientRef.current = client; + + const [view, setView] = useState(null); + const [forbidden, setForbidden] = useState(false); + const [url, setUrl] = useState(''); + const [identity, setIdentity] = useState(''); + const [secret, setSecret] = useState(''); + const [test, setTest] = useState(null); + const [error, setError] = useState(null); + const [busy, setBusy] = useState<'test' | 'save' | 'delete' | null>(null); + const [confirming, setConfirming] = useState(false); + + const load = useCallback(async () => { + try { + const next = await clientRef.current.get('/proxy-manager/_config'); + setView(next); + setUrl(next.url ?? DEFAULT_URL); + setIdentity(next.identity ?? ''); + setForbidden(false); + } catch (err) { + if (isForbidden(err)) return setForbidden(true); + setError(errorText(err)); + } + }, []); + + useEffect(() => { + void load(); + }, [load]); + + const runTest = async () => { + setBusy('test'); + setError(null); + setTest(null); + try { + // The secret is sent only when the field was filled. Testing with the field empty checks the URL and + // nothing else, which is still useful — it is how you find out you are pointed at the wrong thing. + setTest( + await clientRef.current.post('/proxy-manager/_config/test', { + url, + identity: identity || undefined, + secret: secret || undefined, + }), + ); + } catch (err) { + setError(errorText(err)); + } finally { + setBusy(null); + } + }; + + const save = async () => { + setBusy('save'); + setError(null); + try { + const next = await clientRef.current.put('/proxy-manager/_config', { + url, + identity, + // Omitted rather than sent empty — the server reads "absent" as keep, and an empty string as a + // secret would be stored verbatim. + ...(secret ? { secret } : {}), + }); + setView(next); + setSecret(''); + setTest(null); + } catch (err) { + setError(errorText(err)); + } finally { + setBusy(null); + } + }; + + const disconnect = async () => { + setBusy('delete'); + setError(null); + try { + await clientRef.current.delete('/proxy-manager/_config'); + setConfirming(false); + setTest(null); + setSecret(''); + await load(); + } catch (err) { + setError(errorText(err)); + } finally { + setBusy(null); + } + }; + + if (forbidden) { + return ( +
+ +

Only the server owner can change this connection.

+

+ It is an Nginx Proxy Manager admin login, and NPM has no read-only role — anyone holding it can + redirect any hostname this machine serves. You can still see the hosts and certificates. +

+
+ ); + } + + if (!view) return
Loading…
; + + // An empty secret is fine on an existing connection and not on a new one — the server enforces this too, + // and would answer 400. Saying so here means the button explains itself instead of failing. + const canSave = !!url.trim() && !!identity.trim() && (view.hasSecret || !!secret); + + return ( +
+
+ + {view.configured ? 'Connection' : 'Connect Nginx Proxy Manager'} +
+ +
+ {view.configured && ( +
+
+ {view.lastProbeOk === false ? ( + + ) : ( + + )} + {view.url} + {view.version && NPM {view.version}} +
+

+ {view.identity} + {view.hasSecret ? ' · a secret is stored' : ' · no secret stored'} + {view.lastProbeAt ? ` · last checked ${new Date(view.lastProbeAt).toLocaleString()}` : ''} +

+
+ )} + + + + + + {test && ( +
+ {test.authenticated ? ( +

+ Reached NPM {test.version} and the credential works. +

+ ) : test.reachable ? ( + <> +

+ Reached NPM {test.version}, but not signed in. +

+

+ {test.setup === false + ? 'That NPM has not finished its own setup yet — create its first admin user in NPM before connecting.' + : (test.error ?? + 'Fill in the email and password to check them. NPM answers 404 when unauthenticated, so a "not found" here is almost always the credential.')} +

+ + ) : ( + <> +

+ Could not reach it. +

+

{test.error}

+ + )} +
+ )} + + {error &&

{error}

} + +
+ {/* Test comes first, and testing never stores anything — so a wrong password cannot replace a + working one just because someone wanted to check it. */} + + + + {view.configured && + (confirming ? ( + <> + + + + ) : ( + + ))} +
+ + {!view.configured && ( +

+ Nothing is created or changed in NPM by connecting. This reads what is there, and the only write + it ever makes is taking a host in or out of service. +

+ )} +
+
+ ); +}; diff --git a/web/HostsPanel.tsx b/web/HostsPanel.tsx index e55eb7b..eef42c6 100644 --- a/web/HostsPanel.tsx +++ b/web/HostsPanel.tsx @@ -1,7 +1,8 @@ import { useCallback, useEffect, useRef, useState } from "react"; -import { AlertTriangle, Globe, Lock, LockOpen, Power, RefreshCw, ShieldCheck, Users } from "lucide-react"; +import { AlertTriangle, Globe, Lock, LockOpen, Power, RefreshCw, Settings, ShieldCheck, Users } from "lucide-react"; import { useClient } from "hooks/useClient"; import { useSearchParams } from "react-router"; +import { ConnectionPanel } from "./ConnectionPanel"; /** * The proxy hosts, which is the thing anybody opens this plugin to see. @@ -26,6 +27,30 @@ type ProxyHost = { type Failure = { error: string; likelyAuth?: boolean; unreachable?: boolean; unconfigured?: boolean }; +/** + * Turn a thrown `ApiError` into the shape this panel branches on. + * + * **This reads the STATUS, not the body, and that is not a shortcut.** `useClient` throws + * `ApiError(status, message)` — there is no `.body` on it, and there never was. This panel used to read + * `(error as { body?: Failure }).body`, which is always `undefined`, so `unconfigured` and `likelyAuth` + * were never once true and every failure fell through to the generic message. The careful explanation of + * NPM's 404 quirk was written, shipped, and unreachable. + * + * `api/router.ts` already encodes the distinction in the status it chooses, so it survives the trip: + * 409 is "no connection stored", 503 is "NPM did not answer", 502 is "NPM answered and refused". The + * message carries the detail, including the sentence about a 404 really meaning the credential. + */ +const asFailure = (error: unknown): Failure => { + const status = (error as { status?: number })?.status; + const message = error instanceof Error ? error.message : "Could not load hosts."; + return { + error: message, + unconfigured: status === 409, + unreachable: status === 503, + likelyAuth: status === 502, + }; +}; + export const HostsPanel = () => { const client = useClient(); // useClient() rebuilds its verbs every render, so it must not sit in a dependency list. @@ -37,14 +62,16 @@ export const HostsPanel = () => { const [busy, setBusy] = useState(null); const [params, setParams] = useSearchParams(); const selected = params.get("host"); + // `?config=1` is the settings page; an unconfigured plugin shows it whether or not it was asked for. + // In the URL rather than in state, so it survives a refresh and can be linked to — `docs/navigation-audit.md`. + const configuring = params.get("config") === "1"; const load = useCallback(async () => { try { setHosts(await clientRef.current.get("/proxy-manager/hosts")); setFailure(null); } catch (error) { - const body = (error as { body?: Failure })?.body; - setFailure(body ?? { error: error instanceof Error ? error.message : "Could not load hosts." }); + setFailure(asFailure(error)); setHosts(null); } }, []); @@ -65,9 +92,12 @@ export const HostsPanel = () => { } }; + // Order matters: an explicit `?config=1` wins over everything, then "there is nothing to connect to" + // wins over the generic failure screen. Otherwise the first thing a new install shows is an error. + if (configuring || failure?.unconfigured) return ; if (failure) return void load()} />; if (!hosts) return Loading hosts…; - if (hosts.length === 0) return Nginx Proxy Manager has no proxy hosts.; + return (
@@ -75,11 +105,27 @@ export const HostsPanel = () => { {hosts.length} host{hosts.length === 1 ? "" : "s"} - + + + {/* Not a Link: this panel lives inside a workspace and the target is a parameter on the page it + is already on. A button that sets the parameter is the same navigation without a full route. */} + + + {hosts.length === 0 ? ( + Nginx Proxy Manager has no proxy hosts. + ) : null} +
    {hosts.map((host) => { const offline = host.meta?.nginx_online === false;