ImpressCMS / impresscms

A multilingual, extensible, community oriented CMS developed in PHP
https://www.impresscms.org
Other
27 stars 35 forks source link

PHP Fatal error: Uncaught Error: Call to a member function getGroups() on nul #1226

Closed MekDrop closed 2 years ago

MekDrop commented 2 years ago

See CI tests logs for ImpressCMS/system-module#10

MekDrop commented 2 years ago

It seems this error happens because when updating a module with blocks, new blocks are created based on user groups from the currently logged-in user. I think the current user should not be involved in such automated decisions. But I'm not sure what groups to use in that case.

MekDrop commented 2 years ago

Problem code place -> https://github.com/ImpressCMS/impresscms/blob/cad7b22f6016b854148a4657fea38dd50d3778d1/core/Extensions/SetupSteps/Module/Update/BlocksSetupStep.php#L141

Xoops this solves in this way -> https://github.com/XOOPS/XoopsCore25/blob/dfc2ef634c69db8ff337098021faa2c822aed547/htdocs/modules/system/admin/modulesadmin/modulesadmin.php#L1013 (~6 lines below) I'm not sure if that is also a correct way here.

skenow commented 2 years ago

In other instances, we do use the current user's permissions to determine admin actions - like what modules they can access in admin and what groups they can modify. Is this the problem? Not being able to get the groups of a user object is what seems to be triggering the error. If the module was installed by another user with different privileges over groups, there could be conflicts with the current user who doesn't have the same privileges over groups.

MekDrop commented 2 years ago

Probably main issue with our current way is that when installing unattended there is no logged-in user. So is not possible to get groups.

But yeah how to deal with modules updates I think is valid also issue. I think we need to rethink this somehow that such issues wouldn't arise.

skenow commented 2 years ago

Ahh - I didn't know this came from an unattended install and thought it was an interactive session. How was it working before with Softaculous?

skenow commented 2 years ago

I remember - This is why we didn't use the core user object during our current install process - there can't be a user until after we install and create one.

MekDrop commented 2 years ago

I found this error when I tried to install or update a module from command line. This is needed for https://github.com/impresscms-dev/test-impresscms-addon-action CI action for easier to know when new things or some fixes in ImpressCMS breaks compatibility with modules.

Probably if not such case I wouldn't find this issue... at least not now...

fiammybe commented 2 years ago

Good question about softaculous. I suspect they create the admin user from the start, like we do in a certain page of the setup procedure

skenow commented 2 years ago

In an interactive install, and even for Softaculous, we do create a user, but the core permissions is overwritten by install classes and methods.

github-actions[bot] commented 2 years ago

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or assign sombody or this will be closed in 5 days.