Problem/Motivation

Whenever I authenticate using the plugin, a warning concerning leaking cacheability metadata is written to the log:

While processing SAML authentication response, code leaked cacheability metadata. This indicates a bug somewhere (but it is hard to pinpoint where): if the same code is called in other scenarios too, it may cause fatal crashes, or bloat the render cache unnecessarily. Please investigate. Metadata: i:6;:O:37:"Drupal\Core\Render\BubbleableMetadata":4:{s:16:"*cacheContexts";a:0:{}s:12:"*cacheTags";a:0:{}s:14:"*cacheMaxAge";i:-1;s:14:"*attachments";a:0:{}}

Steps to reproduce

Use the SAML login to authenticate.

Proposed resolution

Fix metadata leak.

Comments

fkohrt created an issue.

plessas’s picture

Are you sure this happens in every login attempt? I am experiencing the same issue, however only when the user logs in for the first time and the drupal user is created.

fkohrt’s picture

Yes, I can confirm this happens every time an existing user logs in. There is always a succession of three log events:

  1. user: Session opened for <username>.
  2. externalauth: External login of user <username>
  3. samlauth: While processing SAML authentication response, code leaked cacheability metadata. […]
roderik’s picture

Proposed resolution: Fix metadata leak...

Yes, I would like that. But the samlauth module isn't leaking metadata. It's detecting that some other code in your website, which was executed during the login process, is leaking metadata and should be fixed.

What is happening? What does this mean?

On the surface, nothing is wrong. Your logins work fine.

It means cached responses somewhere in your site may be off, which could lead to either cache bloat, or wrongly cached data (which in theory can lead to security issues). The login/logout requests from samlauth take care to work around these issues, but any other parts of your site - where the same (unknown) code that is leaking metadata, is executed - might introduce these issues unwittingly.

What can also happen is that after you install another module, your site suddenly and unexpectedly stops working, and you'll spend hours, possibly days, debugging the fatal error that Drupal throws at you: LogicException: The controller result claims to be providing relevant cache metadata, but leaked metadata was detected.
In practice, this would likely be another module that performs external redirects to other services, or serves REST responses.

Sounds hard to believe. Why is it up to some contrib module to warn me about other problems in my site? Why doesn't Drupal Core report or fix this?

If you want to know more, #2638686: Correctly handle cache data instead of throwing an Exception in EarlyRenderingControllerWrapperSubscriber() has been stalled since the beginning of 2016 and is a jump-off point into a swamp of information about this.

I summarized the swamp in comments for the same code that is spamming your watchdog logs: https://git.drupalcode.org/project/samlauth/blob/8.x-3.x/src/Controller/...

Can't you just stop spamming my logs?

I could make an option to turn off the logs if people really insisted on this. But I would really rather that people fix their sites and post known patches here, to prevent the (real or potential - but potentially grave) issues listed above.

Known or suspected leakers

  • Rules 8.x-3.0-alpha6 (still present in alpha7) - I posted two alternative patches at #3161036-6: CurrentPathContext inadvertently bubbles up 'route' cache context. I'm running the first patch on my sites, which gets rid of the leakage - but need a Rules expert to tell me if it's actually the correct alternative.
  • https://www.drupal.org/project/nodeaccess (suspected). There are two sites I've worked on the SAML implementation, which are now spamming logs. Both have nodeaccess enabled. I do not maintain the sites myself however, and my amount of volunteer time vs. priorities has kept me from digging into them.
plessas’s picture

Thanks for the info roderik. I can confirm that in a fresh drupal (9.3.2) installation with the latest version of the SAMLAuth module and no other contrib module enabled, there are no log entries regarding leaked cacheability metadata, while my SAML configuration remains the same. In my case, it seems that the problem comes from code executed when the user is created for the first time, so I will have to check which modules are involved in user creation process...

plessas’s picture

