PageManager pages access broken

"Someone" reporting in :p (A link to the question might be helpful, and back)

We need a try/catch in \Drupal\page_manager\Entity\PageAccess::checkAccess, just like in \Drupal\block\BlockAccessControlHandler::checkAccess

But the bigger problem is apparently that the context doesn't work if this isn't executed on the actual page, we need to get this into \Drupal\page_manager\EventSubscriber\RouteParamContext::onPageContext() somehow.

This is a pretty huge conceptual problem with page variant access and I'm not sure how to deal with it.

The simple fact is that almost anything you can do only works on the actual page you're looking at. anything that requires context that's somehow from the path simply doesn't work and causes all kinds of bugs.

For example, during menu rebuilding, if you have menu links that point to /some/path and there you have variants that have some access checks, based on the path, or in this case the provided node, than those contexts will not work on another page, where you are rebuilding the menu tree. That means the links will be created with base:some/path and not with the route, and then active trail and many other things will not work.

I experimented a bit with pushing the current request object onto the request stack but couldn't fully get it working and of course that's inside symfony code..

Any progress on this Berdir? Trying to debug this and was wondering where you were looking at. I am uploading this patch as a mean to start debugging this issue, this seems to have fixed the edit menu pages, but I am not sure if it will cause permissions issues elsewhere, if it will, please let me know and ill continue to debug on it.

Is this a bug in Page Manager or in the context system that it uses?

I get the same bug and try/catch solved the problem.

Also ran into this problem when I tried to create a custom route (pointing to a form) that shares the first part of its path with a Page Manager page.

Noticed that the patch contained tabs (instead of spaces) so I've converted them to spaces and generated a new patch.

Apparently, at the time of writing, the 8.x-1.x-dev branch itself is failing with the same errors: https://www.drupal.org/node/2272255/qa

So I assume that means the failure is unrelated to this patch.

Going to try to see if I can mark the issue as "needs review" but temporarily cancel the test run.

hi all, this issue bugging me out a lot as im huge panels(page_manager) fun and here is what i found

first, to replicate you need to have some page (via page manager) that takes "some" context from URL (most commonly its a node)
second - you need to use that context in Access check (you will see why) for this page
third - you need to invoke path validation for this page on a different URL

in my case it was

page_manager: 8.x-4.0-beta2
current url: /sg_common/test
access check: bundle filter
invoked code:

$path = 'node/2';
$is_valid1 = \Drupal::service('path.validator')->isValid($path);

at this point you should reliebly get

Drupal\Component\Plugin\Exception\ContextException: Required contexts without a value: node. in Drupal\Core\Plugin\Context\ContextHandler->applyContextMapping() (line 112 of core/lib/Drupal/Core/Plugin/Context/ContextHandler.php).

firts thing i found interesting is that this code actually runs twice on the page, once its returns valid node context and second one its a WSOD with some debug_backtrace i found that its caused by

Drupal\Core\Routing\AccessAwareRouter:matchRequest()

first time execution path goes through

$parameters = $this->router->matchRequest($request);

second time (this is why you need to use context in access to trigger error)

$this->checkAccess($request);

then i found that when event to collect context is triggered and handled by

Drupal\page_manager\EventSubscriber\RouteParamContext:onPageContext()

request stack is differnt

$request = $this->requestStack->getCurrentRequest();

in first invocation its tip is '/node/2' but in second its '/sg_common/test' and ofc attempting to extract node argument form the second route is a failure, so between those two lines someone operate on request Stack service

looking for suspects i found that

Drupal\page_manager\Routing\VariantRouteFilter:getVariantRouteName()

contains

$this->requestStack->pop();

but it pushes one level just before, so in fact THIS what cause proper context creation and not the inproper one.

so the catual problem is that page_manager uses current request from stack, but nobody pushes it there :/ last function i have found info about request that wasnt in core was

Drupal\page_manager\Entity\PageAccessCheck:access()

there is no Request object there so i build it from $route_match best i can :/

    $parameters = $route_match->getRawParameters();
    $replace = [];
    foreach ($parameters->all() as $key => $value) {
      $replace['{' . $key . '}'] = $value;
    }
    $path = $route_match->getRouteObject()->getPath();
    $real_path = strtr($path, $replace);
    $request = Request::create($real_path);
    $request->attributes->add($route_match->getParameters()->all());

and push it before access check function and pop it after so whole thing looks like in the patch attached.
for me it not only fixes WSOD but also starts returning valid access info and i finaly can filter my menu links, hope this will help page_manager's team find a proper fix to this issue as mine seems a little bit too straightforward. (i also didnt see 8.x-4.x-dev to test against)

