fix(ssh): state what installing a key actually costs, and ask before doing it (v0.23.25-beta)
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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
1ab2e6547e
commit
864ca40427
@@ -6,7 +6,7 @@
|
||||
</p>
|
||||
|
||||
<p align="center">
|
||||
<img src="https://img.shields.io/badge/version-0.23.24_Beta-blue" alt="Version" />
|
||||
<img src="https://img.shields.io/badge/version-0.23.25_Beta-blue" alt="Version" />
|
||||
<img src="https://img.shields.io/badge/license-AGPLv3-blue" alt="License" />
|
||||
<img src="https://img.shields.io/badge/docker-compose-2496ED?logo=docker&logoColor=white" alt="Docker" />
|
||||
<img src="https://img.shields.io/badge/TypeScript-5.3-3178C6?logo=typescript&logoColor=white" alt="TypeScript" />
|
||||
|
||||
@@ -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": {
|
||||
|
||||
@@ -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<any>(
|
||||
`SELECT ${SSH_TARGET_COLS} FROM devices d
|
||||
WHERE d.status != 'disabled' AND d.ssh_username IS NOT NULL
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "mikrotik-manager-frontend",
|
||||
"version": "0.23.24-beta",
|
||||
"version": "0.23.25-beta",
|
||||
"private": true,
|
||||
"type": "module",
|
||||
"scripts": {
|
||||
|
||||
@@ -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 <div className="text-sm text-gray-500 dark:text-slate-400">Loading key status…</div>;
|
||||
|
||||
const verified = data?.status === 'verified';
|
||||
const user = data?.ssh_username || sshUsername || 'this user';
|
||||
|
||||
return (
|
||||
<div className="space-y-3">
|
||||
@@ -90,24 +97,53 @@ export default function SshKeyCard({ deviceId }: { deviceId: number }) {
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
) : (
|
||||
<div className="flex gap-2 rounded-lg border border-amber-300 bg-amber-50 p-3 text-xs dark:border-amber-700/60 dark:bg-amber-900/20">
|
||||
<AlertTriangle className="w-4 h-4 shrink-0 mt-0.5 text-amber-600 dark:text-amber-400" />
|
||||
<div className="text-amber-800 dark:text-amber-300">
|
||||
<span className="font-medium">Installing a key disables password SSH for this user.</span>{' '}
|
||||
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. */}
|
||||
<div className={`flex gap-2 rounded-lg border p-3 text-xs ${
|
||||
data
|
||||
? 'border-gray-200 bg-gray-50 dark:border-slate-700 dark:bg-slate-800/60'
|
||||
: 'border-amber-300 bg-amber-50 dark:border-amber-700/60 dark:bg-amber-900/20'}`}>
|
||||
<AlertTriangle className={`w-4 h-4 shrink-0 mt-0.5 ${
|
||||
data ? 'text-gray-400 dark:text-slate-500' : 'text-amber-600 dark:text-amber-400'}`} />
|
||||
<div className={data ? 'text-gray-600 dark:text-slate-300' : 'text-amber-800 dark:text-amber-300'}>
|
||||
<div className="font-medium">
|
||||
{data
|
||||
? `Password SSH is disabled for "${user}" on this device.`
|
||||
: `Installing a key disables password SSH for "${user}".`}
|
||||
</div>
|
||||
<ul className="mt-1 space-y-0.5 list-disc list-inside">
|
||||
<li>RouterOS requires key authentication once a key is bound to an account.</li>
|
||||
<li>The binary API and WinBox are unaffected, and remain a way in.</li>
|
||||
<li>Anyone who signs in to this device over SSH with a password — a colleague,
|
||||
a script, another tool — will stop being able to.</li>
|
||||
<li>
|
||||
<span className="font-medium">If this server loses its database or encryption key,
|
||||
the private key goes with it.</span>{' '}
|
||||
SSH would then need recovering through the API or WinBox, because the password
|
||||
will already have been refused.
|
||||
</li>
|
||||
<li>Revoking the key restores password SSH.</li>
|
||||
</ul>
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
|
||||
<div className="flex flex-wrap items-center gap-2">
|
||||
{!data && (
|
||||
<button className="btn-primary text-xs flex items-center gap-1.5"
|
||||
onClick={() => deploy.mutate()} disabled={busy}>
|
||||
{deploy.isPending ? <RefreshCw className="w-3.5 h-3.5 animate-spin" /> : <KeyRound className="w-3.5 h-3.5" />}
|
||||
Install key
|
||||
</button>
|
||||
<>
|
||||
<label className="flex items-start gap-2 text-xs text-gray-700 dark:text-slate-300 w-full">
|
||||
<input type="checkbox" className="mt-0.5" checked={acknowledged}
|
||||
onChange={(e) => setAcknowledged(e.target.checked)} />
|
||||
<span>I understand that password SSH will stop working for “{user}” on this device.</span>
|
||||
</label>
|
||||
<button className="btn-primary text-xs flex items-center gap-1.5"
|
||||
onClick={() => deploy.mutate()} disabled={busy || !acknowledged}>
|
||||
{deploy.isPending ? <RefreshCw className="w-3.5 h-3.5 animate-spin" /> : <KeyRound className="w-3.5 h-3.5" />}
|
||||
Install key
|
||||
</button>
|
||||
</>
|
||||
)}
|
||||
{data && (
|
||||
<>
|
||||
|
||||
@@ -579,7 +579,7 @@ export default function SystemConfigTab({ deviceId, device }: Props) {
|
||||
|
||||
<div className="card p-5">
|
||||
|
||||
<SshKeyCard deviceId={deviceId} />
|
||||
<SshKeyCard deviceId={deviceId} sshUsername={device.ssh_username} />
|
||||
|
||||
</div>
|
||||
|
||||
|
||||
@@ -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 = {
|
||||
|
||||
@@ -1 +1 @@
|
||||
export const APP_VERSION = 'v0.23.24 Beta';
|
||||
export const APP_VERSION = 'v0.23.25 Beta';
|
||||
|
||||
Reference in New Issue
Block a user