EZP-31868: improve richtext embed access check on 404 page

[ITernovtsii]

Question	Answer
JIRA issue	EZP-31868
Bug/Improvement	yes
New feature	no
Target version	1.1
BC breaks	no
Tests pass	yes
Doc needed	no

1. Render richtext field in pagelayout (for example, in footer).
2. Put embed image in the richtext field.
3. Open valid URL to verify anonymous has access to the image.
4. Open some 404 URL

AR: 500 error returned after max nested level reached

ER: 404 Page properly displayed

Developer notes: It's because there is no firewall for 404 page, and there is no token in token storage service. So, it threw "AuthenticationCredentialsNotFoundException". Subrequest tries to render an error page, which extends pagelayout, which tries to render the same richtext field, and it leads to an infinitive loop.

TODO:

• [x] Implement feature / fix a bug.
• [x] Implement tests.
• [x] Fix new code according to Coding Standards ($ composer fix-cs).
• [x] Ask for Code Review.

[ITernovtsii]

@kaff Initially, I did it with minimal changes to make sure it's something Ibexa team consider to fix. Please review it now. Changes:

• I refactored render methods to use a single renderEmbed function, to not duplicate code.
• used PermissionResolver::canUser instead of AuthorizationChecker::isGranted
• updated tests to test new methods
• deperecate $authorizationChecker argument and checkLocation function

Let me know if it's acceptable to do a small BC break here, and I'll remove checkLocation, checkContent(deprecated since 6.7) functions and $authorizationChecker dependency. And I'll use dependency injectiion for PermissionResolver instead of loading it from $this->repository->getPermissionResolver()

Thanks.

[ITernovtsii]

@alongosz

No, it's not acceptable

Good, as I expected) so we can leave it as is and deprecate in the next released version.

Diff is quite complicated

Yes, diff is big, while real changes are small - using PermissionResolver instead of AuthorizationChecker. A big part of changes is refactoring of exception catches, so code is not duplicated in two functions.

What I'm not following right now already is that you've reported this bug for 3.x, yet you're fixing it in RichText version corresponding to 2.5. Which is it then?

Initially, I reproduced it in v3.1 and v3.0. After I started this PR, I found that the same code used in v2.5, so I rebased it to target 1.1 branch. Let me know if I should rebase this back to v3.x only. (personally, I need this for 3.1+)

[ITernovtsii]

@alongosz Sure, with clean install, it can be reproduced in this way:

• in pagelayout.html.twig, add: {{ render(controller('App\\Controller\\TestController::footerInfo')) }}
• Create a minimal controller (src/Controller/TestController.php) to load content and render template:

namespace App\Controller;

use eZ\Bundle\EzPublishCoreBundle\Controller;
use eZ\Publish\API\Repository\ContentService;
use Symfony\Component\HttpFoundation\Response;

class TestController extends Controller
{
    private ContentService $contentService;

    public function __construct(ContentService $contentService)
    {
        $this->contentService = $contentService;
    }

    public function footerInfo(): Response
    {
        return $this->render('@ezdesign/footer_info.html.twig', [
            'content' => $this->contentService->loadContent(1)
        ]);
    }
}

• Create template for richtext field rendering templates/themes/standard/footer_info.html.twig

RichText field: {{ ez_render_field(content, "description") }}

• Create error templates, content can be the same for both: templates/bundles/TwigBundle/Exception/error500.html.twig templates/bundles/TwigBundle/Exception/error404.html.twig

{% extends "@ezdesign/pagelayout.html.twig" %}
{% block content %}Error 404 or 500{% endblock %}

• Embed image in description field of content with id=1
• Open some URL which leads to 404
• You will see max nested level error, instead of 404 page.

I wrote instruction using existing project, so let me know if it's not reproducible with provided steps and I'll work on clean install locally and making sure it's reproducible Thanks.

[alongosz]

@ITernovtsiy unfortunately I cannot reproduce the issue on 2.5. I tried to adapt the code for 2.5 meta: https://github.com/ezsystems/ezplatform/compare/2.5...alongosz:review-2.5/ezp-31868-test-case For me embedded image on an error page (accessed via Symfony _error dev testing route) is displayed w/o any errors.

What I don't follow in your code is the fact that you use content block in error templates, yet pagelayout itself does not define that block. Moreover the flow here is a bit risky: you're extending pagelayout for the error pages which via subrequest performs heavy operation of rendering a controller, which in turn might result in any error, not just the one you're experiencing. No wonder that you get infinite loop for an error. While indeed the error should not occur in the first place, the error page should rather be ready to handle all errors.

[ITernovtsii]

@alongosz I installed clean v2.5 and I can reproduce this after small changes in your code:

• rename app/Resources/TwigBundle/views/Exception/error400.html.twig to app/Resources/TwigBundle/views/Exception/error404.html.twig
• make sure symfony is in prod mode
• open some random page, like /page-not-found-404

I understand that extending pagelayout for error is not ideal, but it's what we have as a client requirement. Anyway, using pagelayout for 404 pages is quite common, for example - https://www.ibexa.co/page-not-found So, it would be nice to be able to render richtext on 404 page.