From 7ae10f9c04c3cb07499babb3b95621ba8d50f8cb Mon Sep 17 00:00:00 2001 From: rumblefrog Date: Tue, 26 May 2026 13:08:38 -0400 Subject: [PATCH 1/2] fix(demos): add download link and remove button on ban edit page Closes the first two gaps from #1464: uploaded demos now link to getdemo.php for download, and admins can remove an attached demo via a new bans.remove_demo JSON action with row-level edit permission checks. --- web/api/handlers/_register.php | 1 + web/api/handlers/bans.php | 57 +++++++++++++ web/includes/View/AdminBansEditView.php | 14 ++- web/pages/admin.edit.ban.php | 85 ++++++++++++++++--- web/scripts/api-contract.js | 5 ++ web/tests/api/BansTest.php | 44 ++++++++++ web/tests/api/PermissionMatrixTest.php | 1 + .../integration/AdminEditBanDemoTest.php | 36 ++++++++ web/themes/default/page_admin_edit_ban.tpl | 27 ++++-- 9 files changed, 245 insertions(+), 25 deletions(-) create mode 100644 web/tests/integration/AdminEditBanDemoTest.php diff --git a/web/api/handlers/_register.php b/web/api/handlers/_register.php index 584af2641..0938b471c 100644 --- a/web/api/handlers/_register.php +++ b/web/api/handlers/_register.php @@ -69,6 +69,7 @@ Api::register('bans.add_comment', 'api_bans_add_comment', 0, true); Api::register('bans.edit_comment', 'api_bans_edit_comment', 0, true); Api::register('bans.remove_comment', 'api_bans_remove_comment', ADMIN_OWNER); +Api::register('bans.remove_demo', 'api_bans_remove_demo', 0, true); Api::register('bans.group_ban', 'api_bans_group_ban', ADMIN_OWNER | ADMIN_ADD_BAN); Api::register('bans.ban_member_of_group', 'api_bans_ban_member_of_group', ADMIN_OWNER | ADMIN_ADD_BAN); Api::register('bans.ban_friends', 'api_bans_ban_friends', ADMIN_OWNER | ADMIN_ADD_BAN); diff --git a/web/api/handlers/bans.php b/web/api/handlers/bans.php index 9495c1135..04e7b1914 100644 --- a/web/api/handlers/bans.php +++ b/web/api/handlers/bans.php @@ -349,6 +349,63 @@ function api_bans_edit_comment(array $params): array ]; } +function api_bans_remove_demo(array $params): array +{ + global $userbank, $username; + + $bid = (int) ($params['bid'] ?? 0); + if ($bid <= 0) { + throw new ApiError('validation', 'Missing or invalid ban id.', 'bid'); + } + + $row = $GLOBALS['PDO'] + ->query( + 'SELECT ba.aid, ad.gid + FROM `:prefix_bans` AS ba + LEFT JOIN `:prefix_admins` AS ad ON ba.aid = ad.aid + WHERE ba.bid = :bid' + ) + ->single([':bid' => $bid]); + + if (!$row) { + throw new ApiError('not_found', 'Ban not found.'); + } + + $canEdit = $userbank->HasAccess(WebPermission::mask(WebPermission::Owner, WebPermission::EditAllBans)) + || ($userbank->HasAccess(WebPermission::EditOwnBans) && (int) $row['aid'] === $userbank->GetAid()) + || ($userbank->HasAccess(WebPermission::EditGroupBans) && (int) $row['gid'] === (int) $userbank->GetProperty('gid')); + + if (!$canEdit) { + throw new ApiError('forbidden', 'You do not have permission to edit this ban.'); + } + + $demo = $GLOBALS['PDO'] + ->query( + "SELECT filename FROM `:prefix_demos` + WHERE demid = :bid AND UPPER(demtype) = 'B'" + ) + ->single([':bid' => $bid]); + + if (!$demo) { + throw new ApiError('not_found', 'No demo is attached to this ban.'); + } + + $onDisk = basename((string) $demo['filename']); + if ($onDisk !== '' && is_file(SB_DEMOS . '/' . $onDisk)) { + @unlink(SB_DEMOS . '/' . $onDisk); + } + + $GLOBALS['PDO'] + ->query("DELETE FROM `:prefix_demos` WHERE demid = :bid AND UPPER(demtype) = 'B'") + ->execute([':bid' => $bid]); + + Log::add(LogType::Message, 'Demo Removed', "$username removed the demo from ban #$bid"); + + return [ + 'removed' => true, + ]; +} + function api_bans_remove_comment(array $params): array { global $username; diff --git a/web/includes/View/AdminBansEditView.php b/web/includes/View/AdminBansEditView.php index 37563d45b..600cacac2 100644 --- a/web/includes/View/AdminBansEditView.php +++ b/web/includes/View/AdminBansEditView.php @@ -16,10 +16,14 @@ * - `$ban_name` Current player name on the ban row. * - `$ban_authid` Current Steam2 authid on the ban row. * - `$ban_ip` Current IP on the ban row. - * - `$ban_demo` Pre-built "Uploaded: safename" snippet - * (or empty), htmlspecialchars'd in the page handler - * per #1113. Rendered with `{nofilter}`; the safety - * annotation lives at the call site in the template. + * - `$ban_id` Ban row id (`:prefix_bans.bid`); drives the demo + * download URL and the remove-demo JSON action. + * - `$has_demo` Whether a demo row exists for this ban. + * - `$ban_demo` Pre-built "Uploaded: safename" + * snippet (or empty), htmlspecialchars'd in the + * page handler per #1113. Rendered with + * `{nofilter}`; the safety annotation lives at the + * call site in the template. * - `$customreason` `false` when custom reasons are disabled, otherwise * the list of reason strings from `bans.customreasons`. * @@ -38,6 +42,8 @@ final class AdminBansEditView extends View */ public function __construct( public readonly bool $can_edit_ban, + public readonly int $ban_id, + public readonly bool $has_demo, public readonly string $ban_name, public readonly string $ban_authid, public readonly string $ban_ip, diff --git a/web/pages/admin.edit.ban.php b/web/pages/admin.edit.ban.php index 57e3ff49f..6d8f3c553 100644 --- a/web/pages/admin.edit.ban.php +++ b/web/pages/admin.edit.ban.php @@ -338,22 +338,31 @@ function emitEditBanToastAndRedirect(string $kind, string $title, string $body, : false; /** @var false|list $customReason */ -\Sbpp\View\Renderer::render($theme, new \Sbpp\View\AdminBansEditView( - can_edit_ban: $canEditBan, - ban_name: (string) $res['name'], - ban_authid: trim((string) $res['authid']), - ban_ip: (string) $res['ip'], +$hasDemo = !empty($res['dname']); +$banDemoHtml = ''; +if ($hasDemo) { // Issue #1113: dname is the admin-supplied original filename of the // demo (POST'd by whoever edited the ban + uploaded the demo, stored // as `:prefix_demos.origname`). It used to be interpolated raw into // HTML rendered with `nofilter`, so a filename like // `` turned the edit-ban page into stored XSS // for any admin viewing that ban. htmlspecialchars + ENT_QUOTES so - // the value is safe inside both the surrounding `…` text and - // the `nofilter` render in the template. - ban_demo: !empty($res['dname']) - ? 'Uploaded: ' . htmlspecialchars((string) $res['dname'], ENT_QUOTES, 'UTF-8') . '' - : '', + // the value is safe inside the link text. The href is server-built + // from the int-cast bid only (#1464). + $safeName = htmlspecialchars((string) $res['dname'], ENT_QUOTES, 'UTF-8'); + $demoUrl = 'getdemo.php?type=B&id=' . rawurlencode((string) $_GET['id']); + $banDemoHtml = 'Uploaded: ' + . $safeName . ''; +} + +\Sbpp\View\Renderer::render($theme, new \Sbpp\View\AdminBansEditView( + can_edit_ban: $canEditBan, + ban_id: (int) $_GET['id'], + has_demo: $hasDemo, + ban_name: (string) $res['name'], + ban_authid: trim((string) $res['authid']), + ban_ip: (string) $res['ip'], + ban_demo: $banDemoHtml, customreason: $customReason, )); @@ -380,6 +389,7 @@ function emitEditBanToastAndRedirect(string $kind, string $title, string $body, $redirectUrl = 'index.php?p=banlist' . $pagelink; $redirectUrlJs = json_encode($redirectUrl, JSON_THROW_ON_ERROR | JSON_HEX_TAG | JSON_HEX_AMP | JSON_HEX_APOS | JSON_HEX_QUOT); $postSuccessJs = $postSuccess ? 'true' : 'false'; +$banIdJs = json_encode((int) $_GET['id'], JSON_THROW_ON_ERROR); ?>