Plugin filtering for admin list pages

Bloke · 2009-07-09 16:33:16

I wonder if this, currently rather dangerous patch, is too hackish a concept to be considered? It’s 1 extra line in txp_list.php to allow additional clauses in the criteria.

This hook allows plugin authors to more easily show only the current author’s articles, or hide articles in certain sections, etc. For reference, my first attempt was using pluggable_ui(), which seemed a lot cleaner to me. The drawback was that simply omitting list items screws up the pagination :-(

Anyway, is this idea stupid or does it have a valid application? It could of course be extended to Image, File and Link pages if deemed useful. Or is there a better way to do this, besides allowing raw SQL to be added to the clause, which I must admit makes me slightly nervous.

Thoughts?

Last edited by Bloke (2012-07-17 23:42:06)

net-carver · 2009-07-09 17:00:55

How now StefBloke?

Is this idea stupid or does it have an application alongside the power of per-author and per-priv enhancements in 4.2.0? <snip> Or is there a better way to do this, besides allowing raw SQL to be added to the clause, which I must admit makes me slightly nervous.

I can think of at least one place in the MLP pack where this would come in handy.

Perhaps the hook should not allow totally raw SQL, but do some kind of sanity check on the returned values. For example, it could truncate at the first ' OR' in the returned value and only allow the addition of ‘ANDs’ to a WHERE clause. So the hook returns a string like `author` <> 'stef' and this gets tacked on as WHERE blahblah AND (`author` <> 'stef'). Should a rogue string like `author` = 'stef' OR 1=1 be returned, the truncation would kick in.

Probably need to cover other cases as well; this is just off the top of my head to kick things off.

Bloke · 2009-07-09 17:37:06

net-carver wrote:

Perhaps the hook should not allow totally raw SQL, but do some kind of sanity check on the returned values [snip]

Works for me. The forced ‘AND’ is a good idea (might make the patch an extra line to detect if the callback has been used). I can certainly think of a situation where chaining a few clauses might be useful, for example, AuthorID = 'stef' AND Section NOT IN ('omitme1', 'omitme2'). Struggling to find an OR case, so your logic appears sound, but there’s probably a legitimate use of OR out there. Somewhere?

One thing I didn’t consider is how the thing behaves if two plugins try to employ the hook. I think it still works. Ummm, I think.

Last edited by Bloke (2009-07-09 17:40:51)

hakjoon · 2009-07-09 22:05:20

I think this would be great. I’ve had a few ideas that I wanted to tackle that would have benefited from being able to edit the output lists in the admin to exclude certain things.

Bloke · 2009-09-13 16:03:47

Just an addendum to this mod. If you’re mad enough to use it in its above version, some plugins (e.g. lam_browseby) will show their content twice on the page because that plugin hooks into the event="list" and ignores ‘step’ — since there isn’t normally a step associated with the list page, this makes perfect sense.

The temporary solution, albeit mucky, is to rename the event for this mod, for example:

$criteria .= callback_event('list_filter', 'criteria');

And then use that event in your register_callback()s instead. There’s probably a far better way to achieve this kind of functionality.

Last edited by Bloke (2009-09-13 16:04:17)

Bloke · 2012-07-17 23:49:14

Reviving this thread. Looking ahead to an unspecified version of Textpattern > 4.5.0, I think there’s a use case for plugins to be able to alter the list pages. For example (as mentioned above), to omit certain sections or authors from the list. A “show only your assets” plugin would then be a cinch to write, if a callback similar in concept to the one I proposed in the OP was applied on the pages of all core types.

There are three main problems though:

Security
Security
Plugin contention

Point (1) is potentially massive and should really occupy two slots in the list: if a plugin could rewrite or append to the SQL statement then any kind of injection imaginable is possible unless some hardening is applied, akin to the kind of avenue net-carver mentioned above.

Not sure if point (3) is a concern for regular callbacks (istr pluggable_ui can only service one plugin at a time and one will overwrite the other, not augment it) but if two plugins try to alter the same SQL statement on a list panel then there could be big time breakuality.

So my question is whether this concept could ever make it to core and, if so, in what capacity would the core have to be altered to ensure it could be done safely?

Any ideas anyone?

Last edited by Bloke (2012-07-17 23:50:56)

mrdale · 2012-07-17 23:59:45

no ideas, but if it let’s me cull one section (that’s 90% of the articles, but edited 1% of the time) from the articles page, I think I’m plus one-ing this bad boy [+1?]

etc · 2012-07-25 21:42:24

What makes the existing callbacks more secure than this one?

Bloke · 2012-07-25 21:51:26

etc wrote:

What makes the existing callbacks more secure than this one?

Hmmm, good point. I guess anyone could write a plugin that hooked into any of the callback points and execute some SQL that dropped the entire database.

In which case, the only consideration is making sure the final statement — which may be the result of more than one plugin tinkering with the where clause — is valid SQL to avoid (or at least minimise) ugly error messages. Or do we turn a blind eye to this as well: caveat utilitor?

Last edited by Bloke (2012-07-25 21:54:13)

Gocom · 2012-07-25 22:40:03

I second etc. I do not see any security hazards relating to this new potential callback.

Bloke wrote:

which may be the result of more than one plugin tinkering with the where clause — is valid SQL to avoid (or at least minimise) ugly error messages.

If this callback only allows appending new filtering rules to the where clause, then the quantity of plugins will not have an effect to the statement’s validity in terms of actual errors. There is always the case that filters get too tight or contradict each other, giving you no results. But honestly, I don’t see that as an issue either.

Or do we turn a blind eye to this as well

I wouldn’t worry about validation much. I think it’s best to keep things as simple as possible. For instance, adding just a callback_event in the SQL statement’s where clause would be enough as far as I see. Developers know what they can do and can not. If you really want you could make so that the callback functions’ returned values are joined with a forced “and” operator, but even that isn’t necessary. If the callback_event is placed at the beginning of the statement, then developers know that they will need to end their added statement with an operator to keep things working. If someone (like me, doh) screws up and writes bad code that breaks shit up, then its not the core’s fault.

I mean, filtering and security related sanitation are needed for public facing access points and everything used by end-users and frontend. Due to the nature of the platform itself, same isn’t true for programs running internally.

Last edited by Gocom (2012-07-25 22:41:11)

Bloke · 2012-07-25 23:19:29

Gocom wrote:

If the callback_event is placed at the beginning of the statement

Hadn’t thought of doing that. I guess it’s the difference between:

$criteria .= callback_event('list', 'criteria');

and

$criteria = callback_event('list', 'criteria') . $criteria;

Is one way “better” than the other? Dunno. I suppose the first one is more logical as it appends to what is already there and places less burden on the developer to remember the trailing operator, but as you say, as long as the rules are known then either would work. Toss a coin for it? :-)

