Check datadirectory owner, not config owner.#27613
Check datadirectory owner, not config owner.#27613skjnldsv merged 2 commits intonextcloud:masterfrom
Conversation
This comment has been minimized.
This comment has been minimized.
|
No, we do not squash, we want to preserve the commits history |
d9a142d to
22ad440
Compare
Signed-off-by: skjnldsv <[email protected]>
Signed-off-by: skjnldsv <[email protected]>
|
Thanks for finishing it off, I'm a bit short on time at the moment, so was just making quick updates in the Github UI. |
|
This breaks e.g. in nextcloud/all-in-one#4542 (comment) |
Why are the users different in this case? The upgrade needs write access to that directory, so seems like it'd make sense to me that it is owned by the same user. Though thinking a little more about that check. Surely it only needs to check that we can write to the directory. I'm not clear why it checked the owner of the file in the first place... |
Having write access to a directory doesn't necessarily mean you need to be the owner. In my case, using NFS, the owner is some UID even not existant on the local www server, but nevertheless www-data gets write access.
Exactly. So, how can I disable that check and get OCC back to work? |
Well, right now, comment out the code. That's what I've had to do for the past 3+ years while waiting for this PR to merge... |
Thanks. Just did so - in both places (cron.php and console.php). Probably a php function like is_writable() would be more appropriate for the task here. |
Changes permission checks to use datadirectory, rather than config file.
Step 1 towards root-owned configs (#11075).
For root-owned configs, the only thing that needs to be writeable is the data directory. For non-root configs, this check shouldn't make any difference, it's very likely the same user is used for both config and data dir.
Currently, the user shouldn't see any difference in behaviour, as the check here would still disallow a root-owned config:
https://github.com/nextcloud/server/blob/master/lib/base.php#L247
Once the rest of the work to support root-owned configs is complete, then this check should be removed (
|| !$configFileWritable && \OCP\Util::needUpgrade()).