From 864ca4042754f7dc5a5daf828b5e001f3bbba85b Mon Sep 17 00:00:00 2001 From: 2GT-Rich <107219997+2GT-Rich@users.noreply.github.com> Date: Wed, 2 Sep 2026 12:01:20 -0700 Subject: [PATCH] fix(ssh): state what installing a key actually costs, and ask before doing it (v0.23.25-beta) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit v0.23.24 shipped the SSH key feature with a warning that undersold it and a confirmation flow that was the wrong way round. Five gaps, all in how the change was presented rather than in what it does. **Installing was one click; revoking asked for confirmation.** Backwards. Revoking restores password SSH, while installing takes it away — the consequential action was the unguarded one. Installation now requires an explicit acknowledgement naming the account that will lose password access. **The warning disappeared once a key existed.** Whoever arrives at a device later and finds their password refused over SSH is exactly the person who needs the explanation, and they were the one person not shown it. The notice is now permanent, worded for the current state rather than only the impending one. **It said "this user" rather than naming the account**, which is the difference between a caution and a fact someone can act on. **It never mentioned losing the database.** That is the serious one. Password SSH is already refused once a key is bound, so a manager that loses its database or encryption key leaves SSH recoverable only through the API or WinBox. That consequence outlives the session, the operator, and quite possibly the person who clicked the button, and it went unstated. **Fleet-wide deployment had no guard whatsoever.** One request would disable password SSH on every device it touched, while the single-device path asked for an acknowledgement — an endpoint that can do a thing sixty times should not be easier to trigger than the one that does it once. It now refuses without an explicit confirm and explains why in the refusal. The notice also spells out what was previously left implicit: colleagues, scripts and other tools that SSH in with a password will stop working. On a shared fleet that is a change to announce, not to discover. Co-Authored-By: Claude Opus 5 --- README.md | 2 +- backend/package.json | 2 +- backend/src/routes/devices.ts | 14 +++- frontend/package.json | 2 +- .../components/device-detail/SshKeyCard.tsx | 64 +++++++++++++++---- .../device-detail/SystemConfigTab.tsx | 2 +- frontend/src/services/api.ts | 3 +- frontend/src/version.ts | 2 +- 8 files changed, 70 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index 248d633..932eed8 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@

