dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> file_unix.cpp:244
> #endif
> - dest_file.remove(); // don't keep partly copied file
> + if (!!QFile::remove(dest)) { // don't keep partly copied file
> + execWithElevatedPrivilege(DEL, _dest);
Double negation, looks like one too many.
> file_unix.cpp:290
> #endif
> - dest_file.remove(); // don't keep partly copied file
> + if (!!QFile::remove(dest)) { // don't keep partly copied file
> + execWithElevatedPrivilege(DEL, _dest);
same
> file_unix.cpp:327
> } else {
> - qCWarning(KIO_FILE) << QStringLiteral("Couldn't preserve group for
> '%1'").arg(dest);
> + if (!execWithElevatedPrivilege(CHOWN, _dest, buff_src.st_uid,
> buff_src.st_gid))
> + qCWarning(KIO_FILE) << QStringLiteral("Couldn't preserve group
> for '%1'").arg(dest);
Can you test copying a file onto a FAT partition mounted as another uid? The
ownership and permissions cannot be applied due to FAT limitations. Does that
then trigger a kauth prompt, with this patch? That would be annoying, and wrong
since root can't do better anyway.
If I'm right (please test it) then I think this method should remember "I
needed root permissions for the main operation" and only in that case use
elevated privileges for the small operations at the end like chmod, chown or
utime.
Please review whether other methods need the same kind of change.
REVISION DETAIL
https://phabricator.kde.org/D6830
To: chinmoyr, dfaure, #frameworks
Cc: elvisangelaccio, #frameworks
