subject:"Review Request 129983\: \[RFC\] PoC patch for polkit support in kio."

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

2018-10-27 Thread Chinmoy Ranjan Pradhan

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129983/
---

(Updated Oct. 27, 2018, 4:38 p.m.)

Status
--

This change has been discarded.

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/6/

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

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

2017-04-30 Thread Albert Astals Cid


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129983/#review103165
---




src/ioslaves/file/file.h (line 116)


please move this to the constructor, it is *very* confusing when half the 
variables are intitialized in the constructor and half in the header definition.


- Albert Astals Cid


On March 27, 2017, noon, Chinmoy Ranjan Pradhan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129983/
> ---
> 
> (Updated March 27, 2017, noon)
> 
> 
> 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
> 
>

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

2017-04-29 Thread Anthony Fieroni


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129983/#review103158
---



ping apps 17.04 are out, this ca be useful

- Anthony Fieroni


On Март 27, 2017, 3 след обяд, Chinmoy Ranjan Pradhan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129983/
> ---
> 
> (Updated Март 27, 2017, 3 след обяд)
> 
> 
> 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
> 
>

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

2017-03-30 Thread Chinmoy Ranjan Pradhan



> On March 22, 2017, 10:08 a.m., Elvis Angelaccio wrote:
> > src/ioslaves/file/kauth/file.actions, lines 1-5
> > 
> >
> > 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 ``` key="org.freedesktop.policykit.imply">org.kde.kio.file.rmdir``` 
> 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.
> 
> Chinmoy Ranjan Pradhan wrote:
> 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_
>

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

2017-03-28 Thread Elvis Angelaccio



> On March 22, 2017, 10:08 a.m., Elvis Angelaccio wrote:
> > src/ioslaves/file/kauth/file.actions, lines 1-5
> > 
> >
> > 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 ``` key="org.freedesktop.policykit.imply">org.kde.kio.file.rmdir``` 
> 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.
> 
> Chinmoy Ranjan Pradhan wrote:
> 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_
>

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

2017-03-27 Thread Chinmoy Ranjan Pradhan



> On March 25, 2017, 4:30 p.m., Elvis Angelaccio wrote:
> > src/ioslaves/file/file.cpp, line 1428
> > 
> >
> > I tried to delete a folder (`kioclient5 remove /opt/test/`) but I 
> > didn't get this warning.
> 
> Chinmoy Ranjan Pradhan wrote:
> so was it the warning for file? or auth dialog? or the folder was delted 
> straightaway? In my system I am getting this warning only if the folder is 
> empty.
> 
> Elvis Angelaccio wrote:
> I only got the auth dialog (the folder was not empty).

seems like kioclient5 doesn't acknowledge the threshold time. It works fine 
when using kioslavetest.


- Chinmoy Ranjan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129983/#review102944
---


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
> 
>

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

2017-03-27 Thread Chinmoy Ranjan Pradhan

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129983/
---

(Updated March 27, 2017, noon)

Review request for KDE Frameworks, David Faure and Elvis Angelaccio.

Changes
---

Added a subAction parameter to execWithRoot
Fixed wrong warning when deleting folder

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.

Diffs (updated)
-

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

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

2017-03-27 Thread Chinmoy Ranjan Pradhan



> On March 22, 2017, 10:08 a.m., Elvis Angelaccio wrote:
> > src/ioslaves/file/kauth/file.actions, lines 1-5
> > 
> >
> > 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 ``` key="org.freedesktop.policykit.imply">org.kde.kio.file.rmdir``` 
> 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_**

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

2017-03-26 Thread Elvis Angelaccio



> On March 25, 2017, 4:30 p.m., Elvis Angelaccio wrote:
> > src/ioslaves/file/file.cpp, line 1428
> > 
> >
> > I tried to delete a folder (`kioclient5 remove /opt/test/`) but I 
> > didn't get this warning.
> 
> Chinmoy Ranjan Pradhan wrote:
> so was it the warning for file? or auth dialog? or the folder was delted 
> straightaway? In my system I am getting this warning only if the folder is 
> empty.

I only got the auth dialog (the folder was not empty).


- Elvis


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129983/#review102944
---


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
> 
>

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

2017-03-26 Thread Elvis Angelaccio



