fix: Add all rights to cron and manage session values for CronTask
| Q | A |
|---|---|
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | !33948 |
The ticket in question reported a problem with the sending of SLA reminders.
The notification was not being generated and fatal errors were interrupting the notification generation mechanism due to undefined variables (glpiname and glpigroups in this case).
Access rights to GLPI objects were blocking the notification filter system, which was unable to find the objects due to a lack of rights.
Maybe time to revisit #14652 to design a more controlled way to handle when we need to selectively ignore permission checks/have more than the user's current permissions.
The method of changing the rights in the session is sketchy because if the crontask is running in the user's session, you could end up with the session being saved with the changed permissions (if the script exits early).
If so, would it be safe to use
session_write_close()at the start offront/cron.phpto prevent this ?
Probably. Although, I don't think it makes sense that the user's session would have been opened to begin with or that a new session would be created.
When running cron in
glpimode (where the cron is called using an ajax call tofront/cron.php), isn't there a risk that if a fatal error occur the user is stuck with the wrong session data as it would not be restored ?If so, would it be safe to use
session_write_close()at the start offront/cron.phpto prevent this ?
The solution is to call ini_set('session.use_cookies', 0); to make sure that the response does not contain any session cookie, and then reinit the session to make sure you use a clean session. However, this kind of "hack" should probably be done at the controller level, not in the Crontask::lauch() method itself.
Maybe time to revisit https://github.com/glpi-project/glpi/pull/14652 to design a more controlled way to handle when we need to selectively ignore permission checks/have more than the user's current permissions
for the current need (and issue in !34188), I may be wrong, but I think I'm ok with the current changes.
Anyway, BEFORE the release of 11.0, we need a solution for the workflow plugin @cedric-anne @cconard96 . It's not urgent and can be managed later, but please keep that on your to-do list. We need to work on this plugin later and any barrier should be down before the release.