Prevent creating users with existing files

[nickvergessen]

Signed-off-by: Joas Schilling [email protected]

[rullzer]

🙈

[rullzer]

tests do boom

[nickvergessen]

I fixed our "unit" tests.... arg

[thomas-mc-work]

This change prevented us from a migration of files: We have provided only the files from an old Nextcloud instance into the data directory of a new one. With Nextcloud 17 this scenario was fine. The user could log in and see his files. Now they get an error 403 without any further info. I couldn't even find something about that in the log file.

1. A meaningful log entry would be very useful to avoid hours of research for other admins in that situation
2. Why has that been changed at all? This PR isn't linked to an issue.

[nickvergessen]

The link was missing, it is #16196
and is a follow up to #14652 which is the result of a reported security issue: https://nextcloud.com/security/advisory/?id=NC-SA-2019-015

[thomas-mc-work]

Thank you for your quick reply @nickvergessen!

Actually my on only problem is about line 642:

return !file_exists(rtrim($dataDirectory, '/') . '/' . $uid);

This was only return true; in the first PR. Also this isn't addressed by the mentioned security advisory. So my questions are only focused on that line.

[nickvergessen]

Well before it was hardcoded to what we know. But if an app creates something in root, it could still be overwritten by creating a user.

I don't know how you create your users, but what you can do is:

1. Create the user
2. Move their files in place
3. Run ./occ files:scan <user id> to make the files appear for the user

[thomas-mc-work]

Yes, that's possible. But we've got about 700 Users. And they are provided by an external auth module (user_cas), so Nextcloud isn't aware of them before they log in the first time. Maybe it would have been good to clean the data dir by moving the user accounts into a separate sub folder named users.

But if an app creates something in root, it could still be overwritten by creating a user.

Has this really been an issue to someone? Are apps even encouraged to write into this folder?

[nickvergessen]

Has this really been an issue to someone?

Well it's a security thread so we fix it.

Are apps even encouraged to write into this folder?

Not encouraged, but we also don't prevent it

We will not revert this, in case you accept this security risk you can change your code. But don't come back here when your nextcloud got hacked because of it 😉