diff --git a/lib/Controller/AdminController.php b/lib/Controller/AdminController.php index 25ddcef..e9fc061 100644 --- a/lib/Controller/AdminController.php +++ b/lib/Controller/AdminController.php @@ -13,7 +13,6 @@ use OCA\Forum\Db\ForumUserMapper; use OCA\Forum\Db\PostMapper; use OCA\Forum\Db\RoleMapper; use OCA\Forum\Db\ThreadMapper; -use OCA\Forum\Db\UserRoleMapper; use OCA\Forum\Migration\SeedHelper; use OCA\Forum\Service\AdminSettingsService; use OCA\Forum\Service\UserRoleService; @@ -39,7 +38,6 @@ class AdminController extends OCSController { private ThreadMapper $threadMapper, private PostMapper $postMapper, private CategoryMapper $categoryMapper, - private UserRoleMapper $userRoleMapper, private RoleMapper $roleMapper, private UserRoleService $userRoleService, private IUserManager $userManager, @@ -216,25 +214,6 @@ class AdminController extends OCSController { } } - /** - * Check if user has admin role - */ - private function isUserAdmin(string $userId): bool { - try { - $userRoles = $this->userRoleMapper->findByUserId($userId); - foreach ($userRoles as $userRole) { - // Role ID 1 is Admin role (from migration) - if ($userRole->getRoleId() === 1) { - return true; - } - } - return false; - } catch (\Exception $e) { - $this->logger->warning('Error checking admin role: ' . $e->getMessage()); - return false; - } - } - /** * Run the repair seeds command to restore default forum data * diff --git a/lib/Controller/CategoryController.php b/lib/Controller/CategoryController.php index f467e29..3a83243 100644 --- a/lib/Controller/CategoryController.php +++ b/lib/Controller/CategoryController.php @@ -16,6 +16,7 @@ use OCA\Forum\Db\ReadMarkerMapper; use OCA\Forum\Db\Role; use OCA\Forum\Db\RoleMapper; use OCA\Forum\Db\ThreadMapper; +use OCA\Forum\Service\PermissionService; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\ApiRoute; @@ -37,6 +38,7 @@ class CategoryController extends OCSController { private ThreadMapper $threadMapper, private ReadMarkerMapper $readMarkerMapper, private RoleMapper $roleMapper, + private PermissionService $permissionService, private IUserSession $userSession, private LoggerInterface $logger, ) { @@ -60,9 +62,13 @@ class CategoryController extends OCSController { $allCategories = $this->categoryMapper->findAll(); $lastActivityMap = $this->threadMapper->getLastActivityByCategories(); + // Filter categories by canView permission + $user = $this->userSession->getUser(); + $userId = $user ? $user->getUID() : null; + $accessibleCategoryIds = $this->permissionService->getAccessibleCategories($userId); + // Fetch category read markers for authenticated users $readMarkerMap = []; - $user = $this->userSession->getUser(); if ($user) { $markers = $this->readMarkerMapper->findCategoryMarkersByUserId($user->getUID()); foreach ($markers as $marker) { @@ -70,9 +76,12 @@ class CategoryController extends OCSController { } } - // Group categories by header_id + // Group accessible categories by header_id $categoriesByHeader = []; foreach ($allCategories as $category) { + if (!in_array($category->getId(), $accessibleCategoryIds, true)) { + continue; + } $headerId = $category->getHeaderId(); if (!isset($categoriesByHeader[$headerId])) { $categoriesByHeader[$headerId] = []; @@ -83,11 +92,15 @@ class CategoryController extends OCSController { $categoriesByHeader[$headerId][] = $categoryData; } - // Build result with nested categories + // Build result with nested categories (only include headers that have accessible categories) $result = []; foreach ($headers as $header) { + $categories = $categoriesByHeader[$header->getId()] ?? []; + if (empty($categories)) { + continue; + } $headerData = $header->jsonSerialize(); - $headerData['categories'] = $categoriesByHeader[$header->getId()] ?? []; + $headerData['categories'] = $categories; $result[] = $headerData; } @@ -111,8 +124,13 @@ class CategoryController extends OCSController { #[ApiRoute(verb: 'GET', url: '/api/headers/{headerId}/categories')] public function byHeader(int $headerId): DataResponse { try { + $user = $this->userSession->getUser(); + $userId = $user ? $user->getUID() : null; + $accessibleCategoryIds = $this->permissionService->getAccessibleCategories($userId); + $categories = $this->categoryMapper->findByHeaderId($headerId); - return new DataResponse(array_map(fn ($cat) => $cat->jsonSerialize(), $categories)); + $filtered = array_filter($categories, fn ($cat) => in_array($cat->getId(), $accessibleCategoryIds, true)); + return new DataResponse(array_values(array_map(fn ($cat) => $cat->jsonSerialize(), $filtered))); } catch (\Exception $e) { $this->logger->error('Error fetching categories by header: ' . $e->getMessage()); return new DataResponse(['error' => 'Failed to fetch categories'], Http::STATUS_INTERNAL_SERVER_ERROR); @@ -337,39 +355,12 @@ class CategoryController extends OCSController { #[ApiRoute(verb: 'GET', url: '/api/categories/{id}/permissions/{permission}')] public function checkPermission(int $id, string $permission): DataResponse { try { - // Get current user $user = $this->userSession->getUser(); - if (!$user) { - return new DataResponse(['hasPermission' => false]); - } + $userId = $user?->getUID(); - $getter = 'get' . ucfirst($permission); + $hasPermission = $this->permissionService->hasCategoryPermission($userId, $id, $permission); - // Check role-based permissions - $roles = $this->roleMapper->findByUserId($user->getUID()); - $roleIds = array_map(fn ($role) => $role->getId(), $roles); - - if (!empty($roleIds)) { - // Admin role has all permissions - foreach ($roles as $role) { - if ($role->getRoleType() === Role::ROLE_TYPE_ADMIN) { - return new DataResponse(['hasPermission' => true]); - } - } - - $categoryPerms = $this->categoryPermMapper->findByCategoryAndRoles($id, $roleIds); - foreach ($categoryPerms as $perm) { - try { - if ($perm->$getter()) { - return new DataResponse(['hasPermission' => true]); - } - } catch (\BadMethodCallException $e) { - break; - } - } - } - - return new DataResponse(['hasPermission' => false]); + return new DataResponse(['hasPermission' => $hasPermission]); } catch (\Exception $e) { $this->logger->error("Error checking permission {$permission} for category {$id}: " . $e->getMessage()); return new DataResponse(['hasPermission' => false]); diff --git a/lib/Db/CategoryMapper.php b/lib/Db/CategoryMapper.php index b038c16..8fe2a65 100644 --- a/lib/Db/CategoryMapper.php +++ b/lib/Db/CategoryMapper.php @@ -12,8 +12,6 @@ use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\QBMapper; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; -use OCP\IUserSession; -use Psr\Log\LoggerInterface; /** * @template-extends QBMapper @@ -21,9 +19,6 @@ use Psr\Log\LoggerInterface; class CategoryMapper extends QBMapper { public function __construct( IDBConnection $db, - private IUserSession $userSession, - private RoleMapper $roleMapper, - private LoggerInterface $logger, ) { parent::__construct($db, Application::tableName('forum_categories'), Category::class); } @@ -60,117 +55,6 @@ class CategoryMapper extends QBMapper { return $this->findEntity($qb); } - /** - * Check if current user has admin role - * - * @return bool - */ - private function isCurrentUserAdmin(): bool { - $user = $this->userSession->getUser(); - if (!$user) { - return false; - } - - $roles = $this->roleMapper->findByUserId($user->getUID()); - foreach ($roles as $role) { - if ($role->getRoleType() === Role::ROLE_TYPE_ADMIN) { - return true; - } - } - return false; - } - - /** - * Get role IDs for the current user (or guest role if not authenticated) - * - * @return array - */ - private function getUserRoleIds(): array { - $user = $this->userSession->getUser(); - - if (!$user) { - // User not authenticated - use guest role if available - try { - $guestRole = $this->roleMapper->findByRoleType(Role::ROLE_TYPE_GUEST); - return [$guestRole->getId()]; - } catch (DoesNotExistException $e) { - // No guest role configured - this should never happen after migration - $this->logger->error('Guest role not found - guest access will not work. This indicates a migration issue.'); - return []; - } - } - - $roles = $this->roleMapper->findByUserId($user->getUID()); - return array_map(fn ($role) => $role->getId(), $roles); - } - - /** - * Filter categories by user permissions - * - * @param array $categories - * @return array - */ - private function filterByPermissions(array $categories): array { - if (empty($categories)) { - return []; - } - - // Admin role has access to all categories - skip filtering - if ($this->isCurrentUserAdmin()) { - return $categories; - } - - $userRoleIds = $this->getUserRoleIds(); - - $categoryIds = array_map(fn ($cat) => $cat->getId(), $categories); - - // Get all permissions for these categories - $qb = $this->db->getQueryBuilder(); - $qb->select('category_id', 'target_type', 'target_id', 'can_view') - ->from(Application::tableName('forum_category_perms')) - ->where($qb->expr()->in('category_id', $qb->createNamedParameter($categoryIds, IQueryBuilder::PARAM_INT_ARRAY))); - - $result = $qb->executeQuery(); - $permissions = []; - while ($row = $result->fetch()) { - $categoryId = (int)$row['category_id']; - if (!isset($permissions[$categoryId])) { - $permissions[$categoryId] = []; - } - $permissions[$categoryId][] = [ - 'target_type' => $row['target_type'], - 'target_id' => $row['target_id'], - 'can_view' => (bool)$row['can_view'], - ]; - } - $result->closeCursor(); - - $roleIdStrings = array_map('strval', $userRoleIds); - - // Filter categories based on permissions - return array_values(array_filter($categories, function ($category) use ($permissions, $roleIdStrings) { - $categoryId = $category->getId(); - - // If no permissions exist for this category, it's public - if (!isset($permissions[$categoryId]) || empty($permissions[$categoryId])) { - return true; - } - - // Check if user has any role with can_view permission - foreach ($permissions[$categoryId] as $perm) { - if (!$perm['can_view']) { - continue; - } - - if ($perm['target_type'] === CategoryPerm::TARGET_TYPE_ROLE - && in_array($perm['target_id'], $roleIdStrings, true)) { - return true; - } - } - - return false; - })); - } /** * @return array @@ -185,8 +69,7 @@ class CategoryMapper extends QBMapper { ->eq('header_id', $qb->createNamedParameter($headerId, IQueryBuilder::PARAM_INT)) ) ->orderBy('sort_order', 'ASC'); - $categories = $this->findEntities($qb); - return $this->filterByPermissions($categories); + return $this->findEntities($qb); } /** @@ -198,8 +81,7 @@ class CategoryMapper extends QBMapper { $qb->select('*') ->from($this->getTableName()) ->orderBy('sort_order', 'ASC'); - $categories = $this->findEntities($qb); - return $this->filterByPermissions($categories); + return $this->findEntities($qb); } /** diff --git a/lib/Service/PermissionService.php b/lib/Service/PermissionService.php index c232e33..780e82f 100644 --- a/lib/Service/PermissionService.php +++ b/lib/Service/PermissionService.php @@ -263,23 +263,6 @@ class PermissionService { $accessibleCategoryIds = []; try { - // Handle guest users (null userId) - use guest role - if ($userId === null) { - try { - $guestRole = $this->roleMapper->findByRoleType(Role::ROLE_TYPE_GUEST); - $roles = [$guestRole]; - } catch (DoesNotExistException $e) { - return []; - } - } else { - // Get all user roles using JOIN - $roles = $this->roleMapper->findByUserId($userId); - - if (empty($roles)) { - return []; - } - } - // Get all categories $categories = $this->categoryMapper->findAll(); @@ -289,7 +272,6 @@ class PermissionService { $accessibleCategoryIds[] = $category->getId(); } } - return $accessibleCategoryIds; } catch (\Exception $e) { $this->logger->error('Error getting accessible categories: ' . $e->getMessage()); @@ -305,14 +287,32 @@ class PermissionService { * @param string $getter Getter method name (e.g., 'getCanView') * @return bool True if user has the permission via a team */ - private function hasTeamCategoryPermission(string $userId, int $categoryId, string $getter): bool { + /** + * Get circle IDs for a user, with caching to avoid repeated Circles API calls + * + * @param string $userId Nextcloud user ID + * @return array|null Circle IDs, or null if Circles is unavailable + */ + /** + * Get circle IDs for a user, with caching to avoid repeated Circles API calls + * + * @param string $userId Nextcloud user ID + * @return array|null Circle IDs, or null if Circles is unavailable + */ + protected function getUserCircleIds(string $userId): ?array { + // Cache circle IDs per-request to avoid repeated Circles API calls + static $cache = []; + if (array_key_exists($userId, $cache)) { + return $cache[$userId]; + } + try { if (!class_exists(\OCA\Circles\CirclesManager::class)) { - return false; + $cache[$userId] = null; + return null; } $circlesManager = \OCP\Server::get(\OCA\Circles\CirclesManager::class); - $federatedUser = $circlesManager->getFederatedUser($userId, \OCA\Circles\Model\Member::TYPE_USER); $circlesManager->startSession($federatedUser); @@ -325,28 +325,37 @@ class PermissionService { ->filterSingleCircles(); $circles = $circlesManager->getCircles($probe); - $circleIds = array_map(fn ($c) => $c->getSingleId(), $circles); - - if (empty($circleIds)) { - return false; - } - - $teamPerms = $this->categoryPermMapper->findByCategoryAndTeamIds($categoryId, $circleIds); - foreach ($teamPerms as $perm) { - try { - if ($perm->$getter()) { - return true; - } - } catch (\BadMethodCallException $e) { - continue; - } - } + $cache[$userId] = array_map(fn ($c) => $c->getSingleId(), $circles); } finally { $circlesManager->stopSession(); } } catch (\Exception $e) { - // Circles app not available or other error - skip team permission check - $this->logger->debug('Team permission check skipped: ' . $e->getMessage()); + $this->logger->warning('Forum: Failed to get circles for user ' . $userId . ': ' . $e->getMessage()); + $cache[$userId] = null; + } + + return $cache[$userId]; + } + + private function hasTeamCategoryPermission(string $userId, int $categoryId, string $getter): bool { + $circleIds = $this->getUserCircleIds($userId); + if ($circleIds === null || empty($circleIds)) { + return false; + } + + try { + $teamPerms = $this->categoryPermMapper->findByCategoryAndTeamIds($categoryId, $circleIds); + foreach ($teamPerms as $perm) { + try { + if ($perm->$getter()) { + return true; + } + } catch (\BadMethodCallException $e) { + continue; + } + } + } catch (\Exception $e) { + $this->logger->warning('Forum: Team permission check failed for user ' . $userId . ' on category ' . $categoryId . ': ' . $e->getMessage()); } return false; diff --git a/tests/unit/Controller/CategoryControllerTest.php b/tests/unit/Controller/CategoryControllerTest.php index 6e82bf9..048c98c 100644 --- a/tests/unit/Controller/CategoryControllerTest.php +++ b/tests/unit/Controller/CategoryControllerTest.php @@ -17,6 +17,7 @@ use OCA\Forum\Db\ReadMarkerMapper; use OCA\Forum\Db\Role; use OCA\Forum\Db\RoleMapper; use OCA\Forum\Db\ThreadMapper; +use OCA\Forum\Service\PermissionService; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Http; use OCP\IRequest; @@ -40,6 +41,8 @@ class CategoryControllerTest extends TestCase { private ReadMarkerMapper $readMarkerMapper; /** @var RoleMapper&MockObject */ private RoleMapper $roleMapper; + /** @var PermissionService&MockObject */ + private PermissionService $permissionService; /** @var IUserSession&MockObject */ private IUserSession $userSession; /** @var LoggerInterface&MockObject */ @@ -55,6 +58,13 @@ class CategoryControllerTest extends TestCase { $this->threadMapper = $this->createMock(ThreadMapper::class); $this->readMarkerMapper = $this->createMock(ReadMarkerMapper::class); $this->roleMapper = $this->createMock(RoleMapper::class); + $this->permissionService = $this->createMock(PermissionService::class); + // By default, grant access to all categories (tests that need filtering can override) + $this->permissionService->method('getAccessibleCategories') + ->willReturnCallback(function () { + // Return IDs 1-100 to cover all test categories + return range(1, 100); + }); $this->userSession = $this->createMock(IUserSession::class); $this->logger = $this->createMock(LoggerInterface::class); @@ -67,6 +77,7 @@ class CategoryControllerTest extends TestCase { $this->threadMapper, $this->readMarkerMapper, $this->roleMapper, + $this->permissionService, $this->userSession, $this->logger ); @@ -592,31 +603,10 @@ class CategoryControllerTest extends TestCase { $user->method('getUID')->willReturn($userId); $this->userSession->method('getUser')->willReturn($user); - // User has a role - $role = new Role(); - $role->setId(1); - $role->setName('User'); - - $this->roleMapper->expects($this->once()) - ->method('findByUserId') - ->with($userId) - ->willReturn([$role]); - - // Category permission allows viewing - $categoryPerm = new CategoryPerm(); - $categoryPerm->setId(1); - $categoryPerm->setCategoryId($categoryId); - $categoryPerm->setTargetType('role'); - $categoryPerm->setTargetId('1'); - $categoryPerm->setCanView(true); - $categoryPerm->setCanPost(false); - $categoryPerm->setCanReply(false); - $categoryPerm->setCanModerate(false); - - $this->categoryPermMapper->expects($this->once()) - ->method('findByCategoryAndRoles') - ->with($categoryId, [1]) - ->willReturn([$categoryPerm]); + $this->permissionService->expects($this->once()) + ->method('hasCategoryPermission') + ->with($userId, $categoryId, $permission) + ->willReturn(true); $response = $this->controller->checkPermission($categoryId, $permission); @@ -634,29 +624,10 @@ class CategoryControllerTest extends TestCase { $user->method('getUID')->willReturn($userId); $this->userSession->method('getUser')->willReturn($user); - $role = new Role(); - $role->setId(1); - $role->setName('User'); - - $this->roleMapper->expects($this->once()) - ->method('findByUserId') - ->willReturn([$role]); - - // Category permission does not allow moderating - $categoryPerm = new CategoryPerm(); - $categoryPerm->setId(1); - $categoryPerm->setCategoryId($categoryId); - $categoryPerm->setTargetType('role'); - $categoryPerm->setTargetId('1'); - $categoryPerm->setCanView(true); - $categoryPerm->setCanPost(false); - $categoryPerm->setCanReply(false); - $categoryPerm->setCanModerate(false); - - $this->categoryPermMapper->expects($this->once()) - ->method('findByCategoryAndRoles') - ->with($categoryId, [1]) - ->willReturn([$categoryPerm]); + $this->permissionService->expects($this->once()) + ->method('hasCategoryPermission') + ->with($userId, $categoryId, $permission) + ->willReturn(false); $response = $this->controller->checkPermission($categoryId, $permission); @@ -674,16 +645,10 @@ class CategoryControllerTest extends TestCase { $user->method('getUID')->willReturn($userId); $this->userSession->method('getUser')->willReturn($user); - // User has Admin role - $adminRole = new Role(); - $adminRole->setId(1); - $adminRole->setName('Admin'); - $adminRole->setRoleType(Role::ROLE_TYPE_ADMIN); - - $this->roleMapper->expects($this->once()) - ->method('findByUserId') - ->with($userId) - ->willReturn([$adminRole]); + $this->permissionService->expects($this->once()) + ->method('hasCategoryPermission') + ->with($userId, $categoryId, $permission) + ->willReturn(true); $response = $this->controller->checkPermission($categoryId, $permission); diff --git a/tests/unit/Db/CategoryMapperTest.php b/tests/unit/Db/CategoryMapperTest.php index c09b95c..4a7f1aa 100644 --- a/tests/unit/Db/CategoryMapperTest.php +++ b/tests/unit/Db/CategoryMapperTest.php @@ -4,404 +4,25 @@ declare(strict_types=1); namespace OCA\Forum\Tests\Db; -use OCA\Forum\Db\Category; use OCA\Forum\Db\CategoryMapper; -use OCA\Forum\Db\Role; -use OCA\Forum\Db\RoleMapper; -use OCP\AppFramework\Db\DoesNotExistException; use OCP\IDBConnection; -use OCP\IUser; -use OCP\IUserSession; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; -use Psr\Log\LoggerInterface; class CategoryMapperTest extends TestCase { private CategoryMapper $mapper; /** @var IDBConnection&MockObject */ private IDBConnection $db; - /** @var IUserSession&MockObject */ - private IUserSession $userSession; - /** @var RoleMapper&MockObject */ - private RoleMapper $roleMapper; - /** @var LoggerInterface&MockObject */ - private LoggerInterface $logger; protected function setUp(): void { $this->db = $this->createMock(IDBConnection::class); - $this->userSession = $this->createMock(IUserSession::class); - $this->roleMapper = $this->createMock(RoleMapper::class); - $this->logger = $this->createMock(LoggerInterface::class); $this->mapper = new CategoryMapper( $this->db, - $this->userSession, - $this->roleMapper, - $this->logger ); } - public function testIsCurrentUserAdminReturnsTrueForAdminRole(): void { - $user = $this->createMock(IUser::class); - $user->method('getUID')->willReturn('admin1'); - - $this->userSession->method('getUser')->willReturn($user); - - $adminRole = new Role(); - $adminRole->setId(1); - $adminRole->setRoleType(Role::ROLE_TYPE_ADMIN); - - $this->roleMapper->expects($this->once()) - ->method('findByUserId') - ->with('admin1') - ->willReturn([$adminRole]); - - // Use reflection to call private method - $reflection = new \ReflectionClass($this->mapper); - $method = $reflection->getMethod('isCurrentUserAdmin'); - $method->setAccessible(true); - - $result = $method->invoke($this->mapper); - - $this->assertTrue($result); - } - - public function testIsCurrentUserAdminReturnsFalseForNonAdminRole(): void { - $user = $this->createMock(IUser::class); - $user->method('getUID')->willReturn('user1'); - - $this->userSession->method('getUser')->willReturn($user); - - $userRole = new Role(); - $userRole->setId(3); - $userRole->setRoleType(Role::ROLE_TYPE_DEFAULT); - - $this->roleMapper->expects($this->once()) - ->method('findByUserId') - ->with('user1') - ->willReturn([$userRole]); - - // Use reflection to call private method - $reflection = new \ReflectionClass($this->mapper); - $method = $reflection->getMethod('isCurrentUserAdmin'); - $method->setAccessible(true); - - $result = $method->invoke($this->mapper); - - $this->assertFalse($result); - } - - public function testIsCurrentUserAdminReturnsFalseWhenNotAuthenticated(): void { - $this->userSession->method('getUser')->willReturn(null); - - // Use reflection to call private method - $reflection = new \ReflectionClass($this->mapper); - $method = $reflection->getMethod('isCurrentUserAdmin'); - $method->setAccessible(true); - - $result = $method->invoke($this->mapper); - - $this->assertFalse($result); - } - - public function testGetUserRoleIdsReturnsGuestRoleForUnauthenticatedUser(): void { - $this->userSession->method('getUser')->willReturn(null); - - $guestRole = new Role(); - $guestRole->setId(4); - $guestRole->setRoleType(Role::ROLE_TYPE_GUEST); - - $this->roleMapper->expects($this->once()) - ->method('findByRoleType') - ->with('guest') - ->willReturn($guestRole); - - // Use reflection to call private method - $reflection = new \ReflectionClass($this->mapper); - $method = $reflection->getMethod('getUserRoleIds'); - $method->setAccessible(true); - - $result = $method->invoke($this->mapper); - - $this->assertEquals([4], $result); - } - - public function testGetUserRoleIdsReturnsEmptyArrayWhenGuestRoleNotFound(): void { - $this->userSession->method('getUser')->willReturn(null); - - $this->roleMapper->expects($this->once()) - ->method('findByRoleType') - ->with('guest') - ->willThrowException(new DoesNotExistException('Guest role not found')); - - // Expect logger to be called when guest role is not found - $this->logger->expects($this->once()) - ->method('error') - ->with($this->stringContains('Guest role not found')); - - // Use reflection to call private method - $reflection = new \ReflectionClass($this->mapper); - $method = $reflection->getMethod('getUserRoleIds'); - $method->setAccessible(true); - - $result = $method->invoke($this->mapper); - - $this->assertEquals([], $result); - } - - public function testGetUserRoleIdsReturnsUserRolesForAuthenticatedUser(): void { - $user = $this->createMock(IUser::class); - $user->method('getUID')->willReturn('user1'); - - $this->userSession->method('getUser')->willReturn($user); - - $role1 = new Role(); - $role1->setId(3); - $role1->setRoleType(Role::ROLE_TYPE_DEFAULT); - - $role2 = new Role(); - $role2->setId(5); - $role2->setRoleType(Role::ROLE_TYPE_CUSTOM); - - $this->roleMapper->expects($this->once()) - ->method('findByUserId') - ->with('user1') - ->willReturn([$role1, $role2]); - - // Use reflection to call private method - $reflection = new \ReflectionClass($this->mapper); - $method = $reflection->getMethod('getUserRoleIds'); - $method->setAccessible(true); - - $result = $method->invoke($this->mapper); - - $this->assertEquals([3, 5], $result); - } - - public function testFilterByPermissionsSkipsFilteringForAdminUser(): void { - $user = $this->createMock(IUser::class); - $user->method('getUID')->willReturn('admin1'); - - $this->userSession->method('getUser')->willReturn($user); - - $adminRole = new Role(); - $adminRole->setId(1); - $adminRole->setRoleType(Role::ROLE_TYPE_ADMIN); - - $this->roleMapper->expects($this->once()) - ->method('findByUserId') - ->with('admin1') - ->willReturn([$adminRole]); - - $category1 = new Category(); - $category1->setId(1); - - $category2 = new Category(); - $category2->setId(2); - - $categories = [$category1, $category2]; - - // Use reflection to call private method - $reflection = new \ReflectionClass($this->mapper); - $method = $reflection->getMethod('filterByPermissions'); - $method->setAccessible(true); - - $result = $method->invoke($this->mapper, $categories); - - // Admin should see all categories - $this->assertEquals($categories, $result); - } - - public function testFilterByPermissionsReturnsEmptyArrayWhenNoCategories(): void { - // Use reflection to call private method - $reflection = new \ReflectionClass($this->mapper); - $method = $reflection->getMethod('filterByPermissions'); - $method->setAccessible(true); - - $result = $method->invoke($this->mapper, []); - - $this->assertEquals([], $result); - } - - public function testIsCurrentUserAdminReturnsTrueForModeratorRole(): void { - $user = $this->createMock(IUser::class); - $user->method('getUID')->willReturn('mod1'); - - $this->userSession->method('getUser')->willReturn($user); - - $modRole = new Role(); - $modRole->setId(2); - $modRole->setRoleType(Role::ROLE_TYPE_MODERATOR); - - $adminRole = new Role(); - $adminRole->setId(1); - $adminRole->setRoleType(Role::ROLE_TYPE_ADMIN); - - // User has both moderator and admin roles - $this->roleMapper->expects($this->once()) - ->method('findByUserId') - ->with('mod1') - ->willReturn([$modRole, $adminRole]); - - // Use reflection to call private method - $reflection = new \ReflectionClass($this->mapper); - $method = $reflection->getMethod('isCurrentUserAdmin'); - $method->setAccessible(true); - - $result = $method->invoke($this->mapper); - - // Should return true because one of the roles is admin - $this->assertTrue($result); - } - - public function testGetUserRoleIdsReturnsMultipleRoles(): void { - $user = $this->createMock(IUser::class); - $user->method('getUID')->willReturn('user1'); - - $this->userSession->method('getUser')->willReturn($user); - - $role1 = new Role(); - $role1->setId(3); - $role1->setRoleType(Role::ROLE_TYPE_DEFAULT); - - $role2 = new Role(); - $role2->setId(5); - $role2->setRoleType(Role::ROLE_TYPE_CUSTOM); - - $role3 = new Role(); - $role3->setId(7); - $role3->setRoleType(Role::ROLE_TYPE_CUSTOM); - - $this->roleMapper->expects($this->once()) - ->method('findByUserId') - ->with('user1') - ->willReturn([$role1, $role2, $role3]); - - // Use reflection to call private method - $reflection = new \ReflectionClass($this->mapper); - $method = $reflection->getMethod('getUserRoleIds'); - $method->setAccessible(true); - - $result = $method->invoke($this->mapper); - - $this->assertEquals([3, 5, 7], $result); - } - - public function testIsCurrentUserAdminReturnsFalseWhenUserHasNoRoles(): void { - $user = $this->createMock(IUser::class); - $user->method('getUID')->willReturn('user1'); - - $this->userSession->method('getUser')->willReturn($user); - - $this->roleMapper->expects($this->once()) - ->method('findByUserId') - ->with('user1') - ->willReturn([]); - - // Use reflection to call private method - $reflection = new \ReflectionClass($this->mapper); - $method = $reflection->getMethod('isCurrentUserAdmin'); - $method->setAccessible(true); - - $result = $method->invoke($this->mapper); - - $this->assertFalse($result); - } - - public function testGetUserRoleIdsReturnsEmptyArrayWhenUserHasNoRoles(): void { - $user = $this->createMock(IUser::class); - $user->method('getUID')->willReturn('user1'); - - $this->userSession->method('getUser')->willReturn($user); - - $this->roleMapper->expects($this->once()) - ->method('findByUserId') - ->with('user1') - ->willReturn([]); - - // Use reflection to call private method - $reflection = new \ReflectionClass($this->mapper); - $method = $reflection->getMethod('getUserRoleIds'); - $method->setAccessible(true); - - $result = $method->invoke($this->mapper); - - $this->assertEquals([], $result); - } - - public function testIsCurrentUserAdminWithModeratorRoleOnly(): void { - $user = $this->createMock(IUser::class); - $user->method('getUID')->willReturn('mod1'); - - $this->userSession->method('getUser')->willReturn($user); - - $modRole = new Role(); - $modRole->setId(2); - $modRole->setRoleType(Role::ROLE_TYPE_MODERATOR); - - $this->roleMapper->expects($this->once()) - ->method('findByUserId') - ->with('mod1') - ->willReturn([$modRole]); - - // Use reflection to call private method - $reflection = new \ReflectionClass($this->mapper); - $method = $reflection->getMethod('isCurrentUserAdmin'); - $method->setAccessible(true); - - $result = $method->invoke($this->mapper); - - // Moderator is not admin - $this->assertFalse($result); - } - - public function testIsCurrentUserAdminWithDefaultRoleType(): void { - $user = $this->createMock(IUser::class); - $user->method('getUID')->willReturn('user1'); - - $this->userSession->method('getUser')->willReturn($user); - - $defaultRole = new Role(); - $defaultRole->setId(3); - $defaultRole->setRoleType(Role::ROLE_TYPE_DEFAULT); - - $this->roleMapper->expects($this->once()) - ->method('findByUserId') - ->with('user1') - ->willReturn([$defaultRole]); - - // Use reflection to call private method - $reflection = new \ReflectionClass($this->mapper); - $method = $reflection->getMethod('isCurrentUserAdmin'); - $method->setAccessible(true); - - $result = $method->invoke($this->mapper); - - $this->assertFalse($result); - } - - public function testIsCurrentUserAdminWithCustomRoleType(): void { - $user = $this->createMock(IUser::class); - $user->method('getUID')->willReturn('user1'); - - $this->userSession->method('getUser')->willReturn($user); - - $customRole = new Role(); - $customRole->setId(10); - $customRole->setRoleType(Role::ROLE_TYPE_CUSTOM); - - $this->roleMapper->expects($this->once()) - ->method('findByUserId') - ->with('user1') - ->willReturn([$customRole]); - - // Use reflection to call private method - $reflection = new \ReflectionClass($this->mapper); - $method = $reflection->getMethod('isCurrentUserAdmin'); - $method->setAccessible(true); - - $result = $method->invoke($this->mapper); - - $this->assertFalse($result); + public function testConstructor(): void { + $this->assertInstanceOf(CategoryMapper::class, $this->mapper); } } diff --git a/tests/unit/Service/PermissionServiceTest.php b/tests/unit/Service/PermissionServiceTest.php index 615be28..f8b750b 100644 --- a/tests/unit/Service/PermissionServiceTest.php +++ b/tests/unit/Service/PermissionServiceTest.php @@ -710,8 +710,12 @@ class PermissionServiceTest extends TestCase { } public function testGetAccessibleCategoriesForGuestUserWithNoGuestRole(): void { - $this->roleMapper->expects($this->once()) - ->method('findByRoleType') + $category1 = $this->createCategory(1, 'Category 1', 'category-1'); + + $this->categoryMapper->method('findAll') + ->willReturn([$category1]); + + $this->roleMapper->method('findByRoleType') ->with(Role::ROLE_TYPE_GUEST) ->willThrowException(new DoesNotExistException('Guest role not found')); @@ -722,12 +726,18 @@ class PermissionServiceTest extends TestCase { public function testGetAccessibleCategoriesReturnsEmptyWhenUserHasNoRoles(): void { $userId = 'user1'; + $category1 = $this->createCategory(1, 'Category 1', 'category-1'); - $this->roleMapper->expects($this->once()) - ->method('findByUserId') + $this->categoryMapper->method('findAll') + ->willReturn([$category1]); + + $this->roleMapper->method('findByUserId') ->with($userId) ->willReturn([]); + $this->categoryPermMapper->method('findByCategoryAndRole') + ->willThrowException(new DoesNotExistException('Not found')); + $result = $this->service->getAccessibleCategories($userId); $this->assertCount(0, $result); @@ -735,12 +745,6 @@ class PermissionServiceTest extends TestCase { public function testGetAccessibleCategoriesReturnsEmptyWhenNoCategoriesExist(): void { $userId = 'user1'; - $role = $this->createRole(1, 'User', false, false, false, false, Role::ROLE_TYPE_CUSTOM); - - $this->roleMapper->expects($this->once()) - ->method('findByUserId') - ->with($userId) - ->willReturn([$role]); $this->categoryMapper->expects($this->once()) ->method('findAll') @@ -754,9 +758,8 @@ class PermissionServiceTest extends TestCase { public function testGetAccessibleCategoriesHandlesExceptions(): void { $userId = 'user1'; - $this->roleMapper->expects($this->once()) - ->method('findByUserId') - ->with($userId) + $this->categoryMapper->expects($this->once()) + ->method('findAll') ->willThrowException(new \Exception('Database error')); $result = $this->service->getAccessibleCategories($userId); @@ -786,6 +789,378 @@ class PermissionServiceTest extends TestCase { return $role; } + // ---- Team (circle) permission tests ---- + + /** + * Create a PermissionService partial mock where getUserCircleIds is overridden + * to return the given circle IDs, allowing team permission paths to be tested + * without the Circles app installed. + * + * @param array|null $circleIds Circle IDs to return, or null for "Circles unavailable" + */ + private function createServiceWithCircleIds(?array $circleIds): PermissionService { + $service = $this->getMockBuilder(PermissionService::class) + ->setConstructorArgs([ + $this->userRoleMapper, + $this->roleMapper, + $this->categoryPermMapper, + $this->categoryMapper, + $this->threadMapper, + $this->postMapper, + $this->userManager, + $this->logger, + ]) + ->onlyMethods(['getUserCircleIds']) + ->getMock(); + + $service->method('getUserCircleIds') + ->willReturn($circleIds); + + return $service; + } + + public function testHasCategoryPermissionGrantedByTeamWhenRoleDenies(): void { + $userId = 'user1'; + $categoryId = 3; + + // User role denies view on this category + $role = $this->createRole(3, 'User', false, false, false, false, Role::ROLE_TYPE_DEFAULT); + $rolePerm = $this->createCategoryPerm(1, $categoryId, 3, false, false, false, false); + + // But team grants view + $teamPerm = $this->createTeamCategoryPerm(10, $categoryId, 'circle-abc', true, false, false, false); + + $this->roleMapper->method('findByUserId') + ->with($userId) + ->willReturn([$role]); + + $this->categoryPermMapper->method('findByCategoryAndRole') + ->willReturn($rolePerm); + + $this->categoryPermMapper->method('findByCategoryAndTeamIds') + ->with($categoryId, ['circle-abc']) + ->willReturn([$teamPerm]); + + $service = $this->createServiceWithCircleIds(['circle-abc']); + + $this->assertTrue($service->hasCategoryPermission($userId, $categoryId, 'canView')); + } + + public function testHasCategoryPermissionGrantedByTeamWhenNoRolePermEntryExists(): void { + $userId = 'user1'; + $categoryId = 3; + + // User role has no permission entry for this category at all + $role = $this->createRole(3, 'User', false, false, false, false, Role::ROLE_TYPE_DEFAULT); + + $teamPerm = $this->createTeamCategoryPerm(10, $categoryId, 'circle-abc', true, true, true, false); + + $this->roleMapper->method('findByUserId') + ->with($userId) + ->willReturn([$role]); + + $this->categoryPermMapper->method('findByCategoryAndRole') + ->willThrowException(new DoesNotExistException('Not found')); + + $this->categoryPermMapper->method('findByCategoryAndTeamIds') + ->with($categoryId, ['circle-abc']) + ->willReturn([$teamPerm]); + + $service = $this->createServiceWithCircleIds(['circle-abc']); + + $this->assertTrue($service->hasCategoryPermission($userId, $categoryId, 'canPost')); + } + + public function testHasCategoryPermissionDeniedByBothRoleAndTeam(): void { + $userId = 'user1'; + $categoryId = 3; + + $role = $this->createRole(3, 'User', false, false, false, false, Role::ROLE_TYPE_DEFAULT); + $rolePerm = $this->createCategoryPerm(1, $categoryId, 3, false, false, false, false); + + // Team also denies + $teamPerm = $this->createTeamCategoryPerm(10, $categoryId, 'circle-abc', false, false, false, false); + + $this->roleMapper->method('findByUserId') + ->with($userId) + ->willReturn([$role]); + + $this->categoryPermMapper->method('findByCategoryAndRole') + ->willReturn($rolePerm); + + $this->categoryPermMapper->method('findByCategoryAndTeamIds') + ->with($categoryId, ['circle-abc']) + ->willReturn([$teamPerm]); + + $service = $this->createServiceWithCircleIds(['circle-abc']); + + $this->assertFalse($service->hasCategoryPermission($userId, $categoryId, 'canView')); + } + + public function testHasCategoryPermissionTeamNotCheckedForGuestUser(): void { + $categoryId = 1; + + $guestRole = $this->createRole(4, 'Guest', false, false, false, true, Role::ROLE_TYPE_GUEST); + $rolePerm = $this->createCategoryPerm(1, $categoryId, 4, false, false, false, false); + + $this->roleMapper->method('findByRoleType') + ->with(Role::ROLE_TYPE_GUEST) + ->willReturn($guestRole); + + $this->categoryPermMapper->method('findByCategoryAndRole') + ->willReturn($rolePerm); + + // Team mapper should never be called for guest users + $this->categoryPermMapper->expects($this->never()) + ->method('findByCategoryAndTeamIds'); + + $result = $this->service->hasCategoryPermission(null, $categoryId, 'canView'); + + $this->assertFalse($result); + } + + public function testHasCategoryPermissionTeamNotCheckedWhenCirclesUnavailable(): void { + $userId = 'user1'; + $categoryId = 3; + + $role = $this->createRole(3, 'User', false, false, false, false, Role::ROLE_TYPE_DEFAULT); + + $this->roleMapper->method('findByUserId') + ->with($userId) + ->willReturn([$role]); + + $this->categoryPermMapper->method('findByCategoryAndRole') + ->willThrowException(new DoesNotExistException('Not found')); + + // Circles unavailable → null circle IDs → no team check + $this->categoryPermMapper->expects($this->never()) + ->method('findByCategoryAndTeamIds'); + + $service = $this->createServiceWithCircleIds(null); + + $this->assertFalse($service->hasCategoryPermission($userId, $categoryId, 'canView')); + } + + public function testHasCategoryPermissionTeamNotCheckedWhenUserHasNoCircles(): void { + $userId = 'user1'; + $categoryId = 3; + + $role = $this->createRole(3, 'User', false, false, false, false, Role::ROLE_TYPE_DEFAULT); + + $this->roleMapper->method('findByUserId') + ->with($userId) + ->willReturn([$role]); + + $this->categoryPermMapper->method('findByCategoryAndRole') + ->willThrowException(new DoesNotExistException('Not found')); + + // User is in no circles + $this->categoryPermMapper->expects($this->never()) + ->method('findByCategoryAndTeamIds'); + + $service = $this->createServiceWithCircleIds([]); + + $this->assertFalse($service->hasCategoryPermission($userId, $categoryId, 'canView')); + } + + public function testHasCategoryPermissionMultipleTeamsOneGrants(): void { + $userId = 'user1'; + $categoryId = 3; + + $role = $this->createRole(3, 'User', false, false, false, false, Role::ROLE_TYPE_DEFAULT); + + $this->roleMapper->method('findByUserId') + ->with($userId) + ->willReturn([$role]); + + $this->categoryPermMapper->method('findByCategoryAndRole') + ->willThrowException(new DoesNotExistException('Not found')); + + // User is in two teams; first denies, second grants + $teamPerm1 = $this->createTeamCategoryPerm(10, $categoryId, 'circle-aaa', false, false, false, false); + $teamPerm2 = $this->createTeamCategoryPerm(11, $categoryId, 'circle-bbb', true, true, false, false); + + $this->categoryPermMapper->method('findByCategoryAndTeamIds') + ->with($categoryId, ['circle-aaa', 'circle-bbb']) + ->willReturn([$teamPerm1, $teamPerm2]); + + $service = $this->createServiceWithCircleIds(['circle-aaa', 'circle-bbb']); + + $this->assertTrue($service->hasCategoryPermission($userId, $categoryId, 'canView')); + } + + public function testHasCategoryPermissionTeamGrantsSpecificPermissionOnly(): void { + $userId = 'user1'; + $categoryId = 3; + + $role = $this->createRole(3, 'User', false, false, false, false, Role::ROLE_TYPE_DEFAULT); + + $this->roleMapper->method('findByUserId') + ->with($userId) + ->willReturn([$role]); + + $this->categoryPermMapper->method('findByCategoryAndRole') + ->willThrowException(new DoesNotExistException('Not found')); + + // Team grants view but not post + $teamPerm = $this->createTeamCategoryPerm(10, $categoryId, 'circle-abc', true, false, false, false); + + $this->categoryPermMapper->method('findByCategoryAndTeamIds') + ->willReturn([$teamPerm]); + + $service = $this->createServiceWithCircleIds(['circle-abc']); + + $this->assertTrue($service->hasCategoryPermission($userId, $categoryId, 'canView')); + $this->assertFalse($service->hasCategoryPermission($userId, $categoryId, 'canPost')); + } + + public function testHasCategoryPermissionAdminBypassesTeamCheck(): void { + $userId = 'admin1'; + $categoryId = 3; + + $adminRole = $this->createRole(1, 'Admin', true, true, true, true, Role::ROLE_TYPE_ADMIN); + + $this->roleMapper->method('findByUserId') + ->with($userId) + ->willReturn([$adminRole]); + + // Neither role perms nor team perms should be checked + $this->categoryPermMapper->expects($this->never()) + ->method('findByCategoryAndRole'); + $this->categoryPermMapper->expects($this->never()) + ->method('findByCategoryAndTeamIds'); + + $service = $this->createServiceWithCircleIds(['circle-abc']); + + $this->assertTrue($service->hasCategoryPermission($userId, $categoryId, 'canView')); + } + + public function testHasCategoryPermissionRoleGrantsBeforeTeamCheck(): void { + $userId = 'user1'; + $categoryId = 1; + + // Role already grants the permission + $role = $this->createRole(3, 'User', false, false, false, false, Role::ROLE_TYPE_DEFAULT); + $rolePerm = $this->createCategoryPerm(1, $categoryId, 3, true, true, true, false); + + $this->roleMapper->method('findByUserId') + ->with($userId) + ->willReturn([$role]); + + $this->categoryPermMapper->method('findByCategoryAndRole') + ->willReturn($rolePerm); + + // Team mapper should not be called since role already granted + $this->categoryPermMapper->expects($this->never()) + ->method('findByCategoryAndTeamIds'); + + $service = $this->createServiceWithCircleIds(['circle-abc']); + + $this->assertTrue($service->hasCategoryPermission($userId, $categoryId, 'canView')); + } + + public function testGetAccessibleCategoriesIncludesTeamOnlyCategory(): void { + $userId = 'user1'; + + $role = $this->createRole(3, 'User', false, false, false, false, Role::ROLE_TYPE_DEFAULT); + $category1 = $this->createCategory(1, 'Role Access', 'role-access'); + $category2 = $this->createCategory(2, 'Team Only', 'team-only'); + $category3 = $this->createCategory(3, 'No Access', 'no-access'); + + $rolePerm1 = $this->createCategoryPerm(1, 1, 3, true, true, true, false); + $teamPerm2 = $this->createTeamCategoryPerm(10, 2, 'circle-abc', true, false, false, false); + + $this->roleMapper->method('findByUserId') + ->with($userId) + ->willReturn([$role]); + + $this->categoryMapper->method('findAll') + ->willReturn([$category1, $category2, $category3]); + + $this->categoryPermMapper->method('findByCategoryAndRole') + ->willReturnCallback(function ($catId, $roleId) use ($rolePerm1) { + if ($catId === 1 && $roleId === 3) { + return $rolePerm1; + } + throw new DoesNotExistException('Not found'); + }); + + $this->categoryPermMapper->method('findByCategoryAndTeamIds') + ->willReturnCallback(function ($catId, $teamIds) use ($teamPerm2) { + if ($catId === 2) { + return [$teamPerm2]; + } + return []; + }); + + $service = $this->createServiceWithCircleIds(['circle-abc']); + + $result = $service->getAccessibleCategories($userId); + + $this->assertCount(2, $result); + $this->assertContains(1, $result); + $this->assertContains(2, $result); + $this->assertNotContains(3, $result); + } + + public function testGetAccessibleCategoriesNoTeamAccessWhenCirclesUnavailable(): void { + $userId = 'user1'; + + $role = $this->createRole(3, 'User', false, false, false, false, Role::ROLE_TYPE_DEFAULT); + $category1 = $this->createCategory(1, 'Role Access', 'role-access'); + $category2 = $this->createCategory(2, 'Team Only', 'team-only'); + + $rolePerm1 = $this->createCategoryPerm(1, 1, 3, true, true, true, false); + + $this->roleMapper->method('findByUserId') + ->with($userId) + ->willReturn([$role]); + + $this->categoryMapper->method('findAll') + ->willReturn([$category1, $category2]); + + $this->categoryPermMapper->method('findByCategoryAndRole') + ->willReturnCallback(function ($catId, $roleId) use ($rolePerm1) { + if ($catId === 1 && $roleId === 3) { + return $rolePerm1; + } + throw new DoesNotExistException('Not found'); + }); + + // Circles unavailable + $service = $this->createServiceWithCircleIds(null); + + $result = $service->getAccessibleCategories($userId); + + // Only role-granted category, not the team-only one + $this->assertCount(1, $result); + $this->assertContains(1, $result); + } + + public function testHasCategoryPermissionTeamMapperExceptionHandledGracefully(): void { + $userId = 'user1'; + $categoryId = 3; + + $role = $this->createRole(3, 'User', false, false, false, false, Role::ROLE_TYPE_DEFAULT); + + $this->roleMapper->method('findByUserId') + ->with($userId) + ->willReturn([$role]); + + $this->categoryPermMapper->method('findByCategoryAndRole') + ->willThrowException(new DoesNotExistException('Not found')); + + $this->categoryPermMapper->method('findByCategoryAndTeamIds') + ->willThrowException(new \Exception('Database error')); + + $service = $this->createServiceWithCircleIds(['circle-abc']); + + // Should return false, not throw + $this->assertFalse($service->hasCategoryPermission($userId, $categoryId, 'canView')); + } + + // ---- Helper methods ---- + private function createCategoryPerm(int $id, int $categoryId, int $roleId, bool $canView, bool $canPost, bool $canReply, bool $canModerate): CategoryPerm { $perm = new CategoryPerm(); $perm->setId($id); @@ -799,6 +1174,19 @@ class PermissionServiceTest extends TestCase { return $perm; } + private function createTeamCategoryPerm(int $id, int $categoryId, string $teamId, bool $canView, bool $canPost, bool $canReply, bool $canModerate): CategoryPerm { + $perm = new CategoryPerm(); + $perm->setId($id); + $perm->setCategoryId($categoryId); + $perm->setTargetType(CategoryPerm::TARGET_TYPE_TEAM); + $perm->setTargetId($teamId); + $perm->setCanView($canView); + $perm->setCanPost($canPost); + $perm->setCanReply($canReply); + $perm->setCanModerate($canModerate); + return $perm; + } + private function createCategory(int $id, string $name, string $slug): Category { $category = new Category(); $category->setId($id);