> On March 22, 2017, 10:08 a.m., Elvis Angelaccio wrote:
> > src/ioslaves/file/kauth/file.actions, lines 1-5
> > 
> >
> > 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 ``` key="org.freedesktop.policykit.imply">org.kde.kio.file.rmdir``` 
> 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.

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.


- Elvis


---
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

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

2017-03-26 Thread Chinmoy Ranjan Pradhan



> On March 25, 2017, 4:30 p.m., Elvis Angelaccio wrote:
> > src/ioslaves/file/file.cpp, line 1423
> > 
> >
> > `kioclient5 remove /opt/test/ /opt/test2/` shows this warning, while I 
> > was expecting the one about folders.

Its indeed a logic error. Occurs when the folder contains some files.


> On March 25, 2017, 4:30 p.m., Elvis Angelaccio wrote:
> > src/ioslaves/file/file.cpp, line 1428
> > 
> >
> > I tried to delete a folder (`kioclient5 remove /opt/test/`) but I 
> > didn't get this warning.

so was it the warning for file? or auth dialog? or the folder was delted 
straightaway? In my system I am getting this warning only if the folder is 
empty.


- Chinmoy Ranjan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129983/#review102944
---


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
> 
>

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

2017-03-26 Thread Chinmoy Ranjan Pradhan

> On March 22, 2017, 10:08 a.m., Elvis Angelaccio wrote:
> > src/ioslaves/file/kauth/file.actions, lines 1-5
> > 
> >
> > 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.

>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 ```org.kde.kio.file.rmdir``` 
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.

- 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

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

2017-03-25 Thread Elvis Angelaccio



> On March 22, 2017, 10:08 a.m., Elvis Angelaccio wrote:
> > src/ioslaves/file/kauth/file.actions, lines 1-5
> > 
> >
> > 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.

> 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.


- Elvis


---
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
> 
>

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

2017-03-25 Thread Elvis Angelaccio


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129983/#review102944
---




src/ioslaves/file/file.cpp (line 1423)


`kioclient5 remove /opt/test/ /opt/test2/` shows this warning, while I was 
expecting the one about folders.



src/ioslaves/file/file.cpp (line 1428)


I tried to delete a folder (`kioclient5 remove /opt/test/`) but I didn't 
get this warning.


- Elvis Angelaccio


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
> 
>

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

2017-03-22 Thread Chinmoy Ranjan Pradhan



> On March 22, 2017, 10:08 a.m., Elvis Angelaccio wrote:
> > src/ioslaves/file/kauth/file.actions, lines 1-5
> > 
> >
> > 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.

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.


- 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
> 
>

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

2017-03-22 Thread Elvis Angelaccio


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129983/#review102932
---




src/ioslaves/file/kauth/file.actions (lines 1 - 5)


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.


- Elvis Angelaccio


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
> 
>

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

2017-03-16 Thread Elvis Angelaccio



> On March 13, 2017, 12:10 p.m., Elvis Angelaccio wrote:
> > I'm not sure if we should show the warning dialog at the kio_file level. 
> > This would result in a double dialog when deleting files with Shift+Del 
> > from file managers. What about making jobuidelegate.cpp smarter? It could 
> > show a more "scary" message if the file that's being deleted is 
> > write-protected.
> 
> Chinmoy Ranjan Pradhan wrote:
> In my system when i use Shift+del there is no warning dialog (few months 
> back dolphin used to show the dialog but strangly it doesn't shows now) so 
> this case didn't occured to me. 
> 
> If I am not wrong jobuidelegate's warning is shown before the job 
> executes and kauth's authentication dialog is shown only after job executes. 
> But its just the opposite we want. So if there's a way for slave to notify 
> the job of starting of a priviledged action then there should be no problem 
> in making jobuidelegate smarter. 
> (But the thing is at this very moment i can't think of or find any 
> possible way)
> 
> Elvis Angelaccio wrote:
> You probably checked the "don't show again" checkbox ;) The 
> jobuidelegate's dialog is shown by default, so we must assume the user will 
> see it => we need a way to prevent showing the dialog twice. You are right 
> that jobuidelegate is run before kio_file is involved. I was thinking of 
> checking the permissions from jobuidelegate: can you check if QFileInfo would 
> do the trick? (e.g. QFileInfo::isWritable()?)
> 
> Chinmoy Ranjan Pradhan wrote:
> >You probably checked the "don't show again" checkbox ;)
> 
> Completely forgot about that :P
> 
> >we need a way to prevent showing the dialog twice
> 
> I hope you mean the warning dialog because if the user is not authorised 
> then there will be two dialogs.
> 
> >I was thinking of checking the permissions from jobuidelegate: can you 
> check if QFileInfo would do the trick?
> 
> Checking permissions beforehand and then showing the dangerous warning is 
> definitely a good idea. Now QFileInfo::isWritable will work only when 
> deleteJob is started inside a write-protected location. Suppose if its a 
> regular folder inside some regular location but containing a write-protected 
> subfolder then the check's result will be useless. 
> One thing we can do is stat the contents and as soon we find a 
> write-protected location we can abort the stat'ing and show our special 
> warning.

