[#25152] - [4.0] [PoC] Leverage access check form the plugin loader in to the plugin
- Closed
- 8 Dec 2019
- Medium
- Build: 4.0-dev
- Draft PR
- # 25152
- Diff
- HLeithner:remove-access-check-from-plugin-loader
- Pending Hound Hound is busy sniffing around... Details
User tests: Successful: Unsuccessful:
This is a proof of concept to remove the "always session creation" because of loading plugins incl. full b/c.
I know that this could be done better, this is only to get some feedback.
Summary of Changes
- Removed the access level check from the plugin load query.
- Add a combat layer for all plugins not being loaded with a service handler
- Only load the user session if the plugin is not public (legacy and new)
- Add support to override the access field in the plugin manager
- Added a helper function in CMSPlugin to check the access level with less overhead
The quickicon joomlaupdate plugin is the only plugin at the moment that uses the service helper. I changed this to support access check on event trigger and disabled the access field as example. (make no sense to do both).
before patch
#0 Joomla\CMS\User\User::getInstance(0) called at [/libraries/src/Access/Access.php:997]
#1 Joomla\CMS\Access\Access::getAuthorisedViewLevels(0) called at [/libraries/src/User/User.php:445]
#2 Joomla\CMS\User\User->getAuthorisedViewLevels() called at [/libraries/src/Plugin/PluginHelper.php:258]
#3 Joomla\CMS\Plugin\PluginHelper::load() called at [/libraries/src/Plugin/PluginHelper.php:180]
#4 Joomla\CMS\Plugin\PluginHelper::importPlugin(system) called at [/libraries/src/Application/CMSApplication.php:229]
#5 Joomla\CMS\Application\CMSApplication->execute() called at [/includes/app.php:63]
#6 require_once(/includes/app.php) called at [/index.php:36]
after patch
#0 Joomla\CMS\User\User::getInstance(0) called at [/libraries/src/Access/Access.php:997]
#1 Joomla\CMS\Access\Access::getAuthorisedViewLevels(0) called at [/libraries/src/User/User.php:445]
#2 Joomla\CMS\User\User->getAuthorisedViewLevels() called at [/libraries/src/Menu/AbstractMenu.php:322]
#3 Joomla\CMS\Menu\AbstractMenu->authorise(101) called at [/libraries/src/Application/SiteApplication.php:97]
#4 Joomla\CMS\Application\SiteApplication->authorise(101) called at [/libraries/src/Application/SiteApplication.php:771]
#5 Joomla\CMS\Application\SiteApplication->route() called at [/libraries/src/Application/SiteApplication.php:235]
#6 Joomla\CMS\Application\SiteApplication->doExecute() called at [/libraries/src/Application/CMSApplication.php:241]
#7 Joomla\CMS\Application\CMSApplication->execute() called at [/includes/app.php:63]
#8 require_once(/includes/app.php) called at [/index.php:36]
This mean we are not finished decoupling the cms from the user session, we have to do something similar to the menu. The simplest is to check for public access before loading the user.
Documentation Changes Required
Plugin authors has to do the access check on each event listener
if (!$this->accessCheck())
{
return;
}If the plugin doesn't need support for access levels it can override the access field with a hidden field in the config file.
<config>
<field name="access" type="hidden" />
</config>| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration com_plugins Libraries Front End Plugins |
franz-wohlkoenig
- | Title |
|
||||||
| Labels |
Added:
?
?
|
||
The parameter I used was not the public access level it was the "default" access level which is used when creating new items.
Are you saying that you are using the value of "default" access level in global configuration to determine if an item is public?
The parameter I used was not the public access level it was the "default" access level which is used when creating new items.
Are you saying that you are using the value of "default" access level in global configuration to determine if an item is public?
Yes that was my fault assuming that a config option that class "access" and the title "default access level" could be public... but @bembelimen corrected me that this that only the default value for newly created (content?) items.
Thats the reason for the last commit going to use the hardcoded value "1" instead of the config option.
If you are referring to an access level with an id of 1 (not at a pc right now) then that wont work. There is nothing to say that a site has an access level with an id of 1
There is nothing to say that a site has an access level with an id of 11
Not sure what you mean with this.
But if you mean that public is not id 1 then you are wrong, as already explained in #25152 (comment) Joomla always add the access level with the id 1 to the user access level that means every user and non user has the access level which is per definition public.
It's also not possible to delete this access level, if you manage to deleted it your site will no longer work because the login doesn't work any longer and without login you can't see anything because everything that has a access level that should be seen by the public has to be 1.
Sorry i was unclear. There is nothing to say that the access level with id =1 is in use on the site. It is perfectly possible to build a site that doesn't use the access level that has an id of 1
Sorry i was unclear. There is nothing to say that the access level with id =1 is in use on the site. It is perfectly possible to build a site that doesn't use the access level that has an id of 1
No it's not possible, the value 1 is hard coded in getAuthorisedViewLevels see
https://github.com/joomla/joomla-cms/blob/4.0-dev/libraries/src/Access/Access.php#L994
And even if you get this magically done the code still works because all menu and plugin items will by pass the "short cut" and validate against the user object as it is now.
Well that's a bug then if you are saying that even if you remove any "User Groups Having Viewing Access" from the "access level" public that it still works
technical it's a limitation of the system and it's not too bad in my opinion, at least I can't create a use case that needs a change. But I'm sure it's possible to construct one.
Forcing there to be one access group isn't bad. Hardcoding it to ID 1 is. Even on a closed off system, making arbitrary assumptions that a specific row in a specific table will have a specific identifier is a bad idea.
Forcing there to be one access group isn't bad. Hardcoding it to ID 1 is. Even on a closed off system, making arbitrary assumptions that a specific row in a specific table will have a specific identifier is a bad idea.
Changing this into a configurable value shouldn't be hard because iirc it's only on one position but it will impossible to change it at runtime. If someone wants to implement it I'm happy to look at it, I see no benefit.
No it's not possible, the value 1 is hard coded in getAuthorisedViewLevels see
https://github.com/joomla/joomla-cms/blob/4.0-dev/libraries/src/Access/Access.php#L994
I guess we should be using JConfig::$access here. Assuming it holds the ID of public access level.
I guess we should be using
JConfig::$accesshere. Assuming it holds the ID of public access level.
No it doesn't.
Then what does it do?
It holds the access level new items (e.g. articles) get as default. See #25152 (comment)
This is a massive backwards breaking change that serves almost no benefit at all.
Whilst I understand the benefits of this in terms of there being no active sessions I'm not sure the benefits of this outweigh the cons. I'm going to choose to close this
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2019-12-08 12:31:27 |
| Closed_By | ⇒ | wilsonge |
At the moment Benjamin is trying to delete the public access level only with the backend ;-) but I think he is failing.
It's hardcoded in the getAuthorisedViewLevels funtion:
https://github.com/joomla/joomla-cms/blob/4.0-dev/libraries/src/Access/Access.php#L994
The parameter I used was not the public access level it was the "default" access level which is used when creating new items.