• I too fell foul of the 2.2.0 upgrade. Whilst 2.2.1 removes the PHP errors, it does not seem to have returned the functionality. Trying to trace what is happening using the filters in the “developers” code showed that a number of the filters (e.g. content_is_restricted) where comments suggest argument is post_id are now being passed either post_id or term_id. Given that posts and terms can have the same ids this looks like a recipe for disaster.

Viewing 15 replies - 1 through 15 (of 15 total)
  • Thread Starter peter8nss

    (@peter8nss)

    It looks like I can get the functionality back by commenting out the Query Terms functionality. Specifically, by commenting out the add_action in QueryTerms::init().

    I was considering doing this using the query_filter_init_hook filter but I can’t because the posts and terms filters use the same name.

    In reality, I’m going to have to stick with version 2.1.0 until the new query terms functionality has been properly resolved/tested.

    Plugin Author Daniel Iser

    (@danieliser)

    @peter8nss – We are already moving to disable that as a generic feature, enabling it only where its safe for now (rest API).

    That said we need to know where the conflict is coming from, it has to be conflicting with another plugin/theme/block that is calling those queries, and duplicating that issue on our test sites will go a long way to working out how to resolve it properly. Otherwise will just be guessing as to why and solutions.

    That said all of our code that accepts both WP_Query or WP_Term_Query, do type checking, and our rules checks also do contextual checking, meaning it knows if its checking Term queries or posts, and subsequently applies the correct logic with that ID.

    Agree its not ideal, but having to duplicate all our functions for 1-2 changes under the hood was not ideal, especially as we already had context based methodology there from v2.0.

    IE rules know if they are checking the main query, rest query, main post loop, secondary post loops, term loops etc.

    Plugin Author Daniel Iser

    (@danieliser)

    We will revisit this entirely and refine it in v2.3.

    v2.2.2 has this query filter disabled outside the rest api.

    Thread Starter peter8nss

    (@peter8nss)

    v2.4.0 still has classes/Controllers/Frontend/Restrictions/QueryTerms.php so the query terms functionality is switched on in that version.

    When I do a WP CLI: “wp term list post_tag” I get no results despite there being a number of tags defined. I think the plugin is applying my rule “Content Is Not A Page with <taxonomy>” to these terms and since it doesn’t think WP CLI is a logged in user, it blocks their display.

    Looks like a problem that “Content is not…” rules will be applied to terms as well as posts.

    To get around this, I’ve had to turn off the Query Terms functionality by:

    add_filter( 'content_control/pre_query_can_be_ignored', '__return_true' );
    Thread Starter peter8nss

    (@peter8nss)

    I’ve done some more investigating and this appears to be linked to the negated content conditions. If I have a restriction:

    • General: Logged in
    • Protection: Redirect
    • Content: Content Is Not A Page with <taxonomy> = <value>

    And my home page has <taxonomy> = <value>.

    So, I would expect to be able to view my home page whether I’m logged in or not. However, when I try to view it when not logged in I get redirected. I think what is happening is:

    • Page check works fine
    • Query Terms check matches “Content Is Not A Page with <taxonomy> = <value>”, so it triggers a redirect because I’m not logged in

    If I turn Query Terms checking off using the filter described in the previous post, I can view the home page whilst not logged in.

    Plugin Support Kim L

    (@kimmyx)

    Hi @peter8nss,

    Thanks for the update!

    WordPress Pages don’t have taxonomies by default. Are you adding your page taxonomies using a plugin?

    We’ve informed our dev team about this and regarding your recent follow-up on the previous post as well.

    We’ll let you know once we receive feedback.

    We appreciate your patience! ??

    • This reply was modified 4 months ago by Kim L.
    Plugin Author Daniel Iser

    (@danieliser)

    @peter8nss – Given your home page matches restricted content profile, I would fully expect it to be restricted.

    There is no reason it shouldn’t be given your setup example.

    If you want it to not be restricted explicitly, you can add an AND Content Is Not Selected Page (Home Page).

    Otherwise you would effectively be bypassing matched rules incorrectly.

    As for the WP CLI, all requests by default are unauthenticated, as such restrictions should be applied (otherwise we would get “Security vulnerability reports” saying our plugin has vulnerabilities ??).

    However you can pass --user with an ID I believe and force it to run as a proper authed user.

    Let me know if I missed something.

    Plugin Author Daniel Iser

    (@danieliser)

    Sorry, reread that and noticed you said

    Content: Content Is Not A Page with <taxonomy> = <value>

    That said the negative conditions simply check for a false value instead of a true, there is no inherent difference in how they are processed at all.

    Checking more on that now, but each context processes conditions differently, IE post query & main query checks rules differently than term queries.

    Plugin Author Daniel Iser

    (@danieliser)

    Ok so after some more digging/testing, it all seems to be working as expected for those tests.

    The one issue might be that since your using negative conditions, those would broadly match any content on that pages loop as well. IE if your home page shows a list of posts, and one of those posts is restricted, it could still trigger the redirect depending on your protection settings.

    I am seeing a small discrepancy in tag archive handling, but that seems unrelated to your particular report.

    I’m hesitant to make random changes without fully being able to deduce the root problem, but I’m happy to keep exploring this issue, especially if you have further guidance.

    Thread Starter peter8nss

    (@peter8nss)

    Re WP CLI

    Isn’t WP CLI considered to be an “admin” access as it is only available to those with access to the website server not web users. So should it be subject to the “Exclude administrators from being restricted” setting?

    You are correct you can set any user for WP_CLI using –user. And, if you set a user, the command does produce results.

    I can also bypass content control for WP CLI by adding a filter for “content_control/protection_is_disabled” such as:

    return ( defined( 'WP_CLI' ) && WP_CLI ) ? true : $protection_disabled;

    Re negated rules (e.g. Not this AND Not that)

    I think the problem is that before Query Terms checking was introduced the megation rules were implicitly: A post (of some post type) AND not this. Now rules are (A query OR A post (of some type)) AND not this (post). So whilst the rules work fine when when working on a post, page, etc, they match ANY query.

    I tried explicitly adding “Content is entire site (Any page, post or archive)” but that doesn’t work as the callback for that is always true. IMHO either the callback needs renaming (as the brackets are now inaccurate) or there should be a callback for that rule that actually checks it is a post of some post_type. What does seem to fix things is if I explicitly AND in a group containing IF “Content is A Post” OR “Content is A Page” OR “Content is A Media” etc.

    ps: I’m struggling to think of any case where “Content is entire site (Any page, post or archive)” returning true is actually of use, i.e.:

    • if you removed it from any ANDed restriction/group the restriction the result would still be the same;
    • if it was in an ORed restriction/group, you might as well delete the entire restriction/group as it doesn’t matter whether the other elements return true or false, the ORed set will still return true.

    Having it, just slows down execution.

    Plugin Author Daniel Iser

    (@danieliser)

    Isn’t WP CLI considered to be an “admin” access as it is only available to those with access to the website server not web users.

    Yes, and no. Requests are explicitly unauthenticated by default. Yes you can manipulate things and install plugins etc, but querying a list of content on a site with content restrictions should in all likelyhood respect those restrictions unless otherwise specified, less someone will say it isn’t properly restricting things.

    So if you didn’t have any content restriction, then yes you should expect full results, but given you have gone through the process of adding access control, it should be respected as well.

    Not hugely different than the REST API really when talking about fetching data.

    In terms of this plugin’s user base, my guess is less than 0.1% of the users would realistically be using it, and of those how many honestly are using it to fetch content (not a common use case at all really).

    So I’m not even sure it warrants a dedicated separate option to bypass restrictions for it, since users who are using it are much more likely to be able to simply add the required filter like you mentioned above.

    I think there is a bit of confusion about how rule processing is handled. The rules were always applied to terms via archives and the main query. The only addition was processing Term_Query term loops, such as a tag cloud, or list of terms shown in a blog posts meta. The rules were simply adapted to not do anything if they have nothing in common with the given context.

    For example rules like Is a Post or Is a page will simply bail early with false when the context is a Term_Query. Rules like Is Post with Term, or Is Term Archive were always there, just only applied to the main query itself.

    ##

    As for the Entire Site rule, that was added for only a couple use cases, but specifically where they do want the entire site locked down. IE I want my entire site protected, but I don’t want to have some convoluted set of rules like “Content is a page OR Content is a post OR content is a tag OR content is a …”.

    Without that rule, protecting the entire site requires adding rules for every potential content type.

    With it you can add one rule.

    If you were to want to protect everything except one type of content, then you could simply use a negative rule of “Content is not a Post”.

    As for slowing execution, no that simple rule always returns true, ZERO processing required, thus much faster than … OR … OR … OR … which would process all of them effectively until it found a match.

    Overall the rule engine here hasn’t changed the way it works since before CC v1 was released. This same rule engine powers our much larger plugin Popup Maker as well and has been refined and improved mainly.

    The key differences now though are context aware rules, if there are any issues this is the only place it could come from given the rest of the functionality has been well tested and perfected over the course of nearly 10 years.

    For example Posts with Tax should always return false for a Term_Query, as no Term is a Post.

    But for the main query, main query posts, or Post_Query it should be checked consistently.

    So my thoughts for your scenario (though I’m not quite sure what your setup should be), is that you likely just need help aligning the rules you set with the expectations of what content you want it to restrict.

    Can you tell us exactly what you are trying to do, lots of what aboutisms here but we can chase those for a long time an never really find any issues to solve. If you say you need content x/y/z protected, we can guide you on exactly what rules and combinations of AND/OR/Groups to use.

    I’m reviewing all of the term query rule context handling today, but I don’t foresee it leading to any bug fixes, more likely simple performance enhancements with bailing early where appropriate, and even that has minimal real world impact given we cache rule processing for the duration of the request (and long term via Object Caching in Content Control Pro).

    Hope I didn’t miss anything.

    Thread Starter peter8nss

    (@peter8nss)

    Thank you for your detailed response. Sorry this topic is now covering quite a few separate issues

    WP_CLI:

    Agreed it is debatable how this should behave, and the filter is a relatively simple alternative. So no action required.

    Entire site rule:

    • I accept it could be used to allow login or restrictions.
    • With the introduction of Query Terms, it might be useful to have a rule which matched any post (of any type) and returned false if used to test a query term.

    REST API

    Would be nice to have a filter that allowed REST API checking to be turned off, as many people won’t need it, or will already have something else dealing with it.

    Query Terms:

    I accept that rules about “is page…” etc will bail early when encountering a term query. The problem is the negation rules. For instance “is not page…” will generate true for both “posts” and term queries!

    See also Term Rules Oddity

    My situation

    I have a site where the content visibility can splits into three groups:

    • Public – available to anyone whether logged in or not
    • Members – default if content is not marked otherwise, available only if logged in
    • Leaders – a few pages, etc marked for limited viewing by certain roles

    This is implemented by having a custom taxonomy with values Public and Leaders. And two restrictions:

    • 1 leaders only
      • General: Logged in users with a few specific roles
      • Protection: Redirect
      • Content: Pages with <my taxonomy> = Leaders
    • 2 Not public
      • General: Logged in users any role
      • Protection: Redirect
      • Content: Not Pages with <my taxonomy> = Public AND not search page

    Which worked until Query Terms were introduced and still works fine if I turn it off using a filter.

    The reason I’ve noticed so much about query terms is that I use two plugins: The Event Calendar and Quick and Easy FAQs. Including bits of their content on pages means I get queries when loading some pages and hence stumbled across the problem. And my mathematical/IT background made me curious about the implication of negation rules.

    Plugin Author Daniel Iser

    (@danieliser)

    With the introduction of Query Terms, it might be useful to have a rule which matched any post (of any type) and returned false if used to test a query term.

    I like this idea, but that would have to be a new minor release (2.X) rather than a simple patch. Simple enough though.

    Would be nice to have a filter that allowed REST API checking to be turned off, as many people won’t need it, or will already have something else dealing with it.

    Theoretically you can already using the filter discussed previously bypassing restriction checks.

    On the other hand we can’t realistically assume anything such as users won’t need it. In fact I’ll point you here as to why it was ever added in the first place: https://wpscan.com/vulnerability/4a683d85-6528-49f7-9f10-c1f59bf1db6b/

    Now we dispute that it was ever a “vulnerability”, we did so at the time, in our readme we list it as a new feature instead of a security patch, and openly disputed it with both www.remarpro.com team and the original reporters.

    In fact we had code that explicitly opted out of applying restrictions to the REST API from v1.0 all the way to v2.1.X, but were essentially forced to add new functionality (that nobody had actually requested).

    Removing it now effectively reopens that, not realistic for us.

    For instance “is not page…” will generate true for both “posts” and term queries!

    As it should?

    For your setup, what exactly is going wrong at the moment?

    • Not a search page is a special rule type, it only applies to main query contexts, as does other specialized rules like “Is Home Page” etc. Those should have no bearing on term queries.
    • What do you have set in your restrictions for the Posts in Archive & Posts in other locations additional options? These are what control how it works outside of the main query (the url visited), such as loops in widgets, footers, post meta etc.
    Thread Starter peter8nss

    (@peter8nss)

    Rule which matched post (of any post type)

    That would be helpful. Thank you.

    REST API

    I agreed WordPress’ approach to what is visible using REST API is a bit odd.

    Whilst I do have some of my own code to lockdown REST API, I would like to make as much use of the Content Control provided functionality as possible.

    However, I’ve noticed the following REST API behaviour. If I’m logged in at a browser and I issue a request …/wp-json/wp/v2/pages/<page_id>. If <page_id> is for a page that matches a restriction:

    • for not logged in users, then the request is allowed.
    • that my browser’s logged on user can see, the request is still refused

    Presumably the latter is because the REST API is not considered to be logged in, but it does seem odd that the same browser can view the page content but not the json REST version of it.

    What exactly is going wrong at the moment

    With the various points already discussed in this and other topics I think my problems should be addressed.

    Plugin Author Daniel Iser

    (@danieliser)

    REST API

    Presumably the latter is because the REST API is not considered to be logged in, but it does seem odd that the same browser can view the page content but not the json REST version of it.

    Unless you are passing a proper authorization bearer header with the request, or a valid JWT token in the url, the request is treated as unauthed. It doesn’t care about the wp login cookie used by front end pages.

    for not logged in users, then the request is allowed.

    Are you saying that a restricted page is accessible, or that the request doesn’t throw an error and shows something else?

    What exactly is going wrong at the moment

    That wasn’t a question of whether to address your problem, it was to make sure your attacking it the best / current way that will do the job.

    It might just be that your thinking about it differently and overlooking simpler solutions that might exist now.

    Knowing you want to protect X for user Y, and Z for user A we can tell you how we would do it, or better understand exactly how you keep running into issues others are not, rather than constantly chasing “possible” issues. IE it might work how you want already, just not the way you expected.

    Sometimes we (users of software), don’t use it entirely the way it was intended and expecting it should just work, often over complicating things to some degree. Especially those of us who are technically proficient. I know because I do this too.

    Simplest way to get to a working solution is to define a problem like this:

    1. Expected: How do you expect it to work?
    2. Outcome: How is it working now?
    3. Setup: What have you tried so far?

    For your case though this should be specifically applied to your entire setup, mainly because you’ve defined multiple “problems”, all disparate, but somehow still related to how you want it set up. So instead of doing the list for each separate issue, do it once for your overall expectation/outcome/setup, and let us try to work it out as an entire solution instead.

Viewing 15 replies - 1 through 15 (of 15 total)
  • You must be logged in to reply to this topic.