Bug 28957 - Many plugins require the full plugins permission set to run
Summary: Many plugins require the full plugins permission set to run
Status: NEW
Alias: None
Product: Koha
Classification: Unclassified
Component: Plugin architecture (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal
Assignee: Martin Renvoize (ashimema)
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-09-07 08:53 UTC by Martin Renvoize (ashimema)
Modified: 2025-07-21 03:36 UTC (History)
10 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize (ashimema) 2021-09-07 08:53:34 UTC
Some plugins with hooks into the UI outside of the tools and plugin management page require the overall 'plugins' permission to run.

This however has the side effect of allowing users with those permissions to also manage plugins which may not be the effect wanted.

Example: A site has both an Online Payments plugin and the CLA Permissions Check plugin installed.  To use the button from the catalogueing toolbar for the CLA plugin you are required to have the overall plugins permission. This then exposes the login credentials for the Online payments plugin as the user gets access to the tools pages.
Comment 1 Katrin Fischer 2023-07-01 13:37:15 UTC
Do we need to fix the permissions in Koha or in the plugins?
Comment 2 Martin Renvoize (ashimema) 2023-07-03 11:53:57 UTC
In the case of the CLA plugin, we add a button that triggers a call to `plugins/run.pl?class=[% CLASS %]&method=...`

Anything that follows that pattern to trigger a plugin method to run, will require some more involved permissions.

In the CLA case, I think I'd like to generally modernise the approach so we don't need to call the run method.. so I'm not sure what wider effects this may have on other plugins.
Comment 3 Fridolin Somers 2023-12-19 11:23:48 UTC
I run into the same problem.

Indeed the permissions are on the plugin method :
https://git.koha-community.org/Koha-community/Koha/src/commit/ac02c1d2adc883cc6ff6b749c271cb165d337436/plugins/run.pl#L40

But currently only exists tool and report
and plugins create there own methods.

Should we change for only one subpermission "run plugin" ?
And plugins have to implement permissions.

Best regards,
Comment 4 David Cook 2025-07-21 02:58:15 UTC
So I was just about to report this as a security vulnerability... 

My perspective is if you have top-level "plugins" permissions, you can run any method in a plugin, and that's bad.
Comment 5 David Cook 2025-07-21 03:06:22 UTC
(In reply to Fridolin Somers from comment #3)
> Indeed the permissions are on the plugin method :
> https://git.koha-community.org/Koha-community/Koha/src/commit/
> ac02c1d2adc883cc6ff6b749c271cb165d337436/plugins/run.pl#L40
> 
> But currently only exists tool and report
> and plugins create there own methods.

These days, there are permissions for these methods: 
"report", "tool", "admin", or "configure"
 
> Should we change for only one subpermission "run plugin" ?
> And plugins have to implement permissions.

That could be difficult to apply retroactively, but might not be a bad idea. 

I've been thinking a bit about "permission policies" which could be applied at different levels (like the borrower as user/subject or the plugin as object/resource). It could be an opportunity to experiment with the idea. The idea being Koha core would have the code like "Koha::Authz::authorize($user,$action,$resource). 

I haven't fully thought out the design, but the idea would be that there's a permission policy attached to that Koha plugin $resource and it says whether a particular user or categorycode (or whatever) is allowed to do $action against it. 

That "run plugin" could be the minimum level of permission that a user needs in order to run "run.pl" at all, and then permission policies could kick in from there.

For backwards compatibility, we'd probably need to let "report", "tool", "admin" have "run_plugin". Not sure about "configure".
Comment 6 David Cook 2025-07-21 03:08:47 UTC
(In reply to Martin Renvoize (ashimema) from comment #0)
> Some plugins with hooks into the UI outside of the tools and plugin
> management page require the overall 'plugins' permission to run.
> 
> This however has the side effect of allowing users with those permissions to
> also manage plugins which may not be the effect wanted.
> 
> Example: A site has both an Online Payments plugin and the CLA Permissions
> Check plugin installed.  To use the button from the catalogueing toolbar for
> the CLA plugin you are required to have the overall plugins permission. This
> then exposes the login credentials for the Online payments plugin as the
> user gets access to the tools pages.

Ah right so "configure" needs to be separate from "run" for sure. But I think "report", "tool", and "admin" might all be synonymous with "run"? 

Although I'm guessing that people have used them to limit people to plugins in particular areas of Koha...

Tricky.
Comment 7 David Cook 2025-07-21 03:12:50 UTC
(In reply to Fridolin Somers from comment #3)
> Should we change for only one subpermission "run plugin" ?
> And plugins have to implement permissions.

Thinking about this logically... what are we actually trying to accomplish?

"Run plugin" sounds good to a developer, but it doesn't really make sense in terms of the user story. 

If a person is using a reports plugin, what they really need are reports permissions. If they're using a tools plugin, they need tools permissions. Admin plugin - admin permissions.

I'm not familiar with the "Online Payments plugin" or the "CLA Permissions Check plugin", but it sounds like they exist beyond this existing framework. 

And it does sound like they really need to handle their own permissions.
Comment 8 David Cook 2025-07-21 03:25:26 UTC
Ok, I have an idea germinating. 

We change ./plugins/run.pl so that flagsrequired => { catalogue => 1 }. This means that anyone with staff interface access can execute this staff interface Perl script.

We wrap Koha::Plugins::Handler->run() with a "C4::Auth::haspermission()" check. We take care of our existing known use cases of "report", "tool", "admin", and "configure" which are pre-existing "plugins" subpermissions.

If a method is not "report", "tool", "admin", or "configure", then we need to do something else. This is the use case mentioned by Martin here. And historically these users have required full plugins permissions to run the plugin.

So some options coming to mind:

1. We interpret any other method as requiring a "report", "tool", or "admin" permission. In some ways it would make things more secure (ie these people wouldn't need full/configure permissions in order to run the plugins), but in other ways it would be less secure because now anyone could run weird plugin methods if they have any of the 'report', 'tool', or 'admin'. Not very optimal...

2. We add a new plugin class method called "koha_authz". If $class->can('koha_authz'), we try to delegate authorization to the plugin. If it doesn't have that class method, then we fallback to the status quo of requiring full "plugins" permissions? Yet, this is still a problem as I noted before. People with full plugins permission have access to too many plugin methods...

--

We're stuck in a tricky place. If we want to fix the security, we might have to break some plugins.
Comment 9 David Cook 2025-07-21 03:31:38 UTC
(In reply to David Cook from comment #8)
> We're stuck in a tricky place. If we want to fix the security, we might have
> to break some plugins.

Or... 

We add a system preference. Something like "PluginsStrictPermissions", which requires run.pl to only work with methods "report", "tool", "admin", or "configure". 

Most plugins should be fitting in this paradigm.

For plugins that aren't... they have to disable "PluginsStrictPermissions" and there they read a warning saying they need to either align their permissions with "report", "tool", "admin", or "configure" OR use this new "koha_authz" Koha plugin class method so that they handle permissions on their own. 

Maybe we start with "ExperimentalPluginsStrictPermissions" and default it to "off" or "warn" where "warn" would generates some warnings (maybe just in the logs) saying that the plugin is using an unconventional method for run.pl.

And then in a release or two we change "ExperimentalPluginsStrictPermissions" to "PluginsStrictPermissions" and default it to "on" so people are forced to turn it off or update their plugins.
Comment 10 David Cook 2025-07-21 03:36:35 UTC
Lastly... my one concern about letting plugins manage their own permissions is that I suspect that they just will allow anyone to do anything.

I don't think I've ever reviewed a Koha Plugin that didn't have some kind of security vulnerability in it. Although usually it's SQL injection.