- Version + Version License Docker TypeScript diff --git a/backend/package.json b/backend/package.json index 15efc2c..be0c8f4 100644 --- a/backend/package.json +++ b/backend/package.json @@ -1,6 +1,6 @@ { "name": "mikrotik-manager-backend", - "version": "0.23.24-beta", + "version": "0.23.25-beta", "description": "Mikrotik Network Management Platform - Backend API", "main": "dist/index.js", "scripts": { diff --git a/backend/src/routes/devices.ts b/backend/src/routes/devices.ts index 1f00f40..d8ca450 100644 --- a/backend/src/routes/devices.ts +++ b/backend/src/routes/devices.ts @@ -1663,7 +1663,19 @@ router.delete('/:id/ssh-key', requireWrite, async (req: Request, res: Response) * ends by proving the key, and running sixty of those at once would be a * thundering herd against the fleet for no wall-clock gain worth having. */ -router.post('/ssh-keys/deploy-all', requireWrite, async (_req: Request, res: Response) => { +router.post('/ssh-keys/deploy-all', requireWrite, async (req: Request, res: Response) => { + // This is the most consequential call in the feature: it disables password SSH + // across an entire fleet in one request. The per-device path asks for an + // acknowledgement in the UI, and an endpoint that can do it sixty times over + // should not be easier to trigger than the one that does it once. + if ((req.body as { confirm?: boolean })?.confirm !== true) { + return res.status(400).json({ + error: 'Refusing to deploy fleet-wide without confirmation. This disables password ' + + 'SSH for the configured user on every device it touches; the API and WinBox ' + + 'are unaffected. Repeat with {"confirm": true} once that is understood.', + }); + } + const targets = await query( `SELECT ${SSH_TARGET_COLS} FROM devices d WHERE d.status != 'disabled' AND d.ssh_username IS NOT NULL diff --git a/frontend/package.json b/frontend/package.json index 0754e77..387bb92 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,6 +1,6 @@ { "name": "mikrotik-manager-frontend", - "version": "0.23.24-beta", + "version": "0.23.25-beta", "private": true, "type": "module", "scripts": { diff --git a/frontend/src/components/device-detail/SshKeyCard.tsx b/frontend/src/components/device-detail/SshKeyCard.tsx index 7b0b253..a7ba98f 100644 --- a/frontend/src/components/device-detail/SshKeyCard.tsx +++ b/frontend/src/components/device-detail/SshKeyCard.tsx @@ -11,10 +11,16 @@ import { sshKeyApi } from '../../services/api'; * so installing a key is a real trade, not a pure addition, and the operator has * to be told before they make it rather than after (#110). */ -export default function SshKeyCard({ deviceId }: { deviceId: number }) { +export default function SshKeyCard( + { deviceId, sshUsername }: { deviceId: number; sshUsername?: string }, +) { const queryClient = useQueryClient(); const [msg, setMsg] = useState<{ ok: boolean; text: string } | null>(null); const [confirmRevoke, setConfirmRevoke] = useState(false); + // Installing a key changes how this device can be reached, and cannot be + // undone by retrying — it is undone by revoking. A single click was the wrong + // weight for that, especially when revoking already asked for confirmation. + const [acknowledged, setAcknowledged] = useState(false); const { data, isLoading } = useQuery({ queryKey: ['ssh-key', deviceId], @@ -43,6 +49,7 @@ export default function SshKeyCard({ deviceId }: { deviceId: number }) { if (isLoading) return

Loading key status…
; const verified = data?.status === 'verified'; + const user = data?.ssh_username || sshUsername || 'this user'; return (
@@ -90,24 +97,53 @@ export default function SshKeyCard({ deviceId }: { deviceId: number }) {
)} - ) : ( -
- -
- Installing a key disables password SSH for this user.{' '} - RouterOS requires key authentication once a key is bound to an account. The binary API and - WinBox are unaffected, and revoking the key restores password SSH. + ) : null} + + {/* Shown before installing *and* after. Someone arriving at a keyed device + needs to know why their password no longer works over SSH just as much + as the person about to cause it. */} +
+ +
+
+ {data + ? `Password SSH is disabled for "${user}" on this device.` + : `Installing a key disables password SSH for "${user}".`}
+
    +
  • RouterOS requires key authentication once a key is bound to an account.
    • +
  • The binary API and WinBox are unaffected, and remain a way in.
    • +
  • Anyone who signs in to this device over SSH with a password — a colleague, + a script, another tool — will stop being able to.
    • +
  • + If this server loses its database or encryption key, + the private key goes with it.{' '} + SSH would then need recovering through the API or WinBox, because the password + will already have been refused. +
    • +
  • Revoking the key restores password SSH.
    • +
- )} +
{!data && ( - + <> + + + )} {data && ( <> diff --git a/frontend/src/components/device-detail/SystemConfigTab.tsx b/frontend/src/components/device-detail/SystemConfigTab.tsx index 282b46d..f13371d 100644 --- a/frontend/src/components/device-detail/SystemConfigTab.tsx +++ b/frontend/src/components/device-detail/SystemConfigTab.tsx @@ -579,7 +579,7 @@ export default function SystemConfigTab({ deviceId, device }: Props) {
- +
diff --git a/frontend/src/services/api.ts b/frontend/src/services/api.ts index 5e485a5..2ac773c 100644 --- a/frontend/src/services/api.ts +++ b/frontend/src/services/api.ts @@ -381,9 +381,10 @@ export const sshKeyApi = { api.post<{ message: string }>(`/devices/${deviceId}/ssh-key`, { rotate }), verify: (deviceId: number) => api.post<{ message: string }>(`/devices/${deviceId}/ssh-key/verify`), revoke: (deviceId: number) => api.delete<{ message: string }>(`/devices/${deviceId}/ssh-key`), + /** Requires explicit confirmation: this disables password SSH fleet-wide. */ deployAll: () => api.post<{ considered: number; succeeded: number; results: { device_id: number; name: string; ok: boolean; error?: string }[] }>( - '/devices/ssh-keys/deploy-all', undefined, { timeout: 600_000 }), + '/devices/ssh-keys/deploy-all', { confirm: true }, { timeout: 600_000 }), }; export const devicesApi = { diff --git a/frontend/src/version.ts b/frontend/src/version.ts index 7937c07..28ccace 100644 --- a/frontend/src/version.ts +++ b/frontend/src/version.ts @@ -1 +1 @@ -export const APP_VERSION = 'v0.23.24 Beta'; +export const APP_VERSION = 'v0.23.25 Beta';