Daniel Iser
Forum Replies Created
-
@crazyserb I haven’t been able to replicate the issue, can you tell me if you have specific extensions enabled and whether they have current or expired keys?
That said I patched so these errors shouldn’t be shown, just not sure what caused them to show in the first place.
@crazyserb – Fix coming for this.
@stlpl – In this case we stuck with WordPress’s own labeling of this relationship.
There is no concept of a descendant within WP page relationship, functions or documentation even.
It should probably read “Content Has An Ancestor in Selected Pages”.
This is the specific function used when checking that rule, so you can see where it got the label from https://developer.ww.wp.xz.cn/reference/functions/get_ancestors/
We could simply rename it Descendent, but our concern is diverging from known WP concepts in ways that lead to their own confusion.Will consider it.
Thanks for the feedback.
@peter8nss – I added your simple change proposed above for now, but I think in a near future version we will completely separate how we handle terms, specifically adding internal get_term methods and separating those concerns into separate specific handlers.
@peter8nss – Unfortunately
WP_Block_Type_Registry::get_instance()->get_all_registered()will never be a reliable source as many blocks are only registered in the JavaScript build files of their plugins/themes.I’ve just updated it to do so going forward, that said WP core already did this recently so it should be converted to not autoload automatically at some point: https://make.ww.wp.xz.cn/core/2024/06/18/options-api-disabling-autoload-for-large-options/
That said run this once and then you can revisit a few of your block editor pages to regenerate that list.
delete_option( 'content_control_known_blockTypes' );We don’t remove/clean the list by default as you may have used those blocks, even if you removed that plugin. In that case our restrictions shouldn’t fail to be handled. Loading it all the time though is indeed not necessary.
We can probably set this one to not auto load, will have to see how to best handle that. Working on 2.5.0 release now with rewritten rules (with explicit contexts). Need testers if you’ve got staging already available with your configs.
@villelai – Thanks for confirming!!
@flymike – Sorry to hear you had issues.
- Your correct, priority does matter, mainly for performance of the handling. We stop processing after we find a match.
If you don’t have a lot of restrictions you can use a snippet of code to check all restrictions and bypass priority to some degree. Not the ideal solution, just something you can do. - The error is likely coming from another plugin/theme causing interference. You can try the Health Check & Troubleshooting plugin’s “Troubleshooting Mode” to test just our plugin then activate others one by one to find the culprit, but while you have only our plugin active, might as well go ahead and update your priority order to.
There is very little we can do on our end if some other code is globally hooking into the TinyMCE editor and extending it. We enqueue it the WP way, and don’t have any code related toMouseEvent.mozPressure, our build tools would automatically generate compliant modern replacement or flag it in our code editors with highlighting and build time error notices, sl highly likely it comes from something else.
We also have a guide from one of our other plugin’s docs on using error notices in the console to try and find the problem JS file (plugin/theme): https://docs.wppopupmaker.com/article/373-checking-javascript-errors
Let us know.
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:
- Expected: How do you expect it to work?
- Outcome: How is it working now?
- 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.
@peter8nss – I think I see the issue.
We have “default” switch cases currently, I think removing them, and returning false by default on fall through (no switch case match) should resolve it efficiently.
Adding a
case 'terms': return false, effectively does the same thing, explicitly, but I don’t think explicitly is necessary if we remove thedefault:and fail gracefully for all non matches.Our goal is to minimize case matches, rather than match every option. Mainly for maintainability, supposing we add new contexts in the future for example.
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 ww.wp.xz.cn 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.
@peter8nss – Super helpful thanks. Will get this patched.
Also if you added custom taxonomies to your pages, those would also appear and work as expected as well.
But we can’t just add rules for content types/structures that WP doesn’t recognize, that would be very confusing.
@flymike – The content rules are auto generated from content types registered within WP.
To be clear, if you install a plugin that enables categories for pages, those rules will magically appear 😉.
@peter8nss – Do you happen to know how you triggered this? I have QM on 100% of the time during development and haven’t come across this, there must be a conflict or config set that were are not testing against.
Furthermore, I don’t think all the post_ids that are passed are actually post ids. Some are term_ids, so the title lookup is getting the title of a post with the same id as that term.
Very possible, the QM integration was added before TermQuery processing was added. I’ll have to rework that a bit to ensure we keep term & post lists separate.
For terms it might help to indicate the taxonomy they come from, e.g.
Column titles might also need revising to be more generic, e.g. ID, Title/Name, Type
Excellent ideas.
In this case your running into a performance optimization. By default only the first matched restriction is used (sorted by priority of restriction sets). This prevents needless extra rule checks etc.
To process all of them, in cases where a user might match more than one at a time, you can enable processing of all restrictions:
The filter there will do what your after, processing the secondary restrictions as well.
—
Keep in mind 98%+ of our users only have one restriction set up for any given user role/type match, so that is what we optimize for, but we do offer the flexibility to do it the other way as well for those that need it.
- This reply was modified 1 year, 10 months ago by Daniel Iser.
- Your correct, priority does matter, mainly for performance of the handling. We stop processing after we find a match.