Admin Toolbar cached when Logged-In Users caching enabled

[raamdev]

Steps to reproduce this bug

1. Enable Logged-In User caching and set it to "Yes, but DON'T maintain a separate cache"
2. In Dashboard → Users → My Profile ensure that Show Toolbar when viewing site is Enabled
3. Visit the site as an administrator to generate a cache file for a page
4. Logout and visit that same page, now as a user who is not logged in

Expected Behavior

The cached page should be loaded, but without any Admin Toolbar at the top. Since the visitor visiting the page is not logged in, seeing an Admin Toolbar is unexpected.

Observed Behavior

The cached page includes the Admin Toolbar and so the logged-out visitor sees a version of that page that includes that toolbar (attempting to use anything on the toolbar obviously redirects to the login page, but that toolbar really shouldn't be there in the first place).

---

Potential Fixes

• When generating a cache file for a page, we could detect when Logged-In User caching is set to "Yes, but DON'T maintain a separate cache" and when the current user has Show Toolbar when viewing site enabled, and then delete the <div> inside the cached HTML that contains id="wpadminbar" before generating the cache file, so that the resulting cache file does not contain that div at all.
• The other option would be to force-disable the Admin Toolbar when visiting the front-end of the site when Logged-In User caching is set to "Yes, but DON'T maintain a separate cache", with a message inside the Logged-In Users Option Panel that explains why the Admin Toolbar will be disabled. We could then provide a filter to override that behavior in case an advanced site owner wants to disable that functionality and show the Admin Toolbar anyway.

---

Support threads referencing this issue

• https://wordpress.org/support/topic/zencache-pro-real-static-cache-how?replies=2

[jaswrks]

The other option would be to force-disable the Admin Toolbar

Of those two choices, I would vote for this one. Or, I would also vote to get rid of that option altogether; only because this particular feature has the potential to create a security issue if not configured properly. The Admin Bar showing is one mild example of this, but suppose the site owner was running some plugin that displayed blog stats or DB details in some way. If something like that got cached, and the site owner doesn't realize the impact of the ZenCache configuration, it creates a security issue.

This was my doing; i.e., I put that option in there to improve flexibility, but the case you mentioned here is a good reason for us not to have that in my view. I imagine it is used rarely anyway.

[raamdev]

I would also vote to get rid of that option altogether;

I agree with that. Maybe we can get rid of that option, but make enabling such an option possible through the use of an MU-Plugin, so that if an advanced site owner wants that functionality and knows what they're doing, they can still enable it.

[raamdev]

Just thinking about what removing the "Logged-In User Caching" feature might entail and what kinds of scenarios we'd need to consider:

• Existing users who have this option enabled and who are actively using it on their site (maybe they even purchased the Pro version specifically for this feature).

  For these users we'll want to keep showing the UI option--removing it and forcing them to enable this via an MU Plugin wouldn't be very good UX. Of course if they disable this option after having been using it, it shouldn't just disappear on them. There should be a flag that, if they had this feature enabled, this feature stays visible to them. (If we want to eventually hide this UI option from even these users, then we'll need to come up with a plan that announces it to them through a Dashboard message, along with instructions for how to continue using it, e.g., via an MU Plugin. I personally don't feel this is necessary.)

• Existing users who don't have this option enabled.

  We could simply hide the UI menu altogether, so that this option suddenly disappears. (Since this option is potentially dangerous, especially to novice site owners, we shouldn't be presenting it as an option that can simply be enabled.) Then, whenever we get a query asking us what happened to this option, we can link them to a KB Article that explains why we removed it from the UI and demonstrates how to reactivate this feature using an MU Plugin.

• New users.

  As with existing users who didn't have this option enabled, this feature would simply be hidden from the UI and it would only be possible to enable it via an MU Plugin, after a site owner has had the opportunity to read through a KB Article that explains the dangers of enabling Logged-In User Caching.

[jaswrks]

Next Actions

Covering the Use Cases Listed Above

1. Existing users who have this option enabled

• See this line of code in the MenuPageOptions class.
  Wrap this line with the following conditional check:

  if ($this->plugin->options['when_logged_in'] === '1') { // i.e., not already set to this option.
      echo '            <option value="1"'.selected($this->plugin->options['when_logged_in'], '1', false).'>'.__('Yes, but DON\'T maintain a separate cache for each user (I know what I\'m doing).', SLUG_TD).'</option>'."\n";
  }

• Now take it a step further and flag an option that shows this was enabled at some point.

  if ($this->plugin->options['when_logged_in'] === '1' || get_site_option(GLOBAL_NS.'_when_logged_in_was_1')) {
      update_site_option(GLOBAL_NS.'_when_logged_in_was_1', '1');
      echo '            <option value="1"'.selected($this->plugin->options['when_logged_in'], '1', false).'>'.__('Yes, but DON\'T maintain a separate cache for each user (I know what I\'m doing).', SLUG_TD).'</option>'."\n";
  }

2. Existing users who don't have this option enabled.

Already covered by the above suggested changes in point 1.

3. New users.

Already covered by the above suggested changes in point 1.

---

Disable Admin Bar if when_logged_in is Set to 1

• After this line of code in the Plugin class, add the following.

  /*[pro strip-from="lite"]*/
  if ($this->options['when_logged_in'] === '1' && $this->applyWpFilters(GLOBAL_NS.'_when_logged_in_no_admin_bar', true)) {
      show_admin_bar(FALSE); // Prevent admin bar from being cached.
  }
  /*[/pro]*/

• Now go back and add a note in the Logged-In Users config. panel to mention this being the case.

---

Create MU plugin to enable this feature in future versions of ZenCache whenever it is requested by site owners through our support center; if that happens.

• Referencing this line of code where a filter is exposed.

  <?php
  add_filter('zencache_options', function(array $options){
      $options['when_logged_in'] = '1';
      return $options;
  });

[jaswrks]

@raamdev Would you mind if @renzms works on this?

[jaswrks]

Next Actions for this Issue

Create a new feature branch feature/497 in websharks/zencache-pro and begin work as follows.

Covering the Use Cases Listed Above

1. Existing users who have this option enabled

• See this line of code in the MenuPageOptions class.
  Wrap this line with the following conditional check:

  if ($this->plugin->options['when_logged_in'] === '1') { // i.e., not already set to this option.
      echo '            <option value="1"'.selected($this->plugin->options['when_logged_in'], '1', false).'>'.__('Yes, but DON\'T maintain a separate cache for each user (I know what I\'m doing).', SLUG_TD).'</option>'."\n";
  }

• Now take it a step further and flag an option that shows this was enabled at some point.

  if ($this->plugin->options['when_logged_in'] === '1' || get_site_option(GLOBAL_NS.'_when_logged_in_was_1')) {
      update_site_option(GLOBAL_NS.'_when_logged_in_was_1', '1');
      echo '            <option value="1"'.selected($this->plugin->options['when_logged_in'], '1', false).'>'.__('Yes, but DON\'T maintain a separate cache for each user (I know what I\'m doing).', SLUG_TD).'</option>'."\n";
  }

2. Existing users who don't have this option enabled.

Already covered by the above suggested changes in point 1.

3. New users.

Already covered by the above suggested changes in point 1.

---

Disable Admin Bar if when_logged_in is Set to 1

• After this line of code in the Plugin class, add the following.

  /*[pro strip-from="lite"]*/
  if ($this->options['when_logged_in'] === '1' && $this->applyWpFilters(GLOBAL_NS.'_when_logged_in_no_admin_bar', true)) {
      show_admin_bar(FALSE); // Prevent admin bar from being cached.
  }
  /*[/pro]*/

• Now go back and add a note in the Logged-In Users config. panel to mention this being the case.

[raamdev]

Would you mind if @renzms works on this?

@jaswsinc Sounds great to me! :-)

[jaswrks]

@renzms Here's a visual of what you're tweaking (i.e., hiding) as part of your work on this issue.

[renzms]

Hi @jaswsinc , uploaded to my test site. Tested and confirmed the option is now hidden.

As for this step:

Now go back and add a note in the Logged-In Users config. panel to mention this being the case.

What should I mention in the notice?

[jaswrks]

Cool.

Now go back and add a note in the Logged-In Users config. panel to mention this being the case.
What should I mention in the notice?

In the instructions above, there is a section titled, "Disable Admin Bar if when_logged_in is Set to 1". So whenever you implement that code that calls upon show_admin_bar(false);, you are hiding the Admin Bar.

What we need is a note in the config. panel whenever when_logged_in = 1. In that scenario, we should explain that the Admin Bar is not going to be shown on the front-end of the site. Otherwise, they won't understand why the Admin Bar suddenly disappeared from their site.

e.g., <p class="warning"> ... </p>

[jaswrks]

Note that while you are hiding the when_logged_in = 1 option by default, it is still possible to enable this; e.g., if it was already enabled, or if you use an MU plugin. That's why we need a note in the config. panel.

[renzms]

Hi @jaswsinc,

Can the note be the following?

Note for users who have set "Yes, but DON'T maintain a separate cache": If you had this feature previously enabled, please be aware that the Admin Bar will no longer be shown on the front-end of your website. The option for "Yes, but DON'T maintain a separate cache", is no longer accessible unless using an MU plugin.

[jaswrks]

Something like this...

<?php if($this->plugin->options['when_logged_in'] === '1' && $this->plugin->applyWpFilters(GLOBAL_NS.'_when_logged_in_no_admin_bar', true)): ?>
    echo '<p class="warning">'.sprintf(__('<strong>Warning:</strong> Whenever you enable caching for logged-in users (without a separate cache for each user), the WordPress Admin Bar <em>must</em> be disabled to prevent one user from seeing another user\'s details in the Admin Bar. <strong>Given your current configuration, %1$s will automatically hide the WordPress Admin Bar on the front-end of your site.</strong>', SLUG_TD), esc_html(NAME)).'</p>';
<?php endif; ?>

Warning: Whenever you enable caching for logged-in users (without a separate cache for each user), the WordPress Admin Bar must be disabled to prevent one user from seeing another user's details in the Admin Bar. Given your current configuration, ZenCache will automatically hide the WordPress Admin Bar on the front-end of your site.

[renzms]

@jaswsinc ,

Ok thanks, I'll add that in, also which file will I add this to? Plugin.php?

How will I test this though as I never set it as enabled before? I will have hidden the option from my 497 version. If I rollback to a previous version, enable that setting, then overwrite with my 497 version, won't it delete that in the database and not remember I enabled it before?

[jaswrks]

Ok thanks, I'll add that in, also which file will I add this to? Plugin.php?

This should appear just underneath the dropdown menu that you worked on here.

How will I test this though as I never set it as enabled before?

In your working copy of ZenCache, create an MU plugin to enable this feature by force.

Create: /wp-content/mu-plugins/zc-hacks.php

  <?php
  add_filter('zencache_options', function(array $options){
      $options['when_logged_in'] = '1';
      return $options;
  });

[renzms]

@jaswsinc Thanks! I will try that to test out the changes via MU plugin.

[renzms]

@jaswsinc

Tested and successfully working, let me know if its ready for a push. Thanks!:

[jaswrks]

Tested and successfully working, let me know if its ready for a push. Thanks!

Thank you. Yes, lookin' great in your screenshot!

I need to see a PR though. Don't worry about having everything just right before you open a PR. If you feel ready for a review, even if you're not sure that everything is just right, go ahead and open a PR so that I can review the diff and the changes that you made.

[renzms]

@jaswsinc

I need to see a PR though. Don't worry about having everything just right before you open a PR. If you feel ready for a review, even if you're not sure that everything is just right, go ahead and open a PR so that I can review the diff and the changes that you made.

Noted! Will submit a PR now. Thanks!

[raamdev]

@jaswsinc Reviewing Renz's PR it appears that the intention here is to only remove the option to enable Logged-In User Caching without maintaining a separate cache, is that correct? Earlier in this GitHub issue (namely here: #497 (comment)) I was under the impression that we were removing the entire Logged-In User Caching feature altogether.

However, if that's not the case that's good, because my feeling is that this is a really popular feature (being able to cache logged-in users) and I'd rather not see it go. :-)

[jaswrks]

only remove the option to enable Logged-In User Caching without maintaining a separate cache, is that correct?

That is correct :-) This particular setting has the potential to create a security problem, so removing it from the available options seems like a good idea to me. It can still be enabled with a filter though.

