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

Make trashbin:cleanup clean all users an explicit option not the default #10001

Closed
liamdennehy opened this issue Jun 26, 2018 · 4 comments
Closed

Comments

@liamdennehy
Copy link
Contributor

liamdennehy commented Jun 26, 2018

Steps to reproduce

  1. php ./occ trashbin:cleanup <user> cleans one user
  2. php ./occ trashbin:cleanup cleans all users

Expected behaviour

  • Not specifying a user should request which users to clean, or show help, as with other commands

Actual behaviour

  • Failing to specify a user cleans all trash for all users, making file recovery impossible
  • This is easy to accidentally invoke, especially when probing for usage as other commands display, risking data loss

Server configuration

Core component "files" on NC13.0.4

Operating system:
All
Web server:
All
Database:
All
PHP version:
All
Nextcloud version: (see Nextcloud admin page)
13.0.4

Comments

The default behaviour (the fall-through) is more dangerous than the specific case. e.g. user:disable without parameters doesn't disable all users, but throws an exception. The trashbin:cleanup command should also fall through safely without parameters.

  • Replace the default "no uid specified" case with an exception
  • Only invoke action on all users with trashbin:cleanup --all(-users).
@liamdennehy
Copy link
Contributor Author

I will take a crack at making a PR myself, but I've never worked on a production-grade PHP project so will likely be very poor quality

@nextcloud-bot nextcloud-bot added the stale Ticket or PR with no recent activity label Jun 26, 2018
@nextcloud-bot
Copy link
Member

GitMate.io thinks possibly related issues are #4909 (make changes to users.), #5165 (make logrotation active by default), #9848 (Customize default for the resharing option), #8836 (Where is my trashbin?), and #3173 (corruption cleanup?).

@nextcloud-bot nextcloud-bot added enhancement stale Ticket or PR with no recent activity and removed stale Ticket or PR with no recent activity labels Jun 26, 2018
@MorrisJobke
Copy link
Member

I will take a crack at making a PR myself, but I've never worked on a production-grade PHP project so will likely be very poor quality

Don't worry - we can assist once you have questions. Feel free to join IRC at freenode: #nextcloud-dev

@rullzer
Copy link
Member

rullzer commented Jun 29, 2018

Fixed by #10041

@rullzer rullzer closed this as completed Jun 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants