Re: Review Request 127948: Avoid skipping KAuthorized check

2016-05-19 Thread David Edmundson 

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

(Updated May 19, 2016, 9:19 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Matthew Dawson.


Changes
---

Submitted with commit b1c64c165eb7e498f6cfed65e2d439d86f21a116 by David 
Edmundson to branch master.


Repository: kconfig


Description
---

Previously, if a .desktop file had a TryExec and a X-KDE-AuthorizeAction
entry we would skip the KAuthorized check. This is clearly wrong.


Diffs
-

  autotests/kdesktopfiletest.h f4e0c96b8751b2b116b98a52553ad6698b9d40b4 
  autotests/kdesktopfiletest.cpp a90faf3ca44b6a8f5d7d7c9de4d58d17ba360d16 
  src/core/kdesktopfile.cpp eda7c292da9aa2bc97f6066e90034767134c2e07 

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


Testing
---

Disabled shell_access in kde5rc.

With I was unable to open konsole from kicker.
Opening dolphin still worked as normal.


Thanks,

David Edmundson

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127948: Avoid skipping KAuthorized check

2016-05-19 Thread Matthew Dawson 

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


Fix it, then Ship it!




Excellent, thanks!  Just one comment below about the name of the test.  If that 
name is good with you, feel free to change + push.


autotests/kdesktopfiletest.h (line 36)


Can you change this name to ```testTryExecWithAuthorizeAction```?  
testTryExec sound like it is testing TryExec in general, which is being tested 
above.  That name is hopefully a little more descriptive of the issue under 
test.


- Matthew Dawson


On May 19, 2016, 4:17 p.m., David Edmundson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127948/
> ---
> 
> (Updated May 19, 2016, 4:17 p.m.)
> 
> 
> Review request for KDE Frameworks and Matthew Dawson.
> 
> 
> Repository: kconfig
> 
> 
> Description
> ---
> 
> Previously, if a .desktop file had a TryExec and a X-KDE-AuthorizeAction
> entry we would skip the KAuthorized check. This is clearly wrong.
> 
> 
> Diffs
> -
> 
>   autotests/kdesktopfiletest.h f4e0c96b8751b2b116b98a52553ad6698b9d40b4 
>   autotests/kdesktopfiletest.cpp a90faf3ca44b6a8f5d7d7c9de4d58d17ba360d16 
>   src/core/kdesktopfile.cpp eda7c292da9aa2bc97f6066e90034767134c2e07 
> 
> Diff: https://git.reviewboard.kde.org/r/127948/diff/
> 
> 
> Testing
> ---
> 
> Disabled shell_access in kde5rc.
> 
> With I was unable to open konsole from kicker.
> Opening dolphin still worked as normal.
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127948: Avoid skipping KAuthorized check

2016-05-19 Thread David Edmundson 

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

(Updated May 19, 2016, 8:17 p.m.)


Review request for KDE Frameworks and Matthew Dawson.


Changes
---

Wrote a test. 
It fails without my fix, it passes with the fix.


Repository: kconfig


Description
---

Previously, if a .desktop file had a TryExec and a X-KDE-AuthorizeAction
entry we would skip the KAuthorized check. This is clearly wrong.


Diffs (updated)
-

  autotests/kdesktopfiletest.h f4e0c96b8751b2b116b98a52553ad6698b9d40b4 
  autotests/kdesktopfiletest.cpp a90faf3ca44b6a8f5d7d7c9de4d58d17ba360d16 
  src/core/kdesktopfile.cpp eda7c292da9aa2bc97f6066e90034767134c2e07 

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


Testing
---

Disabled shell_access in kde5rc.

With I was unable to open konsole from kicker.
Opening dolphin still worked as normal.


Thanks,

David Edmundson

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127948: Avoid skipping KAuthorized check

2016-05-17 Thread Matthew Dawson 

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



+1 from me.  Before pushing, can you please add a unit test for this behaviour? 
 Otherwise looks good to go.

- Matthew Dawson


On May 17, 2016, 3:57 p.m., David Edmundson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127948/
> ---
> 
> (Updated May 17, 2016, 3:57 p.m.)
> 
> 
> Review request for KDE Frameworks and Matthew Dawson.
> 
> 
> Repository: kconfig
> 
> 
> Description
> ---
> 
> Previously, if a .desktop file had a TryExec and a X-KDE-AuthorizeAction
> entry we would skip the KAuthorized check. This is clearly wrong.
> 
> 
> Diffs
> -
> 
>   src/core/kdesktopfile.cpp eda7c292da9aa2bc97f6066e90034767134c2e07 
> 
> Diff: https://git.reviewboard.kde.org/r/127948/diff/
> 
> 
> Testing
> ---
> 
> Disabled shell_access in kde5rc.
> 
> With I was unable to open konsole from kicker.
> Opening dolphin still worked as normal.
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127948: Avoid skipping KAuthorized check

2016-05-17 Thread Kai Uwe Broulik 


> On Mai 17, 2016, 8:12 nachm., Kai Uwe Broulik wrote:
> > +1
> > 
> > Dolphin probably uses KRun which uses KDesktopFile::isAuthorizedDesktopFile 
> > which does not check for X-KDE-AuthorizeAction
> 
> David Edmundson wrote:
> With the comment about Dolphin, I simply meant I had tested that I hadn't 
> broken the normal case and that we could still load other apps (firefox, 
> dolphin, kate whatever)

Ah, misread it as "opening in dolphin" :)


