Merge pull request #6011 from nextcloud/chore/property-promition

lib/Service/PermissionService.php

@@ -1,4 +1,5 @@
 <?php
+
 /**
  * SPDX-FileCopyrightText: 2016 Nextcloud GmbH and Nextcloud contributors
  * SPDX-License-Identifier: AGPL-3.0-or-later
@@ -20,57 +21,27 @@ use OCP\AppFramework\Db\MultipleObjectsReturnedException;
 use OCP\Cache\CappedMemoryCache;
 use OCP\IConfig;
 use OCP\IGroupManager;
-use OCP\ILogger;
 use OCP\IUserManager;
 use OCP\Share\IManager;
+use Psr\Log\LoggerInterface;
 
 class PermissionService {
-
+	private array $users = [];
-	/** @var CirclesService */
-	private $circlesService;
-	/** @var BoardMapper */
-	private $boardMapper;
-	/** @var AclMapper */
-	private $aclMapper;
-	/** @var ILogger */
-	private $logger;
-	/** @var IUserManager */
-	private $userManager;
-	/** @var IGroupManager */
-	private $groupManager;
-	/** @var IConfig */
-	private $config;
-	/** @var IManager */
-	private $shareManager;
-	/** @var string */
-	private $userId;
-	/** @var array */
-	private $users = [];
 
 	private CappedMemoryCache $boardCache;
 	private CappedMemoryCache $permissionCache;
 
 	public function __construct(
-		ILogger $logger,
+		private LoggerInterface $logger,
-		CirclesService $circlesService,
+		private CirclesService $circlesService,
-		AclMapper $aclMapper,
+		private AclMapper $aclMapper,
-		BoardMapper $boardMapper,
+		private BoardMapper $boardMapper,
-		IUserManager $userManager,
+		private IUserManager $userManager,
-		IGroupManager $groupManager,
+		private IGroupManager $groupManager,
-		IManager $shareManager,
+		private IManager $shareManager,
-		IConfig $config,
+		private IConfig $config,
-		$userId
+		private ?string $userId
 	) {
-		$this->circlesService = $circlesService;
-		$this->aclMapper = $aclMapper;
-		$this->boardMapper = $boardMapper;
-		$this->logger = $logger;
-		$this->userManager = $userManager;
-		$this->groupManager = $groupManager;
-		$this->shareManager = $shareManager;
-		$this->config = $config;
-		$this->userId = $userId;
-
 		$this->boardCache = new CappedMemoryCache();
 		$this->permissionCache = new CappedMemoryCache();
 	}
@@ -78,10 +49,9 @@ class PermissionService {
 	/**
 	 * Get current user permissions for a board by id
 	 *
-	 * @param $boardId
 	 * @return bool|array
 	 */
-	public function getPermissions($boardId, ?string $userId = null) {
+	public function getPermissions(int $boardId, ?string $userId = null) {
 		if ($userId === null) {
 			$userId = $this->userId;
 		}
@@ -99,7 +69,7 @@ class PermissionService {
 			$owner = false;
 			$acls = [];
 		}
-		
+
 		$permissions = [
 			Acl::PERMISSION_READ => $owner || $this->userCan($acls, Acl::PERMISSION_READ, $userId),
 			Acl::PERMISSION_EDIT => $owner || $this->userCan($acls, Acl::PERMISSION_EDIT, $userId),
@@ -133,11 +103,10 @@ class PermissionService {
 	/**
 	 * check permissions for replacing dark magic middleware
 	 *
-	 * @param numeric $id
 	 * @throws NoPermissionException
 	 */
 	public function checkPermission(?IPermissionMapper $mapper, $id, int $permission, $userId = null, bool $allowDeletedCard = false): bool {
-		$boardId = $id;
+		$boardId = (int)$id;
 		if ($mapper instanceof IPermissionMapper && !($mapper instanceof BoardMapper)) {
 			$boardId = $mapper->findBoardId($id);
 		}
@@ -150,7 +119,7 @@ class PermissionService {
 		if ($permissions[$permission] === true) {
 
 			if (!$allowDeletedCard && $mapper instanceof CardMapper) {
-				$card = $mapper->find($id);
+				$card = $mapper->find((int)$id);
 				if ($card->getDeletedAt() > 0) {
 					throw new NoPermissionException('Card is deleted');
 				}

tests/unit/Service/PermissionServiceTest.php

@@ -34,39 +34,28 @@ use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\IConfig;
 use OCP\IGroup;
 use OCP\IGroupManager;
-use OCP\ILogger;
 use OCP\IRequest;
 use OCP\IUser;
 use OCP\IUserManager;
 use OCP\Share\IManager;
+use PHPUnit\Framework\MockObject\MockObject;
+use Psr\Log\LoggerInterface;
 
 class PermissionServiceTest extends \Test\TestCase {
-
+	private IRequest|MockObject $request;
-	/** @var IRequest */
+	private CirclesService|MockObject $circlesService;
-	private $request;
+	private PermissionService $service;
-	private $circlesService;
+	private LoggerInterface|MockObject $logger;
-	/** @var PermissionService*/
+	private AclMapper|MockObject $aclMapper;
-	private $service;
+	private BoardMapper|MockObject $boardMapper;
-	/** @var ILogger */
+	private IUserManager $userManager;
-	private $logger;
+	private IGroupManager|MockObject $groupManager;
-	/** @var AclMapper */
+	private MockObject|IManager $shareManager;
-	private $aclMapper;
+	private IConfig|MockObject $config;
-	/** @var BoardMapper */
-	private $boardMapper;
-	/** @var IUserManager */
-	private $userManager;
-	/** @var IGroupManager */
-	private $groupManager;
-	/** @var IManager */
-	private $shareManager;
-	/** @var IConfig */
-	private $config;
-	/** @var string */
-	private $userId = 'admin';
 
 	public function setUp(): void {
 		parent::setUp();
-		$this->logger = $this->createMock(ILogger::class);
+		$this->logger = $this->createMock(LoggerInterface::class);
 		$this->request = $this->createMock(IRequest::class);
 		$this->circlesService = $this->createMock(CirclesService::class);
 		$this->aclMapper = $this->createMock(AclMapper::class);