File 0002-Revert-KAuth-integration-in-document-savin... of Package ktexteditor

Overview Repositories Revisions Requests Users Attributes Meta

File 0002-Revert-KAuth-integration-in-document-saving.patch of Package ktexteditor (Revision be91179e1ae55ae455b27e4b477de0d1)

Currently displaying revision be91179e1ae55ae455b27e4b477de0d1 , Show latest

From a76f9a88ae11e73682e1b1109142e04632b18bf8 Mon Sep 17 00:00:00 2001
From: Fabian Vogt <fvogt@suse.com>
Date: Mon, 15 May 2017 14:06:39 +0200
Subject: [PATCH 2/2] Revert "KAuth integration in document saving"

This reverts commit ae60880c5f9ba6c703db404ec45723381d7a3fa1.
---
 autotests/src/katetextbuffertest.cpp               |  34 -----
 autotests/src/katetextbuffertest.h                 |   1 -
 src/CMakeLists.txt                                 |  10 --
 src/buffer/katesecuretextbuffer.cpp                | 156 --------------------
 src/buffer/katesecuretextbuffer_p.h                |  89 -----------
 src/buffer/katetextbuffer.cpp                      | 163 ++++-----------------
 src/buffer/katetextbuffer.h                        |   7 +-
 8 files changed, 33 insertions(+), 504 deletions(-)
 delete mode 100644 src/buffer/katesecuretextbuffer.cpp
 delete mode 100644 src/buffer/katesecuretextbuffer_p.h

diff --git a/autotests/src/katetextbuffertest.cpp b/autotests/src/katetextbuffertest.cpp
index bf252497..fc55e64f 100644
--- a/autotests/src/katetextbuffertest.cpp
+++ b/autotests/src/katetextbuffertest.cpp
@@ -466,37 +466,3 @@ void KateTextBufferTest::saveFileInUnwritableFolder()
     QVERIFY(f.remove());
     QVERIFY(dir.remove());
 }
-
-void KateTextBufferTest::saveFileWithElevatedPrivileges()
-{
-    // create temp dir and get file name inside
-    QTemporaryDir dir;
-    QVERIFY(dir.isValid());
-    const QString file_path = dir.path() + QLatin1String("/foo");
-
-    QFile f(file_path);
-    QVERIFY(f.open(QIODevice::WriteOnly | QIODevice::Truncate));
-    f.write("1234567890");
-    QVERIFY(f.flush());
-    f.close();
-
-    Kate::TextBuffer buffer(nullptr, 1, true);
-    buffer.setTextCodec(QTextCodec::codecForName("UTF-8"));
-    buffer.setFallbackTextCodec(QTextCodec::codecForName("UTF-8"));
-    bool a, b;
-    int c;
-    buffer.load(file_path, a, b, c, true);
-    buffer.clear();
-    buffer.startEditing();
-    buffer.insertText(KTextEditor::Cursor(0, 0), QLatin1String("ABC"));
-    buffer.finishEditing();
-    qDebug() << buffer.text();
-    buffer.save(file_path);
-
-    f.open(QIODevice::ReadOnly);
-    QCOMPARE(f.readAll(), QByteArray("ABC"));
-    f.close();
-
-    QVERIFY(f.remove());
-    QVERIFY(dir.remove());
-}
diff --git a/autotests/src/katetextbuffertest.h b/autotests/src/katetextbuffertest.h
index 7a3df3c1..0ddcff08 100644
--- a/autotests/src/katetextbuffertest.h
+++ b/autotests/src/katetextbuffertest.h
@@ -40,7 +40,6 @@ private Q_SLOTS:
     void foldingTest();
     void nestedFoldingTest();
     void saveFileInUnwritableFolder();