#12 works for me. Thanks

#12 worked for me as well

Looks like patch still applies.

Re-roll of #12.

This is working for me.

The Drupal 10 failures also exist on the dev branch.

#12 works with D9.3.22. It also solved my page_manager problem related to contexts and controllers matching.

When upgrading from 9.1.2 to 9.3.22 received this error caused by page_manager when attempting drush updb.

PHP Fatal error: Declaration of Drupal\page_manager\Entity\Page::getContexts() must be compatible with Drupal\page_manager\PageInterface::getContexts

Applying this patch at #17 solved the issue.

(I am in Drupal upgrade prison with a large site. Hopefully this will get indexed into Google and help any others in same boat).

We first used this patch as well and it seemed to fix our problem. But manipulating the request this way will lead to a loss of information on the route in some circumstances which will result in other, unfixable issues.

For us the solution was to use page managers PAGE_CONTEXT Event to add our missing context which looked kinda like this for our missing group context:

  public function onPageManagerPageContext(PageManagerContextEvent $event) {
    $page = $event->getPage();
    foreach ($page->getContexts() as $context) {
      if (!$context->hasContextValue()) {
        $context_definition = $context->getContextDefinition();
        if ($context_definition->getDataType() == 'entity:group') {
          // Tell page_manager about our context value.
          $page->addContext('group', new Context($context_definition, $this->getGroupFromRoute()));
        }
      }
    }
  }

@Hydra #20:
> But manipulating the request this way will lead to a loss of information on the route in some circumstances which will result in other, unfixable issues.

This is a bit foggy and not productive in getting this further. Can you elaborate?

Patch #17 applies cleanly on current core 10.2.2 and fixes the issue for me.

Code looks good and reasonable, and i can not see or guess what the #20 objection may mean.
(#17 manipulates a separate request and pushes / pops it onto the stack.)

This fix MAY be affected by the related long-standing core issue though. EDIT: It is not.

Setting NW for an automated test, which might look like this:
- Create a page with a context, use the context to control page access
- Create a $url for that page and call $url->access()

@geek-merlin - it's a while ago I faced this issue. But as far as I can remember (and my git history tells me) we had a local task (tab) added to the group tasks which pointed to a page_manager page route. That page of course has the group context configured and used a custom condition plugin to determine the access. That condition used the group context to check for some information related to the group in order to grant or deny access. In combination with this patch, this resulted in an error on the local task access check level - kinda related to what berdir described what could happen to menu links in #5
Removing the patch and adding the event subscriber with the provided code in #20 solved that for us and solved the original issue we used the patch for in the first place.

#23: Thanks. Nothing that can be reproduced though.

Looked a bit deeper into this and found that the logic of #17 is not enough in many cases.

Rolled a MR that is way more robust and fixes the issue for me. Still needs tests though.

Made a simplification, then squashed.
Patch url: https://git.drupalcode.org/project/page_manager/-/commit/02e263051d80646...

Ah OK, this needs a rebase.
The related issue changed the code here a lot.

Maybe it solved this issue too? Let's see.

Finally i found some time to look into this thoroughly.
@berdir is right that the former fake-request approach is at least limited and possibly FUBAR.

So...
- created a SubRequestAccessChecker and fixed some core limitations.
- added tests, which are green.
- also test-only changes are red (nice we have that now!), which proves that the MR fixes the issue.

Nice.

Squashed commit for local application: https://git.drupalcode.org/project/page_manager/-/commit/d3764a67c50e69e...

Looks great by skimming!

btw why test-only fails? https://git.drupalcode.org/issue/page_manager-2837833/-/jobs/3186611

Meantime different approach started since #2613044-123: Requests are pushed onto the request stack twice, popped once

Also some workaround via #3390193: Add a drupalGet() method to KernelTestBase

@andypost #31
> Looks great by skimming!

Nice you looked into it and like it too! I think the approach has quite some potential.
Can you give a code review (make all corrections you think fit) and help move this forward?

#32: Looks enigmatic to me... wrong issue?

Let's handle #2613044: Requests are pushed onto the request stack twice, popped once then.

Squashed: https://git.drupalcode.org/project/page_manager/-/commit/5a62749ad8bf170...

Yep, that works for me on a complex site.

Ah, request context must be fixed too.

I can confirm that the issue merge request fixes the issue, but we have fails in PHPUnit. Looks like it is related to smth else.