- Kai Uwe


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


On Mai 17, 2016, 7:57 nachm., David Edmundson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127948/
> ---
> 
> (Updated Mai 17, 2016, 7:57 nachm.)
> 
> 
> Review request for KDE Frameworks and Matthew Dawson.
> 
> 
> Repository: kconfig
> 
> 
> Description
> ---
> 
> Previously, if a .desktop file had a TryExec and a X-KDE-AuthorizeAction
> entry we would skip the KAuthorized check. This is clearly wrong.
> 
> 
> Diffs
> -
> 
>   src/core/kdesktopfile.cpp eda7c292da9aa2bc97f6066e90034767134c2e07 
> 
> Diff: https://git.reviewboard.kde.org/r/127948/diff/
> 
> 
> Testing
> ---
> 
> Disabled shell_access in kde5rc.
> 
> With I was unable to open konsole from kicker.
> Opening dolphin still worked as normal.
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127948: Avoid skipping KAuthorized check

2016-05-17 Thread David Edmundson 


> On May 17, 2016, 8:12 p.m., Kai Uwe Broulik wrote:
> > +1
> > 
> > Dolphin probably uses KRun which uses KDesktopFile::isAuthorizedDesktopFile 
> > which does not check for X-KDE-AuthorizeAction

With the comment about Dolphin, I simply meant I had tested that I hadn't 
broken the normal case and that we could still load other apps (firefox, 
dolphin, kate whatever)


- David


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


On May 17, 2016, 7:57 p.m., David Edmundson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127948/
> ---
> 
> (Updated May 17, 2016, 7:57 p.m.)
> 
> 
> Review request for KDE Frameworks and Matthew Dawson.
> 
> 
> Repository: kconfig
> 
> 
> Description
> ---
> 
> Previously, if a .desktop file had a TryExec and a X-KDE-AuthorizeAction
> entry we would skip the KAuthorized check. This is clearly wrong.
> 
> 
> Diffs
> -
> 
>   src/core/kdesktopfile.cpp eda7c292da9aa2bc97f6066e90034767134c2e07 
> 
> Diff: https://git.reviewboard.kde.org/r/127948/diff/
> 
> 
> Testing
> ---
> 
> Disabled shell_access in kde5rc.
> 
> With I was unable to open konsole from kicker.
> Opening dolphin still worked as normal.
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 127948: Avoid skipping KAuthorized check

2016-05-17 Thread Kai Uwe Broulik 

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



+1

Dolphin probably uses KRun which uses KDesktopFile::isAuthorizedDesktopFile 
which does not check for X-KDE-AuthorizeAction

- Kai Uwe Broulik


On Mai 17, 2016, 7:57 nachm., David Edmundson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127948/
> ---
> 
> (Updated Mai 17, 2016, 7:57 nachm.)
> 
> 
> Review request for KDE Frameworks and Matthew Dawson.
> 
> 
> Repository: kconfig
> 
> 
> Description
> ---
> 
> Previously, if a .desktop file had a TryExec and a X-KDE-AuthorizeAction
> entry we would skip the KAuthorized check. This is clearly wrong.
> 
> 
> Diffs
> -
> 
>   src/core/kdesktopfile.cpp eda7c292da9aa2bc97f6066e90034767134c2e07 
> 
> Diff: https://git.reviewboard.kde.org/r/127948/diff/
> 
> 
> Testing
> ---
> 
> Disabled shell_access in kde5rc.
> 
> With I was unable to open konsole from kicker.
> Opening dolphin still worked as normal.
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel