mirror of
https://github.com/projectsend/projectsend.git
synced 2026-09-17 17:15:08 +00:00
Split the virus scanning screen in two, and make the Test button answer
The Test button did nothing visible. The page read `scanner_test_result` off the shared props, and HandleInertiaRequests shares `success` and `error` and nothing else — so the answer was set on the session and never arrived. It is read in the controller and handed over as a prop now, the way the CAPTCHA screen does it. The screen is two tabs, Scanner and Options, following the scheduler's `?tab=` links. Scanner holds the connection and the Test button; Options holds the policies and the backfill. Both end with their Save, and nothing sits below it — before this, "Files already here" and its button were stranded under the Save button of a form they had nothing to do with. Verified by clicking the real button in a browser against a real ClamAV: "Working. ClamAV 1.5.4 detected the test file as Eicar-Test-Signature."
This commit is contained in:
ignacionelson
@@ -45,9 +45,20 @@ class VirusScanningSettingsController extends Controller
|
||||
private readonly ActivityLogger $activity,
|
||||
) {}
|
||||
|
||||
public function edit(): Response
|
||||
public function edit(Request $request): Response
|
||||
{
|
||||
return Inertia::render('system/settings/virus-scanning', [
|
||||
// Which half of the screen is open. The connection and the
|
||||
// policies are two different jobs — one is done once when the
|
||||
// scanner is set up, the other is revisited — and a single
|
||||
// column of fields with two Save buttons reads as one form
|
||||
// that saves half of itself.
|
||||
'tab' => $request->query('tab') === 'options' ? 'options' : 'scanner',
|
||||
// Read from the session here rather than shared as a flash
|
||||
// prop: HandleInertiaRequests shares `success` and `error` and
|
||||
// nothing else, which is why the Test button appeared to do
|
||||
// nothing at all. Same shape the CAPTCHA screen uses.
|
||||
'test_result' => $request->session()->get('scanner_test_result'),
|
||||
'enabled' => $this->config->enabled(),
|
||||
'managed' => $this->config->isManaged(),
|
||||
'address' => $this->config->isManaged() ? '' : $this->settings->get(Setting::VirusScannerAddress),
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { type BreadcrumbItem } from '@/types';
|
||||
import { Head, router, useForm, usePage } from '@inertiajs/react';
|
||||
import { Head, Link, router, useForm } from '@inertiajs/react';
|
||||
import { CheckCircle2, ShieldAlert, TriangleAlert } from 'lucide-react';
|
||||
import { FormEventHandler } from 'react';
|
||||
|
||||
@@ -16,7 +16,12 @@ import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '@
|
||||
import { useTranslation } from '@/hooks/use-translation';
|
||||
import AppLayout from '@/layouts/app-layout';
|
||||
|
||||
type Tab = 'scanner' | 'options';
|
||||
|
||||
interface VirusScanningProps {
|
||||
tab: Tab;
|
||||
/** The Test button's answer, carried through the session. */
|
||||
test_result: { ok: boolean; message: string } | null;
|
||||
enabled: boolean;
|
||||
/** The scanner is supplied by the platform: no address to set, and no switch. */
|
||||
managed: boolean;
|
||||
@@ -35,6 +40,8 @@ interface VirusScanningProps {
|
||||
}
|
||||
|
||||
export default function VirusScanningSettings({
|
||||
tab,
|
||||
test_result,
|
||||
enabled,
|
||||
managed,
|
||||
address,
|
||||
@@ -46,13 +53,15 @@ export default function VirusScanningSettings({
|
||||
counts,
|
||||
}: VirusScanningProps) {
|
||||
const { t } = useTranslation();
|
||||
const testResult = usePage().props.scanner_test_result as { ok: boolean; message: string } | undefined;
|
||||
|
||||
const breadcrumbs: BreadcrumbItem[] = [
|
||||
{ title: t('Settings'), href: '/system/settings' },
|
||||
{ title: t('Virus scanning'), href: '/system/settings/virus-scanning' },
|
||||
];
|
||||
|
||||
// One form behind both tabs, and one Save. The server takes every
|
||||
// field on every save, so switching tabs never loses what was typed on
|
||||
// the other one.
|
||||
const { data, setData, patch, errors, processing, recentlySuccessful } = useForm({
|
||||
enabled,
|
||||
address,
|
||||
@@ -65,21 +74,23 @@ export default function VirusScanningSettings({
|
||||
|
||||
const submit: FormEventHandler = (e) => {
|
||||
e.preventDefault();
|
||||
patch(route('system-settings.virus-scanning.update'));
|
||||
patch(route('system-settings.virus-scanning.update', tab === 'options' ? { tab: 'options' } : {}), { preserveScroll: true });
|
||||
};
|
||||
|
||||
const tabs: { key: Tab; label: string }[] = [
|
||||
{ key: 'scanner', label: t('Scanner') },
|
||||
{ key: 'options', label: t('Options') },
|
||||
];
|
||||
|
||||
return (
|
||||
<AppLayout breadcrumbs={breadcrumbs}>
|
||||
<Head title={t('Virus scanning')} />
|
||||
|
||||
<div className="px-4 py-6">
|
||||
<Heading
|
||||
title={t('Virus scanning')}
|
||||
description={t('Uploaded files are checked before anyone can download them')}
|
||||
/>
|
||||
<div className="space-y-6 px-4 py-6">
|
||||
<Heading title={t('Virus scanning')} description={t('Uploaded files are checked before anyone can download them')} />
|
||||
|
||||
{counts.let_through > 0 && (
|
||||
<Alert variant="destructive" className="mb-6 max-w-xl">
|
||||
<Alert variant="destructive" className="max-w-xl">
|
||||
<TriangleAlert className="size-4" />
|
||||
<AlertTitle>{t(':count files were allowed through without being scanned', { count: counts.let_through })}</AlertTitle>
|
||||
<AlertDescription>
|
||||
@@ -90,179 +101,219 @@ export default function VirusScanningSettings({
|
||||
</Alert>
|
||||
)}
|
||||
|
||||
<form onSubmit={submit} className="max-w-xl space-y-6">
|
||||
{managed ? (
|
||||
<Alert className="max-w-xl">
|
||||
<ShieldAlert className="size-4" />
|
||||
<AlertTitle>{t('Scanning is managed for you')}</AlertTitle>
|
||||
<AlertDescription>
|
||||
{t('Every upload on this site is scanned. The scanner itself is run for you, so there is nothing to connect here.')}
|
||||
</AlertDescription>
|
||||
</Alert>
|
||||
) : (
|
||||
<>
|
||||
<div className="grid gap-2">
|
||||
<div className="flex items-center gap-2">
|
||||
<Checkbox
|
||||
id="enabled"
|
||||
checked={data.enabled}
|
||||
onCheckedChange={(checked) => setData('enabled', checked === true)}
|
||||
/>
|
||||
<Label htmlFor="enabled" className="font-normal">
|
||||
{t('Scan uploaded files for viruses')}
|
||||
</Label>
|
||||
</div>
|
||||
<InputError message={errors.enabled} />
|
||||
</div>
|
||||
<div className="border-border flex gap-1 border-b">
|
||||
{tabs.map(({ key, label }) => (
|
||||
<Link
|
||||
key={key}
|
||||
href={route('system-settings.virus-scanning.edit', key === 'options' ? { tab: 'options' } : {})}
|
||||
preserveScroll
|
||||
className={`-mb-px border-b-2 px-3 py-2 text-sm ${
|
||||
tab === key ? 'border-primary text-foreground font-medium' : 'text-muted-foreground border-transparent'
|
||||
}`}
|
||||
>
|
||||
{label}
|
||||
</Link>
|
||||
))}
|
||||
</div>
|
||||
|
||||
{tab === 'scanner' && (
|
||||
<div className="max-w-xl space-y-6">
|
||||
{/* Above the form on purpose: these act on the scanner
|
||||
as it is now, and nothing belongs after a Save
|
||||
button. */}
|
||||
<div className="space-y-3 rounded-lg border p-4">
|
||||
<HeadingSmall
|
||||
title={t('Check the connection')}
|
||||
description={t('Sends the standard test file, which is harmless and every scanner recognises.')}
|
||||
/>
|
||||
|
||||
<Button
|
||||
type="button"
|
||||
variant="outline"
|
||||
className="w-fit"
|
||||
onClick={() => router.post(route('system-settings.virus-scanning.test'), {}, { preserveScroll: true })}
|
||||
>
|
||||
{t('Test scanner')}
|
||||
</Button>
|
||||
|
||||
{test_result && (
|
||||
<Alert variant={test_result.ok ? 'default' : 'destructive'}>
|
||||
{test_result.ok ? <CheckCircle2 className="size-4" /> : <TriangleAlert className="size-4" />}
|
||||
<AlertDescription>{test_result.message}</AlertDescription>
|
||||
</Alert>
|
||||
)}
|
||||
</div>
|
||||
|
||||
<form onSubmit={submit} className="space-y-6">
|
||||
{managed ? (
|
||||
<Alert>
|
||||
<ShieldAlert className="size-4" />
|
||||
<AlertTitle>{t('Scanning is managed for you')}</AlertTitle>
|
||||
<AlertDescription>
|
||||
{t(
|
||||
'Every upload on this site is scanned. The scanner itself is run for you, so there is nothing to connect here.',
|
||||
)}
|
||||
</AlertDescription>
|
||||
</Alert>
|
||||
) : (
|
||||
<>
|
||||
<div className="grid gap-2">
|
||||
<div className="flex items-center gap-2">
|
||||
<Checkbox
|
||||
id="enabled"
|
||||
checked={data.enabled}
|
||||
onCheckedChange={(checked) => setData('enabled', checked === true)}
|
||||
/>
|
||||
<Label htmlFor="enabled" className="font-normal">
|
||||
{t('Scan uploaded files for viruses')}
|
||||
</Label>
|
||||
</div>
|
||||
<InputError message={errors.enabled} />
|
||||
</div>
|
||||
|
||||
<div className="grid gap-2">
|
||||
<Label htmlFor="address">{t('Scanner address')}</Label>
|
||||
<Input
|
||||
id="address"
|
||||
value={data.address}
|
||||
onChange={(e) => setData('address', e.target.value)}
|
||||
placeholder="tcp://clamav:3310"
|
||||
/>
|
||||
<p className="text-muted-foreground text-sm">
|
||||
{t('A ClamAV daemon, as tcp://host:3310 or unix:///path/to/clamd.sock.')}
|
||||
</p>
|
||||
<InputError message={errors.address} />
|
||||
</div>
|
||||
</>
|
||||
)}
|
||||
|
||||
<SaveButton processing={processing} recentlySuccessful={recentlySuccessful} />
|
||||
</form>
|
||||
</div>
|
||||
)}
|
||||
|
||||
{tab === 'options' && (
|
||||
<div className="max-w-xl space-y-6">
|
||||
<div className="space-y-3 rounded-lg border p-4">
|
||||
<HeadingSmall
|
||||
title={t('Files already here')}
|
||||
description={t('Anything uploaded before scanning was switched on has never been checked.')}
|
||||
/>
|
||||
|
||||
<p className="text-muted-foreground text-sm">
|
||||
{t('Never scanned: :never · Being checked: :pending · In quarantine: :quarantined', {
|
||||
never: counts.never_scanned,
|
||||
pending: counts.pending,
|
||||
quarantined: counts.quarantined,
|
||||
})}
|
||||
</p>
|
||||
|
||||
<Button
|
||||
type="button"
|
||||
variant="outline"
|
||||
disabled={!enabled || counts.never_scanned === 0}
|
||||
onClick={() => router.post(route('system-settings.virus-scanning.scan-existing'), {}, { preserveScroll: true })}
|
||||
>
|
||||
{t('Scan existing files')}
|
||||
</Button>
|
||||
</div>
|
||||
|
||||
<form onSubmit={submit} className="space-y-6">
|
||||
<div className="grid gap-2">
|
||||
<Label htmlFor="address">{t('Scanner address')}</Label>
|
||||
<Label htmlFor="max_size_mb">{t('Largest file to scan (MB)')}</Label>
|
||||
<Input
|
||||
id="address"
|
||||
value={data.address}
|
||||
onChange={(e) => setData('address', e.target.value)}
|
||||
placeholder="tcp://clamav:3310"
|
||||
id="max_size_mb"
|
||||
type="number"
|
||||
min={0}
|
||||
max={4096}
|
||||
value={data.max_size_mb}
|
||||
onChange={(e) => setData('max_size_mb', Number(e.target.value))}
|
||||
/>
|
||||
<p className="text-muted-foreground text-sm">
|
||||
{t('A ClamAV daemon, as tcp://host:3310 or unix:///path/to/clamd.sock.')}
|
||||
{t(
|
||||
'Bigger files are handled by the rule below. Your scanner has its own limit too, and this should not exceed it.',
|
||||
)}
|
||||
</p>
|
||||
<InputError message={errors.address} />
|
||||
<InputError message={errors.max_size_mb} />
|
||||
</div>
|
||||
|
||||
<div className="grid gap-3">
|
||||
<HeadingSmall
|
||||
title={t('Files that cannot be scanned')}
|
||||
description={t('Too large, or an encrypted archive or document the scanner cannot open.')}
|
||||
/>
|
||||
<Select
|
||||
value={data.unscannable_policy}
|
||||
onValueChange={(value: string) => setData('unscannable_policy', value as 'allow' | 'block')}
|
||||
>
|
||||
<SelectTrigger id="unscannable_policy" className="w-full">
|
||||
<SelectValue />
|
||||
</SelectTrigger>
|
||||
<SelectContent>
|
||||
<SelectItem value="allow">{t('Allow them, marked "not scanned"')}</SelectItem>
|
||||
<SelectItem value="block">{t('Block them, like an infected file')}</SelectItem>
|
||||
</SelectContent>
|
||||
</Select>
|
||||
<InputError message={errors.unscannable_policy} />
|
||||
</div>
|
||||
|
||||
<div className="grid gap-3">
|
||||
<HeadingSmall
|
||||
title={t('If the scanner cannot be reached')}
|
||||
description={t('What happens to new uploads while the scanner is down.')}
|
||||
/>
|
||||
<Select
|
||||
value={data.scanner_down_policy}
|
||||
onValueChange={(value: string) => setData('scanner_down_policy', value as 'allow' | 'hold')}
|
||||
>
|
||||
<SelectTrigger id="scanner_down_policy" className="w-full">
|
||||
<SelectValue />
|
||||
</SelectTrigger>
|
||||
<SelectContent>
|
||||
<SelectItem value="allow">{t('Allow them after the wait below, marked "not scanned"')}</SelectItem>
|
||||
<SelectItem value="hold">{t('Hold them until the scanner is back')}</SelectItem>
|
||||
</SelectContent>
|
||||
</Select>
|
||||
<p className="text-muted-foreground text-sm">
|
||||
{t('Files allowed through this way are scanned again automatically once the scanner answers.')}
|
||||
</p>
|
||||
<InputError message={errors.scanner_down_policy} />
|
||||
</div>
|
||||
|
||||
<div className="grid gap-2">
|
||||
<Button
|
||||
type="button"
|
||||
variant="outline"
|
||||
className="w-fit"
|
||||
onClick={() => router.post(route('system-settings.virus-scanning.test'), {}, { preserveScroll: true })}
|
||||
>
|
||||
{t('Test scanner')}
|
||||
</Button>
|
||||
<p className="text-muted-foreground text-sm">
|
||||
{t('Sends the standard test file, which is harmless and every scanner recognises.')}
|
||||
</p>
|
||||
{testResult && (
|
||||
<Alert variant={testResult.ok ? 'default' : 'destructive'}>
|
||||
{testResult.ok ? <CheckCircle2 className="size-4" /> : <TriangleAlert className="size-4" />}
|
||||
<AlertDescription>{testResult.message}</AlertDescription>
|
||||
</Alert>
|
||||
)}
|
||||
<Label htmlFor="wait_minutes">{t('How long to wait for the scanner (minutes)')}</Label>
|
||||
<Input
|
||||
id="wait_minutes"
|
||||
type="number"
|
||||
min={1}
|
||||
max={1440}
|
||||
value={data.wait_minutes}
|
||||
onChange={(e) => setData('wait_minutes', Number(e.target.value))}
|
||||
/>
|
||||
<InputError message={errors.wait_minutes} />
|
||||
</div>
|
||||
</>
|
||||
)}
|
||||
|
||||
<div className="grid gap-2">
|
||||
<Label htmlFor="max_size_mb">{t('Largest file to scan (MB)')}</Label>
|
||||
<Input
|
||||
id="max_size_mb"
|
||||
type="number"
|
||||
min={0}
|
||||
max={4096}
|
||||
value={data.max_size_mb}
|
||||
onChange={(e) => setData('max_size_mb', Number(e.target.value))}
|
||||
/>
|
||||
<p className="text-muted-foreground text-sm">
|
||||
{t('Bigger files are handled by the rule below. Your scanner has its own limit too, and this should not exceed it.')}
|
||||
</p>
|
||||
<InputError message={errors.max_size_mb} />
|
||||
<div className="grid gap-2">
|
||||
<Label htmlFor="existing_rate_per_minute">{t('Files to scan per minute')}</Label>
|
||||
<Input
|
||||
id="existing_rate_per_minute"
|
||||
type="number"
|
||||
min={1}
|
||||
max={6000}
|
||||
className="w-32"
|
||||
value={data.existing_rate_per_minute}
|
||||
onChange={(e) => setData('existing_rate_per_minute', Number(e.target.value))}
|
||||
/>
|
||||
<p className="text-muted-foreground text-sm">
|
||||
{t('Applies to the button above, so a backfill does not starve the scanner of new uploads.')}
|
||||
</p>
|
||||
<InputError message={errors.existing_rate_per_minute} />
|
||||
</div>
|
||||
|
||||
<SaveButton processing={processing} recentlySuccessful={recentlySuccessful} />
|
||||
</form>
|
||||
</div>
|
||||
|
||||
<div className="grid gap-3">
|
||||
<HeadingSmall
|
||||
title={t('Files that cannot be scanned')}
|
||||
description={t('Too large, or an encrypted archive or document the scanner cannot open.')}
|
||||
/>
|
||||
<Select
|
||||
value={data.unscannable_policy}
|
||||
onValueChange={(value: string) => setData('unscannable_policy', value as 'allow' | 'block')}
|
||||
>
|
||||
<SelectTrigger id="unscannable_policy" className="w-full">
|
||||
<SelectValue />
|
||||
</SelectTrigger>
|
||||
<SelectContent>
|
||||
<SelectItem value="allow">{t('Allow them, marked "not scanned"')}</SelectItem>
|
||||
<SelectItem value="block">{t('Block them, like an infected file')}</SelectItem>
|
||||
</SelectContent>
|
||||
</Select>
|
||||
<InputError message={errors.unscannable_policy} />
|
||||
</div>
|
||||
|
||||
<div className="grid gap-3">
|
||||
<HeadingSmall
|
||||
title={t('If the scanner cannot be reached')}
|
||||
description={t('What happens to new uploads while the scanner is down.')}
|
||||
/>
|
||||
<Select
|
||||
value={data.scanner_down_policy}
|
||||
onValueChange={(value: string) => setData('scanner_down_policy', value as 'allow' | 'hold')}
|
||||
>
|
||||
<SelectTrigger id="scanner_down_policy" className="w-full">
|
||||
<SelectValue />
|
||||
</SelectTrigger>
|
||||
<SelectContent>
|
||||
<SelectItem value="allow">{t('Allow them after the wait below, marked "not scanned"')}</SelectItem>
|
||||
<SelectItem value="hold">{t('Hold them until the scanner is back')}</SelectItem>
|
||||
</SelectContent>
|
||||
</Select>
|
||||
<p className="text-muted-foreground text-sm">
|
||||
{t('Files allowed through this way are scanned again automatically once the scanner answers.')}
|
||||
</p>
|
||||
<InputError message={errors.scanner_down_policy} />
|
||||
</div>
|
||||
|
||||
<div className="grid gap-2">
|
||||
<Label htmlFor="wait_minutes">{t('How long to wait for the scanner (minutes)')}</Label>
|
||||
<Input
|
||||
id="wait_minutes"
|
||||
type="number"
|
||||
min={1}
|
||||
max={1440}
|
||||
value={data.wait_minutes}
|
||||
onChange={(e) => setData('wait_minutes', Number(e.target.value))}
|
||||
/>
|
||||
<InputError message={errors.wait_minutes} />
|
||||
</div>
|
||||
|
||||
<SaveButton processing={processing} recentlySuccessful={recentlySuccessful} />
|
||||
</form>
|
||||
|
||||
<div className="mt-10 max-w-xl space-y-3">
|
||||
<HeadingSmall
|
||||
title={t('Files already here')}
|
||||
description={t('Anything uploaded before scanning was switched on has never been checked.')}
|
||||
/>
|
||||
|
||||
<p className="text-muted-foreground text-sm">
|
||||
{t('Never scanned: :never · Being checked: :pending · In quarantine: :quarantined', {
|
||||
never: counts.never_scanned,
|
||||
pending: counts.pending,
|
||||
quarantined: counts.quarantined,
|
||||
})}
|
||||
</p>
|
||||
|
||||
<div className="grid gap-2">
|
||||
<Label htmlFor="existing_rate_per_minute">{t('Files to scan per minute')}</Label>
|
||||
<Input
|
||||
id="existing_rate_per_minute"
|
||||
type="number"
|
||||
min={1}
|
||||
max={6000}
|
||||
className="w-32"
|
||||
value={data.existing_rate_per_minute}
|
||||
onChange={(e) => setData('existing_rate_per_minute', Number(e.target.value))}
|
||||
/>
|
||||
<p className="text-muted-foreground text-sm">{t('Kept low so the scanner stays free for new uploads. Save first.')}</p>
|
||||
</div>
|
||||
|
||||
<Button
|
||||
type="button"
|
||||
variant="outline"
|
||||
disabled={!enabled || counts.never_scanned === 0}
|
||||
onClick={() => router.post(route('system-settings.virus-scanning.scan-existing'), {}, { preserveScroll: true })}
|
||||
>
|
||||
{t('Scan existing files')}
|
||||
</Button>
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
</AppLayout>
|
||||
);
|
||||
|
||||
@@ -117,6 +117,35 @@ test('the test button says when the scanner cannot be reached', function () {
|
||||
->assertSessionHas('scanner_test_result', fn (array $result): bool => $result['ok'] === false);
|
||||
});
|
||||
|
||||
test('the answer actually reaches the screen', function () {
|
||||
// It did not, at first: the page read a flash prop that nothing
|
||||
// shares, so the button appeared to do nothing at all. The result is
|
||||
// handed over as a page prop, like the CAPTCHA screen's.
|
||||
app()->instance(VirusScanner::class, new FakeVirusScanner(ScanVerdict::infected('Eicar-Test-Signature')));
|
||||
|
||||
$this->actingAs($this->admin)->post('/system/settings/virus-scanning/test');
|
||||
|
||||
$this->actingAs($this->admin)->get('/system/settings/virus-scanning')->assertInertia(
|
||||
fn (AssertableInertia $page) => $page->where('test_result.ok', true),
|
||||
);
|
||||
});
|
||||
|
||||
test('the screen opens on the scanner tab, and the other one is a link away', function () {
|
||||
$this->actingAs($this->admin)->get('/system/settings/virus-scanning')->assertInertia(
|
||||
fn (AssertableInertia $page) => $page->where('tab', 'scanner'),
|
||||
);
|
||||
|
||||
$this->actingAs($this->admin)->get('/system/settings/virus-scanning?tab=options')->assertInertia(
|
||||
fn (AssertableInertia $page) => $page->where('tab', 'options'),
|
||||
);
|
||||
|
||||
// Anything else is the default rather than an error: a stale
|
||||
// bookmark should open the page, not break it.
|
||||
$this->actingAs($this->admin)->get('/system/settings/virus-scanning?tab=nonsense')->assertInertia(
|
||||
fn (AssertableInertia $page) => $page->where('tab', 'scanner'),
|
||||
);
|
||||
});
|
||||
|
||||
test('the test button says when the scanner answers but detects nothing', function () {
|
||||
// The failure that looks like success: reachable, and blind. Empty or
|
||||
// broken virus definitions do exactly this.
|
||||
|
||||
Reference in New Issue
Block a user