Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
91 changes: 91 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,58 @@ of the diff ship together or not at all.
- In JS, reference perms as `Perms.ADMIN_*` from the autogenerated
contract — never raw integers.

### Backed enums for column-typed fields

Where a `:prefix_*` column carries a small fixed set of values
(letter codes for log types, integer kinds for ban types, the
varchar removal-type tag, the integer bitmask for web permissions),
use a backed enum to wrap the on-disk type:

- `LogType: string` — letter codes (`'m'`, `'w'`, `'e'`) — matches
`:prefix_log.type enum('m','w','e')`.
- `LogSearchType: string` — `advType=` query param tags
(`'admin'` / `'message'` / `'date'` / `'type'`); the enum carries
the WHERE-fragment builder so `Log::getAll()` / `Log::getCount()`
no longer carry parallel `switch ($type)` blocks.
- `BanType: int` — wraps `:prefix_bans.type tinyint`
(`Steam=0`, `Ip=1`).
- `BanRemoval: string` — wraps the ban / comm removal-type column
(`:prefix_bans.RemoveType varchar(3)` / `:prefix_comms.RemoveType
varchar(3)`: `Deleted='D'`, `Unbanned='U'`, `Expired='E'`).
String-backed because the column is `varchar(3)` on disk —
the enum's job is to mirror the on-disk type.
- `WebPermission: int` — wraps the integer bitmask flags from
`web/configs/permissions/web.json` (mirrors `init.php`'s `define`d
`ADMIN_*` constants — both shapes coexist for backward
compatibility).

The on-disk schema is unchanged; the enum is purely a PHP-side
wrapper. At every SQL bind site, always pass `$enum->value` (not the
enum case itself) so the dba plugin and the underlying PDO see the
column-typed primitive. `enum('m','w','e')` / `varchar(3)` columns
get `string` values; `int` columns get `int` values; this is the
contract.

For variadic permission masks,
`WebPermission::mask(WebPermission::Owner, WebPermission::AddBan)`
returns the integer bitmask. The `HasAccess()` signature is
`WebPermission|int|string $flags` to keep both the modern
enum-passing shape and the legacy
`HasAccess(ADMIN_OWNER | ADMIN_ADD_BAN)` shape working. Single-flag
checks read naturally as `HasAccess(WebPermission::Owner)`;
multi-flag checks go through `WebPermission::mask(…)`. The
`int|` and `string|` arms keep working for dynamic-value sites
(`HasAccess($mask)` where `$mask` was assembled at runtime, or
`HasAccess(SM_RCON . SM_ROOT)` for SourceMod char flags) and for
the `ALL_WEB` rolled-up bitmask which deliberately stays out of the
enum.

`LogType` / `LogSearchType` / `BanType` / `BanRemoval` /
`WebPermission` all live in the global namespace under
`web/includes/` (not `Sbpp\…`). They're loaded by `require_once` in
`init.php` + `tests/bootstrap.php` so they're available before
`Log.php` / `CUserManager.php` reference them. Issue #1290 phase D.

### Frontend (`web/scripts/`)

