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
This commit is contained in:
Maurice
2026-10-18 12:32:55 +02:00
parent fc631d36b6
commit dc1ff06573
6 changed files with 42 additions and 21 deletions
@@ -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<DocSyncVerdict | null>(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 (
<span className="flex min-w-0 items-center gap-2 text-caption text-danger">
{blocked ? <ShieldAlert size={14} /> : <AlertTriangle size={14} />}
<span className="truncate">{t(`docsync.error.${verdict.error || 'unknown'}`)}</span>
</span>
<Tooltip label={verdict.detail ? `${message} ${verdict.detail}` : message} placement="top">
<span className="flex min-w-0 items-center gap-2 text-caption text-danger">
{blocked ? <ShieldAlert size={14} className="flex-none" /> : <AlertTriangle size={14} className="flex-none" />}
<span className="line-clamp-2 [overflow-wrap:anywhere]">{message}</span>
</span>
</Tooltip>
)
}
@@ -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<DocSyncVerdict>
save: () => Promise<boolean>
}
@@ -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<string, string>, 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)
}
@@ -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<DocSyncVerdict | null>(null)
return (
<div className="mt-3 flex flex-col gap-3">
@@ -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 && (
<span className="mt-0.5 block text-m-muted [overflow-wrap:anywhere]">{verdict.detail}</span>
)}
</p>
)}
@@ -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',
@@ -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;