Sounds good. Please make sure to include this point in your proposal ;)


- Elvis


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129983/#review102826
---


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:

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

2017-03-15 Thread Chinmoy Ranjan Pradhan



> On March 15, 2017, 7:36 a.m., David Faure wrote:
> > src/ioslaves/file/kauth/helper.h, line 40
> > 
> >
> > Can you name it mReply for readability?
> > 
> > But in fact I wonder if it should really be a member variable. Can the 
> > Helper be reused for more than one action? In that case the error from one 
> > operation will pollute the next one.
> > 
> > Why not just return an ActionReply from del and rmdir?

The helper is not reused so returning an ActionReply from del and rmdir is just 
fine.


> On March 15, 2017, 7:36 a.m., David Faure wrote:
> > src/ioslaves/file/file.cpp, line 1376
> > 
> >
> > You could have one variant of this method in file_win.cpp which just 
> > returns false and move this code to file_unix.cpp indeed.

But i find using the macro rather neat.


- Chinmoy Ranjan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129983/#review102848
---


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
> 
>

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

2017-03-15 Thread Chinmoy Ranjan Pradhan



> On March 13, 2017, 12:10 p.m., Elvis Angelaccio wrote:
> > I'm not sure if we should show the warning dialog at the kio_file level. 
> > This would result in a double dialog when deleting files with Shift+Del 
> > from file managers. What about making jobuidelegate.cpp smarter? It could 
> > show a more "scary" message if the file that's being deleted is 
> > write-protected.
> 
> Chinmoy Ranjan Pradhan wrote:
> In my system when i use Shift+del there is no warning dialog (few months 
> back dolphin used to show the dialog but strangly it doesn't shows now) so 
> this case didn't occured to me. 
> 
> If I am not wrong jobuidelegate's warning is shown before the job 
> executes and kauth's authentication dialog is shown only after job executes. 
> But its just the opposite we want. So if there's a way for slave to notify 
> the job of starting of a priviledged action then there should be no problem 
> in making jobuidelegate smarter. 
> (But the thing is at this very moment i can't think of or find any 
> possible way)
> 
> Elvis Angelaccio wrote:
> You probably checked the "don't show again" checkbox ;) The 
> jobuidelegate's dialog is shown by default, so we must assume the user will 
> see it => we need a way to prevent showing the dialog twice. You are right 
> that jobuidelegate is run before kio_file is involved. I was thinking of 
> checking the permissions from jobuidelegate: can you check if QFileInfo would 
> do the trick? (e.g. QFileInfo::isWritable()?)

>You probably checked the "don't show again" checkbox ;)

Completely forgot about that :P

>we need a way to prevent showing the dialog twice

I hope you mean the warning dialog because if the user is not authorised then 
there will be two dialogs.

>I was thinking of checking the permissions from jobuidelegate: can you check 
>if QFileInfo would do the trick?

Checking permissions beforehand and then showing the dangerous warning is 
definitely a good idea. Now QFileInfo::isWritable will work only when deleteJob 
is started inside a write-protected location. Suppose if its a regular folder 
inside some regular location but containing a write-protected subfolder then 
the check's result will be useless. 
One thing we can do is stat the contents and as soon we find a write-protected 
location we can abort the stat'ing and show our special warning.


