diff --git a/.agents/runs/2026-04-24-action-csrf-centralization-step3/analysis.json b/.agents/runs/2026-04-24-action-csrf-centralization-step3/analysis.json new file mode 100644 index 0000000..a158b8b --- /dev/null +++ b/.agents/runs/2026-04-24-action-csrf-centralization-step3/analysis.json @@ -0,0 +1,49 @@ +{ + "task_id": "2026-04-24-action-csrf-centralization-step3", + "issue_summary": "Nach Wave 2 waren in pages/admin noch einzelne Endpunkte mit direkten requestInput()->method() und Session::checkCsrfToken()-Checks vorhanden (JSON/Data und einige User-POST-Endpoints). Ziel ist die komplette Zentralisierung der Request/CSRF-Guards in pages/admin.", + "affected_layers": ["pages", "core/Support", "tests"], + "affected_files": [ + { "path": "core/Support/helpers/request.php", "role": "Action guard helpers (POST/CSRF) extended for JSON-focused endpoints" }, + { "path": "pages/admin/users/theme().php", "role": "User theme switch endpoint (POST + optional JSON)" }, + { "path": "pages/admin/users/switch-tenant().php", "role": "Tenant context switch endpoint (POST + optional JSON)" }, + { "path": "pages/admin/session-ping/data().php", "role": "JSON keepalive endpoint (POST-only + CSRF)" }, + { "path": "pages/admin/tenants/ldap-test-connection/data().php", "role": "JSON LDAP test endpoint (POST-only + CSRF)" }, + { "path": "pages/admin/users/create().php", "role": "User create form action (POST + CSRF)" }, + { "path": "pages/admin/users/bulk($action).php", "role": "Bulk user mutation endpoint (JSON + CSRF)" }, + { "path": "pages/admin/users/api-token-create($id).php", "role": "API token create action (POST + CSRF)" }, + { "path": "pages/admin/users/api-token-revoke($id).php", "role": "API token revoke action (POST + CSRF)" }, + { "path": "pages/admin/imports/index().php", "role": "Import multi-stage POST flow with in-page error rendering" }, + { "path": "tests/Architecture/AdminActionRequestGuardContractTest.php", "role": "New architecture contract to block direct legacy method/csrf checks in pages/admin" } + ], + "existing_patterns": [ + "Most admin actions already migrated to actionRequirePost/actionRequireCsrf in waves 1+2", + "Remaining inline checks were concentrated in a small set of JSON/data and legacy user actions", + "Some endpoints need JSON payload/status customization and non-redirect CSRF failure handling" + ], + "related_tests": [ + "tests/Architecture/AdminActionRequestGuardContractTest.php", + "tests/Architecture/PostEndpointCsrfContractTest.php", + "tests/Architecture/PostRedirectGetContractTest.php", + "tests/Architecture/AuthzAdminSettingsContractTest.php", + "tests/Architecture/AuthzAdminMasterDataContractTest.php", + "tests/Architecture/AuthzAdminUsersContractTest.php" + ], + "security_surface": { + "authz_relevant": true, + "tenant_scope_relevant": true, + "input_handling": true, + "crypto_relevant": false, + "file_upload_relevant": false, + "notes": "Focus remains on GR-SEC-001 CSRF hardening and preserving existing authz / redirect/json behavior." + }, + "assumptions": [ + "Scope is strictly pages/admin completion; non-admin pages and module pages are out-of-scope for this step.", + "Existing JSON response payload shapes must be preserved where already consumed by frontend code.", + "No service/repository business logic changes are required." + ], + "risks_discovered": [ + "JSON-only data endpoints need helper support beyond default Request::wantsJson detection and default payload shape.", + "imports/index keeps CSRF errors in-page (non-redirect), requiring non-redirect helper mode.", + "api-token create/revoke previously had no explicit POST method guard; adding it must keep redirect behavior safe." + ] +} diff --git a/.agents/runs/2026-04-24-action-csrf-centralization-step3/execution-report.json b/.agents/runs/2026-04-24-action-csrf-centralization-step3/execution-report.json new file mode 100644 index 0000000..0abef71 --- /dev/null +++ b/.agents/runs/2026-04-24-action-csrf-centralization-step3/execution-report.json @@ -0,0 +1,80 @@ +{ + "task_id": "2026-04-24-action-csrf-centralization-step3", + "plan_ref": ".agents/runs/2026-04-24-action-csrf-centralization-step3/plan.json", + "status": "done", + "changed_files": [ + { "path": "core/Support/helpers/request.php", "summary": "Extended helper API with jsonStatusCode/jsonPayload/forceJson options for POST/CSRF guard responses." }, + { "path": "pages/admin/users/theme().php", "summary": "Replaced inline method/csrf checks with actionRequirePost/actionRequireCsrf while preserving JSON error payload." }, + { "path": "pages/admin/users/switch-tenant().php", "summary": "Replaced inline method/csrf checks with helper-based flow and preserved flash/json behavior." }, + { "path": "pages/admin/session-ping/data().php", "summary": "Migrated to helper guards using forceJson and status-preserving JSON payloads." }, + { "path": "pages/admin/tenants/ldap-test-connection/data().php", "summary": "Migrated method/csrf checks to helper guards with forceJson and original error payloads." }, + { "path": "pages/admin/users/create().php", "summary": "Replaced inline CSRF block with actionRequireCsrf in POST branch." }, + { "path": "pages/admin/users/bulk($action).php", "summary": "Added helper-based POST guard and migrated CSRF check to actionRequireCsrf in JSON mode." }, + { "path": "pages/admin/users/api-token-create($id).php", "summary": "Added actionRequirePost and actionRequireCsrf; kept redirect/flash semantics." }, + { "path": "pages/admin/users/api-token-revoke($id).php", "summary": "Added actionRequirePost and actionRequireCsrf; kept redirect/flash semantics." }, + { "path": "pages/admin/imports/index().php", "summary": "Replaced inline CSRF check with non-redirect actionRequireCsrf mode to keep in-page validation behavior." }, + { "path": "tests/Architecture/AdminActionRequestGuardContractTest.php", "summary": "Added contract test to prevent direct legacy method/csrf checks in pages/admin." } + ], + "guard_evidence": [ + { + "guard_id": "GR-CORE-003", + "status": "pass", + "evidence": "All affected admin actions keep requestInput() usage; no superglobal access introduced." + }, + { + "guard_id": "GR-CORE-012", + "status": "pass", + "evidence": "PRG flows remain intact where applicable; PostRedirectGetContractTest passes." + }, + { + "guard_id": "GR-SEC-001", + "status": "pass", + "evidence": "Inline Session::checkCsrfToken removed from pages/admin; helper-based CSRF validation now centralized." + }, + { + "guard_id": "GR-SEC-008", + "status": "pass", + "evidence": "Authorization checks remain unchanged in migrated actions." + }, + { + "guard_id": "GR-SEC-009", + "status": "pass", + "evidence": "Tenant scope and actor context validation flows remain unchanged." + }, + { + "guard_id": "GR-TEST-001", + "status": "pass", + "evidence": "Targeted architecture tests and full PHPUnit suite pass." + } + ], + "commands": [ + { "cmd": "docker compose exec -T php vendor/bin/phpunit tests/Architecture/AdminActionRequestGuardContractTest.php", "result": "pass" }, + { "cmd": "docker compose exec -T php vendor/bin/phpunit tests/Architecture/PostEndpointCsrfContractTest.php tests/Architecture/PostRedirectGetContractTest.php", "result": "pass" }, + { "cmd": "docker compose exec -T php vendor/bin/phpunit tests/Architecture/AuthzAdminSettingsContractTest.php tests/Architecture/AuthzAdminMasterDataContractTest.php tests/Architecture/AuthzAdminUsersContractTest.php", "result": "pass" }, + { "cmd": "docker compose exec -T php vendor/bin/phpunit", "result": "pass" }, + { "cmd": "docker compose exec -T php vendor/bin/phpstan analyse -c phpstan.neon --no-progress", "result": "pass" }, + { "cmd": "docker compose exec -T php vendor/bin/phpunit tests/Architecture/CoreStarterkitContractTest.php", "result": "pass" }, + { "cmd": "docker compose exec -T php vendor/bin/php-cs-fixer fix --config=tools/php-cs-fixer/.php-cs-fixer.dist.php --dry-run --diff --verbose", "result": "pass" }, + { "cmd": "bin/docs-link-check.sh", "result": "pass" }, + { "cmd": "bin/docs-drift-check.sh", "result": "pass" }, + { "cmd": "bin/codex-skills-sync.sh --check", "result": "pass" } + ], + "quality_gate_results": [ + { "gate_id": "QG-001", "result": "pass", "notes": "PHPUnit full run green: 2025 tests, 29191 assertions." }, + { "gate_id": "QG-002", "result": "pass", "notes": "PHPStan level 5: no errors." }, + { "gate_id": "QG-003", "result": "pass", "notes": "CoreStarterkitContractTest green: 13 tests, 6980 assertions." }, + { "gate_id": "QG-006", "result": "pass", "notes": "php-cs-fixer dry-run green." }, + { "gate_id": "QG-008", "result": "pass", "notes": "docs-link-check and docs-drift-check green." }, + { "gate_id": "QG-009", "result": "pass", "notes": "codex-skills-sync --check green." } + ], + "metrics": { + "step3_target_files": 9, + "before_requestinput_method_calls": 4, + "after_requestinput_method_calls": 0, + "before_inline_session_csrf_checks": 9, + "after_inline_session_csrf_checks": 0, + "admin_inline_method_checks_total_after": 0, + "admin_inline_csrf_checks_total_after": 0 + }, + "open_items": [] +} diff --git a/.agents/runs/2026-04-24-action-csrf-centralization-step3/finalize.json b/.agents/runs/2026-04-24-action-csrf-centralization-step3/finalize.json new file mode 100644 index 0000000..b32a831 --- /dev/null +++ b/.agents/runs/2026-04-24-action-csrf-centralization-step3/finalize.json @@ -0,0 +1,12 @@ +{ + "task_id": "2026-04-24-action-csrf-centralization-step3", + "ready_to_finalize": true, + "code_review": "pass", + "security_review": "pass", + "acceptance_review": "pass", + "ci_status": "required-gates-pass", + "final_action": "commit", + "hold_reason": "", + "commit_message": "refactor(actions): complete admin request/csrf guard centralization", + "notes": "Final admin wave completed; pages/admin no longer contains direct requestInput()->method() or Session::checkCsrfToken() checks." +} diff --git a/.agents/runs/2026-04-24-action-csrf-centralization-step3/plan.json b/.agents/runs/2026-04-24-action-csrf-centralization-step3/plan.json new file mode 100644 index 0000000..e2d325a --- /dev/null +++ b/.agents/runs/2026-04-24-action-csrf-centralization-step3/plan.json @@ -0,0 +1,92 @@ +{ + "task_id": "2026-04-24-action-csrf-centralization-step3", + "analysis_ref": ".agents/runs/2026-04-24-action-csrf-centralization-step3/analysis.json", + "summary": "Final wave to complete POST/CSRF guard centralization for all remaining admin actions, including JSON/data endpoints and architecture enforcement for future drift prevention.", + "scope": { + "in": [ + "Extend actionRequirePost/actionRequireCsrf for JSON status/payload and forceJson use-cases", + "Migrate all remaining inline method/csrf checks in pages/admin to helper-based guards", + "Add architecture test that forbids direct requestInput()->method() and Session::checkCsrfToken() usage in pages/admin", + "Run required quality gates and admin-related architecture checks" + ], + "out": [ + "Non-admin core pages", + "Module pages under modules/*/pages", + "Business-logic changes in services/repositories" + ] + }, + "guardrails": { + "required_guard_ids": [ + "GR-CORE-003", + "GR-CORE-012", + "GR-SEC-001", + "GR-SEC-008", + "GR-SEC-009", + "GR-TEST-001" + ], + "required_quality_gate_ids": [ + "QG-001", + "QG-002", + "QG-003", + "QG-006", + "QG-008", + "QG-009" + ] + }, + "success_criteria": [ + { + "id": "SC-001", + "criterion": "All remaining pages/admin inline method checks are replaced by actionRequirePost() or requestInput()->isMethod('POST') + actionRequireCsrf() patterns." + }, + { + "id": "SC-002", + "criterion": "All remaining pages/admin inline Session::checkCsrfToken() checks are replaced by actionRequireCsrf()." + }, + { + "id": "SC-003", + "criterion": "New architecture contract prevents reintroduction of direct requestInput()->method() and Session::checkCsrfToken() in pages/admin." + }, + { + "id": "SC-004", + "criterion": "CSRF and PRG architecture tests remain green and authz admin contract suites remain green." + }, + { + "id": "SC-005", + "criterion": "Required quality gate set QG-001/002/003/006/008/009 passes." + } + ], + "implementation_steps": [ + { + "id": "S1", + "title": "Helper capability extension", + "description": "Add optional JSON status/payload and forceJson flags to actionRequirePost/actionRequireCsrf without breaking existing callers." + }, + { + "id": "S2", + "title": "Remaining admin endpoint migration", + "description": "Refactor users/theme, users/switch-tenant, session-ping/data, tenants/ldap-test-connection/data, users/create, users/bulk, users/api-token-create, users/api-token-revoke, imports/index." + }, + { + "id": "S3", + "title": "Architecture hardening", + "description": "Add and run admin contract test to enforce helper-based guards in pages/admin going forward." + }, + { + "id": "S4", + "title": "Verification and gate execution", + "description": "Run targeted architecture checks and full required gates; capture metrics and run artifacts." + } + ], + "tests": [ + "docker compose exec -T php vendor/bin/phpunit tests/Architecture/AdminActionRequestGuardContractTest.php", + "docker compose exec -T php vendor/bin/phpunit tests/Architecture/PostEndpointCsrfContractTest.php tests/Architecture/PostRedirectGetContractTest.php", + "docker compose exec -T php vendor/bin/phpunit tests/Architecture/AuthzAdminSettingsContractTest.php tests/Architecture/AuthzAdminMasterDataContractTest.php tests/Architecture/AuthzAdminUsersContractTest.php", + "docker compose exec -T php vendor/bin/phpunit", + "docker compose exec -T php vendor/bin/phpstan analyse -c phpstan.neon --no-progress", + "docker compose exec -T php vendor/bin/phpunit tests/Architecture/CoreStarterkitContractTest.php", + "docker compose exec -T php vendor/bin/php-cs-fixer fix --config=tools/php-cs-fixer/.php-cs-fixer.dist.php --dry-run --diff --verbose", + "bin/docs-link-check.sh", + "bin/docs-drift-check.sh", + "bin/codex-skills-sync.sh --check" + ] +} diff --git a/.agents/runs/2026-04-24-action-csrf-centralization-step3/review-acceptance.json b/.agents/runs/2026-04-24-action-csrf-centralization-step3/review-acceptance.json new file mode 100644 index 0000000..cd7a59a --- /dev/null +++ b/.agents/runs/2026-04-24-action-csrf-centralization-step3/review-acceptance.json @@ -0,0 +1,44 @@ +{ + "task_id": "2026-04-24-action-csrf-centralization-step3", + "verdict": "pass", + "checked_criterion_ids": [ + "SC-001", + "SC-002", + "SC-003", + "SC-004", + "SC-005" + ], + "checks": [ + { + "criterion_id": "SC-001", + "criterion": "All remaining pages/admin inline method checks are replaced by actionRequirePost() or requestInput()->isMethod('POST') + actionRequireCsrf() patterns.", + "result": "pass", + "evidence": "Direct requestInput()->method() usage in pages/admin was reduced to 0." + }, + { + "criterion_id": "SC-002", + "criterion": "All remaining pages/admin inline Session::checkCsrfToken() checks are replaced by actionRequireCsrf().", + "result": "pass", + "evidence": "Direct Session::checkCsrfToken() usage in pages/admin was reduced to 0." + }, + { + "criterion_id": "SC-003", + "criterion": "New architecture contract prevents reintroduction of direct requestInput()->method() and Session::checkCsrfToken() in pages/admin.", + "result": "pass", + "evidence": "AdminActionRequestGuardContractTest added and passing." + }, + { + "criterion_id": "SC-004", + "criterion": "CSRF and PRG architecture tests remain green and authz admin contract suites remain green.", + "result": "pass", + "evidence": "PostEndpointCsrfContractTest, PostRedirectGetContractTest, and Authz admin suites pass." + }, + { + "criterion_id": "SC-005", + "criterion": "Required quality gate set QG-001/002/003/006/008/009 passes.", + "result": "pass", + "evidence": "All required gates passed in this run." + } + ], + "missing_or_wrong": [] +} diff --git a/.agents/runs/2026-04-24-action-csrf-centralization-step3/review-code.json b/.agents/runs/2026-04-24-action-csrf-centralization-step3/review-code.json new file mode 100644 index 0000000..3403c12 --- /dev/null +++ b/.agents/runs/2026-04-24-action-csrf-centralization-step3/review-code.json @@ -0,0 +1,18 @@ +{ + "task_id": "2026-04-24-action-csrf-centralization-step3", + "verdict": "pass", + "checked_guard_ids": [ + "GR-CORE-003", + "GR-CORE-012", + "GR-TEST-001" + ], + "checked_quality_gate_ids": [ + "QG-001", + "QG-002", + "QG-003", + "QG-006", + "QG-008", + "QG-009" + ], + "findings": [] +} diff --git a/.agents/runs/2026-04-24-action-csrf-centralization-step3/review-security.json b/.agents/runs/2026-04-24-action-csrf-centralization-step3/review-security.json new file mode 100644 index 0000000..5c1a29e --- /dev/null +++ b/.agents/runs/2026-04-24-action-csrf-centralization-step3/review-security.json @@ -0,0 +1,10 @@ +{ + "task_id": "2026-04-24-action-csrf-centralization-step3", + "verdict": "pass", + "checked_guard_ids": [ + "GR-SEC-001", + "GR-SEC-008", + "GR-SEC-009" + ], + "findings": [] +} diff --git a/core/Support/helpers/request.php b/core/Support/helpers/request.php index 872760e..727c55e 100644 --- a/core/Support/helpers/request.php +++ b/core/Support/helpers/request.php @@ -50,16 +50,22 @@ function flashFormErrors(FormErrors $errors, ?string $scope = null, string $keyP function actionRequirePost( string $redirect, bool $jsonAware = false, - string $jsonError = 'method_not_allowed' + string $jsonError = 'method_not_allowed', + int $jsonStatusCode = 405, + array $jsonPayload = [], + bool $forceJson = false ): bool { if (requestInput()->isMethod('POST')) { return true; } - if ($jsonAware && Request::wantsJson()) { + if ($jsonAware && ($forceJson || Request::wantsJson())) { header('Allow: POST'); - http_response_code(405); - Router::json(['error' => trim($jsonError) !== '' ? trim($jsonError) : 'method_not_allowed']); + http_response_code($jsonStatusCode); + $payload = $jsonPayload !== [] + ? $jsonPayload + : ['error' => trim($jsonError) !== '' ? trim($jsonError) : 'method_not_allowed']; + Router::json($payload); return false; } @@ -79,15 +85,21 @@ function actionRequireCsrf( bool $jsonAware = false, string $jsonError = 'csrf', bool $flashOnFailure = true, - bool $redirectOnFailure = true + bool $redirectOnFailure = true, + int $jsonStatusCode = 400, + array $jsonPayload = [], + bool $forceJson = false ): bool { if (Session::checkCsrfToken()) { return true; } - if ($jsonAware && Request::wantsJson()) { - http_response_code(400); - Router::json(['error' => trim($jsonError) !== '' ? trim($jsonError) : 'csrf']); + if ($jsonAware && ($forceJson || Request::wantsJson())) { + http_response_code($jsonStatusCode); + $payload = $jsonPayload !== [] + ? $jsonPayload + : ['error' => trim($jsonError) !== '' ? trim($jsonError) : 'csrf']; + Router::json($payload); return false; } diff --git a/pages/admin/imports/index().php b/pages/admin/imports/index().php index fff81e5..d200cec 100644 --- a/pages/admin/imports/index().php +++ b/pages/admin/imports/index().php @@ -57,7 +57,7 @@ $hasImportPermission = static function (string $type) use ($currentUserId, $impo if ($request->isMethod('POST')) { $stage = trim((string) $request->body('stage', '')); - if (!Session::checkCsrfToken()) { + if (!actionRequireCsrf('admin/imports', flashOnFailure: false, redirectOnFailure: false)) { $errorBag->addGlobal(t('Form expired, please try again')); } elseif ($stage === 'analyze') { if ($selectedType === '' || !$hasImportPermission($selectedType)) { diff --git a/pages/admin/session-ping/data().php b/pages/admin/session-ping/data().php index c28e647..a18de00 100644 --- a/pages/admin/session-ping/data().php +++ b/pages/admin/session-ping/data().php @@ -3,11 +3,13 @@ use MintyPHP\Http\SessionStoreInterface; use MintyPHP\Router; use MintyPHP\Service\Settings\SettingsSessionGateway; -use MintyPHP\Session; -if ((requestInput()->method()) !== 'POST') { - http_response_code(405); - Router::json(['ok' => false, 'error' => 'method_not_allowed']); +if (!actionRequirePost( + 'admin', + jsonAware: true, + jsonPayload: ['ok' => false, 'error' => 'method_not_allowed'], + forceJson: true +)) { return; } @@ -20,9 +22,15 @@ if ($currentUserId <= 0) { return; } -if (!Session::checkCsrfToken()) { - http_response_code(403); - Router::json(['ok' => false, 'error' => 'csrf']); +if (!actionRequireCsrf( + 'admin', + jsonAware: true, + jsonStatusCode: 403, + jsonPayload: ['ok' => false, 'error' => 'csrf'], + flashOnFailure: false, + redirectOnFailure: false, + forceJson: true +)) { return; } diff --git a/pages/admin/tenants/ldap-test-connection/data().php b/pages/admin/tenants/ldap-test-connection/data().php index 4699fb4..c3cf0a4 100644 --- a/pages/admin/tenants/ldap-test-connection/data().php +++ b/pages/admin/tenants/ldap-test-connection/data().php @@ -1,22 +1,32 @@ method() !== 'POST') { - http_response_code(405); - echo json_encode(['ok' => false, 'error' => 'Method not allowed']); +if (!actionRequirePost( + 'admin/tenants', + jsonAware: true, + jsonError: 'Method not allowed', + jsonPayload: ['ok' => false, 'error' => 'Method not allowed'], + forceJson: true +)) { return; } -if (!Session::checkCsrfToken()) { - http_response_code(403); - echo json_encode(['ok' => false, 'error' => 'CSRF token invalid']); +if (!actionRequireCsrf( + 'admin/tenants', + jsonAware: true, + jsonError: 'CSRF token invalid', + jsonStatusCode: 403, + jsonPayload: ['ok' => false, 'error' => 'CSRF token invalid'], + flashOnFailure: false, + redirectOnFailure: false, + forceJson: true +)) { return; } diff --git a/pages/admin/users/api-token-create($id).php b/pages/admin/users/api-token-create($id).php index ff02156..1681049 100644 --- a/pages/admin/users/api-token-create($id).php +++ b/pages/admin/users/api-token-create($id).php @@ -3,7 +3,6 @@ use MintyPHP\Http\SessionStoreInterface; use MintyPHP\Router; use MintyPHP\Service\Access\UserAuthorizationPolicy; -use MintyPHP\Session; use MintyPHP\Support\Flash; use MintyPHP\Support\Guard; @@ -20,6 +19,7 @@ $user = $uuid !== '' ? $userAccountService->findByUuid($uuid) : null; if (!$user) { Router::redirect('admin/users'); } +$redirectTarget = "admin/users/edit/{$uuid}"; $userId = (int) ($user['id'] ?? 0); $currentUserId = (int) ($session['user']['id'] ?? 0); @@ -32,27 +32,29 @@ if (!$decision->isAllowed()) { return; } -if (!Session::checkCsrfToken()) { - Flash::error(t('Form expired, please try again'), "admin/users/edit/{$uuid}", 'csrf_expired'); - Router::redirect("admin/users/edit/{$uuid}"); +if (!actionRequirePost($redirectTarget)) { + return; +} + +if (!actionRequireCsrf($redirectTarget, $redirectTarget, 'csrf_expired')) { return; } $name = trim((string) (requestInput()->bodyAll()['token_name'] ?? '')); if ($name === '') { - Flash::error(t('Token name is required'), "admin/users/edit/{$uuid}", 'api_token_error'); - Router::redirect("admin/users/edit/{$uuid}"); + Flash::error(t('Token name is required'), $redirectTarget, 'api_token_error'); + Router::redirect($redirectTarget); } $result = $apiTokenService->create($userId, $name, null, null, $currentUserId); if ($result['ok'] ?? false) { $sessionStore->set('api_token_created', ['token' => $result['token'], 'uuid' => $uuid]); - Flash::success(t('API token created. Copy it now — it will not be shown again.'), "admin/users/edit/{$uuid}", 'api_token_created'); + Flash::success(t('API token created. Copy it now — it will not be shown again.'), $redirectTarget, 'api_token_created'); } else { $error = $result['error'] ?? 'create_failed'; - Flash::error(t('Could not create API token: %s', $error), "admin/users/edit/{$uuid}", 'api_token_error'); + Flash::error(t('Could not create API token: %s', $error), $redirectTarget, 'api_token_error'); } -Router::redirect("admin/users/edit/{$uuid}"); +Router::redirect($redirectTarget); diff --git a/pages/admin/users/api-token-revoke($id).php b/pages/admin/users/api-token-revoke($id).php index 46c1164..ac4d58b 100644 --- a/pages/admin/users/api-token-revoke($id).php +++ b/pages/admin/users/api-token-revoke($id).php @@ -3,7 +3,6 @@ use MintyPHP\Http\SessionStoreInterface; use MintyPHP\Router; use MintyPHP\Service\Access\UserAuthorizationPolicy; -use MintyPHP\Session; use MintyPHP\Support\Flash; use MintyPHP\Support\Guard; @@ -18,6 +17,7 @@ $user = $uuid !== '' ? $userAccountService->findByUuid($uuid) : null; if (!$user) { Router::redirect('admin/users'); } +$redirectTarget = "admin/users/edit/{$uuid}"; $userId = (int) ($user['id'] ?? 0); $currentUserId = (int) ($session['user']['id'] ?? 0); @@ -30,9 +30,11 @@ if (!$decision->isAllowed()) { return; } -if (!Session::checkCsrfToken()) { - Flash::error(t('Form expired, please try again'), "admin/users/edit/{$uuid}", 'csrf_expired'); - Router::redirect("admin/users/edit/{$uuid}"); +if (!actionRequirePost($redirectTarget)) { + return; +} + +if (!actionRequireCsrf($redirectTarget, $redirectTarget, 'csrf_expired')) { return; } @@ -40,7 +42,7 @@ $tokenId = (int) (requestInput()->bodyAll()['token_id'] ?? 0); if ($tokenId > 0) { $apiTokenService->revoke($tokenId); - Flash::success(t('API token revoked'), "admin/users/edit/{$uuid}", 'api_token_revoked'); + Flash::success(t('API token revoked'), $redirectTarget, 'api_token_revoked'); } -Router::redirect("admin/users/edit/{$uuid}"); +Router::redirect($redirectTarget); diff --git a/pages/admin/users/bulk($action).php b/pages/admin/users/bulk($action).php index 0d39468..43205e9 100644 --- a/pages/admin/users/bulk($action).php +++ b/pages/admin/users/bulk($action).php @@ -4,14 +4,29 @@ use MintyPHP\Http\SessionStoreInterface; use MintyPHP\Router; use MintyPHP\Service\Access\UserAuthorizationPolicy; use MintyPHP\Service\User\UserAccessTemplateService; -use MintyPHP\Session; use MintyPHP\Support\Guard; $session = app(SessionStoreInterface::class)->all(); Guard::requireLogin(); -if (!Session::checkCsrfToken()) { - Router::json(['ok' => false, 'error' => 'csrf_expired']); +if (!actionRequirePost( + 'admin/users', + jsonAware: true, + jsonPayload: ['ok' => false, 'error' => 'method_not_allowed'], + forceJson: true +)) { + return; +} + +if (!actionRequireCsrf( + 'admin/users', + jsonAware: true, + jsonStatusCode: 200, + jsonPayload: ['ok' => false, 'error' => 'csrf_expired'], + flashOnFailure: false, + redirectOnFailure: false, + forceJson: true +)) { return; } diff --git a/pages/admin/users/create().php b/pages/admin/users/create().php index aaa972c..6dfa2b6 100644 --- a/pages/admin/users/create().php +++ b/pages/admin/users/create().php @@ -9,7 +9,6 @@ use MintyPHP\Service\Access\UiAccessService; use MintyPHP\Service\Access\UiCapabilityMap; use MintyPHP\Service\Access\UserAuthorizationPolicy; use MintyPHP\Service\CustomField\UserCustomFieldValueService; -use MintyPHP\Session; use MintyPHP\Support\Flash; use MintyPHP\Support\Guard; @@ -99,9 +98,7 @@ $form = [ 'active' => 1, ]; -if ($request->isMethod('POST') && !Session::checkCsrfToken()) { - Flash::error(t('Form expired, please try again'), $createTarget, 'csrf_expired'); - Router::redirect($createTarget); +if ($request->isMethod('POST') && !actionRequireCsrf($createTarget, $createTarget, 'csrf_expired')) { return; } diff --git a/pages/admin/users/switch-tenant().php b/pages/admin/users/switch-tenant().php index 332b22c..9151ed1 100644 --- a/pages/admin/users/switch-tenant().php +++ b/pages/admin/users/switch-tenant().php @@ -3,7 +3,6 @@ use MintyPHP\Http\Request; use MintyPHP\Http\SessionStoreInterface; use MintyPHP\Router; -use MintyPHP\Session; use MintyPHP\Support\Guard; $sessionStore = app(SessionStoreInterface::class); @@ -13,20 +12,17 @@ Guard::requireLogin(); $userTenantContextService = app(\MintyPHP\Service\User\UserTenantContextService::class); $errorBag = formErrors(); -if ((requestInput()->method()) !== 'POST') { - Router::redirect('admin'); +if (!actionRequirePost('admin')) { return; } -if (!Session::checkCsrfToken()) { - if (Request::wantsJson()) { - http_response_code(400); - Router::json(['ok' => false, 'error' => 'csrf']); - return; - } - $errorBag->addGlobal(t('Form expired, please try again')); - flashFormErrors($errorBag, 'admin', 'switch_tenant'); - Router::redirect('admin'); +if (!actionRequireCsrf( + 'admin', + 'admin', + 'switch_tenant', + jsonAware: true, + jsonPayload: ['ok' => false, 'error' => 'csrf'] +)) { return; } diff --git a/pages/admin/users/theme().php b/pages/admin/users/theme().php index 81a4d03..5db6b66 100644 --- a/pages/admin/users/theme().php +++ b/pages/admin/users/theme().php @@ -3,7 +3,6 @@ use MintyPHP\Http\Request; use MintyPHP\Http\SessionStoreInterface; use MintyPHP\Router; -use MintyPHP\Session; use MintyPHP\Support\Guard; $sessionStore = app(SessionStoreInterface::class); @@ -13,18 +12,16 @@ Guard::requireLogin(); $userAccountService = app(\MintyPHP\Service\User\UserAccountService::class); $errorBag = formErrors(); -if ((requestInput()->method()) !== 'POST') { - Router::redirect('admin'); +if (!actionRequirePost('admin')) { return; } -if (!Session::checkCsrfToken()) { - if (Request::wantsJson()) { - http_response_code(400); - Router::json(['ok' => false, 'error' => 'csrf']); - return; - } - Router::redirect('admin'); +if (!actionRequireCsrf( + 'admin', + jsonAware: true, + jsonPayload: ['ok' => false, 'error' => 'csrf'], + flashOnFailure: false +)) { return; } diff --git a/tests/Architecture/AdminActionRequestGuardContractTest.php b/tests/Architecture/AdminActionRequestGuardContractTest.php new file mode 100644 index 0000000..7cb5756 --- /dev/null +++ b/tests/Architecture/AdminActionRequestGuardContractTest.php @@ -0,0 +1,53 @@ +adminActionFiles() as $file) { + $content = $this->readProjectFile($file); + $this->assertStringNotContainsString('requestInput()->method()', $content, $file); + $this->assertStringNotContainsString('(requestInput()->method())', $content, $file); + $this->assertStringNotContainsString("strtoupper((string) (requestInput()->method()))", $content, $file); + } + } + + public function testAdminActionsDoNotInlineLegacyCsrfChecks(): void + { + foreach ($this->adminActionFiles() as $file) { + $content = $this->readProjectFile($file); + $this->assertStringNotContainsString('Session::checkCsrfToken()', $content, $file); + } + } + + /** + * @return array + */ + private function adminActionFiles(): array + { + $adminRoot = $this->projectRootPath() . '/pages/admin'; + $iterator = new RecursiveIteratorIterator(new RecursiveDirectoryIterator($adminRoot)); + $files = []; + + foreach ($iterator as $file) { + if (!$file->isFile() || $file->getExtension() !== 'php') { + continue; + } + + $relativePath = 'pages/admin/' . ltrim(str_replace($adminRoot, '', $file->getPathname()), '/'); + $files[] = $relativePath; + } + + sort($files); + $this->assertNotEmpty($files, 'Expected admin action files in pages/admin.'); + return $files; + } +}