feat: allow to configure php.user

[szaimen]

• Needs Revert: "Check datadirectory owner, not config owner." #45302

[come-nc]

I insist that it should use the new config value instead of the config.php owner if the value is filled.
Is if php.user is empty, it checks that running user is the owner of config.php.
If php.user is filled, it checks that running user is the one in php.user.

See my previous change suggestion.

[szaimen]

I insist that it should use the new config value instead of the config.php owner if the value is filled. Is if php.user is empty, it checks that running user is the owner of config.php. If php.user is filled, it checks that running user is the one in php.user.

See my previous change suggestion.

but we already get the user via posix_getuid or not?

[come-nc]

I insist that it should use the new config value instead of the config.php owner if the value is filled. Is if php.user is empty, it checks that running user is the owner of config.php. If php.user is filled, it checks that running user is the one in php.user.
See my previous change suggestion.

but we already get the user via posix_getuid or not?

We do, and we want to check that it’s the correct one.

[szaimen]

I insist that it should use the new config value instead of the config.php owner if the value is filled. Is if php.user is empty, it checks that running user is the owner of config.php. If php.user is filled, it checks that running user is the one in php.user.
See my previous change suggestion.

but we already get the user via posix_getuid or not?

We do, and we want to check that it’s the correct one.

all right, done!

[come-nc]

Not completely happy about it but too much time was spend already, and it does fix the usecase.

[ChristophWurst]

👍 otherwise

[szaimen]

Looks like this is not true? How can I fix this?

[Dreamsorcerer]

Are you sure? Documentation says it should return false on failure: https://www.php.net/manual/en/function.posix-getpwuid.php

[icewind1991]

I think as worded this is misleading, the option doesn't overwrite the check, it changes that the expected value of the check is.

I would go with something like

If the config file is not owned by the user running the webserver you can set the correct user by setting the 'php.user' option in your config.php

[icewind1991]

This check always needs to check for $configuredUser if set. Also accepting $configUser can lead to the very issue this is trying to prevent.

[icewind1991]

something like

$phpUser = $config->getSystemValueString('php.user', '');
if (!$phpUser) {
    $userNameArray = posix_getpwuid($user);
	if ($userNameArray !== false) {
		$phpUser = $userNameArray['name'];
	}
}
if ($user != $phpUser) {

maybe

[icewind1991]

same here

[Dreamsorcerer]

Looks like a reasonable solution, if there's not an easy way to just verify if we can write to the directory (as the user is not really relevant, just whether we have write access). I don't remember my PHP so well, but in Python I'd probably just try/except a file write as a test.

[icewind1991]

as the user is not really relevant, just whether we have write access

The user is actually relevant (there is a separate check for checking write access to the data dir iirc). If an occ/cron command ends up create a file it will be owned by whatever user the command is ran as. So if a user does sudo occ .... Any file created by that will then be read-only to the webserver.

[szaimen]

As I wrote in the internal chat: can someone please take over this PR? It looked like a simple change but got more and more complicated. I fear I currently do not have the capacity to finish this as I am busy with AIO and Enterprise-AIO.