- Chinmoy Ranjan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129983/#review102826
---


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
>   
>

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

2017-03-15 Thread David Faure


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129983/#review102848
---




src/ioslaves/file/file.cpp (line 1376)


You could have one variant of this method in file_win.cpp which just 
returns false and move this code to file_unix.cpp indeed.



src/ioslaves/file/file.cpp (line 1412)


the if() brings nothing



src/ioslaves/file/kauth/helper.h (line 40)


Can you name it mReply for readability?

But in fact I wonder if it should really be a member variable. Can the 
Helper be reused for more than one action? In that case the error from one 
operation will pollute the next one.

Why not just return an ActionReply from del and rmdir?



src/ioslaves/file/kauth/helper.cpp (line 36)


QLatin1String would be sufficient


- David Faure


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
> 
>

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

2017-03-15 Thread David Faure



> On March 12, 2017, 3:49 p.m., David Faure wrote:
> > src/ioslaves/file/CMakeLists.txt, line 37
> > 
> >
> > if (UNIX)
> >add_subdirectory(kauth)
> > endif()
> > 
> > Apparently kauth doesn't exist on Windows.
> 
> Chinmoy Ranjan Pradhan wrote:
> Shall I move all the kauth specific code to file_unix ?

Or in #ifdef Q_OS_UNIX, for the cases where there's shared code in the same 
method.


- David


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129983/#review102823
---


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
> 
>

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

2017-03-14 Thread Elvis Angelaccio



> On March 13, 2017, 12:10 p.m., Elvis Angelaccio wrote:
> > I'm not sure if we should show the warning dialog at the kio_file level. 
> > This would result in a double dialog when deleting files with Shift+Del 
> > from file managers. What about making jobuidelegate.cpp smarter? It could 
> > show a more "scary" message if the file that's being deleted is 
> > write-protected.
> 
> Chinmoy Ranjan Pradhan wrote:
> In my system when i use Shift+del there is no warning dialog (few months 
> back dolphin used to show the dialog but strangly it doesn't shows now) so 
> this case didn't occured to me. 
> 
> If I am not wrong jobuidelegate's warning is shown before the job 
> executes and kauth's authentication dialog is shown only after job executes. 
> But its just the opposite we want. So if there's a way for slave to notify 
> the job of starting of a priviledged action then there should be no problem 
> in making jobuidelegate smarter. 
> (But the thing is at this very moment i can't think of or find any 
> possible way)

You probably checked the "don't show again" checkbox ;) The jobuidelegate's 
dialog is shown by default, so we must assume the user will see it => we need a 
way to prevent showing the dialog twice. You are right that jobuidelegate is 
run before kio_file is involved. I was thinking of checking the permissions 
from jobuidelegate: can you check if QFileInfo would do the trick? (e.g. 
QFileInfo::isWritable()?)


- Elvis


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129983/#review102826
---


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
> 
>

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

2017-03-14 Thread Chinmoy Ranjan Pradhan



> On March 12, 2017, 3:49 p.m., David Faure wrote:
> > src/ioslaves/file/kauth/helper.cpp, line 14
> > 
> >
> > Why is this using a QueuedConnection? This prevents any sort of error 
> > handling because this method returns too early.
> > 
> > Testing that invokeMethod worked is only half the story. If the 
> > QueuedConnection can be removed, you can use a return argument of type bool 
> > for error handling?
> > Or better, a KIO error code, so we can give the exact error message (I 
> > see that we didn't have "directory not empty" for rmdir, but if this is 
> > extended to other actions then it will be useful to have).
> > 
> > In fact, instead of invokeMethod this could be a set of if(method == 
> > "...")? Seems simpler to me, but I have no strong opinion.

I used invokeMethod for compactness but an if-else construct is indeed simpler


> On March 12, 2017, 3:49 p.m., David Faure wrote:
> > src/ioslaves/file/kauth/helper.cpp, line 26
> > 
> >
> > This statement does nothing, the method would return anyway immediately 
> > afterwards. Is there missing error handling here? Shouldn't this return a 
> > bool?

Error handling was supposed to be there but somehow I messed up.


