processwire / processwire-issues

ProcessWire issue reports.
45 stars 2 forks source link

Images inside repeaters inside repeater matrix fields are removed due to access control settings #1548

Open ivangretsky opened 2 years ago

ivangretsky commented 2 years ago

Short description of the issue

Images that are inserted inside (in my case multilang) textarea fields, that are part of Repeaters, that are in their case, are part of (inside) Repeater matrix fields are stripped out of output due to access settings. Images inside RM items are not - probably, there is some "special treatment" for them.

The Repeaters structure

Levels

The textarea settings

Repeaters

Expected behavior

Images should not be removed from any fields, that are filled from current page. Not from 1st level RM items, not from 2nd level Repeater/PM items inside of the 1st level items. I guess 3 levels deep would be ok if any depth restriction has to be imposed.

Setup/Environment

ryancramerdesign commented 2 years ago

@ivangretsky Can you try changing line 825 in /wire/core/MarkupQA.php from this:

if(($pagefile->page->id != $this->page->id && !$user->hasPermission('page-view', $pagefile->page))

to this:

if(($pagefile->page->id != $this->page->id && !$pagefile->page->viewable(false))

Does that enable it to work?

ivangretsky commented 2 years ago

@ryancramerdesign , did try that and changing that line did not help.

ryancramerdesign commented 2 years ago

@ivangretsky Thanks for trying it. Do you by any chance have access control enabled manually on any of your repeater templates or fields within them? If so, disable that. If not, then it sounds like we might need a full repeater check here then. Usually we don't because the PagePermissions module handles it internally, but occasionally there are cases where we do need to add a repeater check. Here's the entire block of code updated to add a full repeater access check, are you able to try this? (I don't yet have an environment with the same setup as yours). If so, replace the entire if($options['removeNoAccess']) { ... } with this new one below:

if($options['removeNoAccess']) {
    $page = $pagefile->page; /** @var Page|RepeaterPage $page */
    $field = $pagefile->field;
    $removeImage = false;
    if(wireInstanceOf($page, 'RepeaterPage')) {
        $page = $page->getForPageRoot();
        $field = $page->getForFieldRoot();
    }
    if($page->id != $this->page->id && !$page->viewable(false)) {
        $this->error("Image on page ($page) that user does not have view access to: $src");
        $removeImage = true;
    } else if($field && !$page->viewable($field)) {
        $this->error("Image on page:field ($page:$field) that user does not have view access to: $src");
        $removeImage = true;
    }
    if($removeImage) {
        if($this->page->of()) $value = str_replace($img, '', $value);
        return;
    }
}
matjazpotocnik commented 2 years ago

@ivangretsky can you provide more info for Ryan?

ryancramerdesign commented 2 years ago

@ivangretsky Also the issue fixed in #1561 may be related

ivangretsky commented 2 years ago

I have tried to replace the code as @ryancramerdesign suggested . The results are mixed.

  1. I get an error Exception: Method DefaultPage::getForFieldRoot does not exist or is not callable in this context unless I comment out the one string in the new code (see below).
  2. If I do comment out that string, the image is shown like it should. изображение
  3. Strangely, If comment out the other line the error is also gone (but the image is not shown). изображение

So we must be getting close) Sorry for not doing it earlier.

matjazpotocnik commented 2 years ago

@ivangretsky Ivan, thank you for testing. Did you try the latest PW version, which contains the fix for issue 1561?

ivangretsky commented 2 years ago

Nope. Will try upgrading.

ivangretsky commented 2 years ago

Did upgrade to 3.0.199, but it did NOT fix the issue.

cb2004 commented 2 years ago

@ivangretsky are you using ProDrafts?

ivangretsky commented 2 years ago

@cb2004 , yes, it is installed.

cb2004 commented 2 years ago

@ivangretsky https://github.com/processwire/processwire-issues/issues/1564

ivangretsky commented 2 years ago

@cb2004 , thanks for your suggestion. Did try moving that line from init() to ready(), but it didn't seem to help,

@ryancramerdesign , do you have any ideas how we can move this forward? Did my testing results above help?

matjazpotocnik commented 1 year ago

@ivangretsky can you provide instructions for a setup without using RepeaterMatrix? If I understand correctly, you have a repeater with textarea and CKE as inputfield, and enabled image access control? Can you do bd($page->getForRoot());?

matjazpotocnik commented 4 months ago

@ivangretsky ping