Refactor setting and checking of file permissions

Create methods to explicitly set default file permissions on Unix
platforms, use these on installer created files and installation
target directory. Add unit test for introduced functions.

Remove method introduced in 46aecc23b2983c807ff2232ae9cb9651b4d2fdc2
as the same effect is achieved more efficiently by ensuring target
directory will be written with explicit permissions on initial
installation. Unlike the removed method, this will also not break if
an installed component contains owner non-writable directories.

Further simplify PackageManagerCore::directoryWritable() introduced
in 89f772f819178ee2502768c3d259d22ecb910fbe. Remove orphan unit
test for removed PackageManagerCore::subdirectoriesWritable().

This does not change permissions of files and directories extracted
for installed components.

Task-number: QTIFW-1412
Change-Id: I59698c78aceef874b1f79482bff5a618b9a1b536
Reviewed-by: Katja Marttila <katja.marttila@qt.io>

14 files changed, 187 insertions, 72 deletions

diff --git a/src/libs/installer/createdesktopentryoperation.cpp b/src/libs/installer/createdesktopentryoperation.cpp
index 17e165777..e6d3b16e7 100644
--- a/src/libs/installer/createdesktopentryoperation.cpp
+++ b/src/libs/installer/createdesktopentryoperation.cpp
@@ -137,8 +137,7 @@ bool CreateDesktopEntryOperation::performOperation()
         return false;
     }
 
-    QFile::setPermissions(filename, QFile::ReadOwner | QFile::WriteOwner | QFile::ReadUser | QFile::ReadGroup
-        | QFile::ReadOther | QFile::ExeOwner | QFile::ExeGroup | QFile::ExeOther);
+    setDefaultFilePermissions(filename, DefaultFilePermissions::Executable);
 
     QTextStream stream(&file);
     stream.setCodec("UTF-8");
diff --git a/src/libs/installer/createlocalrepositoryoperation.cpp b/src/libs/installer/createlocalrepositoryoperation.cpp
index 0eabc8c35..8f9aa1ddb 100644
--- a/src/libs/installer/createlocalrepositoryoperation.cpp
+++ b/src/libs/installer/createlocalrepositoryoperation.cpp
@@ -82,8 +82,7 @@ static void fixPermissions(const QString &repoPath)
         if (!it.fileInfo().isFile())
             continue;
 
-        if (!QFile::setPermissions(it.filePath(), QFile::ReadOwner | QFile::WriteOwner
-            | QFile::ReadUser | QFile::WriteUser | QFile::ReadGroup | QFile::ReadOther)) {
+        if (!setDefaultFilePermissions(it.filePath(), DefaultFilePermissions::NonExecutable)) {
                 throw Error(CreateLocalRepositoryOperation::tr("Cannot set permissions for file \"%1\".")
                     .arg(QDir::toNativeSeparators(it.filePath())));
         }
diff --git a/src/libs/installer/fileutils.cpp b/src/libs/installer/fileutils.cpp
index 18c5f90fd..b000247e6 100644
--- a/src/libs/installer/fileutils.cpp
+++ b/src/libs/installer/fileutils.cpp
@@ -293,6 +293,43 @@ void QInstaller::removeSystemGeneratedFiles(const QString &path)
 #endif
 }
 
