From 8ad7759195e76a20e7757397ce2931b4d3caeb2d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andr=C3=A9=20R?= <andre.romcke@gmail.com>
Date: Mon, 10 Sep 2018 04:00:18 +0200
Subject: [PATCH] EZP-29474: Fix wrong usage of hasAccess in repository (#2413)
* EZP-29474: Fix wrong usage of hasAccess in repository
Fix wrong usage\* of `hasAccess()` in repository in favor of `canUser()` in order to solve:
- Issues when there are Role Assignment Limitation
- As policies are now extendable, better be prepared in case someone needs to customize and add limitations to any of these functions
I effect this makes Repo more correctly use permissions system, and will avoid quite some unneeded exceptions all across the repo.
There is also two behavour changes _(in terms of api spec)_ which will simplify API usage:
- loadSections(): PR proposes to change this to just filter sections _(preparing it for we/others adding policy limitations on the function, & avoiding issue with role limitation)_
- load user drafts: Same, proposes to filter the returned drafts instead of throwing
\* _Long story short, we forgot about Role limitations + extensibility when coding this, and made shortcuts hard coding that assumption, this PR fixes that._
* [API Doc] Add some api doc to clearify usage.
Also fix CS issue on test.
Other fixes:
* Always use target location on trash restore to properly take subtree limitations into account
* Adapt SubtreeLimitation to abstain on trashed content when no location target is provided (abstain is for for role limitations, means they will be ignored)
---
.../API/Repository/PermissionResolver.php | 9 ++--
.../Core/Limitation/SubtreeLimitationType.php | 9 +++-
.../Tests/SubtreeLimitationTypeTest.php | 20 ++++----
eZ/Publish/Core/Repository/ContentService.php | 1 +
.../Core/Repository/ContentTypeService.php | 28 +++++------
.../Core/Repository/LanguageService.php | 10 ++--
.../Core/Repository/ObjectStateService.php | 14 +++---
eZ/Publish/Core/Repository/SectionService.php | 49 +++++++++++--------
.../Tests/Service/Integration/TrashBase.php | 20 ++++++--
.../Tests/Service/Mock/UrlWildcardTest.php | 31 ++++++++----
eZ/Publish/Core/Repository/TrashService.php | 22 ++++++---
.../Core/Repository/URLWildcardService.php | 2 +-
12 files changed, 132 insertions(+), 83 deletions(-)
diff --git a/eZ/Publish/API/Repository/PermissionResolver.php b/eZ/Publish/API/Repository/PermissionResolver.php
index 0efb3de963d..030f869d692 100644
--- a/eZ/Publish/API/Repository/PermissionResolver.php
+++ b/eZ/Publish/API/Repository/PermissionResolver.php
@@ -29,10 +29,13 @@ public function getCurrentUserReference();
public function setCurrentUserReference(UserReference $userReference);
/**
- * Returns boolean value or an array of limitations describing user's permissions
- * on the given module and function.
+ * Low level permission function: Returns boolean value, or an array of limitations that user permission depends on.
*
- * Note: boolean value describes full access (true) or no access at all (false).
+ * Note: boolean value describes full access (true) or no access at all (false), array can be seen as a maybe..
+ *
+ * WARNING: This is a low level method, if possible strongly prefer to use canUser() as it is able to handle limitations.
+ * This includes Role Assignment limitations, but also future policy limitations added in kernel,
+ * or as plain user configuration and/or extending the system.
*
* @param string $module The module, aka controller identifier to check permissions on
* @param string $function The function, aka the controller action to check permissions on
diff --git a/eZ/Publish/Core/Limitation/SubtreeLimitationType.php b/eZ/Publish/Core/Limitation/SubtreeLimitationType.php
index d7730afc92d..47626c2d35a 100644
--- a/eZ/Publish/Core/Limitation/SubtreeLimitationType.php
+++ b/eZ/Publish/Core/Limitation/SubtreeLimitationType.php
@@ -137,10 +137,15 @@ public function evaluate(APILimitationValue $value, APIUserReference $currentUse
// Load locations if no specific placement was provided
if ($targets === null) {
- if ($object->published) {
+ // Skip check if content is in trash and no location is provided to check against
+ if ($object->isTrashed()) {
+ return self::ACCESS_ABSTAIN;
+ }
+
+ if ($object->isPublished()) {
$targets = $this->persistence->locationHandler()->loadLocationsByContent($object->id);
} else {
- // @todo Need support for draft locations to to work correctly
+ // @todo Need support for draft locations to work correctly
$targets = $this->persistence->locationHandler()->loadParentLocationsForDraftContent($object->id);
}
}
diff --git a/eZ/Publish/Core/Limitation/Tests/SubtreeLimitationTypeTest.php b/eZ/Publish/Core/Limitation/Tests/SubtreeLimitationTypeTest.php
index 4b858693889..31c59bd1c2b 100644
--- a/eZ/Publish/Core/Limitation/Tests/SubtreeLimitationTypeTest.php
+++ b/eZ/Publish/Core/Limitation/Tests/SubtreeLimitationTypeTest.php
@@ -283,7 +283,7 @@ public function providerForTestEvaluate()
$versionInfoMock
->expects($this->once())
->method('getContentInfo')
- ->will($this->returnValue(new ContentInfo(array('published' => true))));
+ ->willReturn(new ContentInfo(['published' => true, 'status' => ContentInfo::STATUS_PUBLISHED]));
$versionInfoMock2 = $this->getMock(
'eZ\\Publish\\API\\Repository\\Values\\Content\\VersionInfo',
@@ -296,13 +296,13 @@ public function providerForTestEvaluate()
$versionInfoMock2
->expects($this->once())
->method('getContentInfo')
- ->will($this->returnValue(new ContentInfo(array('published' => true))));
+ ->willReturn(new ContentInfo(['published' => true, 'status' => ContentInfo::STATUS_PUBLISHED]));
return array(
// ContentInfo, with targets, no access
array(
'limitation' => new SubtreeLimitation(),
- 'object' => new ContentInfo(array('published' => true)),
+ 'object' => new ContentInfo(['published' => true, 'status' => ContentInfo::STATUS_PUBLISHED]),
'targets' => array(new Location()),
'persistence' => array(),
'expected' => LimitationType::ACCESS_DENIED,
@@ -310,7 +310,7 @@ public function providerForTestEvaluate()
// ContentInfo, with targets, no access
array(
'limitation' => new SubtreeLimitation(array('limitationValues' => array('/1/2/'))),
- 'object' => new ContentInfo(array('published' => true)),
+ 'object' => new ContentInfo(['published' => true, 'status' => ContentInfo::STATUS_PUBLISHED]),
'targets' => array(new Location(array('pathString' => '/1/55/'))),
'persistence' => array(),
'expected' => LimitationType::ACCESS_DENIED,
@@ -318,7 +318,7 @@ public function providerForTestEvaluate()
// ContentInfo, with targets, with access
array(
'limitation' => new SubtreeLimitation(array('limitationValues' => array('/1/2/'))),
- 'object' => new ContentInfo(array('published' => true)),
+ 'object' => new ContentInfo(['published' => true, 'status' => ContentInfo::STATUS_PUBLISHED]),
'targets' => array(new Location(array('pathString' => '/1/2/'))),
'persistence' => array(),
'expected' => LimitationType::ACCESS_GRANTED,
@@ -326,7 +326,7 @@ public function providerForTestEvaluate()
// ContentInfo, no targets, with access
array(
'limitation' => new SubtreeLimitation(array('limitationValues' => array('/1/2/'))),
- 'object' => new ContentInfo(array('published' => true)),
+ 'object' => new ContentInfo(['published' => true, 'status' => ContentInfo::STATUS_PUBLISHED]),
'targets' => null,
'persistence' => array(new Location(array('pathString' => '/1/2/'))),
'expected' => LimitationType::ACCESS_GRANTED,
@@ -334,7 +334,7 @@ public function providerForTestEvaluate()
// ContentInfo, no targets, no access
array(
'limitation' => new SubtreeLimitation(array('limitationValues' => array('/1/2/', '/1/43/'))),
- 'object' => new ContentInfo(array('published' => true)),
+ 'object' => new ContentInfo(['published' => true, 'status' => ContentInfo::STATUS_PUBLISHED]),
'targets' => null,
'persistence' => array(new Location(array('pathString' => '/1/55/'))),
'expected' => LimitationType::ACCESS_DENIED,
@@ -342,7 +342,7 @@ public function providerForTestEvaluate()
// ContentInfo, no targets, un-published, with access
array(
'limitation' => new SubtreeLimitation(array('limitationValues' => array('/1/2/'))),
- 'object' => new ContentInfo(array('published' => false)),
+ 'object' => new ContentInfo(['published' => false, 'status' => ContentInfo::STATUS_DRAFT]),
'targets' => null,
'persistence' => array(new Location(array('pathString' => '/1/2/'))),
'expected' => LimitationType::ACCESS_GRANTED,
@@ -350,7 +350,7 @@ public function providerForTestEvaluate()
// ContentInfo, no targets, un-published, no access
array(
'limitation' => new SubtreeLimitation(array('limitationValues' => array('/1/2/', '/1/43/'))),
- 'object' => new ContentInfo(array('published' => false)),
+ 'object' => new ContentInfo(['published' => false, 'status' => ContentInfo::STATUS_DRAFT]),
'targets' => null,
'persistence' => array(new Location(array('pathString' => '/1/55/'))),
'expected' => LimitationType::ACCESS_DENIED,
@@ -406,7 +406,7 @@ public function providerForTestEvaluate()
// invalid target
array(
'limitation' => new SubtreeLimitation(),
- 'object' => new ContentInfo(array('published' => true)),
+ 'object' => new ContentInfo(['published' => true, 'status' => ContentInfo::STATUS_PUBLISHED]),
'targets' => array(new ObjectStateLimitation()),
'persistence' => array(),
'expected' => LimitationType::ACCESS_ABSTAIN,
diff --git a/eZ/Publish/Core/Repository/ContentService.php b/eZ/Publish/Core/Repository/ContentService.php
index 9506eec41c2..47f7e6336c6 100644
--- a/eZ/Publish/Core/Repository/ContentService.php
+++ b/eZ/Publish/Core/Repository/ContentService.php
@@ -1111,6 +1111,7 @@ public function loadContentDrafts(User $user = null)
$versionInfoList = array();
foreach ($spiVersionInfoList as $spiVersionInfo) {
$versionInfo = $this->domainMapper->buildVersionInfoDomainObject($spiVersionInfo);
+ // @todo: Change this to filter returned drafts by permissions instead of throwing
if (!$this->repository->canUser('content', 'versionread', $versionInfo)) {
throw new UnauthorizedException('content', 'versionread', array('contentId' => $versionInfo->contentInfo->id));
}
diff --git a/eZ/Publish/Core/Repository/ContentTypeService.php b/eZ/Publish/Core/Repository/ContentTypeService.php
index 765334005e1..828dbe6c9bc 100644
--- a/eZ/Publish/Core/Repository/ContentTypeService.php
+++ b/eZ/Publish/Core/Repository/ContentTypeService.php
@@ -116,7 +116,7 @@ public function __construct(
*/
public function createContentTypeGroup(ContentTypeGroupCreateStruct $contentTypeGroupCreateStruct)
{
- if ($this->repository->hasAccess('class', 'create') !== true) {
+ if (!$this->repository->canUser('class', 'create', $contentTypeGroupCreateStruct)) {
throw new UnauthorizedException('ContentType', 'create');
}
@@ -235,7 +235,7 @@ public function loadContentTypeGroups()
*/
public function updateContentTypeGroup(APIContentTypeGroup $contentTypeGroup, ContentTypeGroupUpdateStruct $contentTypeGroupUpdateStruct)
{
- if ($this->repository->hasAccess('class', 'update') !== true) {
+ if (!$this->repository->canUser('class', 'update', $contentTypeGroup)) {
throw new UnauthorizedException('ContentType', 'update');
}
@@ -298,7 +298,7 @@ public function updateContentTypeGroup(APIContentTypeGroup $contentTypeGroup, Co
*/
public function deleteContentTypeGroup(APIContentTypeGroup $contentTypeGroup)
{
- if ($this->repository->hasAccess('class', 'delete') !== true) {
+ if (!$this->repository->canUser('class', 'delete', $contentTypeGroup)) {
throw new UnauthorizedException('ContentType', 'delete');
}
@@ -655,7 +655,7 @@ protected function validateInputFieldDefinitionCreateStruct(
*/
public function createContentType(APIContentTypeCreateStruct $contentTypeCreateStruct, array $contentTypeGroups)
{
- if ($this->repository->hasAccess('class', 'create') !== true) {
+ if (!$this->repository->canUser('class', 'create', $contentTypeCreateStruct, $contentTypeGroups)) {
throw new UnauthorizedException('ContentType', 'create');
}
@@ -1022,7 +1022,7 @@ function ($id) {
*/
public function createContentTypeDraft(APIContentType $contentType)
{
- if ($this->repository->hasAccess('class', 'create') !== true) {
+ if (!$this->repository->canUser('class', 'create', $contentType)) {
throw new UnauthorizedException('ContentType', 'create');
}
@@ -1075,7 +1075,7 @@ function ($id) {
*/
public function updateContentTypeDraft(APIContentTypeDraft $contentTypeDraft, ContentTypeUpdateStruct $contentTypeUpdateStruct)
{
- if ($this->repository->hasAccess('class', 'update') !== true) {
+ if (!$this->repository->canUser('class', 'update', $contentTypeDraft)) {
throw new UnauthorizedException('ContentType', 'update');
}
@@ -1149,7 +1149,7 @@ public function updateContentTypeDraft(APIContentTypeDraft $contentTypeDraft, Co
*/
public function deleteContentType(APIContentType $contentType)
{
- if ($this->repository->hasAccess('class', 'delete') !== true) {
+ if (!$this->repository->canUser('class', 'delete', $contentType)) {
throw new UnauthorizedException('ContentType', 'delete');
}
@@ -1189,7 +1189,7 @@ public function deleteContentType(APIContentType $contentType)
*/
public function copyContentType(APIContentType $contentType, User $creator = null)
{
- if ($this->repository->hasAccess('class', 'create') !== true) {
+ if (!$this->repository->canUser('class', 'create', $contentType)) {
throw new UnauthorizedException('ContentType', 'create');
}
@@ -1224,7 +1224,7 @@ public function copyContentType(APIContentType $contentType, User $creator = nul
*/
public function assignContentTypeGroup(APIContentType $contentType, APIContentTypeGroup $contentTypeGroup)
{
- if ($this->repository->hasAccess('class', 'update') !== true) {
+ if (!$this->repository->canUser('class', 'update', $contentType)) {
throw new UnauthorizedException('ContentType', 'update');
}
@@ -1266,7 +1266,7 @@ public function assignContentTypeGroup(APIContentType $contentType, APIContentTy
*/
public function unassignContentTypeGroup(APIContentType $contentType, APIContentTypeGroup $contentTypeGroup)
{
- if ($this->repository->hasAccess('class', 'update') !== true) {
+ if (!$this->repository->canUser('class', 'update', $contentType, [$contentTypeGroup])) {
throw new UnauthorizedException('ContentType', 'update');
}
@@ -1322,7 +1322,7 @@ public function unassignContentTypeGroup(APIContentType $contentType, APIContent
*/
public function addFieldDefinition(APIContentTypeDraft $contentTypeDraft, FieldDefinitionCreateStruct $fieldDefinitionCreateStruct)
{
- if ($this->repository->hasAccess('class', 'update') !== true) {
+ if (!$this->repository->canUser('class', 'update', $contentTypeDraft)) {
throw new UnauthorizedException('ContentType', 'update');
}
@@ -1395,7 +1395,7 @@ public function addFieldDefinition(APIContentTypeDraft $contentTypeDraft, FieldD
*/
public function removeFieldDefinition(APIContentTypeDraft $contentTypeDraft, APIFieldDefinition $fieldDefinition)
{
- if ($this->repository->hasAccess('class', 'update') !== true) {
+ if (!$this->repository->canUser('class', 'update', $contentTypeDraft)) {
throw new UnauthorizedException('ContentType', 'update');
}
@@ -1439,7 +1439,7 @@ public function removeFieldDefinition(APIContentTypeDraft $contentTypeDraft, API
*/
public function updateFieldDefinition(APIContentTypeDraft $contentTypeDraft, APIFieldDefinition $fieldDefinition, FieldDefinitionUpdateStruct $fieldDefinitionUpdateStruct)
{
- if ($this->repository->hasAccess('class', 'update') !== true) {
+ if (!$this->repository->canUser('class', 'update', $contentTypeDraft)) {
throw new UnauthorizedException('ContentType', 'update');
}
@@ -1494,7 +1494,7 @@ public function updateFieldDefinition(APIContentTypeDraft $contentTypeDraft, API
*/
public function publishContentTypeDraft(APIContentTypeDraft $contentTypeDraft)
{
- if ($this->repository->hasAccess('class', 'update') !== true) {
+ if (!$this->repository->canUser('class', 'update', $contentTypeDraft)) {
throw new UnauthorizedException('ContentType', 'update');
}
diff --git a/eZ/Publish/Core/Repository/LanguageService.php b/eZ/Publish/Core/Repository/LanguageService.php
index 339b716b5b8..4e6b9a82012 100644
--- a/eZ/Publish/Core/Repository/LanguageService.php
+++ b/eZ/Publish/Core/Repository/LanguageService.php
@@ -83,7 +83,7 @@ public function createLanguage(LanguageCreateStruct $languageCreateStruct)
throw new InvalidArgumentValue('enabled', $languageCreateStruct->enabled, 'LanguageCreateStruct');
}
- if ($this->repository->hasAccess('content', 'translations') !== true) {
+ if (!$this->repository->canUser('content', 'translations', $languageCreateStruct)) {
throw new UnauthorizedException('content', 'translations');
}
@@ -133,7 +133,7 @@ public function updateLanguageName(Language $language, $newName)
throw new InvalidArgumentValue('newName', $newName);
}
- if ($this->repository->hasAccess('content', 'translations') !== true) {
+ if (!$this->repository->canUser('content', 'translations', $language)) {
throw new UnauthorizedException('content', 'translations');
}
@@ -171,7 +171,7 @@ public function updateLanguageName(Language $language, $newName)
*/
public function enableLanguage(Language $language)
{
- if ($this->repository->hasAccess('content', 'translations') !== true) {
+ if (!$this->repository->canUser('content', 'translations', $language)) {
throw new UnauthorizedException('content', 'translations');
}
@@ -209,7 +209,7 @@ public function enableLanguage(Language $language)
*/
public function disableLanguage(Language $language)
{
- if ($this->repository->hasAccess('content', 'translations') !== true) {
+ if (!$this->repository->canUser('content', 'translations', $language)) {
throw new UnauthorizedException('content', 'translations');
}
@@ -303,7 +303,7 @@ public function loadLanguageById($languageId)
*/
public function deleteLanguage(Language $language)
{
- if ($this->repository->hasAccess('content', 'translations') !== true) {
+ if (!$this->repository->canUser('content', 'translations', $language)) {
throw new UnauthorizedException('content', 'translations');
}
diff --git a/eZ/Publish/Core/Repository/ObjectStateService.php b/eZ/Publish/Core/Repository/ObjectStateService.php
index 358ff2d63c0..96bf9a3428e 100644
--- a/eZ/Publish/Core/Repository/ObjectStateService.php
+++ b/eZ/Publish/Core/Repository/ObjectStateService.php
@@ -81,7 +81,7 @@ public function __construct(RepositoryInterface $repository, Handler $objectStat
*/
public function createObjectStateGroup(ObjectStateGroupCreateStruct $objectStateGroupCreateStruct)
{
- if ($this->repository->hasAccess('state', 'administrate') !== true) {
+ if (!$this->repository->canUser('state', 'administrate', $objectStateGroupCreateStruct)) {
throw new UnauthorizedException('state', 'administrate');
}
@@ -182,7 +182,7 @@ public function loadObjectStates(APIObjectStateGroup $objectStateGroup)
*/
public function updateObjectStateGroup(APIObjectStateGroup $objectStateGroup, ObjectStateGroupUpdateStruct $objectStateGroupUpdateStruct)
{
- if ($this->repository->hasAccess('state', 'administrate') !== true) {
+ if (!$this->repository->canUser('state', 'administrate', $objectStateGroup)) {
throw new UnauthorizedException('state', 'administrate');
}
@@ -234,7 +234,7 @@ public function updateObjectStateGroup(APIObjectStateGroup $objectStateGroup, Ob
*/
public function deleteObjectStateGroup(APIObjectStateGroup $objectStateGroup)
{
- if ($this->repository->hasAccess('state', 'administrate') !== true) {
+ if (!$this->repository->canUser('state', 'administrate', $objectStateGroup)) {
throw new UnauthorizedException('state', 'administrate');
}
@@ -266,7 +266,7 @@ public function deleteObjectStateGroup(APIObjectStateGroup $objectStateGroup)
*/
public function createObjectState(APIObjectStateGroup $objectStateGroup, ObjectStateCreateStruct $objectStateCreateStruct)
{
- if ($this->repository->hasAccess('state', 'administrate') !== true) {
+ if (!$this->repository->canUser('state', 'administrate', $objectStateCreateStruct, [$objectStateGroup])) {
throw new UnauthorizedException('state', 'administrate');
}
@@ -340,7 +340,7 @@ public function loadObjectState($stateId)
*/
public function updateObjectState(APIObjectState $objectState, ObjectStateUpdateStruct $objectStateUpdateStruct)
{
- if ($this->repository->hasAccess('state', 'administrate') !== true) {
+ if (!$this->repository->canUser('state', 'administrate', $objectState)) {
throw new UnauthorizedException('state', 'administrate');
}
@@ -401,7 +401,7 @@ public function setPriorityOfObjectState(APIObjectState $objectState, $priority)
throw new InvalidArgumentValue('priority', $priority);
}
- if ($this->repository->hasAccess('state', 'administrate') !== true) {
+ if (!$this->repository->canUser('state', 'administrate', $objectState)) {
throw new UnauthorizedException('state', 'administrate');
}
@@ -430,7 +430,7 @@ public function setPriorityOfObjectState(APIObjectState $objectState, $priority)
*/
public function deleteObjectState(APIObjectState $objectState)
{
- if ($this->repository->hasAccess('state', 'administrate') !== true) {
+ if (!$this->repository->canUser('state', 'administrate', $objectState)) {
throw new UnauthorizedException('state', 'administrate');
}
diff --git a/eZ/Publish/Core/Repository/SectionService.php b/eZ/Publish/Core/Repository/SectionService.php
index a83395b66b2..956f27297f2 100644
--- a/eZ/Publish/Core/Repository/SectionService.php
+++ b/eZ/Publish/Core/Repository/SectionService.php
@@ -33,6 +33,11 @@ class SectionService implements SectionServiceInterface
*/
protected $repository;
+ /**
+ * @var \eZ\Publish\API\Repository\PermissionResolver
+ */
+ protected $permissionResolver;
+
/**
* @var \eZ\Publish\SPI\Persistence\Content\Section\Handler
*/
@@ -53,6 +58,7 @@ class SectionService implements SectionServiceInterface
public function __construct(RepositoryInterface $repository, Handler $sectionHandler, array $settings = array())
{
$this->repository = $repository;
+ $this->permissionResolver = $repository->getPermissionResolver();
$this->sectionHandler = $sectionHandler;
// Union makes sure default settings are ignored if provided in argument
$this->settings = $settings + array(
@@ -80,7 +86,7 @@ public function createSection(SectionCreateStruct $sectionCreateStruct)
throw new InvalidArgumentValue('identifier', $sectionCreateStruct->identifier, 'SectionCreateStruct');
}
- if ($this->repository->hasAccess('section', 'edit') !== true) {
+ if (!$this->permissionResolver->canUser('section', 'edit', $sectionCreateStruct)) {
throw new UnauthorizedException('section', 'edit');
}
@@ -129,7 +135,7 @@ public function updateSection(Section $section, SectionUpdateStruct $sectionUpda
throw new InvalidArgumentValue('identifier', $section->identifier, 'Section');
}
- if ($this->repository->canUser('section', 'edit', $section) !== true) {
+ if (!$this->permissionResolver->canUser('section', 'edit', $section)) {
throw new UnauthorizedException('section', 'edit');
}
@@ -176,13 +182,15 @@ public function updateSection(Section $section, SectionUpdateStruct $sectionUpda
*/
public function loadSection($sectionId)
{
- if ($this->repository->hasAccess('section', 'view') !== true) {
+ $section = $this->buildDomainSectionObject(
+ $this->sectionHandler->load($sectionId)
+ );
+
+ if (!$this->permissionResolver->canUser('section', 'view', $section)) {
throw new UnauthorizedException('section', 'view');
}
- $spiSection = $this->sectionHandler->load($sectionId);
-
- return $this->buildDomainSectionObject($spiSection);
+ return $section;
}
/**
@@ -194,15 +202,14 @@ public function loadSection($sectionId)
*/
public function loadSections()
{
- if ($this->repository->hasAccess('section', 'view') !== true) {
- throw new UnauthorizedException('section', 'view');
- }
-
- $spiSections = $this->sectionHandler->loadAll();
+ $sections = [];
+ foreach ($this->sectionHandler->loadAll() as $spiSection) {
+ $sections[] = $section = $this->buildDomainSectionObject($spiSection);
- $sections = array();
- foreach ($spiSections as $spiSection) {
- $sections[] = $this->buildDomainSectionObject($spiSection);
+ // @todo change API to just filter instead of throwing here
+ if (!$this->permissionResolver->canUser('section', 'view', $section)) {
+ throw new UnauthorizedException('section', 'view');
+ }
}
return $sections;
@@ -224,13 +231,15 @@ public function loadSectionByIdentifier($sectionIdentifier)
throw new InvalidArgumentValue('sectionIdentifier', $sectionIdentifier);
}
- if ($this->repository->hasAccess('section', 'view') !== true) {
+ $section = $this->buildDomainSectionObject(
+ $this->sectionHandler->loadByIdentifier($sectionIdentifier)
+ );
+
+ if (!$this->permissionResolver->canUser('section', 'view', $section)) {
throw new UnauthorizedException('section', 'view');
}
- $spiSection = $this->sectionHandler->loadByIdentifier($sectionIdentifier);
-
- return $this->buildDomainSectionObject($spiSection);
+ return $section;
}
/**
@@ -279,7 +288,7 @@ public function assignSection(ContentInfo $contentInfo, Section $section)
$loadedContentInfo = $this->repository->getContentService()->loadContentInfo($contentInfo->id);
$loadedSection = $this->loadSection($section->id);
- if ($this->repository->canUser('section', 'assign', $loadedContentInfo, $loadedSection) !== true) {
+ if (!$this->permissionResolver->canUser('section', 'assign', $loadedContentInfo, [$loadedSection])) {
throw new UnauthorizedException(
'section',
'assign',
@@ -318,7 +327,7 @@ public function deleteSection(Section $section)
{
$loadedSection = $this->loadSection($section->id);
- if ($this->repository->canUser('section', 'edit', $loadedSection) !== true) {
+ if (!$this->permissionResolver->canUser('section', 'edit', $loadedSection)) {
throw new UnauthorizedException('section', 'edit', array('sectionId' => $loadedSection->id));
}
diff --git a/eZ/Publish/Core/Repository/Tests/Service/Integration/TrashBase.php b/eZ/Publish/Core/Repository/Tests/Service/Integration/TrashBase.php
index cc1d66fc7d8..735bdd32d2f 100644
--- a/eZ/Publish/Core/Repository/Tests/Service/Integration/TrashBase.php
+++ b/eZ/Publish/Core/Repository/Tests/Service/Integration/TrashBase.php
@@ -8,6 +8,7 @@
*/
namespace eZ\Publish\Core\Repository\Tests\Service\Integration;
+use eZ\Publish\API\Repository\Values\Content\ContentInfo;
use eZ\Publish\Core\Repository\Tests\Service\Integration\Base as BaseServiceTest;
use eZ\Publish\Core\Repository\Values\Content\TrashItem;
use eZ\Publish\Core\Repository\Values\Content\Location;
@@ -288,8 +289,9 @@ public function testRecoverNonExistingTrashItem()
{
$trashService = $this->repository->getTrashService();
- $trashItem = new TrashItem(array('id' => APIBaseTest::DB_INT_MAX, 'parentLocationId' => APIBaseTest::DB_INT_MAX));
- $trashService->recover($trashItem);
+ $trashService->recover(
+ $this->getTrashItem(APIBaseTest::DB_INT_MAX, 2, APIBaseTest::DB_INT_MAX)
+ );
}
/**
@@ -398,8 +400,9 @@ public function testDeleteNonExistingTrashItem()
{
$trashService = $this->repository->getTrashService();
- $trashItem = new TrashItem(array('id' => APIBaseTest::DB_INT_MAX));
- $trashService->deleteTrashItem($trashItem);
+ $trashService->deleteTrashItem(
+ $this->getTrashItem(APIBaseTest::DB_INT_MAX)
+ );
}
/**
@@ -472,4 +475,13 @@ protected function createTestContentLocation($parentLocationId)
)->versionInfo
);
}
+
+ private function getTrashItem($id, $contentId = 44, $parentLocationId = 2)
+ {
+ return new TrashItem([
+ 'id' => $id,
+ 'parentLocationId' => $parentLocationId,
+ 'contentInfo' => new ContentInfo(['id' => $contentId]),
+ ]);
+ }
}
diff --git a/eZ/Publish/Core/Repository/Tests/Service/Mock/UrlWildcardTest.php b/eZ/Publish/Core/Repository/Tests/Service/Mock/UrlWildcardTest.php
index 8b1b0616666..00c4503fa6e 100644
--- a/eZ/Publish/Core/Repository/Tests/Service/Mock/UrlWildcardTest.php
+++ b/eZ/Publish/Core/Repository/Tests/Service/Mock/UrlWildcardTest.php
@@ -6,7 +6,7 @@
* @copyright Copyright (C) eZ Systems AS. All rights reserved.
* @license For full copyright and license information view LICENSE file distributed with this source code.
*/
-namespace eZ\Publish\Core\Repository\Tests\Service\Integration;
+namespace eZ\Publish\Core\Repository\Tests\Service\Mock;
use eZ\Publish\Core\Repository\Tests\Service\Mock\Base as BaseServiceMockTest;
use eZ\Publish\API\Repository\Values\Content\URLWildcard;
@@ -295,20 +295,25 @@ public function testCreateWithRollback()
*/
public function testRemoveThrowsUnauthorizedException()
{
+ $wildcard = new URLWildcard(['id' => 'McBoom']);
+
$mockedService = $this->getPartlyMockedURLWildcardService();
$repositoryMock = $this->getRepositoryMock();
$repositoryMock->expects(
$this->once()
)->method(
- 'hasAccess'
+ 'canUser'
)->with(
$this->equalTo('content'),
- $this->equalTo('urltranslator')
+ $this->equalTo('urltranslator'),
+ $this->equalTo($wildcard)
)->will(
$this->returnValue(false)
);
- $mockedService->remove(new URLWildcard());
+ $repositoryMock->expects($this->never())->method('beginTransaction');
+
+ $mockedService->remove($wildcard);
}
/**
@@ -319,6 +324,8 @@ public function testRemoveThrowsUnauthorizedException()
*/
public function testRemove()
{
+ $wildcard = new URLWildcard(['id' => 'McBomb']);
+
$mockedService = $this->getPartlyMockedURLWildcardService();
/** @var \PHPUnit_Framework_MockObject_MockObject $handlerMock */
$handlerMock = $this->getPersistenceMock()->urlWildcardHandler();
@@ -327,10 +334,11 @@ public function testRemove()
$repositoryMock->expects(
$this->once()
)->method(
- 'hasAccess'
+ 'canUser'
)->with(
$this->equalTo('content'),
- $this->equalTo('urltranslator')
+ $this->equalTo('urltranslator'),
+ $this->equalTo($wildcard)
)->will(
$this->returnValue(true)
);
@@ -346,7 +354,7 @@ public function testRemove()
$this->equalTo('McBomb')
);
- $mockedService->remove(new URLWildcard(array('id' => 'McBomb')));
+ $mockedService->remove($wildcard);
}
/**
@@ -358,6 +366,8 @@ public function testRemove()
*/
public function testRemoveWithRollback()
{
+ $wildcard = new URLWildcard(['id' => 'McBoo']);
+
$mockedService = $this->getPartlyMockedURLWildcardService();
/** @var \PHPUnit_Framework_MockObject_MockObject $handlerMock */
$handlerMock = $this->getPersistenceMock()->urlWildcardHandler();
@@ -366,10 +376,11 @@ public function testRemoveWithRollback()
$repositoryMock->expects(
$this->once()
)->method(
- 'hasAccess'
+ 'canUser'
)->with(
$this->equalTo('content'),
- $this->equalTo('urltranslator')
+ $this->equalTo('urltranslator'),
+ $this->equalTo($wildcard)
)->will(
$this->returnValue(true)
);
@@ -387,7 +398,7 @@ public function testRemoveWithRollback()
$this->throwException(new \Exception())
);
- $mockedService->remove(new URLWildcard(array('id' => 'McBoo')));
+ $mockedService->remove($wildcard);
}
/**
diff --git a/eZ/Publish/Core/Repository/TrashService.php b/eZ/Publish/Core/Repository/TrashService.php
index d7efaeea01e..7cd1728870a 100644
--- a/eZ/Publish/Core/Repository/TrashService.php
+++ b/eZ/Publish/Core/Repository/TrashService.php
@@ -86,16 +86,16 @@ public function __construct(
*/
public function loadTrashItem($trashItemId)
{
- if ($this->repository->hasAccess('content', 'restore') !== true) {
- throw new UnauthorizedException('content', 'restore');
- }
-
$spiTrashItem = $this->persistenceHandler->trashHandler()->loadTrashItem($trashItemId);
$trash = $this->buildDomainTrashItemObject($spiTrashItem);
if (!$this->repository->canUser('content', 'read', $trash->getContentInfo())) {
throw new UnauthorizedException('content', 'read');
}
+ if (!$this->repository->canUser('content', 'restore', $trash->getContentInfo())) {
+ throw new UnauthorizedException('content', 'restore');
+ }
+
return $trash;
}
@@ -117,7 +117,7 @@ public function trash(Location $location)
throw new InvalidArgumentValue('id', $location->id, 'Location');
}
- if (!$this->repository->canUser('content', 'remove', $location->getContentInfo(), $location)) {
+ if (!$this->repository->canUser('content', 'remove', $location->getContentInfo(), [$location])) {
throw new UnauthorizedException('content', 'remove');
}
@@ -171,7 +171,12 @@ public function recover(APITrashItem $trashItem, Location $newParentLocation = n
throw new InvalidArgumentValue('parentLocationId', $newParentLocation->id, 'Location');
}
- if ($this->repository->hasAccess('content', 'restore') !== true) {
+ if (!$this->repository->canUser(
+ 'content',
+ 'restore',
+ $trashItem->getContentInfo(),
+ [$newParentLocation ?: $trashItem]
+ )) {
throw new UnauthorizedException('content', 'restore');
}
@@ -216,6 +221,9 @@ public function recover(APITrashItem $trashItem, Location $newParentLocation = n
*/
public function emptyTrash()
{
+ // Will throw if you have Role assignment limitation where you have content/cleantrash permission.
+ // This is by design and means you can only delete one and one trash item, or you'll need to change how
+ // permissions is assigned to be on separate role with no Role assignment limitation.
if ($this->repository->hasAccess('content', 'cleantrash') !== true) {
throw new UnauthorizedException('content', 'cleantrash');
}
@@ -242,7 +250,7 @@ public function emptyTrash()
*/
public function deleteTrashItem(APITrashItem $trashItem)
{
- if ($this->repository->hasAccess('content', 'cleantrash') !== true) {
+ if (!$this->repository->canUser('content', 'cleantrash', $trashItem->getContentInfo())) {
throw new UnauthorizedException('content', 'cleantrash');
}
diff --git a/eZ/Publish/Core/Repository/URLWildcardService.php b/eZ/Publish/Core/Repository/URLWildcardService.php
index 08701028974..cc8d30bd698 100644
--- a/eZ/Publish/Core/Repository/URLWildcardService.php
+++ b/eZ/Publish/Core/Repository/URLWildcardService.php
@@ -139,7 +139,7 @@ protected function cleanUrl($url)
*/
public function remove(URLWildcard $urlWildcard)
{
- if ($this->repository->hasAccess('content', 'urltranslator') !== true) {
+ if (!$this->repository->canUser('content', 'urltranslator', $urlWildcard)) {
throw new UnauthorizedException('content', 'urltranslator');
}