diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index ef862d0f57..6a3c904267 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -23196,18 +23196,6 @@ parameters: count: 1 path: tests/integration/Core/Repository/ContentServiceAuthorizationTest.php - - - message: '#^Method Ibexa\\Tests\\Integration\\Core\\Repository\\ContentServiceAuthorizationTest\:\:testLoadContentDraftsThrowsUnauthorizedException\(\) has no return type specified\.$#' - identifier: missingType.return - count: 1 - path: tests/integration/Core/Repository/ContentServiceAuthorizationTest.php - - - - message: '#^Method Ibexa\\Tests\\Integration\\Core\\Repository\\ContentServiceAuthorizationTest\:\:testLoadContentDraftsThrowsUnauthorizedExceptionWithUser\(\) has no return type specified\.$#' - identifier: missingType.return - count: 1 - path: tests/integration/Core/Repository/ContentServiceAuthorizationTest.php - - message: '#^Method Ibexa\\Tests\\Integration\\Core\\Repository\\ContentServiceAuthorizationTest\:\:testLoadContentInfoByRemoteIdThrowsUnauthorizedException\(\) has no return type specified\.$#' identifier: missingType.return @@ -23256,18 +23244,6 @@ parameters: count: 1 path: tests/integration/Core/Repository/ContentServiceAuthorizationTest.php - - - message: '#^Method Ibexa\\Tests\\Integration\\Core\\Repository\\ContentServiceAuthorizationTest\:\:testLoadRelationsForDraftVersionThrowsUnauthorizedException\(\) has no return type specified\.$#' - identifier: missingType.return - count: 1 - path: tests/integration/Core/Repository/ContentServiceAuthorizationTest.php - - - - message: '#^Method Ibexa\\Tests\\Integration\\Core\\Repository\\ContentServiceAuthorizationTest\:\:testLoadRelationsThrowsUnauthorizedException\(\) has no return type specified\.$#' - identifier: missingType.return - count: 1 - path: tests/integration/Core/Repository/ContentServiceAuthorizationTest.php - - message: '#^Method Ibexa\\Tests\\Integration\\Core\\Repository\\ContentServiceAuthorizationTest\:\:testLoadRelationsWithUnauthorizedRelations\(\) has no return type specified\.$#' identifier: missingType.return diff --git a/tests/integration/Core/Repository/ContentServiceAuthorizationTest.php b/tests/integration/Core/Repository/ContentServiceAuthorizationTest.php index b6ed19fc8c..35f0522783 100644 --- a/tests/integration/Core/Repository/ContentServiceAuthorizationTest.php +++ b/tests/integration/Core/Repository/ContentServiceAuthorizationTest.php @@ -723,32 +723,31 @@ public function testCountContentDraftsReturnZero() /** * @covers \Ibexa\Contracts\Core\Repository\ContentService::loadContentDraftList() * - * @depends Ibexa\Tests\Integration\Core\Repository\ContentServiceTest::testLoadContentDrafts - * @depends Ibexa\Tests\Integration\Core\Repository\ContentServiceTest::testLoadContentDrafts + * @depends Ibexa\Tests\Integration\Core\Repository\ContentServiceTest::testLoadContentDraftList */ - public function testLoadContentDraftsThrowsUnauthorizedException() + public function testLoadContentDraftListReturnsEmptyListForUserWithoutVersionReadAccess(): void { $this->permissionResolver->setCurrentUserReference($this->anonymousUser); - $this->expectException(UnauthorizedException::class); - $this->expectExceptionMessageMatches('/\'versionread\' \'content\'/'); + $draftList = $this->contentService->loadContentDraftList(); - $this->contentService->loadContentDraftList(); + self::assertSame(0, $draftList->totalCount); + self::assertEmpty($draftList->items); } /** * @covers \Ibexa\Contracts\Core\Repository\ContentService::loadContentDraftList($user) * - * @depends Ibexa\Tests\Integration\Core\Repository\ContentServiceTest::testLoadContentDrafts + * @depends Ibexa\Tests\Integration\Core\Repository\ContentServiceTest::testLoadContentDraftList */ - public function testLoadContentDraftsThrowsUnauthorizedExceptionWithUser() + public function testLoadContentDraftListReturnsEmptyListForUserWithoutVersionReadAccessWithUser(): void { $this->permissionResolver->setCurrentUserReference($this->anonymousUser); - $this->expectException(UnauthorizedException::class); - $this->expectExceptionMessageMatches('/\'versionread\' \'content\'/'); + $draftList = $this->contentService->loadContentDraftList($this->administratorUser); - $this->contentService->loadContentDraftList($this->administratorUser); + self::assertSame(0, $draftList->totalCount); + self::assertEmpty($draftList->items); } /** @@ -911,13 +910,11 @@ public function testCopyContentThrowsUnauthorizedExceptionWithGivenVersion() } /** - * Test for the loadRelations() method. - * * @covers \Ibexa\Contracts\Core\Repository\ContentService::loadRelationList() * * @depends Ibexa\Tests\Integration\Core\Repository\ContentServiceTest::testLoadRelationList */ - public function testLoadRelationsThrowsUnauthorizedException() + public function testLoadRelationsReturnsEmptyListForUserWithoutReadAccess(): void { $mediaEditor = $this->createMediaUserVersion1(); @@ -931,29 +928,27 @@ public function testLoadRelationsThrowsUnauthorizedException() $this->permissionResolver->setCurrentUserReference($mediaEditor); - $this->expectException(UnauthorizedException::class); - $this->expectExceptionMessageMatches('/\'read\' \'content\'/'); + $relationList = $this->contentService->loadRelationList($versionInfo); - $this->contentService->loadRelationList($versionInfo); + self::assertSame(0, $relationList->totalCount); + self::assertEmpty($relationList->items); } /** - * Test for the loadRelations() method. - * * @covers \Ibexa\Contracts\Core\Repository\ContentService::loadRelationList() * * @depends Ibexa\Tests\Integration\Core\Repository\ContentServiceTest::testLoadRelationList */ - public function testLoadRelationsForDraftVersionThrowsUnauthorizedException() + public function testLoadRelationsForDraftVersionReturnsEmptyListForUserWithoutVersionReadAccess(): void { $draft = $this->createContentDraftVersion1(); $this->permissionResolver->setCurrentUserReference($this->anonymousUser); - $this->expectException(UnauthorizedException::class); - $this->expectExceptionMessageMatches('/\'versionread\' \'content\'/'); + $relationList = $this->contentService->loadRelationList($draft->versionInfo); - $this->contentService->loadRelationList($draft->versionInfo); + self::assertSame(0, $relationList->totalCount); + self::assertEmpty($relationList->items); } /** @@ -1066,9 +1061,9 @@ private function createAnonymousWithEditorRole() } /** - * Test that for an user that doesn't have access (read permissions) to an - * related object, executing loadRelations() would not throw any exception, - * only that the non-readable related object(s) won't be loaded. + * Test that for a user that doesn't have access (read permissions) to a + * related object, executing loadRelationList() would not throw any exception, and + * would instead expose the non-readable related object(s) as unauthorized list items. * * @covers \Ibexa\Contracts\Core\Repository\ContentService::loadRelationList() * @@ -1180,31 +1175,42 @@ public function testLoadRelationsWithUnauthorizedRelations() $actualRelations = $this->contentService->loadRelationList($testFolder->getVersionInfo()); // assert results - // verify that the only expected relations are from the 2 readable objects + // one item per relation, non-readable targets come back as UnauthorizedRelationListItem + self::assertCount( + 4, + $actualRelations->items, + 'Expected one list item per relation, including the unauthorized ones' + ); + + // verify that the only readable relations are from the 2 readable objects // Main Folder and Available Folder $expectedRelations = [ $mainRelation->destinationContentInfo->id => $mainRelation, $availableRelation->destinationContentInfo->id => $availableRelation, ]; - // assert there are as many expected relations as actual ones - self::assertEquals( - count($expectedRelations), - count($actualRelations->items), - "Expected '" . count($expectedRelations) - . "' relations found '" . count($actualRelations->items) . "'" - ); + $unauthorizedItemsCount = 0; // assert each relation /** * @var \Ibexa\Contracts\Core\Repository\Values\Content\RelationList\RelationListItemInterface $relationListItem */ foreach ($actualRelations as $relationListItem) { + if (!$relationListItem->hasRelation()) { + // non-readable target + ++$unauthorizedItemsCount; + continue; + } + /** @var \Ibexa\Contracts\Core\Repository\Values\Content\Relation $relation */ $relation = $relationListItem->getRelation(); $destination = $relation->destinationContentInfo; + self::assertArrayHasKey( + $destination->id, + $expectedRelations, + "Non expected relation with '{$destination->id}' id found" + ); $expected = $expectedRelations[$destination->id]->destinationContentInfo; - self::assertNotEmpty($expected, "Non expected relation with '{$destination->id}' id found"); self::assertEquals( $expected->id, $destination->id, @@ -1220,13 +1226,20 @@ public function testLoadRelationsWithUnauthorizedRelations() unset($expectedRelations[$destination->id]); } - // verify all expected relations were found + // verify all expected (readable) relations were found self::assertCount( 0, $expectedRelations, "Expected to find '" . (count($expectedRelations) + count($actualRelations->items)) . "' relations found '" . count($actualRelations->items) . "'" ); + + // verify the 2 non-readable relations came back as unauthorized placeholders + self::assertSame( + 2, + $unauthorizedItemsCount, + 'Expected the 2 relations towards non-readable content to be reported as unauthorized' + ); } /** diff --git a/tests/integration/Core/Repository/ContentServiceTest.php b/tests/integration/Core/Repository/ContentServiceTest.php index b7b14e3812..04022d0aaf 100644 --- a/tests/integration/Core/Repository/ContentServiceTest.php +++ b/tests/integration/Core/Repository/ContentServiceTest.php @@ -38,6 +38,7 @@ use Ibexa\Core\Repository\Values\Content\ContentUpdateStruct; use InvalidArgumentException; use ReflectionClass; +use ReflectionProperty; use Symfony\Bridge\PhpUnit\ClockMock; /** @@ -3673,13 +3674,14 @@ public function testAddRelationThrowsBadStateException() * @covers \Ibexa\Contracts\Core\Repository\ContentService::loadRelationList() * * @depends testAddRelation - * @depends loadRelationList + * @depends testLoadRelationList */ public function testLoadRelationsSkipsArchivedContent() { $trashService = $this->getRepository()->getTrashService(); - $draft = $this->createContentDraftVersion1(); + // trashing location 56 below would purge a draft created under it + $draft = $this->createContentDraftVersion1(2); // Load other content objects $media = $this->contentService->loadContentInfoByRemoteId(self::MEDIA_REMOTE_ID); @@ -3736,7 +3738,7 @@ public function testLoadRelationsSkipsArchivedContent() * @covers \Ibexa\Contracts\Core\Repository\ContentService::loadRelationList() * * @depends testAddRelation - * @depends loadRelationList + * @depends testLoadRelationList */ public function testLoadRelationsSkipsDraftContent() { @@ -5075,7 +5077,7 @@ public function testDeleteVersionInTransactionWithRollback() * * @depends testCreateContent * @depends testLoadContentInfo - * @depends testLoadContentDrafts + * @depends testLoadContentDraftList */ public function testDeleteVersionInTransactionWithCommit() { @@ -7172,20 +7174,43 @@ public function testLoadContentWithinGracePeriod(): void $anonymousUserId = $this->generateId('user', 10); $repository->getPermissionResolver()->setCurrentUserReference($repository->getUserService()->loadUser($anonymousUserId)); + // the ContentService instance is shared across the test run, restore the setting afterwards + $originalGracePeriod = $this->getGracePeriod(); $this->setGracePeriod(10); - //Reset clock, to make sure that upfront operations did not exceed grace period. - ClockMock::withClockMock(strtotime('2025-04-01 14:00:02')); - $this->contentService->loadContent($unPublishedVersionOneContent->getId(), null, $unPublishedVersionOneContent->getVersionInfo()->versionNo); - - ClockMock::sleep(20); - $this->expectException(CoreUnauthorizedException::class); - $this->contentService->loadContent($unPublishedVersionOneContent->getId(), null, $unPublishedVersionOneContent->getVersionInfo()->versionNo); + try { + //Reset clock, to make sure that upfront operations did not exceed grace period. + ClockMock::withClockMock(strtotime('2025-04-01 14:00:02')); + $this->contentService->loadContent($unPublishedVersionOneContent->getId(), null, $unPublishedVersionOneContent->getVersionInfo()->versionNo); + + ClockMock::sleep(20); + $this->expectException(CoreUnauthorizedException::class); + $this->contentService->loadContent($unPublishedVersionOneContent->getId(), null, $unPublishedVersionOneContent->getVersionInfo()->versionNo); + } finally { + ClockMock::withClockMock(false); + $this->setGracePeriod($originalGracePeriod); + } + } - ClockMock::withClockMock(false); + private function getGracePeriod(): int + { + return $this->getInnerContentServiceSettingsProperty()->getValue( + $this->getInnerContentService() + )['grace_period_in_seconds']; } private function setGracePeriod(int $value): void + { + $innerService = $this->getInnerContentService(); + $settingsProperty = $this->getInnerContentServiceSettingsProperty(); + + $settings = $settingsProperty->getValue($innerService); + $settings['grace_period_in_seconds'] = $value; + + $settingsProperty->setValue($innerService, $settings); + } + + private function getInnerContentService(): object { $reflection = new ReflectionClass($this->contentService); $serviceProperty = $reflection->getProperty('service'); @@ -7197,15 +7222,15 @@ private function setGracePeriod(int $value): void $innerServiceProperty = $serviceReflection->getProperty('innerService'); $innerServiceProperty->setAccessible(true); - $innerService = $innerServiceProperty->getValue($service); + return $innerServiceProperty->getValue($service); + } - $innerServiceReflection = new ReflectionClass($innerService); + private function getInnerContentServiceSettingsProperty(): ReflectionProperty + { + $innerServiceReflection = new ReflectionClass($this->getInnerContentService()); $settingsProperty = $innerServiceReflection->getProperty('settings'); $settingsProperty->setAccessible(true); - $settings = $settingsProperty->getValue($innerService); - $settings['grace_period_in_seconds'] = $value; - - $settingsProperty->setValue($innerService, $settings); + return $settingsProperty; } }