+/*!
+    Sets permissions of file or directory specified by \a fileName to \c 644 or \c 755
+    based by the value of \a permissions.
+*/
+bool QInstaller::setDefaultFilePermissions(const QString &fileName, DefaultFilePermissions permissions)
+{
+    QFile file(fileName);
+    return setDefaultFilePermissions(&file, permissions);
+}
+
+/*!
+    Sets permissions of file or directory specified by \a file to \c 644 or \c 755
+    based by the value of \a permissions. This is effective only on Unix platforms
+    as \c setPermissions() does not manipulate ACLs. On Windows this does nothing
+    and always returns \c true.
+*/
+bool QInstaller::setDefaultFilePermissions(QFile *file, DefaultFilePermissions permissions)
+{
+#ifdef Q_OS_UNIX
+    if (!file->exists()) {
+        qWarning() << "Target" << file->fileName() << "does not exists.";
+        return false;
+    }
+    if (file->permissions() == static_cast<QFileDevice::Permission>(permissions))
+        return true;
+
+    if (!file->setPermissions(static_cast<QFileDevice::Permission>(permissions))) {
+        qWarning() << "Cannot set default permissions for target"
+                   << file->fileName() << ":" << file->errorString();
+        return false;
+    }
+    return true;
+#else
+    return true;
+#endif
+}
+
 void QInstaller::copyDirectoryContents(const QString &sourceDir, const QString &targetDir)
 {
     Q_ASSERT(QFileInfo(sourceDir).isDir());
diff --git a/src/libs/installer/fileutils.h b/src/libs/installer/fileutils.h
index ae08f5ff3..3607a189b 100644
--- a/src/libs/installer/fileutils.h
+++ b/src/libs/installer/fileutils.h
@@ -36,10 +36,17 @@
 
 QT_BEGIN_NAMESPACE
 class QFileInfo;
+class QFile;
 class QUrl;
 QT_END_NAMESPACE
 
 namespace QInstaller {
+
+enum DefaultFilePermissions {
+    NonExecutable = 0x6644,
+    Executable = 0x7755
+};
+
 class INSTALLER_EXPORT TempDirDeleter
 {
 public:
@@ -80,6 +87,9 @@ private:
     void INSTALLER_EXPORT removeDirectoryThreaded(const QString &path, bool ignoreErrors = false);
     void INSTALLER_EXPORT removeSystemGeneratedFiles(const QString &path);
 
+    bool INSTALLER_EXPORT setDefaultFilePermissions(const QString &fileName, DefaultFilePermissions permissions);
+    bool INSTALLER_EXPORT setDefaultFilePermissions(QFile *file, DefaultFilePermissions permissions);
+
     QString INSTALLER_EXPORT generateTemporaryFileName(const QString &templ=QString());
 
     void INSTALLER_EXPORT moveDirectoryContents(const QString &sourceDir, const QString &targetDir);
diff --git a/src/libs/installer/packagemanagercore.cpp b/src/libs/installer/packagemanagercore.cpp
index c154edf76..d3d3b5be7 100644
--- a/src/libs/installer/packagemanagercore.cpp
+++ b/src/libs/installer/packagemanagercore.cpp
@@ -1585,16 +1585,15 @@ Component *PackageManagerCore::componentByName(const QString &name, const QList<
     return nullptr;
 }
 
+/*!
+    Returns \c true if directory specified by \a path is writable by
+    the current user.
+*/
 bool PackageManagerCore::directoryWritable(const QString &path) const
 {
     return d->directoryWritable(path);
 }
 
-bool PackageManagerCore::subdirectoriesWritable(const QString &path) const
-{
-    return d->subdirectoriesWritable(path);
-}
-
 /*!
     Returns a list of components that are marked for installation. The list can
     be empty.
diff --git a/src/libs/installer/packagemanagercore.h b/src/libs/installer/packagemanagercore.h
index f4240fde0..fd74a6cf7 100644
--- a/src/libs/installer/packagemanagercore.h
+++ b/src/libs/installer/packagemanagercore.h
@@ -122,7 +122,6 @@ public:
     static Component *componentByName(const QString &name, const QList<Component *> &components);
 
     bool directoryWritable(const QString &path) const;
-    bool subdirectoriesWritable(const QString &path) const;
 
     bool fetchLocalPackagesTree();
     LocalPackagesHash localInstalledPackages();
diff --git a/src/libs/installer/packagemanagercore_p.cpp b/src/libs/installer/packagemanagercore_p.cpp
index 308bfa097..7547a126f 100644
--- a/src/libs/installer/packagemanagercore_p.cpp
+++ b/src/libs/installer/packagemanagercore_p.cpp
@@ -347,23 +347,8 @@ QString PackageManagerCorePrivate::targetDir() const
 
 bool PackageManagerCorePrivate::directoryWritable(const QString &path) const
 {
-    QTemporaryFile tempFile(path + QStringLiteral("/tempFile") + QString::number(qrand() % 1000));
-    if (!tempFile.open() || !tempFile.isWritable())
-        return false;
-    else
-        return true;
-}
-
-bool PackageManagerCorePrivate::subdirectoriesWritable(const QString &path) const
-{
-    // Iterate over target directory subdirectories for writing access
-    QDirIterator iterator(path, QDir::AllDirs | QDir::NoDotAndDotDot, QDirIterator::Subdirectories);
-    while (iterator.hasNext()) {
-        QTemporaryFile tempFile(iterator.next() + QLatin1String("/tempFile"));
-        if (!tempFile.open() || !tempFile.isWritable())
-            return false;
-    }
-    return true;
+    QTemporaryFile tempFile(path + QLatin1String("/tempFile.XXXXXX"));
+    return (tempFile.open() && tempFile.isWritable());
 }
 
 QString PackageManagerCorePrivate::configurationFileName() const
@@ -805,6 +790,7 @@ void PackageManagerCorePrivate::writeMaintenanceConfigFiles()
             : tr("Format error");
         throw Error(tr("Cannot write installer configuration to %1: %2").arg(iniPath, reason));
     }
+    setDefaultFilePermissions(iniPath, DefaultFilePermissions::NonExecutable);
 
     QFile file(targetDir() + QLatin1Char('/') + QLatin1String("network.xml"));
     if (file.open(QIODevice::WriteOnly | QIODevice::Truncate)) {
@@ -842,6 +828,7 @@ void PackageManagerCorePrivate::writeMaintenanceConfigFiles()
             writer.writeEndElement();
         writer.writeEndElement();
     }
+    setDefaultFilePermissions(&file, DefaultFilePermissions::NonExecutable);
 }
 
 void PackageManagerCorePrivate::readMaintenanceConfigFiles(const QString &targetDir)
@@ -1073,8 +1060,7 @@ void PackageManagerCorePrivate::writeMaintenanceToolBinary(QFile *const input, q
             throw Error(tr("Cannot write maintenance tool data to %1: %2").arg(dataOut.fileName(),
                 dataOut.errorString()));
         }
-        dataOut.setPermissions(dataOut.permissions() | QFile::WriteUser | QFile::ReadGroup
-            | QFile::ReadOther);
+        setDefaultFilePermissions(&dataOut, DefaultFilePermissions::NonExecutable);
     }
 
     {
@@ -1091,12 +1077,10 @@ void PackageManagerCorePrivate::writeMaintenanceToolBinary(QFile *const input, q
     }
 
     QFile mt(maintenanceToolRenamedName);
-    if (mt.setPermissions(out.permissions() | QFile::WriteUser | QFile::ReadGroup | QFile::ReadOther
-                          | QFile::ExeOther | QFile::ExeGroup | QFile::ExeUser)) {
+    if (setDefaultFilePermissions(&mt, DefaultFilePermissions::Executable))
         qDebug() << "Wrote permissions for maintenance tool.";
-    } else {
+    else
         qDebug() << "Failed to write permissions for maintenance tool.";
-    }
 
     if (out.exists() && !out.remove()) {
         qWarning() << tr("Cannot remove temporary data file \"%1\": %2")
@@ -1386,8 +1370,7 @@ void PackageManagerCorePrivate::writeMaintenanceTool(OperationList performedOper
                 throw Error(tr("Cannot write maintenance tool binary data to %1: %2")
                     .arg(file.fileName(), file.errorString()));
             }
-            file.setPermissions(file.permissions() | QFile::WriteUser | QFile::ReadGroup
-                | QFile::ReadOther);
+            setDefaultFilePermissions(&file, DefaultFilePermissions::NonExecutable);
         } catch (const Error &/*error*/) {
             if (!newBinaryWritten) {
                 newBinaryWritten = true;
@@ -1494,6 +1477,8 @@ bool PackageManagerCorePrivate::runInstaller()
             if (!performOperationThreaded(mkdirOp))
                 throw Error(mkdirOp->errorString());
         }
+        setDefaultFilePermissions(target, DefaultFilePermissions::Executable);
+
         const QString remove = m_core->value(scRemoveTargetDir);
         if (QVariant(remove).toBool())
             addPerformed(takeOwnedOperation(mkdirOp));
@@ -1643,16 +1628,10 @@ bool PackageManagerCorePrivate::runPackageUpdater()
         //to have some progress for the cleanup/write component.xml step
         ProgressCoordinator::instance()->addReservePercentagePoints(1);
 
-#if defined(Q_OS_LINUX) || defined(Q_OS_MACOS)
-        // check if we need admin rights and ask before the action happens
-        // on Linux and macOS also check target directory subdirectories
-        if (!directoryWritable(targetDir()) || !subdirectoriesWritable(targetDir()))
-            adminRightsGained = m_core->gainAdminRights();
-#else
         // check if we need admin rights and ask before the action happens
         if (!directoryWritable(targetDir()))
             adminRightsGained = m_core->gainAdminRights();
-#endif
+
         const QList<Component *> componentsToInstall = m_core->orderedComponentsToInstall();
         qDebug() << "Install size:" << componentsToInstall.size() << "components";
 
diff --git a/src/libs/installer/packagemanagercore_p.h b/src/libs/installer/packagemanagercore_p.h
index 3e8f831a3..42b1879c7 100644
--- a/src/libs/installer/packagemanagercore_p.h
+++ b/src/libs/installer/packagemanagercore_p.h
@@ -93,7 +93,6 @@ public:
     QString registerPath();
 
     bool directoryWritable(const QString &path) const;
-    bool subdirectoriesWritable(const QString &path) const;
 
     QString maintenanceToolName() const;
     QString installerBinaryPath() const;
diff --git a/src/libs/installer/utils.cpp b/src/libs/installer/utils.cpp
index e4e3212ac..dae5f8d34 100644
--- a/src/libs/installer/utils.cpp
+++ b/src/libs/installer/utils.cpp
@@ -28,6 +28,8 @@
 
 #include "utils.h"
 
+#include "fileutils.h"
+
 #include <QCoreApplication>
 #include <QDateTime>
 #include <QDir>
@@ -277,6 +279,7 @@ bool QInstaller::PlainVerboseWriterOutput::write(const QString &fileName, QIODev
     QFile output(fileName);
     if (output.open(openMode)) {
         output.write(data);
+        setDefaultFilePermissions(&output, DefaultFilePermissions::NonExecutable);
         return true;
     }
     return false;
diff --git a/src/libs/kdtools/localpackagehub.cpp b/src/libs/kdtools/localpackagehub.cpp
index 11ff36a88..14e6b5738 100644
--- a/src/libs/kdtools/localpackagehub.cpp
+++ b/src/libs/kdtools/localpackagehub.cpp
@@ -28,6 +28,7 @@
 ****************************************************************************/
 
 #include "localpackagehub.h"
+#include "fileutils.h"
 #include "globals.h"
 #include "constants.h"
 
@@ -431,6 +432,11 @@ void LocalPackageHub::writeToDisk()
 
         file.write(doc.toByteArray(4));
         file.close();
+
+        // Write permissions for installation information file
+        QInstaller::setDefaultFilePermissions(
+            &file, DefaultFilePermissions::NonExecutable);
+
         d->modified = false;
     }
 }
diff --git a/tests/auto/installer/fileutils/fileutils.pro b/tests/auto/installer/fileutils/fileutils.pro
new file mode 100644
index 000000000..eee7ec537
--- /dev/null
+++ b/tests/auto/installer/fileutils/fileutils.pro
@@ -0,0 +1,6 @@
+include(../../qttest.pri)
+
+QT -= gui
+QT += testlib
+
+SOURCES += tst_fileutils.cpp
diff --git a/tests/auto/installer/fileutils/tst_fileutils.cpp b/tests/auto/installer/fileutils/tst_fileutils.cpp
new file mode 100644
index 000000000..feb96d30c
--- /dev/null
+++ b/tests/auto/installer/fileutils/tst_fileutils.cpp
@@ -0,0 +1,107 @@
+/**************************************************************************
+**
+** Copyright (C) 2019 The Qt Company Ltd.
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of the Qt Installer Framework.
+**
+** $QT_BEGIN_LICENSE:GPL-EXCEPT$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see https://www.qt.io/terms-conditions. For further
+** information use the contact form at https://www.qt.io/contact-us.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3 as published by the Free Software
+** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT
+** included in the packaging of this file. Please review the following
+** information to ensure the GNU General Public License requirements will
+** be met: https://www.gnu.org/licenses/gpl-3.0.html.
+**
+** $QT_END_LICENSE$
+**
+**************************************************************************/
+
+#include <qinstallerglobal.h>
+#include <fileutils.h>
+
+#include <QObject>
+#include <QTest>
+#include <QFile>
+#include <QDir>
+
+using namespace QInstaller;
+
+class tst_fileutils : public QObject
+{
+    Q_OBJECT
+
+private slots:
+    void testSetDefaultFilePermissions()
+    {
+#if defined(Q_OS_WIN)
+        QVERIFY(setDefaultFilePermissions(QInstaller::generateTemporaryFileName(),
+            DefaultFilePermissions::NonExecutable));
+
+        QVERIFY(setDefaultFilePermissions(QInstaller::generateTemporaryFileName(),
+            DefaultFilePermissions::Executable));
+#elif defined(Q_OS_UNIX)
+        // Need to include the "user" flags here as they will be returned
+        // by QFile::permissions(). Same as owner permissions of the file.
+        QFlags<QFileDevice::Permission> permissions(QFileDevice::ReadOwner
+            | QFileDevice::WriteOwner | QFileDevice::ReadUser | QFileDevice::WriteUser
+            | QFileDevice::ReadGroup | QFileDevice::ReadOther);
+
+        QFlags<QFileDevice::Permission> exePermissions(permissions | QFileDevice::ExeOwner
+            | QFileDevice::ExeUser | QFileDevice::ExeGroup | QFileDevice::ExeOther);
+
+        QString fileName = QInstaller::generateTemporaryFileName();
+        QFile testFile(fileName);
+
+        const QString message = "Target \"%1\" does not exists.";
+
+        // Test non-existing file
+        QTest::ignoreMessage(QtWarningMsg, qPrintable(message.arg(fileName)));
+        QVERIFY(!setDefaultFilePermissions(fileName, DefaultFilePermissions::NonExecutable));
+
+        QTest::ignoreMessage(QtWarningMsg, qPrintable(message.arg(testFile.fileName())));
+        QVERIFY(!setDefaultFilePermissions(&testFile, DefaultFilePermissions::NonExecutable));
+
+        QVERIFY(testFile.open(QIODevice::ReadWrite));
+        QVERIFY(testFile.exists());
+        testFile.close();
+
+        // Test with file name
+        QVERIFY(setDefaultFilePermissions(fileName, DefaultFilePermissions::NonExecutable));
+        QCOMPARE(QFile().permissions(fileName), permissions);
+
+        QVERIFY(setDefaultFilePermissions(fileName, DefaultFilePermissions::Executable));
+        QCOMPARE(QFile().permissions(fileName), exePermissions);
+
+        // Test with QFile object
+        QVERIFY(setDefaultFilePermissions(&testFile, DefaultFilePermissions::NonExecutable));
+        QCOMPARE(QFile().permissions(fileName), permissions);
+
+        QVERIFY(setDefaultFilePermissions(&testFile, DefaultFilePermissions::Executable));
+        QCOMPARE(QFile().permissions(fileName), exePermissions);
+
+        // Test with directory path
+        QString testDir = QDir().tempPath() + QLatin1String("/testDir");
+        QVERIFY(QDir().mkdir(testDir));
+
+        QVERIFY(setDefaultFilePermissions(testDir, DefaultFilePermissions::Executable));
+        QCOMPARE(QFile().permissions(testDir), exePermissions);
+
+        QVERIFY(QDir().rmdir(testDir));
+        QVERIFY(testFile.remove());
+#endif
+    }
+};
+
+QTEST_MAIN(tst_fileutils)
+
+#include "tst_fileutils.moc"
diff --git a/tests/auto/installer/installer.pro b/tests/auto/installer/installer.pro
index db94a23cb..6db98d9e9 100644
--- a/tests/auto/installer/installer.pro
+++ b/tests/auto/installer/installer.pro
@@ -10,6 +10,7 @@ SUBDIRS += \
     messageboxhandler \
     extractarchiveoperationtest \
     lib7zfacade \
+    fileutils \
     unicodeexecutable \
     scriptengine \
     consumeoutputoperationtest \
diff --git a/tests/auto/installer/packagemanagercore/tst_packagemanagercore.cpp b/tests/auto/installer/packagemanagercore/tst_packagemanagercore.cpp
index 67fa7e2c5..cf5a53b8b 100644
--- a/tests/auto/installer/packagemanagercore/tst_packagemanagercore.cpp
+++ b/tests/auto/installer/packagemanagercore/tst_packagemanagercore.cpp
@@ -284,35 +284,6 @@ private slots:
 #endif
         QVERIFY(QDir().rmdir(testDirectory));
     }
-
-    void testSubdirectoriesWritable()
-    {
-        PackageManagerCore core;
-
-        const QString testDirectory = QInstaller::generateTemporaryFileName();
-        QVERIFY(QDir().mkpath(testDirectory));
-        QVERIFY(QDir(testDirectory).exists());
-
-        const QString testSubdirectory = testDirectory + "/" + QString::number(qrand() % 1000);
-
-        QVERIFY(QDir().mkpath(testSubdirectory));
-        QVERIFY(QDir(testSubdirectory).exists());
-
-        // should be writable
-        QVERIFY(core.subdirectoriesWritable(testDirectory));
-
-#if defined(Q_OS_LINUX) || defined(Q_OS_MACOS)
-        QFile dirDevice(testSubdirectory);
-        dirDevice.setPermissions(QFileDevice::ReadOwner | QFileDevice::ExeOwner);
-
-        // should not be writable
-        QVERIFY(!core.subdirectoriesWritable(testDirectory));
-
-        dirDevice.setPermissions(QFileDevice::ReadOwner | QFileDevice::WriteOwner | QFileDevice::ExeOwner);
-#endif
-        QVERIFY(QDir().rmdir(testSubdirectory));
-        QVERIFY(QDir().rmdir(testDirectory));
-    }
 };