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;