diff --git a/lib/Controller/CategoryController.php b/lib/Controller/CategoryController.php index 3195da5..2927363 100644 --- a/lib/Controller/CategoryController.php +++ b/lib/Controller/CategoryController.php @@ -14,6 +14,7 @@ use OCA\Forum\Db\CategoryPermMapper; use OCA\Forum\Db\CatHeaderMapper; use OCA\Forum\Db\ThreadMapper; use OCA\Forum\Db\UserRoleMapper; +use OCA\Forum\Service\UserRoleService; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\ApiRoute; @@ -384,7 +385,8 @@ class CategoryController extends OCSController { #[ApiRoute(verb: 'GET', url: '/api/categories/{id}/permissions')] public function getPermissions(int $id): DataResponse { try { - $permissions = $this->categoryPermMapper->findByCategoryId($id); + // Exclude Admin role - it has hardcoded full access to all categories + $permissions = $this->categoryPermMapper->findByCategoryIdExcludingAdmin($id); return new DataResponse(array_map(fn ($perm) => $perm->jsonSerialize(), $permissions)); } catch (\Exception $e) { $this->logger->error('Error fetching category permissions: ' . $e->getMessage()); @@ -412,8 +414,13 @@ class CategoryController extends OCSController { // Delete existing permissions for this category $this->categoryPermMapper->deleteByCategoryId($id); + // Filter out Admin role - it has hardcoded full access + $filteredPermissions = array_filter($permissions, fn ($perm) + => ($perm['roleId'] ?? null) !== UserRoleService::ROLE_ADMIN + ); + // Insert new permissions - foreach ($permissions as $perm) { + foreach ($filteredPermissions as $perm) { $categoryPerm = new CategoryPerm(); $categoryPerm->setCategoryId($id); $categoryPerm->setRoleId($perm['roleId']); diff --git a/lib/Controller/RoleController.php b/lib/Controller/RoleController.php index 529bbeb..3bd7d3e 100644 --- a/lib/Controller/RoleController.php +++ b/lib/Controller/RoleController.php @@ -164,14 +164,22 @@ class RoleController extends OCSController { if ($colorDark !== null) { $role->setColorDark($colorDark); } - if ($canAccessAdminTools !== null) { - $role->setCanAccessAdminTools($canAccessAdminTools); - } - if ($canEditRoles !== null) { - $role->setCanEditRoles($canEditRoles); - } - if ($canEditCategories !== null) { - $role->setCanEditCategories($canEditCategories); + + // Admin role always has all permissions - cannot be changed + if ($id === UserRoleService::ROLE_ADMIN) { + $role->setCanAccessAdminTools(true); + $role->setCanEditRoles(true); + $role->setCanEditCategories(true); + } else { + if ($canAccessAdminTools !== null) { + $role->setCanAccessAdminTools($canAccessAdminTools); + } + if ($canEditRoles !== null) { + $role->setCanEditRoles($canEditRoles); + } + if ($canEditCategories !== null) { + $role->setCanEditCategories($canEditCategories); + } } /** @var \OCA\Forum\Db\Role */ diff --git a/lib/Db/CategoryPermMapper.php b/lib/Db/CategoryPermMapper.php index 73903e8..8db74bb 100644 --- a/lib/Db/CategoryPermMapper.php +++ b/lib/Db/CategoryPermMapper.php @@ -8,6 +8,7 @@ declare(strict_types=1); namespace OCA\Forum\Db; use OCA\Forum\AppInfo\Application; +use OCA\Forum\Service\UserRoleService; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\QBMapper; use OCP\DB\QueryBuilder\IQueryBuilder; @@ -67,6 +68,26 @@ class CategoryPermMapper extends QBMapper { return $this->findEntities($qb); } + /** + * Find permissions for a category, excluding Admin role (which has implicit full access) + * + * @param int $categoryId Category ID + * @return array + */ + public function findByCategoryIdExcludingAdmin(int $categoryId): array { + /* @var $qb IQueryBuilder */ + $qb = $this->db->getQueryBuilder(); + $qb->select('*') + ->from($this->getTableName()) + ->where( + $qb->expr()->eq('category_id', $qb->createNamedParameter($categoryId, IQueryBuilder::PARAM_INT)) + ) + ->andWhere( + $qb->expr()->neq('role_id', $qb->createNamedParameter(UserRoleService::ROLE_ADMIN, IQueryBuilder::PARAM_INT)) + ); + return $this->findEntities($qb); + } + /** * Find permission for specific category and role * diff --git a/lib/Migration/Version6Date20251122233018.php b/lib/Migration/Version6Date20251122233018.php new file mode 100644 index 0000000..c766dfe --- /dev/null +++ b/lib/Migration/Version6Date20251122233018.php @@ -0,0 +1,64 @@ + +// SPDX-License-Identifier: AGPL-3.0-or-later + +namespace OCA\Forum\Migration; + +use Closure; +use OCA\Forum\AppInfo\Application; +use OCA\Forum\Service\UserRoleService; +use OCP\DB\ISchemaWrapper; +use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\IDBConnection; +use OCP\Migration\IOutput; +use OCP\Migration\SimpleMigrationStep; + +class Version6Date20251122233018 extends SimpleMigrationStep { + public function __construct( + private IDBConnection $db, + ) { + } + /** + * @param IOutput $output + * @param Closure(): ISchemaWrapper $schemaClosure + * @param array $options + */ + public function preSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void { + } + + /** + * @param IOutput $output + * @param Closure(): ISchemaWrapper $schemaClosure + * @param array $options + * @return null|ISchemaWrapper + */ + public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { + /** @var ISchemaWrapper $schema */ + $schema = $schemaClosure(); + + // TODO add migration logic + + return $schema; + } + + /** + * @param IOutput $output + * @param Closure(): ISchemaWrapper $schemaClosure + * @param array $options + */ + public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options): void { + // Remove Admin role permissions from categories + // Admin role now has hardcoded full access to all categories + $qb = $this->db->getQueryBuilder(); + $qb->delete(Application::tableName('forum_category_perms')) + ->where( + $qb->expr()->eq('role_id', $qb->createNamedParameter(UserRoleService::ROLE_ADMIN, IQueryBuilder::PARAM_INT)) + ); + $deletedCount = $qb->executeStatement(); + + $output->info("Removed $deletedCount Admin role permission entries from categories (Admin has hardcoded full access)"); + } +} diff --git a/lib/Service/PermissionService.php b/lib/Service/PermissionService.php index a886f3c..69dc22f 100644 --- a/lib/Service/PermissionService.php +++ b/lib/Service/PermissionService.php @@ -28,11 +28,34 @@ class PermissionService { ) { } + /** + * Check if user has Admin role + * + * @param string $userId Nextcloud user ID + * @return bool True if user has Admin role + */ + private function hasAdminRole(string $userId): bool { + try { + $userRoles = $this->userRoleMapper->findByUserId($userId); + + foreach ($userRoles as $userRole) { + if ($userRole->getRoleId() === UserRoleService::ROLE_ADMIN) { + return true; + } + } + + return false; + } catch (\Exception $e) { + $this->logger->error("Error checking admin role for user $userId: " . $e->getMessage()); + return false; + } + } + /** * Check if user has Admin or Moderator role * * @param string $userId Nextcloud user ID - * @return bool True if user has Admin (roleId 1) or Moderator (roleId 2) role + * @return bool True if user has Admin or Moderator role */ public function hasAdminOrModeratorRole(string $userId): bool { try { @@ -40,8 +63,7 @@ class PermissionService { foreach ($userRoles as $userRole) { $roleId = $userRole->getRoleId(); - // Admin role = 1, Moderator role = 2 - if ($roleId === 1 || $roleId === 2) { + if ($roleId === UserRoleService::ROLE_ADMIN || $roleId === UserRoleService::ROLE_MODERATOR) { return true; } } @@ -125,6 +147,12 @@ class PermissionService { * @return bool True if user has the permission */ public function hasCategoryPermission(string $userId, int $categoryId, string $permission): bool { + // Admin role has hardcoded full access to all categories + if ($this->hasAdminRole($userId)) { + $this->logger->debug("User $userId has Admin role - granting category permission '$permission' on category $categoryId"); + return true; + } + try { $userRoles = $this->userRoleMapper->findByUserId($userId); diff --git a/src/views/admin/AdminCategoryEdit.vue b/src/views/admin/AdminCategoryEdit.vue index a0f4d6e..b395736 100644 --- a/src/views/admin/AdminCategoryEdit.vue +++ b/src/views/admin/AdminCategoryEdit.vue @@ -323,10 +323,13 @@ export default defineComponent({ })) }, roleOptions(): Array<{ id: number; label: string }> { - return this.roles.map((role) => ({ - id: role.id, - label: role.name, - })) + // Filter out Admin role - it has implicit full access to all categories + return this.roles + .filter((role) => role.id !== SystemRole.ADMIN) + .map((role) => ({ + id: role.id, + label: role.name, + })) }, }, watch: { diff --git a/src/views/admin/AdminRoleEdit.vue b/src/views/admin/AdminRoleEdit.vue index 918d027..c33b2cd 100644 --- a/src/views/admin/AdminRoleEdit.vue +++ b/src/views/admin/AdminRoleEdit.vue @@ -115,21 +115,21 @@
- + {{ strings.canAccessAdminTools }} {{ strings.canAccessAdminToolsDesc }}
- + {{ strings.canEditRoles }} {{ strings.canEditRolesDesc }}
- + {{ strings.canEditCategories }} {{ strings.canEditCategoriesDesc }} @@ -399,6 +399,13 @@ export default defineComponent({ this.formData.canEditRoles = role.canEditRoles || false this.formData.canEditCategories = role.canEditCategories || false + // Admin role always has all permissions + if (this.isAdmin) { + this.formData.canAccessAdminTools = true + this.formData.canEditRoles = true + this.formData.canEditCategories = true + } + // If colors are different, mark dark as modified if (role.colorLight && role.colorDark && role.colorLight !== role.colorDark) { this.darkColorModified = true @@ -452,9 +459,9 @@ export default defineComponent({ description: this.formData.description.trim() || null, colorLight: this.formData.colorLight || null, colorDark: this.formData.colorDark || null, - canAccessAdminTools: this.formData.canAccessAdminTools, - canEditRoles: this.formData.canEditRoles, - canEditCategories: this.formData.canEditCategories, + canAccessAdminTools: this.isAdmin ? true : this.formData.canAccessAdminTools, + canEditRoles: this.isAdmin ? true : this.formData.canEditRoles, + canEditCategories: this.isAdmin ? true : this.formData.canEditCategories, } let roleId: number