@sgiehl
Description:
Our process for performing updates is currently designed in a way that allows anyone (even without permission) to trigger the database updates (once the files were updated).
This can currently lead to errors or differing results depending on the current users permissions.
I saw this basically for this update script where I saw differing results:
https://github.com/matomo-org/matomo/blob/4fe950ae94b0fe3bd13f928544ff876275fe6733/core/Updates/4.6.0-b4.php#L54-L58
Initializing a segment using new Segment tries to perform an API request to API.getSegmentsMetadata in the background. Based on the current user this might return either the segments (user with view permission) or an error (user without permission). This causes the resulting list of generated queries to be different depending on the user.
In that case this might not be that problematic, but there could be cases where it could even break something.
Therefor it's imho required to perform the migration scripts as superuser always.
Review
- [ ] Functional review done
- [ ] Potential edge cases thought about (behavior of the code with strange input, with strange internal state or possible interactions with other Matomo subsystems)
- [ ] Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
- [ ] Security review done
- [ ] Wording review done
- [ ] Code review done
- [ ] Tests were added if useful/possible
- [ ] Reviewed for breaking changes
- [ ] Developer changelog updated if needed
- [ ] Documentation added if needed
- [ ] Existing documentation updated if needed
@tsteur moved setting the super user permission to the parts in Updater class, where it's needed. Guess that makes more sense, than doing it in a controller