Merge branch 'main' into embedded-vnc

client/ui/frontend/src/contexts/SettingsContext.tsx

@@ -22,12 +22,23 @@ const logSaveError = (err: unknown) => console.error("[SettingsContext] save fai
 
 export type AutostartState = { supported: boolean; enabled: boolean };
 
+// GuardedField is a setting the daemon only accepts from root/administrator.
+// Turning one on goes through saveGuardedField, which asks the operating system
+// for the privileges rather than sending a request that would be refused.
+export type GuardedField =
+    | "serverSshAllowed"
+    | "enableSshRoot"
+    | "disableSshAuth"
+    | "serverVncAllowed"
+    | "disableVncApproval";
+
 type SettingsContextValue = {
     config: Config;
     guiVersion: string;
     setField: <K extends keyof Config>(k: K, v: Config[K]) => void;
     saveField: <K extends keyof Config>(k: K, v: Config[K]) => Promise<void>;
     saveFields: (partial: Partial<Config>, opts?: { preSharedKey?: string }) => Promise<void>;
+    saveGuardedField: (k: GuardedField, v: boolean) => Promise<void>;
     saveNow: () => Promise<void>;
 };
 
@@ -63,6 +74,12 @@ const useSettingsState = () => {
     const [guiVersion, setGuiVersion] = useState<string>("—");
     const saveTimer = useRef<ReturnType<typeof setTimeout> | null>(null);
     const loadedRef = useRef<LoadedConfig | null>(null);
+    // Set when the daemon's config changed while a save was pending, so the read
+    // that was skipped to protect the pending edit happens once it is through.
+    // Without it the form keeps values the daemon no longer has and the next save
+    // submits them, which for a guarded setting means asking the user to authorize
+    // a change they never made.
+    const reloadOwed = useRef(false);
 
     useEffect(() => {
         loadedRef.current = loaded;
@@ -73,6 +90,7 @@ const useSettingsState = () => {
     // update the daemon then rejected.
     const reload = useCallback(
         async (profileName: string) => {
+            reloadOwed.current = false;
             try {
                 const data = await SettingsSvc.GetConfig({ profileName, username });
                 setLoaded({ profileName, data });
@@ -94,7 +112,12 @@ const useSettingsState = () => {
                     username,
                 });
                 if (cancelled) return;
-                if (saveTimer.current) return;
+                // A pending edit outranks the daemon's copy until it is saved, so
+                // the read is owed rather than dropped: see reloadOwed.
+                if (saveTimer.current) {
+                    reloadOwed.current = true;
+                    return;
+                }
                 setLoaded({ profileName: activeProfileId, data });
             } catch (e) {
                 if (cancelled || !showError) return;
@@ -141,12 +164,17 @@ const useSettingsState = () => {
         async (profileName: string, next: Config, preSharedKey?: string) => {
             const preSharedKeyWrite = preSharedKey === undefined ? {} : { preSharedKey };
             try {
-                await SettingsSvc.SetConfig({
+                const { declined } = await SettingsSvc.SetConfig({
                     ...next,
                     ...preSharedKeyWrite,
                     profileName,
                     username,
                 });
+                // The change needed authorization and the user said no, so the
+                // optimistic update is wrong. Nothing to report: they know.
+                if (declined || reloadOwed.current) {
+                    await reload(profileName);
+                }
             } catch (e) {
                 // The optimistic update is wrong now: the daemon refused it
                 // (a change that needs elevated privileges, an MDM-managed
@@ -206,6 +234,59 @@ const useSettingsState = () => {
         [loaded, save],
     );
 
+    // saveGuardedField applies a setting the daemon restricts to
+    // root/administrator by having the Go side run the app again under the
+    // platform's elevation prompt (UAC, the macOS authentication dialog, polkit).
+    // The prompt is the user's, so the call is made straight from their gesture
+    // and never from the debounce.
+    const saveGuardedField = useCallback(
+        async (k: GuardedField, v: boolean) => {
+            const cur = loadedRef.current;
+            if (!cur) return;
+
+            // Flush what the debounce still owes, before the optimistic update
+            // below joins it: a later save carrying the guarded value would be
+            // refused, and its error dialog would be the second one for a change
+            // the user already authorized.
+            if (saveTimer.current) {
+                clearTimeout(saveTimer.current);
+                saveTimer.current = null;
+                await save(cur.profileName, cur.data);
+            }
+
+            const next: LoadedConfig = {
+                profileName: cur.profileName,
+                data: { ...cur.data, [k]: v },
+            };
+            loadedRef.current = next;
+            setLoaded(next);
+
+            try {
+                await SettingsSvc.SetGuardedSettings({
+                    profileName: cur.profileName,
+                    username,
+                    [k]: v,
+                });
+            } catch (e) {
+                // The daemon is authoritative either way, so re-read before
+                // reporting. A declined prompt is not an error and does not come
+                // through here at all; this is a prompt that could not be raised,
+                // which carries the command that would have done it.
+                await reload(cur.profileName);
+                await errorDialog({
+                    Title: i18next.t("settings.error.saveTitle"),
+                    Message: errorMessage(e),
+                    Command: errorCommand(e),
+                });
+                return;
+            }
+            // Either the change went through or the user declined it. The daemon
+            // says which.
+            await reload(cur.profileName);
+        },
+        [username, save, reload],
+    );
+
     const saveFields = useCallback(
         async (partial: Partial<Config>, opts?: { preSharedKey?: string }) => {
             if (!loaded) return;
@@ -225,15 +306,27 @@ const useSettingsState = () => {
         [loaded, save],
     );
 
-    return { config: loaded?.data ?? null, guiVersion, setField, saveField, saveFields, saveNow };
+    return {
+        config: loaded?.data ?? null,
+        guiVersion,
+        setField,
+        saveField,
+        saveFields,
+        saveGuardedField,
+        saveNow,
+    };
 };
 
 export const SettingsProvider = ({ children }: { children: ReactNode }) => {
-    const { config, guiVersion, setField, saveField, saveFields, saveNow } = useSettingsState();
+    const { config, guiVersion, setField, saveField, saveFields, saveGuardedField, saveNow } =
+        useSettingsState();
 
     const value = useMemo<SettingsContextValue | null>(
-        () => (config ? { config, guiVersion, setField, saveField, saveFields, saveNow } : null),
-        [config, guiVersion, setField, saveField, saveFields, saveNow],
+        () =>
+            config
+                ? { config, guiVersion, setField, saveField, saveFields, saveGuardedField, saveNow }
+                : null,
+        [config, guiVersion, setField, saveField, saveFields, saveGuardedField, saveNow],
     );
 
     if (!value) {

client/ui/frontend/src/hooks/usePrivilege.ts

@@ -1,6 +1,6 @@
 import { useEffect, useState } from "react";
 import { Settings as SettingsSvc } from "@bindings/services";
-import { Privilege } from "@bindings/services/models.js";
+import { type Privilege } from "@bindings/services/models.js";
 
 // usePrivilege reports whether this UI process may perform the changes the daemon
 // restricts to root/administrator. It is answered in-process from our own token

client/ui/frontend/src/modules/settings/PrivilegeGuard.tsx

@@ -1,86 +1,174 @@
+import { type TFunction } from "i18next";
 import { useTranslation } from "react-i18next";
 import { CopyToClipboard } from "@/components/CopyToClipboard";
+import { type GuardedField, useSettings } from "@/contexts/SettingsContext.tsx";
 import { usePrivilege } from "@/hooks/usePrivilege.ts";
-import { Privilege } from "@bindings/services/models.js";
-import { type ReactNode } from "react";
+import type { Privilege } from "@bindings/services/models.js";
+import { type ReactNode, useState } from "react";
 
+// GuardedControl is what a settings page needs to render one control the daemon
+// restricts to root/administrator: how to apply a change to it, whether it can be
+// touched at all, and the explanation that belongs under it.
 export type GuardedControl = {
+    apply: (value: boolean) => void;
     disabled: boolean;
     hint: ReactNode;
 };
 
-// useGuardedControl returns a guard for the settings controls the daemon
-// restricts to root/administrator: enabling a remote-access server, or removing
-// one of its safeguards.
+// useGuardedControl returns a guard for the settings the daemon restricts to
+// root/administrator: enabling a remote-access server, or removing one of its
+// safeguards.
 //
 // The daemon restricts only the direction that hands out access from a process
-// running as root. So for an unprivileged user a guarded control is either
-// unavailable (it is off and only they could turn it on) or a one-way switch (it
-// is on, they may turn it off, but not back on) — say which, either way.
+// running as root: for every one of these settings that is switching the field on.
 //
-// A null privilege means we could not determine it: leave the control alone
-// rather than greying it out with nothing to explain why. The daemon enforces
-// this regardless, and a rejected save reports its own guidance.
+// An unprivileged user gets that direction routed through the platform's elevation
+// prompt where there is one to raise, and otherwise the old arrangement, where the
+// control is either unavailable (it is off and only a privileged caller could turn
+// it on) or a one-way switch (it is on, they may turn it off but not back on) with
+// the command that does it.
+//
+// A null privilege means we could not determine it: leave the control alone rather
+// than greying it out with nothing to explain why. The daemon enforces this
+// regardless, and a rejected save reports its own guidance.
 export const useGuardedControl = () => {
+    const { t } = useTranslation();
+    const { config, setField, saveGuardedField } = useSettings();
     const privilege = usePrivilege();
+    // The field whose elevation prompt is currently up, if any. The prompt is
+    // modal to the operating system, not to us, so the guarded controls are held
+    // still meanwhile rather than allowed to stack a second one behind it.
+    const [authorizing, setAuthorizing] = useState<GuardedField | null>(null);
+
+    const authorize = async (field: GuardedField, value: boolean) => {
+        setAuthorizing(field);
+        try {
+            await saveGuardedField(field, value);
+        } finally {
+            setAuthorizing(null);
+        }
+    };
 
     return (
-        guardedDirectionActive: boolean,
+        field: GuardedField,
         command: (p: Privilege) => string,
-        // inverted marks a control whose guarded direction is switching it off,
-        // so the one-way warning has to read the other way round.
+        // inverted marks a control whose guarded direction is switching it off, so
+        // the one-way warning has to read the other way round.
         inverted = false,
     ): GuardedControl => {
+        const plain = (value: boolean) => setField(field, value);
         if (!privilege || privilege.privileged) {
-            return { disabled: false, hint: undefined };
+            return { apply: plain, disabled: false, hint: undefined };
         }
-        const hint = (
-            <PrivilegeHint
-                actor={privilege.actor}
-                command={command(privilege)}
+
+        const guardedDirectionActive = config[field];
+        const hint = (pending: boolean, command?: string) => (
+            <GuardedHint
+                actor={actorLabel(privilege, t)}
                 oneWay={guardedDirectionActive}
                 inverted={inverted}
+                pending={pending}
+                command={command}
             />
         );
-        return { disabled: !guardedDirectionActive, hint };
+
+        if (privilege.canElevate) {
+            return {
+                // Switching off is ours to do; only switching on is authorized.
+                apply: (value: boolean) => {
+                    if (!value) {
+                        plain(value);
+                        return;
+                    }
+                    void authorize(field, value);
+                },
+                disabled: authorizing !== null,
+                hint: hint(authorizing === field),
+            };
+        }
+        return {
+            apply: plain,
+            disabled: !guardedDirectionActive,
+            hint: hint(false, command(privilege)),
+        };
     };
 };
 
-// PrivilegeHint explains what an unprivileged user can and cannot do with a
-// guarded control, and offers the command that does it with the privileges the
-// daemon requires. oneWay covers the control being in the guarded state already:
-// switching it back is the part that needs privileges.
-export function PrivilegeHint({
+// actorLabel names the principal the daemon requires, in the user's language. The
+// Go side reports which one it is rather than wording it, because "administrator
+// privileges" is English and a translated sentence cannot borrow it.
+function actorLabel(privilege: Privilege, t: TFunction): string {
+    return privilege.actorKey === "administrator"
+        ? t("settings.privilege.actorAdministrator")
+        : t("settings.privilege.actorRoot");
+}
+
+// GuardedHint is what a control the daemon guards says to an unprivileged user.
+// There are three things worth saying, and it says at most one:
+//
+//   - A prompt is open. Worth a line because it can take a few seconds to appear,
+//     long enough that a control which merely went inert would read as a hang.
+//   - The setting is in its guarded state already (oneWay), so the user may switch
+//     it back as they please and it is switching it away again that will ask. No
+//     command either way: the direction they can take is theirs to take.
+//   - Only a privileged caller can move it at all, and there is no prompt to
+//     raise: the command that does it belongs here, and nothing else will do.
+//
+// Which leaves the case of a control whose guarded direction is still ahead of the
+// user and a prompt that can be raised for it: nothing to say, because clicking it
+// raises the prompt and the prompt explains itself.
+function GuardedHint({
     actor,
-    command,
     oneWay,
     inverted,
+    pending,
+    command,
 }: {
     actor: string;
-    command: string;
     oneWay: boolean;
     inverted: boolean;
+    pending: boolean;
+    command?: string;
 }): ReactNode {
     const { t } = useTranslation();
+
+    if (pending) {
+        return <HintBox>{t("settings.privilege.authorizePending")}</HintBox>;
+    }
+    if (oneWay) {
+        return (
+            <HintBox>
+                <span>
+                    {inverted
+                        ? t("settings.privilege.oneWayInverted", { actor })
+                        : t("settings.privilege.oneWay", { actor })}
+                </span>
+            </HintBox>
+        );
+    }
     if (!command) return null;
+    return (
+        <HintBox>
+            <span>{t("settings.privilege.hint", { actor })}</span>
+            <CopyToClipboard message={command} alwaysShowIcon wrap variant={"bright"}>
+                <code className={"select-text break-all font-mono text-xs text-nb-gray-200"}>
+                    {command}
+                </code>
+            </CopyToClipboard>
+        </HintBox>
+    );
+}
+
+// HintBox is the box a guarded control puts its explanation in, directly under the
+// control it belongs to.
+function HintBox({ children }: { children: ReactNode }): ReactNode {
     return (
         <div
             className={
                 "-mt-2 flex flex-col gap-1 rounded-md bg-nb-gray-930 px-3 py-2 text-xs text-nb-gray-300"
             }
         >
-            <span>
-                {!oneWay
-                    ? t("settings.privilege.hint", { actor })
-                    : inverted
-                      ? t("settings.privilege.oneWayInverted", { actor })
-                      : t("settings.privilege.oneWay", { actor })}
-            </span>
-            <CopyToClipboard message={command} alwaysShowIcon wrap variant={"bright"}>
-                <code className={"select-text break-all font-mono text-xs text-nb-gray-200"}>
-                    {command}
-                </code>
-            </CopyToClipboard>
+            {children}
         </div>
     );
 }

client/ui/frontend/src/modules/settings/SettingsSSH.tsx

@@ -14,11 +14,12 @@ export function SettingsSSH() {
     const { config, setField } = useSettings();
     const guarded = useGuardedControl();
     const isSSHServerEnabled = config.serverSshAllowed;
-    const sshServer = guarded(config.serverSshAllowed, (p) => p.allowSshServer);
-    const sshRoot = guarded(config.enableSshRoot, (p) => p.enableSshRoot);
+
+    const sshServer = guarded("serverSshAllowed", (p) => p.allowSshServer);
+    const sshRoot = guarded("enableSshRoot", (p) => p.enableSshRoot);
     // Inverted control: the guarded direction is switching authentication off, so
     // it is the already-disabled state that is the one-way one.
-    const sshAuth = guarded(config.disableSshAuth, (p) => p.disableSshAuth, true);
+    const sshAuth = guarded("disableSshAuth", (p) => p.disableSshAuth, true);
     const jwtTtlId = useId();
     const [jwtTtlInput, setJwtTtlInput] = useState(String(config.sshJwtCacheTtl));
 
@@ -52,7 +53,7 @@ export function SettingsSSH() {
             <SectionGroup title={t("settings.ssh.section.server")}>
                 <FancyToggleSwitch
                     value={config.serverSshAllowed}
-                    onChange={(v) => setField("serverSshAllowed", v)}
+                    onChange={sshServer.apply}
                     disabled={sshServer.disabled}
                     label={t("settings.ssh.server.label")}
                     helpText={t("settings.ssh.server.help")}
@@ -66,7 +67,7 @@ export function SettingsSSH() {
             >
                 <FancyToggleSwitch
                     value={config.enableSshRoot}
-                    onChange={(v) => setField("enableSshRoot", v)}
+                    onChange={sshRoot.apply}
                     disabled={sshRoot.disabled}
                     label={t("settings.ssh.root.label")}
                     helpText={t("settings.ssh.root.help")}
@@ -98,7 +99,7 @@ export function SettingsSSH() {
             >
                 <FancyToggleSwitch
                     value={!config.disableSshAuth}
-                    onChange={(v) => setField("disableSshAuth", !v)}
+                    onChange={(v) => sshAuth.apply(!v)}
                     disabled={sshAuth.disabled}
                     label={t("settings.ssh.jwt.label")}
                     helpText={t("settings.ssh.jwt.help")}

client/ui/frontend/src/modules/settings/SettingsVNC.tsx

@@ -7,23 +7,23 @@ import { useRestrictions } from "@/contexts/RestrictionsContext.tsx";
 
 export function SettingsVNC() {
     const { t } = useTranslation();
-    const { config, setField } = useSettings();
+    const { config } = useSettings();
     const { mdm } = useRestrictions();
     const guarded = useGuardedControl();
     const isVNCServerEnabled = config.serverVncAllowed;
     const vncServerManaged = mdm.allowServerVNC != null;
 
-    const vncServer = guarded(config.serverVncAllowed, (p) => p.allowVncServer);
+    const vncServer = guarded("serverVncAllowed", (p) => p.allowVncServer);
     // Inverted control: the guarded direction is switching the approval prompt
     // off, so the already-disabled state is the one-way one.
-    const vncApproval = guarded(config.disableVncApproval, (p) => p.disableVncApproval, true);
+    const vncApproval = guarded("disableVncApproval", (p) => p.disableVncApproval, true);
 
     return (
         <>
             <SectionGroup title={t("settings.vnc.section.server")}>
                 <FancyToggleSwitch
                     value={config.serverVncAllowed}
-                    onChange={(v) => setField("serverVncAllowed", v)}
+                    onChange={vncServer.apply}
                     label={t("settings.vnc.server.label")}
                     helpText={t("settings.vnc.server.help")}
                     disabled={vncServerManaged || vncServer.disabled}
@@ -38,7 +38,7 @@ export function SettingsVNC() {
                 >
                     <FancyToggleSwitch
                         value={!config.disableVncApproval}
-                        onChange={(v) => setField("disableVncApproval", !v)}
+                        onChange={(v) => vncApproval.apply(!v)}
                         label={t("settings.vnc.approval.label")}
                         helpText={t("settings.vnc.approval.help")}
                         disabled={vncApproval.disabled}