> On March 12, 2017, 3:49 p.m., David Faure wrote:
> > src/ioslaves/file/file.cpp, line 1391
> > 
> >
> > can you explain the meaning of the isRoot member to me? It sounds like 
> > "I am authorized to be root", but here it's set while in "auth required" 
> > state, sounds like we're not root yet?
> > (I know nothing about polkit/kauth but at least this code should make 
> > sense to me ;)

The name isRoot is misleading so I changed it to mPriviledgeOpStarted. It is 
used to show that the priviledged operation has started and will be set to 
false once the job ends. This along with kauthStatus are used to decide whether 
to show warning dialog or not.

The "execute" method in KAuth is similar to sudo command. The priviledge 
persist for about 5 minutes. It is during this time we need to show the 
warning. Now when deleting multiple files we can either delete them using one 
job(deleteRecursive) or multiple jobs. So to determine whether the call to 
execWithRoot was from the same job or not and show the warning accordingly I 
used the aforementioned members.

So my logic here is,
- Prepare the arguments
- If kauth status is Auth Required status(i.e. execWithRoot is called for the 
first time) then set mPriviledgeOpStarted to true.
- If kauth status is Authorized status (i.e. priviledges persists) and 
mPriviledgeOpStarted is false(this is a different job) then set it to true.
- If the user entered the correct password or chose continue proceed with the 
action else halt.


> On March 12, 2017, 3:49 p.m., David Faure wrote:
> > src/ioslaves/file/CMakeLists.txt, line 37
> > 
> >
> > if (UNIX)
> >add_subdirectory(kauth)
> > endif()
> > 
> > Apparently kauth doesn't exist on Windows.

Shall I move all the kauth specific code to file_unix ?


- Chinmoy Ranjan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129983/#review102823
---


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

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

2017-03-14 Thread Chinmoy Ranjan Pradhan



> On March 13, 2017, 12:10 p.m., Elvis Angelaccio wrote:
> > I'm not sure if we should show the warning dialog at the kio_file level. 
> > This would result in a double dialog when deleting files with Shift+Del 
> > from file managers. What about making jobuidelegate.cpp smarter? It could 
> > show a more "scary" message if the file that's being deleted is 
> > write-protected.

