-
Notifications
You must be signed in to change notification settings - Fork 85
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
Activity Stream doesn't seem to respect advanced permissions #1057
Comments
Had a similar issue, try to logout and login again (even the other users have to). This config option comes with the following limitations:
|
This comment has been minimized.
This comment has been minimized.
I'll see the same behaviour. Activity logs are seen by users not having access to those files. Using advanced permissions in my case. |
To clarify we are not using the advanced permissions. So it seems this is an issue with the advanced permissions settings. |
we got the same issue via "advanced permissions". All users receive the notification E-Mail even if the got no access to a folder in a group folder. hope this will be implemented soon. |
I have the same issue. Our users receive activity notifications even if they haven't access to a file or folder. |
It looks like this has the potential to leak sensitive data. I will therefore mark this as security issue and raise the priority. |
Can confirm this too. Tested with nextcloud 20.0.11 and groupfolders 8.2.2 |
@icewind1991 @juliushaertl This seems to be a pretty serious thing from my point of view. Any chances that this will be handled with high priority or any other devs that should be notified? |
@fschrempf @icewind1991 @juliushaertl is there any new information about this ? |
@nextcloud/security |
I had a closer look at the code and the obvious problem is that we use the filesystem mounts to get the users which should be notified. This approach has already been disabled by default because of other issues (see nextcloud/activity#190) and it looks more like a hack than a proper solution. Without an interface to query the permissions from the groupfolders app or implementing a hook to get the users that have at least read permissions, there's currently no easy way to fix this. If anyone could help with a rough idea of a proper solution I could try to implement it at some point. Currently I don't really have an idea how it should look like. |
Is it at least possible the add this fact to documentation? |
This extends the warning about activities in groufolders to mention the potential of leaking sensitive data if "Advanced Permissions" are used. See nextcloud/groupfolders#1057 for more information.
This extends the warning about activities in groufolders to mention the potential of leaking sensitive data if "Advanced Permissions" are used. See nextcloud/groupfolders#1057 for more information. Signed-off-by: Frieder Schrempf <frieder@fris.de>
Indeed, here is a PR for extending the documentation: nextcloud/documentation#8047. |
There is a warning in the docs now: https://docs.nextcloud.com/server/latest/admin_manual/configuration_server/activity_configuration.html#activities-in-groupfolders. |
IMO This is a serious issue, and the documentation is too late b/c many won't notice it until too late. IMO this should not be a featured app, at all, until this glaring security issue is addressed. |
The security issue only exists if you enable |
Wishful would be the possibility to configure some knobs from the admin side settings, which subactivites users/groups are allowed to see and not. Users should only see their own activites and commonly shared files/docs. Is that possible by just disabling ACL or the above mentioned mountpoints or do I need to manually modify some config/php file? I would personally prefer having the possibility to disable some of the options permanently since having too many options to click on will just be confusing and unnecessary for the simple user that just need very basic information. |
This can be closed since nextcloud/activity#992 |
Thanks @ArtificialOwl ArtificialOwl for pointing to the merge request. Do I understand correctly that as of now (Nextcloud 25.x) the Activity app does not leak sensitive Information anymore when the configuration flag activity_use_cached_mountpoints=true is set? Point 2. and 3. still need to be resolve right..? |
Sorry, but I think that it still is not working. Environment: |
@ingetics Can you provide more details so I can try to reproduce the issue locally ? |
Yes, of course. In fact, we have desastivacted the advanced permissions for this reason. Folder A with child B and C User Group BB BB and CC read permissions on Folder A If I make a change in Folder C, the people that is inside of the group BB receive a notification. |
I initially had the same observation that it was not working. Users with no access to specific files according to the advanced permissions still saw notifications for these files. This seems to suggest that it actually should be set to 'false'. Since I changed the config "activity_use_cached_mountpoints" from true to false the behavior seems to work as expected. I did change the configuration a few days ago in a production system and did several reviews of the database table oc_activity to look for unexpected usernames in the 'affecteduser' column. So far it seems to work as expected, no activity logs are created for users without access to certain files. |
No, this line checks if Effectively this means. that only if Otherwise there are no activities for users from groupfolders at all. At least that's the theory. |
Upgrading to NC26.x also seems to have resolved the issue for us. |
With activity_use_cached_mountpoints -> true the Activity stream shows a lot of changes/uploads/etc. for files the user doesn't have permission to see, as those permissions are denied via "advanced permissions".
Could it be, that those are not correctly respected?
The text was updated successfully, but these errors were encountered: