> On March 22, 2017, 10:08 a.m., Elvis Angelaccio wrote: > > src/ioslaves/file/kauth/file.actions, lines 1-5 > > <https://git.reviewboard.kde.org/r/129983/diff/5/?file=492986#file492986line1> > > > > I don't see the advantage of using a single kauth action. This way you > > are generating only one "generic" polkit action that you can either enable > > or disable. Instead if we used one polkit action per operation (del, rmdir, > > etc.) we could allow a more fine-grained control. For example, a sysadmin > > could decide that the users can create files in write protected locations, > > but they cannot delete existing files. > > Chinmoy Ranjan Pradhan wrote: > Some operation may use more than one polkit action like Delete. If we use > one polkit action per operation then we will have to provide credentials for > every polkit action that operation might use. Though we can add necessary > code for this but it will only complicate the matter. > > > we could allow a more fine-grained control > > Does this involve editing the policy file directly or just writing a > config file? In later case we can provide control over execution of actions > within the ioslave. 'execWithRoot' can perform a check prior to execution of > the polkit action. > > Elvis Angelaccio wrote: > > Some operation may use more than one polkit action like Delete. If we > use one polkit action per operation then we will have to provide credentials > for every polkit action that operation might use. > > But only if the actions are different, right? Otherwise we should be fine > within the 5 minutes threshold after the first authentication. Can you show > some concrete examples where this issue would happen? > > > Does this involve editing the policy file directly or just writing a > config file? > > Have a look at polkit rules. For example, one could write a rule that > says "org.kde.kio.file.copy is allowed, org.kde.kio.file.del is not." and it > would be very cool if we could actually support this. > > Chinmoy Ranjan Pradhan wrote: > >But only if the actions are different, right? Otherwise we should be > fine within the 5 minutes threshold after the first authentication. Can you > show some concrete examples where this issue would happen? > > Delteing a non-empty directory > In this operation 'del' and 'rmdir' actions are used. In this case we > will have to authenticate for both the actions. And after the first > authentication, doing a similar operation will show the warning dialog twice. > > Copy operation can also be considered. > This operation will consist of six actions which are file_open, sendfile, > read, write, file_close, chown. Although all the actions will not be used at > once usage of atleast two can be expected. Here also we will have to > authenticate ouself atleast twice just for copying a single file which is > very annoying. > > >it would be very cool if we could actually support this. > > Indeed its very cool and useful,but still, we will have all these > problems. > > One possible solution can be using the **annotate** tag. Polkit doc > mentions its **used for annotating an action with a key/value pair**. If the > _value_ is **org.freedesktop.policykit.imply** then **the subject if > authorized for an action with this annotation is also authorized for any > action specified by the annotation**. > So what I did is added this line ```<annotate > key="org.freedesktop.policykit.imply">org.kde.kio.file.rmdir</annotate>``` > after the defaults tag of _del_ action. After this when I tried deleting a > non-empty folder I only got one auth dialog. One more thing I noticed is, > with this line in place, when I deleted a single file deleting an empty > folder afterwards only showed one warning even though the action was > different and was executing for this first time. > > I'm still not sure about this so I might be completely wrong as well. If > you have any knowledge of this please let me know if I got everything or > anything right. And if what I have mentioned is indeed correct then we will > have to add the support for _annotate_ tag to kauth policy generator. > > Elvis Angelaccio wrote: > The *imply* annotation sounds very promising, might be exactly what we > are looking for. Unfortunately I don't know more details about it, at the > moment. > > Another solution (possibly simpler) could be a middle ground between > "only one generic action" and "one action per operation". For example, we > could have a `org.kde.kio.file.delete` action that handles everything about > deletion: single file, empty folder, non-empty folder, etc.
Merging actions sounds good. I have updated the patch as per your suggestion. We can merge some other action as well, like, file_open, sendfile, read, write, file_close --> to _copy_ symlink, put, mkdir ---> to _create__**_something_** chown, chmod ---> to _change_something_ or perhaps we can leave them what do you say? > Another solution (possibly simpler) could be a middle ground between "only > one generic action" and "one action per operation". It is only a part of solution. Say, if a user wants to perform a privileged copy after a privileged delete then he must be shown a warning instead of authentication dialog no? For this to happen we need some way to authorize the copy action without any interaction as soon as delete is authorized and I now strongly believe _imply_ annotation is what we need. Maybe its meant for this very purpose only. The use case mentioned in the doc - **A typical use of this annotation is when defining an UI shell with a single lock button that should unlock multiple actions from distinct mechanisms**. - Chinmoy Ranjan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129983/#review102932 ----------------------------------------------------------- On March 14, 2017, 2:48 p.m., Chinmoy Ranjan Pradhan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/129983/ > ----------------------------------------------------------- > > (Updated March 14, 2017, 2:48 p.m.) > > > Review request for KDE Frameworks, David Faure and Elvis Angelaccio. > > > Repository: kio > > > Description > ------- > > This is regarding the GSOC idea > https://community.kde.org/GSoC/2017/Ideas#Project:_Polkit_support_in_KIO. > > This patch intends to demonstrate one possible approach to provide polkit > support in kio. Here its only for the delete operation. This is based on the > patch in task https://phabricator.kde.org/T5070. > > The approach is as follows; > 1. Whenever file ioslave gets access denied error it calls the method > *execWithRoot* with the action that requires priviledge, the path of items > upon which action needs to be performed and a warning ID as arguments. > 2. *execWithRoot* then executes the KAuth::Action *org.kde.kio.file.execute*. > 3. This Kauth::Action has its Persistence set too 'session'. This means that > after authentication the restrictions are dropped for a while, for > about 5 minutes. This is similar to the behaviour of sudo command. > 4. During this time we can perform any action as a privileged user without > any authentication. So to prevent any mishap i added a warning box which > would popup before performing any action(only during this period). > 5. After the said time interval the root privileges are droped and calling > *execWithRoot* should show the usual authentication dialog. > > > Diffs > ----- > > src/ioslaves/file/CMakeLists.txt b9132ce > src/ioslaves/file/file.h 109ea80 > src/ioslaves/file/file.cpp eaf6c88 > src/ioslaves/file/file_unix.cpp 82eb11a > src/ioslaves/file/kauth/CMakeLists.txt PRE-CREATION > src/ioslaves/file/kauth/file.actions PRE-CREATION > src/ioslaves/file/kauth/helper.h PRE-CREATION > src/ioslaves/file/kauth/helper.cpp PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/129983/diff/ > > > Testing > ------- > > > File Attachments > ---------------- > > warning dialog > > https://git.reviewboard.kde.org/media/uploaded/files/2017/03/09/d42570e8-aedf-4c02-801e-362a68755c2c__polkit_integration.png > > > Thanks, > > Chinmoy Ranjan Pradhan > >
