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

allow non-admins to list their own groupfolders #2327

Merged
merged 2 commits into from Apr 19, 2023
Merged

Conversation

icewind1991
Copy link
Member

Allow regular users access to

  • GET apps/groupfolders/folders
  • GET apps/groupfolders/folders/$folderId

They only see groupfolders they have access to and only the permissions for groups they belong to

@juliushaertl
Copy link
Member

juliushaertl commented Apr 13, 2023

Should we somehow also limit the listing for the admin for the cases where this is used in the clients or files app? Otherwise for admins and delegated admins it would list all folders, also those that are not accessible through the filesystem just for group folder management.

https://github.com/nextcloud/groupfolders/pull/2326/files seems much cleaner for that approach, but as I understood clients would prefer ocs, right? Maybe we can just have separate endpoints for the user scoped list.

@tobiasKaminsky
Copy link
Member

Yes, ocs is better than webdav in this case, as we only want to use these folders as shortcuts to our regular webdav.
Having a separate webdav path would require way much work.

Good point with admin, so a separate endpoint is indeed better.

@AndyScherzinger
Copy link
Member

Would we want/need brute force / throttling on them ? cc @nickvergessen since we lately discussed this and touch endpoints here.

@nickvergessen
Copy link
Member

I don't have troubles with listing them. Listing does not DDoS/load other users, so that should be fine.

Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good codewise, not sure about Julius remark.

Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991
Copy link
Member Author

Added an option to filter the listing response for admins by adding applicable=1 to the request params.

Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991 icewind1991 merged commit 423fffb into master Apr 19, 2023
18 checks passed
@icewind1991 icewind1991 deleted the non-admin-api branch April 19, 2023 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Items that need to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants