Modernize Attachment Download popup attachmentDownload - sebiboga/testlink-upgraded GitHub Wiki

Modernize-Attachment-Download-popup-attachmentDownload (#1794)

The Attachment Download stream โ€” lib/attachments/attachmentdownload.php (TestLink 1.9.20), the controller that echoed attachment bytes to the browser โ€” is modernized as a Dashio popup backed by a plain-PHP REST BFF. Together with #1525 (upload) and #1638 (delete) this completes the Attachments area.

Screens: gui/templates/attachments/attachmentDownload.html BFF API: api/attachmentsdownload/index.php (action=init | action=download) Docs mirror: docs/attachment-download.md ยท Bug: #1795

Legacy Modern
lib/attachments/attachmentdownload.php api/attachmentsdownload/index.php (metadata + authorized stream)
gui/templates/tl-classic/attachmentdownload.tpl gui/templates/attachments/attachmentDownload.html
attachments.inc.tpl / inc_attachments.tpl (dashio + tl-classic) same lists, now opening the modern popup

Attachment Download - image preview

The security hole โ€” bug #1795

The legacy controller called testlinkInitPage($db) without a rights-check callback: checkRights() was defined at the bottom of the file and never invoked (lib/functions/common.php:538-542). The only effective gate was config_get('attachments')->enabled โ€” a global installation switch, not a permission โ€” and $args->id is a sequential attachments.id.

Measured impact: any authenticated user, including a global <no rights> role (role_id = 3, zero role_rights rows), could stream the full bytes of every attachment of the installation by enumerating ?id= โ€” test case, requirement, test plan and execution attachments (defect evidence, specifications) โ€” including of projects they hold no role on, because is_public was never consulted either. Read-side twin of #1768.

The BFF resolves the owner from the stored fk_table/fk_id (never from caller input) and gates it with the shared right sets of api/_attachauth.php (mgt_view_tc/mgt_modify_tc, mgt_view_req/mgt_modify_req, testplan_*), failing closed when the owner cannot be resolved.

Attachment Download - access denied

The screen

  • Attachment card โ€” title, file name, type + human size + type badge (IMAGE / SVG / FILE), upload date, description, owner with project/plan context chips.
  • Preview card โ€” raster images in place on a transparent checkerboard; explicit states otherwise.
  • Actions โ€” Download (always a download), Open in new tab (only when the stream really renders the file), Refresh and Close in the toolbar, so both survive every error state.
  • Locale switcher โ€” 39 adl.* keys + footers.attachmentDownload in all 10 locale bundles.

Attachment Download - SVG preview blocked for safety

An SVG is never rendered inline: the payload may carry script, so TestLink always serves it as a download (XSS_StringScriptSafe() parity, reinforced by a hard can_inline=false).

Attachment Download - no preview available

Attachment Download - Romanian locale

API

GET ?action=init&id=<attachments.id>

{"status":"ok","attachment":{
  "id":6,"file_name":"screenshot.png","title":"โ€ฆ","description":"โ€ฆ",
  "file_type":"image/png","file_size":89,"file_size_human":"89 B","date_added":"โ€ฆ",
  "token":"<sha256(file_name)>","is_svg":false,"is_image":true,"can_inline":true,
  "preview_url":"โ€ฆ&disposition=inline","inline_url":"โ€ฆ&disposition=inline",
  "download_url":"โ€ฆ&disposition=attachment",
  "owner":{"table":"tcversions","id":10,"label":"10 - ADL test case"},
  "context":{"tproject_id":7,"tplan_id":0,"testproject":"ADL1794","testplan":""}}}

GET ?action=download&id=<id>[&token=<t>][&disposition=inline|attachment]

Condition Answer
no session 401 NOT_AUTHENTICATED
attachments globally disabled 403 ATTACHMENTS_DISABLED
no right on the owning object 403 FORBIDDEN
token present but wrong 403 INVALID_TOKEN
bad / missing id, unknown action, bad disposition 400
unknown attachment, empty or undecodable content 404
wrong verb 405

Hardening added by code review

  • Fail-closed disposition. A missing disposition means attachment; only an explicit disposition=inline renders and the value is whitelisted (else 400). Before that fix the Download button was answered inline โ€” it did not download, and a text/html attachment would have executed in the app origin (stored XSS; attachments are user uploaded).
  • Inline allowlist โ€” image/*, audio/*, video/*, application/pdf, text/plain only; the inline branch additionally carries Content-Security-Policy: sandbox + X-Frame-Options: DENY.
  • Cache-Control: private, no-store on the rights-gated bytes (legacy sent Pragma: public, so a shared cache could replay them for another user).
  • Token compared with hash_equals(); control bytes stripped from the file name plus an RFC 5987 filename*; nosniff on stream and JSON.
  • Open in new tab no longer silently saves what the stream refuses to render โ€” the BFF publishes can_inline/inline_url from the same allowlist it enforces.

Backward compatibility

lib/attachments/attachmentdownload.php is now a session-guarded 302 shim forwarding id (+ skipCheck, + apikey). It must stay: the byte stream is still the <img> src of every printed attachment โ€” print.inc.php (5 call sites), testcase.class.php:8396, testsuite.class.php:1721, requirement_mgr.class.php:4060 โ€” and of the eye toggle in the attachment lists. The 4 Dashio and tl-classic list templates now open the modern popup; getImageURL()/toogleImageURL() stream inline through the BFF with fRoot, so sub-directory installs keep working.

Verification

tmp/TLU_Test_Cases.md โ†’ Modernize โ€” Issue #1794 โ€” 55/55 PASS (entry points, metadata, stream headers and dispositions, rights/token/session, legacy shim with skipCheck and apikey, UI states, i18n, Event Viewer). Event Viewer after the full sweep: no new Error/Warning entries.

โš ๏ธ **GitHub.com Fallback** โš ๏ธ