From dc1ff065734d53d45b90886412b06ea2d6b1a2c7 Mon Sep 17 00:00:00 2001 From: Maurice Date: Sun, 18 Oct 2026 12:32:55 +0200 Subject: [PATCH] fix(docsync): Synology login sends plain credentials, a refused test shows its reason The login went to auth.cgi with account and password JSON-quoted, the way the FileStation calls on entry.cgi take their values. entry.cgi reads JSON where it can; auth.cgi takes a value as it comes, so DSM looked for a user named "trek" with the quotes and refused every account. Both go plain now. The test verdict sat truncated next to the dialog buttons ("The credentials were re..."), with nothing to read the rest by. It wraps to two lines and shows the store's own reason on hover; the phone sheet prints it below. Closes #2666 --- .../Files/docsync/DocSyncConnectModal.tsx | 21 ++++++++++++------- .../Files/docsync/useConnectForm.ts | 4 ++-- .../components/Files/docsync/useDocSync.ts | 14 +++++++++---- .../screens/trip/tabs/MDocSyncSheet.tsx | 8 +++++-- .../providers/synology-drive.client.ts | 7 +++++-- .../doc-sync/synology-drive.provider.test.ts | 9 ++++---- 6 files changed, 42 insertions(+), 21 deletions(-) diff --git a/client/src/components/Files/docsync/DocSyncConnectModal.tsx b/client/src/components/Files/docsync/DocSyncConnectModal.tsx index 754f6e538..828dfb7b8 100644 --- a/client/src/components/Files/docsync/DocSyncConnectModal.tsx +++ b/client/src/components/Files/docsync/DocSyncConnectModal.tsx @@ -1,12 +1,13 @@ import { useId, useState } from 'react' import { AlertTriangle, Check, Loader2, Plug, ShieldAlert } from 'lucide-react' import { DialogButton, DialogFooter, DialogHeader, DialogShell, DialogTile, FooterSpacer, NEUTRAL_TINT } from '../../shared/DialogShell' +import { Tooltip } from '../../shared/Tooltip' import ToggleSwitch from '../../Settings/ToggleSwitch' import { useTranslation } from '../../../i18n/TranslationContext' import { useConnectForm } from './useConnectForm' import { Badge } from './DocSyncBits' import { DOCUMENT_PROVIDER_ICONS } from '../../shared/DocumentProviderIcons' -import type { DocSyncProvider, useDocSync } from './useDocSync' +import type { DocSyncProvider, DocSyncVerdict, useDocSync } from './useDocSync' /** * Connecting a store, as a dialog rather than a panel that unfolds in place. @@ -32,7 +33,7 @@ export default function DocSyncConnectModal({ const Icon = DOCUMENT_PROVIDER_ICONS[provider.id] const form = useConnectForm(provider, existing, sync) - const [verdict, setVerdict] = useState<{ connected: boolean; account?: string; error?: string } | null>(null) + const [verdict, setVerdict] = useState(null) const test = async () => setVerdict(await form.probe()) const labelId = useId() @@ -134,7 +135,7 @@ function Verdict({ busy, failure, }: { - verdict: { connected: boolean; account?: string; error?: string } | null + verdict: DocSyncVerdict | null busy: boolean /** A call that failed outside the connection test (saving, most of all). */ failure: string | null @@ -172,10 +173,16 @@ function Verdict({ ) } const blocked = verdict.error === 'ssrf_blocked' || verdict.error === 'tls_untrusted' + const message = t(`docsync.error.${verdict.error || 'unknown'}`) + // The footer shares its row with the buttons, and a refusal cut to "The + // credentials were re..." with no way to read the rest left nothing to go on + // (#2666). Two lines, and the whole of it with the store's reason on hover. return ( - - {blocked ? : } - {t(`docsync.error.${verdict.error || 'unknown'}`)} - + + + {blocked ? : } + {message} + + ) } diff --git a/client/src/components/Files/docsync/useConnectForm.ts b/client/src/components/Files/docsync/useConnectForm.ts index 4e6d2ea6e..4905f2b41 100644 --- a/client/src/components/Files/docsync/useConnectForm.ts +++ b/client/src/components/Files/docsync/useConnectForm.ts @@ -1,5 +1,5 @@ import { useState } from 'react' -import type { DocSyncConnection, DocSyncProvider, useDocSync } from './useDocSync' +import type { DocSyncConnection, DocSyncProvider, DocSyncVerdict, useDocSync } from './useDocSync' /** * The credential form's rules, for both shells. @@ -39,7 +39,7 @@ export interface ConnectForm { setInsecureTls: (on: boolean) => void /** Every required field is either filled in or already stored. */ complete: boolean - probe: () => Promise<{ connected: boolean; account?: string; error?: string }> + probe: () => Promise save: () => Promise } diff --git a/client/src/components/Files/docsync/useDocSync.ts b/client/src/components/Files/docsync/useDocSync.ts index 9cd8564f8..8f1861451 100644 --- a/client/src/components/Files/docsync/useDocSync.ts +++ b/client/src/components/Files/docsync/useDocSync.ts @@ -12,6 +12,14 @@ import { useServerPing } from '../../../sync/useServerPing' * reason for the same problem. */ +/** What a connection test answers; `detail` is the store's own reason, for the person fixing it. */ +export interface DocSyncVerdict { + connected: boolean + account?: string + error?: string + detail?: string +} + export interface DocSyncField { field_key: string label: string @@ -264,11 +272,9 @@ export function useDocSync(tripId: number | string, enabled: boolean) { const testConnection = useCallback(async (providerId: string, baseUrl: string, credentials: Record, allowInsecureTls: boolean) => { setBusy('test') try { - return await docsyncApi.testConnection(tripId, { providerId, baseUrl, credentials, allowInsecureTls }) as { - connected: boolean; account?: string; error?: string; detail?: string - } + return await docsyncApi.testConnection(tripId, { providerId, baseUrl, credentials, allowInsecureTls }) as DocSyncVerdict } catch (e: unknown) { - return { connected: false, error: readError(e) } + return { connected: false, error: readError(e) } satisfies DocSyncVerdict } finally { setBusy(null) } diff --git a/client/src/mobile/screens/trip/tabs/MDocSyncSheet.tsx b/client/src/mobile/screens/trip/tabs/MDocSyncSheet.tsx index fd239d0aa..43b52e25a 100644 --- a/client/src/mobile/screens/trip/tabs/MDocSyncSheet.tsx +++ b/client/src/mobile/screens/trip/tabs/MDocSyncSheet.tsx @@ -12,7 +12,7 @@ import { useTranslation } from '../../../../i18n' import { DOCUMENT_PROVIDER_ICONS } from '../../../../components/shared/DocumentProviderIcons' import TrekIcon from '../../../../components/shared/TrekIcon' import { - bindingNotice, needsReauth, storeName, useDocSync, type DocSyncLink, type DocSyncProvider, + bindingNotice, needsReauth, storeName, useDocSync, type DocSyncLink, type DocSyncProvider, type DocSyncVerdict, } from '../../../../components/Files/docsync/useDocSync' import { useConnectForm } from '../../../../components/Files/docsync/useConnectForm' import { conflictPolicyKey, nextConflictPolicy } from '../../../../components/Files/docsync/DocSyncBits' @@ -450,7 +450,7 @@ function ConnectView({ }) { const { t } = useTranslation() const form = useConnectForm(provider, sync.connectionFor(provider.id), sync) - const [verdict, setVerdict] = useState<{ connected: boolean; account?: string; error?: string } | null>(null) + const [verdict, setVerdict] = useState(null) return (
@@ -503,6 +503,10 @@ function ConnectView({ ? t('docsync.connect.okAs', { account: verdict.account }) : t('docsync.connected') : t(`docsync.error.${verdict.error ?? 'unknown'}`)} + {/* The store's own reason, so a refusal can be told apart from a typo (#2666). */} + {!verdict.connected && verdict.detail && ( + {verdict.detail} + )}

)} diff --git a/server/src/nest/doc-sync/providers/synology-drive.client.ts b/server/src/nest/doc-sync/providers/synology-drive.client.ts index dcb39ea54..60008dcac 100644 --- a/server/src/nest/doc-sync/providers/synology-drive.client.ts +++ b/server/src/nest/doc-sync/providers/synology-drive.client.ts @@ -780,8 +780,11 @@ export class SynologyDriveClient { api: 'SYNO.API.Auth', version: '6', method: 'login', - account: JSON.stringify(creds.username), - passwd: JSON.stringify(creds.password), + // Plain, unlike the FileStation parameters below. entry.cgi reads a value + // as JSON where it can, auth.cgi takes it as it comes, so a quoted account + // logged in as `"trek"`, quotes included, and every account was refused (#2666). + account: creds.username, + passwd: creds.password, session: SESSION_NAME, format: 'sid', client: 'browser', diff --git a/server/tests/unit/nest/doc-sync/synology-drive.provider.test.ts b/server/tests/unit/nest/doc-sync/synology-drive.provider.test.ts index fa5d1c8c3..4a3a51d46 100644 --- a/server/tests/unit/nest/doc-sync/synology-drive.provider.test.ts +++ b/server/tests/unit/nest/doc-sync/synology-drive.provider.test.ts @@ -286,7 +286,7 @@ describe('SynologyDriveDocumentProvider: capabilities', () => { // ── login and session ──────────────────────────────────────────────────────── describe('SynologyDriveDocumentProvider: login', () => { - it('SYNO-PROVIDER-010: logs in on auth.cgi with JSON-quoted values and asks for a CSRF token', async () => { + it('SYNO-PROVIDER-010: logs in on auth.cgi with plain values and asks for a CSRF token (#2666)', async () => { route({ 'SYNO.FileStation.Info:get': ok({ hostname: 'nas' }) }); const result = await provider.probe(connection()); @@ -296,8 +296,9 @@ describe('SynologyDriveDocumentProvider: login', () => { api: 'SYNO.API.Auth', version: '6', method: 'login', - account: '"trek"', - passwd: '"nas-secret"', + // auth.cgi does not read JSON: a quoted account is a user named "trek" with quotes. + account: 'trek', + passwd: 'nas-secret', session: 'FileStation', format: 'sid', enable_syno_token: 'yes', @@ -643,7 +644,7 @@ describe('SynologyDriveDocumentProvider: device tokens', () => { store.password = 'rotated'; restart(); await provider.listScopes(saved(store)); - expect(logins()[0].params).toMatchObject({ device_id: 'DEVICE-111111', passwd: '"rotated"' }); + expect(logins()[0].params).toMatchObject({ device_id: 'DEVICE-111111', passwd: 'rotated' }); expect(logins()[0].params.otp_code).toBeUndefined(); delete store.otp_code;