- Group Reviewers
- Maniphest Tasks
- T6561: Polkit support in KIO
- R241:13ab6a85bbb8: Use kauth helper in member functions of FileProtocol
Please resubmit with context.
Also, did you check that unittests still pass? No kauth prompt should occur :-)
the 'm' in "from" was actually good looking there :)
Picking this random one as an example:
remove new empty line, there's already one
what happens if the user cancels the root-password prompt? He will then get another msg box with "cannot create directory?" even though he/she purposefully canceled the operation? When that happens the slave should use error(KIO::ERR_USER_CANCELED) instead (which means no error box), but of course only in that case...
With current version of my code this is not possible. The method isPrivilegeOperationAllowed returns false when user hits cancel or when PrivilegeExecution flag isn't set. To distinguish between them some kind of data needs to be sent to slave. Here addMetaData doesn't work. So another signal is needed. Or maybe readData can help here.
If you agree that showing an error message after hitting Cancel on the kauth prompt is suboptimal (I didn't actually test it), then it seems to me that a simple solution to turn bool isPrivilegeOperationAllowed() into a method that returns an enum? (same for execWithElevatedPrivilege)
The values would be
- Allowed (-> proceed)
- Not allowed / not supported ( -> show error)
- Canceled (-> don't show error)
It's already a string true/false in the socket, that's easily extendable.
Ah! I know how to improve the readability of (auto ret = execWithElevatedPrivilege)... please rename all "ret" variables to "err" (short for error, which is already taken), or if you prefer something longer, maybe execError ? eWEPError ?
1.Added check for unit test mode at beginning of put.
2.put was setting the wrong permissions because I initialized 'filemode' to 0 and did't update it. Now it is fixed.
3.Replaced file.setPermissions with Qfile::setPermissions and used tryChangeFileAttr in put.
4.Moved 'finished()' in chmod out of the else block.
Without looking too much into details of the patch: what happens, if you are already root? Does that work as expected?
I am asking, since we introduced regressions into Kate since using kauth to save data with elevated privileges: Kate disallows running as root. But if you really are root, Kate should just work, see https://bugs.kde.org/show_bug.cgi?id=387973 or also https://bugs.kde.org/show_bug.cgi?id=388041 .