From 4a421564c26f82d653f97bab5982a3ff3a190934 Mon Sep 17 00:00:00 2001 From: tp_dhu Date: Sat, 10 May 2025 08:50:41 +0100 Subject: [PATCH] implemented csrf --- .../Admin/TicketOptionsController.php | 10 +- app/controllers/AttachmentController.php | 6 +- app/controllers/AuthController.php | 4 + app/controllers/CommentController.php | 4 + app/controllers/KBController.php | 7 +- app/controllers/TagController.php | 3 +- app/controllers/ThemeController.php | 4 + app/controllers/TicketController.php | 21 ++- app/controllers/UserController.php | 3 +- app/extensions/CSRFHelper.php | 36 ++++ app/traits/CheckCSRF.php | 13 ++ app/ui/parts/clipboard.html | 171 +++++++++--------- app/ui/templates/layout.html | 3 +- app/ui/views/admin/priorities/create.html | 6 +- app/ui/views/attachment/index.html | 1 + app/ui/views/comments/view.html | 1 + app/ui/views/kb/create.html | 1 + app/ui/views/kb/edit.html | 1 + app/ui/views/login.html | 2 +- app/ui/views/tag/create.html | 1 + app/ui/views/ticket/create.html | 30 +-- app/ui/views/ticket/edit.html | 13 +- app/ui/views/user/edit.html | 3 +- 23 files changed, 219 insertions(+), 125 deletions(-) create mode 100644 app/extensions/CSRFHelper.php create mode 100644 app/traits/CheckCSRF.php diff --git a/app/controllers/Admin/TicketOptionsController.php b/app/controllers/Admin/TicketOptionsController.php index 924660f..09824a7 100644 --- a/app/controllers/Admin/TicketOptionsController.php +++ b/app/controllers/Admin/TicketOptionsController.php @@ -2,8 +2,13 @@ namespace Admin; +use CheckCSRF; + class TicketOptionsController extends \BaseController { + + use CheckCSRF; + public function listPriorities() { $this->requireLogin(); @@ -24,10 +29,12 @@ class TicketOptionsController extends \BaseController $this->renderView('views/admin/priorities/create.html'); } - public function createPriority() + public function createPriority($f3) { $this->requireLogin(); $this->requireAdmin(); // Added admin check + $this->checkCSRF($f3, '/admin/priority/create'); + $p = new \TicketPriority($this->getDB()); $p->name = $this->f3->get('POST.name'); $p->sort_order = $this->f3->get('POST.sort_order'); @@ -60,6 +67,7 @@ class TicketOptionsController extends \BaseController { $this->requireLogin(); $this->requireAdmin(); + $this->checkCSRF($f3, '/admin/priority/', $params['id'] . '/edit'); $priorityId = $params['id']; $model = new \TicketPriority($this->getDB()); diff --git a/app/controllers/AttachmentController.php b/app/controllers/AttachmentController.php index d744971..d62f00b 100644 --- a/app/controllers/AttachmentController.php +++ b/app/controllers/AttachmentController.php @@ -2,7 +2,7 @@ class AttachmentController { - use RequiresAuth; + use RequiresAuth, CheckCSRF; // list attachments public function index($f3){ @@ -33,6 +33,7 @@ class AttachmentController { // handle file upload public function upload($f3){ $this->check_access($f3); + $this->checkCSRF($f3, '/ticket/'.$f3->get('PARAMS.id')); // not ideal for AJAX $ticket_id = (int) $f3->get('PARAMS.id'); $uploaded_by = $f3->get('SESSION.user.id'); @@ -80,6 +81,9 @@ class AttachmentController { ); $f3->reroute('/ticket/'.$ticket_id.''); + + // ideal ajax response: + // $f3->json(['success' => true, 'message' => 'file upload success.', 'filename' => $original_name, 'version' => $new_version]); } // download attachment diff --git a/app/controllers/AuthController.php b/app/controllers/AuthController.php index 18918a9..fe3d049 100644 --- a/app/controllers/AuthController.php +++ b/app/controllers/AuthController.php @@ -2,6 +2,7 @@ class AuthController { + use CheckCSRF; public function showLoginForm($f3){ @@ -16,6 +17,9 @@ class AuthController { } public function login($f3){ + // CSRF + $this->checkCSRF($f3, '/login'); + $username = $f3->get('POST.username'); $password = $f3->get('POST.password'); diff --git a/app/controllers/CommentController.php b/app/controllers/CommentController.php index 2a237fa..d19cea2 100644 --- a/app/controllers/CommentController.php +++ b/app/controllers/CommentController.php @@ -2,6 +2,8 @@ class CommentController { + use CheckCSRF; + /** * Add a new comment to a ticket. * Expects POST data: comment (text) @@ -13,6 +15,8 @@ class CommentController { $f3->reroute('/login'); } + $this->checkCSRF($f3, '/ticket/' . $f3->get('PARAMS.id')); + $ticket_id = (int) $f3->get('PARAMS.id'); $comment_text = $f3->get('POST.comment'); $current_user_id = $f3->get('SESSION.user.id'); diff --git a/app/controllers/KBController.php b/app/controllers/KBController.php index c631ab4..b08d406 100644 --- a/app/controllers/KBController.php +++ b/app/controllers/KBController.php @@ -2,7 +2,7 @@ class KBController implements CRUD { - use RequiresAuth; + use RequiresAuth, CheckCSRF; public function index($f3){ @@ -66,6 +66,8 @@ class KBController implements CRUD { public function create($f3){ $this->check_access($f3); + $this->checkCSRF($f3, '/kb/create'); + $title = $f3->get('POST.title'); $content = $f3->get('POST.content'); $created_by = $f3->get('SESSION.user.id'); @@ -160,7 +162,10 @@ class KBController implements CRUD { * Handle POST to edit existing article */ public function update($f3){ + $this->check_access($f3); + $this->checkCSRF($f3, '/kb/' . $f3->get('PARAMS.id') . '/edit'); + $article_id = $f3->get('PARAMS.id'); $db = $f3->get('DB'); diff --git a/app/controllers/TagController.php b/app/controllers/TagController.php index f1046fc..d3dfc58 100644 --- a/app/controllers/TagController.php +++ b/app/controllers/TagController.php @@ -2,7 +2,7 @@ class TagController implements CRUD { - use RequiresAuth; + use RequiresAuth, CheckCSRF; /** * List all tags @@ -27,6 +27,7 @@ class TagController implements CRUD { public function create($f3){ $this->check_access($f3); + $this->checkCSRF($f3, '/tag/create'); $name = $f3->get('POST.name'); $color = $f3->get('POST.color'); diff --git a/app/controllers/ThemeController.php b/app/controllers/ThemeController.php index 81beeb8..dc8c5b7 100644 --- a/app/controllers/ThemeController.php +++ b/app/controllers/ThemeController.php @@ -2,8 +2,12 @@ class ThemeController { + use CheckCSRF; + function toggle($f3) { + $this->checkCSRF($f3, $f3->get('HEADERS.Referer') ?: '/'); + $current = $f3->get('SESSION.theme') ?: 'light'; $new_theme = ($current === 'light') ? 'dark' : 'light'; $f3->set('SESSION.theme', $new_theme); diff --git a/app/controllers/TicketController.php b/app/controllers/TicketController.php index 74f1d5d..1282c38 100644 --- a/app/controllers/TicketController.php +++ b/app/controllers/TicketController.php @@ -2,7 +2,7 @@ class TicketController extends BaseController implements CRUD { - use RequiresAuth; + use RequiresAuth, CheckCSRF; // list all tickts public function index($f3){ @@ -58,10 +58,15 @@ class TicketController extends BaseController implements CRUD { $priorities = (new TicketPriority($db))->findAll(); $statuses = (new TicketStatus($db))->findAll(); + // TODO: this needs moving into a model? + $users = $this->getDB()->exec('SELECT id, username, display_name FROM users ORDER BY display_name ASC'); + $users = array_merge([['id'=>'-1', 'display_name'=>'--']], $users); + $this->requireLogin(); $this->renderView('views/ticket/create.html',[ 'priorities' => $priorities, - 'statuses' => $statuses + 'statuses' => $statuses, + 'users' => $users ]); } @@ -70,6 +75,7 @@ class TicketController extends BaseController implements CRUD { public function create($f3){ $this->requireLogin(); + $this->checkCSRF($f3, '/ticket/create'); $data = [ 'title' => $this->f3->get('POST.title'), @@ -114,12 +120,16 @@ class TicketController extends BaseController implements CRUD { // dropdowns $priorities = (new TicketPriority($this->getDB()))->findAll(); $statuses = (new TicketStatus($this->getDB()))->findAll(); + // TODO: this needs moving into a model? + $users = $this->getDB()->exec('SELECT id, username, display_name FROM users ORDER BY display_name ASC'); + $users = array_merge([['id'=>'-1', 'display_name'=>'--']], $users); $this->renderView('views/ticket/edit.html',[ 'ticket' => $ticket, 'ticket_meta' => $ticket->getMeta(), 'priorities' => $priorities, - 'statuses' => $statuses + 'statuses' => $statuses, + 'users' => $users ] ); return; @@ -130,6 +140,7 @@ class TicketController extends BaseController implements CRUD { { $this->requireLogin(); + $this->checkCSRF($f3, '/ticket/create'); $ticket_id = $this->f3->get('PARAMS.id'); $ticket_mapper = new Ticket($this->getDB()); @@ -146,7 +157,8 @@ class TicketController extends BaseController implements CRUD { 'description' => $this->f3->get('POST.description'), 'priority_id' => $this->f3->get('POST.priority_id'), 'status_id' => $this->f3->get('POST.status_id'), - 'updated_by' => $this->f3->get('SESSION.user.id') + 'updated_by' => $this->f3->get('SESSION.user.id') , + 'assigned_to' => $this->f3->get('POST.assigned_to') ?: null ]; $ticket->updateTicket($data); @@ -162,6 +174,7 @@ class TicketController extends BaseController implements CRUD { // subtask public function addSubtask($f3){ $this->requireLogin(); + $this->checkCSRF($f3, '/ticket/create'); $parent_id = (int) $f3->get('PARAMS.id'); $child_id = (int) $f3->get('POST.child_ticket_id'); diff --git a/app/controllers/UserController.php b/app/controllers/UserController.php index 6748d3c..101a027 100644 --- a/app/controllers/UserController.php +++ b/app/controllers/UserController.php @@ -2,7 +2,7 @@ class UserController implements CRUD { - use RequiresAuth; + use RequiresAuth, CheckCSRF; // list all users (admin only) @@ -44,6 +44,7 @@ class UserController implements CRUD { public function update($f3){ $this->check_access($f3); + $this->checkCSRF($f3, '/user/' . $f3->get('PARAMS.id') . '/edit'); $user_id = (int) $f3->get('PARAMS.id'); $new_username = $f3->get('POST.username'); diff --git a/app/extensions/CSRFHelper.php b/app/extensions/CSRFHelper.php new file mode 100644 index 0000000..5f989dd --- /dev/null +++ b/app/extensions/CSRFHelper.php @@ -0,0 +1,36 @@ +exists('SESSION.' . self::TOKEN_NAME)) { + $token = bin2hex(random_bytes(32)); + $f3->set('SESSION.' . self::TOKEN_NAME, $token); + } + return $f3->get('SESSION.' . self::TOKEN_NAME); + } + + public static function verify(?string $submitted_token): bool { + $f3 = \Base::instance(); + $session_token = $f3->get('SESSION.' . self::TOKEN_NAME); + + if(empty($submitted_token) || empty($session_token)){ + return false; + } + + if(hash_equals($session_token, $submitted_token)){ + $f3->clear('SESSION.' . self::TOKEN_NAME); + return true; + } + + return false; + } + + public static function field(): string { + return ''; + } + +} \ No newline at end of file diff --git a/app/traits/CheckCSRF.php b/app/traits/CheckCSRF.php new file mode 100644 index 0000000..50783a5 --- /dev/null +++ b/app/traits/CheckCSRF.php @@ -0,0 +1,13 @@ +get('POST.' . \CSRFHelper::TOKEN_NAME))){ + $f3->set('SESSION.error', 'CSRF token validation failed.'); + $f3->reroute($reroute); + return; + } + } + +} \ No newline at end of file diff --git a/app/ui/parts/clipboard.html b/app/ui/parts/clipboard.html index e399501..794f79f 100644 --- a/app/ui/parts/clipboard.html +++ b/app/ui/parts/clipboard.html @@ -1,89 +1,98 @@
- - -
- Paste or drag an image here -
- -

- - + }); + + // drag and drop + area.addEventListener('dragover', (e) => { + e.preventDefault(); + area.classList.add('hover'); + }); + + area.addEventListener('dragLeave', () => { + area.classList.remove('hover'); + }); + + area.addEventListener('drop', (e) => { + e.preventDefault(); + area.classList.remove('hover'); + const files = e.dataTransfer.files; + for (let file of files) { + if (file.type.startsWith('image/')) { + uploadImage(file); + } + } + }); +
\ No newline at end of file diff --git a/app/ui/templates/layout.html b/app/ui/templates/layout.html index e9e83c1..305b5d4 100644 --- a/app/ui/templates/layout.html +++ b/app/ui/templates/layout.html @@ -55,7 +55,8 @@
-
+ + {{ \CSRFHelper::field() | raw }}