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-6217: Forced RichText\Rendererto utilize PermissionResolver instead of AuthorizationChecker #118

Merged
merged 10 commits into from
Aug 16, 2023
30 changes: 0 additions & 30 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -2965,36 +2965,6 @@ parameters:
count: 1
path: tests/lib/RichText/RendererTest.php

-
message: "#^Property Ibexa\\\\Tests\\\\FieldTypeRichText\\\\RichText\\\\RendererTest\\:\\:\\$authorizationCheckerMock \\(PHPUnit\\\\Framework\\\\MockObject\\\\MockObject&Symfony\\\\Component\\\\Security\\\\Core\\\\Authorization\\\\AuthorizationCheckerInterface\\) does not accept PHPUnit\\\\Framework\\\\MockObject\\\\MockObject\\.$#"
count: 1
path: tests/lib/RichText/RendererTest.php

-
message: "#^Property Ibexa\\\\Tests\\\\FieldTypeRichText\\\\RichText\\\\RendererTest\\:\\:\\$configResolverMock \\(PHPUnit\\\\Framework\\\\MockObject\\\\MockObject&Psr\\\\Log\\\\LoggerInterface\\) does not accept PHPUnit\\\\Framework\\\\MockObject\\\\MockObject\\.$#"
count: 1
path: tests/lib/RichText/RendererTest.php

-
message: "#^Property Ibexa\\\\Tests\\\\FieldTypeRichText\\\\RichText\\\\RendererTest\\:\\:\\$loaderMock \\(PHPUnit\\\\Framework\\\\MockObject\\\\MockObject&Twig\\\\Loader\\\\LoaderInterface\\) does not accept PHPUnit\\\\Framework\\\\MockObject\\\\MockObject\\.$#"
count: 1
path: tests/lib/RichText/RendererTest.php

-
message: "#^Property Ibexa\\\\Tests\\\\FieldTypeRichText\\\\RichText\\\\RendererTest\\:\\:\\$loggerMock \\(PHPUnit\\\\Framework\\\\MockObject\\\\MockObject&Psr\\\\Log\\\\LoggerInterface\\) does not accept PHPUnit\\\\Framework\\\\MockObject\\\\MockObject\\.$#"
count: 1
path: tests/lib/RichText/RendererTest.php

-
message: "#^Property Ibexa\\\\Tests\\\\FieldTypeRichText\\\\RichText\\\\RendererTest\\:\\:\\$repositoryMock \\(Ibexa\\\\Contracts\\\\Core\\\\Repository\\\\Repository&PHPUnit\\\\Framework\\\\MockObject\\\\MockObject\\) does not accept PHPUnit\\\\Framework\\\\MockObject\\\\MockObject\\.$#"
count: 1
path: tests/lib/RichText/RendererTest.php

-
message: "#^Property Ibexa\\\\Tests\\\\FieldTypeRichText\\\\RichText\\\\RendererTest\\:\\:\\$templateEngineMock \\(PHPUnit\\\\Framework\\\\MockObject\\\\MockObject&Symfony\\\\Component\\\\Templating\\\\EngineInterface\\) does not accept PHPUnit\\\\Framework\\\\MockObject\\\\MockObject\\.$#"
count: 1
path: tests/lib/RichText/RendererTest.php

-
message: "#^Method Ibexa\\\\Tests\\\\FieldTypeRichText\\\\RichText\\\\Validator\\\\CustomTagsValidatorTest\\:\\:providerForTestValidateDocument\\(\\) return type has no value type specified in iterable type array\\.$#"
count: 1
Expand Down
21 changes: 10 additions & 11 deletions src/bundle/Resources/config/fieldtype_services.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -31,18 +31,17 @@ services:
http://ibexa.co/namespaces/ezpublish5/xhtml5/edit: '@ibexa.richtext.converter.input.xhtml5'

