diff --git a/core/ormdocument.class.inc.php b/core/ormdocument.class.inc.php index 0227ac347..7e7b2bb20 100644 --- a/core/ormdocument.class.inc.php +++ b/core/ormdocument.class.inc.php @@ -350,15 +350,18 @@ class ormDocument if (!is_object($oObj)) { // If access to the document is not granted, check if the access to the host object is allowed $oObj = MetaModel::GetObject($sClass, $id, false, true); + $bHasHostRights = false; if ($oObj instanceof Attachment) { $sItemClass = $oObj->Get('item_class'); $sItemId = $oObj->Get('item_id'); $oHost = MetaModel::GetObject($sItemClass, $sItemId, false, false); - if (!is_object($oHost)) { - $oObj = null; + if (is_object($oHost)) { + $bHasHostRights = true; } } - if (!is_object($oObj)) { + + // We could neither read the object nor get a host object matching our rights + if ($bHasHostRights !== true) { throw new Exception("Invalid id ($id) for class '$sClass' - the object does not exist or you are not allowed to view it"); } } diff --git a/tests/php-unit-tests/unitary-tests/core/ormDocumentTest.php b/tests/php-unit-tests/unitary-tests/core/ormDocumentTest.php index c083667ed..b9e7a090c 100644 --- a/tests/php-unit-tests/unitary-tests/core/ormDocumentTest.php +++ b/tests/php-unit-tests/unitary-tests/core/ormDocumentTest.php @@ -7,14 +7,36 @@ namespace Combodo\iTop\Test\UnitTest\Core; +use Combodo\iTop\Application\WebPage\CaptureWebPage; use Combodo\iTop\Test\UnitTest\ItopDataTestCase; use ormDocument; +use UserRights; /** * Tests of the ormDocument class */ class ormDocumentTest extends ItopDataTestCase { + private const RESTRICTED_PROFILE = 'Configuration Manager'; + private int $iUserOrg; + private int $iOrgDifferentFromUser; + + protected function setUp(): void + { + parent::setUp(); + + $this->iUserOrg = $this->GivenObjectInDB('Organization', [ + 'name' => 'UserOrg', + ]); + + $this->iOrgDifferentFromUser = $this->GivenObjectInDB('Organization', [ + 'name' => 'OrgDifferentFromUser', + ]); + + $this->LoginRestrictedUser($this->iUserOrg, self::RESTRICTED_PROFILE); + $this->ResetMetaModelQueyCacheGetObject(); + } + /** * @inheritDoc */ @@ -139,4 +161,107 @@ class ormDocumentTest extends ItopDataTestCase ], ]; } + + /** + * Test that DownloadDocument enforces rights for documents + * + * @dataProvider DownloadDocumentRightsProvider + */ + public function testDownloadDocumentDifferentOrg(string $sTargetClass, string $sAttCode, string $sData, string $sFileName, ?string $sHostClass) + { + $iDeniedDocumentId = $this->CreateDownloadTargetInOrg($sTargetClass, $sAttCode, $this->iOrgDifferentFromUser, $sData, $sFileName, $sHostClass); + + $oPageDenied = new CaptureWebPage(); + ormDocument::DownloadDocument($oPageDenied, $sTargetClass, $iDeniedDocumentId, $sAttCode); + $sDeniedHtml = (string) $oPageDenied->GetHtml(); + $this->assertStringContainsString( + 'the object does not exist or you are not allowed to view it', + $sDeniedHtml, + 'Expected error message when rights are missing.' + ); + $this->assertStringNotContainsString($sData, $sDeniedHtml, 'Unexpected file data present when rights are missing.'); + } + + /** + * Test that DownloadDocument allows to retrieve document with the same org (or host object org) + * + * @dataProvider DownloadDocumentRightsProvider + */ + public function testDownloadDocumentSameOrg(string $sTargetClass, string $sAttCode, string $sData, string $sFileName, ?string $sHostClass) + { + $iAllowedDocumentId = $this->CreateDownloadTargetInOrg($sTargetClass, $sAttCode, $this->iUserOrg, $sData, $sFileName, $sHostClass); + + $oPageAllowed = new CaptureWebPage(); + ormDocument::DownloadDocument($oPageAllowed, $sTargetClass, $iAllowedDocumentId, $sAttCode); + $sAllowedHtml = (string) $oPageAllowed->GetHtml(); + $this->assertStringContainsString($sData, $sAllowedHtml, 'Expected file data present when rights are sufficient.'); + $this->assertStringNotContainsString('the object does not exist or you are not allowed to view it', $sAllowedHtml, 'Unexpected error message when rights are sufficient.'); + } + + public function DownloadDocumentRightsProvider(): array + { + return [ + 'DocumentFile' => [ + 'class' => 'DocumentFile', + 'data_attribute_id' => 'file', + 'data' => 'document_data', + 'file_name' => 'document.txt', + 'host_class' => null], + 'Attachment' => [ + 'class' => 'Attachment', + 'data_attribute_id' => 'contents', + 'data' => 'attachment_data', + 'file_name' => 'attachment.txt', + 'host_class' => 'UserRequest'], + ]; + } + + /** + * Helper to avoid duplicating object creation in tests + * Created objects and host objects depending on the Document class + * @param string $sTargetClass + * @param string $sAttCode + * @param int $iOrgId + * @param string $sData + * @param string $sFileName + * @param string|null $sHostClass + * @return int + * @throws \Exception + */ + private function CreateDownloadTargetInOrg(string $sTargetClass, string $sAttCode, int $iOrgId, string $sData, string $sFileName, ?string $sHostClass): int + { + + if ($sTargetClass === 'DocumentFile') { + return $this->GivenObjectInDB($sTargetClass, [ + 'name' => 'UnitTestDocFile_'.uniqid(), + 'org_id' => $iOrgId, + $sAttCode => new ormDocument($sData, 'text/plain', $sFileName), + ]); + } + + if ($sTargetClass === 'Attachment') { + $iHostId = $this->GivenObjectInDB($sHostClass, [ + 'title' => 'UnitTestUserRequest_'.uniqid(), + 'org_id' => $iOrgId, + 'description' => 'A user request for testing attachment download rights', + ]); + + return $this->GivenObjectInDB('Attachment', [ + 'item_class' => $sHostClass, + 'item_id' => $iHostId, + $sAttCode => new ormDocument($sData, 'text/plain', $sFileName), + ]); + } + + throw new \Exception("Unsupported target class: $sTargetClass"); + } + + private function LoginRestrictedUser(int $iAllowedOrgId, string $sProfileName): void + { + if (UserRights::IsLoggedIn()) { + UserRights::Logoff(); + } + $sLogin = $this->GivenUserRestrictedToAnOrganizationInDB($iAllowedOrgId, self::$aURP_Profiles[$sProfileName]); + UserRights::Login($sLogin); + } }