[raamdev]

@jaswsinc writes (in #497 (comment))...

I would also vote to get rid of that option altogether; only because this particular feature has the potential to create a security issue if not configured properly.

This was where I misunderstood you. Re-reading that now, I realize you were just referring to the Yes, but DON'T maintain a separate cache for each user option, not the entire Logged-In User Caching feature. :-) My mistake.

[raamdev]

@jaswsinc writes...

It can still be enabled with a filter though.

Has that been implemented yet? I didn't see that in the PR.

[jaswrks]

No, it wasn't introduced as a part of this PR. However, you can accomplish this with a filter that we have for the options array.

<?php
add_filter('zencache_options', function(array $options){
    $options['when_logged_in'] = '1';
    return $options;
});

[raamdev]

you can accomplish this with a filter that we have for the options array.

Ah, right. I forgot about that. Very cool. :-)

[raamdev]

Next Pro Release Changelog:

• Enhancement: In Logged-In User Caching, the "Yes, but DON'T maintain a separate cache for each user" option has been hidden because this particular option has the potential to create a security issue if not configured properly. If you were already using this option, it will not be hidden and it will continue to work. Otherwise, if you require this particular option you can now enable it using a filter (see this comment). Props @renzms @jaswsinc. See Issue #497.

[raamdev]

ZenCache v151114 has been released and includes changes from this GitHub Issue. See the v151114 announcement for further details.

---

This issue will now be locked to further updates. If you have something to add related to this GitHub Issue, please open a new GitHub Issue and reference this one (#497).