Skip to content

C
Clubphp
Commit d39d9293 authored by Mahmoud Aglan's avatar Mahmoud Aglan
Browse files
Options

fix(auth): resolve route/menu/role permission drift causing phantom 403s 

Users saw sidebar links that returned 403. Root cause was drift between four
independently-authored declaration sets that nothing reconciles: the permission
catalogue (bootstrap.php), the route gate (Routes.php), the menu gate
(MenuRegistry) and the role grants (seeds).

Route shadowing (Router::dispatch is first-match-wins over a sorted module glob):
- GET /reports was declared by both Members and Reports; Members won and enforced
  member.reports while the sidebar gated on report.view_membership. Members'
  report routes moved to /members/reports/*.
- GET /sports-dashboard[/export] was declared by three modules, so the dashboard
  index and its drill-downs were served by different modules. Disciplines ->
  /disciplines/dashboard, PlaygroundAdmin -> /playgrounds/dashboard[/export];
  /sports-dashboard is now wholly owned by SportsDashboard.
- Members/Routes.php used unconstrained {id} in 15 routes, so /members/<anything>
  was swallowed by MemberController@show. Constrained to {id:\d+}, matching every
  other module. All 25 affected links updated.

Gate alignment:
- Six menu entries gated on a different permission than the route they link to
  (/members/search, /sports, /carnets, /rentals/entities,
  /notifications/templates, /reports).

Authorization bypasses:
- RetroactiveWizardController hardcoded a role_code = 'super_admin' query,
  throwing "هذه الأداة متاحة فقط لمدير النظام". Replaced with a registered
  member.retroactive permission enforced by the route and grantable via the
  Roles UI.
- report_definitions.required_permission was stored and displayed but never
  checked, so report.view_membership was enough to open ANY report by code,
  including financial ones. Now enforced on view/export/print; the listing
  filters to what the viewer can actually run.

Role grants (Phase_105_001, idempotent):
- Closes the reported gaps for report_viewer, general_manager, receptionist,
  sports_officer, academy_manager and membership_director; grants the sports
  report keys to board_member/auditor so enforcing the per-report permission
  does not silently remove reports; revokes member.view/member.search from
  facilities_manager, who keeps bookings and reservations.

Data correctness:
- SaFinanceReportService read base_price from sa_pricing_rules, a facility
  booking table with neither that column nor activity_type, and derived revenue
  as headcount x a rate-card price. Now sums actual sa_registrations
  .registration_fee, matching how subscription and booking revenue are computed.

Regression guard:
- php cli.php permissions:audit reconciles all four declaration sets, reproduces
  the router's load order, and exits non-zero on drift. Run it after touching any
  Routes.php, menu block or role seed.

Docs: new docs/architecture-maps/Authorization.md; cross-module authorization
section added to DEPENDENCY-GRAPH.md.

Note: the live DB was unreachable from the dev environment, so role grants were
verified by replaying the seeds and schema came from migrations, not the live DB.
PHPUnit is not installed locally; all changed files lint clean.
Co-Authored-By: 's avatarClaude Opus 5 <noreply@anthropic.com>
parent e139a8a2
Hide whitespace changes
Inline Side-by-side
Showing with 855 additions and 89 deletions
app/Modules/AccessMatrix/Services/PermissionDiscoveryService.php
View file @ d39d9293
......@@ -4,6 +4,7 @@ declare(strict_types=1);
namespace App\Modules\AccessMatrix\Services;
use App\Core\App;
use App\Core\Registries\MenuRegistry;
use App\Core\Registries\PermissionRegistry;
final class PermissionDiscoveryService
......@@ -142,4 +143,232 @@ final class PermissionDiscoveryService
return array_unique($permissions);
}
// ─── Declaration-drift audits ───────────────────────────────────────────────
// The permission catalogue, the route gates, the menu gates and the role
// grants are four independently-authored declaration sets. Nothing in the
// framework reconciles them, so they drift. These methods reconcile them.
// See docs/architecture-maps/Authorization.md.
/**
* Every route tuple, in the exact order Router::dispatch() sees them.
* Router returns on FIRST match, and App::boot() sort()s the module glob,
* so registration order is alphabetical by module directory and decides
* which of two identical paths actually serves the request.
*/
public static function getAllRoutes(): array
{
$modulesPath = App::getInstance()->basePath() . '/app/Modules';
$routeFiles = glob($modulesPath . '/*/Routes.php');
sort($routeFiles);
$all = [];
foreach ($routeFiles as $routeFile) {
$routes = require $routeFile;
if (!is_array($routes)) {
continue;
}
$module = basename(dirname($routeFile));
foreach ($routes as $route) {
if (!is_array($route) || count($route) < 3) {
continue;
}
$all[] = [
'module' => $module,
'method' => strtoupper((string) $route[0]),
'path' => (string) $route[1],
'handler' => (string) $route[2],
'middleware' => $route[3] ?? [],
'permission' => $route[4] ?? null,
];
}
}
return $all;
}
/**
* Resolve a URL the way Router::dispatch() would: first registered match wins.
* Mirrors Router::matchPath(), including {name:constraint} placeholders.
*/
public static function resolveRoute(string $url, string $method = 'GET', ?array $routes = null): ?array
{
$routes ??= self::getAllRoutes();
$url = '/' . trim($url, '/');
foreach ($routes as $route) {
if ($route['method'] !== strtoupper($method)) {
continue;
}
$path = '/' . trim($route['path'], '/');
if ($path === $url) {
return $route;
}
$pattern = preg_replace_callback('/\{(\w+)(?::([^}]+))?\}/', function ($m) {
return '(?P<' . $m[1] . '>' . ($m[2] ?? '[^/]+') . ')';
}, $path);
if (preg_match('#^' . $pattern . '$#', $url)) {
return $route;
}
}
return null;
}
/**
* Paths declared by more than one module. The alphabetically-first module
* wins and silently shadows the others — including their permission gate.
*/
public static function findDuplicateRoutes(): array
{
$seen = [];
foreach (self::getAllRoutes() as $route) {
$seen[$route['method'] . ' ' . $route['path']][] = $route;
}
$duplicates = [];
foreach ($seen as $key => $group) {
if (count($group) < 2) {
continue;
}
$permissions = array_unique(array_map(fn($r) => (string) $r['permission'], $group));
$duplicates[] = [
'route' => $key,
'winner' => $group[0],
'shadowed' => array_slice($group, 1),
'conflicting' => count($permissions) > 1,
];
}
return $duplicates;
}
/**
* Sidebar entries whose permission differs from the permission actually
* enforced on the route they link to. This is what users experience as
* "the page is in my menu but returns 403".
*/
public static function findMenuRouteMismatches(): array
{
$routes = self::getAllRoutes();
$mismatches = [];
$check = function (string $label, ?string $menuPerm, ?string $url) use ($routes, &$mismatches): void {
if ($url === null || $url === '' || $url === '#') {
return;
}
$route = self::resolveRoute($url, 'GET', $routes);
if ($route === null) {
$mismatches[] = [
'kind' => 'no_route',
'label' => $label,
'url' => $url,
'menu' => $menuPerm,
'route' => null,
];
return;
}
if ($menuPerm === $route['permission']) {
return;
}
$mismatches[] = [
'kind' => $menuPerm === null ? 'menu_ungated' : 'mismatch',
'label' => $label,
'url' => $url,
'menu' => $menuPerm,
'route' => $route['permission'],
'module' => $route['module'],
];
};
foreach (MenuRegistry::getAll() as $item) {
$check((string) ($item['label_ar'] ?? ''), $item['permission'] ?? null, $item['route'] ?? null);
foreach ($item['children'] ?? [] as $child) {
$check((string) ($child['label_ar'] ?? ''), $child['permission'] ?? null, $child['route'] ?? null);
}
}
return $mismatches;
}
/**
* App::db() is declared `: Database` but its backing property is nullable,
* so it throws a TypeError rather than returning null when the DB was never
* initialised (notably in CLI context). Probe it safely.
*/
private static function dbOrNull(): ?\App\Core\Database
{
try {
return App::getInstance()->db();
} catch (\Throwable $e) {
return null;
}
}
/**
* Permissions a route enforces that no role holds — except super_admin,
* whose '*' satisfies everything. These pages are super-admin-only by
* accident rather than by decision.
*
* Requires a database connection; returns null when unavailable.
*/
public static function findUngrantedRoutePermissions(): ?array
{
$db = self::dbOrNull();
if ($db === null) {
return null;
}
$granted = [];
foreach ($db->select(
"SELECT DISTINCT rp.permission_key
FROM role_permissions rp
JOIN roles r ON r.id = rp.role_id AND r.is_active = 1
WHERE rp.permission_key <> '*'"
) as $row) {
$granted[$row['permission_key']] = true;
}
$ungranted = [];
foreach (self::getAllRoutes() as $route) {
$permission = $route['permission'];
if ($permission && !isset($granted[$permission]) && !isset($ungranted[$permission])) {
$ungranted[$permission] = $route['module'];
}
}
ksort($ungranted);
return $ungranted;
}
/**
* Keys granted in role_permissions that no module ever registered. They can
* never be satisfied by a route check, so they are dead grants.
*
* Requires a database connection; returns null when unavailable.
*/
public static function findPhantomGrants(): ?array
{
$db = self::dbOrNull();
if ($db === null) {
return null;
}
$registered = PermissionRegistry::getAll();
$phantom = [];
foreach ($db->select(
"SELECT DISTINCT rp.permission_key, r.role_code
FROM role_permissions rp
JOIN roles r ON r.id = rp.role_id
WHERE rp.permission_key <> '*'
ORDER BY rp.permission_key"
) as $row) {
if (!isset($registered[$row['permission_key']])) {
$phantom[$row['permission_key']][] = $row['role_code'];
}
}
return $phantom;
}
}
app/Modules/Dashboard/Config/widgets.php
View file @ d39d9293
......@@ -764,7 +764,7 @@ return [
'section' => 'membership',
'icon' => 'baby',
'color' => 'primary',
'drill_link' => '/reports/children-aging',
'drill_link' => '/members/reports/children-aging',
'sql' => 'SELECT COUNT(*) AS total, SUM(CASE WHEN date_of_birth <= DATE_SUB(CURDATE(), INTERVAL 25 YEAR) THEN 1 ELSE 0 END) AS already_over_25, SUM(CASE WHEN date_of_birth > DATE_SUB(CURDATE(), INTERVAL 25 YEAR) THEN 1 ELSE 0 END) AS within_12_months FROM children WHERE is_archived = 0 AND status = \'active\' AND gender = \'male\' AND classification <> \'separated\' AND date_of_birth <= DATE_SUB(CURDATE(), INTERVAL 24 YEAR)',
'params' => static fn(array $ctx): array => [],
],
......@@ -775,7 +775,7 @@ return [
'section' => 'membership',
'icon' => 'baby',
'color' => 'primary',
'drill_link' => '/reports/children-aging',
'drill_link' => '/members/reports/children-aging',
'sql' => 'SELECT c.id AS child_id, c.full_name_ar AS child_name, m.id AS member_id, m.full_name_ar AS member_name, m.membership_number, c.date_of_birth, DATE_ADD(c.date_of_birth, INTERVAL 25 YEAR) AS turns_25_on, DATEDIFF(DATE_ADD(c.date_of_birth, INTERVAL 25 YEAR), CURDATE()) AS days_left, CONCAT(\'/members/\', m.id, \'/children/\', c.id) AS url FROM children c JOIN members m ON m.id = c.member_id WHERE c.is_archived = 0 AND c.status = \'active\' AND c.gender = \'male\' AND c.classification <> \'separated\' AND c.date_of_birth <= DATE_SUB(CURDATE(), INTERVAL 24 YEAR) ORDER BY days_left ASC LIMIT 10',
'params' => static fn(array $ctx): array => [],
],
......@@ -786,7 +786,7 @@ return [
'section' => 'revenue',
'icon' => 'hand-coins',
'color' => 'success',
'drill_link' => '/reports/unpaid-debts',
'drill_link' => '/members/reports/unpaid-debts',
'sql' => 'SELECT b.name_ar AS branch, COUNT(*) AS rows_billed, COALESCE(SUM(s.total_amount), 0) AS billed, COALESCE(SUM(s.paid_amount), 0) AS collected, COALESCE(SUM(CASE WHEN s.status IN (\'pending\',\'overdue\') THEN GREATEST(s.total_amount - s.paid_amount + s.fine_amount, 0) ELSE 0 END), 0) AS outstanding FROM subscriptions s JOIN members m ON m.id = s.member_id AND m.is_archived = 0 JOIN branches b ON b.id = m.branch_id WHERE s.financial_year = ? GROUP BY b.id, b.name_ar ORDER BY outstanding DESC',
'params' => static fn(array $ctx): array => [(date('n') >= 7 ? date('Y') . '/' . (date('Y') + 1) : (date('Y') - 1) . '/' . date('Y'))],
],
......
app/Modules/Disciplines/Routes.php
View file @ d39d9293
......@@ -2,7 +2,7 @@
declare(strict_types=1);
return [
['GET', '/sports-dashboard', 'Disciplines\Controllers\SportsDashboardController@index', ['auth'], 'discipline.view'],
['GET', '/disciplines/dashboard', 'Disciplines\Controllers\SportsDashboardController@index', ['auth'], 'discipline.view'],
['GET', '/disciplines', 'Disciplines\Controllers\DisciplineController@index', ['auth'], 'discipline.view'],
['GET', '/disciplines/create', 'Disciplines\Controllers\DisciplineController@create', ['auth'], 'discipline.manage'],
['POST', '/disciplines', 'Disciplines\Controllers\DisciplineController@store', ['auth', 'csrf'], 'discipline.manage'],
......
app/Modules/Members/Controllers/RetroactiveWizardController.php
View file @ d39d9293
......@@ -11,24 +11,8 @@ use App\Modules\Members\Services\RetroactiveMembershipService;
class RetroactiveWizardController extends Controller
{
private static function isSuperAdmin(): bool
{
$employee = App::getInstance()->currentEmployee();
if (!$employee) return false;
$db = App::getInstance()->db();
$row = $db->selectOne(
"SELECT 1 FROM employee_roles er JOIN roles r ON r.id = er.role_id WHERE er.employee_id = ? AND r.role_code = 'super_admin' AND er.is_active = 1 LIMIT 1",
[(int) $employee->id]
);
return $row !== null;
}
public function index(Request $request): Response
{
if (!self::isSuperAdmin()) {
throw new \RuntimeException('هذه الأداة متاحة فقط لمدير النظام', 403);
}
$db = App::getInstance()->db();
$branches = $db->select("SELECT id, name_ar FROM branches WHERE is_active = 1 ORDER BY name_ar");
$qualifications = $db->select("SELECT id, name_ar FROM qualifications WHERE is_active = 1 ORDER BY sort_order");
......@@ -41,10 +25,6 @@ class RetroactiveWizardController extends Controller
public function store(Request $request): Response
{
if (!self::isSuperAdmin()) {
throw new \RuntimeException('هذه الأداة متاحة فقط لمدير النظام', 403);
}
$data = $this->collectWizardData($request);
$result = RetroactiveMembershipService::createRetroactiveMember($data);
......@@ -63,10 +43,6 @@ class RetroactiveWizardController extends Controller
public function preview(Request $request): Response
{
if (!self::isSuperAdmin()) {
return $this->json(['error' => 'غير مصرح'], 403);
}
$data = $this->collectWizardData($request);
$preview = [
......
app/Modules/Members/Routes.php
View file @ d39d9293
......@@ -7,36 +7,36 @@ return [
['GET', '/members', 'Members\Controllers\MemberController@index', ['auth'], 'member.view'],
['GET', '/members/create', 'Members\Controllers\MemberController@create', ['auth'], 'member.create'],
['POST', '/members', 'Members\Controllers\MemberController@store', ['auth', 'csrf'], 'member.create'],
['GET', '/members/search', 'Members\Controllers\MemberController@search', ['auth'], 'member.view'],
['GET', '/members/search', 'Members\Controllers\MemberController@search', ['auth'], 'member.search'],
// Retroactive Wizard (super admin only)
['GET', '/members/retroactive-wizard', 'Members\Controllers\RetroactiveWizardController@index', ['auth'], 'member.create'],
['POST', '/members/retroactive-wizard', 'Members\Controllers\RetroactiveWizardController@store', ['auth', 'csrf'], 'member.create'],
['POST', '/members/retroactive-wizard/preview', 'Members\Controllers\RetroactiveWizardController@preview', ['auth', 'csrf'], 'member.create'],
['GET', '/members/{id}', 'Members\Controllers\MemberController@show', ['auth'], 'member.view'],
['GET', '/members/{id}/edit', 'Members\Controllers\MemberController@edit', ['auth'], 'member.edit'],
['POST', '/members/{id}', 'Members\Controllers\MemberController@update', ['auth', 'csrf'], 'member.edit'],
['POST', '/members/{id}/photo', 'Members\Controllers\MemberController@uploadPhoto', ['auth', 'csrf'], 'member.edit'],
['POST', '/members/{id}/photo/delete', 'Members\Controllers\MemberController@deletePhoto', ['auth', 'csrf'], 'member.edit'],
['POST', '/members/{id}/status', 'Members\Controllers\MemberController@changeStatus', ['auth', 'csrf'], 'member.change_status'],
['POST', '/members/{id}/pay-form-fee', 'Members\Controllers\MemberController@payFormFee', ['auth', 'csrf'], 'member.pay_form_fee'],
['POST', '/members/{id}/pay-membership', 'Members\Controllers\MemberController@payMembership',['auth', 'csrf'], 'member.pay_membership'],
['POST', '/members/{id}/apply-discount','Members\Controllers\MemberController@applyDiscount', ['auth', 'csrf'], 'member.edit'],
['POST', '/members/{id}/pay-addition', 'Members\Controllers\MemberController@payAdditionFee', ['auth', 'csrf'], 'member.pay_membership'],
['GET', '/members/{id}/fill-form', 'Members\Controllers\MemberController@fillForm', ['auth'], 'member.fill_form'],
['POST', '/members/{id}/fill-form', 'Members\Controllers\MemberController@saveFillForm', ['auth', 'csrf'], 'member.fill_form'],
['GET', '/members/{id}/changelog', 'Members\Controllers\MemberController@changelog', ['auth'], 'member.view'],
['GET', '/members/{id}/insurance-record', 'Members\Controllers\MemberController@insuranceRecord', ['auth'], 'member.view'],
['GET', '/members/retroactive-wizard', 'Members\Controllers\RetroactiveWizardController@index', ['auth'], 'member.retroactive'],
['POST', '/members/retroactive-wizard', 'Members\Controllers\RetroactiveWizardController@store', ['auth', 'csrf'], 'member.retroactive'],
['POST', '/members/retroactive-wizard/preview', 'Members\Controllers\RetroactiveWizardController@preview', ['auth', 'csrf'], 'member.retroactive'],
['GET', '/members/{id:\d+}', 'Members\Controllers\MemberController@show', ['auth'], 'member.view'],
['GET', '/members/{id:\d+}/edit', 'Members\Controllers\MemberController@edit', ['auth'], 'member.edit'],
['POST', '/members/{id:\d+}', 'Members\Controllers\MemberController@update', ['auth', 'csrf'], 'member.edit'],
['POST', '/members/{id:\d+}/photo', 'Members\Controllers\MemberController@uploadPhoto', ['auth', 'csrf'], 'member.edit'],
['POST', '/members/{id:\d+}/photo/delete', 'Members\Controllers\MemberController@deletePhoto', ['auth', 'csrf'], 'member.edit'],
['POST', '/members/{id:\d+}/status', 'Members\Controllers\MemberController@changeStatus', ['auth', 'csrf'], 'member.change_status'],
['POST', '/members/{id:\d+}/pay-form-fee', 'Members\Controllers\MemberController@payFormFee', ['auth', 'csrf'], 'member.pay_form_fee'],
['POST', '/members/{id:\d+}/pay-membership', 'Members\Controllers\MemberController@payMembership',['auth', 'csrf'], 'member.pay_membership'],
['POST', '/members/{id:\d+}/apply-discount','Members\Controllers\MemberController@applyDiscount', ['auth', 'csrf'], 'member.edit'],
['POST', '/members/{id:\d+}/pay-addition', 'Members\Controllers\MemberController@payAdditionFee', ['auth', 'csrf'], 'member.pay_membership'],
['GET', '/members/{id:\d+}/fill-form', 'Members\Controllers\MemberController@fillForm', ['auth'], 'member.fill_form'],
['POST', '/members/{id:\d+}/fill-form', 'Members\Controllers\MemberController@saveFillForm', ['auth', 'csrf'], 'member.fill_form'],
['GET', '/members/{id:\d+}/changelog', 'Members\Controllers\MemberController@changelog', ['auth'], 'member.view'],
['GET', '/members/{id:\d+}/insurance-record', 'Members\Controllers\MemberController@insuranceRecord', ['auth'], 'member.view'],
['POST', '/api/members/parse-nid', 'Members\Controllers\MemberApiController@parseNid', ['auth'], 'member.create'],
['POST', '/api/members/check-nid', 'Members\Controllers\MemberApiController@checkNid', ['auth'], 'member.view'],
['GET', '/api/members/search', 'Members\Controllers\MemberApiController@searchGet', ['auth'], 'member.view'],
['POST', '/api/members/search', 'Members\Controllers\MemberApiController@search', ['auth'], 'member.view'],
['GET', '/api/members/{id}/debts', 'Members\Controllers\MemberApiController@debts', ['auth'], 'member.view'],
['GET', '/api/members/{id:\d+}/debts', 'Members\Controllers\MemberApiController@debts', ['auth'], 'member.view'],
// Reports
['GET', '/reports', 'Members\Controllers\ReportController@index', ['auth'], 'member.reports'],
['GET', '/reports/children-aging', 'Members\Controllers\ReportController@childrenAgingOut', ['auth'], 'member.reports'],
['GET', '/reports/transfers', 'Members\Controllers\ReportController@transfers', ['auth'], 'member.reports'],
['GET', '/reports/waivers', 'Members\Controllers\ReportController@waivers', ['auth'], 'member.reports'],
['GET', '/reports/subscription-status', 'Members\Controllers\ReportController@subscriptionStatus', ['auth'], 'member.reports'],
['GET', '/reports/age-report', 'Members\Controllers\ReportController@ageReport', ['auth'], 'member.reports'],
['GET', '/reports/unpaid-debts', 'Members\Controllers\ReportController@unpaidDebts', ['auth'], 'member.reports'],
['GET', '/members/reports', 'Members\Controllers\ReportController@index', ['auth'], 'member.reports'],
['GET', '/members/reports/children-aging', 'Members\Controllers\ReportController@childrenAgingOut', ['auth'], 'member.reports'],
['GET', '/members/reports/transfers', 'Members\Controllers\ReportController@transfers', ['auth'], 'member.reports'],
['GET', '/members/reports/waivers', 'Members\Controllers\ReportController@waivers', ['auth'], 'member.reports'],
['GET', '/members/reports/subscription-status', 'Members\Controllers\ReportController@subscriptionStatus', ['auth'], 'member.reports'],
['GET', '/members/reports/age-report', 'Members\Controllers\ReportController@ageReport', ['auth'], 'member.reports'],
['GET', '/members/reports/unpaid-debts', 'Members\Controllers\ReportController@unpaidDebts', ['auth'], 'member.reports'],
];
\ No newline at end of file
app/Modules/Members/Views/reports/age-report.php
View file @ d39d9293
......@@ -3,7 +3,7 @@
<?php $__template->section('content'); ?>
<div class="card" style="margin-bottom:20px;padding:15px;">
<form method="GET" action="/reports/age-report" style="display:flex;gap:10px;flex-wrap:wrap;align-items:end;">
<form method="GET" action="/members/reports/age-report" style="display:flex;gap:10px;flex-wrap:wrap;align-items:end;">
<div>
<label class="form-label" style="font-size:12px;">العمر من</label>
<input type="number" name="min_age" value="<?= (int) $minAge ?>" min="0" max="120" class="form-input" style="width:80px;">
......@@ -13,7 +13,7 @@
<input type="number" name="max_age" value="<?= (int) $maxAge ?>" min="0" max="120" class="form-input" style="width:80px;">
</div>
<button type="submit" class="btn btn-outline">تصفية</button>
<a href="/reports/age-report" class="btn btn-sm btn-outline" style="color:#6B7280;">مسح</a>
<a href="/members/reports/age-report" class="btn btn-sm btn-outline" style="color:#6B7280;">مسح</a>
</form>
</div>
......
app/Modules/Members/Views/reports/children-aging.php
View file @ d39d9293
......@@ -3,7 +3,7 @@
<?php $__template->section('content'); ?>
<div class="card" style="margin-bottom:20px;padding:15px;">
<form method="GET" action="/reports/children-aging" style="display:flex;gap:10px;flex-wrap:wrap;align-items:end;">
<form method="GET" action="/members/reports/children-aging" style="display:flex;gap:10px;flex-wrap:wrap;align-items:end;">
<div>
<label class="form-label" style="font-size:12px;">العمر من</label>
<input type="number" name="min_age" value="<?= (int) $minAge ?>" min="0" max="99" class="form-input" style="width:80px;">
......@@ -13,7 +13,7 @@
<input type="number" name="max_age" value="<?= (int) $maxAge ?>" min="0" max="99" class="form-input" style="width:80px;">
</div>
<button type="submit" class="btn btn-outline">تصفية</button>
<a href="/reports/children-aging" class="btn btn-sm btn-outline" style="color:#6B7280;">مسح</a>
<a href="/members/reports/children-aging" class="btn btn-sm btn-outline" style="color:#6B7280;">مسح</a>
</form>
</div>
......
app/Modules/Members/Views/reports/index.php
View file @ d39d9293
......@@ -3,27 +3,27 @@
<?php $__template->section('content'); ?>
<div style="display:grid;grid-template-columns:repeat(auto-fill,minmax(280px,1fr));gap:20px;">
<a href="/reports/children-aging" class="card" style="padding:20px;text-decoration:none;color:inherit;">
<a href="/members/reports/children-aging" class="card" style="padding:20px;text-decoration:none;color:inherit;">
<h3 style="margin:0 0 8px;">أبناء يقتربون من السن القانوني</h3>
<p style="color:#6B7280;font-size:14px;margin:0;">الأبناء المقتربون من سن 25 (موعد التحويل/الفصل)</p>
</a>
<a href="/reports/transfers" class="card" style="padding:20px;text-decoration:none;color:inherit;">
<a href="/members/reports/transfers" class="card" style="padding:20px;text-decoration:none;color:inherit;">
<h3 style="margin:0 0 8px;">التحويلات والانفصالات</h3>
<p style="color:#6B7280;font-size:14px;margin:0;">جميع التحويلات المكتملة بحسب الفترة الزمنية</p>
</a>
<a href="/reports/waivers" class="card" style="padding:20px;text-decoration:none;color:inherit;">
<a href="/members/reports/waivers" class="card" style="padding:20px;text-decoration:none;color:inherit;">
<h3 style="margin:0 0 8px;">التنازلات</h3>
<p style="color:#6B7280;font-size:14px;margin:0;">طلبات التنازل المكتملة مع المبالغ</p>
</a>
<a href="/reports/subscription-status" class="card" style="padding:20px;text-decoration:none;color:inherit;">
<a href="/members/reports/subscription-status" class="card" style="padding:20px;text-decoration:none;color:inherit;">
<h3 style="margin:0 0 8px;">حالة الاشتراكات</h3>
<p style="color:#6B7280;font-size:14px;margin:0;">ملخص الاشتراكات السنوية — مدفوع/متأخر/معلق</p>
</a>
<a href="/reports/age-report" class="card" style="padding:20px;text-decoration:none;color:inherit;">
<a href="/members/reports/age-report" class="card" style="padding:20px;text-decoration:none;color:inherit;">
<h3 style="margin:0 0 8px;">تقرير الأعمار</h3>
<p style="color:#6B7280;font-size:14px;margin:0;">توزيع الأعضاء حسب الفئة العمرية</p>
</a>
<a href="/reports/unpaid-debts" class="card" style="padding:20px;text-decoration:none;color:inherit;">
<a href="/members/reports/unpaid-debts" class="card" style="padding:20px;text-decoration:none;color:inherit;">
<h3 style="margin:0 0 8px;">المديونيات المعلقة</h3>
<p style="color:#6B7280;font-size:14px;margin:0;">غرامات وأقساط غير مسددة</p>
</a>
......
app/Modules/Members/Views/reports/subscription-status.php
View file @ d39d9293
......@@ -3,7 +3,7 @@
<?php $__template->section('content'); ?>
<div class="card" style="margin-bottom:20px;padding:15px;">
<form method="GET" action="/reports/subscription-status" style="display:flex;gap:10px;flex-wrap:wrap;align-items:end;">
<form method="GET" action="/members/reports/subscription-status" style="display:flex;gap:10px;flex-wrap:wrap;align-items:end;">
<div>
<label class="form-label" style="font-size:12px;">السنة المالية</label>
<input type="text" name="financial_year" value="<?= e($financialYear) ?>" placeholder="2024/2025" class="form-input" style="width:120px;">
......
app/Modules/Members/Views/reports/transfers.php
View file @ d39d9293
......@@ -3,7 +3,7 @@
<?php $__template->section('content'); ?>
<div class="card" style="margin-bottom:20px;padding:15px;">
<form method="GET" action="/reports/transfers" style="display:flex;gap:10px;flex-wrap:wrap;align-items:end;">
<form method="GET" action="/members/reports/transfers" style="display:flex;gap:10px;flex-wrap:wrap;align-items:end;">
<div>
<label class="form-label" style="font-size:12px;">من تاريخ</label>
<input type="date" name="date_from" value="<?= e($dateFrom) ?>" class="form-input">
......@@ -22,7 +22,7 @@
</select>
</div>
<button type="submit" class="btn btn-outline">تصفية</button>
<a href="/reports/transfers" class="btn btn-sm btn-outline" style="color:#6B7280;">مسح</a>
<a href="/members/reports/transfers" class="btn btn-sm btn-outline" style="color:#6B7280;">مسح</a>
</form>
</div>
......
app/Modules/Members/Views/reports/waivers.php
View file @ d39d9293
......@@ -3,7 +3,7 @@
<?php $__template->section('content'); ?>
<div class="card" style="margin-bottom:20px;padding:15px;">
<form method="GET" action="/reports/waivers" style="display:flex;gap:10px;flex-wrap:wrap;align-items:end;">
<form method="GET" action="/members/reports/waivers" style="display:flex;gap:10px;flex-wrap:wrap;align-items:end;">
<div>
<label class="form-label" style="font-size:12px;">من تاريخ</label>
<input type="date" name="date_from" value="<?= e($dateFrom) ?>" class="form-input">
......@@ -13,7 +13,7 @@
<input type="date" name="date_to" value="<?= e($dateTo) ?>" class="form-input">
</div>
<button type="submit" class="btn btn-outline">تصفية</button>
<a href="/reports/waivers" class="btn btn-sm btn-outline" style="color:#6B7280;">مسح</a>
<a href="/members/reports/waivers" class="btn btn-sm btn-outline" style="color:#6B7280;">مسح</a>
</form>
</div>
......
app/Modules/Members/bootstrap.php
View file @ d39d9293
......@@ -16,12 +16,12 @@ MenuRegistry::register('membership', [
// ── Core Members ────────────────────────────
['label_ar' => 'كل الأعضاء', 'label_en' => 'All Members', 'route' => '/members', 'permission' => 'member.view', 'order' => 1],
['label_ar' => 'عضو جديد', 'label_en' => 'New Member', 'route' => '/members/create', 'permission' => 'member.create', 'order' => 2],
['label_ar' => 'إدخال بأثر رجعي', 'label_en' => 'Retroactive Wizard', 'route' => '/members/retroactive-wizard', 'permission' => 'member.create', 'order' => 3],
['label_ar' => 'إدخال بأثر رجعي', 'label_en' => 'Retroactive Wizard', 'route' => '/members/retroactive-wizard', 'permission' => 'member.retroactive', 'order' => 3],
['label_ar' => 'بحث الأعضاء', 'label_en' => 'Search Members', 'route' => '/members/search', 'permission' => 'member.search', 'order' => 3],
// ── Membership Types ────────────────────────
['label_ar' => 'الأعضاء المؤقتون', 'label_en' => 'Temporary Members', 'route' => '/temporary', 'permission' => 'temp.view', 'order' => 10],
['label_ar' => 'العضوية الموسمية', 'label_en' => 'Seasonal Memberships', 'route' => '/seasonal', 'permission' => 'temp.view', 'order' => 11],
['label_ar' => 'العضوية الرياضية', 'label_en' => 'Sports Membership', 'route' => '/sports', 'permission' => 'temp.view', 'order' => 12],
['label_ar' => 'العضوية الرياضية', 'label_en' => 'Sports Membership', 'route' => '/sports', 'permission' => 'sports.view', 'order' => 12],
['label_ar' => 'العضوية الشرفية', 'label_en' => 'Honorary Membership', 'route' => '/honorary', 'permission' => 'member.view', 'order' => 13],
['label_ar' => 'الأعضاء الأجانب', 'label_en' => 'Foreign Members', 'route' => '/foreign', 'permission' => 'member.view', 'order' => 14],
// ── Subscriptions & Financial ───────────────
......@@ -31,7 +31,7 @@ MenuRegistry::register('membership', [
['label_ar' => 'المخالفات والغرامات', 'label_en' => 'Violations & Fines', 'route' => '/violations', 'permission' => 'fine.view', 'order' => 23],
// ── Procedures ──────────────────────────────
['label_ar' => 'المقابلات', 'label_en' => 'Interviews', 'route' => '/interviews', 'permission' => 'interview.view', 'order' => 30],
['label_ar' => 'الكارنيهات', 'label_en' => 'Carnets', 'route' => '/carnets', 'permission' => 'carnet.view_log', 'order' => 31],
['label_ar' => 'الكارنيهات', 'label_en' => 'Carnets', 'route' => '/carnets', 'permission' => 'carnet.view', 'order' => 31],
['label_ar' => 'المستندات', 'label_en' => 'Documents', 'route' => '/documents', 'permission' => 'document.view', 'order' => 32],
// ── Transfers & Separations ─────────────────
['label_ar' => 'طلبات التحويل', 'label_en' => 'Transfer Requests', 'route' => '/transfers', 'permission' => 'transfer.view', 'order' => 40],
......@@ -41,7 +41,7 @@ MenuRegistry::register('membership', [
// ── Archive ─────────────────────────────────
['label_ar' => 'الأرشيف', 'label_en' => 'Archive', 'route' => '/members/archive', 'permission' => 'member.archive', 'order' => 45],
// ── Reports ─────────────────────────────────
['label_ar' => 'التقارير', 'label_en' => 'Reports', 'route' => '/reports', 'permission' => 'member.reports', 'order' => 50],
['label_ar' => 'التقارير', 'label_en' => 'Reports', 'route' => '/members/reports', 'permission' => 'member.reports', 'order' => 50],
],
]);
......@@ -57,4 +57,5 @@ PermissionRegistry::register('members', [
'member.pay_membership' => ['ar' => 'دفع رسوم العضوية', 'en' => 'Pay Membership Fee'],
'member.fill_form' => ['ar' => 'تعبئة نموذج العضو', 'en' => 'Fill Member Form'],
'member.reports' => ['ar' => 'تقارير العضوية', 'en' => 'Membership Reports'],
'member.retroactive' => ['ar' => 'إدخال عضوية بأثر رجعي', 'en' => 'Retroactive Membership Entry'],
]);
\ No newline at end of file
app/Modules/Notifications/bootstrap.php
View file @ d39d9293
......@@ -18,7 +18,7 @@ MenuRegistry::register('notifications', [
'children' => [
['label_ar' => 'سجل الرسائل', 'label_en' => 'SMS Log', 'route' => '/notifications/log', 'permission' => 'sms.view_log', 'order' => 1],
['label_ar' => 'إرسال رسالة', 'label_en' => 'Send SMS', 'route' => '/notifications/send', 'permission' => 'sms.send_single', 'order' => 2],
['label_ar' => 'القوالب', 'label_en' => 'Templates', 'route' => '/notifications/templates', 'permission' => 'sms.edit_templates', 'order' => 3],
['label_ar' => 'القوالب', 'label_en' => 'Templates', 'route' => '/notifications/templates', 'permission' => 'sms.view_log', 'order' => 3],
['label_ar' => 'المشغّلات التلقائية', 'label_en' => 'Auto Triggers', 'route' => '/notifications/triggers', 'permission' => 'notification.triggers.view', 'order' => 4],
],
]);
......
app/Modules/PlaygroundAdmin/Routes.php
View file @ d39d9293
......@@ -36,6 +36,6 @@ return [
['GET', '/playgrounds/{id:\d+}/dashboard/export', 'PlaygroundAdmin\Controllers\PlaygroundDashboardController@export', ['auth'], 'playground.dashboard'],
// Club-wide Sports Dashboard
['GET', '/sports-dashboard', 'PlaygroundAdmin\Controllers\ClubSportsDashboardController@index', ['auth'], 'playground.dashboard'],
['GET', '/sports-dashboard/export', 'PlaygroundAdmin\Controllers\ClubSportsDashboardController@export', ['auth'], 'playground.dashboard'],
['GET', '/playgrounds/dashboard', 'PlaygroundAdmin\Controllers\ClubSportsDashboardController@index', ['auth'], 'playground.dashboard'],
['GET', '/playgrounds/dashboard/export', 'PlaygroundAdmin\Controllers\ClubSportsDashboardController@export', ['auth'], 'playground.dashboard'],
];
app/Modules/PlaygroundAdmin/Views/club_dashboard.php
View file @ d39d9293
......@@ -2,7 +2,7 @@
<?php $__template->section('title'); ?>التقرير الرياضي الشامل<?php $__template->endSection(); ?>
<?php $__template->section('page_actions'); ?>
<a href="/sports-dashboard/export?filter=<?= e($filter) ?>&from=<?= e($data['period']['from'] ?? '') ?>&to=<?= e($data['period']['to'] ?? '') ?>"
<a href="/playgrounds/dashboard/export?filter=<?= e($filter) ?>&from=<?= e($data['period']['from'] ?? '') ?>&to=<?= e($data['period']['to'] ?? '') ?>"
class="btn btn-outline-success" target="_blank">تصدير التقرير</a>
<?php $__template->endSection(); ?>
......
app/Modules/Rentals/bootstrap.php
View file @ d39d9293
......@@ -21,6 +21,6 @@ MenuRegistry::register('rentals', [
'order' => 290,
'children' => [
['label_ar' => 'عقود الإيجار', 'label_en' => 'Contracts', 'route' => '/rentals', 'permission' => 'rental.view', 'order' => 1],
['label_ar' => 'الكيانات', 'label_en' => 'Entities', 'route' => '/rentals/entities', 'permission' => 'rental.manage_entity', 'order' => 2],
['label_ar' => 'الكيانات', 'label_en' => 'Entities', 'route' => '/rentals/entities', 'permission' => 'rental.view', 'order' => 2],
],
]);
app/Modules/Reports/Controllers/ReportController.php
View file @ d39d9293
......@@ -18,6 +18,9 @@ class ReportController extends Controller
$reports = ReportDefinition::getAllActive();
$grouped = [];
foreach ($reports as $r) {
if (!self::canRun($r)) {
continue;
}
$grouped[$r['category']][] = $r;
}
return $this->view('Reports.Views.index', ['grouped' => $grouped]);
......@@ -28,6 +31,8 @@ class ReportController extends Controller
$definition = ReportDefinition::findByCode($code);
if (!$definition) return $this->redirect('/reports')->withError('التقرير غير موجود');
$this->authorizeReport($definition);
$filters = [
'date_from' => $request->get('date_from', ''),
'date_to' => $request->get('date_to', ''),
......@@ -52,6 +57,8 @@ class ReportController extends Controller
$definition = ReportDefinition::findByCode($code);
if (!$definition) return $this->redirect('/reports')->withError('التقرير غير موجود');
$this->authorizeReport($definition);
$filters = [
'date_from' => $request->get('date_from', ''),
'date_to' => $request->get('date_to', ''),
......@@ -75,6 +82,8 @@ class ReportController extends Controller
$definition = ReportDefinition::findByCode($code);
if (!$definition) return $this->redirect('/reports')->withError('التقرير غير موجود');
$this->authorizeReport($definition);
$filters = [
'date_from' => $request->get('date_from', ''),
'date_to' => $request->get('date_to', ''),
......@@ -90,4 +99,32 @@ class ReportController extends Controller
'filters' => $filters,
]);
}
}
\ No newline at end of file
/**
* Each report row carries its own required_permission. It was previously
* stored and displayed but never enforced, so route access to /reports
* (report.view_membership) was enough to open ANY report — including the
* financial ones. Enforce the per-report gate at every data-producing entry
* point. See docs/architecture-maps/Authorization.md risk R6.
*/
private function authorizeReport(array $definition): void
{
$required = $definition['required_permission'] ?? null;
if (is_string($required) && $required !== '') {
$this->authorize($required);
}
}
private static function canRun(array $definition): bool
{
$required = $definition['required_permission'] ?? null;
if (!is_string($required) || $required === '') {
return true;
}
$employee = App::getInstance()->currentEmployee();
return $employee !== null
&& method_exists($employee, 'hasPermission')
&& $employee->hasPermission($required);
}
}
app/Modules/SportsActivity/Services/SaFinanceReportService.php
View file @ d39d9293
......@@ -132,23 +132,27 @@ final class SaFinanceReportService
private static function getRegistrationRevenue(\App\Core\Database $db, string $from, string $to, ?int $branchId): array
{
$where = "sp.registration_fee_paid = 1 AND sp.is_archived = 0 AND sp.created_at BETWEEN ? AND ?";
// Registration revenue is the sum of the fees actually charged, read from
// the transactional record (sa_registrations), exactly as subscription and
// booking revenue are. It must NOT be derived as headcount x a rate-card
// price: the member/non-member fee differs, and the configured rate has
// changed over time, so a rate card cannot reconstruct historical revenue.
$where = "r.payment_status = 'paid' AND r.status <> 'cancelled' AND r.created_at BETWEEN ? AND ?";
$params = [$from . ' 00:00:00', $to . ' 23:59:59'];
if ($branchId !== null) {
$where .= " AND sp.branch_id = ?";
$where .= " AND r.branch_id = ?";
$params[] = $branchId;
}
$row = $db->selectOne("SELECT COUNT(*) AS cnt FROM sa_players sp WHERE {$where}", $params);
$count = (int) ($row['cnt'] ?? 0);
$feeRow = $db->selectOne("SELECT base_price FROM sa_pricing_rules WHERE activity_type = 'registration' AND is_active = 1 ORDER BY id DESC LIMIT 1");
$feePerPlayer = $feeRow['base_price'] ?? '0.00';
$total = bcmul((string) $count, (string) $feePerPlayer, 2);
$row = $db->selectOne(
"SELECT COALESCE(SUM(r.registration_fee), 0) AS total, COUNT(*) AS cnt
FROM sa_registrations r
WHERE {$where}",
$params
);
return ['total' => $total, 'count' => $count];
return ['total' => $row['total'] ?? '0.00', 'count' => (int) ($row['cnt'] ?? 0)];
}
private static function getCoachCosts(\App\Core\Database $db, string $from, string $to, ?int $disciplineId, ?int $branchId): array
......
cli.php
View file @ d39d9293
......@@ -273,6 +273,106 @@ switch ($command) {
}
break;
case 'permissions:audit':
$app = \App\Core\App::getInstance();
$app->boot();
$svc = \App\Modules\AccessMatrix\Services\PermissionDiscoveryService::class;
$problems = 0;
echo "PERMISSION DECLARATION AUDIT\n";
echo str_repeat('=', 78) . "\n";
echo "Reconciles the four declaration sets: route gates, menu gates,\n";
echo "the permission catalogue, and role grants.\n";
echo "See docs/architecture-maps/Authorization.md\n\n";
// ── 1. Duplicate route paths (silently shadowed by module sort order) ──
$dupes = $svc::findDuplicateRoutes();
echo "1) DUPLICATE ROUTE PATHS\n" . str_repeat('-', 78) . "\n";
if (empty($dupes)) {
echo " OK no duplicate route paths.\n";
} else {
foreach ($dupes as $d) {
$flag = $d['conflicting'] ? 'CONFLICT' : 'dup';
echo " [{$flag}] {$d['route']}\n";
echo " serves : [{$d['winner']['module']}] permission="
. var_export($d['winner']['permission'], true) . "\n";
foreach ($d['shadowed'] as $sh) {
echo " DEAD : [{$sh['module']}] permission="
. var_export($sh['permission'], true) . " ({$sh['handler']})\n";
}
if ($d['conflicting']) {
$problems++;
}
}
}
// ── 2. Menu gate vs route gate ────────────────────────────────────────
$mismatches = $svc::findMenuRouteMismatches();
echo "\n2) MENU GATE != ROUTE GATE (link visible, page 403s)\n" . str_repeat('-', 78) . "\n";
if (empty($mismatches)) {
echo " OK every menu entry is gated on the permission its route enforces.\n";
} else {
foreach ($mismatches as $m) {
if ($m['kind'] === 'no_route') {
echo " [no route] {$m['url']} ({$m['label']})\n";
} elseif ($m['kind'] === 'menu_ungated') {
echo " [ungated ] {$m['url']} visible to ALL, route needs '{$m['route']}' ({$m['label']})\n";
} else {
echo " [mismatch] {$m['url']} menu='{$m['menu']}' route='{$m['route']}' ({$m['label']})\n";
}
$problems++;
}
}
// ── 3. Route permissions no role holds ────────────────────────────────
$ungranted = $svc::findUngrantedRoutePermissions();
echo "\n3) ROUTE PERMISSIONS GRANTED TO NO ROLE (super-admin-only by accident)\n" . str_repeat('-', 78) . "\n";
if ($ungranted === null) {
echo " SKIPPED no database connection.\n";
} elseif (empty($ungranted)) {
echo " OK every route permission is held by at least one role.\n";
} else {
$byModule = [];
foreach ($ungranted as $perm => $module) {
$byModule[$module][] = $perm;
}
ksort($byModule);
foreach ($byModule as $module => $perms) {
echo " " . str_pad($module, 24) . count($perms) . " " . implode(', ', array_slice($perms, 0, 4))
. (count($perms) > 4 ? ', ...' : '') . "\n";
}
echo "\n Total: " . count($ungranted) . " permissions reachable only by super_admin.\n";
$problems += count($ungranted);
}
// ── 4. Granted keys nobody registered ─────────────────────────────────
$phantom = $svc::findPhantomGrants();
echo "\n4) PHANTOM GRANTS (granted key never registered by any module)\n" . str_repeat('-', 78) . "\n";
if ($phantom === null) {
echo " SKIPPED no database connection.\n";
} elseif (empty($phantom)) {
echo " OK every granted key exists in the catalogue.\n";
} else {
foreach ($phantom as $perm => $roles) {
echo " " . str_pad($perm, 38) . implode(', ', array_unique($roles)) . "\n";
$problems++;
}
}
// ── 5. Routes with no gate at all ─────────────────────────────────────
$unprotected = $svc::findUnprotectedRoutes();
echo "\n5) ROUTES WITH NO PERMISSION\n" . str_repeat('-', 78) . "\n";
echo " " . count($unprotected) . " route(s) declare no permission "
. "(run `php cli.php permissions:unprotected` for the list).\n";
echo "\n" . str_repeat('=', 78) . "\n";
if ($problems === 0) {
echo "AUDIT PASSED — declaration sets agree.\n";
exit(0);
}
echo "AUDIT FAILED — {$problems} problem(s). Fix before shipping.\n";
exit(1);
case 'export:book':
echo "📖 Generating Book of the ERP PDF...\n";
$storageDir = __DIR__ . '/storage/cache';
......@@ -367,6 +467,7 @@ switch ($command) {
echo " php cli.php routes List all routes\n";
echo " php cli.php export:book Generate Book of the ERP PDF\n";
echo " php cli.php permissions:orphans Registered but unused permissions\n";
echo " php cli.php permissions:audit Reconcile route/menu/catalogue/role declarations\n";
echo " php cli.php permissions:unprotected Routes without permissions\n";
echo " php cli.php permissions:stats Permission count per module\n";
echo " php cli.php help Show this help\n";
......
database/seeds/Phase_105_001_fix_role_access_gaps.php 0 → 100644
View file @ d39d9293
<?php
declare(strict_types=1);
use App\Core\Database;
/**
* Close the role-access gaps reported by operations, and align role grants with
* the permissions the routes actually enforce.
*
* Context: the permission catalogue grew with each module, but the role seeds did
* not keep pace, leaving ~49% of route permissions held by no role except
* super_admin (whose '*' satisfies everything). Roles therefore saw sidebar links
* they could not open. See docs/architecture-maps/Authorization.md.
*
* Idempotent: every grant is existence-checked, so re-running is safe.
*/
return function (Database $db): void {
$now = date('Y-m-d H:i:s');
// ── Grants ───────────────────────────────────────────────────────────────
$grants = [
// Reviews reports across membership AND sports activity.
'report_viewer' => [
'temp.view', 'sports.view', 'member.reports', 'sa.report.players', 'sa.report.finance',
'sa.report.export', 'report.view_sports', 'report.view_sports_financial'
],
// Reported: no access to the sports activity reports.
'general_manager' => [
'sa.report.players', 'sa.report.finance', 'sa.report.export', 'report.view_sports',
'report.view_sports_financial'
],
// Reported: only 4 membership pages reachable; carnet issuing missing.
'receptionist' => [
'temp.view', 'sports.view', 'carnet.issue', 'carnet.print'
],
// Reported: cannot assign players to groups via /sa/coach-assessment.
'sports_officer' => [
'sa.coach_assessment.view', 'sa.coach_assessment.manage', 'sa.player.assign'
],
// Owns the sports activity module end to end: full access to every sa.* page.
// NOTE: includes sa.subscription.exempt, which waives money. Revoke that one
// key if the academy manager should not be able to grant exemptions.
'academy_manager' => [
'sa.academy.manage', 'sa.academy.view', 'sa.attendance.manage', 'sa.attendance.view',
'sa.booking.create', 'sa.booking.manage', 'sa.booking.view', 'sa.booking_wizard.use',
'sa.card.manage', 'sa.card.print', 'sa.card.view', 'sa.coach.manage', 'sa.coach.view',
'sa.coach_assessment.manage', 'sa.coach_assessment.view', 'sa.contract.approve',
'sa.contract.manage', 'sa.contract.view', 'sa.dashboard', 'sa.discipline.manage',
'sa.discipline.view', 'sa.enrollment.manage', 'sa.facility.manage', 'sa.facility.view',
'sa.game.manage', 'sa.game.view', 'sa.gate.scan', 'sa.gate.view', 'sa.group.enroll',
'sa.group.force_enroll', 'sa.group.manage', 'sa.group.view', 'sa.institution.manage',
'sa.institution.view', 'sa.locker.manage', 'sa.locker.view', 'sa.locker_rental.create',
'sa.locker_rental.evict', 'sa.locker_rental.manage', 'sa.locker_rental.view',
'sa.makeup.manage', 'sa.makeup.view', 'sa.medical.approve', 'sa.mirror.view',
'sa.player.assign', 'sa.player.manage', 'sa.player.view', 'sa.pool-grid.manage',
'sa.pool_reservation.create', 'sa.pool_reservation.manage', 'sa.pool_reservation.view',
'sa.pool_ticket.issue', 'sa.pool_ticket.manage', 'sa.pool_ticket.view', 'sa.pricing.manage',
'sa.pricing.view', 'sa.program.manage', 'sa.program.view', 'sa.registration.manage',
'sa.registration.view', 'sa.report.export', 'sa.report.finance', 'sa.report.players',
'sa.schedule.manage', 'sa.schedule.view', 'sa.subscription.collect',
'sa.subscription.exempt', 'sa.subscription.generate', 'sa.subscription.view',
'sa.swimming.assign', 'sa.swimming.coach_manage', 'sa.swimming.dashboard',
'sa.swimming.register', 'sa.waitlist.manage', 'sa.waitlist.view'
],
// Reported: must have full access to شئون العضوية, including إدخال بأثر رجعي.
// member.retroactive is a new permission that replaces the hardcoded
// super-admin check previously inside RetroactiveWizardController.
'membership_director' => [
'member.retroactive', 'member.reports', 'sports.view', 'subscription.view',
'subscription.generate_batch', 'installment.view', 'fine.view', 'waiver.view', 'carnet.view',
'carnet.issue'
],
// Governance roles: report_definitions.required_permission is now actually
// enforced (it was stored but never checked). Grant the sports report keys
// so tightening enforcement does not remove reports these roles oversee.
'board_member' => ['report.view_sports', 'report.view_sports_financial'],
'auditor' => ['report.view_sports', 'report.view_sports_financial'],
// Retroactive entry stays available to the system administrator role.
'it_admin' => ['member.retroactive'],
];
// ── Revocations ──────────────────────────────────────────────────────────
// Reported: the facilities manager handles bookings and reservations only,
// and must not be able to view or search members (this also removes
// العضوية الشرفية and الأعضاء الأجانب, both gated on member.view).
$revokes = [
'facilities_manager' => ['member.view', 'member.search'],
];
$roleId = static function (string $code) use ($db): ?int {
$row = $db->selectOne("SELECT id FROM roles WHERE role_code = ?", [$code]);
return $row ? (int) $row['id'] : null;
};
$granted = 0;
foreach ($grants as $roleCode => $permissions) {
$id = $roleId($roleCode);
if ($id === null) {
echo " skip: role {$roleCode} not found\n";
continue;
}
foreach (array_unique($permissions) as $permission) {
$exists = $db->selectOne(
"SELECT 1 FROM role_permissions WHERE role_id = ? AND permission_key = ?",
[$id, $permission]
);
if ($exists) {
continue;
}
$db->insert('role_permissions', [
'role_id' => $id,
'permission_key' => $permission,
'granted_at' => $now,
]);
$granted++;
}
}
$revoked = 0;
foreach ($revokes as $roleCode => $permissions) {
$id = $roleId($roleCode);
if ($id === null) {
echo " skip: role {$roleCode} not found\n";
continue;
}
foreach ($permissions as $permission) {
$db->delete('role_permissions', '`role_id` = ? AND `permission_key` = ?', [$id, $permission]);
$revoked++;
}
}
echo " Role access gaps: {$granted} permission(s) granted, {$revoked} revoked.\n";
};
docs/architecture-maps/Authorization.md 0 → 100644
View file @ d39d9293
# Architecture Map — Authorization (Permissions, Roles, Menu Gating)
> Living document. Never regenerate from scratch; append and refine.
> Created 2026-08-30 during the "pages appear but 403" investigation.
## Module Purpose
Authorization is **not a module**. It is a cross-cutting concern implemented by
four cooperating pieces that live in different places and are declared by all 74
modules independently. This map exists because that distribution is exactly what
makes it break.
## The Four Declaration Sets
| # | What | Declared in | Consumed by | Effect |
|---|------|-------------|-------------|--------|
| 1 | **Permission catalogue** | `app/Modules/*/bootstrap.php` -> `PermissionRegistry::register($group, $defs)` | Roles admin UI (checkbox list) | Defines which permission keys *exist*. Purely descriptive — registering a key grants nothing and enforces nothing. |
| 2 | **Route gate** | `app/Modules/*/Routes.php`, 5th tuple element | `AuthMiddleware` | **The only real enforcement.** |
| 3 | **Menu gate** | `app/Modules/*/bootstrap.php` -> `MenuRegistry::register()` (`permission` key, and per-child `permission`) | `MenuRegistry::getVisible()` | Decides sidebar visibility only. A *hint*, not enforcement. |
| 4 | **Role grants** | `database/seeds/Phase_*.php` -> inserts into `role_permissions` | `Employee::getAllPermissions()` | Decides which roles actually hold a key. |
**Nothing reconciles sets 2, 3 and 4.** That is the single most important fact in
this document, and the root cause of every access defect found so far.
## Entry Points and Flow
```
public/index.php
-> App::boot()
glob('app/Modules/*/bootstrap.php') + sort() <-- ALPHABETICAL, load order matters
glob('app/Modules/*/Routes.php') + sort() <-- ALPHABETICAL, load order matters
-> Router::dispatch(Request)
foreach ($this->routes as $route) { ... return $response; } <-- FIRST MATCH WINS
$request->setAttribute('_permission', $route['permission'])
-> AuthMiddleware::handle()
session -> Employee::find() -> is_active / is_archived / session timeout
-> force_password_change redirect
-> if ($requiredPermission && !$employee->hasPermission($requiredPermission))
return 403 render403() "403 غير مصرح لك بالوصول لهذه الصفحة"
-> Controller
(some controllers add their OWN hardcoded role checks here - see Risk Areas)
```
### Two distinct 403 pages — use them to localise a report
| Text | Emitted by | Meaning |
|------|-----------|---------|
| `غير مصرح لك بالوصول لهذه الصفحة` | `AuthMiddleware::render403()` | Route-level permission denial |
| `ليس لديك صلاحية لهذا الإجراء` | `PermissionMiddleware::handle()` | Same check, via the optional `permission` middleware |
| `هذه الأداة متاحة فقط لمدير النظام` (RuntimeException 403) | a controller's own hardcoded check | Permission system was bypassed |
`PermissionMiddleware` is **redundant**: `AuthMiddleware` already performs the
identical `_permission` check. Routes listing `['auth','permission']` are checked
twice; routes listing only `['auth']` are still fully enforced.
## Permission Resolution — `Employee::getAllPermissions()`
Source: `app/Modules/Users/Models/Employee.php`. Order of operations:
1. `employee_roles` JOIN `role_permissions` JOIN `roles`
filtered by `er.is_active = 1`, `r.is_active = 1`,
and `er.expires_at IS NULL OR er.expires_at > NOW()`
2. `+` parent-role permissions (`getParentRolePermissions()`, one level)
3. `+` direct grants from `employee_permissions` where `is_denied = 0`
4. `-` direct denials from `employee_permissions` where `is_denied = 1`
5. Result cached per-request in `$cachedPermissions`; denials in `$cachedDenials`
`hasPermission($key)`:
- `'*'` in the permission set -> **true for everything** (this is how `super_admin` works;
there is no separate super-admin branch in the middleware)
- explicit denial -> false
- otherwise -> `in_array($key, $permissions, true)`
**Consequence:** `super_admin` is just a role holding the single key `*`. Any
hardcoded `role_code = 'super_admin'` query in a controller is a *second,
divergent* implementation of that idea and is the wrong pattern (see Risk Areas).
## Menu Gating — `MenuRegistry::getVisible()`
```php
$perm = $item['permission'] ?? null;
if ($perm === null || in_array($perm, $userPermissions) || in_array('*', $userPermissions))
```
Two behaviours worth knowing:
- A menu item (or child) with `permission => null` is **visible to every
authenticated user**, regardless of what its route requires.
- Children are filtered independently, but only *after* the parent passes. A
child whose permission the user holds is still hidden if they lack the
parent's permission. This is why granting a leaf permission alone often
appears to do nothing.
## Database Schema (authorization tables)
> NOTE: the live DB (`srv-captain--mysql-db:3306`) was NOT reachable from the
> dev environment at time of writing, and no `pdo_mysql` driver is installed
> locally. The shapes below are derived from migrations + consuming queries and
> are marked UNVERIFIED against production. Per the project DATABASE TRUTH RULE
> these must be re-verified against the live DB when access is available.
- `roles` — `id, role_code, name_ar, name_en, description_ar, is_system,
category, level, is_active, parent_role_id?, all_branches?` (UNVERIFIED)
- `role_permissions` — `role_id, permission_key, granted_at` (UNVERIFIED)
- `employee_roles` — `employee_id, role_id, is_active, expires_at` (UNVERIFIED)
- `employee_permissions` — `employee_id, permission_key, is_denied, notes` (UNVERIFIED)
`permission_key` is a free-text string with **no foreign key** to any catalogue
table — the catalogue exists only in PHP (`PermissionRegistry`). Nothing prevents
granting a key that does not exist, or requiring a key nobody was granted.
## Role Inventory (28 roles, from seed replay)
`super_admin`(*), `board_member`, `membership_director`, `membership_officer`,
`treasury_manager`, `treasury_officer`, `sales_agent`, `security_officer`,
`report_viewer`, `auditor`, `general_manager`, `hr_manager`, `hr_officer`,
`cashier_operator`, `accountant`, `sports_coordinator`, `academy_manager`,
`sports_coach`, `facilities_manager`, `receptionist`, `gate_guard`, `it_admin`,
`department_head`, `membership_cashier`, `sports_cashier`, `main_cashier`,
`sports_officer`, `sports_director`
Seeds that define or amend grants, in apply order:
`Phase_02_001` -> `Phase_02_002` -> `Phase_38_001` -> `Phase_71_001` ->
`Phase_72_002` -> `Phase_73_001` -> `Phase_75_001`
Treasury model (three tiers, established in `Phase_75_001`):
- `membership_cashier` — خزنة العضويات; membership payments only
- `sports_cashier` — الخزنة الفرعية; sports activity and other non-membership collection
- `main_cashier` — الخزنة الرئيسية; receives settlements from the sub-treasuries, bank deposits
## Risk Areas
### R1 — Duplicate route paths are silently shadowed
`Router::dispatch()` returns on first match; modules load alphabetically. A path
declared by two modules resolves to the alphabetically-earlier module, with *its*
permission. Grep cannot see this; only load-order reproduction can.
### R2 — Menu gate and route gate drift
They are separate strings with no contract. Drift presents to users as
"the page is in my menu but 403s", which reads as a broken permission rather than
a broken declaration.
### R3 — Permission catalogue grows, role grants do not
Each new module registers keys and gates routes with them, but the role seeds are
rarely updated. Keys therefore default to super-admin-only. As of 2026-08-30,
**208 of 426 route permissions (49%) were held by no role except super_admin.**
### R4 — Hardcoded `role_code = 'super_admin'` checks inside controllers
A parallel, divergent authorization path that the permission system cannot see,
cannot grant, and the Roles admin UI cannot configure. Known sites:
`Members\RetroactiveWizardController`, `Members\MemberController`,
`Children\ChildController`, `Temporary\TemporaryController`,
`Accounting\ReportController`, `Payments\PaymentController`.
Only the *gate* form (deny access) is a defect; the *capability* form
(e.g. "super admin may edit a locked membership number") is legitimate business
logic but should still resolve through a named permission.
### R6 — Row-level permissions that are stored but never checked
`report_definitions.required_permission` was persisted, displayed, and never
enforced: route access to `/reports` was sufficient to open ANY report by code,
including financial ones. Fixed 2026-08-30 (`Reports\ReportController`:
`authorizeReport()` on view/export/print, `canRun()` filters the listing).
**Generalised rule:** any permission stored as data must have exactly one
enforcement call site per entry point. Search for other `*_permission` columns
before assuming this was the only instance.
### R5 — No referential integrity on `permission_key`
Grants of non-existent keys are accepted silently (found: `receipts.print`).
## Change Impact Analysis — what to check when touching authorization
- Adding a route -> is its permission granted to any role? Is a menu item gated on the *same* key?
- Adding a menu item -> does its `permission` equal the permission of the route its `route` resolves to?
- Adding a route path -> does that exact path already exist in another module?
- Adding a permission key -> which roles should hold it? Add to a seed in the same change.
- Changing a role's grants -> re-run the audit; check parent-role inheritance.
## Verification Tooling
`php cli.php audit:permissions` (see `app/Console/PermissionAudit.php`) reproduces
module load order and reports R1-R5 as a single pass. Run it after ANY change to
a `Routes.php`, a `bootstrap.php` menu/permission block, or a role seed. It is
the regression check for this whole map.
## Related Maps
`Members.md`, `SportsActivity.md`, `Dashboard.md`, and `DEPENDENCY-GRAPH.md`
(authorization is an upstream dependency of every module).
## Change Log
### 2026-08-30 — "pages appear in the sidebar but 403" investigation
Reported by operations for board member, general manager, report viewer,
receptionist, sports officer, facilities manager, academy manager and
membership director. All symptoms traced to five root causes; all fixed.
| Fix | Change |
|-----|--------|
| R1 | `GET /reports` was declared by both `Members` and `Reports`; `Members` won on module sort and enforced `member.reports` while the sidebar gated on `report.view_membership`. Members' report routes relocated to `/members/reports/*`. |
| R1 | `GET /sports-dashboard[/export]` was declared by three modules. `Disciplines` -> `/disciplines/dashboard`; `PlaygroundAdmin` -> `/playgrounds/dashboard[/export]`. `/sports-dashboard` is now wholly owned by `SportsDashboard`, so its index and its drill-downs finally come from the same module. |
| R1 | `Members/Routes.php` used unconstrained `{id}` in 15 routes, so `/members/<anything>` was swallowed by `MemberController@show`. Constrained to `{id:\d+}`, matching every other module. This is what made the `/members/reports` relocation resolve to `member.view` until fixed. |
| R2 | Six menu/route gate mismatches aligned (`/members/search`, `/sports`, `/carnets`, `/rentals/entities`, `/notifications/templates`, `/reports`). |
| R3 | `database/seeds/Phase_105_001_fix_role_access_gaps.php` grants the reported roles the permissions their routes enforce, and revokes `member.view`/`member.search` from `facilities_manager`. |
| R4 | `RetroactiveWizardController`'s hardcoded `role_code = 'super_admin'` gate replaced with a new registered permission `member.retroactive`, enforced by the route. |
| R6 | Per-report `required_permission` now enforced. |
**Still open (not defects introduced here):**
- ~200 further route permissions remain granted to no role but `super_admin`.
`php cli.php permissions:audit` lists them by module; each needs a business
decision about which role should hold it. Nothing is broken by them today —
those pages are simply reachable only by the super admin.
- Five further hardcoded `super_admin` checks remain (R4): `MemberController`,
`ChildController`, `TemporaryController`, `Accounting\ReportController`,
`PaymentController`. Most are *capability* checks (e.g. "may edit a locked
membership number") rather than page gates, so they do not cause 403s, but
they should still resolve through named permissions.
- `/seasonal` is gated on `temp.view` while a registered `seasonal.view` exists
and is used by nothing. Harmless today; re-gating would require granting
`seasonal.view` to everyone currently holding `temp.view`.
- `treasuries.SUB_MEM` (membership sub-treasury) is created by migration
`Phase_75_002`, and `TreasuryService::membershipTreasury()` returns null
without it. That migration also creates GL account `12060103`, but
`Phase_43_001_seed_full_chart_of_accounts` seeds `12060103` as
"الصندوق باليورو". Whichever runs first wins, and the migration's
`if (!$glExists)` guard means SUB_MEM can silently end up pointing at the
Euro cash account. **Verify against the live DB.**
docs/architecture-maps/DEPENDENCY-GRAPH.md
View file @ d39d9293
......@@ -688,3 +688,55 @@ arrives from the client and is never trusted as an authorisation decision.
`ReportEngine::outstandingReport()` (`RPT_OUTSTANDING`). The two still differ in one respect:
the report includes outstanding `fines`, the widget does not. Immaterial today (no fines due),
but they will diverge once fines are used — keep them in sync if that changes.
---
## Authorization — a cross-cutting upstream dependency of every module
Full detail: `docs/architecture-maps/Authorization.md`. Recorded here because
authorization defects are inherently cross-module: the declaration that breaks
access usually lives in a *different* module from the page that 403s.
### The dependency nobody declares
Every module independently declares four things that must agree but are never
reconciled by the framework:
```
bootstrap.php --PermissionRegistry::register--> catalogue (descriptive only)
bootstrap.php --MenuRegistry::register--------> menu gate (visibility hint)
Routes.php --tuple[4]---------------------> route gate (ACTUAL enforcement)
database/seeds --insert role_permissions------> role grants
```
A module can be internally perfect and still be unreachable, because the grant
that makes its permission usable lives in `database/seeds/` and the sidebar entry
that advertises it may live in another module's `bootstrap.php`.
### Cascading-change rules
| If you change… | Then you must also… | Because |
|---|---|---|
| a route's `permission` | update the menu entry pointing at that path, in whatever module declares it | menu gate and route gate drift silently; the user sees the link and gets a 403 |
| a route's **path** | grep for the old path across ALL modules' views, controllers, `Dashboard/Config/widgets.php` `drill_link`s, and `TutorialRegistry` | links are hardcoded strings, not generated from route names |
| add a route path | check no other module already declares it | `Router::dispatch()` is first-match-wins over an alphabetically sorted module glob; the loser is dead code |
| add a `{param}` without a constraint | verify it cannot swallow sibling literal paths | `/members/{id}` (unconstrained) matched `/members/reports` |
| register a permission | grant it to at least one role in the same change | otherwise it is super-admin-only by accident (~200 keys are currently in this state) |
| revoke a permission from a role | check every module whose menu/route uses it | e.g. revoking `member.view` removes `/members`, `/honorary` and `/foreign` at once — three modules, one key |
| add a `*_permission` **column** | add exactly one enforcement call per entry point | stored-but-unchecked permissions read as security that is not there (`report_definitions.required_permission`) |
### Modules that share a permission key (revoking one affects all)
| Key | Modules gated on it |
|---|---|
| `member.view` | Members, Honorary, Foreign (+ member lookups elsewhere) |
| `temp.view` | Temporary, Seasonal |
| `carnet.view` | Carnets, and the Members sidebar group |
| `report.view_membership` | Reports (route), Dashboard (widgets) |
| `sa.*` | SportsActivity, and read-only views in Dashboard/SportsDashboard |
### Verification
`php cli.php permissions:audit` reconciles all four declaration sets and exits
non-zero on drift. Run it after touching any `Routes.php`, any `bootstrap.php`
menu/permission block, or any role seed.
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment