Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IBX-6592: Content Object State assignment should rely on Location instead of ContentInfo #391

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions eZ/Publish/API/Repository/ObjectStateService.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
namespace eZ\Publish\API\Repository;

use eZ\Publish\API\Repository\Values\Content\ContentInfo;
use eZ\Publish\API\Repository\Values\Content\Location;
use eZ\Publish\API\Repository\Values\ObjectState\ObjectState;
use eZ\Publish\API\Repository\Values\ObjectState\ObjectStateCreateStruct;
use eZ\Publish\API\Repository\Values\ObjectState\ObjectStateGroup;
Expand Down Expand Up @@ -172,12 +173,13 @@ public function deleteObjectState(ObjectState $objectState): void;
*
* @throws \eZ\Publish\API\Repository\Exceptions\InvalidArgumentException if the object state does not belong to the given group
* @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException if the user is not allowed to change the object state
*
* @param \eZ\Publish\API\Repository\Values\Content\ContentInfo $contentInfo
* @param \eZ\Publish\API\Repository\Values\ObjectState\ObjectStateGroup $objectStateGroup
* @param \eZ\Publish\API\Repository\Values\ObjectState\ObjectState $objectState
*/
public function setContentState(ContentInfo $contentInfo, ObjectStateGroup $objectStateGroup, ObjectState $objectState): void;
public function setContentState(
ContentInfo $contentInfo,
ObjectStateGroup $objectStateGroup,
ObjectState $objectState,
?Location $location = null
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary? This Location parameter does not affect method logic and is just workaround for PermissionResolver. It is also security risk, as not setting this parameter will omit some of limitations checks.

): void;

/**
* Gets the object-state of object identified by $contentId.
Expand Down
6 changes: 4 additions & 2 deletions eZ/Publish/Core/Event/ObjectStateService.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
use eZ\Publish\API\Repository\Events\ObjectState\UpdateObjectStateGroupEvent;
use eZ\Publish\API\Repository\ObjectStateService as ObjectStateServiceInterface;
use eZ\Publish\API\Repository\Values\Content\ContentInfo;
use eZ\Publish\API\Repository\Values\Content\Location;
use eZ\Publish\API\Repository\Values\ObjectState\ObjectState;
use eZ\Publish\API\Repository\Values\ObjectState\ObjectStateCreateStruct;
use eZ\Publish\API\Repository\Values\ObjectState\ObjectStateGroup;
Expand Down Expand Up @@ -214,7 +215,8 @@ public function deleteObjectState(ObjectState $objectState): void
public function setContentState(
ContentInfo $contentInfo,
ObjectStateGroup $objectStateGroup,
ObjectState $objectState
ObjectState $objectState,
?Location $location = null
): void {
$eventData = [
$contentInfo,
Expand All @@ -229,7 +231,7 @@ public function setContentState(
return;
}

$this->innerService->setContentState($contentInfo, $objectStateGroup, $objectState);
$this->innerService->setContentState($contentInfo, $objectStateGroup, $objectState, $location);

$this->eventDispatcher->dispatch(
new SetContentStateEvent(...$eventData)
Expand Down
8 changes: 4 additions & 4 deletions eZ/Publish/Core/Limitation/NewObjectStateLimitationType.php
Original file line number Diff line number Diff line change
Expand Up @@ -127,11 +127,11 @@ public function evaluate(APILimitationValue $value, APIUserReference $currentUse
return false;
}

foreach ($targets as $target) {
if (!$target instanceof ObjectState && !$target instanceof SPIObjectState) {
throw new InvalidArgumentException('$targets', 'Must contain ObjectState objects');
}
$targets = array_filter($targets, static function ($target) {
return $target instanceof ObjectState || $target instanceof SPIObjectState;
});

foreach ($targets as $target) {
if (!in_array($target->id, $value->limitationValues)) {
return false;
}
Expand Down
16 changes: 9 additions & 7 deletions eZ/Publish/Core/Repository/ObjectStateService.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use eZ\Publish\API\Repository\PermissionResolver;
use eZ\Publish\API\Repository\Repository as RepositoryInterface;
use eZ\Publish\API\Repository\Values\Content\ContentInfo;
use eZ\Publish\API\Repository\Values\Content\Location;
use eZ\Publish\API\Repository\Values\ObjectState\ObjectState as APIObjectState;
use eZ\Publish\API\Repository\Values\ObjectState\ObjectStateCreateStruct;
use eZ\Publish\API\Repository\Values\ObjectState\ObjectStateGroup as APIObjectStateGroup;
Expand Down Expand Up @@ -463,14 +464,15 @@ public function deleteObjectState(APIObjectState $objectState): void
*
* @throws \eZ\Publish\API\Repository\Exceptions\InvalidArgumentException if the object state does not belong to the given group
* @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException if the user is not allowed to change the object state
*
* @param \eZ\Publish\API\Repository\Values\Content\ContentInfo $contentInfo
* @param \eZ\Publish\API\Repository\Values\ObjectState\ObjectStateGroup $objectStateGroup
* @param \eZ\Publish\API\Repository\Values\ObjectState\ObjectState $objectState
*/
public function setContentState(ContentInfo $contentInfo, APIObjectStateGroup $objectStateGroup, APIObjectState $objectState): void
{
if (!$this->permissionResolver->canUser('state', 'assign', $contentInfo, [$objectState])) {
public function setContentState(
ContentInfo $contentInfo,
APIObjectStateGroup $objectStateGroup,
APIObjectState $objectState,
?Location $location = null
): void {
$targets = $location !== null ? [$location, $objectState] : [$objectState];
if (!$this->permissionResolver->canUser('state', 'assign', $contentInfo, $targets)) {
throw new UnauthorizedException('state', 'assign', ['contentId' => $contentInfo->id]);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use eZ\Publish\API\Repository\LanguageResolver;
use eZ\Publish\API\Repository\ObjectStateService as ObjectStateServiceInterface;
use eZ\Publish\API\Repository\Values\Content\ContentInfo;
use eZ\Publish\API\Repository\Values\Content\Location;
use eZ\Publish\API\Repository\Values\ObjectState\ObjectState;
use eZ\Publish\API\Repository\Values\ObjectState\ObjectStateCreateStruct;
use eZ\Publish\API\Repository\Values\ObjectState\ObjectStateGroup;
Expand Down Expand Up @@ -127,9 +128,13 @@ public function deleteObjectState(ObjectState $objectState): void
$this->service->deleteObjectState($objectState);
}

public function setContentState(ContentInfo $contentInfo, ObjectStateGroup $objectStateGroup, ObjectState $objectState): void
{
$this->service->setContentState($contentInfo, $objectStateGroup, $objectState);
public function setContentState(
ContentInfo $contentInfo,
ObjectStateGroup $objectStateGroup,
ObjectState $objectState,
?Location $location = null
): void {
$this->service->setContentState($contentInfo, $objectStateGroup, $objectState, $location);
}

public function getContentState(ContentInfo $contentInfo, ObjectStateGroup $objectStateGroup): ObjectState
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

use eZ\Publish\API\Repository\ObjectStateService;
use eZ\Publish\API\Repository\Values\Content\ContentInfo;
use eZ\Publish\API\Repository\Values\Content\Location;
use eZ\Publish\API\Repository\Values\ObjectState\ObjectState;
use eZ\Publish\API\Repository\Values\ObjectState\ObjectStateCreateStruct;
use eZ\Publish\API\Repository\Values\ObjectState\ObjectStateGroup;
Expand Down Expand Up @@ -121,9 +122,10 @@ public function deleteObjectState(ObjectState $objectState): void
public function setContentState(
ContentInfo $contentInfo,
ObjectStateGroup $objectStateGroup,
ObjectState $objectState
ObjectState $objectState,
?Location $location = null
): void {
$this->innerService->setContentState($contentInfo, $objectStateGroup, $objectState);
$this->innerService->setContentState($contentInfo, $objectStateGroup, $objectState, $location);
}

public function getContentState(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
<?php

/**
* @copyright Copyright (C) Ibexa AS. All rights reserved.
* @license For full copyright and license information view LICENSE file distributed with this source code.
*/
declare(strict_types=1);

namespace Ibexa\Tests\Integration\Core\Repository\ObjectStateService;

use eZ\Publish\API\Repository\Exceptions\UnauthorizedException;
use eZ\Publish\API\Repository\Values\User\Limitation\ObjectStateLimitation;
use eZ\Publish\API\Repository\Values\User\Limitation\SubtreeLimitation;
use Ibexa\Tests\Integration\Core\RepositoryTestCase;

/**
* @covers \eZ\Publish\API\Repository\ObjectStateService
*/
final class SetContentStateTest extends RepositoryTestCase
{
/**
* @dataProvider dataProviderForTestSetContentObjectStateWithSubtreeLimitation
*/
public function testSetContentObjectStateWithSubtreeLimitation(
?string $subtreeLimitationValue,
bool $isInsideLimitation
): void {
$permissionResolver = self::getPermissionResolver();
$objectStateService = self::getObjectStateService();

$objectState = $objectStateService->loadObjectState(2);

$subtreeLimitationFolder = $this->createFolder(['eng-GB' => 'Subtree limitation type'], 2);
$contentInfo = $subtreeLimitationFolder->getVersionInfo()->getContentInfo();
$mainLocation = $contentInfo->getMainLocation();

$limitations = [
new SubtreeLimitation(
[
'limitationValues' => [$subtreeLimitationValue ?? $mainLocation->getPathString()],
],
),
new ObjectStateLimitation(
[
'limitationValues' => [1, 2],
],
),
];

$user = $this->createUserWithPolicies(
'object_state_user',
[
['module' => 'content', 'function' => '*'],
['module' => 'state', 'function' => 'assign', 'limitations' => $limitations],
]
);

$permissionResolver->setCurrentUserReference($user);

$childFolder = $this->createFolder(['eng-GB' => 'Child folder'], $mainLocation->id);
$childContentInfo = $childFolder->getVersionInfo()->getContentInfo();

if (!$isInsideLimitation) {
self::expectException(UnauthorizedException::class);
}

$objectStateService->setContentState(
$childContentInfo,
$objectState->getObjectStateGroup(),
$objectState,
$childContentInfo->getMainLocation(),
);

$contentState = $objectStateService->getContentState($childContentInfo, $objectState->getObjectStateGroup());

self::assertSame($objectState->identifier, $contentState->identifier);
}

public function dataProviderForTestSetContentObjectStateWithSubtreeLimitation(): iterable
{
yield 'inside subtree limitation' => [
null,
true,
];

yield 'outside limitation passes' => [
'/1/43',
false,
];
}
}
72 changes: 72 additions & 0 deletions tests/integration/Core/RepositoryTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
namespace Ibexa\Tests\Integration\Core;

use eZ\Publish\API\Repository\Values\Content\Content;
use eZ\Publish\API\Repository\Values\User\Role;
use eZ\Publish\API\Repository\Values\User\User;
use Ibexa\Contracts\Core\Test\IbexaKernelTestCase;
use InvalidArgumentException;

Expand Down Expand Up @@ -70,4 +72,74 @@ public function createFolderDraft(array $names, int $parentLocationId = self::CO
]
);
}

/**
* @param array<mixed> $policiesData
*
* @throws \eZ\Publish\API\Repository\Exceptions\LimitationValidationException
* @throws \eZ\Publish\API\Repository\Exceptions\NotFoundException
* @throws \eZ\Publish\API\Repository\Exceptions\InvalidArgumentException
* @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException
*/
public function createRoleWithPolicies(string $roleName, array $policiesData): Role
{
$roleService = self::getRoleService();

$roleCreateStruct = $roleService->newRoleCreateStruct($roleName);
foreach ($policiesData as $policyData) {
$policyCreateStruct = $roleService->newPolicyCreateStruct(
$policyData['module'],
$policyData['function']
);

if (isset($policyData['limitations'])) {
foreach ($policyData['limitations'] as $limitation) {
$policyCreateStruct->addLimitation($limitation);
}
}

$roleCreateStruct->addPolicy($policyCreateStruct);
}

$roleDraft = $roleService->createRole($roleCreateStruct);

$roleService->publishRoleDraft($roleDraft);

return $roleService->loadRole($roleDraft->id);
}

/**
* @param array<array<array-key, array<string|array<\eZ\Publish\API\Repository\Values\User\Limitation>>>> $policiesData
*
* @throws \eZ\Publish\API\Repository\Exceptions\LimitationValidationException
* @throws \eZ\Publish\API\Repository\Exceptions\NotFoundException
* @throws \eZ\Publish\API\Repository\Exceptions\InvalidArgumentException
* @throws \eZ\Publish\API\Repository\Exceptions\UnauthorizedException
* @throws \eZ\Publish\API\Repository\Exceptions\ContentFieldValidationException
* @throws \eZ\Publish\API\Repository\Exceptions\ContentValidationException
*/
public function createUserWithPolicies(string $login, array $policiesData): User
{
$roleService = self::getRoleService();
$userService = self::getUserService();

$userCreateStruct = $userService->newUserCreateStruct(
$login,
"{$login}@test.dev",
$login,
'eng-GB'
);

$userCreateStruct->setField('first_name', $login);
$userCreateStruct->setField('last_name', $login);
$user = $userService->createUser($userCreateStruct, []);

$role = $this->createRoleWithPolicies(
uniqid('role_for_' . $login . '_', true),
$policiesData
);
$roleService->assignRoleToUser($role, $user);

return $user;
}
}
Loading