Ibexa\FieldTypeRichText\RichText\Renderer:
class: Ibexa\FieldTypeRichText\RichText\Renderer
arguments:
- '@ibexa.api.repository'
- '@security.authorization_checker'
- '@ibexa.config.resolver'
- '@twig'
- '%ibexa.field_type.richtext.tag.namespace%'
- '%ibexa.field_type.richtext.style.namespace%'
- '%ibexa.field_type.richtext.embed.namespace%'
- '@?logger'
- '%ibexa.field_type.richtext.custom_tags%'
- '%ibexa.field_type.richtext.custom_styles%'
$repository: '@ibexa.api.repository'
$configResolver: '@ibexa.config.resolver'
$templateEngine: '@twig'
$permissionResolver: '@Ibexa\Contracts\Core\Repository\PermissionResolver'
$tagConfigurationNamespace: '%ibexa.field_type.richtext.tag.namespace%'
$styleConfigurationNamespace: '%ibexa.field_type.richtext.style.namespace%'
$embedConfigurationNamespace: '%ibexa.field_type.richtext.embed.namespace%'
$logger: '@?logger'
$customTagsConfiguration: '%ibexa.field_type.richtext.custom_tags%'
$customStylesConfiguration: '%ibexa.field_type.richtext.custom_styles%'

Ibexa\FieldTypeRichText\RichText\Converter\Link:
class: Ibexa\FieldTypeRichText\RichText\Converter\Link
Expand Down
69 changes: 22 additions & 47 deletions src/lib/RichText/Renderer.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,14 @@

use Exception;
use Ibexa\Contracts\Core\Repository\Exceptions\NotFoundException;
use Ibexa\Contracts\Core\Repository\PermissionResolver;
use Ibexa\Contracts\Core\Repository\Repository;
use Ibexa\Contracts\Core\Repository\Values\Content\Content;
use Ibexa\Contracts\Core\SiteAccess\ConfigResolverInterface;
use Ibexa\Contracts\FieldTypeRichText\RichText\RendererInterface;
use Ibexa\Core\MVC\Symfony\Security\Authorization\Attribute as AuthorizationAttribute;
use Psr\Log\LoggerInterface;
use Psr\Log\NullLogger;
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
use Symfony\Component\Security\Core\Exception\AccessDeniedException;
use Twig\Environment;

Expand All @@ -35,10 +34,7 @@ class Renderer implements RendererInterface
*/
protected $repository;

/**
* @var \Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface
*/
private $authorizationChecker;
private PermissionResolver $permissionResolver;

/**
* @var string
Expand Down Expand Up @@ -80,34 +76,22 @@ class Renderer implements RendererInterface
*/
private $customStylesConfiguration;

/**
* @param \Ibexa\Contracts\Core\Repository\Repository $repository
* @param \Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface $authorizationChecker
* @param \Ibexa\Contracts\Core\SiteAccess\ConfigResolverInterface $configResolver
* @param \Twig\Environment $templateEngine
* @param string $tagConfigurationNamespace
* @param string $styleConfigurationNamespace
* @param string $embedConfigurationNamespace
* @param \Psr\Log\LoggerInterface|null $logger
* @param array $customTagsConfiguration
* @param array $customStylesConfiguration
*/
public function __construct(
Repository $repository,
AuthorizationCheckerInterface $authorizationChecker,
ConfigResolverInterface $configResolver,
Environment $templateEngine,
$tagConfigurationNamespace,
$styleConfigurationNamespace,
$embedConfigurationNamespace,
PermissionResolver $permissionResolver,
string $tagConfigurationNamespace,
string $styleConfigurationNamespace,
string $embedConfigurationNamespace,
LoggerInterface $logger = null,
array $customTagsConfiguration = [],
array $customStylesConfiguration = []
) {
$this->repository = $repository;
$this->authorizationChecker = $authorizationChecker;
$this->configResolver = $configResolver;
$this->templateEngine = $templateEngine;
$this->permissionResolver = $permissionResolver;
$this->tagConfigurationNamespace = $tagConfigurationNamespace;
$this->styleConfigurationNamespace = $styleConfigurationNamespace;
$this->embedConfigurationNamespace = $embedConfigurationNamespace;
Expand Down Expand Up @@ -452,30 +436,23 @@ protected function getEmbedTemplateName($resourceType, $isInline, $isDenied)
/**
* Check embed permissions for the given Content.
*
* @throws \Symfony\Component\Security\Core\Exception\AccessDeniedException
*
* @param \Ibexa\Contracts\Core\Repository\Values\Content\Content $content
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\InvalidArgumentException
* @throws \Ibexa\Contracts\Core\Repository\Exceptions\BadStateException
*/
protected function checkContentPermissions(Content $content)
{
// Check both 'content/read' and 'content/view_embed'.
if (
!$this->authorizationChecker->isGranted(
new AuthorizationAttribute('content', 'read', ['valueObject' => $content])
)
&& !$this->authorizationChecker->isGranted(
new AuthorizationAttribute('content', 'view_embed', ['valueObject' => $content])
)
!$this->permissionResolver->canUser('content', 'read', $content)
&& !$this->permissionResolver->canUser('content', 'view_embed', $content)
) {
throw new AccessDeniedException();
}

// Check that Content is published, since sudo allows loading unpublished content.
if (
!$content->getVersionInfo()->isPublished()
&& !$this->authorizationChecker->isGranted(
new AuthorizationAttribute('content', 'versionread', ['valueObject' => $content])
)
&& !$this->permissionResolver->canUser('content', 'versionread', $content)
) {
throw new AccessDeniedException();
}
Expand All @@ -501,19 +478,17 @@ static function (Repository $repository) use ($id) {

// Check both 'content/read' and 'content/view_embed'.
if (
!$this->authorizationChecker->isGranted(
new AuthorizationAttribute(
'content',
'read',
['valueObject' => $location->contentInfo, 'targets' => [$location]]
)
!$this->permissionResolver->canUser(
'content',
'read',
$location->contentInfo,
[$location]
)
&& !$this->authorizationChecker->isGranted(
new AuthorizationAttribute(
'content',
'view_embed',
['valueObject' => $location->contentInfo, 'targets' => [$location]]
)
&& !$this->permissionResolver->canUser(
'content',
'view_embed',
$location->contentInfo,
[$location]
)
) {
throw new AccessDeniedException();
Expand Down
64 changes: 27 additions & 37 deletions tests/lib/RichText/RendererTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
namespace Ibexa\Tests\FieldTypeRichText\RichText;

use Exception;
use Ibexa\Contracts\Core\Repository\PermissionResolver;
use Ibexa\Contracts\Core\Repository\Values\Content\Content;
use Ibexa\Contracts\Core\Repository\Values\Content\ContentInfo;
use Ibexa\Contracts\Core\Repository\Values\Content\Location;
Expand All @@ -19,7 +20,6 @@
use PHPUnit\Framework\TestCase;
use Psr\Log\LoggerInterface;
use Symfony\Component\HttpKernel\Exception\NotFoundHttpException;
use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
use Symfony\Component\Security\Core\Exception\AccessDeniedException;
use Twig\Environment;
use Twig\Loader\LoaderInterface;
Expand All @@ -29,7 +29,7 @@ class RendererTest extends TestCase
public function setUp(): void
{
$this->repositoryMock = $this->getRepositoryMock();
$this->authorizationCheckerMock = $this->getAuthorizationCheckerMock();
$this->permissionResolverMock = $this->getPermissionResolverMock();
$this->configResolverMock = $this->getConfigResolverMock();
$this->templateEngineMock = $this->getTemplateEngineMock();
$this->loggerMock = $this->getLoggerMock();
Expand Down Expand Up @@ -1707,9 +1707,9 @@ protected function getMockedRenderer(array $methods = [])
->setConstructorArgs(
[
$this->repositoryMock,
$this->authorizationCheckerMock,
$this->configResolverMock,
$this->templateEngineMock,
$this->permissionResolverMock,
'test.name.space.tag',
'test.name.space.style',
'test.name.space.embed',
Expand All @@ -1720,67 +1720,57 @@ protected function getMockedRenderer(array $methods = [])
->getMock();
}

/**
* @var \Ibexa\Contracts\Core\Repository\Repository|\PHPUnit\Framework\MockObject\MockObject
*/
/** @var \Ibexa\Contracts\Core\Repository\Repository&\PHPUnit\Framework\MockObject\MockObject */
protected $repositoryMock;
barw4 marked this conversation as resolved.
Show resolved Hide resolved

/**
* @return \PHPUnit\Framework\MockObject\MockObject
* @return \Ibexa\Core\Repository\Repository&\PHPUnit\Framework\MockObject\MockObject
*/
protected function getRepositoryMock()
protected function getRepositoryMock(): Repository
{
return $this->createMock(Repository::class);
}

/**
* @var \Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface|\PHPUnit\Framework\MockObject\MockObject
*/
protected $authorizationCheckerMock;
/** @var \Ibexa\Contracts\Core\SiteAccess\ConfigResolverInterface&\PHPUnit\Framework\MockObject\MockObject */
protected ConfigResolverInterface $configResolverMock;

/**
* @return \PHPUnit\Framework\MockObject\MockObject
* @return \Ibexa\Contracts\Core\SiteAccess\ConfigResolverInterface&\PHPUnit\Framework\MockObject\MockObject
*/
protected function getAuthorizationCheckerMock()
protected function getConfigResolverMock(): ConfigResolverInterface
{
return $this->createMock(AuthorizationCheckerInterface::class);
return $this->createMock(ConfigResolverInterface::class);
}

/**
* @var \Psr\Log\LoggerInterface|\PHPUnit\Framework\MockObject\MockObject
*/
protected $configResolverMock;
/** @var \Twig\Environment&\PHPUnit\Framework\MockObject\MockObject */
protected Environment $templateEngineMock;

/**
* @return \PHPUnit\Framework\MockObject\MockObject
* @return \Twig\Environment&\PHPUnit\Framework\MockObject\MockObject
*/
protected function getConfigResolverMock()
protected function getTemplateEngineMock(): Environment
{
return $this->createMock(ConfigResolverInterface::class);
return $this->createMock(Environment::class);
}

/**
* @var \Symfony\Component\Templating\EngineInterface|\PHPUnit\Framework\MockObject\MockObject
*/
protected $templateEngineMock;
/** @var \Ibexa\Contracts\Core\Repository\PermissionResolver&\PHPUnit\Framework\MockObject\MockObject */
protected PermissionResolver $permissionResolverMock;

/**
* @return \PHPUnit\Framework\MockObject\MockObject
* @return \Ibexa\Contracts\Core\Repository\PermissionResolver&\PHPUnit\Framework\MockObject\MockObject
*/
protected function getTemplateEngineMock()
protected function getPermissionResolverMock(): PermissionResolver
{
return $this->createMock(Environment::class);
return $this->createMock(PermissionResolver::class);
}

/**
* @var \Psr\Log\LoggerInterface|\PHPUnit\Framework\MockObject\MockObject
*/
protected $loggerMock;
/** @var \Psr\Log\LoggerInterface&\PHPUnit\Framework\MockObject\MockObject */
protected LoggerInterface $loggerMock;

/**
* @return \PHPUnit\Framework\MockObject\MockObject
* @return \Psr\Log\LoggerInterface&\PHPUnit\Framework\MockObject\MockObject
*/
protected function getLoggerMock()
protected function getLoggerMock(): LoggerInterface
{
return $this->createMock(LoggerInterface::class);
}
Expand All @@ -1791,9 +1781,9 @@ protected function getLoggerMock()
protected $loaderMock;

/**
* @return \PHPUnit\Framework\MockObject\MockObject
* @return \Twig\Loader\LoaderInterface&\PHPUnit\Framework\MockObject\MockObject
*/
protected function getLoaderMock()
protected function getLoaderMock(): LoaderInterface
{
return $this->createMock(LoaderInterface::class);
}
Expand Down