fix: corrected and improved permission checks

thorsten · thorsten · commit 96a7777828d5 · 2026-04-10T11:51:02.000+02:00
diff --git a/phpmyfaq/admin/assets/src/api/tags.test.ts b/phpmyfaq/admin/assets/src/api/tags.test.ts
@@ -12,7 +12,7 @@ describe('Tags API', () => {
         { id: '1', name: 'Tag1' },
         { id: '2', name: 'Tag2' },
       ];
-      global.fetch = vi.fn(() =>
+      globalThis.fetch = vi.fn(() =>
         Promise.resolve({
           ok: true,
           json: () => Promise.resolve(mockResponse),
@@ -23,7 +23,7 @@ describe('Tags API', () => {
       const result = await fetchTags(searchString);
 
       expect(result).toEqual(mockResponse);
-      expect(global.fetch).toHaveBeenCalledWith('./api/content/tags?search=Tag', {
+      expect(globalThis.fetch).toHaveBeenCalledWith('./api/content/tags?search=Tag', {
         method: 'GET',
         cache: 'no-cache',
         headers: {
@@ -36,7 +36,7 @@ describe('Tags API', () => {
 
     it('should throw an error if fetch fails', async () => {
       const mockError = new Error('Fetch failed');
-      global.fetch = vi.fn(() => Promise.reject(mockError));
+      globalThis.fetch = vi.fn(() => Promise.reject(mockError));
 
       const searchString = 'Tag';
 
@@ -47,34 +47,37 @@ describe('Tags API', () => {
   describe('deleteTag', () => {
     it('should delete tag and return JSON response if successful', async () => {
       const mockResponse = { success: true, message: 'Tag deleted' };
-      global.fetch = vi.fn(() =>
+      globalThis.fetch = vi.fn(() =>
         Promise.resolve({
           ok: true,
           json: () => Promise.resolve(mockResponse),
         } as Response)
       );
 
       const tagId = '1';
-      await deleteTag(tagId);
+      const csrfToken = 'test-csrf';
+      await deleteTag(tagId, csrfToken);
 
-      expect(global.fetch).toHaveBeenCalledWith('./api/content/tags/1', {
+      expect(globalThis.fetch).toHaveBeenCalledWith('./api/content/tags/1', {
         method: 'DELETE',
         cache: 'no-cache',
         headers: {
           'Content-Type': 'application/json',
         },
+        body: JSON.stringify({ csrfToken: 'test-csrf' }),
         redirect: 'follow',
         referrerPolicy: 'no-referrer',
       });
     });
 
     it('should throw an error if fetch fails', async () => {
       const mockError = new Error('Fetch failed');
-      global.fetch = vi.fn(() => Promise.reject(mockError));
+      globalThis.fetch = vi.fn(() => Promise.reject(mockError));
 
       const tagId = '1';
+      const csrfToken = 'test-csrf';
 
-      await expect(deleteTag(tagId)).rejects.toThrow(mockError);
+      await expect(deleteTag(tagId, csrfToken)).rejects.toThrow(mockError);
     });
   });
 });
diff --git a/phpmyfaq/admin/assets/src/api/tags.ts b/phpmyfaq/admin/assets/src/api/tags.ts
@@ -32,13 +32,14 @@ export const fetchTags = async (searchString: string): Promise<TagResponse[]> =>
   return await response.json();
 };
 
-export const deleteTag = async (tagId: string): Promise<{ success?: string; error?: string }> => {
+export const deleteTag = async (tagId: string, csrfToken: string): Promise<{ success?: string; error?: string }> => {
   const response = await fetch(`./api/content/tags/${tagId}`, {
     method: 'DELETE',
     cache: 'no-cache',
     headers: {
       'Content-Type': 'application/json',
     },
+    body: JSON.stringify({ csrfToken: csrfToken }),
     redirect: 'follow',
     referrerPolicy: 'no-referrer',
   });
diff --git a/phpmyfaq/admin/assets/src/content/tags.ts b/phpmyfaq/admin/assets/src/content/tags.ts
@@ -62,8 +62,9 @@ export const handleTags = (): void => {
       element.addEventListener('click', async (event: Event) => {
         const target = event.target as HTMLElement;
         const tagId = target.getAttribute('data-pmf-id') as string;
+        const csrfToken = (document.querySelector('input[name=pmf-csrf-token]') as HTMLInputElement).value;
 
-        const response = await deleteTag(tagId);
+        const response = await deleteTag(tagId, csrfToken);
         if (response.success) {
           pushNotification(response.success);
           const row = document.getElementById(`pmf-row-tag-id-${tagId}`) as HTMLElement;
diff --git a/phpmyfaq/assets/templates/admin/configuration/instances.edit.twig b/phpmyfaq/assets/templates/admin/configuration/instances.edit.twig
@@ -9,6 +9,7 @@
     </h1>
   </div>
   <form action="./instance/update" method="post" accept-charset="utf-8">
+    <input type="hidden" name="pmf-csrf-token" value="{{ csrfTokenUpdateInstance }}"/>
     <input type="hidden" name="id" value="{{ instance.id }}"/>
     <div class="row mb-2">
       <label for="url" class="col-lg-2 col-form-label">{{ ad_instance_url }}:</label>
diff --git a/phpmyfaq/assets/templates/admin/user/group.twig b/phpmyfaq/assets/templates/admin/user/group.twig
@@ -46,6 +46,7 @@
           <i class="bi bi-info-circle" aria-hidden="true"></i> {{ 'ad_group_details' | translate }}
         </h5>
         <form action="./group/update" method="post">
+          <input type="hidden" name="pmf-csrf-token" value="{{ csrfTokenUpdateGroup }}">
           <input id="update_group_id" type="hidden" name="group_id" value="0">
           <div class="card-body">
             <div class="row mb-2">
@@ -91,6 +92,7 @@
 
     <div class="col-lg-4" id="groupMemberships">
       <form id="group_membership" name="group_membership" method="post" action="./group/update/members">
+        <input type="hidden" name="pmf-csrf-token" value="{{ csrfTokenUpdateGroupMembers }}">
         <input id="update_member_group_id" type="hidden" name="group_id" value="0">
         <div class="card shadow mb-4">
           <h5 class="card-header py-3">
@@ -170,6 +172,7 @@
 
       <div id="groupRights" class="card shadow mb-4">
         <form id="rightsForm" action="./group/update/permissions" method="post">
+          <input type="hidden" name="pmf-csrf-token" value="{{ csrfTokenUpdateGroupPermissions }}">
           <input id="rights_group_id" type="hidden" name="group_id" value="0">
           <h5 class="card-header py-3" id="user_rights_legend">
             <i aria-hidden="true" class="bi bi-lock"></i> {{ 'ad_group_rights' | translate }}
diff --git a/phpmyfaq/src/phpMyFAQ/Controller/AbstractController.php b/phpmyfaq/src/phpMyFAQ/Controller/AbstractController.php
@@ -161,7 +161,7 @@ protected function isSecured(): void
     /**
      * @throws UnauthorizedHttpException
      */
-    protected function userIsAuthenticated(): void
+    public function userIsAuthenticated(): void
     {
         if (!$this->currentUser->isLoggedIn()) {
             throw new UnauthorizedHttpException(challenge: 'User is not authenticated.');
diff --git a/phpmyfaq/src/phpMyFAQ/Controller/Administration/Api/CategoryController.php b/phpmyfaq/src/phpMyFAQ/Controller/Administration/Api/CategoryController.php
@@ -94,7 +94,7 @@ public function delete(Request $request): JsonResponse
     #[Route(path: 'admin/api/category/permissions', name: 'admin.api.category.permissions', methods: ['GET'])]
     public function permissions(Request $request): JsonResponse
     {
-        $this->userIsAuthenticated();
+        $this->userHasPermission(PermissionType::CATEGORY_EDIT);
 
         $categoryPermission = $this->container->get(id: 'phpmyfaq.category.permission');
 
@@ -119,7 +119,7 @@ public function permissions(Request $request): JsonResponse
     #[Route(path: 'admin/api/category/translations', name: 'admin.api.category.translations', methods: ['GET'])]
     public function translations(Request $request): JsonResponse
     {
-        $this->userIsAuthenticated();
+        $this->userHasPermission(PermissionType::CATEGORY_EDIT);
 
         $category = new Category($this->configuration, [], false);
 
diff --git a/phpmyfaq/src/phpMyFAQ/Controller/Administration/Api/MarkdownController.php b/phpmyfaq/src/phpMyFAQ/Controller/Administration/Api/MarkdownController.php
@@ -25,6 +25,7 @@
 use League\CommonMark\Extension\GithubFlavoredMarkdownExtension;
 use League\CommonMark\MarkdownConverter;
 use phpMyFAQ\Controller\AbstractController;
+use phpMyFAQ\Enums\PermissionType;
 use phpMyFAQ\Filter;
 use Symfony\Component\HttpFoundation\JsonResponse;
 use Symfony\Component\HttpFoundation\Request;
@@ -39,6 +40,8 @@ final class MarkdownController extends AbstractController
     #[Route(path: 'admin/api/content/markdown')]
     public function renderMarkdown(Request $request): JsonResponse
     {
+        $this->userHasPermission(PermissionType::FAQ_EDIT);
+
         $data = json_decode($request->getContent());
 
         $answer = Filter::filterVar($data->text, FILTER_SANITIZE_SPECIAL_CHARS);
diff --git a/phpmyfaq/src/phpMyFAQ/Controller/Administration/Api/StatisticsController.php b/phpmyfaq/src/phpMyFAQ/Controller/Administration/Api/StatisticsController.php
@@ -40,7 +40,7 @@ final class StatisticsController extends AbstractController
     #[Route(path: './admin/api/statistics/admin-log', methods: ['DELETE'])]
     public function deleteAdminLog(Request $request): JsonResponse
     {
-        $this->userHasPermission(PermissionType::STATISTICS_VIEWLOGS);
+        $this->userHasPermission(PermissionType::STATISTICS_ADMINLOG);
 
         $data = json_decode($request->getContent(), false, 512, JSON_THROW_ON_ERROR);
 
diff --git a/phpmyfaq/src/phpMyFAQ/Controller/Administration/Api/TagController.php b/phpmyfaq/src/phpMyFAQ/Controller/Administration/Api/TagController.php
@@ -116,6 +116,12 @@ public function delete(Request $request): JsonResponse
     {
         $this->userHasPermission(PermissionType::FAQ_EDIT);
 
+        $data = json_decode($request->getContent());
+
+        if (!Token::getInstance($this->container->get(id: 'session'))->verifyToken('tags', $data->csrfToken ?? '')) {
+            return $this->json(['error' => Translation::get(key: 'msgNoPermission')], Response::HTTP_UNAUTHORIZED);
+        }
+
         $tagId = (int) Filter::filterVar($request->attributes->get('tagId'), FILTER_VALIDATE_INT);
 
         if ($this->container->get(id: 'phpmyfaq.tags')->delete($tagId)) {
diff --git a/phpmyfaq/src/phpMyFAQ/Controller/Administration/AuthenticationController.php b/phpmyfaq/src/phpMyFAQ/Controller/Administration/AuthenticationController.php
@@ -30,7 +30,7 @@
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\Routing\Attribute\Route;
 
-final class AuthenticationController extends AbstractAdministrationController
+final class AuthenticationController extends AbstractAdministrationController implements SkipsAuthenticationCheck
 {
     #[Route(path: '/authenticate', name: 'admin.auth.authenticate', methods: ['POST'])]
     public function authenticate(Request $request): Response
diff --git a/phpmyfaq/src/phpMyFAQ/Controller/Administration/GroupController.php b/phpmyfaq/src/phpMyFAQ/Controller/Administration/GroupController.php
@@ -194,6 +194,11 @@ public function update(Request $request): Response
     {
         $this->userHasPermission(PermissionType::GROUP_EDIT);
 
+        $csrfToken = Filter::filterVar($request->request->get('pmf-csrf-token'), FILTER_SANITIZE_SPECIAL_CHARS);
+        if (!Token::getInstance($this->container->get(id: 'session'))->verifyToken('update-group', $csrfToken)) {
+            throw new UnauthorizedHttpException('Invalid CSRF token');
+        }
+
         $groupId = (int) Filter::filterVar($request->request->get('group_id'), FILTER_VALIDATE_INT);
 
         $groupData = [];
@@ -242,6 +247,14 @@ public function updateMembers(Request $request): Response
     {
         $this->userHasPermission(PermissionType::GROUP_EDIT);
 
+        $csrfToken = Filter::filterVar($request->request->get('pmf-csrf-token'), FILTER_SANITIZE_SPECIAL_CHARS);
+        if (!Token::getInstance($this->container->get(id: 'session'))->verifyToken(
+            'update-group-members',
+            $csrfToken,
+        )) {
+            throw new UnauthorizedHttpException('Invalid CSRF token');
+        }
+
         $groupId = (int) Filter::filterVar($request->request->get('group_id'), FILTER_VALIDATE_INT);
         $groupMembers = $request->request->all()['group_members'];
 
@@ -280,6 +293,14 @@ public function updatePermissions(Request $request): Response
     {
         $this->userHasPermission(PermissionType::GROUP_EDIT);
 
+        $csrfToken = Filter::filterVar($request->request->get('pmf-csrf-token'), FILTER_SANITIZE_SPECIAL_CHARS);
+        if (!Token::getInstance($this->container->get(id: 'session'))->verifyToken(
+            'update-group-permissions',
+            $csrfToken,
+        )) {
+            throw new UnauthorizedHttpException('Invalid CSRF token');
+        }
+
         $groupId = (int) Filter::filterVar($request->request->get('group_id'), FILTER_VALIDATE_INT);
         $groupPermissions = $request->request->all()['group_rights'];
 
@@ -317,6 +338,15 @@ private function getBaseTemplateVars(): array
         $user = $this->container->get(id: 'phpmyfaq.user');
         return [
             'rightData' => $user->perm->getAllRightsData(),
+            'csrfTokenUpdateGroup' => Token::getInstance($this->container->get(id: 'session'))->getTokenString(
+                'update-group',
+            ),
+            'csrfTokenUpdateGroupMembers' => Token::getInstance($this->container->get(id: 'session'))->getTokenString(
+                'update-group-members',
+            ),
+            'csrfTokenUpdateGroupPermissions' => Token::getInstance($this->container->get(
+                id: 'session',
+            ))->getTokenString('update-group-permissions'),
         ];
     }
 }
diff --git a/phpmyfaq/src/phpMyFAQ/Controller/Administration/InstanceController.php b/phpmyfaq/src/phpMyFAQ/Controller/Administration/InstanceController.php
@@ -28,6 +28,7 @@
 use phpMyFAQ\Translation;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\HttpKernel\Exception\UnauthorizedHttpException;
 use Symfony\Component\Routing\Attribute\Route;
 use Twig\Error\LoaderError;
 
@@ -77,6 +78,9 @@ public function edit(Request $request): Response
             'ad_instance_config' => Translation::get(key: 'ad_instance_config'),
             'ad_entry_back' => Translation::get(key: 'ad_entry_back'),
             'instance' => $instanceData,
+            'csrfTokenUpdateInstance' => Token::getInstance($this->container->get(id: 'session'))->getTokenString(
+                'update-instance',
+            ),
         ]);
     }
 
@@ -90,6 +94,11 @@ public function update(Request $request): Response
     {
         $this->userHasPermission(PermissionType::INSTANCE_EDIT);
 
+        $csrfToken = Filter::filterVar($request->request->get('pmf-csrf-token'), FILTER_SANITIZE_SPECIAL_CHARS);
+        if (!Token::getInstance($this->container->get(id: 'session'))->verifyToken('update-instance', $csrfToken)) {
+            throw new UnauthorizedHttpException('Invalid CSRF token');
+        }
+
         $instanceId = (int) Filter::filterVar($request->attributes->get('id'), FILTER_VALIDATE_INT);
 
         $fileSystem = new Filesystem(PMF_ROOT_DIR);
diff --git a/phpmyfaq/src/phpMyFAQ/Controller/Administration/PluginController.php b/phpmyfaq/src/phpMyFAQ/Controller/Administration/PluginController.php
@@ -20,6 +20,7 @@
 namespace phpMyFAQ\Controller\Administration;
 
 use phpMyFAQ\Core\Exception;
+use phpMyFAQ\Enums\PermissionType;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\Routing\Attribute\Route;
@@ -35,6 +36,8 @@ final class PluginController extends AbstractAdministrationController
     #[Route(path: '/plugins')]
     public function index(Request $request): Response
     {
+        $this->userHasPermission(PermissionType::CONFIGURATION_EDIT);
+
         $pluginManager = $this->container->get(id: 'phpmyfaq.plugin.plugin-manager');
         $pluginManager->loadPlugins();
 
diff --git a/phpmyfaq/src/phpMyFAQ/Controller/Administration/SkipsAuthenticationCheck.php b/phpmyfaq/src/phpMyFAQ/Controller/Administration/SkipsAuthenticationCheck.php
@@ -0,0 +1,28 @@
+<?php
+
+/**
+ * Marker interface for admin controllers that intentionally bypass the
+ * automatic authentication enforcement performed by the
+ * ControllerContainerListener.
+ *
+ * Only the AuthenticationController itself (handling login/logout/token
+ * endpoints) should implement this interface. Every other controller in
+ * the Administration namespace must require an authenticated user.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public License,
+ * v. 2.0. If a copy of the MPL was not distributed with this file, You can
+ * obtain one at https://mozilla.org/MPL/2.0/.
+ *
+ * @package   phpMyFAQ
+ * @author    Thorsten Rinne <thorsten@phpmyfaq.de>
+ * @copyright 2026 phpMyFAQ Team
+ * @license   https://www.mozilla.org/MPL/2.0/ Mozilla Public License Version 2.0
+ * @link      https://www.phpmyfaq.de
+ * @since     2026-04-10
+ */
+
+declare(strict_types=1);
+
+namespace phpMyFAQ\Controller\Administration;
+
+interface SkipsAuthenticationCheck {}
diff --git a/phpmyfaq/src/phpMyFAQ/Controller/Administration/TagController.php b/phpmyfaq/src/phpMyFAQ/Controller/Administration/TagController.php
@@ -20,6 +20,7 @@
 namespace phpMyFAQ\Controller\Administration;
 
 use phpMyFAQ\Core\Exception;
+use phpMyFAQ\Enums\PermissionType;
 use phpMyFAQ\Session\Token;
 use phpMyFAQ\Translation;
 use Symfony\Component\HttpFoundation\Request;
@@ -37,7 +38,7 @@ final class TagController extends AbstractAdministrationController
     #[Route(path: '/tags', name: 'admin.tags', methods: ['GET'])]
     public function index(Request $request): Response
     {
-        $this->userIsAuthenticated();
+        $this->userHasPermission(PermissionType::FAQ_EDIT);
 
         $tagData = $this->container->get(id: 'phpmyfaq.tags')->getAllTags();
 
diff --git a/phpmyfaq/src/phpMyFAQ/Controller/Administration/UserController.php b/phpmyfaq/src/phpMyFAQ/Controller/Administration/UserController.php
@@ -84,6 +84,10 @@ public function edit(Request $request): Response
     #[Route(path: '/user/list', name: 'admin.user.list', methods: ['GET'])]
     public function list(Request $request): Response
     {
+        $this->userHasPermission(PermissionType::USER_ADD);
+        $this->userHasPermission(PermissionType::USER_DELETE);
+        $this->userHasPermission(PermissionType::USER_EDIT);
+
         $user = $this->container->get(id: 'phpmyfaq.user');
         $allUsers = $user->getAllUsers(false);
         $numUsers = is_countable($allUsers) ? count($allUsers) : 0;

Original file line number	Diff line number	Diff line change
`@@ -161,7 +161,7 @@ protected function isSecured(): void`
`161`	`161`	`/**`
`162`	`162`	`* @throws UnauthorizedHttpException`
`163`	`163`	`*/`
`164`		`- protected function userIsAuthenticated(): void`
	`164`	`+ public function userIsAuthenticated(): void`
`165`	`165`	`{`
`166`	`166`	`if (!$this->currentUser->isLoggedIn()) {`
`167`	`167`	`throw new UnauthorizedHttpException(challenge: 'User is not authenticated.');`
Original file line number	Diff line number	Diff line change
`@@ -94,7 +94,7 @@ public function delete(Request $request): JsonResponse`
`94`	`94`	`#[Route(path: 'admin/api/category/permissions', name: 'admin.api.category.permissions', methods: ['GET'])]`
`95`	`95`	`public function permissions(Request $request): JsonResponse`
`96`	`96`	`{`
`97`		`- $this->userIsAuthenticated();`
	`97`	`+ $this->userHasPermission(PermissionType::CATEGORY_EDIT);`
`98`	`98`
`99`	`99`	`$categoryPermission = $this->container->get(id: 'phpmyfaq.category.permission');`
`100`	`100`
`@@ -119,7 +119,7 @@ public function permissions(Request $request): JsonResponse`
`119`	`119`	`#[Route(path: 'admin/api/category/translations', name: 'admin.api.category.translations', methods: ['GET'])]`
`120`	`120`	`public function translations(Request $request): JsonResponse`
`121`	`121`	`{`
`122`		`- $this->userIsAuthenticated();`
	`122`	`+ $this->userHasPermission(PermissionType::CATEGORY_EDIT);`
`123`	`123`
`124`	`124`	`$category = new Category($this->configuration, [], false);`
`125`	`125`
Original file line number	Diff line number	Diff line change
`@@ -40,7 +40,7 @@ final class StatisticsController extends AbstractController`
`40`	`40`	`#[Route(path: './admin/api/statistics/admin-log', methods: ['DELETE'])]`
`41`	`41`	`public function deleteAdminLog(Request $request): JsonResponse`
`42`	`42`	`{`
`43`		`- $this->userHasPermission(PermissionType::STATISTICS_VIEWLOGS);`
	`43`	`+ $this->userHasPermission(PermissionType::STATISTICS_ADMINLOG);`
`44`	`44`
`45`	`45`	`$data = json_decode($request->getContent(), false, 512, JSON_THROW_ON_ERROR);`
`46`	`46`