_csrf_token is broken due to cacheability metadata integration, results in rendered links without valid CSRF tokens

I think I've opened an issue about this already.

See #2575519: Twig template variables containing result of Drupal::url() and Drupal:l:() don't bubble up their cacheability and attachment metadata (e.g. token placeholder) and #2541166: Browse available tokens not shown

Yeha, seems like a different issue. This will always just leave the 'raw' sha1 hash as your link token. You use that as a redirect or something, no banana.

Yes, that's exactly the problem I had in token too, you just get the placeholder hash instead of the token and then you spend hours debugging why the token doesn't match ;)

I don't think those issues are that different, maybe a bit since in my case, it *should* work as it ends up in a render array while it's just a redirect for you.

That said, it might actually be exactly the same. RouteProcessorCsrf has two modes. Using a placeholder or not. And I think we incorrectly pass down that in this case, the placeholder approach won't work. It's either the same fix or the same fix in two places AFAICS.

Agreed they're duplicates.

I think this should fix it.

Perhaps a cacheable response is not necessary. But there sure is cacheability metadata associated with this URL and therefore with this redirect.

You're right of course that responses to POST requests are never cacheable.

But what about GET forms?

Oops.

I have the same issue of links being rendered with a placeholder rather than the actual token, however I'm not trying to use it with forms, but rather in a hook_preprocess_page_title(), it seems like renderPlaceholderCsrfToken() is never called.

#16: can you post your hook_preprocess_page_title() implementation?

Of course:

function mymodule_preprocess_page_title(&$variables) {
  $route = \Drupal::routeMatch();
  $account = \Drupal::currentUser();
  if ($route->getRouteName() == 'view.myview.mydisplay' && $account->hasPermission('mymodule permission')) {
    $url = Url::fromRoute('myroute');
    $link = Link::fromTextAndUrl('mylinktext', $url);
    $variables['title_suffix'] = array(
      'mykey' => array(
        '#markup' => '(' . $link->toString() . ')',
      ),
    ) + $variables['title_suffix'];
  }
}

Actually if I make the following change it works; is this intended?

- '#markup' => '(' . $link->toString() . ')',
+ '#markup' => '(' . render($link->toRenderable()) . ')',

Tagging as triaged after discussion with alexpott, effulgentsia, xjm and cottser.

Not sure of a better title, but just to clarify the issue here is that the link won't work, we're not leaking tokens anywhere, so firmly major rather than critical.

dawehner points out this one I filed may be a duplicate (same underlying cause) #2787083: [PP-1] CSRF tokens broken for check updates or run cron for update module are 403

This is a problem when setting $element['#ajax']['url'] to a custom route that has a _csrf_token requirement as well. Related issue has a temporary "workaround" as well as some step through debugging for "why" this is happening.

I look to be having the same / similar issue with drupal_set_message.

@timmillwood - any more specifics?

@pwolanin, see #2804327: CSRF links in a drupal_set_message don't render correctly.

Marked #2804327: CSRF links in a drupal_set_message don't render correctly as a duplicate of this. Inheriting its tags.

We also need to inherit the tests from that issue, and people who worked on it :)

Yep!

Ensuring issue credit for the people who worked on #2804327: CSRF links in a drupal_set_message don't render correctly.

If this turns out to fix #2787083: [PP-1] CSRF tokens broken for check updates or run cron for update module are 403, we should add tests for that as well.

For anyone looking for a workaround, you basically need to render the link first. e.g.

$link = Link::createFromRoute('Do stuff', 'some.route', ['foo' => $foo, 'bar' => $bar]);
$link_render = $link->toRenderable();
$link = \Drupal::service('renderer')->renderPlain($link_render);
drupal_set_message(t('This is awesome. @do_stuff_link', ['@do_stuff_link' => $link]));

I'm having the same issue, but in a different situation: I'm adding urls with token placeholders to drupalSettings, which are never replaced with the actual CSRF token. This is because drupalSettings are not yet rendered when the placeholders are replaced by their actual values during the rendering process. See HtmlResponseAttachmentsProcessor::renderPlaceholders.

I'm not sure if there's a possible solution that would fix both this and the other reported issues.

The test-only patch from #2804327: CSRF links in a drupal_set_message don't render correctly never made it here, so uploading that.

I just bumped into what feels like something related to this issue on Drupal 9.4.5 writing a KernelTestCase for a custom route.

This is the kind of pseudo-code I had:

        $url = $this->urlGenerator->generateFromRoute(
            'my_cool_route',
            ['my_node_id' => 1234]
        );
        $this->makeGetRequest($url);
        $this->assertThatThePageReturnedTheRightStuffAndDidTheRightThingEtcEtc();

Issue is $url had ?token placed in the query-string, but it was the sha1 placeholder, and not the actual proper token. I experimented with two workarounds, and went with the latter in my case, although it feels fragile:

1. Spin up a render context and render the link:

/**
     * A drop-in replacement for route() or GeneratorUrl::generateFromRoute() that supports CSRF tokens.
     *
     * @param $name
     *   See UrlGeneratorInterface::generateFromRoute().
     * @param $parameters
     *   See UrlGeneratorInterface::generateFromRoute().
     * @param $options
     *   See UrlGeneratorInterface::generateFromRoute().
     *
     * @return string
     *   The URL.
     */
    private function generateUrlFromRouteSupportingCsrf(
        string $name,
        array $parameters = [],
        array $options = []
    ): string {
        return $this->renderer->executeInRenderContext(new RenderContext(), function () use ($name, $parameters, $options) {
            $generatedUrl = $this->urlGenerator->generateFromRoute($name, $parameters, $options, TRUE);
            $build = [
                '#markup' => $generatedUrl->getGeneratedUrl(),
            ];
            $generatedUrl->applyTo($build);
            return $this->renderer->render($build, true);
        });
    }

2. Use the (private) inner 'url_generator.non_bubbling' service directly to avoid all the bubbling stuff:

    protected function setUp(): void
    {
        parent::setUp();
        ...
        // The following is equivalent to:
        //     $this->urlGenerator = $this->container->get('url_generator.non_bubbling');
        // See https://www.drupal.org/project/drupal/issues/2630920
        $this->urlGenerator = new \Drupal\Core\Routing\UrlGenerator(
            $this->container->get('router.route_provider'),
            $this->container->get('path_processor_manager'),
            $this->container->get('route_processor_manager'),
            $this->container->get('request_stack'),
            $this->container->getParameter('filter_protocols')
        );
    }

Is this the reason why I can not download the archive? I get 404 Not Found error in Docker container

This came up as a daily BSI target

Reading the comments sounds like we need to add the test patch from #48 but for solution not 100% sure the older patches still apply here. Not just physically but with todays core infrastructure.

But think it would be good to get concrete steps to reproduce.

Thanks all

Wanted to bump this 1 more time, if still an issue in D11.

This is still an issue, at least as described in #2804327: CSRF links in a drupal_set_message don't render correctly. I've rerolled the test-only patch and ensured the test user is logged in so that it's actually testing the CSRF token.

The workaround of calling:

  /** @var \Drupal\Core\Link $link */
  $build = $link->toRenderable();
  $this->messenger->addStatus($this->renderer->renderInIsolation($build));

Still works in this case.

I came across this issue while doing bug-smash triage.
I see that test-only patch is failing as expected.
The patch seems to be derived from #48.
Moving this to needs work to fix the issue.