In my system when i use Shift+del there is no warning dialog (few months back 
dolphin used to show the dialog but strangly it doesn't shows now) so this case 
didn't occured to me. 

If I am not wrong jobuidelegate's warning is shown before the job executes and 
kauth's authentication dialog is shown only after job executes. But its just 
the opposite we want. So if there's a way for slave to notify the job of 
starting of a priviledged action then there should be no problem in making 
jobuidelegate smarter. 
(But the thing is at this very moment i can't think of or find any possible way)


- Chinmoy Ranjan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129983/#review102826
---


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
> 
>

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

2017-03-14 Thread Chinmoy Ranjan Pradhan

---
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.

Changes
---

Addressed all the raised issues.

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.

Diffs (updated)
-

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

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

2017-03-13 Thread Elvis Angelaccio


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129983/#review102826
---



I'm not sure if we should show the warning dialog at the kio_file level. This 
would result in a double dialog when deleting files with Shift+Del from file 
managers. What about making jobuidelegate.cpp smarter? It could show a more 
"scary" message if the file that's being deleted is write-protected.

- Elvis Angelaccio


On March 9, 2017, 3:57 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 9, 2017, 3:57 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
> 
>

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

2017-03-09 Thread Chinmoy Ranjan Pradhan

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129983/
---

(Updated March 9, 2017, 3:57 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.

Diffs (updated)
-

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

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

2017-03-09 Thread Chinmoy Ranjan Pradhan



> On March 6, 2017, 11:38 p.m., Aleix Pol Gonzalez wrote:
> > src/ioslaves/file/file.cpp, line 1378
> > 
> >
> > I'm not convinced, what's the point of requestroot? Why don't you let 
> > it call the action right away?
> 
> Chinmoy Ranjan Pradhan wrote:
> For a single file calling action right away might work but doing this for 
> multiple files will show the authentication dialog everytime before file is 
> deleted. This is something we don't want. 
> One way to solve this is to store all the paths and delete them in helper 
> but this approach might  cause problem while porting FileProtocol::copy.
> 
> Elvis Angelaccio wrote:
> > doing this for multiple files will show the authentication dialog 
> everytime before file is deleted.
> 
> This should not happen, sounds like a bug in KAuth? With 
> `Persistence=session` the authorization is supposed to last until the user 
> logs out, according to 
> https://api.kde.org/frameworks/kauth/html/namespaceKAuth.html

It happened because I ommited the Persistence=session statement and this 
behaviour is not a bug. 
My previous comment was somewhat misleading, really sorry for that.


- Chinmoy Ranjan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129983/#review102738
---


On March 9, 2017, 2:57 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 9, 2017, 2:57 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;
> The file ioslave gets three methods, /*getRootPermission, execWithRoot and 
> unsetRoot*/ with a variable /*isRoot*/ to store the persistance. The helper 
> gets two actions, /*org.kde.kio.file.requestroot*/ and 
> /*org.kde.kio.file.execute*/. 
> When an action encounters access denied error the method "execWithRoot" is 
> called with the action you want to perform and the path of objects upon which 
> you want to perform the action as arguments. This method then calls 
> "getRootPermission" for authorisation purpose. Upon successfull authorisation 
> this will then go on performing the desired action as privileged user. Once 
> the job is finished "unsetRoot" is called.
> For authorisation a call to "org.kde.kio.file.requestroot" will be made. This 
> action has its "Policy" set to "auth_admin" so as to prompt for password 
> every time its called. And the action "org.kde.kio.file.execute" has its 
> "Policy" set to "yes" so that it can carry out the desired action as a 
> priviledged user without asking for authentication. 
> 
> As for the deletion of files and directories are concerned, the 
> authentication dialog will pop up only once i.e, for the first file/directory 
> that needs requires a priviledged user to delete them. If there are more 
> files which only priviledge users can delete then they will be deleted 
> straightaway without asking for authentication. This is decided by the truth 
> of variable "isRoot". Once the delete job is finished "isRoot" is set to 
> false. In short once the job has started and authentication's been done, the 
> root access will persist and once the job is finished the root access will 
> reset.
> 
> 
> 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
> 
>

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

2017-03-09 Thread Chinmoy Ranjan Pradhan

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129983/
---

(Updated March 9, 2017, 2:57 p.m.)

Review request for KDE Frameworks, David Faure and Elvis Angelaccio.

Changes
---

Addressed Aleix's issue.
Removed 'requestroot' method which was pointless.
Added method to display warning dialog if root access persists.

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;
The file ioslave gets three methods, /*getRootPermission, execWithRoot and
unsetRoot*/ with a variable /*isRoot*/ to store the persistance. The helper
gets two actions, /*org.kde.kio.file.requestroot*/ and
/*org.kde.kio.file.execute*/.
When an action encounters access denied error the method "execWithRoot" is
called with the action you want to perform and the path of objects upon which
you want to perform the action as arguments. This method then calls
"getRootPermission" for authorisation purpose. Upon successfull authorisation
this will then go on performing the desired action as privileged user. Once the
job is finished "unsetRoot" is called.
For authorisation a call to "org.kde.kio.file.requestroot" will be made. This
action has its "Policy" set to "auth_admin" so as to prompt for password every
time its called. And the action "org.kde.kio.file.execute" has its "Policy" set
to "yes" so that it can carry out the desired action as a priviledged user
without asking for authentication.

As for the deletion of files and directories are concerned, the authentication
dialog will pop up only once i.e, for the first file/directory that needs
requires a priviledged user to delete them. If there are more files which only
priviledge users can delete then they will be deleted straightaway without
asking for authentication. This is decided by the truth of variable "isRoot".
Once the delete job is finished "isRoot" is set to false. In short once the job
has started and authentication's been done, the root access will persist and
once the job is finished the root access will reset.

Diffs (updated)
-

Diff: https://git.reviewboard.kde.org/r/129983/diff/

Testing
---

File Attachments (updated)

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

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

2017-03-08 Thread Elvis Angelaccio



> On March 6, 2017, 11:38 p.m., Aleix Pol Gonzalez wrote:
> > src/ioslaves/file/file.cpp, line 1378
> > 
> >
> > I'm not convinced, what's the point of requestroot? Why don't you let 
> > it call the action right away?
> 
> Chinmoy Ranjan Pradhan wrote:
> For a single file calling action right away might work but doing this for 
> multiple files will show the authentication dialog everytime before file is 
> deleted. This is something we don't want. 
> One way to solve this is to store all the paths and delete them in helper 
> but this approach might  cause problem while porting FileProtocol::copy.

> doing this for multiple files will show the authentication dialog everytime 
> before file is deleted.

This should not happen, sounds like a bug in KAuth? With `Persistence=session` 
the authorization is supposed to last until the user logs out, according to 
https://api.kde.org/frameworks/kauth/html/namespaceKAuth.html


- Elvis


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129983/#review102738
---


On March 6, 2017, 3:51 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 6, 2017, 3:51 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;
> The file ioslave gets three methods, /*getRootPermission, execWithRoot and 
> unsetRoot*/ with a variable /*isRoot*/ to store the persistance. The helper 
> gets two actions, /*org.kde.kio.file.requestroot*/ and 
> /*org.kde.kio.file.execute*/. 
> When an action encounters access denied error the method "execWithRoot" is 
> called with the action you want to perform and the path of objects upon which 
> you want to perform the action as arguments. This method then calls 
> "getRootPermission" for authorisation purpose. Upon successfull authorisation 
> this will then go on performing the desired action as privileged user. Once 
> the job is finished "unsetRoot" is called.
> For authorisation a call to "org.kde.kio.file.requestroot" will be made. This 
> action has its "Policy" set to "auth_admin" so as to prompt for password 
> every time its called. And the action "org.kde.kio.file.execute" has its 
> "Policy" set to "yes" so that it can carry out the desired action as a 
> priviledged user without asking for authentication. 
> 
> As for the deletion of files and directories are concerned, the 
> authentication dialog will pop up only once i.e, for the first file/directory 
> that needs requires a priviledged user to delete them. If there are more 
> files which only priviledge users can delete then they will be deleted 
> straightaway without asking for authentication. This is decided by the truth 
> of variable "isRoot". Once the delete job is finished "isRoot" is set to 
> false. In short once the job has started and authentication's been done, the 
> root access will persist and once the job is finished the root access will 
> reset.
> 
> 
> 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
> ---
> 
> 
> Thanks,
> 
> Chinmoy Ranjan Pradhan
> 
>

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

2017-03-07 Thread Chinmoy Ranjan Pradhan



> On March 6, 2017, 11:38 p.m., Aleix Pol Gonzalez wrote:
> > src/ioslaves/file/file.cpp, line 1378
> > 
> >
> > I'm not convinced, what's the point of requestroot? Why don't you let 
> > it call the action right away?

For a single file calling action right away might work but doing this for 
multiple files will show the authentication dialog everytime before file is 
deleted. This is something we don't want. 
One way to solve this is to store all the paths and delete them in helper but 
this approach might  cause problem while porting FileProtocol::copy.


- Chinmoy Ranjan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129983/#review102738
---


On March 6, 2017, 3:51 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 6, 2017, 3:51 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;
> The file ioslave gets three methods, /*getRootPermission, execWithRoot and 
> unsetRoot*/ with a variable /*isRoot*/ to store the persistance. The helper 
> gets two actions, /*org.kde.kio.file.requestroot*/ and 
> /*org.kde.kio.file.execute*/. 
> When an action encounters access denied error the method "execWithRoot" is 
> called with the action you want to perform and the path of objects upon which 
> you want to perform the action as arguments. This method then calls 
> "getRootPermission" for authorisation purpose. Upon successfull authorisation 
> this will then go on performing the desired action as privileged user. Once 
> the job is finished "unsetRoot" is called.
> For authorisation a call to "org.kde.kio.file.requestroot" will be made. This 
> action has its "Policy" set to "auth_admin" so as to prompt for password 
> every time its called. And the action "org.kde.kio.file.execute" has its 
> "Policy" set to "yes" so that it can carry out the desired action as a 
> priviledged user without asking for authentication. 
> 
> As for the deletion of files and directories are concerned, the 
> authentication dialog will pop up only once i.e, for the first file/directory 
> that needs requires a priviledged user to delete them. If there are more 
> files which only priviledge users can delete then they will be deleted 
> straightaway without asking for authentication. This is decided by the truth 
> of variable "isRoot". Once the delete job is finished "isRoot" is set to 
> false. In short once the job has started and authentication's been done, the 
> root access will persist and once the job is finished the root access will 
> reset.
> 
> 
> 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
> ---
> 
> 
> Thanks,
> 
> Chinmoy Ranjan Pradhan
> 
>

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

2017-03-06 Thread Aleix Pol Gonzalez


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129983/#review102738
---




src/ioslaves/file/file.cpp (line 1378)


I'm not convinced, what's the point of requestroot? Why don't you let it 
call the action right away?


- Aleix Pol Gonzalez


On mar. 6, 2017, 4:51 p.m., Chinmoy Ranjan Pradhan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129983/
> ---
> 
> (Updated mar. 6, 2017, 4:51 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;
> The file ioslave gets three methods, /*getRootPermission, execWithRoot and 
> unsetRoot*/ with a variable /*isRoot*/ to store the persistance. The helper 
> gets two actions, /*org.kde.kio.file.requestroot*/ and 
> /*org.kde.kio.file.execute*/. 
> When an action encounters access denied error the method "execWithRoot" is 
> called with the action you want to perform and the path of objects upon which 
> you want to perform the action as arguments. This method then calls 
> "getRootPermission" for authorisation purpose. Upon successfull authorisation 
> this will then go on performing the desired action as privileged user. Once 
> the job is finished "unsetRoot" is called.
> For authorisation a call to "org.kde.kio.file.requestroot" will be made. This 
> action has its "Policy" set to "auth_admin" so as to prompt for password 
> every time its called. And the action "org.kde.kio.file.execute" has its 
> "Policy" set to "yes" so that it can carry out the desired action as a 
> priviledged user without asking for authentication. 
> 
> As for the deletion of files and directories are concerned, the 
> authentication dialog will pop up only once i.e, for the first file/directory 
> that needs requires a priviledged user to delete them. If there are more 
> files which only priviledge users can delete then they will be deleted 
> straightaway without asking for authentication. This is decided by the truth 
> of variable "isRoot". Once the delete job is finished "isRoot" is set to 
> false. In short once the job has started and authentication's been done, the 
> root access will persist and once the job is finished the root access will 
> reset.
> 
> 
> 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
> ---
> 
> 
> Thanks,
> 
> Chinmoy Ranjan Pradhan
> 
>

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

2017-03-06 Thread Chinmoy Ranjan Pradhan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129983/
---

(Updated March 6, 2017, 3:51 p.m.)


Review request for KDE Frameworks, David Faure and Elvis Angelaccio.


Summary (updated)
-

[RFC] PoC patch for polkit support in kio. 


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;
The file ioslave gets three methods, /*getRootPermission, execWithRoot and 
unsetRoot*/ with a variable /*isRoot*/ to store the persistance. The helper 
gets two actions, /*org.kde.kio.file.requestroot*/ and 
/*org.kde.kio.file.execute*/. 
When an action encounters access denied error the method "execWithRoot" is 
called with the action you want to perform and the path of objects upon which 
you want to perform the action as arguments. This method then calls 
"getRootPermission" for authorisation purpose. Upon successfull authorisation 
this will then go on performing the desired action as privileged user. Once the 
job is finished "unsetRoot" is called.
For authorisation a call to "org.kde.kio.file.requestroot" will be made. This 
action has its "Policy" set to "auth_admin" so as to prompt for password every 
time its called. And the action "org.kde.kio.file.execute" has its "Policy" set 
to "yes" so that it can carry out the desired action as a priviledged user 
without asking for authentication. 

As for the deletion of files and directories are concerned, the authentication 
dialog will pop up only once i.e, for the first file/directory that needs 
requires a priviledged user to delete them. If there are more files which only 
priviledge users can delete then they will be deleted straightaway without 
asking for authentication. This is decided by the truth of variable "isRoot". 
Once the delete job is finished "isRoot" is set to false. In short once the job 
has started and authentication's been done, the root access will persist and 
once the job is finished the root access will reset.


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
---


Thanks,

Chinmoy Ranjan Pradhan

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

Re: Review Request 129983: [RFC] PoC patch for polkit support in kio.

33 matches

Site Navigation

Mail list logo

Footer information