diff --git a/docs/administration.md b/docs/administration.md index 817556db3e..3c2cc9f035 100644 --- a/docs/administration.md +++ b/docs/administration.md @@ -57,6 +57,22 @@ permissions for groups in the same way as for users described in the topic above Please note that the permissions for a group are higher rated than the permissions on a user. To enable the group permissions, please set the permission level from _basic_ to _medium_ in the main configuration. +### 5.1.3 Category restrictions (Medium permission mode) + +Groups can be restricted per right to a set of categories (Admin → Groups → Category restrictions). The rules are: + +- **No restriction selected = the right applies to all categories.** +- A group restricted to categories X and Y cannot add, edit, translate, approve, or delete FAQs in other categories, and + the admin UI only shows the allowed categories. +- **Direct user rights always remain global** — category restrictions only apply to rights granted through groups. Grant + rights via groups if you want category-level control. +- Restrictions match exact categories; they are not inherited by subcategories. +- **Basic permission mode has no groups**, so category restrictions do not apply there; every right is global. +- CSV import and the AI translation endpoint are gated by the global `add_faq` / `translate_faq` rights; category checks + apply when the translated or imported content is saved. +- A blocked action returns HTTP 403 with a message naming the missing right and the category. +- FAQs without any category assignment can only be modified by users whose rights are not category-restricted. + ## 5.2 Content ### 5.2.1 Category Administration diff --git a/phpmyfaq/src/phpMyFAQ/Category/CategoryTreeRestrictionFilter.php b/phpmyfaq/src/phpMyFAQ/Category/CategoryTreeRestrictionFilter.php new file mode 100644 index 0000000000..ea8d0ea98c --- /dev/null +++ b/phpmyfaq/src/phpMyFAQ/Category/CategoryTreeRestrictionFilter.php @@ -0,0 +1,74 @@ + + * @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-08-10 + */ + +declare(strict_types=1); + +namespace phpMyFAQ\Category; + +/** + * Filters linear category trees down to the categories a user may act on, + * based on group-level category restrictions. Null means unrestricted. + */ +final class CategoryTreeRestrictionFilter +{ + /** + * @param array> $categoryTree Linear tree from Category::getCategoryTree() + * @param array|null $allowedCategoryIds Null = unrestricted + * @return array> + */ + public static function filter(array $categoryTree, ?array $allowedCategoryIds): array + { + if ($allowedCategoryIds === null) { + return $categoryTree; + } + + return array_values(array_filter($categoryTree, static fn(array $entry): bool => in_array( + needle: (int) $entry['id'], + haystack: $allowedCategoryIds, + strict: true, + ))); + } + + /** + * Filters a nested tree (map of categoryId => children map) as produced by + * Category\Order::getCategoryTree() and Category::buildAdminCategoryTree(). + * Removing a node removes its whole subtree. + * + * @param array $categoryTree + * @param array|null $allowedCategoryIds Null = unrestricted + * @return array + */ + public static function filterNested(array $categoryTree, ?array $allowedCategoryIds): array + { + if ($allowedCategoryIds === null) { + return $categoryTree; + } + + $filtered = []; + foreach ($categoryTree as $categoryId => $children) { + if (!in_array(needle: (int) $categoryId, haystack: $allowedCategoryIds, strict: true)) { + continue; + } + + $filtered[$categoryId] = is_array($children) + ? self::filterNested($children, $allowedCategoryIds) + : $children; + } + + return $filtered; + } +} diff --git a/phpmyfaq/src/phpMyFAQ/Category/Relation.php b/phpmyfaq/src/phpMyFAQ/Category/Relation.php index 39098fade1..d51d235d11 100644 --- a/phpmyfaq/src/phpMyFAQ/Category/Relation.php +++ b/phpmyfaq/src/phpMyFAQ/Category/Relation.php @@ -337,6 +337,7 @@ private function aggregateRecursively( * * @param int $faqId FAQ id * @param string $faqLang FAQ language + * @return array */ public function getCategories(int $faqId, string $faqLang): array { diff --git a/phpmyfaq/src/phpMyFAQ/Controller/AbstractController.php b/phpmyfaq/src/phpMyFAQ/Controller/AbstractController.php index 52bfe9c94a..ef2a0f5d0d 100644 --- a/phpmyfaq/src/phpMyFAQ/Controller/AbstractController.php +++ b/phpmyfaq/src/phpMyFAQ/Controller/AbstractController.php @@ -55,6 +55,7 @@ contact: new OA\Contact(name: 'phpMyFAQ Team', email: 'support@phpmyfaq.de'), )] #[OA\Server(url: 'https://localhost', description: 'Local dockerized server')] +/* @mago-expect lint:too-many-methods - permission guard methods grow with each enforced right; a dedicated guard trait is planned */ #[OA\License(name: 'Mozilla Public Licence 2.0', url: 'https://www.mozilla.org/MPL/2.0/')] abstract class AbstractController { @@ -356,6 +357,60 @@ protected function userHasPermission(PermissionType $permissionType): void } } + /** + * Ensures the user owns the permission in every given category. + * Direct user-rights remain global; in basic permission mode this is + * identical to userHasPermission(). + * + * Empty-list policy: when $categoryIds resolves to an empty list after + * filtering (e.g. an orphaned FAQ with no category relations), only users + * whose right is unrestricted (getAllowedCategoriesForRight() returns null) + * may proceed. A category-restricted user is denied with a ForbiddenException, + * because there is no category membership to verify against and allowing + * unrestricted access would defeat the restriction. + * + * @param int[] $categoryIds + * @throws UnauthorizedHttpException|ForbiddenException + */ + protected function userHasPermissionForCategories(PermissionType $permissionType, array $categoryIds): void + { + if (!$this->currentUser->isLoggedIn()) { + throw new UnauthorizedHttpException(challenge: 'User is not authenticated.'); + } + + $currentUser = $this->currentUser; + $categoryIds = array_filter(array_unique($categoryIds), static fn(int $id): bool => $id > 0); + + if ($categoryIds === []) { + $allowed = $currentUser->perm->getAllowedCategoriesForRight( + $currentUser->getUserId(), + $permissionType->value, + ); + if ($allowed !== null) { + throw new ForbiddenException(message: sprintf( + 'User has no "%s" permission for uncategorized content.', + $permissionType->name, + )); + } + + return; + } + + foreach ($categoryIds as $categoryId) { + if (!$currentUser->perm->hasPermissionForCategory( + $currentUser->getUserId(), + $permissionType->value, + $categoryId, + )) { + throw new ForbiddenException(message: sprintf( + 'User has no "%s" permission for category %d.', + $permissionType->name, + $categoryId, + )); + } + } + } + /** * Grants access when the user owns at least one of the given permissions. * diff --git a/phpmyfaq/src/phpMyFAQ/Controller/Administration/Api/CategoryController.php b/phpmyfaq/src/phpMyFAQ/Controller/Administration/Api/CategoryController.php index 42ba5c1b4b..fee78a77a0 100644 --- a/phpmyfaq/src/phpMyFAQ/Controller/Administration/Api/CategoryController.php +++ b/phpmyfaq/src/phpMyFAQ/Controller/Administration/Api/CategoryController.php @@ -24,6 +24,7 @@ use phpMyFAQ\Category\Order; use phpMyFAQ\Category\Permission; use phpMyFAQ\Category\Relation; +use phpMyFAQ\Controller\Exception\ForbiddenException; use phpMyFAQ\Core\Exception; use phpMyFAQ\Enums\AdminLogType; use phpMyFAQ\Enums\PermissionType; @@ -64,6 +65,8 @@ public function delete(Request $request): JsonResponse $categoryId = (int) ($data->categoryId ?? 0); $categoryLang = (string) ($data->language ?? ''); + $this->userHasPermissionForCategories(PermissionType::CATEGORY_DELETE, [$categoryId]); + [$currentAdminUser, $currentAdminGroups] = CurrentUser::getCurrentUserGroupId($this->currentUser); $category = new Category($this->configuration, [], false); @@ -157,6 +160,20 @@ public function updateOrder(Request $request): JsonResponse } $categoryId = (int) ($data->categoryId ?? 0); + + // Reordering persists the entire submitted tree (all categories), so a + // per-category check is not enough: the right must be unrestricted. + $allowedCategories = $this->currentUser->perm->getAllowedCategoriesForRight( + $this->currentUser->getUserId(), + PermissionType::CATEGORY_EDIT->value, + ); + if ($allowedCategories !== null) { + throw new ForbiddenException(message: sprintf( + 'User has no "%s" permission to reorder the category tree.', + PermissionType::CATEGORY_EDIT->name, + )); + } + $categoryTreeRaw = $data->categoryTree ?? []; $categoryTree = array_values(array_filter( is_array($categoryTreeRaw) ? $categoryTreeRaw : [], diff --git a/phpmyfaq/src/phpMyFAQ/Controller/Administration/Api/FaqController.php b/phpmyfaq/src/phpMyFAQ/Controller/Administration/Api/FaqController.php index 62e2754329..c7f5980222 100644 --- a/phpmyfaq/src/phpMyFAQ/Controller/Administration/Api/FaqController.php +++ b/phpmyfaq/src/phpMyFAQ/Controller/Administration/Api/FaqController.php @@ -123,6 +123,8 @@ public function create(Request $request): JsonResponse ? array_map(static fn(mixed $categoryId): int => (int) $categoryId, $rawCategories) : [(int) Filter::filterVar($rawCategories, FILTER_VALIDATE_INT)]; + $this->userHasPermissionForCategories(PermissionType::FAQ_ADD, $categories); + $language = Filter::filterVar($data->lang ?? '', FILTER_SANITIZE_SPECIAL_CHARS, ''); $tags = Filter::filterVar($data->tags ?? '', FILTER_SANITIZE_SPECIAL_CHARS, ''); $active = Filter::filterVar($data->active ?? 'no', FILTER_SANITIZE_SPECIAL_CHARS, 'no'); @@ -355,6 +357,11 @@ public function update(Request $request): JsonResponse : [(int) Filter::filterVar($rawCategories, FILTER_VALIDATE_INT)]; $faqLang = Filter::filterVar($data->lang ?? '', FILTER_SANITIZE_SPECIAL_CHARS, ''); + + $categoryRelation = new Relation($this->configuration, $category); + $currentCategoryIds = array_keys($categoryRelation->getCategories($faqId, $faqLang)); + $this->userHasPermissionForCategories(PermissionType::FAQ_EDIT, [...$categories, ...$currentCategoryIds]); + $tags = Filter::filterVar($data->tags ?? '', FILTER_SANITIZE_SPECIAL_CHARS, ''); $active = Filter::filterVar($data->active ?? 'no', FILTER_SANITIZE_SPECIAL_CHARS, 'no'); $sticky = Filter::filterVar($data->sticky ?? 'no', FILTER_SANITIZE_SPECIAL_CHARS, 'no'); @@ -442,7 +449,6 @@ public function update(Request $request): JsonResponse $faqId = $faqData->getId() ?? $faqId; - $categoryRelation = new Relation($this->configuration, $category); $categoryRelation->deleteByFaq($faqId, $faqLang); $categoryRelation->add($categories, $faqId, $faqLang); @@ -550,6 +556,8 @@ public function listByCategory(Request $request): JsonResponse $categoryId = (int) Filter::filterVar($request->attributes->get(key: 'categoryId'), FILTER_VALIDATE_INT); $language = Filter::filterVar($request->attributes->get(key: 'language'), FILTER_SANITIZE_SPECIAL_CHARS, ''); + $this->userHasPermissionForCategories(PermissionType::FAQ_EDIT, [$categoryId]); + $onlyInactive = Filter::filterVar( $request->query->get(key: 'only-inactive'), FILTER_VALIDATE_BOOLEAN, @@ -562,9 +570,10 @@ public function listByCategory(Request $request): JsonResponse return $this->json([ 'faqs' => $faq->getAllFaqsByCategory($categoryId, $onlyInactive, $onlyNew), - 'isAllowedToTranslate' => $this->currentUser?->perm->hasPermission( + 'isAllowedToTranslate' => $this->currentUser?->perm->hasPermissionForCategory( $this->currentUser->getUserId(), PermissionType::FAQ_TRANSLATE->value, + $categoryId, ), ], Response::HTTP_OK); } @@ -592,6 +601,15 @@ public function activate(Request $request): JsonResponse } if ($faqIds !== []) { + $activateCategory = new Category($this->configuration, [], withPermission: false); + $activateCategoryRelation = new Relation($this->configuration, $activateCategory); + foreach ($faqIds as $faqId) { + $this->userHasPermissionForCategories( + PermissionType::FAQ_APPROVE, + array_keys($activateCategoryRelation->getCategories($faqId, $faqLanguage)), + ); + } + $faq = new FaqAdministration($this->configuration); $success = false; @@ -637,6 +655,15 @@ public function sticky(Request $request): JsonResponse } if ($faqIds !== []) { + $stickyCategory = new Category($this->configuration, [], withPermission: false); + $stickyCategoryRelation = new Relation($this->configuration, $stickyCategory); + foreach ($faqIds as $faqId) { + $this->userHasPermissionForCategories( + PermissionType::FAQ_EDIT, + array_keys($stickyCategoryRelation->getCategories($faqId, $faqLanguage)), + ); + } + $faq = new FaqAdministration($this->configuration); $success = false; @@ -682,6 +709,13 @@ public function delete(Request $request): JsonResponse ], Response::HTTP_UNAUTHORIZED); } + $deleteCategory = new Category($this->configuration, [], withPermission: false); + $deleteCategoryRelation = new Relation($this->configuration, $deleteCategory); + $this->userHasPermissionForCategories( + PermissionType::FAQ_DELETE, + array_keys($deleteCategoryRelation->getCategories($faqId, $faqLanguage)), + ); + $this->adminLog->log($this->currentUser, AdminLogType::FAQ_DELETE->value . ':' . $faqId); try { diff --git a/phpmyfaq/src/phpMyFAQ/Controller/Administration/CategoryController.php b/phpmyfaq/src/phpMyFAQ/Controller/Administration/CategoryController.php index cc0fb7f0f0..8b1fe11f00 100644 --- a/phpmyfaq/src/phpMyFAQ/Controller/Administration/CategoryController.php +++ b/phpmyfaq/src/phpMyFAQ/Controller/Administration/CategoryController.php @@ -21,6 +21,7 @@ use phpMyFAQ\Administration\Category as AdminCategory; use phpMyFAQ\Category; +use phpMyFAQ\Category\CategoryTreeRestrictionFilter; use phpMyFAQ\Category\Image; use phpMyFAQ\Category\Language\CategoryLanguageService; use phpMyFAQ\Category\Order; @@ -77,6 +78,18 @@ public function index(Request $request): Response $categoryInfo = $category->getAllCategories(); + $allowedCategories = $this->currentUser->perm->getAllowedCategoriesForRight( + $currentUserId, + PermissionType::CATEGORY_EDIT->value, + ); + if ($allowedCategories !== null) { + $categoryInfo = array_filter( + $categoryInfo, + static fn(int $categoryId): bool => in_array($categoryId, $allowedCategories, strict: true), + ARRAY_FILTER_USE_KEY, + ); + } + $orderedCategories = $this->categoryOrder->getAllCategories(); $categoryTree = $this->categoryOrder->getCategoryTree($orderedCategories); @@ -85,6 +98,8 @@ public function index(Request $request): Response $categoryTree = $category->buildAdminCategoryTree($categoryInfo); } + $categoryTree = CategoryTreeRestrictionFilter::filterNested($categoryTree, $allowedCategories); + // Per-category translation state for the badge + popover (same source as hierarchy()) $categoryLanguageService = new CategoryLanguageService(); $allLanguages = $categoryLanguageService->getLanguagesInUse($this->configuration); @@ -163,6 +178,8 @@ public function addChild(Request $request): Response $parentId = (int) Filter::filterVar($request->attributes->get(key: 'parentId'), FILTER_VALIDATE_INT); + $this->userHasPermissionForCategories(PermissionType::CATEGORY_ADD, [$parentId]); + $templateVars = []; if ($this->currentUser->perm instanceof MediumPermission) { $templateVars = [ @@ -212,6 +229,9 @@ public function create(Request $request): Response $category->setGroups($currentAdminGroups); $parentId = (int) Filter::filterVar($request->request->get(key: 'parent_id'), FILTER_VALIDATE_INT); + + $this->userHasPermissionForCategories(PermissionType::CATEGORY_ADD, [$parentId]); + $categoryId = $this->configuration->getDb()->nextId(Database::getTablePrefix() . 'faqcategories', 'id'); $categoryLang = Filter::filterVar($request->request->get(key: 'lang'), FILTER_SANITIZE_SPECIAL_CHARS, ''); @@ -370,6 +390,8 @@ public function edit(Request $request): Response default: 0, ); + $this->userHasPermissionForCategories(PermissionType::CATEGORY_EDIT, [$categoryId]); + $category = new Category($this->configuration, [], withPermission: false); $category ->setUser($currentAdminUser) @@ -466,8 +488,13 @@ public function hierarchy(Request $request): Response $categoryLanguageService = new CategoryLanguageService(); $languages = $categoryLanguageService->getLanguagesInUse($this->configuration); // [code => name] + $categoryTree = CategoryTreeRestrictionFilter::filter($category->getCategoryTree(), $this->currentUser->perm->getAllowedCategoriesForRight( + $this->currentUser->getUserId(), + PermissionType::CATEGORY_EDIT->value, + )); + $translations = []; - foreach ($category->getCategoryTree() as $cat) { + foreach ($categoryTree as $cat) { $categoryTreeId = (int) ($cat['id'] ?? 0); // [code => name] $existing = $categoryLanguageService->getExistingTranslations($this->configuration, $categoryTreeId); @@ -490,7 +517,7 @@ public function hierarchy(Request $request): Response 'currentLanguage' => $currentLanguage, 'allLangs' => $languages, 'allLangCodes' => $languageCodes, - 'categoryTree' => $category->getCategoryTree(), + 'categoryTree' => $categoryTree, 'basePath' => $request->getBasePath(), 'faqlangcode' => $currentLangCode, 'msgCategoryRemark_overview' => Translation::get(key: 'msgCategoryRemark_overview'), @@ -525,6 +552,9 @@ public function translate(Request $request): Response $category->setGroups($currentAdminGroups); $categoryId = (int) Filter::filterVar($request->attributes->get(key: 'categoryId'), FILTER_VALIDATE_INT); + + $this->userHasPermissionForCategories(PermissionType::CATEGORY_EDIT, [$categoryId]); + $translateTo = Filter::filterVar($request->query->get(key: 'translateTo'), FILTER_SANITIZE_SPECIAL_CHARS); // Re-add permission arrays used in the template @@ -585,6 +615,9 @@ public function update(Request $request): Response $parentId = (int) Filter::filterVar($request->request->get(key: 'parent_id'), FILTER_VALIDATE_INT); $categoryId = (int) Filter::filterVar($request->request->get(key: 'id'), FILTER_VALIDATE_INT); + + $this->userHasPermissionForCategories(PermissionType::CATEGORY_EDIT, [$categoryId]); + $categoryLang = Filter::filterVar($request->request->get(key: 'catlang'), FILTER_SANITIZE_SPECIAL_CHARS, ''); $existingImage = Filter::filterVar( $request->request->get(key: 'existing_image'), diff --git a/phpmyfaq/src/phpMyFAQ/Controller/Administration/FaqController.php b/phpmyfaq/src/phpMyFAQ/Controller/Administration/FaqController.php index 8d38d60f85..a81f0c63ac 100644 --- a/phpmyfaq/src/phpMyFAQ/Controller/Administration/FaqController.php +++ b/phpmyfaq/src/phpMyFAQ/Controller/Administration/FaqController.php @@ -23,6 +23,7 @@ use phpMyFAQ\Administration\Revision; use phpMyFAQ\Attachment\AttachmentFactory; use phpMyFAQ\Category; +use phpMyFAQ\Category\CategoryTreeRestrictionFilter; use phpMyFAQ\Category\Relation; use phpMyFAQ\Comments; use phpMyFAQ\Core\Exception; @@ -101,7 +102,7 @@ public function index(Request $request): Response ...$this->getFooter(), 'csrfTokenSearch' => Token::getInstance($this->session)->getTokenInput('pmf-csrf-token'), 'csrfTokenOverview' => Token::getInstance($this->session)->getTokenString('pmf-csrf-token'), - 'categories' => $category->getCategoryTree(), + 'categories' => $this->getFilteredCategoryTree($category, PermissionType::FAQ_EDIT), 'numberOfRecords' => $categoryRelation->getNumberOfFaqsPerCategory(), 'numberOfComments' => $this->comments->getNumberOfCommentsByCategory(), ]); @@ -153,7 +154,7 @@ public function add(Request $request): Response 'openQuestionId' => 0, 'notifyUser' => '', 'notifyEmail' => '', - 'categoryTree' => $category->getCategoryTree(), + 'categoryTree' => $this->getFilteredCategoryTree($category, PermissionType::FAQ_ADD), 'selectedCategories' => $categories, 'languageOptions' => LanguageHelper::renderSelectLanguage($faqData['lang'], false, [], 'lang'), 'attachments' => [], @@ -196,6 +197,8 @@ public function addInCategory(Request $request): Response '', ); + $this->userHasPermissionForCategories(PermissionType::FAQ_ADD, [$categoryId]); + $this->categoryHelper->setCategory($category); $this->adminLog->log($this->currentUser, AdminLogType::FAQ_ADD->value); @@ -223,7 +226,7 @@ public function addInCategory(Request $request): Response 'openQuestionId' => 0, 'notifyUser' => '', 'notifyEmail' => '', - 'categoryTree' => $category->getCategoryTree(), + 'categoryTree' => $this->getFilteredCategoryTree($category, PermissionType::FAQ_ADD), 'selectedCategories' => $categoryId, 'languageOptions' => LanguageHelper::renderSelectLanguage($faqData['lang'], false, [], 'lang'), 'attachments' => [], @@ -276,10 +279,12 @@ public function edit(Request $request): Response $categoryRelation = new Relation($this->configuration, $category); - $this->adminLog->log($this->currentUser, AdminLogType::FAQ_EDIT->value . ':' . $faqId); - $categories = $categoryRelation->getCategories($faqId, $faqLanguage); + $this->userHasPermissionForCategories(PermissionType::FAQ_EDIT, array_keys($categories)); + + $this->adminLog->log($this->currentUser, AdminLogType::FAQ_EDIT->value . ':' . $faqId); + $this->faq->getFaq($faqId, null, true); $faqData = $this->faq->faqRecord; @@ -360,7 +365,7 @@ public function edit(Request $request): Response 'openQuestionId' => 0, 'notifyUser' => '', 'notifyEmail' => '', - 'categoryTree' => $category->getCategoryTree(), + 'categoryTree' => $this->getFilteredCategoryTree($category, PermissionType::FAQ_EDIT), 'selectedCategories' => $categories, 'languageOptions' => LanguageHelper::renderSelectLanguage($faqLanguage, false, [], 'lang'), 'attachments' => $attachmentList, @@ -432,7 +437,7 @@ public function copy(Request $request): Response 'openQuestionId' => 0, 'notifyUser' => '', 'notifyEmail' => '', - 'categoryTree' => $category->getCategoryTree(), + 'categoryTree' => $this->getFilteredCategoryTree($category, PermissionType::FAQ_ADD), 'selectedCategories' => $categories, 'languageOptions' => LanguageHelper::renderSelectLanguage($faqLanguage, false, [], 'lang'), 'attachments' => [], @@ -504,7 +509,7 @@ public function translate(Request $request): Response 'openQuestionId' => 0, 'notifyUser' => '', 'notifyEmail' => '', - 'categoryTree' => $category->getCategoryTree(), + 'categoryTree' => $this->getFilteredCategoryTree($category, PermissionType::FAQ_ADD), 'selectedCategories' => $categories, 'languageOptions' => LanguageHelper::renderSelectLanguage($faqLanguage, false, [], 'lang'), 'attachments' => [], @@ -587,7 +592,7 @@ public function answer(Request $request): Response 'openQuestionId' => 0, 'notifyUser' => $questionData['username'] ?? $this->currentUser->getUserData('display_name'), 'notifyEmail' => $questionData['email'] ?? $this->currentUser->getUserData('email'), - 'categoryTree' => $category->getCategoryTree(), + 'categoryTree' => $this->getFilteredCategoryTree($category, PermissionType::FAQ_ADD), 'selectedCategories' => $categories, 'languageOptions' => LanguageHelper::renderSelectLanguage($faqLanguage, false, [], 'lang'), 'attachments' => [], @@ -673,4 +678,18 @@ private function getBaseTemplateVars(): array 'ad_view_faq' => Translation::get(key: 'ad_view_faq'), ]; } + + /** + * Returns the category tree reduced to the categories the current user + * may exercise the given right in (null restrictions = full tree). + * + * @return array> + */ + private function getFilteredCategoryTree(Category $category, PermissionType $permissionType): array + { + return CategoryTreeRestrictionFilter::filter($category->getCategoryTree(), $this->currentUser->perm->getAllowedCategoriesForRight( + $this->currentUser->getUserId(), + $permissionType->value, + )); + } } diff --git a/phpmyfaq/src/phpMyFAQ/Permission/BasicPermission.php b/phpmyfaq/src/phpMyFAQ/Permission/BasicPermission.php index 85086c51f6..11a0a0b4a2 100644 --- a/phpmyfaq/src/phpMyFAQ/Permission/BasicPermission.php +++ b/phpmyfaq/src/phpMyFAQ/Permission/BasicPermission.php @@ -289,4 +289,21 @@ public function getUserGroups(int $userId): array { return []; } + + /** + * Basic mode has no groups and therefore no category restrictions: + * the check is identical to the global permission check. + */ + public function hasPermissionForCategory(int $userId, mixed $right, int $categoryId): bool + { + return $this->hasPermission($userId, $right); + } + + /** + * Basic mode has no groups and therefore no category restrictions. + */ + public function getAllowedCategoriesForRight(int $userId, mixed $right): ?array + { + return null; + } } diff --git a/phpmyfaq/src/phpMyFAQ/Permission/MediumPermission.php b/phpmyfaq/src/phpMyFAQ/Permission/MediumPermission.php index a03ec53ab2..9fa0d66e34 100644 --- a/phpmyfaq/src/phpMyFAQ/Permission/MediumPermission.php +++ b/phpmyfaq/src/phpMyFAQ/Permission/MediumPermission.php @@ -89,16 +89,7 @@ public function hasPermission(int $userId, mixed $right): bool return true; } - // get right id - if (!is_numeric($right) && is_string($right)) { - $right = $this->getRightId($right); - } - - if ($right instanceof PermissionType) { - $right = $this->getRightId($right->value); - } - - $rightId = (int) $right; + $rightId = $this->resolveRightId($right); // check user right and group right if ($this->checkUserGroupRight($userId, $rightId)) { @@ -518,16 +509,7 @@ public function hasPermissionForCategory( return true; } - // Resolve right to ID - if (!is_numeric($right) && is_string($right)) { - $right = $this->getRightId($right); - } - - if ($right instanceof PermissionType) { - $right = $this->getRightId($right->value); - } - - $rightId = (int) $right; + $rightId = $this->resolveRightId($right); // Check direct user right (always global, no category restriction) if ($this->checkUserRight($userId, $rightId)) { @@ -573,4 +555,64 @@ public function setCategoryRestrictions(int $groupId, int $rightId, array $categ { return $this->categoryPermissionRepository->setCategoryRestrictions($groupId, $rightId, $categoryIds); } + + /** + * Returns the category IDs in which the user may exercise the right. + * Null means unrestricted: superadmins, direct user-rights, and group + * grants without category restrictions always apply globally. An empty + * array means the user cannot exercise the right in any category. + * + * @param int $userId User ID + * @param mixed $right Right ID, right name, or PermissionType value + * @return array|null + * @throws Exception + */ + #[\Override] + public function getAllowedCategoriesForRight(int $userId, mixed $right): ?array + { + $currentUser = new CurrentUser($this->configuration); + $currentUser->getUserById($userId); + + if ($currentUser->isSuperAdmin()) { + return null; + } + + $rightId = $this->resolveRightId($right); + + if ($this->checkUserRight($userId, $rightId)) { + return null; + } + + $allowedCategories = []; + foreach ($this->getUserGroups($userId) as $groupId) { + if (!in_array($rightId, $this->getGroupRights($groupId), strict: true)) { + continue; + } + + $restrictions = $this->categoryPermissionRepository->getCategoryRestrictions($groupId, $rightId); + if ($restrictions === []) { + return null; + } + + $allowedCategories = [...$allowedCategories, ...$restrictions]; + } + + return array_values(array_unique($allowedCategories)); + } + + /** + * Resolves a right given as ID, name, or PermissionType to its right ID. + */ + private function resolveRightId(mixed $right): int + { + if (!is_numeric($right) && is_string($right)) { + $right = $this->getRightId($right); + } + + if ($right instanceof PermissionType) { + $right = $this->getRightId($right->value); + } + + return (int) $right; + } } diff --git a/phpmyfaq/src/phpMyFAQ/Permission/PermissionInterface.php b/phpmyfaq/src/phpMyFAQ/Permission/PermissionInterface.php index 6cec1ef968..51b95a052e 100644 --- a/phpmyfaq/src/phpMyFAQ/Permission/PermissionInterface.php +++ b/phpmyfaq/src/phpMyFAQ/Permission/PermissionInterface.php @@ -122,4 +122,27 @@ public function refuseAllUserRights(int $userId): bool; * @return array */ public function getUserGroups(int $userId): array; + + /** + * Returns true if the user owns the right within the given category. + * Permission modes without group support have no category restrictions, + * so this behaves exactly like hasPermission(). Direct user-rights are + * always global and are never limited by category restrictions. + * + * @param int $userId User ID + * @param mixed $right Right ID, right name, or PermissionType value + * @param int $categoryId Category ID + */ + public function hasPermissionForCategory(int $userId, mixed $right, int $categoryId): bool; + + /** + * Returns the category IDs in which the user may exercise the right, + * null if the right is unrestricted (applies to all categories), or an + * empty array if the user cannot exercise the right in any category. + * + * @param int $userId User ID + * @param mixed $right Right ID, right name, or PermissionType value + * @return array|null + */ + public function getAllowedCategoriesForRight(int $userId, mixed $right): ?array; } diff --git a/tests/phpMyFAQ/Administration/AdminMenuBuilderTest.php b/tests/phpMyFAQ/Administration/AdminMenuBuilderTest.php index 7f50d8a3f1..a9fda894cf 100644 --- a/tests/phpMyFAQ/Administration/AdminMenuBuilderTest.php +++ b/tests/phpMyFAQ/Administration/AdminMenuBuilderTest.php @@ -128,6 +128,15 @@ public function getUserGroups(int $userId): array return []; } + public function hasPermissionForCategory(int $userId, mixed $right, int $categoryId): bool + { + return $this->hasPermission($userId, $right); + } + + public function getAllowedCategoriesForRight(int $userId, mixed $right): ?array + { + return null; + } public function getAllUserRights(int $userId): array { diff --git a/tests/phpMyFAQ/Attachment/AttachmentServiceTest.php b/tests/phpMyFAQ/Attachment/AttachmentServiceTest.php index ec6d47c792..2913d0b6c8 100644 --- a/tests/phpMyFAQ/Attachment/AttachmentServiceTest.php +++ b/tests/phpMyFAQ/Attachment/AttachmentServiceTest.php @@ -450,6 +450,15 @@ public function getUserGroups(int $userId): array return []; } + public function hasPermissionForCategory(int $userId, mixed $right, int $categoryId): bool + { + return $this->hasPermission($userId, $right); + } + + public function getAllowedCategoriesForRight(int $userId, mixed $right): ?array + { + return null; + } public function getAllUserRights(int $userId): array { diff --git a/tests/phpMyFAQ/Category/CategoryTreeRestrictionFilterTest.php b/tests/phpMyFAQ/Category/CategoryTreeRestrictionFilterTest.php new file mode 100644 index 0000000000..954f48f665 --- /dev/null +++ b/tests/phpMyFAQ/Category/CategoryTreeRestrictionFilterTest.php @@ -0,0 +1,53 @@ +> */ + private array $tree = [ + ['id' => 1, 'name' => 'Allowed root'], + ['id' => 2, 'name' => 'Forbidden root'], + ['id' => 3, 'name' => 'Allowed child'], + ]; + + public function testNullMeansUnrestricted(): void + { + $this->assertSame($this->tree, CategoryTreeRestrictionFilter::filter($this->tree, null)); + } + + public function testKeepsOnlyAllowedCategories(): void + { + $filtered = CategoryTreeRestrictionFilter::filter($this->tree, [1, 3]); + $this->assertSame([1, 3], array_column($filtered, 'id')); + } + + public function testEmptyAllowListHidesEverything(): void + { + $this->assertSame([], CategoryTreeRestrictionFilter::filter($this->tree, [])); + } + + public function testNestedNullMeansUnrestricted(): void + { + $nested = [1 => [3 => []], 2 => []]; + $this->assertSame($nested, CategoryTreeRestrictionFilter::filterNested($nested, null)); + } + + public function testNestedKeepsOnlyAllowedBranches(): void + { + $nested = [1 => [3 => [], 4 => []], 2 => [5 => []]]; + $this->assertSame( + [1 => [3 => []]], + CategoryTreeRestrictionFilter::filterNested($nested, [1, 3, 5]), + ); + } + + public function testNestedEmptyAllowListHidesEverything(): void + { + $this->assertSame([], CategoryTreeRestrictionFilter::filterNested([1 => [], 2 => []], [])); + } +} diff --git a/tests/phpMyFAQ/Controller/Administration/Api/CategoryControllerTest.php b/tests/phpMyFAQ/Controller/Administration/Api/CategoryControllerTest.php index 3194ce08d4..ea24f1cc5e 100644 --- a/tests/phpMyFAQ/Controller/Administration/Api/CategoryControllerTest.php +++ b/tests/phpMyFAQ/Controller/Administration/Api/CategoryControllerTest.php @@ -8,6 +8,7 @@ use phpMyFAQ\Category\Order; use phpMyFAQ\Category\Permission; use phpMyFAQ\Configuration; +use phpMyFAQ\Controller\Exception\ForbiddenException; use phpMyFAQ\Core\Exception; use phpMyFAQ\Database; use phpMyFAQ\Database\Sqlite3; @@ -135,7 +136,17 @@ private function createControllerWithDependencies( private function createAuthenticatedContainer(?Session $session = null): ContainerInterface { - $permission = $this->createStub(PermissionInterface::class); + return $this->createAuthenticatedContainerWithAllowedCategories($session, null); + } + + /** + * @param array|null $allowedCategories Return value for getAllowedCategoriesForRight (null = unrestricted) + */ + private function createAuthenticatedContainerWithAllowedCategories( + ?Session $session, + ?array $allowedCategories, + ): ContainerInterface { + $permission = $this->createMock(PermissionInterface::class); $permission ->method('hasPermission') ->willReturnCallback( @@ -143,14 +154,31 @@ private function createAuthenticatedContainer(?Session $session = null): Contain && in_array( $right, [ + PermissionType::CATEGORY_ADD->value, PermissionType::CATEGORY_DELETE->value, PermissionType::CATEGORY_EDIT->value, ], true, ), ); + $permission + ->method('hasPermissionForCategory') + ->willReturnCallback( + static fn(int $userId, mixed $right, int $categoryId): bool => $userId === 42 + && in_array( + $right, + [ + PermissionType::CATEGORY_ADD->value, + PermissionType::CATEGORY_DELETE->value, + PermissionType::CATEGORY_EDIT->value, + ], + true, + ) + && $categoryId !== 666, // sentinel forbidden category for tests + ); + $permission->method('getAllowedCategoriesForRight')->willReturn($allowedCategories); - $currentUser = $this->createStub(CurrentUser::class); + $currentUser = $this->createMock(CurrentUser::class); $currentUser->perm = $permission; $currentUser->method('isLoggedIn')->willReturn(true); $currentUser->method('getUserId')->willReturn(42); @@ -426,6 +454,52 @@ public function testDeleteReturnsSuccessForValidCsrfWhenAuthenticated(): void $this->removeCsrfCookie('category'); } + /** + * @throws \Exception + */ + public function testDeleteReturnsForbiddenForRestrictedCategory(): void + { + $session = new Session(new MockArraySessionStorage()); + $csrfToken = Token::getInstance($session)->getTokenString('category'); + $this->setCsrfCookie('category', $csrfToken); + + $controller = $this->createController(); + $controller->setContainer($this->createAuthenticatedContainer($session)); + + $this->expectException(ForbiddenException::class); + $this->expectExceptionMessage('User has no "CATEGORY_DELETE" permission for category 666.'); + + $controller->delete(new Request([], [], [], [], [], [], json_encode([ + 'csrfToken' => $csrfToken, + 'categoryId' => 666, + 'language' => 'en', + ], JSON_THROW_ON_ERROR))); + + $this->removeCsrfCookie('category'); + } + + /** + * @throws \Exception + */ + public function testUpdateOrderReturnsForbiddenForCategoryRestrictedUser(): void + { + $session = new Session(new MockArraySessionStorage()); + $csrfToken = Token::getInstance($session)->getTokenString('category'); + $this->setCsrfCookie('category', $csrfToken); + + $controller = $this->createController(); + $controller->setContainer($this->createAuthenticatedContainerWithAllowedCategories($session, [10])); + + $this->expectException(ForbiddenException::class); + $this->expectExceptionMessage('User has no "CATEGORY_EDIT" permission to reorder the category tree.'); + + $controller->updateOrder(new Request([], [], [], [], [], [], json_encode([ + 'csrfToken' => $csrfToken, + 'categoryTree' => [['id' => 10, 'children' => []]], + 'categoryId' => 10, + ], JSON_THROW_ON_ERROR))); + } + private function setCsrfCookie(string $page, string $token): void { $_COOKIE['pmf-csrf-token-' . substr(md5($page), 0, 10)] = $token; diff --git a/tests/phpMyFAQ/Controller/Administration/Api/FaqControllerTest.php b/tests/phpMyFAQ/Controller/Administration/Api/FaqControllerTest.php index fef861cda3..2a7d8a888d 100644 --- a/tests/phpMyFAQ/Controller/Administration/Api/FaqControllerTest.php +++ b/tests/phpMyFAQ/Controller/Administration/Api/FaqControllerTest.php @@ -8,6 +8,7 @@ use phpMyFAQ\Administration\Changelog; use phpMyFAQ\Administration\Faq as FaqAdministration; use phpMyFAQ\Configuration; +use phpMyFAQ\Controller\Exception\ForbiddenException; use phpMyFAQ\Core\Exception; use phpMyFAQ\Database; use phpMyFAQ\Database\Sqlite3; @@ -182,6 +183,16 @@ private function createControllerWithDependencies( private function createAuthenticatedContainer(?Session $session = null): ContainerInterface { + return $this->createAuthenticatedContainerWithAllowedCategories($session, null); + } + + /** + * @param int[]|null $allowedCategories null = unrestricted, int[] = category-restricted + */ + private function createAuthenticatedContainerWithAllowedCategories( + ?Session $session, + ?array $allowedCategories, + ): ContainerInterface { $permission = $this->createMock(PermissionInterface::class); $permission ->method('hasPermission') @@ -199,6 +210,24 @@ private function createAuthenticatedContainer(?Session $session = null): Contain true, ), ); + $permission + ->method('hasPermissionForCategory') + ->willReturnCallback( + static fn(int $userId, mixed $right, int $categoryId): bool => $userId === 42 + && in_array( + $right, + [ + PermissionType::FAQ_ADD->value, + PermissionType::FAQ_EDIT->value, + PermissionType::FAQ_DELETE->value, + PermissionType::FAQ_APPROVE->value, + PermissionType::FAQ_TRANSLATE->value, + ], + true, + ) + && $categoryId !== 666, // sentinel forbidden category for tests + ); + $permission->method('getAllowedCategoriesForRight')->willReturn($allowedCategories); $currentUser = $this->createMock(CurrentUser::class); $currentUser->perm = $permission; @@ -439,6 +468,47 @@ public function testCreateReturnsUnauthorizedForInvalidCsrfWhenAuthenticated(): self::assertSame(Translation::get('msgNoPermission'), $payload['error']); } + /** + * @throws \Exception + */ + public function testCreateReturnsForbiddenForRestrictedCategory(): void + { + $session = new Session(new MockArraySessionStorage()); + $csrfToken = Token::getInstance($session)->getTokenString('pmf-csrf-token'); + $this->setCsrfCookie('pmf-csrf-token', $csrfToken); + + $request = new Request([], [], [], [], [], [], json_encode([ + 'data' => [ + 'pmf-csrf-token' => $csrfToken, + 'question' => 'Restricted question', + 'categories[]' => [666], + 'lang' => 'en', + 'tags' => '', + 'active' => 'yes', + 'answer' => 'Restricted answer', + 'keywords' => '', + 'author' => 'Author', + 'email' => 'author@example.com', + 'userpermission' => 'restricted', + 'restricted_users' => [], + 'grouppermission' => 'restricted', + 'restricted_groups' => [], + 'changed' => '', + 'notes' => '', + 'serpTitle' => '', + 'serpDescription' => '', + 'openQuestionId' => 0, + ], + ], JSON_THROW_ON_ERROR)); + + $controller = $this->createController(); + $controller->setContainer($this->createAuthenticatedContainer($session)); + + $this->expectException(ForbiddenException::class); + $this->expectExceptionMessage('User has no "FAQ_ADD" permission for category 666.'); + $controller->create($request); + } + /** * @throws \Exception */ @@ -1395,6 +1465,219 @@ public function testImportReturnsSuccessForValidCsvFile(): void $this->removeCsrfCookie('importfaqs'); } + /** + * @throws \Exception + */ + public function testUpdateReturnsForbiddenWhenTargetCategoryIsRestricted(): void + { + $session = new Session(new MockArraySessionStorage()); + $csrfToken = Token::getInstance($session)->getTokenString('pmf-csrf-token'); + $this->setCsrfCookie('pmf-csrf-token', $csrfToken); + + $request = new Request([], [], [], [], [], [], json_encode([ + 'data' => [ + 'pmf-csrf-token' => $csrfToken, + 'faqId' => 1, + 'solutionId' => 1, + 'revisionId' => 0, + 'question' => 'Updated question?', + 'categories[]' => [666], + 'lang' => 'en', + 'tags' => '', + 'active' => 'yes', + 'answer' => 'Updated answer', + 'keywords' => '', + 'author' => 'Author', + 'email' => 'author@example.com', + 'userpermission' => 'restricted', + 'restricted_users' => [], + 'grouppermission' => 'restricted', + 'restricted_groups' => [], + 'changed' => 'Updated', + 'date' => '2026-03-08 10:00:00', + 'notes' => '', + 'revision' => 'no', + 'recordDateHandling' => 'keepDate', + 'serpTitle' => '', + 'serpDescription' => '', + ], + ], JSON_THROW_ON_ERROR)); + + $controller = $this->createController(); + $controller->setContainer($this->createAuthenticatedContainer($session)); + + $this->expectException(ForbiddenException::class); + $this->expectExceptionMessage('User has no "FAQ_EDIT" permission for category 666.'); + $controller->update($request); + } + + /** + * @throws \Exception + */ + public function testDeleteReturnsForbiddenWhenFaqIsInRestrictedCategory(): void + { + $this->seedFaqRecord(categoryId: 666); + + $session = new Session(new MockArraySessionStorage()); + $csrfToken = Token::getInstance($session)->getTokenString('pmf-csrf-token'); + $this->setCsrfCookie('pmf-csrf-token', $csrfToken); + + $request = new Request([], [], [], [], [], [], json_encode([ + 'csrf' => $csrfToken, + 'faqId' => 1, + 'faqLanguage' => 'en', + ], JSON_THROW_ON_ERROR)); + + $controller = $this->createController(); + $controller->setContainer($this->createAuthenticatedContainer($session)); + + $this->expectException(ForbiddenException::class); + $this->expectExceptionMessage('User has no "FAQ_DELETE" permission for category 666.'); + $controller->delete($request); + } + + /** + * @throws \Exception + */ + public function testActivateReturnsForbiddenWhenFaqIsInRestrictedCategory(): void + { + $this->seedFaqRecord(categoryId: 666); + + $session = new Session(new MockArraySessionStorage()); + $csrfToken = Token::getInstance($session)->getTokenString('pmf-csrf-token'); + $this->setCsrfCookie('pmf-csrf-token', $csrfToken); + + $request = new Request([], [], [], [], [], [], json_encode([ + 'csrf' => $csrfToken, + 'faqIds' => [1], + 'faqLanguage' => 'en', + 'checked' => true, + ], JSON_THROW_ON_ERROR)); + + $controller = $this->createController(); + $controller->setContainer($this->createAuthenticatedContainer($session)); + + $this->expectException(ForbiddenException::class); + $this->expectExceptionMessage('User has no "FAQ_APPROVE" permission for category 666.'); + $controller->activate($request); + } + + /** + * @throws \Exception + */ + public function testStickyReturnsForbiddenWhenFaqIsInRestrictedCategory(): void + { + $this->seedFaqRecord(categoryId: 666); + + $session = new Session(new MockArraySessionStorage()); + $csrfToken = Token::getInstance($session)->getTokenString('pmf-csrf-token'); + $this->setCsrfCookie('pmf-csrf-token', $csrfToken); + + $request = new Request([], [], [], [], [], [], json_encode([ + 'csrf' => $csrfToken, + 'faqIds' => [1], + 'faqLanguage' => 'en', + 'checked' => true, + ], JSON_THROW_ON_ERROR)); + + $controller = $this->createController(); + $controller->setContainer($this->createAuthenticatedContainer($session)); + + $this->expectException(ForbiddenException::class); + $this->expectExceptionMessage('User has no "FAQ_EDIT" permission for category 666.'); + $controller->sticky($request); + } + + /** + * @throws \Exception + */ + public function testListByCategoryReturnsForbiddenForRestrictedCategory(): void + { + $controller = $this->createController(); + $controller->setContainer($this->createAuthenticatedContainer()); + + $this->expectException(ForbiddenException::class); + $this->expectExceptionMessage('User has no "FAQ_EDIT" permission for category 666.'); + $controller->listByCategory(new Request([], [], ['categoryId' => 666, 'language' => 'en'])); + } + + private function seedOrphanedFaqRecord(int $faqId = 1, string $language = 'en'): void + { + $this->configuration + ->getDb() + ->query(sprintf( + "INSERT INTO faqdata (id, lang, solution_id, revision_id, active, sticky, keywords, thema, content, author, email, comment, updated, date_start, date_end) + VALUES (%d, '%s', %d, 0, 'no', 0, '', 'Orphaned FAQ', 'Answer', 'Admin', 'admin@example.com', 'y', '20260301120000', '00000000000000', '99991231235959')", + $faqId, + $language, + $faqId + 1000, + )); + // Intentionally no faqcategoryrelations row — this is the orphaned-FAQ case. + } + + /** + * A category-restricted user must be denied when attempting to delete an + * orphaned FAQ (one with no category relations), because the guard cannot + * verify category membership and must err on the side of restriction. + * + * @throws \Exception + */ + public function testDeleteReturnsForbiddenForRestrictedUserWithOrphanedFaq(): void + { + $this->seedOrphanedFaqRecord(); + + $session = new Session(new MockArraySessionStorage()); + $csrfToken = Token::getInstance($session)->getTokenString('pmf-csrf-token'); + $this->setCsrfCookie('pmf-csrf-token', $csrfToken); + + $request = new Request([], [], [], [], [], [], json_encode([ + 'csrf' => $csrfToken, + 'faqId' => 1, + 'faqLanguage' => 'en', + ], JSON_THROW_ON_ERROR)); + + $controller = $this->createController(); + $controller->setContainer( + $this->createAuthenticatedContainerWithAllowedCategories($session, [10]), + ); + + $this->expectException(ForbiddenException::class); + $this->expectExceptionMessage('User has no "FAQ_DELETE" permission for uncategorized content.'); + $controller->delete($request); + } + + /** + * An unrestricted user (getAllowedCategoriesForRight returns null) must + * succeed when deleting an orphaned FAQ — the guard passes on empty lists + * only when the right is not category-restricted. + * + * @throws \Exception + */ + public function testDeleteSucceedsForUnrestrictedUserWithOrphanedFaq(): void + { + $this->seedOrphanedFaqRecord(); + + $session = new Session(new MockArraySessionStorage()); + $csrfToken = Token::getInstance($session)->getTokenString('pmf-csrf-token'); + $this->setCsrfCookie('pmf-csrf-token', $csrfToken); + + $request = new Request([], [], [], [], [], [], json_encode([ + 'csrf' => $csrfToken, + 'faqId' => 1, + 'faqLanguage' => 'en', + ], JSON_THROW_ON_ERROR)); + + $controller = $this->createController(); + $controller->setContainer($this->createAuthenticatedContainer($session)); + + $response = $controller->delete($request); + $payload = json_decode((string) $response->getContent(), true, 512, JSON_THROW_ON_ERROR); + + self::assertSame(Response::HTTP_OK, $response->getStatusCode()); + self::assertSame(Translation::get('ad_entry_delsuc'), $payload['success']); + $this->removeCsrfCookie('pmf-csrf-token'); + } + private function setCsrfCookie(string $page, string $token): void { $_COOKIE['pmf-csrf-token-' . substr(md5($page), 0, 10)] = $token; diff --git a/tests/phpMyFAQ/Controller/Administration/CategoryControllerTest.php b/tests/phpMyFAQ/Controller/Administration/CategoryControllerTest.php index 0ccfd21a8b..c03c9744d1 100644 --- a/tests/phpMyFAQ/Controller/Administration/CategoryControllerTest.php +++ b/tests/phpMyFAQ/Controller/Administration/CategoryControllerTest.php @@ -11,6 +11,7 @@ use phpMyFAQ\Category\Order; use phpMyFAQ\Category\Permission as CategoryPermission; use phpMyFAQ\Configuration; +use phpMyFAQ\Controller\Exception\ForbiddenException; use phpMyFAQ\Core\Exception; use phpMyFAQ\Database; use phpMyFAQ\Database\Sqlite3; @@ -366,6 +367,27 @@ public function testEditRendersCategoryDataAndSeoFields(): void self::assertStringContainsString('SEO description', (string) $response->getContent()); } + /** + * @throws \Exception + */ + public function testEditReturnsForbiddenForRestrictedCategory(): void + { + $controller = new CategoryController( + new AdminCategory($this->configuration), + $this->createStub(Order::class), + $this->createStub(CategoryPermission::class), + $this->createStub(Image::class), + $this->createStub(Seo::class), + $this->createStub(UserHelper::class), + ); + $controller->setContainer($this->createAuthenticatedContainer()); + + $this->expectException(ForbiddenException::class); + $this->expectExceptionMessage('User has no "CATEGORY_EDIT" permission for category 666.'); + + $controller->edit(new Request([], [], ['categoryId' => '666'])); + } + /** * @throws \Exception */ @@ -647,6 +669,13 @@ private function createAuthenticatedContext(string $groupsOptions = ''): array { $permission = $this->createMock(MediumPermission::class); $permission->method('hasPermission')->willReturn(true); + $permission + ->method('hasPermissionForCategory') + ->willReturnCallback( + // sentinel forbidden category for tests + static fn(int $userId, mixed $right, int $categoryId): bool => $categoryId !== 666, + ); + $permission->method('getAllowedCategoriesForRight')->willReturn(null); $permission->method('getAllGroupsOptions')->willReturn($groupsOptions); $currentUser = $this->createMock(CurrentUser::class); diff --git a/tests/phpMyFAQ/Controller/Administration/FaqControllerTest.php b/tests/phpMyFAQ/Controller/Administration/FaqControllerTest.php index b4dfd98954..0012dc0977 100644 --- a/tests/phpMyFAQ/Controller/Administration/FaqControllerTest.php +++ b/tests/phpMyFAQ/Controller/Administration/FaqControllerTest.php @@ -9,6 +9,7 @@ use phpMyFAQ\Administration\Changelog; use phpMyFAQ\Comments; use phpMyFAQ\Configuration; +use phpMyFAQ\Controller\Exception\ForbiddenException; use phpMyFAQ\Core\Exception; use phpMyFAQ\Database; use phpMyFAQ\Database\Sqlite3; @@ -197,6 +198,24 @@ public function testAddInCategoryRendersInCurrentAnonymousAdminContext(): void ); } + /** + * @throws \Exception + */ + public function testAddInCategoryReturnsForbiddenForRestrictedCategory(): void + { + $request = new Request(); + $request->attributes->set('categoryId', '666'); + $request->attributes->set('categoryLanguage', 'en'); + + $controller = $this->createController(); + $controller->setContainer($this->createAuthenticatedContainer()); + + $this->expectException(ForbiddenException::class); + $this->expectExceptionMessage('User has no "FAQ_ADD" permission for category 666.'); + + $controller->addInCategory($request); + } + /** * @throws \Exception */ @@ -353,6 +372,13 @@ private function createAuthenticatedContainer(): ContainerInterface { $permission = $this->createMock(PermissionInterface::class); $permission->method('hasPermission')->willReturn(true); + $permission + ->method('hasPermissionForCategory') + ->willReturnCallback( + // sentinel forbidden category for tests + static fn(int $userId, mixed $right, int $categoryId): bool => $categoryId !== 666, + ); + $permission->method('getAllowedCategoriesForRight')->willReturn(null); $currentUser = $this->createMock(CurrentUser::class); $currentUser->perm = $permission; diff --git a/tests/phpMyFAQ/Permission/BasicPermissionTest.php b/tests/phpMyFAQ/Permission/BasicPermissionTest.php index a0ea1af3a9..bf96c75182 100644 --- a/tests/phpMyFAQ/Permission/BasicPermissionTest.php +++ b/tests/phpMyFAQ/Permission/BasicPermissionTest.php @@ -185,4 +185,23 @@ public function testRefuseAllUserRights(): void $this->basicPermission->grantUserRight(2, 1); $this->assertTrue($this->basicPermission->refuseAllUserRights(2)); } + + public function testHasPermissionForCategoryDelegatesToGlobalCheck(): void + { + // Right 1 granted to user 1 in the fixture DB => category is irrelevant in basic mode + $this->assertSame( + $this->basicPermission->hasPermission(1, 1), + $this->basicPermission->hasPermissionForCategory(1, 1, 99), + ); + } + + public function testHasPermissionForCategoryDeniesWithoutGlobalRight(): void + { + $this->assertFalse($this->basicPermission->hasPermissionForCategory(0, 999, 1)); + } + + public function testGetAllowedCategoriesForRightIsAlwaysUnrestricted(): void + { + $this->assertNull($this->basicPermission->getAllowedCategoriesForRight(1, 1)); + } } diff --git a/tests/phpMyFAQ/Permission/MediumPermissionTest.php b/tests/phpMyFAQ/Permission/MediumPermissionTest.php index e0fe9103cc..50611f5682 100644 --- a/tests/phpMyFAQ/Permission/MediumPermissionTest.php +++ b/tests/phpMyFAQ/Permission/MediumPermissionTest.php @@ -621,6 +621,137 @@ public function testDeleteGroupCleansCategoryRestrictions(): void $this->assertEmpty($this->mediumPermission->getAllCategoryRestrictions(1)); } + /** + * @throws Exception + */ + public function testGetAllowedCategoriesForRightReturnsNullForUnrestrictedGroupRight(): void + { + $this->configuration->getDb()->query('UPDATE faquser SET is_superadmin = 0 WHERE user_id = 1'); + $this->configuration->getDb()->query('DELETE FROM faquser_right WHERE user_id = 1 AND right_id = 1'); + + $this->mediumPermission->addGroup(['name' => 'TestGroup', 'description' => 'Test', 'auto_join' => false]); + $this->mediumPermission->addToGroup(1, 1); + $this->mediumPermission->grantGroupRight(1, 1); + + $this->assertNull($this->mediumPermission->getAllowedCategoriesForRight(1, 1)); + + $this->mediumPermission->deleteGroup(1); + } + + /** + * @throws Exception + */ + public function testGetAllowedCategoriesForRightReturnsRestrictedCategories(): void + { + $this->configuration->getDb()->query('UPDATE faquser SET is_superadmin = 0 WHERE user_id = 1'); + $this->configuration->getDb()->query('DELETE FROM faquser_right WHERE user_id = 1 AND right_id = 1'); + + $this->mediumPermission->addGroup(['name' => 'TestGroup', 'description' => 'Test', 'auto_join' => false]); + $this->mediumPermission->addToGroup(1, 1); + $this->mediumPermission->grantGroupRight(1, 1); + $this->mediumPermission->setCategoryRestrictions(1, 1, [10, 20]); + + $this->assertSame([10, 20], $this->mediumPermission->getAllowedCategoriesForRight(1, 1)); + + $this->mediumPermission->deleteGroup(1); + } + + /** + * @throws Exception + */ + public function testGetAllowedCategoriesForRightReturnsNullForDirectUserRight(): void + { + // Fixture user 1 owns right 1 directly (faquser_right) => global, restrictions ignored + $this->configuration->getDb()->query('UPDATE faquser SET is_superadmin = 0 WHERE user_id = 1'); + + $this->assertNull($this->mediumPermission->getAllowedCategoriesForRight(1, 1)); + } + + /** + * @throws Exception + */ + public function testGetAllowedCategoriesForRightReturnsUnionAcrossGroups(): void + { + $this->configuration->getDb()->query('UPDATE faquser SET is_superadmin = 0 WHERE user_id = 1'); + $this->configuration->getDb()->query('DELETE FROM faquser_right WHERE user_id = 1 AND right_id = 1'); + + $this->mediumPermission->addGroup(['name' => 'TestGroupOne', 'description' => 'Test', 'auto_join' => false]); + $this->mediumPermission->addGroup(['name' => 'TestGroupTwo', 'description' => 'Test', 'auto_join' => false]); + $this->mediumPermission->addToGroup(1, 1); + $this->mediumPermission->addToGroup(1, 2); + $this->mediumPermission->grantGroupRight(1, 1); + $this->mediumPermission->grantGroupRight(2, 1); + $this->mediumPermission->setCategoryRestrictions(1, 1, [10, 20]); + $this->mediumPermission->setCategoryRestrictions(2, 1, [20, 30]); + + $this->assertSame([10, 20, 30], $this->mediumPermission->getAllowedCategoriesForRight(1, 1)); + + $this->mediumPermission->deleteGroup(1); + $this->mediumPermission->deleteGroup(2); + } + + /** + * @throws Exception + */ + public function testGetAllowedCategoriesForRightReturnsEmptyArrayWithoutAnyGrant(): void + { + $this->configuration->getDb()->query('UPDATE faquser SET is_superadmin = 0 WHERE user_id = 1'); + $this->configuration->getDb()->query('DELETE FROM faquser_right WHERE user_id = 1 AND right_id = 1'); + + $this->assertSame([], $this->mediumPermission->getAllowedCategoriesForRight(1, 1)); + } + + /** + * Pins that getAllowedCategoriesForRight() and hasPermissionForCategory() + * agree for every category: a category is in the allowed set (or the set + * is null) exactly when the per-category check grants it. + * + * @throws Exception + */ + public function testGetAllowedCategoriesForRightMatchesPerCategoryChecks(): void + { + $this->configuration->getDb()->query('UPDATE faquser SET is_superadmin = 0 WHERE user_id = 1'); + $this->configuration->getDb()->query('DELETE FROM faquser_right WHERE user_id = 1 AND right_id = 1'); + + $this->mediumPermission->addGroup(['name' => 'TestGroupOne', 'description' => 'Test', 'auto_join' => false]); + $this->mediumPermission->addGroup(['name' => 'TestGroupTwo', 'description' => 'Test', 'auto_join' => false]); + $this->mediumPermission->addToGroup(1, 1); + $this->mediumPermission->addToGroup(1, 2); + $this->mediumPermission->grantGroupRight(1, 1); + $this->mediumPermission->grantGroupRight(2, 1); + + $categories = [10, 20, 30, 40]; + + // Both groups restricted: allowed set is the union of the restrictions + $this->mediumPermission->setCategoryRestrictions(1, 1, [10, 20]); + $this->mediumPermission->setCategoryRestrictions(2, 1, [20, 30]); + $this->assertConsistentWithPerCategoryChecks(1, 1, $categories); + + // One group unrestricted: the right applies globally + $this->mediumPermission->setCategoryRestrictions(2, 1, []); + $this->assertNull($this->mediumPermission->getAllowedCategoriesForRight(1, 1)); + $this->assertConsistentWithPerCategoryChecks(1, 1, $categories); + + $this->mediumPermission->deleteGroup(1); + $this->mediumPermission->deleteGroup(2); + } + + /** + * @param array $categories + * @throws Exception + */ + private function assertConsistentWithPerCategoryChecks(int $userId, int $rightId, array $categories): void + { + $allowed = $this->mediumPermission->getAllowedCategoriesForRight($userId, $rightId); + foreach ($categories as $categoryId) { + $this->assertSame( + $allowed === null || in_array($categoryId, $allowed, strict: true), + $this->mediumPermission->hasPermissionForCategory($userId, $rightId, $categoryId), + sprintf('Mismatch for category %d (allowed: %s)', $categoryId, json_encode($allowed)), + ); + } + } + private function initializeDatabaseStatics(Sqlite3 $dbHandle): void { $databaseReflection = new ReflectionClass(Database::class);