I could stuff this in now on all ‘list’ pages, but does it class as a new feature when we’re technically in feature lockdown? Or is it a relatively minor addition that we can sneak in without too much worry?

Last edited by Bloke (2012-07-25 23:20:11)

Gocom · 2012-07-25 23:54:53

Bloke wrote:

Is one way “better” than the other?

Hmm. The first, no the second.. on second thought the first, no wait… Both…? The more, the better, right?

I could stuff this in now on all ‘list’ pages, but does it class as a new feature when we’re technically in feature lockdown? Or is it a relatively minor addition that we can sneak in without too much worry?

You could classify it as a minor addition, me thinks. New, very great feature nonetheless. Opens new possibilities, that’s for sure.

Textpattern CMS

Textpattern CMS support forum

#1 2009-07-09 16:33:16

Plugin filtering for admin list pages

#2 2009-07-09 17:00:55

Re: Plugin filtering for admin list pages

#3 2009-07-09 17:37:06

Re: Plugin filtering for admin list pages

#4 2009-07-09 22:05:20

Re: Plugin filtering for admin list pages

#5 2009-09-13 16:03:47

Re: Plugin filtering for admin list pages

#6 2012-07-17 23:49:14

Re: Plugin filtering for admin list pages

#7 2012-07-17 23:59:45

Re: Plugin filtering for admin list pages

#8 2012-07-25 21:42:24

Re: Plugin filtering for admin list pages

#9 2012-07-25 21:51:26

Re: Plugin filtering for admin list pages

#10 2012-07-25 22:40:03

Re: Plugin filtering for admin list pages

#11 2012-07-25 23:19:29

Re: Plugin filtering for admin list pages

#12 2012-07-25 23:54:53

Re: Plugin filtering for admin list pages

Board footer