Skip to content

webclient: hide preview button when user lacks download permission - #2229

Open
rupivbluegreen wants to merge 1 commit into
drakkan:mainfrom
rupivbluegreen:fix/2149-hide-viewer-without-download-perm
Open

webclient: hide preview button when user lacks download permission#2229
rupivbluegreen wants to merge 1 commit into
drakkan:mainfrom
rupivbluegreen:fix/2149-hide-viewer-without-download-perm

Conversation

@rupivbluegreen

Copy link
Copy Markdown

Summary

Fixes #2149.

The eye/preview button in the WebClient file listing rendered for every supported preview extension (image / audio-video / PDF / text-editor) regardless of whether the user had PermDownload for the directory. Clicking it always hit the editfile/preview endpoint, which then errored out with Cannot open file editor: You do not have the required permissions.

This produced:

  • Confusing UX (an action that looked clickable was never going to succeed).
  • Spurious "security finding" reports during pen testing, since the UI was advertising an action the server would always reject.

Change

templates/webclient/files.html: wrap the preview-button switch (lines ~680–747) in a {{- if .CanDownload}} ... {{- end}} block, matching the existing //{{- if not .ShareUploadBaseURL}} convention used a few lines below. filesPage.CanDownload is already populated correctly on both code paths (renderFilesPage for the regular view, renderSharedFilesPage for shares), so no Go-side wiring was required.

The server-side authorization in handleClientEditFile / getFileReader is unchanged — this PR is purely a UI fix; it does not change what the server allows.

Test plan

  • go build ./...
  • go vet ./...
  • New unit test TestFilesPageHidesViewerWithoutDownloadPermission in internal/httpd/internal_test.go renders the files.html template with CanDownload: true and CanDownload: false and asserts the preview-button markers (data-iv-name= for the image case, supportedEditExtensions.includes(extension) for the editor case) are present in the first and absent in the second.
  • Existing TestRenderInvalidTemplate, TestRenderUnexistingFolder, and the TestStaticFiles*/TestWebClient*/TestWebUserShare* test families still pass.
  • Manually verified the conditional removes the eye icon from the listing for a user without download permission.

Out of scope

  • Not changing the edit_url field on the directory-listing JSON (internal/httpd/webclient.go:1269). The server-side check is the source of truth; hiding the button is sufficient for the UX bug.
  • Not retitling/refactoring "viewer" vs "preview" vs "editor" naming.

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 drakkan#2149

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@rupivbluegreen
rupivbluegreen requested a review from drakkan as a code owner May 7, 2026 14:43
@CLAassistant

CLAassistant commented May 7, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@drakkan

drakkan commented May 9, 2026

Copy link
Copy Markdown
Owner

Thanks for the contribution. Before we can review or merge it, we require contributors to sign our CLA (not sure if you are a human or an AI agent).

Also, we already have a more complete fix for this small enhancement in SFTPGo Enterprise, and we may backport it to the open-source version in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Web UI: Viewer button shown when no viewer is available

3 participants