-    void saveFileWithElevatedPrivileges();
 };
 
 #endif // KATEBUFFERTEST_H
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index b7adedca..8f414b07 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -52,7 +52,6 @@ include_directories(
 # KTextEditor interface sources
 set(ktexteditor_LIB_SRCS
 # text buffer & buffer helpers
-buffer/katesecuretextbuffer.cpp
 buffer/katetextbuffer.cpp
 buffer/katetextblock.cpp
 buffer/katetextline.cpp
@@ -335,14 +334,5 @@ include(ECMGeneratePriFile)
 ecm_generate_pri_file(BASE_NAME KTextEditor LIB_NAME KF5TextEditor DEPS "KParts" FILENAME_VAR PRI_FILENAME INCLUDE_INSTALL_DIR ${KDE_INSTALL_INCLUDEDIR_KF5}/KTextEditor)
 install(FILES ${PRI_FILENAME} DESTINATION ${ECM_MKSPECS_INSTALL_DIR})
 
-
-add_executable(kauth_ktexteditor_helper buffer/katesecuretextbuffer.cpp)
-target_link_libraries(kauth_ktexteditor_helper
-  KF5::Auth
-)
-install(TARGETS kauth_ktexteditor_helper DESTINATION ${KAUTH_HELPER_INSTALL_DIR} )
-kauth_install_helper_files(kauth_ktexteditor_helper org.kde.ktexteditor.katetextbuffer root)
-kauth_install_actions(org.kde.ktexteditor.katetextbuffer buffer/org.kde.ktexteditor.katetextbuffer.actions)
-
 # add part
 add_subdirectory(part)
diff --git a/src/buffer/katesecuretextbuffer.cpp b/src/buffer/katesecuretextbuffer.cpp
deleted file mode 100644
index 979bb62f..00000000
--- a/src/buffer/katesecuretextbuffer.cpp
+++ /dev/null
@@ -1,156 +0,0 @@
-/*  This file is part of the KTextEditor project.
- *
- *  Copyright (C) 2017 KDE Developers
- *
- *  This library is free software; you can redistribute it and/or
- *  modify it under the terms of the GNU Library General Public
- *  License as published by the Free Software Foundation; either
- *  version 2 of the License, or (at your option) any later version.
- *
- *  This library is distributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- *  Library General Public License for more details.
- *
- *  You should have received a copy of the GNU Library General Public License
- *  along with this library; see the file COPYING.LIB.  If not, write to
- *  the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
- *  Boston, MA 02110-1301, USA.
- */
-
-#include "katesecuretextbuffer_p.h"
-
-#ifndef Q_OS_WIN
-#include <unistd.h>
-#include <errno.h>
-#endif
-
-#include <QString>
-#include <QFile>
-#include <QTemporaryFile>
-#include <QSaveFile>
-
-KAUTH_HELPER_MAIN("org.kde.ktexteditor.katetextbuffer", SecureTextBuffer)
-
-ActionReply SecureTextBuffer::savefile(const QVariantMap &args)
-{
-    const ActionMode actionMode = static_cast<ActionMode>(args[QLatin1String("actionMode")].toInt());
-    const QString targetFile = args[QLatin1String("targetFile")].toString();
-    const uint ownerId = (uint) args[QLatin1String("ownerId")].toInt();
-
-    if (actionMode == ActionMode::Prepare) {
-
-        const QString temporaryFile = prepareTempFileInternal(targetFile, ownerId);
-
-        if (temporaryFile.isEmpty()) {
-            return ActionReply::HelperErrorReply();
-        }
-
-        ActionReply reply;
-        reply.addData(QLatin1String("temporaryFile"), temporaryFile);
-
-        return reply;
-
-    }
-
-    if (actionMode == ActionMode::Move) {
-
-        const QString sourceFile = args[QLatin1String("sourceFile")].toString();
-        const uint groupId = (uint) args[QLatin1String("groupId")].toInt();
-
-        if (moveFileInternal(sourceFile, targetFile, ownerId, groupId)) {
-            return ActionReply::SuccessReply();
-        }
-    }
-
-    return ActionReply::HelperErrorReply();
-}
-
-bool SecureTextBuffer::moveFileInternal(const QString &sourceFile, const QString &targetFile, const uint ownerId, const uint groupId)
-{
-    const bool newFile = !QFile::exists(targetFile);
-    bool atomicRenameSucceeded = false;
-
-    /**
-     * There is no atomic rename operation publicly exposed by Qt.
-     *
-     * We use std::rename for UNIX and for now no-op for windows (triggers fallback).
-     *
-     * As fallback we are copying source file to destination with the help of QSaveFile
-     * to ensure targetFile is overwritten safely.
-     */
-#ifndef Q_OS_WIN
-    const int result = std::rename(QFile::encodeName(sourceFile).constData(), QFile::encodeName(targetFile).constData());
-    if (result == 0) {
-        syncToDisk(QFile(targetFile).handle());
-        atomicRenameSucceeded = true;
-    }
-#else
-    atomicRenameSucceeded = false;
-#endif
-
-    if (!atomicRenameSucceeded) {
-        // as fallback copy the temporary file to the target with help of QSaveFile
-        QFile readFile(sourceFile);
-        QSaveFile saveFile(targetFile);
-        if (!readFile.open(QIODevice::ReadOnly) || !saveFile.open(QIODevice::WriteOnly)) {
-            return false;
-        }
-        char buffer[bufferLength];
-        qint64 read = -1;
-        while ((read = readFile.read(buffer, bufferLength)) > 0) {
-            if (saveFile.write(buffer, read) == -1) {
-                return false;
-            }
-        }
-        if (read == -1 || !saveFile.commit()) {
-            return false;
-        }
-    }
-
-    if (!newFile) {
-        // ensure file has the same owner and group as before
-        setOwner(targetFile, ownerId, groupId);
-    }
-
-    return true;
-}
-
-QString SecureTextBuffer::prepareTempFileInternal(const QString &targetFile, const uint ownerId)
-{
-    QTemporaryFile tempFile(targetFile);
-    if (!tempFile.open()) {
-        return QString();
-    }
-    tempFile.setAutoRemove(false);
-    setOwner(tempFile.fileName(), ownerId, -1);
-    return tempFile.fileName();
-}
-
-void SecureTextBuffer::setOwner(const QString &filename, const uint ownerId, const uint groupId)
-{
-#ifndef Q_OS_WIN
-    if (ownerId != (uint)-2 && groupId != (uint)-2) {
-        const int result = chown(QFile::encodeName(filename).constData(), ownerId, groupId);
-        // set at least correct group if owner cannot be changed
-        if (result != 0 && errno == EPERM) {
-            chown(QFile::encodeName(filename).constData(), getuid(), groupId);
-        }
-    }
-#else
-    // no-op for windows
-#endif
-}
-
-void SecureTextBuffer::syncToDisk(const int fd)
-{
-#ifndef Q_OS_WIN
-#ifdef HAVE_FDATASYNC
-    fdatasync(fd);
-#else
-    fsync(fd);
-#endif
-#else
-    // no-op for windows
-#endif
-}
\ No newline at end of file
diff --git a/src/buffer/katesecuretextbuffer_p.h b/src/buffer/katesecuretextbuffer_p.h
deleted file mode 100644
index b931aa27..00000000
--- a/src/buffer/katesecuretextbuffer_p.h
+++ /dev/null
@@ -1,89 +0,0 @@
-/*  This file is part of the KTextEditor project.
- *
- *  Copyright (C) 2017 KDE Developers
- *
- *  This library is free software; you can redistribute it and/or
- *  modify it under the terms of the GNU Library General Public
- *  License as published by the Free Software Foundation; either
- *  version 2 of the License, or (at your option) any later version.
- *
- *  This library is distributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- *  Library General Public License for more details.
- *
- *  You should have received a copy of the GNU Library General Public License
- *  along with this library; see the file COPYING.LIB.  If not, write to
- *  the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
- *  Boston, MA 02110-1301, USA.
- */
-
-#ifndef KATE_SECURE_TEXTBUFFER_P_H
-#define KATE_SECURE_TEXTBUFFER_P_H
-
-#include <QObject>
-#include <QString>
-
-#include <kauth.h>
-
-using namespace KAuth;
-
-/**
- * Class used as KAuth helper binary.
- * It is supposed to be called through KAuth action.
- *
- * It also contains couple of common methods intended to be used
- * directly by TextBuffer as well as from helper binary.
- *
- * This class should only be used by TextBuffer.
- */
-class SecureTextBuffer : public QObject
-{
-    Q_OBJECT
-
-public:
-
-    /**
-     * We support Prepare action for temporary file creation
-     * and Move action for moving final file to its destination
-     */
-    enum ActionMode {
-        Prepare = 1,
-        Move = 2
-    };
-
-    SecureTextBuffer() {}
-
-    ~SecureTextBuffer() {}
-
-    /**
-     * Common helper methods
-     */
-    static void setOwner(const QString &filename, const uint ownerId, const uint groupId);
-    static void syncToDisk(const int fd);
-
-private:
-    static const qint64 bufferLength = 4096;
-
-    /**
-     * Creates temporary file based on given target file path.
-     * Temporary file is set to not be deleted on object destroy
-     * so KTextEditor can save contents in it.
-     */
-    static QString prepareTempFileInternal(const QString &targetFile, const uint ownerId);
-
-    /**
-     * Move file to its given destination and set owner.
-     */
-    static bool moveFileInternal(const QString &sourceFile, const QString &targetFile, const uint ownerId, const uint groupId);
-
-public Q_SLOTS:
-    /**
-     * KAuth action to perform both prepare or move work based on given parameters.
-     * We keep this code in one method to prevent multiple KAuth user queries during one save action.
-     */
-    static ActionReply savefile(const QVariantMap &args);
-
-};
-
-#endif
diff --git a/src/buffer/katetextbuffer.cpp b/src/buffer/katetextbuffer.cpp
index 95603fa7..91474dff 100644
--- a/src/buffer/katetextbuffer.cpp
+++ b/src/buffer/katetextbuffer.cpp
@@ -19,10 +19,8 @@
  */
 
 #include "config.h"
-#include "kateglobal.h"
 
 #include "katetextbuffer.h"
-#include "katesecuretextbuffer_p.h"
 #include "katetextloader.h"
 
 // this is unfortunate, but needed for performance
@@ -32,12 +30,13 @@
 
 #ifndef Q_OS_WIN
 #include <unistd.h>
+
+// needed for umask application
+#include <sys/types.h>
+#include <sys/stat.h>
 #endif
 
 #include <QSaveFile>
-#include <QTemporaryFile>
-#include <QFileDevice>
-#include <QFileInfo>
 
 #if 0
 #define BUFFER_DEBUG qCDebug(LOG_KTE)
@@ -48,7 +47,7 @@
 namespace Kate
 {
 
-TextBuffer::TextBuffer(KTextEditor::DocumentPrivate *parent, int blockSize, bool alwaysUseKAuth)
+TextBuffer::TextBuffer(KTextEditor::DocumentPrivate *parent, int blockSize)
     : QObject(parent)
     , m_document(parent)
     , m_history(*this)
@@ -68,7 +67,6 @@ TextBuffer::TextBuffer(KTextEditor::DocumentPrivate *parent, int blockSize, bool
     , m_endOfLineMode(eolUnix)
     , m_newLineAtEof(false)
     , m_lineLengthLimit(4096)
-    , m_alwaysUseKAuthForSave(alwaysUseKAuth)
 {
     // minimal block size must be > 0
     Q_ASSERT(m_blockSize > 0);
@@ -765,93 +763,25 @@ bool TextBuffer::save(const QString &filename)
     // codec must be set!
     Q_ASSERT(m_textCodec);
 
+#ifndef Q_OS_WIN
     const bool newFile = !QFile::exists(filename);
-
-    /**
-     * Memorize owner and group. Due to design of QSaveFile we will have to re-set them after save is complete.
-     */
-    uint ownerId = -2;
-    uint groupId = -2;
-    if (!newFile) {
-        QFileInfo fileInfo(filename);
-        ownerId = fileInfo.ownerId();
-        groupId = fileInfo.groupId();
-    }
+#endif
 
     /**
      * use QSaveFile for save write + rename
      */
-    QScopedPointer<QFileDevice> saveFile(new QSaveFile(filename));
-    static_cast<QSaveFile *>(saveFile.data())->setDirectWriteFallback(true);
-
-    bool usingTemporaryFile = false;
-    QScopedPointer<QVariantMap> kAuthActionArgs;
-    QScopedPointer<KAuth::Action> kAuthSaveAction;
-
-    // open QSaveFile for write
-    if (m_alwaysUseKAuthForSave || !saveFile->open(QIODevice::WriteOnly)) {
-
-        // if that fails we need more privileges to save this file
-        // -> we write to temporary file and then move it to target location
-
-        usingTemporaryFile = true;
-
-        QString targetFilename(filename);
-
-        kAuthActionArgs.reset(new QVariantMap());
-        kAuthActionArgs->insert(QLatin1String("actionMode"), SecureTextBuffer::ActionMode::Prepare);
-        kAuthActionArgs->insert(QLatin1String("targetFile"), targetFilename);
-        kAuthActionArgs->insert(QLatin1String("ownerId"), getuid());
-
-        // call save with elevated privileges
-        if (KTextEditor::EditorPrivate::unitTestMode()) {
-
-            // unit testing purposes only
-            ActionReply reply = SecureTextBuffer::savefile(*kAuthActionArgs);
-            if (!reply.succeeded()) {
-                return false;
-            }
-            targetFilename = reply.data()[QLatin1String("temporaryFile")].toString();
+    QSaveFile saveFile(filename);
+    saveFile.setDirectWriteFallback(true);
 
-        } else {
-
-            // call action
-            kAuthSaveAction.reset(new KAuth::Action(QLatin1String("org.kde.ktexteditor.katetextbuffer.savefile")));
-            kAuthSaveAction->setHelperId(QLatin1String("org.kde.ktexteditor.katetextbuffer"));
-            kAuthSaveAction->setArguments(*kAuthActionArgs);
-            KAuth::ExecuteJob *job = kAuthSaveAction->execute();
-            if (!job->exec()) {
-                return false;
-            }
-
-            // get temporary file path from the reply
-            targetFilename = job->data()[QLatin1String("temporaryFile")].toString();
-
-        }
-
-        if (targetFilename.isEmpty()) {
-            return false;
-        }
-
-        // we are now saving to a prepared temporary file
-        saveFile.reset(new QFile(targetFilename));
-
-        // open QTemporaryFile for write
-        if (!saveFile->open(QIODevice::WriteOnly)) {
-            return false;
-        }
-
-        if (!newFile) {
-            // set original file's permissions to temporary file (QSaveFile does this automatically)
-            saveFile->setPermissions(QFile(filename).permissions());
-        }
+    if (!saveFile.open(QIODevice::WriteOnly | QIODevice::Truncate)) {
+        return false;
     }
 
     /**
      * construct correct filter device and try to open
      */
     KCompressionDevice::CompressionType type = KFilterDev::compressionTypeForMimeType(m_mimeTypeForFilterDev);
-    KCompressionDevice file(saveFile.data(), false, type);
+    KCompressionDevice file(&saveFile, false, type);
 
     if (!file.open(QIODevice::WriteOnly)) {
         return false;
@@ -906,67 +836,38 @@ bool TextBuffer::save(const QString &filename)
     file.close();
 
     // flush file
-    if (!saveFile->flush()) {
+    if (!saveFile.flush()) {
         return false;
     }
 
-    if (usingTemporaryFile) {
-        // ensure that the file is written to disk
-        // just for temporary file (QSaveFile does this automatically in commit())
-        SecureTextBuffer::syncToDisk(saveFile->handle());
-    }
+#ifndef Q_OS_WIN
+    // ensure that the file is written to disk
+#ifdef HAVE_FDATASYNC
+    fdatasync(saveFile.handle());
+#else
+    fsync(saveFile.handle());
+#endif
+#endif
 
     // did save work?
     // only finalize if stream status == OK
-    bool ok = (stream.status() == QTextStream::Ok);
-
-    // commit changes
-    if (ok) {
-
-        if (usingTemporaryFile) {
-
-            // temporary file was used to save the file
-            // -> moving this file to original location with KAuth action
-
-            kAuthActionArgs->insert(QLatin1String("actionMode"), SecureTextBuffer::ActionMode::Move);
-            kAuthActionArgs->insert(QLatin1String("sourceFile"), saveFile->fileName());
-            kAuthActionArgs->insert(QLatin1String("targetFile"), filename);
-            kAuthActionArgs->insert(QLatin1String("ownerId"), ownerId);
-            kAuthActionArgs->insert(QLatin1String("groupId"), groupId);
-
-            // call save with elevated privileges
-            if (KTextEditor::EditorPrivate::unitTestMode()) {
-
-                // unit testing purposes only
-                ok = SecureTextBuffer::savefile(*kAuthActionArgs).succeeded();
-
-            } else {
-
-                kAuthSaveAction->setArguments(*kAuthActionArgs);
-                KAuth::ExecuteJob *job = kAuthSaveAction->execute();
-                ok = job->exec();
-
-            }
-
-        } else {
-
-            // standard save without elevated privileges
-
-            ok = static_cast<QSaveFile *>(saveFile.data())->commit();
-
-            if (ok && !newFile) {
-                // ensure correct owner
-                SecureTextBuffer::setOwner(filename, ownerId, groupId);
-            }
-
-        }
-    }
+    bool ok = (stream.status() == QTextStream::Ok) && saveFile.commit();
 
     // remember this revision as last saved if we had success!
     if (ok) {
         m_history.setLastSavedRevision();
     }
 
+#ifndef Q_OS_WIN
+    if (ok && newFile) { // QTemporaryFile sets permissions to 0600, so fixing this
+        const mode_t mask = umask(0);
+        umask(mask);
+
+        const mode_t fileMode = 0666 & ~mask;
+        chmod(QFile::encodeName(filename).constData(), fileMode);
+    }
+#endif
+
     // report CODEC + ERRORS
     BUFFER_DEBUG << "Saved file " << filename << "with codec" << m_textCodec->name() << (ok ? "without" : "with") << "errors";
 
diff --git a/src/buffer/katetextbuffer.h b/src/buffer/katetextbuffer.h
index 8ec356fe..92fddce7 100644
--- a/src/buffer/katetextbuffer.h
+++ b/src/buffer/katetextbuffer.h
@@ -71,7 +71,7 @@ public:
      * @param parent parent qobject
      * @param blockSize block size in lines the buffer should try to hold, default 64 lines
      */
-    TextBuffer(KTextEditor::DocumentPrivate *parent, int blockSize = 64, bool alwaysUseKAuth = false);
+    TextBuffer(KTextEditor::DocumentPrivate *parent, int blockSize = 64);
 
     /**
      * Destruct the text buffer
@@ -644,11 +644,6 @@ private:
      * Limit for line length, longer lines will be wrapped on load
      */
     int m_lineLengthLimit;
-
-    /**
-     * For unit-testing purposes only.
-     */
-    bool m_alwaysUseKAuthForSave;
 };
 
 }
-- 
2.12.0

Places

File 0002-Revert-KAuth-integration-in-document-saving.patch of Package ktexteditor (Revision be91179e1ae55ae455b27e4b477de0d1)

Places