From 9a56cbc1cbe4f94bf79c727fe27675458776ccd9 Mon Sep 17 00:00:00 2001 From: rupivbluegreen Date: Thu, 7 May 2026 16:43:02 +0200 Subject: [PATCH] webclient: hide preview button when user lacks download permission The eye/preview button in the file listing was rendered for every file matching a supported preview extension, regardless of whether the user had download permission. Clicking it always led to a permission error from the editfile/preview endpoint, producing confusing UX and spurious security/pen-test reports. Gate the preview-button branch on .CanDownload so the action is only offered when the server would actually honor it. The underlying server-side check is unchanged. Fixes #2149 Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/httpd/internal_test.go | 43 +++++++++++++++++++++++++++++++++ templates/webclient/files.html | 2 ++ 2 files changed, 45 insertions(+) diff --git a/internal/httpd/internal_test.go b/internal/httpd/internal_test.go index 4749460ba..056389e45 100644 --- a/internal/httpd/internal_test.go +++ b/internal/httpd/internal_test.go @@ -2168,6 +2168,49 @@ func TestRenderInvalidTemplate(t *testing.T) { } } +// TestFilesPageHidesViewerWithoutDownloadPermission is a regression test for +// drakkan/sftpgo#2149. Users that lack PermDownload must not see the +// eye/preview button in the WebClient file listing, since the underlying +// editfile/preview endpoint can only return a permission error for them. +func TestFilesPageHidesViewerWithoutDownloadPermission(t *testing.T) { + loadClientTemplates(filepath.Join("..", "..", "templates")) + tmpl, ok := clientTemplates[templateClientFiles] + require.True(t, ok, "files template must be loaded") + require.NotNil(t, tmpl) + + render := func(canDownload bool) string { + data := filesPage{ + baseClientPage: baseClientPage{ + commonBasePage: commonBasePage{StaticURL: "/static"}, + LoggedUser: &dataprovider.User{}, + }, + CanDownload: canDownload, + QuotaUsage: newUserQuotaUsage(&dataprovider.User{}), + } + var buf bytes.Buffer + require.NoError(t, tmpl.ExecuteTemplate(&buf, templateClientFiles, data)) + return buf.String() + } + + const ( + // markers that only appear inside the preview-button switch block + imagePreviewMarker = "data-iv-name=" + editorPreviewMarker = "supportedEditExtensions.includes(extension)" + ) + + body := render(true) + assert.Contains(t, body, imagePreviewMarker, + "image preview button must render when user has download permission") + assert.Contains(t, body, editorPreviewMarker, + "text editor preview check must render when user has download permission") + + body = render(false) + assert.NotContains(t, body, imagePreviewMarker, + "image preview button must be omitted when user lacks download permission") + assert.NotContains(t, body, editorPreviewMarker, + "text editor preview check must be omitted when user lacks download permission") +} + func TestQuotaScanInvalidFs(t *testing.T) { user := &dataprovider.User{ BaseUser: sdk.BaseUser{ diff --git a/templates/webclient/files.html b/templates/webclient/files.html index f046651c1..d2364664b 100644 --- a/templates/webclient/files.html +++ b/templates/webclient/files.html @@ -677,6 +677,7 @@

if (type === 'display') { let previewDiv = ""; if (row["type"] == "2") { + //{{- if .CanDownload}} let filename = escapeHTML(row["name"]); let extension = filename.slice((filename.lastIndexOf(".") - 1 >>> 0) + 2).toLowerCase(); switch (extension) { @@ -745,6 +746,7 @@

} //{{- end}} } + //{{- end}} } let more = `{{- if not .ShareUploadBaseURL}} {{- if or .CanRename .CanAddFiles .CanShare .CanDelete }}