D6830: Make use of kauth helper in copy method of file ioslave

[David Faure]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20170807/91c15e94/attachment.html>