Details
Bug
Major
Description
When a plugin is upgraded, all instances of the plugin on other companies are also upgraded since they share database tables. New permissions however are only added for the instance upgraded. This should be updated to add the permission for all plugin instances.
Activity
I'm curious your thoughts on this Abdy. Tyson seemed to think that the burden should be on the plugins to ensure that all installation correctly added permission. Personally I think that the plugin manager could be updated to make this easier for them. Regardless, we have a lot of missing plugin permissions. I'd like to see an upgrade in the core the searches for these missing permissions and adds them.
Jonathan Reissmueller I see that the plugins have the permissions inside upgrade() and install(), maybe we could save the permissions inside a function in the plugin class? This way the core would have access to this function and the core would be in charge of verifying and adding the permissions of the plugins to all the companies, this way the plugin would not have to add the permissions in upgrade() or install() and the core would be in charge of managing the permissions of the plugins.
<?php
class OrderPlugin extends Plugin
{
public function permissions($plugin_id) {
$permissions = [];
$group = $this->Permissions->getGroupByAlias('admin_packages');
$permissions[] = [
'plugin_id' => $plugin_id,
'group_id' => $group->id,
'name' => Language::_('OrderPlugin.admin_forms.name', true),
'alias' => 'order.admin_forms',
'action' => '*'
];
// ...
return $permissions;
}
}
However, this would mean updating all the plugins to add this new function, and I don't know to what extent it could break the backwards compatibility
Backward incompatibility? You're speaking of adding a method to the Plugin abstract class that accept permission parameters and ensures the are added for all instances of a plugin, correct? Simply adding a protected method would not make it backward incompatible. You are correct in saying that we would need to update plugins to use this new method though.
This new method will be similar to install() or upgrade() it will only receive plugin_id as the parameter and will return an array of all the plugin permissions, the plugin manager should be updated as well, as will need to add the permissions during installation and check what new permissions should be added on upgrade as well for all companies.
I see. Well in that case I don't think there is any need to make this backward incompatible. It is simple enough to create a default method on the Plugin class that returns an empty array. Then all plugins that don't implement the method will continue to function in the current fashion.
Is there a need for the $plugin_id parameter? It is just getting a list of permissions, yes?
We'll also probably want to remove the permissions on uninstall. Upgrade should check permissions on every instance of the plugin, while install and uninstall should only effect the particular plugin the action is being taken on.
Do we have what we need to get started on this? I'd love to get 4.9.1 out by the end of the week.
I think we can remove the $plugin_id parameter as the method will only return a list of permissions, I see it possible to have it ready by the end of the week. 
I've got some time this afternoon so I'll go ahead and pick this one up and let you focus on filtering
The only backward compatible integration I see here is to assume the plugin will set permissions for all copies of the plugin during upgrade. So the solution would be for each plugin to implement any permissions across all installed plugins. The plugin system would have to be refactored to change the way that plugins handle multiple instances of a single copy and that would require a major version change to accomplish, and likely updates to all plugins.