From 99e420b1b9783a1d6b4eb892c09d0af3340bf314 Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Fri, 8 May 2026 09:46:24 +0200 Subject: [PATCH] fix(events): typed-DELETE confirmation for bulk delete (#417) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bulk-delete modal previously used a password input as a confirmation gate, with an Enter-to-submit handler. Windows Hello / passkey flows that target password fields were able to autofill and synthesise an Enter keystroke, which submitted the form and triggered the destructive delete without an explicit click on the red Delete button (Rekoo's report in #417). Replace the password gate with a GitHub-style typed-literal pattern: the user types the literal "DELETE" (English, case-sensitive) into a plain text input. The Delete button stays disabled until the input matches, and there is no Enter-to-submit handler — only an explicit click on the red button proceeds. Plain text inputs aren't subject to password autofill or passkey ceremony so the auto-submit class of bug is gone. Server side, drop the bcrypt password verify on /admin/events/bulk-delete and the related INVALID_PASSWORD response. The server's auth boundary remains adminAuth + requirePermission('events.delete'); this matches DELETE /admin/events/:id which has never required a re-entered password. The client-side typed gate is the safeguard against accidental clicks. i18n: drop password-related keys, add confirmLabel + confirmHelp across en, de, nl, pt, ru. The literal "DELETE" stays English in all locales to keep the gesture immune to translation drift and unambiguous. Verified locally: typed-DELETE sanity spec covers the gate (wrong case disabled, correct enables, Enter-on-input no-ops, click submits, events deleted). Existing 03-bulk-archive smoke remains green. --- backend/src/routes/adminEvents.js | 37 ++++------ .../src/components/admin/BulkDeleteModal.tsx | 68 ++++++++----------- frontend/src/i18n/locales/de.json | 6 +- frontend/src/i18n/locales/en.json | 6 +- frontend/src/i18n/locales/nl.json | 8 +-- frontend/src/i18n/locales/pt.json | 6 +- frontend/src/i18n/locales/ru.json | 6 +- frontend/src/pages/admin/EventsListPage.tsx | 37 +++------- frontend/src/services/events.service.ts | 11 +-- 9 files changed, 70 insertions(+), 115 deletions(-) diff --git a/backend/src/routes/adminEvents.js b/backend/src/routes/adminEvents.js index d36db7fc..556630d0 100644 --- a/backend/src/routes/adminEvents.js +++ b/backend/src/routes/adminEvents.js @@ -1652,18 +1652,23 @@ router.post('/bulk-archive', adminAuth, requirePermission('events.archive'), [ } }); -// Bulk delete — destructive, irreversible. Requires the calling admin to -// re-enter their password as a confirmation gate (verified against the -// stored bcrypt hash, same pattern as /auth/admin/change-password). Caps at -// 100 events per request to keep request time bounded; the per-event -// cascade touches 5 DB tables + 3 filesystem paths so 1000 events would -// risk timing out the request. Loops via deleteEventCascade so the per- -// event delete behaviour stays in lock-step with DELETE /:id. +// Bulk delete — destructive, irreversible. Caps at 100 events per request +// to keep request time bounded; the per-event cascade touches 5 DB tables +// + 3 filesystem paths so 1000 events would risk timing out the request. +// Loops via deleteEventCascade so the per-event delete behaviour stays in +// lock-step with DELETE /:id. +// +// Confirmation is enforced client-side via the typed-DELETE pattern in +// BulkDeleteModal (#417). The previous server-side bcrypt-password gate +// was dropped because the destructive single-event DELETE /:id has never +// required a password either — events.delete permission + admin session +// is the auth boundary for both. The typed-literal client gate is the +// "accidental click" safeguard, and unlike a password input it isn't +// affected by passkey/Windows Hello autofill that auto-submits the form. const BULK_DELETE_MAX = 100; router.post('/bulk-delete', adminAuth, requirePermission('events.delete'), [ body('eventIds').isArray({ min: 1, max: BULK_DELETE_MAX }).withMessage(`eventIds must be an array of 1-${BULK_DELETE_MAX} ids`), - body('eventIds.*').isInt().withMessage('Each eventId must be an integer'), - body('password').isString().notEmpty().withMessage('Password is required for confirmation') + body('eventIds.*').isInt().withMessage('Each eventId must be an integer') ], async (req, res) => { try { const errors = validationResult(req); @@ -1671,19 +1676,7 @@ router.post('/bulk-delete', adminAuth, requirePermission('events.delete'), [ return res.status(400).json({ errors: errors.array() }); } - const { eventIds, password } = req.body; - - // Verify the admin's password before doing anything destructive. - // Same pattern as /auth/admin/change-password (auth.js). - const admin = await db('admin_users').where({ id: req.admin.id }).first(); - if (!admin) { - return res.status(401).json({ error: 'Authentication required' }); - } - const validPassword = await bcrypt.compare(password, admin.password_hash); - if (!validPassword) { - logger.warn('Incorrect password on bulk-delete attempt', { adminId: req.admin.id, eventCount: eventIds.length }); - return res.status(401).json({ error: 'Incorrect password', code: 'INVALID_PASSWORD' }); - } + const { eventIds } = req.body; // Editor-role events.delete permission is already gated by the route // middleware. We do NOT additionally filter to created_by here because diff --git a/frontend/src/components/admin/BulkDeleteModal.tsx b/frontend/src/components/admin/BulkDeleteModal.tsx index 1cff874a..832a4fca 100644 --- a/frontend/src/components/admin/BulkDeleteModal.tsx +++ b/frontend/src/components/admin/BulkDeleteModal.tsx @@ -1,19 +1,21 @@ import React, { useState } from 'react'; -import { Trash2, AlertTriangle, X, Lock, Eye, EyeOff, Loader2 } from 'lucide-react'; +import { Trash2, AlertTriangle, X, Loader2 } from 'lucide-react'; import { useTranslation } from 'react-i18next'; import { Button, Card, Input } from '../common'; import type { Event } from '../../types'; +// The exact literal a user must type to confirm bulk deletion. Kept English +// across locales (matching GitHub's repo-deletion pattern) so it can never +// be interpreted as autofillable text or be triggered by passkey/Windows +// Hello flows on a password field — see issue #417. +const CONFIRM_LITERAL = 'DELETE'; + interface BulkDeleteModalProps { isOpen: boolean; onClose: () => void; - onConfirm: (password: string) => Promise; + onConfirm: () => Promise; selectedEvents: Event[]; isLoading?: boolean; - /** Set when the server responded 401 INVALID_PASSWORD; surfaces inline. */ - passwordError?: string | null; - /** Clear the inline password error when the user starts typing again. */ - onPasswordErrorClear?: () => void; } export const BulkDeleteModal: React.FC = ({ @@ -22,25 +24,18 @@ export const BulkDeleteModal: React.FC = ({ onConfirm, selectedEvents, isLoading = false, - passwordError = null, - onPasswordErrorClear, }) => { const { t } = useTranslation(); - const [password, setPassword] = useState(''); - const [showPassword, setShowPassword] = useState(false); + const [confirmText, setConfirmText] = useState(''); if (!isOpen) return null; const count = selectedEvents.length; + const confirmed = confirmText === CONFIRM_LITERAL; const handleSubmit = async () => { - if (!password || isLoading) return; - await onConfirm(password); - }; - - const handlePasswordChange = (val: string) => { - setPassword(val); - if (passwordError && onPasswordErrorClear) onPasswordErrorClear(); + if (!confirmed || isLoading) return; + await onConfirm(); }; return ( @@ -61,7 +56,6 @@ export const BulkDeleteModal: React.FC = ({ - {/* Processing-state banner replaces the warning + form when in flight. */} {isLoading ? (
@@ -90,28 +84,22 @@ export const BulkDeleteModal: React.FC = ({
handlePasswordChange(e.target.value)} - placeholder={t('events.bulkDelete.passwordPlaceholder', 'Your admin password')} - helperText={t('events.bulkDelete.passwordHelp', 'We require your password as a safeguard against accidental bulk deletions.')} - error={passwordError || undefined} - leftIcon={} - rightIcon={ - - } + type="text" + label={t( + 'events.bulkDelete.confirmLabel', + 'Type {{literal}} to confirm', + { literal: CONFIRM_LITERAL } + )} + value={confirmText} + onChange={(e) => setConfirmText(e.target.value)} + placeholder={CONFIRM_LITERAL} + helperText={t( + 'events.bulkDelete.confirmHelp', + 'A typed confirmation prevents accidental deletions and isn\'t affected by browser autofill or passkey shortcuts.' + )} autoFocus - onKeyDown={(e) => { - if (e.key === 'Enter' && password) handleSubmit(); - }} + autoComplete="off" + spellCheck={false} />
@@ -126,7 +114,7 @@ export const BulkDeleteModal: React.FC = ({
diff --git a/frontend/src/services/events.service.ts b/frontend/src/services/events.service.ts index 3e452c48..26079194 100644 --- a/frontend/src/services/events.service.ts +++ b/frontend/src/services/events.service.ts @@ -148,10 +148,12 @@ export const eventsService = { return response.data; }, - // Bulk delete events (admin) — destructive. Requires the calling admin's - // password as a server-side confirmation gate. On 401 the server returns - // { error, code: 'INVALID_PASSWORD' } and no events are touched. - async bulkDeleteEvents(eventIds: number[], password: string): Promise<{ + // Bulk delete events (admin) — destructive. The client-side confirmation + // gate is a typed-literal pattern in the modal (issue #417); no password + // is sent because passkey/autofill flows on a password input could + // auto-submit the form. The admin session JWT remains the auth boundary, + // matching DELETE /admin/events/:id which has never required a password. + async bulkDeleteEvents(eventIds: number[]): Promise<{ message: string; results: { successful: Array<{ id: number; name: string }>; @@ -160,7 +162,6 @@ export const eventsService = { }> { const response = await api.post('/admin/events/bulk-delete', { eventIds, - password, }); return response.data; },