- Vanilla JS only — `// @ts-check` + JSDoc on every file.
Expand Down Expand Up @@ -763,6 +815,44 @@ audit (#1207) locked in. New CTAs:
class in `web/includes/` is `View` (subclassed by every concrete
view DTO). Marking final unblocks the JIT's monomorphic-call
optimization. Issue #1290 phase J.
- `Log::add('m', …)` / `Log::add('w', …)` / `Log::add('e', …)` magic
letter codes for the log type column → use
`Log::add(LogType::Message, …)` /
`Log::add(LogType::Warning, …)` /
`Log::add(LogType::Error, …)`. The letter still hits the disk
(the column stays `enum('m','w','e')`); the enum is a PHP-side wrapper so
the call site reads as intent ("this is a message log entry")
rather than as a magic char. Same shape for `BanType`,
`BanRemoval`, `WebPermission`. The static gate is the
`LogType $type` typed parameter on `Log::add()`; the runtime gate
is PHP itself rejecting a string at the call site. Issue #1290
phase D.
- `HasAccess(ADMIN_OWNER | ADMIN_ADD_BAN)` integer-bitmask call
shape → `HasAccess(WebPermission::mask(WebPermission::Owner,
WebPermission::AddBan))`. Single-flag checks read as
`HasAccess(WebPermission::Owner)`. Both compile to the same
integer bitmask under the hood; the enum form documents intent at
the call site. The `ADMIN_*` `define`d constants from `init.php`
are preserved for procedural-code back-compat — both shapes
work. Dynamic-value sites (`HasAccess($mask)` where `$mask` was
assembled at runtime, or `HasAccess(SM_RCON . SM_ROOT)` for
SourceMod char flags, or `HasAccess(ALL_WEB)` for the rolled-up
is-any-web-admin gate) deliberately keep the legacy form because
the enum doesn't fit. Issue #1290 phase D.4.
- `RemoveType = 'U'` / `'D'` / `'E'` SQL string literals for ban /
comm removal types in PHP-driven write paths → bind
`BanRemoval::Unbanned->value` / `BanRemoval::Deleted->value` /
`BanRemoval::Expired->value` (or pass the case directly through
`match()` for read-side branching). Inline literals in pure-SQL
predicates (e.g. `WHERE RemoveType = 'E'` inside cron-style
`PruneBans`/`PruneComms` UPDATEs that don't take a PHP value) are
fine — the enum is for "PHP value crosses the wire" sites, not for
static SQL. Issue #1290 phase D.3.
- `$row['type'] == 0` / `== 1` for ban-type branching →
`BanType::tryFrom((int) $row['type']) === BanType::Steam` (or
`=== BanType::Ip`). Same justification as `BanRemoval` above:
PHP-side branches go through the enum; bare SQL predicates can
keep `WHERE type = '0'`. Issue #1290 phase D.2.
- `xajax` / `sb-callback.php` → use the JSON API.
- ADOdb → use `Database` (PDO).
- MooTools / React / a runtime bundler → vanilla JS in `web/scripts/`.
Expand Down Expand Up @@ -997,6 +1087,7 @@ audit (#1207) locked in. New CTAs:
| Auth / JWT cookie | `web/includes/auth/` |
| CSRF | `web/includes/security/CSRF.php` |
| Schema | `web/install/includes/sql/struc.sql` |
| Wrap a `:prefix_*` column with a backed PHP enum (log letter codes, ban types, removal-type tags, web permissions) | `web/includes/LogType.php` / `LogSearchType.php` / `BanType.php` / `BanRemoval.php` / `WebPermission.php` (global namespace; loaded by `init.php` + `tests/bootstrap.php`). Pass `$enum->value` at every SQL bind site so the dba plugin sees the column-typed primitive; use `WebPermission::mask(…)` to assemble multi-flag bitmasks for `HasAccess()`. Issue #1290 phase D. |
| Seed `sb_settings` rows for fresh installs | `web/install/includes/sql/data.sql` |
| Add a one-off DB upgrade for existing installs | `web/updater/data/<N>.php` + `web/updater/store.json` |
| Test fixtures | `web/tests/Fixture.php`, `web/tests/ApiTestCase.php` |
Expand Down
38 changes: 38 additions & 0 deletions ARCHITECTURE.md
Original file line number Diff line number Diff line change
Expand Up @@ -613,6 +613,44 @@ Reseeded in tests via `web/tests/Fixture.php`, which renders `struc.sql`
+ `data.sql` against a dedicated `sourcebans_test` database before every
test method.

### Column-typed PHP enums (issue #1290 phase D)

Five `:prefix_*` columns carry a small fixed set of values; PHP wraps
each with a backed enum so call sites read as intent rather than as
magic primitives:

| Enum | On-disk column | Backing | Cases |
| ------------------- | --------------------------------------------------------------- | ------- | ------------------------------------------------------------------ |
| `LogType` | `:prefix_log.type enum('m','w','e')` | string | `Message='m'` / `Warning='w'` / `Error='e'` |
| `LogSearchType` | the audit-log `?advType=` query param tag (no DB column) | string | `Admin` / `Message` / `Date` / `Type` (also carries WHERE-builder) |
| `BanType` | `:prefix_bans.type tinyint` | int | `Steam=0` / `Ip=1` |
| `BanRemoval` | `:prefix_bans.RemoveType varchar(3)` / `:prefix_comms.RemoveType varchar(3)` | string | `Deleted='D'` / `Unbanned='U'` / `Expired='E'` |
| `WebPermission` | `:prefix_admins.extraflags int` / `:prefix_groups.flags int` (bitmask) | int | one case per `web/configs/permissions/web.json` flag (`Owner=16777216`, …) |

The on-disk schema stays as `enum('m','w','e')` / `varchar(3)` /
`int` / `tinyint`. The enum is the PHP-side typed wrapper. At every SQL bind
site, pass `$enum->value` (the column-typed primitive); the case
itself is for in-PHP type-safety only. `phpstan/phpstan-dba` types the
raw SQL against the live MariaDB schema, so a wrong-typed bind (e.g.
binding a `BanType` enum case directly instead of `->value`) fails
the gate.

Files live in the global namespace under `web/includes/`
(`LogType.php`, `LogSearchType.php`, `BanType.php`, `BanRemoval.php`,
`WebPermission.php`); they're loaded by `require_once` in `init.php`
+ `tests/bootstrap.php` ahead of `Log.php` / `CUserManager.php` so
the typed parameters compile.

`WebPermission` adds a `mask(WebPermission ...$flags): int` helper
for assembling multi-flag bitmasks; `HasAccess()` accepts
`WebPermission|int|string` so the modern enum form
(`HasAccess(WebPermission::Owner)`) and the legacy procedural form
(`HasAccess(ADMIN_OWNER | ADMIN_ADD_BAN)`) coexist. The `ADMIN_*`
`define`d constants in `init.php` stay as the back-compat surface
for procedural code that pre-dates the enum; the JS-side
`Perms.ADMIN_*` contract in `web/scripts/api-contract.js` is
unchanged.

### Updater (`web/updater/`)

The updater is how *existing* installs catch up to schema or data changes
Expand Down
18 changes: 9 additions & 9 deletions web/api/handlers/account.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ function api_account_check_password(array $params): array
// admin's password through the API.
if ($aid !== $userbank->GetAid()) {
$affected = $userbank->GetProperty('user', $aid);
Log::add('w', 'Hacking Attempt',
Log::add(LogType::Warning, 'Hacking Attempt',
$userbank->GetProperty('user') . " tried to check {$affected}'s password, but doesn't have access.");
return Api::redirect('index.php?p=login&m=no_access');
}
Expand All @@ -43,7 +43,7 @@ function api_account_check_srv_password(array $params): array

if (!$userbank->is_logged_in() || $aid !== $userbank->GetAid()) {
$affected = $userbank->GetProperty('user', $aid);
Log::add('w', 'Hacking Attempt',
Log::add(LogType::Warning, 'Hacking Attempt',
$userbank->GetProperty('user') . " tried to check {$affected}'s server password, but doesn't have access.");
return Api::redirect('index.php?p=login&m=no_access');
}
Expand All @@ -63,8 +63,8 @@ function api_account_change_password(array $params): array
$newPass = (string)($params['new_password'] ?? '');
$oldPass = (string)($params['old_password'] ?? '');

if ($aid !== $userbank->GetAid() && !$userbank->HasAccess(ADMIN_OWNER | ADMIN_EDIT_ADMINS)) {
Log::add('w', 'Hacking Attempt',
if ($aid !== $userbank->GetAid() && !$userbank->HasAccess(WebPermission::mask(WebPermission::Owner, WebPermission::EditAdmins))) {
Log::add(LogType::Warning, 'Hacking Attempt',
($_SERVER['REMOTE_ADDR'] ?? 'unknown') . " tried to change a password without permission.");
return Api::redirect('index.php?p=login&m=no_access');
}
Expand All @@ -82,7 +82,7 @@ function api_account_change_password(array $params): array
$GLOBALS['PDO']->bind(':aid', $aid);
$admin = $GLOBALS['PDO']->single();

Log::add('m', 'Password Changed', "Password changed for admin ({$admin['user']})");
Log::add(LogType::Message, 'Password Changed', "Password changed for admin ({$admin['user']})");
Auth::logout();

return Api::redirect('index.php?p=login');
Expand All @@ -96,7 +96,7 @@ function api_account_change_srv_password(array $params): array

if (!$userbank->is_logged_in() || $aid !== $userbank->GetAid()) {
$affected = $userbank->GetProperty('user', $aid);
Log::add('w', 'Hacking Attempt',
Log::add(LogType::Warning, 'Hacking Attempt',
"$username tried to change {$affected}'s server password, but doesn't have access.");
return Api::redirect('index.php?p=login&m=no_access');
}
Expand All @@ -112,7 +112,7 @@ function api_account_change_srv_password(array $params): array
$GLOBALS['PDO']->execute();
}

Log::add('m', 'Srv Password Changed', "Password changed for admin ($aid)");
Log::add(LogType::Message, 'Srv Password Changed', "Password changed for admin ($aid)");

return [
'message' => [
Expand All @@ -132,7 +132,7 @@ function api_account_change_email(array $params): array
$password = (string)($params['password'] ?? '');

if (!$userbank->is_logged_in() || $aid !== $userbank->GetAid()) {
Log::add('w', 'Hacking Attempt',
Log::add(LogType::Warning, 'Hacking Attempt',
"$username tried to change " . $userbank->GetProperty('user', $aid) . "'s email, but doesn't have access.");
return Api::redirect('index.php?p=login&m=no_access');
}
Expand All @@ -149,7 +149,7 @@ function api_account_change_email(array $params): array
$GLOBALS['PDO']->bind(':aid', $aid);
$GLOBALS['PDO']->execute();

Log::add('m', 'E-mail Changed', "E-mail changed for admin ($aid).");
Log::add(LogType::Message, 'E-mail Changed', "E-mail changed for admin ($aid).");

return [
'message' => [
Expand Down
12 changes: 6 additions & 6 deletions web/api/handlers/admins.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ function api_admins_remove(array $params): array
throw new ApiError('delete_failed', 'There was an error removing the admin from the database, please check the logs');
}

Log::add('m', 'Admin Deleted', "Admin ({$admin['user']}) has been deleted.");
Log::add(LogType::Message, 'Admin Deleted', "Admin ({$admin['user']}) has been deleted.");

return [
'remove' => "aid_$aid",
Expand Down Expand Up @@ -242,7 +242,7 @@ function api_admins_add(array $params): array
}
}

Log::add('m', 'Admin added', "Admin ($name) has been added.");
Log::add(LogType::Message, 'Admin added', "Admin ($name) has been added.");

return [
'aid' => $aid,
Expand All @@ -267,8 +267,8 @@ function api_admins_edit_perms(array $params): array
if ($aid === 0) {
throw new ApiError('bad_request', 'aid is required');
}
if (!$userbank->HasAccess(ADMIN_OWNER) && ($webFlags & ADMIN_OWNER)) {
Log::add('w', 'Hacking Attempt',
if (!$userbank->HasAccess(WebPermission::Owner) && ($webFlags & ADMIN_OWNER)) {
Log::add(LogType::Warning, 'Hacking Attempt',
$userbank->GetProperty('user') . ' tried to gain OWNER admin permissions, but doesnt have access.');
return Api::redirect('index.php?p=login&m=no_access');
}
Expand Down Expand Up @@ -310,7 +310,7 @@ function api_admins_edit_perms(array $params): array
}

$admin = $GLOBALS['PDO']->query("SELECT user FROM `:prefix_admins` WHERE aid = ?")->single([$aid]);
Log::add('m', 'Permissions Changed', "Permissions have been changed for ({$admin['user']})");
Log::add(LogType::Message, 'Permissions Changed', "Permissions have been changed for ({$admin['user']})");

return [
'reload' => true,
Expand Down Expand Up @@ -358,7 +358,7 @@ function api_admins_update_perms(array $params): array
return [
'id' => $id,
'permissions' => $permissions,
'is_owner' => $userbank->HasAccess(ADMIN_OWNER),
'is_owner' => $userbank->HasAccess(WebPermission::Owner),
];
}

Expand Down
Loading
Loading