After several tests, I realized that if I uninstall the Metatag module (for which I haven't changed the default configuration) the warning disappears. However, in a fresh Drupal installation with the Metatag module enabled, there are no warnings. While this is quite confusing, I suppose that the cause of the warning could be a combination of another module with the Metatag module or it could be related to a specific configuration value of my installation.

fkohrt’s picture

Thanks for this detailed explanation from my side as well! I could trace the appearance of the warning back to CiviCRM and opened a corresponding ticket on their issue tracker. I'd be fine with closing this issue.

roderik’s picture

I intend for this issue not to be closed, but to morph into a list of modules that are leaking cacheability metadata, and patches to them. And maybe general directions on how to fix. (Which I for now typed into a comment on the CiviCRM issue, because there was an interested developer there. I might clean it up / copy it into this issue text later.)

Metatag is interesting. Its maintainer would likely be interested if we could get to a reproducible situation. Yes, I guess it can be both a configuration value and the combination with another module. A quick search through its code didn't give anything that I recognized.

plessas’s picture

Investigating further the issue, I realized that it is the combination of Content Sync and Metatag modules that causes the metadata leak. Whenever a new user is created from the SAMLAuth/ExternalAuth login cycle, there is a hook_entity_insert() in Content Sync that calls a function called content_sync_entity_update(). This function serializes the user entity and exports it in orded to save it in the database and as a YAML file. When the Metatag module is enabled, user metadata are included in the serialization process and it seems that somewhere there it is the cause for the leak. I didn't have the time to dig deeper, I will try again the next days.

samaphp’s picture

Thank you @roderik, You saved me.
I've found that my issue was that I've implemented hook_user_login() to redirect the user after authenticate SAML.
The issue, was I've used Url::fromRoute() to get the full URL and use it with `RedirectResponse`. So, I've changed `RedirectResponse` to `CacheableResponse` and that just fixed it while still using Url::fromRoute().

So, I read this issue and the other closed one, and followed some instructions from Lullabot article.
Here is the steps that helped me to catch the issue, it was from my custom code:
1. Enable xdebug.
1. Set a breakpoint in the sole implementation of RendererInterface::render() As per Lullabot article suggestions.
1. Try to make the issue happen
1. You will see the trace in your IDE, Look for Url::fromRoute() usage or any Url usages, you can start with your custom code first.

Thank you again @roderik for describing the issue and referring to Lullabot article.
And always thanks to Lullabot people

rschwab’s picture

I just started coming across this problem as of 3.10. It pops up when using a link with the 'destination' parameter, as in this twig template snippet:
<a href="/saml/login?destination={{ path('<current>') }}">Website Login</a>

With #2638686: Correctly handle cache data instead of throwing an Exception in EarlyRenderingControllerWrapperSubscriber() maybe this isn't a problem anymore? Or rather, the problem is no longer an exception?

roderik’s picture

Embarrassingly, 3.x-1.10 itself contains an error that triggered the warning and therefore should be fixed. Your issue is probably fixed with #3486925: Cacheability Metadata Leakage Error on SAML Login with Samlauth and r4032 Redirect Module. 8.x-3.11 is out now.

matthand’s picture

Just reporting that I am still seeing this error, even in 3.11.

mark_fullmer’s picture

But I would really rather that people fix their sites and post known patches here

Yes, I can do that! I can verify the following:

- Using Samlauth 3.11 on its own does not trigger this warning for multiple sites I tested
- Using the Metatag module (see #6 and #8 above) does not trigger this warning, at least not with the configuration we have.
- What *is* triggering this warning is a custom implementation of hook_user_login() that modifies the destination parameter, passing a Drupal Url object as the parameter WITHOUT SPECIFYING that the metadata should bubble:

From Core's API, the Url::fromRoute->toString() method takes an optional parameter that defaults to FALSE:

public function toString($collect_bubbleable_metadata = FALSE) {

In the case of my custom code, this change suppressed the warning:

/**
 * Implements hook_user_login().
 */
function mymodule_user_login($account) {
  $param = \Drupal::request()->query->all();
  if (!$param || !isset($param['destination'])) {
    // For every user but "user 1", redirect to /dashboard upon login.
    if ($account->id() != 1) {
-      \Drupal::service('request_stack')->getCurrentRequest()->query->set('destination', Url::fromRoute('MYMODULE.ROUTE')->toString());
+      \Drupal::service('request_stack')->getCurrentRequest()->query->set('destination', Url::fromRoute('MYMODULE.ROUTE')->toString(TRUE)->getGeneratedUrl());
    }
  }
}

My conclusion is that for the majority of folks coming to this issue, the problem is probably in custom code that implements hook_user_login()

rosk0’s picture

Thanks for the pointer @mark_fullmer!

I did had the custom code in the hook_user_login() that did a "destination" based redirect to the Url::fromRoute()->toString(), and it did produced the warning for every log in.

I've tried to use suggested Url::fromRoute()->toString(TRUE)->getGeneratedUrl() and it did got rid of the warning.
That made me wonder and I started digging.

The Url::fromRoute()->toString() bubbles metadata by default, but only if called in the render context and everything generated from the controllers is renderer with the render context that is the request that the controller is responding to. See \Drupal\Core\Render\Renderer::getCurrentRenderContext() and the comment in the \Drupal\Core\Render\MetadataBubblingUrlGenerator::bubble() method body ( provided below ):

Bubbling metadata makes sense only if the code is executed inside a render context. All code running outside controllers has no render context by default, so URLs used there are not supposed to affect the response cacheability.

Posting this as a reminder to myself and anyone else confused like me - if you are not rendering anything in your controller, it doesn't mean that there is no render context involved!

karenann’s picture

I am able to recreate this error message with Drupal 10.5.6 and Login Destination 8.x-2.0-beta10.

It was not enough to simply have Login Destination enabled. When I have no login destinations enabled, no warning. When I create/enable one with an applicable role for my test user, the warning appears.

I created an issue in Login Destination #3561568: Code leaked cacheability metadata with SAML Authentication (samlauth) module and I'm going to upload a patch based on the Rules patches mentioned above; #3161036-6: CurrentPathContext inadvertently bubbles up 'route' cache context.