Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Per-user webdav directories #142

Closed
AkselMeola opened this issue Feb 12, 2024 · 14 comments · Fixed by #143
Closed

Per-user webdav directories #142

AkselMeola opened this issue Feb 12, 2024 · 14 comments · Fixed by #143
Assignees
Labels
enhancement New feature or request

Comments

@AkselMeola
Copy link

Hey!

Have been playing with the Davis and so far everything is super nice.

As far as I can currently understand the web-dav dir is shared between all users.
Are there any plans to implement/support a per-user directories for web-dav?
(https://sabre.io/dav/per-user-directories/)

Thanks!

@tchapi tchapi linked a pull request Feb 13, 2024 that will close this issue
@tchapi
Copy link
Owner

tchapi commented Feb 13, 2024

Hi @AkselMeola

This could be rather straightforward to have a /home dir but there are some directory / file rights shenanigans to address in my opinion.

Would you be able to try this branch #143 and tell me if everything seems to work for your use case?

@tchapi tchapi added the enhancement New feature or request label Feb 13, 2024
@tchapi tchapi self-assigned this Feb 13, 2024
@AkselMeola
Copy link
Author

Thanks. I'll have it a go.

@AkselMeola
Copy link
Author

So technically it works.
But the file permissions need to be tested further.

For example on the branch the webdav public dir is a parent node of the home node:

 $nodes[] = new \Sabre\DAV\FS\Directory($this->webdavPublicDir);

And this will allow all users to access all the users homes.

It could be somewhat mitigated with a separate public directory like this:

 $nodes[] = new \Sabre\DAV\FS\Directory($this->webdavPublicDir . '/public');

This just a test, it should probably be ENV config to choose between homes, public or both.. or smth. And provide a separate path for public for example.

But here also lies one bug: Any user can rename the "public" node and it will say it had an error, but on the filesystem it still actually renamed the dir. After refresh app has errors due to the missing path, so a disruption of service could be created.

When I tinkered with the Sabre on my own then due to the hideNodesFromListings being false I was able to also rename other user dirs, but that was likely due to some permissions mess up on my part.
Here I was not able to do such shenanigans, which is good, but I have not gone in deep to check what actually restricts it here and validate this is "safe" (..and even if the node is not visible).


TLDR:
I think you can close this for now, since there are probably some bugs in hiding, unless you want to go deep into it.
If I have more time I might tinker with it a bit further.
(I think OC must rely on something similar, so perhaps there is some easy config to fix the public dir rename issue and validate the homes are "safe").

Thanks for looking into it tho 👍

@AkselMeola
Copy link
Author

AkselMeola commented Feb 15, 2024

Ok. Sorry. Wrong alarm.
The public rename was due to the local file permissions. It can be fixed if the public path's parent is read-only.
E.g.:

/users/homes/{user1, user2, ... } (/users/homes +rw) 
/users/public  (/users/ -w)

This in itself is a small trap for admins tho and maybe a plugin or a rule could be made to not allow the rename action on public node.

@tchapi
Copy link
Owner

tchapi commented Feb 25, 2024

Hi @AkselMeola

Not sure I get your example above. Having the home directory at the same level as the calendars or principals seems correct to me. Are you saying we should apply a specific right to WEBDAV_PUBLIC_DIR ?

@tchapi tchapi moved this to In Progress in Davis Roadmap Feb 26, 2024
@de-es
Copy link

de-es commented Mar 2, 2024

Did some testing with this branch because this feature would be useful to me as well.

I set WEBDAV_PUBLIC_DIR=/var/lib/davis. Afterwards the personal home directory was accessible under https://domain.dom/dav/home/<user>/ as exptected. All home dirs were created correctly like /var/lib/davis/<user>/.

But I can also access all users home dirs with https://domain.dom/dav/davis/<user>/

I guess this should not be possible permission wise.

@tchapi
Copy link
Owner

tchapi commented Mar 3, 2024

Hi @de-es

I guess this should not be possible permission wise.

Did you set the correct folder permissions as per @AkselMeola's message up there?

@de-es
Copy link

de-es commented Mar 7, 2024

Sorry, I don't understand how to define a different home base dir other than the public dir. home or homes doesn't seem to be hardcoded, is it? Do I have to alter the code?

@tchapi
Copy link
Owner

tchapi commented Mar 7, 2024

Sorry, I don't understand how to define a different home base dir other than the public dir

I am talking about the permission of the folder on your device, not changing the home (which indeed would need to alter the code)

@AkselMeola
Copy link
Author

AkselMeola commented Mar 20, 2024

@tchapi @de-es

Sorry for my ramblings.

  • The issue with users renaming the public dir can be fixed by not allowing the php/web user not make changes in the parent of public dir. (By default I had set it too wide permissions.)

  • The other issue where users can access other user homes is still relevant since the permission to access the public dir, which contains the 'homes', seems to override the home folder access. To fix this, it requires having a separate homes directory which is not a child of the public dir (or maybe there is some specific ACL that can be implemented, but unlikely).

@tchapi
Copy link
Owner

tchapi commented Mar 20, 2024

I have updated #143 to differentiate the public directory from the homes directory, it should fix your second point if I'm not mistaken!

@tchapi tchapi moved this from In Progress to In Review in Davis Roadmap Mar 20, 2024
@de-es
Copy link

de-es commented Mar 22, 2024

Sorry for the late reply. Separating homes from public did the trick. Works like a charm. www-data is allowed to write in both previous folders, but not in their parent.

@github-project-automation github-project-automation bot moved this from In Review to Done in Davis Roadmap Mar 29, 2024
@tchapi
Copy link
Owner

tchapi commented Mar 29, 2024

👋🏼
I've finally merged #143 — disabling the home directories by default so that the behavior doesn't change from the existing.

Thanks @AkselMeola and @de-es for chiming and helping on this!

@de-es
Copy link

de-es commented Mar 30, 2024

@tchapi Thanks to you for implementing this feature. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants