Skip to content

Commit 21b656b

Browse files
authored
Improve Admin Management UX: permissions, search, table, tooltips, and mobile fixes (#1532)
* fix(admin): "Edit Admins" permission not working * revive tooltip UI component * fix tables disappearing in mobile * refactor admins table, fix styles * simplify admins search filters and multi-select Text filters on Advanced search always substring-match, so the Exact/Partial toggles are gone. Web and server permission fields use a themed multi-select over the real <select multiple>, so GET submit and no-JS still work. Also fix server-permission filtering: pass aid not authid, keep SM flags as chars, and allow SM_CUSTOM1-6 in the allowlist so custom flags round-trip after search. * improve edit-admin permissions chrome and layout Add per-card select-all and clearer permission group separators. Drop the empty AdminTabs back strip that left dead space under the topbar, and put Back on the shared Details/Group/Servers/Permissions tab row instead. * fix(overrides): stack add-override fields on mobile The fixed 3-column grid overflowed the card on narrow viewports. Stack Type/Name/Flags below 768px and keep the desktop row. * drop always-true password gate after edit access check * fix(admins): keep override form tables visible on mobile The global <=768px .table hide is for list pages with card mirrors. Form tables (overrides, edit-group perms) have no mirror and must opt out via table--keep-mobile so empty/edit rows do not vanish. * extend seeder data
1 parent 43b42f2 commit 21b656b

27 files changed

Lines changed: 1539 additions & 566 deletions

web/includes/View/AdminAdminsSearchView.php

Lines changed: 10 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -22,16 +22,11 @@
2222
* `{load_template file="admin.admins.search"}` Smarty plugin.
2323
*
2424
* The form submits as a plain `GET` to `?p=admin&c=admins` with one
25-
* parameter per populated filter (`name`, `name_match`, `steamid`,
26-
* `steam_match`, `admemail`, `admemail_match`, `webgroup`,
27-
* `srvadmgroup`, `srvgroup`, `admwebflag[]`, `admsrvflag[]`,
28-
* `server`). admin.admins.php AND-combines every non-empty filter —
29-
* see #1207 ADM-4. No CSRF field — search is read-only.
30-
*
31-
* `name_match` / `admemail_match` were added in #1231 so Login and
32-
* E-mail can be flipped between exact / partial mode the way SteamID
33-
* already could; defaults are partial ('1') to preserve pre-#1231
34-
* substring behaviour for legacy URLs.
25+
* parameter per populated filter (`name`, `steamid`, `admemail`,
26+
* `webgroup`, `srvadmgroup`, `srvgroup`, `admwebflag[]`, `admsrvflag[]`,
27+
* `server`). Text filters are always partial (`LIKE %…%`).
28+
* admin.admins.php AND-combines every non-empty filter. No CSRF
29+
* field — search is read-only.
3530
*
3631
* `$active_filter_*` mirror the corresponding $_GET keys so the
3732
* template can pre-fill the form without splattering
@@ -40,14 +35,11 @@
4035
* given $_GET shape came from a modern submit, a legacy
4136
* `advType=…&advSearch=…` URL, or nothing at all.
4237
*
43-
* #1303 — collapsible disclosure
44-
* ------------------------------
4538
* The form is wrapped in a `<details class="card filters-details">`
4639
* default-collapsed disclosure so the unfiltered admin list paints
47-
* above the fold. `$has_active_filters` (derived from the nine
48-
* `active_filter_*` value slots — match-mode toggles don't count
49-
* because they always carry a default) drives the `[open]` attribute,
50-
* so any post-submit page paints with the form expanded. The chrome
40+
* above the fold. `$has_active_filters` (derived from the
41+
* `active_filter_*` value slots) drives the `[open]` attribute, so
42+
* any post-submit page paints with the form expanded. The chrome
5143
* mirrors `core/admin_sidebar.tpl`'s mobile `<details open>` pattern
5244
* (chevron + label + `prefers-reduced-motion: reduce` override). The
5345
* count badge ("Filters · N active") rides `$active_filter_count`.
@@ -88,10 +80,8 @@ final class AdminAdminsSearchView extends View
8880
* `admsrvflag[]` values.
8981
* @param int $active_filter_count Number of non-empty filter
9082
* value slots — drives the `<summary>` count badge ("Filters
91-
* · N active") and `$has_active_filters`. Match-mode toggles
92-
* (`name_match` / `steam_match` / `admemail_match`) are NOT
93-
* counted: they always carry a default ('0' or '1') and only
94-
* refine the matching filter, they don't filter on their own.
83+
* · N active") and `$has_active_filters`. Empty multi-select
84+
* arrays count as zero.
9585
* @param bool $has_active_filters Convenience boolean derived from
9686
* `$active_filter_count > 0`. The template uses it to decide
9787
* whether the disclosure paints `<details open>` (post-submit
@@ -107,11 +97,8 @@ public function __construct(
10797
public readonly array $admwebflag_list,
10898
public readonly array $admsrvflag_list,
10999
public readonly string $active_filter_name = '',
110-
public readonly string $active_filter_name_match = '1',
111100
public readonly string $active_filter_steamid = '',
112-
public readonly string $active_filter_steam_match = '0',
113101
public readonly string $active_filter_admemail = '',
114-
public readonly string $active_filter_admemail_match = '1',
115102
public readonly string $active_filter_webgroup = '',
116103
public readonly string $active_filter_srvadmgroup = '',
117104
public readonly string $active_filter_srvgroup = '',

web/includes/View/EditAdminDetailsView.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,9 @@
1010
* The page handler (`admin.edit.admindetails.php`) gates entry on
1111
* `ADMIN_OWNER | ADMIN_EDIT_ADMINS` (or self-edit) before reaching the
1212
* template, so the View doesn't carry its own access boolean. `$change_pass`
13-
* is a per-request capability flag from the handler — true when the current
14-
* user is allowed to set the target admin's password (root or self).
13+
* is always true here: anyone allowed to open this page may also set
14+
* the target's password. Edit-admins callers cannot open owner targets;
15+
* that block lives in the page handler.
1516
*
1617
* The property set is intentionally identical to the legacy handler's
1718
* `$theme->assign(...)` calls so the existing `$theme->display(...)` path

web/includes/system-functions.php

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,12 @@
3636
* still build link strings server-side.
3737
*
3838
* NOTE: the `$tooltip`-bearing arm picks up the `tip` / `perm` CSS
39-
* class (legacy default theme); the bare arm has no class. The HTML
40-
* is whitespace-padded between the opening and closing tag for
41-
* legacy-template compatibility — the v1.x consumer relied on the
42-
* leading + trailing space when concatenating links inline.
39+
* class (legacy default theme) and emits `data-tooltip` for the
40+
* themed tip in `sb.js` (not the native `title=` bubble). The bare
41+
* arm has no class. The HTML is whitespace-padded between the
42+
* opening and closing tag for legacy-template compatibility — the
43+
* v1.x consumer relied on the leading + trailing space when
44+
* concatenating links inline.
4345
*/
4446
function CreateLinkR(string $title, string $url, string $tooltip = '', string $target = '_self', bool $wide = false, string $onclick = ''): string
4547
{
@@ -50,7 +52,7 @@ function CreateLinkR(string $title, string $url, string $tooltip = '', string $t
5052
];
5153
if ($hasTooltip) {
5254
$attrs['class'] = $wide ? 'perm' : 'tip';
53-
$attrs['title'] = $tooltip;
55+
$attrs['data-tooltip'] = $tooltip;
5456
} else {
5557
$attrs['onclick'] = $onclick;
5658
}

web/pages/admin.admins.php

Lines changed: 41 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,12 @@
5656
*/
5757

5858
/** @var bool $canListAdmins */
59-
$canListAdmins = $userbank->HasAccess(WebPermission::mask(WebPermission::Owner, WebPermission::ListAdmins));
59+
$canListAdmins = $userbank->HasAccess(WebPermission::mask(
60+
WebPermission::Owner,
61+
WebPermission::ListAdmins,
62+
WebPermission::EditAdmins,
63+
WebPermission::DeleteAdmins,
64+
));
6065
/** @var bool $canAddAdmins */
6166
$canAddAdmins = $userbank->HasAccess(WebPermission::mask(WebPermission::Owner, WebPermission::AddAdmins));
6267
/** @var bool $canEditAdmins */
@@ -79,7 +84,7 @@
7984
[
8085
'slug' => 'admins',
8186
'name' => 'Admins',
82-
'permission' => ADMIN_OWNER | ADMIN_LIST_ADMINS,
87+
'permission' => ADMIN_OWNER | ADMIN_LIST_ADMINS | ADMIN_EDIT_ADMINS | ADMIN_DELETE_ADMINS,
8388
'url' => 'index.php?p=admin&c=admins&section=admins',
8489
'icon' => 'users',
8590
],
@@ -195,11 +200,12 @@
195200
* combined filter form and AND the populated filters server-side.
196201
*
197202
* The new wire format reads each filter from its own query parameter
198-
* (`name`, `steamid`, `steam_match`, `admemail`, `webgroup`,
203+
* (`name`, `steamid`, `admemail`, `webgroup`,
199204
* `srvadmgroup`, `srvgroup`, `admwebflag[]`, `admsrvflag[]`, `server`)
200-
* so a single GET submit carries the full filter snapshot. URL-shareable
201-
* searches are preserved by the legacy-shim block below: any incoming
202-
* `?advType=…&advSearch=…` is translated into the new shape so old
205+
* so a single GET submit carries the full filter snapshot. Text filters
206+
* always substring-match (`LIKE %…%`). URL-shareable searches are
207+
* preserved by the legacy-shim block below: any incoming
208+
* `?advType=…&advSearch=…` is translated into the modern shape so old
203209
* bookmarks and cross-page links keep working.
204210
*
205211
* Server-side filters are AND-combined: a request with two non-empty
@@ -238,10 +244,9 @@
238244
case 'steam':
239245
// The legacy form distinguished exact (`steamid`) from
240246
// partial (`steam`) matches as two distinct advTypes. The
241-
// modern form folds both onto `steamid` + `steam_match`.
247+
// modern form folds both onto `steamid` (always partial).
242248
if (!isset($_GET['steamid']) || $_GET['steamid'] === '') {
243-
$_GET['steamid'] = $legacyValue;
244-
$_GET['steam_match'] = '1';
249+
$_GET['steamid'] = $legacyValue;
245250
}
246251
break;
247252
case 'admwebflag':
@@ -253,52 +258,26 @@
253258
}
254259
}
255260

256-
// 1) Login name (exact or partial against ADM.user).
257-
// `name_match` was added in #1231; default is partial ('1') so
258-
// pre-#1231 URLs (`?name=alice` with no name_match) keep their
259-
// substring semantics. `0` flips to exact.
261+
// 1) Login name (partial against ADM.user).
260262
if (!empty($_GET['name']) && is_string($_GET['name'])) {
261-
$partialName = !isset($_GET['name_match']) || (string) $_GET['name_match'] !== '0';
262-
if ($partialName) {
263-
$where .= " AND ADM.user LIKE ?";
264-
$whereParams[] = '%' . $_GET['name'] . '%';
265-
} else {
266-
$where .= " AND ADM.user = ?";
267-
$whereParams[] = $_GET['name'];
268-
}
269-
$activeFilters['name'] = (string) $_GET['name'];
270-
$activeFilters['name_match'] = $partialName ? '1' : '0';
263+
$where .= " AND ADM.user LIKE ?";
264+
$whereParams[] = '%' . $_GET['name'] . '%';
265+
$activeFilters['name'] = (string) $_GET['name'];
271266
}
272267

273-
// 2) Steam ID (exact or partial against ADM.authid).
268+
// 2) Steam ID (partial against ADM.authid).
274269
if (!empty($_GET['steamid']) && is_string($_GET['steamid'])) {
275-
$partial = isset($_GET['steam_match']) && (string) $_GET['steam_match'] === '1';
276-
if ($partial) {
277-
$where .= " AND ADM.authid LIKE ?";
278-
$whereParams[] = '%' . $_GET['steamid'] . '%';
279-
} else {
280-
$where .= " AND ADM.authid = ?";
281-
$whereParams[] = $_GET['steamid'];
282-
}
283-
$activeFilters['steamid'] = (string) $_GET['steamid'];
284-
$activeFilters['steam_match'] = $partial ? '1' : '0';
270+
$where .= " AND ADM.authid LIKE ?";
271+
$whereParams[] = '%' . $_GET['steamid'] . '%';
272+
$activeFilters['steamid'] = (string) $_GET['steamid'];
285273
}
286274

287-
// 3) E-mail (exact or partial; `admemail_match` was added in #1231,
288-
// same default-partial shape as `name_match`). Gated on the same
289-
// flag the search box gates the input field on so URL forgery
290-
// can't bypass the visibility gate.
275+
// 3) E-mail (partial). Gated on the same flag the search box gates
276+
// the input field on so URL forgery can't bypass the visibility gate.
291277
if (!empty($_GET['admemail']) && is_string($_GET['admemail']) && $userbank->HasAccess(WebPermission::mask(WebPermission::Owner, WebPermission::EditAdmins))) {
292-
$partialEmail = !isset($_GET['admemail_match']) || (string) $_GET['admemail_match'] !== '0';
293-
if ($partialEmail) {
294-
$where .= " AND ADM.email LIKE ?";
295-
$whereParams[] = '%' . $_GET['admemail'] . '%';
296-
} else {
297-
$where .= " AND ADM.email = ?";
298-
$whereParams[] = $_GET['admemail'];
299-
}
300-
$activeFilters['admemail'] = (string) $_GET['admemail'];
301-
$activeFilters['admemail_match'] = $partialEmail ? '1' : '0';
278+
$where .= " AND ADM.email LIKE ?";
279+
$whereParams[] = '%' . $_GET['admemail'] . '%';
280+
$activeFilters['admemail'] = (string) $_GET['admemail'];
302281
}
303282

304283
// 4) Web group (`:prefix_groups.gid` -> `:prefix_admins.gid`).
@@ -360,7 +339,9 @@
360339
}
361340
}
362341

363-
// 8) Server permission flags (multi).
342+
// 8) Server permission flags (multi). SM_* constants are single-char
343+
// strings (`SM_ROOT` = `z`); pass them to HasAccess as strings so the
344+
// srv_flags path runs. SM_ROOT implies every other server flag.
364345
$rawSrvFlags = $_GET['admsrvflag'] ?? null;
365346
if (is_string($rawSrvFlags)) {
366347
$rawSrvFlags = explode(',', $rawSrvFlags);
@@ -369,27 +350,29 @@
369350
/** @var list<string> $srvFlagNames */
370351
$srvFlagNames = [];
371352
foreach ($rawSrvFlags as $candidate) {
372-
if (is_string($candidate) && preg_match('/^SM_[A-Z_]+$/', $candidate) && defined($candidate)) {
353+
if (is_string($candidate) && preg_match('/^SM_[A-Z0-9_]+$/', $candidate) && defined($candidate)) {
373354
$srvFlagNames[] = $candidate;
374355
}
375356
}
376357
if (!empty($srvFlagNames)) {
377-
$flagBits = array_map(fn(string $name): int => (int) constant($name), $srvFlagNames);
378-
$alladmins = $GLOBALS['PDO']->query("SELECT aid, authid FROM `:prefix_admins` WHERE aid > 0")->resultset();
358+
/** @var list<string> $flagChars */
359+
$flagChars = array_map(fn(string $name): string => (string) constant($name), $srvFlagNames);
360+
$alladmins = $GLOBALS['PDO']->query("SELECT aid FROM `:prefix_admins` WHERE aid > 0")->resultset();
379361
$accessAids = [];
380362
foreach ($alladmins as $row) {
363+
$aid = (int) $row['aid'];
381364
$matched = false;
382-
foreach ($flagBits as $fla) {
383-
if ($userbank->HasAccess($fla, $row['authid'])) {
365+
foreach ($flagChars as $fla) {
366+
if ($userbank->HasAccess($fla, $aid)) {
384367
$matched = true;
385368
break;
386369
}
387370
}
388-
if (!$matched && $userbank->HasAccess(SM_ROOT, $row['authid'])) {
371+
if (!$matched && $userbank->HasAccess(SM_ROOT, $aid)) {
389372
$matched = true;
390373
}
391374
if ($matched) {
392-
$accessAids[] = (int) $row['aid'];
375+
$accessAids[] = $aid;
393376
}
394377
}
395378
if (empty($accessAids)) {
@@ -470,10 +453,10 @@
470453
$admin['web_group'] = $userbank->GetProperty("group_name", $admin['aid']);
471454
$admin['server_group'] = $userbank->GetProperty("srv_groups", $admin['aid']);
472455
if (empty($admin['web_group']) || $admin['web_group'] == " ") {
473-
$admin['web_group'] = "No Group/Individual Permissions";
456+
$admin['web_group'] = "No groups";
474457
}
475458
if (empty($admin['server_group']) || $admin['server_group'] == " ") {
476-
$admin['server_group'] = "No Group/Individual Permissions";
459+
$admin['server_group'] = "No groups";
477460
}
478461
$GLOBALS['PDO']->query("SELECT count(authid) AS num FROM `:prefix_bans` WHERE aid = :aid");
479462
$GLOBALS['PDO']->bind(':aid', $admin['aid']);

web/pages/admin.admins.search.php

Lines changed: 11 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -158,19 +158,12 @@
158158
$activeSrvFlags = [];
159159
if (is_array($rawSrvFlag)) {
160160
foreach ($rawSrvFlag as $f) {
161-
if (is_string($f) && preg_match('/^SM_[A-Z_]+$/', $f)) {
161+
if (is_string($f) && preg_match('/^SM_[A-Z0-9_]+$/', $f)) {
162162
$activeSrvFlags[] = $f;
163163
}
164164
}
165165
}
166166

167-
// Match-mode defaults differ per filter (#1231):
168-
// - steam_match defaults to '0' (exact) — typical SteamID
169-
// queries are "find this one admin by their full ID".
170-
// - name_match / admemail_match default to '1' (partial) so
171-
// pre-#1231 URLs (`?name=alice`) keep their substring
172-
// behaviour. Adding the toggle widens the UI without
173-
// regressing the default.
174167
$activeFilterName = is_string($_GET['name'] ?? null) ? (string) $_GET['name'] : '';
175168
$activeFilterSteamid = is_string($_GET['steamid'] ?? null) ? (string) $_GET['steamid'] : '';
176169
$activeFilterAdmemail = is_string($_GET['admemail'] ?? null) ? (string) $_GET['admemail'] : '';
@@ -179,26 +172,20 @@
179172
$activeFilterSrvgroup = is_scalar($_GET['srvgroup'] ?? null) ? (string) $_GET['srvgroup'] : '';
180173
$activeFilterServer = is_scalar($_GET['server'] ?? null) ? (string) $_GET['server'] : '';
181174

182-
// #1303 — the `admemail` filter is permission-gated by
183-
// `$can_editadmin` in both the rendering template AND the page
184-
// handler (`admin.admins.php` ignores `?admemail=` from a user without
185-
// `EditAdmins | Owner`). For URL-forgery cases where a non-admin
186-
// passes `?admemail=foo`, the input is hidden in the form and the
187-
// server narrows nothing; the count must mirror that — otherwise the
188-
// "N active" badge would say "1 active" while every visible filter
189-
// row reads empty. Mirror the gate locally so the count stays an
190-
// honest summary of what the visible form actually filters on.
175+
// The `admemail` filter is permission-gated by `$can_editadmin` in
176+
// both the rendering template AND the page handler (`admin.admins.php`
177+
// ignores `?admemail=` from a user without `EditAdmins | Owner`). For
178+
// URL-forgery cases where a non-admin passes `?admemail=foo`, the
179+
// input is hidden in the form and the server narrows nothing; the
180+
// count must mirror that — otherwise the "N active" badge would say
181+
// "1 active" while every visible filter row reads empty.
191182
$canFilterByEmail = $userbank->HasAccess(WebPermission::mask(WebPermission::EditAdmins, WebPermission::Owner));
192183

193184
// #1303 — count populated filter slots so the disclosure can paint a
194185
// "Filters · N active" badge on the <summary> and auto-expand on
195-
// post-submit. Match-mode selects (`name_match` / `steam_match` /
196-
// `admemail_match`) deliberately don't count: they always carry a
197-
// default ('0' or '1') and only refine the matching filter, they
198-
// don't filter on their own. Empty multi-select arrays count as zero
199-
// even though the array itself "exists" — the user hasn't picked a
200-
// permission. The `admemail` slot only counts when the user can
201-
// actually filter by it (see `$canFilterByEmail` above).
186+
// post-submit. Empty multi-select arrays count as zero even though
187+
// the array itself "exists". The `admemail` slot only counts when
188+
// the user can actually filter by it (see `$canFilterByEmail`).
202189
$activeFilterCount =
203190
($activeFilterName !== '' ? 1 : 0)
204191
+ ($activeFilterSteamid !== '' ? 1 : 0)
@@ -220,11 +207,8 @@
220207
admwebflag_list: $webflag,
221208
admsrvflag_list: $serverflag,
222209
active_filter_name: $activeFilterName,
223-
active_filter_name_match: is_scalar($_GET['name_match'] ?? null) ? (string) $_GET['name_match'] : '1',
224210
active_filter_steamid: $activeFilterSteamid,
225-
active_filter_steam_match: is_scalar($_GET['steam_match'] ?? null) ? (string) $_GET['steam_match'] : '0',
226211
active_filter_admemail: $activeFilterAdmemail,
227-
active_filter_admemail_match: is_scalar($_GET['admemail_match'] ?? null) ? (string) $_GET['admemail_match'] : '1',
228212
active_filter_webgroup: $activeFilterWebgroup,
229213
active_filter_srvadmgroup: $activeFilterSrvadmgroup,
230214
active_filter_srvgroup: $activeFilterSrvgroup,

0 commit comments

Comments
 (0)