-
Notifications
You must be signed in to change notification settings - Fork 78
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
Implementation of unified privileges checking for CLI commands #1505
Conversation
Drop from `clean` command as it is not transactional and add it to `builddep`.
The functionality is needed by both the library and the dnf5 app.
A single checkpoint was implemented after parsing command-line data but before downloading metadata and processing commands, ensuring unified handling of cases where commands are executed under insufficient privileges.
I have several concerns regarding this pull request: Firstly, I'm not sure that the centralized Secondly, I don't think that having read/write access to |
Hi Marek,
Yeah, I was also thinking about this approach. Just this would require changing each command and making this a prerequisite when implementing a new one. This PR is a simple approach to the things.
I was focused just on the core commands and plugins here. The 3rd party plugins could handle the situation in their way. Anyway, the current PR is really just trying to setup a single place where the initial check of privileges happens. If user doesn't have privileges for the operation, it would be denied anyway even without this PR.
Yes, it is not sufficient for this use case. This specific scenario could be handled later e.g. by the |
Thanks for clarification! Let's go with this simple approach than. |
Using the existing RPM lock file functionality.
There are a hard-coded list of commands that are not allowed to run without superuser privileges. Then
assumeno
anddownloadonly
options are taken into account. Lastly, we check if provided command has defined thestore
option which should be defined for all transactional commands and therefore it should mean a need for elevated privileges.For: #849.
Successor of: #1504.