[RFC] Why Do Plugins Support Access Levels?

[mbabker]

What is the actual purpose or valid use cases which should allow plugins to not be loaded at all because a user's access level doesn't match the configuration? Are there really legitimate use cases where the voting plugin should not be loaded at all for guests but loaded for registered members? Or the term highlighting capabilities? Or even crazier, the Smart Search plugins?

Doing this forces loading plugins to be dependent on the session.

[Bakual]

I could only think of the debug plugin, which I set to Super Users only. But this one also has a parameter to select user groups which are allowed to see the debug information.

[mbabker]

You should still be able to run the debug plugin as an anonymous user (it'd be more useful if we logged request data somewhere instead of relied 100% on being able to show the console at the end of a page, so you can't debug redirected requests without hacks). Remember there are changes in behavior for guest versus authenticated, and probably Registered versus Manager (using our default ACL levels).

[HLeithner]

If you use a plugin for com_ajax it "could" be useful. But doing the access validation in the plugin it self would be better.

I use thisnalso for my ohne debug plugin but once again, if needed the plugin can do it in the constructer.

[alikon]

#18462

[coolcat-creations]

For Editor Buttons, for Custom Fields, For the debug Plugin for example

[bembelimen]

Hi,
I personally use plugin access levels heavily for 3rd party stuff:

• Payment methods in a shop depending on the access level
• Select options, generated by plugins (a list form field is extended by plugins), which offers different functionality depending on the access levels.
• editor buttons are visible depending on the access level
• many more

I think the question should be: shouldn't we rebuild the debug plugin, so it uses the default behaviour (access level approach)?

[infograf768]

We should indeed keep Access Levels for plugins AND implement @alikon proposal to introduce ACL per plugin in #18462

[Bakual]

The proposal by Alikon isn't about access levels (eg public, registered, special). It's about ACL settings (eg create, edit, publish).

[infograf768]

indeed, and it makes sense.

[mbabker]

All of these are valid use cases for altering user behavior based on the plugin events themselves.

My question here is why is the actual loading of plugins coupled to access levels (as in why is the database query filtering based on access levels and inherently plugins not being instantiated based on that result). What are the valid use cases where a plugin is in full NOT loaded because a user's account doesn't match an access level?

