From e26b7ea8edf780791540444ff22564c0496409e2 Mon Sep 17 00:00:00 2001 From: Administrator Date: Sun, 7 Jun 2026 03:11:13 +0200 Subject: [PATCH] fix(accounts): never persist unverified creds + dedupe-proof modal + label + perf MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit User reported three coupled bugs in account add/edit: (1) Invalid logins still create the account (2) Doodstream gets created multiple times when "Prüfen & Anlegen" is double-clicked or repeatedly OTP-retried (3) Add/Delete in the accounts panel feel laggy Plus a UX/feature request: account label + two-step "Prüfen → Anlegen" flow. Map (workflow wf44zpud4, 3 parallel subagents + adversarial verify) confirmed: - saveAccount() persisted to disk BEFORE the health check (lines 3407-3409) - saveBtn.disabled was set AFTER two awaited IPC roundtrips → 5-100ms race window - OTP-retry path generated a new accountId on every click (editingAccountId stayed null in ADD mode) → DETERMINISTIC duplication on every OTP attempt - runHealthCheck IPC required the account to be already persisted → that's why the old code wrote-first-check-second Fix architecture (advisor: Option A — make the invariant real, not cleanup-based): - main.js + preload.js: NEW `validate-credentials` IPC. Accepts ephemeral {hoster, authType, username, password, apiKey, otp} payload, builds an ephemeral hosterConfig, runs the same per-hoster checker via a shared _dispatchHealthCheck helper. Nothing touches config.hosters. - renderer: two-step modal state machine. - "Prüfen" click → validateCredentials (ephemeral) → green flips button to "Anlegen"/"Speichern" AND caches a snapshot of the validated creds. - "Anlegen"/"Speichern" click → only fires if cached snapshot matches the currently-typed credential-identity (username+password or apiKey; label and OTP are not part of the snapshot key). - Input listeners on the identity fields drop the snapshot the moment any cred is edited post-green → user can't sneak unverified creds through. - _accountModalBusy is set SYNCHRONOUSLY at the top of the click handler, before any await, so a double-click is a no-op. - _accountModalSession token bumps on every modal reset → a stale late response from a closed-and-reopened modal can't stomp the new session's busy flag or UI (lens-2 review fix). - Edit mode flows through the same path → bad edits never reach disk before being validated (fixes the silent good-creds clobber). - closeAccountModal cancels the auto-close timer + clears modal state so a stale 600 ms timer can't close a freshly-reopened modal. - Label field (new): persisted on the account, shown in the card subtitle as "Label: XYZ • API: ABC… — API Key gültig" so identical-looking API accounts are disambiguable. Excluded from snapshot key on purpose — label is metadata. - Perf: drop the redundant `await getConfig()` round-trip in commit+delete (in-memory state was already the source of truth and the old reload was the main lag source). deleteAccount fires-and-forgets the saveConfig and closes the modal synchronously. Commit path uses updateAccountCard for the single-card edit case instead of a 4-panel cascade. Multi-lens review (workflow wyoc3iq4k, 3 reviewers): OTP-correctness SHIP, race-guard SHIP-WITH-FIXES (session-id token + busy-inside-try applied), edit-mode+label SHIP. No blockers. Tests: 6 new regression tests (tests/validate-credentials.test.js) covering the three reported bugs as executable spec: (a) failed validation persists nothing to config.hosters (b) second click with guard set persists exactly one entry (c) OTP-required persists nothing; OTP retry re-validates ephemerally plus snapshot-key identity, post-validation edit invalidation, and the ephemeral hosterConfig shape contract. 210/210 green, lint clean. Co-Authored-By: Claude Opus 4.7 --- main.js | 46 ++++ preload.js | 1 + renderer/app.js | 333 +++++++++++++++++++++-------- renderer/index.html | 4 + tests/validate-credentials.test.js | 195 +++++++++++++++++ 5 files changed, 487 insertions(+), 92 deletions(-) create mode 100644 tests/validate-credentials.test.js diff --git a/main.js b/main.js index e28f245..943d14c 100644 --- a/main.js +++ b/main.js @@ -1115,6 +1115,52 @@ ipcMain.handle('run-health-check', async (_event, payload) => { return runHosterHealthCheck(config, hosters); }); +// Validate ephemeral credentials WITHOUT persisting them to config.hosters. +// This is the IPC that backs the two-step "Prüfen → Anlegen" modal flow: the +// new account is never on disk until the user confirms after a green check, so +// failed/OTP-pending creds can't leak into config (and a double-click on the +// Prüfen button cannot create duplicates because nothing is written until the +// second, distinct "Anlegen" click). NOTE: this payload carries plaintext creds +// across the IPC boundary — same trust level as save-config — DO NOT log it. +ipcMain.handle('validate-credentials', async (_event, payload) => { + if (!payload || !payload.hoster) { + return { status: 'error', message: 'Hoster fehlt' }; + } + const ephemeralHosterConfig = { + username: payload.username || '', + password: payload.password || '', + apiKey: payload.apiKey || '', + enabled: true + }; + try { + return await _dispatchHealthCheck(payload.hoster, ephemeralHosterConfig, payload.otp || ''); + } catch (err) { + return { status: 'error', message: err && err.message ? err.message : 'Validierung fehlgeschlagen' }; + } +}); + +async function _dispatchHealthCheck(hoster, hosterConfig, otp) { + // Mirrors the per-hoster switch in runHosterHealthCheck so both code paths + // (batch check by accountId and ephemeral validate) go through identical + // checkers + timeout wrappers and surface identical result shapes. + if (hoster === 'doodstream.com') { + return withTimeout(checkDoodstreamHealth(hosterConfig, otp), HEALTH_CHECK_TIMEOUT, 'Doodstream-Check'); + } + if (hoster === 'vidmoly.me') { + return withTimeout(checkVidmolyHealth(hosterConfig), HEALTH_CHECK_TIMEOUT, 'Vidmoly-Check'); + } + if (hoster === 'voe.sx') { + return withTimeout(checkVoeHealth(hosterConfig), HEALTH_CHECK_TIMEOUT, 'VOE-Check'); + } + if (hoster === 'byse.sx') { + return withTimeout(checkByseHealth(hosterConfig), HEALTH_CHECK_TIMEOUT, 'Byse-Check'); + } + if (hoster === 'clouddrop.cc') { + return withTimeout(checkClouddropHealth(hosterConfig), HEALTH_CHECK_TIMEOUT, 'Clouddrop-Check'); + } + return { status: 'skipped', message: 'Kein Health-Check fuer diesen Hoster' }; +} + ipcMain.handle('select-files', async () => { const result = await dialog.showOpenDialog(mainWindow, { properties: ['openFile', 'multiSelections'], diff --git a/preload.js b/preload.js index 654793b..ed16a3b 100644 --- a/preload.js +++ b/preload.js @@ -39,6 +39,7 @@ contextBridge.exposeInMainWorld('api', { addJobsToBatch: (payload) => ipcRenderer.invoke('add-jobs-to-batch', payload), finishAfterActive: () => ipcRenderer.invoke('finish-after-active'), runHealthCheck: (payload) => ipcRenderer.invoke('run-health-check', payload), + validateCredentials: (payload) => ipcRenderer.invoke('validate-credentials', payload), // Log import readOwnUploadLog: () => ipcRenderer.invoke('read-own-upload-log'), diff --git a/renderer/app.js b/renderer/app.js index 8441b31..cd32912 100644 --- a/renderer/app.js +++ b/renderer/app.js @@ -3040,6 +3040,11 @@ function _buildAccountCardHtml(name, account, idx) { const statusLabel = isDisabled ? 'Deaktiviert' : (_STATUS_LABELS[st.status] || 'Nicht geprüft'); const statusClass = isDisabled ? 'disabled' : st.status; const credLabel = getCredentialLabel(name, account); + const userLabel = account.label && String(account.label).trim(); + // Subtitle: "Label: XYZ • API: ABC… • " — the user-set label is the + // disambiguator for accounts that otherwise look identical (e.g. two byse + // API-key accounts where you can't tell what's what from the masked key). + const subtitleText = (userLabel ? `Label: ${userLabel} • ` : '') + credLabel; const toggleLabel = isDisabled ? 'Aktivieren' : 'Deaktivieren'; const priorityLabel = idx === 0 ? 'Primär' : `Fallback #${idx}`; @@ -3048,7 +3053,7 @@ function _buildAccountCardHtml(name, account, idx) {