Each of these use cases to me are decisions that should be checked in the event listeners, especially as a plugin may need to perform an action for a non-authorized account (for example the {loadmodule} marker being replaced with an empty string if an account isn't authorized to view the specified module).

So I'm not saying that plugins shouldn't be able to do different actions based on access levels. I'm saying I don't understand why this line exists in the code. To me it's about as artificial as the plugin groups and us trying to excessively lazy load system resources.

[HLeithner]

Is this the first line where the user is loaded? Or is it already loaded?

I'm not sure what you are trying to do, would you like to leverage the access level into the plugin constructor or would you say it's not the business of Joomla core to filter the plugin access and each plugin have to do it on it's own?

Would caching works without access levels? Especially from security point of view?

Is this about performance or less code to maintain?

[mbabker]

That line is essentially where sessions are getting started in 4.0 at all times.

I'm not sure what you are trying to do, would you like to leverage the access level into the plugin constructor or would you say it's not the business of Joomla core to filter the plugin access and each plugin have to do it on it's own?

I'm challenging what capabilities exist that call for plugins to outright not be loaded on a request because the user's access level doesn't match what is configured in the plugin. Given most plugins will always remain at the Public access level because it is destructive to restrict a lot of plugins (the authentication group being a big one, a lot of content plugins need to run regardless of access to at least strip placeholders, system tasks like update notification emails or session garbage collection should always be available and the plugins handle restricting who sees the result of their actions as appropriate), I am not seeing why this capability needs to exist in core.

What I want to see is the access level configuration removed and inherently the dependency to the active user in the load query removed as well. Because I do not believe there is a strong justification for a plugin class to outright not be loaded because its capabilities target registered users, I believe this level of access checks belongs in an event listener (i.e. onContentPrepare since that's the common one affecting frontend) because if the plugin is dealing with restricted access in a lot of cases it will need to do some kind of cleanup (go back to the loadmodule shortcode example).

The debug plugin somewhat handles this correctly. It allows the debug capability to run at any access level but you can restrict the display of the console (and potentially sensitive information) to certain groups. This is the type of configuration I believe should be used in plugins to do ACL type restrictions, not the existing structure which causes plugins to not load at all.

[HLeithner]

So without this access level check it would be possible (or at least we are a step further) to load the frondend without a user session?

If this is the goal or at least a benefit, it would be great the leverage the access check to the plugin it self. Also because every on-call could have it's on restriction may never triggered...

Would it be possible (or better has the same affect?) to do the check in the constructor for b/c if the accesslevel field exists?

[mbabker]

So without this access level check it would be possible (or at least we are a step further) to load the frondend without a user session?

It goes a step further in loosening the hard session couplings. I don't know if we'll ever break them all but the more we can decouple without feature/benefit tradeoffs the better.

If this is the goal or at least a benefit, it would be great the leverage the access check to the plugin it self. Also because every on-call could have it's on restriction may never triggered...

Would it be possible (or better has the same affect?) to do the check in the constructor for b/c if the accesslevel field exists?

There's no B/C "mitigation path" here. If we remove the filter from the query, the plugin is going to get loaded into the event dispatcher without adding some (what are going to be performance tradeoff) checks in either the root CMSPlugin class or the PluginHelper class. The access level isn't stored to the plugin params, it is a separate column in the #__extensions table.

If you need to implement access level restrictions in your plugin, it has to be a separate configuration section in your plugin's parameters (similar to what the debug plugin does for displaying its output). You cannot rely on the current configuration setting exposed by com_plugins, it is too high risk and has significant side effects if you misconfigure it (again the load module plugin, say you restrict that to registered users only, guest users will never have the {loadmodule} shortcode stripped from content because the plugin will never load with the current config, if you wanted that plugin to support access restrictions you need a separate configuration value in the plugin params that is checked when onContentPrepare is dispatched so the plugin knows it should just replace the shortcode with an empty string).

[alikon]

imagine if you don't care for a moment about "B/C"...
don't is this a semver prerogative ? ... even more after all ❓

[mbabker]

Yes there is a B/C break. I'm not saying there isn't. I'm saying the way this filter runs right now is so far beyond wrong that it's more damaging than letting users uninstall core extensions because if you misconfigure these access levels it WILL either bring down your site or lock you out of it completely and that we need to make the architectural level B/C break to fix this (oops, there's that word that's going to scare people away 😉).

[alikon]

maybe semver is a strange thing 4 me
but what's wrong ...
we are simply giving developers the freedom/responsabilty to do fine grained tasks
this is the trade off for me so a win2win

[ggppdk]

Yes there is a B/C break. I'm not saying there isn't. I'm saying the way this filter runs right now is so far beyond wrong that it's more damaging than letting users uninstall core extensions because if you misconfigure these access levels it WILL either bring down your site or lock you out of it completely and that we need to make the architectural level B/C break to fix this (oops, there's that word that's going to scare people away 😉).

Can access levels be allowed to only plugins (or plugin types ?somehow?) that explicitely specify that have a use for it ?

[mbabker]

No. It is added arbitrarily by the com_plugins form. This is the form field that needs to be removed in full in addition to the referenced query filter.

Plugins which DO have a need for access related configuration need to implement a field (or fields if there are different capabilities which need different behaviors) similar to the debug plugin and implement the needed changes in their event listeners (again something the debug plugin already does).

[infograf768]

@mbabker
I guess I understand what you mean, but then what about #20256 (comment)

Shall we start refactoring all these as they should also contain specific fields similar to the debug plugin?

[ggppdk]

Plugins which DO have a need for access related configuration need to implement a field (or fields if there are different capabilities which need different behaviors) similar to the debug plugin and implement the needed changes in their event listeners (again something the debug plugin already does).

But it should have been done in the begining, now it is too much of a B/C break ?

What about making a change in plugin manager (plugin model)
and locking access level (property) field for some plugins protecting it from being changed and add support for this behaviour in XML file of the plugin ?

[mbabker]

Shall we start refactoring all these as they should also contain specific fields similar to the debug plugin?

• For Editor Buttons - If there are legitimate use cases where the article button or page break button should show for one user group and not another, sure
• For Custom Fields - No, field access levels are already set in the com_fields edit screens, the plugins themselves do not also need to be access aware
• The use cases in [RFC] Why Do Plugins Support Access Levels? #20256 (comment) are all prime examples of how this feature makes sense but is misused, the use cases are all valid depending on workflow but again the checks for these things should be done in the plugin event listeners (onContentPrepare, onAfterDispatch, etc.) and not in PluginHelper::load() which causes the plugin to not be loaded in full (which can have bad side effects, again go back to the loadmodule plugin example, if set to Registered then unauthenticated users will see the shortcode in content and not a replaced value)

But it should have been done in the begining, now it is too much of a B/C break ?

What about making a change in plugin manager (plugin model)
and locking access level (property) field for some plugins protecting it from being changed and add support for this behaviour in XML file of the plugin ?

Look, there is no way to do this in a B/C manner and if that is really going to be a roadblock I'll just drop the subject. Without an expensive performance hit, you cannot remove the filter from the query in PluginHelper::load() and emulate it in some other way. Period. Either at this point we accept that Joomla is critically flawed as it relates to this configuration or accept a B/C break without a migration path and fix it. How many plugins in the ecosystem exist today where it is really expected that they should ONLY be run when an authenticated user is browsing the site (as in the site owner/manager really should set the Access Level configuration to something other than the default Public)? How many site owners or plugin developers understand the consequences of this decision? Clearly I seem to be one of the only few really concerned with fixing legitimate architecture concerns and flaws in our UX that makes it too easy for users to screw up sites.

[Bakual]

Isn't there a rewrite how the plugins work in J4 anyway? I haven't looked into it yet but if there is a new system it could make sense to support the access levels in the legacy plugins and drop it from the new way.
That way we could ease the B/C break a bit I guess. If that's possible of course - as said I don't know the specific of the changes in the plugin system.

[mbabker]

Isn't there a rewrite how the plugins work in J4 anyway?

Yes and no. Yes in that plugins can work with the revised Framework Event API, no in that there are zero required changes. In either case, this access level field is not part of a plugin's parameters but part of the form created by com_plugins. So no, you cannot do a "have this field on legacy plugins and not have it on new plugins" switch.

It is seriously all or nothing and given the reactions here I'm about to close this in favor of the nothing option.

[HLeithner]

So the point is that plugins that needs ACL should do it on it's own (maybe with more possibilities ex. per event type call).

That would be an improvement for all, but some work for the plugin authors... I think its worth doing it but I think most people are afraid that they have more work to do in their plugins or the original author of a 3rd party plugin doesn't care about access control...

Any alternatives to do the filter at a later time, optional in the constructor oder plugin loader? Depending on the plugin settings/xml parameters?

[mbabker]

Any alternatives to do the filter at a later time, optional in the constructor oder plugin loader? Depending on the plugin settings/xml parameters?

No, there isn't. Even if you parse plugin parameters in your plugin class' constructor, it still means the plugin is going to be added to the dispatcher and set as a listener for the events unless you really want to run some janky code to unsubscribe your listeners.

I'm done here. I can't seem to convey the issue or what is required for a proper resolution so I'm not pursuing this any further. If anyone else is interested, good luck.

[alikon]

if we can't change even in presence of a new major version then when we can change something ? never ever ?

[csthomas]

IMO Joomla 4 should remove the check of access level from the core.

joomla-cms/libraries/src/Plugin/PluginHelper.php

Line 311 in 69eaf4e

->where('access IN (' . $levels . ')')

Any alternatives to do the filter at a later time, optional in the constructor oder plugin loader? Depending on the plugin settings/xml parameters?

Maybe I do not have enough experience but I see
a way even for J3.9:

1. Load all plugins and store in cache - 1 item only. PluginHelper::load()

2. For each plugin with an access level other than "Public", check the access level in php code.

   • this can be added at the bottom of PluginHelper::load() method or somewhere later.
   • to be B/C it should only returns "allowed" plugins.

3. Remove/deprecate the column/filter "Access Level" from the view of plugin list in backend.

   • probably we could add a red background for plugins with non public access level.

4. Deprecated parameter "Access Level" in the plugin configuration form.

[stutteringp0et]

If the choice is to rely on Joomla ACL to control plugins loading, or 3rd party developers to implement sane ACL within their plugins, I choose Joomla ACL. There is some poor quality code floating around out there, and I'd be willing to bet that most will not implement any ACL, many will implement it badly or wrongly, and there will be a precious few shining stars.

[ggppdk]

I have made a PR #20713
Respect access of editor plugin inside profile form

If access levels for plugins are to be dropped
then maybe not merge PR #20713