From 89f772f819178ee2502768c3d259d22ecb910fbe Mon Sep 17 00:00:00 2001 From: Arttu Tarkiainen Date: Wed, 5 Jun 2019 16:55:04 +0300 Subject: Gain admin rights for writing MaintenanceTool config files if needed Running MaintenanceTool from an elevated directory didn't do proper checks for file writing rights when parsing Updates.xml from a repository, for example when a default repository contains actions inside tags the MaintenanceTool couldn't write changes to the .ini file immediately. Make MaintenanceTool do proper rights checking when parsing Updates.xml. Create uniform methods for checking directory writing access as it is used on several occasions. Add tests for directory checking methods. Task-number: QTIFW-1363 Change-Id: I6d634ddbd59429e5b98af1d8010e70a71de40f69 Reviewed-by: Katja Marttila --- src/libs/installer/metadatajob.cpp | 20 +++++++- src/libs/installer/packagemanagercore.cpp | 17 +++++-- src/libs/installer/packagemanagercore.h | 3 ++ src/libs/installer/packagemanagercore_p.cpp | 27 ++++++----- src/libs/installer/packagemanagercore_p.h | 3 +- .../packagemanagercore/tst_packagemanagercore.cpp | 53 ++++++++++++++++++++++ 6 files changed, 104 insertions(+), 19 deletions(-) diff --git a/src/libs/installer/metadatajob.cpp b/src/libs/installer/metadatajob.cpp index c69fda3d0..ecdf1cd31 100644 --- a/src/libs/installer/metadatajob.cpp +++ b/src/libs/installer/metadatajob.cpp @@ -328,8 +328,16 @@ void MetadataJob::xmlTaskFinished() if (s.updateDefaultRepositories(update) == Settings::UpdatesApplied || s.updateUserRepositories(update) == Settings::UpdatesApplied) { - if (m_core->isMaintainer()) + if (m_core->isMaintainer()) { + bool gainedAdminRights = false; + if (!m_core->directoryWritable(m_core->value(scTargetDir))) { + m_core->gainAdminRights(); + gainedAdminRights = true; + } m_core->writeMaintenanceConfigFiles(); + if (gainedAdminRights) + m_core->dropAdminRights(); + } } } status = XmlDownloadRetry; @@ -716,8 +724,16 @@ MetadataJob::Status MetadataJob::parseUpdatesXml(const QList &re return XmlDownloadRetry; } } else if (s.updateDefaultRepositories(repositoryUpdates) == Settings::UpdatesApplied) { - if (m_core->isMaintainer()) + if (m_core->isMaintainer()) { + bool gainedAdminRights = false; + if (!m_core->directoryWritable(m_core->value(scTargetDir))) { + m_core->gainAdminRights(); + gainedAdminRights = true; + } m_core->writeMaintenanceConfigFiles(); + if (gainedAdminRights) + m_core->dropAdminRights(); + } m_metaFromDefaultRepositories.clear(); QFile::remove(result.target()); return XmlDownloadRetry; diff --git a/src/libs/installer/packagemanagercore.cpp b/src/libs/installer/packagemanagercore.cpp index 002604262..f23afd76e 100644 --- a/src/libs/installer/packagemanagercore.cpp +++ b/src/libs/installer/packagemanagercore.cpp @@ -422,9 +422,7 @@ void PackageManagerCore::writeMaintenanceTool() d->writeMaintenanceTool(d->m_performedOperationsOld + d->m_performedOperationsCurrentSession); bool gainedAdminRights = false; - QTemporaryFile tempAdminFile(d->targetDir() - + QLatin1String("/testjsfdjlkdsjflkdsjfldsjlfds") + QString::number(qrand() % 1000)); - if (!tempAdminFile.open() || !tempAdminFile.isWritable()) { + if (!directoryWritable(d->targetDir())) { gainAdminRights(); gainedAdminRights = true; } @@ -1111,8 +1109,7 @@ void PackageManagerCore::networkSettingsChanged() if (isMaintainer() ) { bool gainedAdminRights = false; - QTemporaryFile tempAdminFile(d->targetDir() + QStringLiteral("/XXXXXX")); - if (!tempAdminFile.open() || !tempAdminFile.isWritable()) { + if (!directoryWritable(d->targetDir())) { gainAdminRights(); gainedAdminRights = true; } @@ -1580,6 +1577,16 @@ Component *PackageManagerCore::componentByName(const QString &name, const QList< return nullptr; } +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 4fa2f2554..809994cfe 100644 --- a/src/libs/installer/packagemanagercore.h +++ b/src/libs/installer/packagemanagercore.h @@ -121,6 +121,9 @@ public: static Component *componentByName(const QString &name, const QList &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 47e1dadb4..1526d8bd0 100644 --- a/src/libs/installer/packagemanagercore_p.cpp +++ b/src/libs/installer/packagemanagercore_p.cpp @@ -345,10 +345,19 @@ QString PackageManagerCorePrivate::targetDir() const return m_core->value(scTargetDir); } -bool PackageManagerCorePrivate::targetSubDirsWritable() +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(targetDir(), QDir::AllDirs | QDir::NoDotAndDotDot, QDirIterator::Subdirectories); + QDirIterator iterator(path, QDir::AllDirs | QDir::NoDotAndDotDot, QDirIterator::Subdirectories); while (iterator.hasNext()) { QTemporaryFile tempFile(iterator.next() + QLatin1String("/tempFile")); if (!tempFile.open() || !tempFile.isWritable()) @@ -1156,9 +1165,7 @@ void PackageManagerCorePrivate::writeMaintenanceToolBinaryData(QFileDevice *outp void PackageManagerCorePrivate::writeMaintenanceTool(OperationList performedOperations) { bool gainedAdminRights = false; - QTemporaryFile tempAdminFile(targetDir() + QLatin1String("/testjsfdjlkdsjflkdsjfldsjlfds") - + QString::number(qrand() % 1000)); - if (!tempAdminFile.open() || !tempAdminFile.isWritable()) { + if (!directoryWritable(targetDir())) { m_core->gainAdminRights(); gainedAdminRights = true; } @@ -1467,8 +1474,7 @@ bool PackageManagerCorePrivate::runInstaller() } } } else if (QDir(target).exists()) { - QTemporaryFile tempAdminFile(target + QLatin1String("/adminrights")); - if (!tempAdminFile.open() || !tempAdminFile.isWritable()) + if (!directoryWritable(targetDir())) adminRightsGained = m_core->gainAdminRights(); } @@ -1637,11 +1643,11 @@ bool PackageManagerCorePrivate::runPackageUpdater() #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 (!QTemporaryFile(targetDir() + QStringLiteral("/XXXXXX")).open() || !targetSubDirsWritable()) + if (!directoryWritable(targetDir()) || !subdirectoriesWritable(targetDir())) adminRightsGained = m_core->gainAdminRights(); #else // check if we need admin rights and ask before the action happens - if (!QTemporaryFile(targetDir() + QStringLiteral("/XXXXXX")).open()) + if (!directoryWritable(targetDir())) adminRightsGained = m_core->gainAdminRights(); #endif const QList componentsToInstall = m_core->orderedComponentsToInstall(); @@ -1810,8 +1816,7 @@ bool PackageManagerCorePrivate::runUninstaller() setStatus(PackageManagerCore::Running); // check if we need to run elevated and ask before the action happens - QTemporaryFile tempAdminFile(targetDir() + QLatin1String("/adminrights")); - if (!tempAdminFile.open() || !tempAdminFile.isWritable()) + if (!directoryWritable(targetDir())) adminRightsGained = m_core->gainAdminRights(); OperationList undoOperations = m_performedOperationsOld; diff --git a/src/libs/installer/packagemanagercore_p.h b/src/libs/installer/packagemanagercore_p.h index 3e3f15ab8..3e8f831a3 100644 --- a/src/libs/installer/packagemanagercore_p.h +++ b/src/libs/installer/packagemanagercore_p.h @@ -92,7 +92,8 @@ public: QString targetDir() const; QString registerPath(); - bool targetSubDirsWritable(); + bool directoryWritable(const QString &path) const; + bool subdirectoriesWritable(const QString &path) const; QString maintenanceToolName() const; QString installerBinaryPath() const; diff --git a/tests/auto/installer/packagemanagercore/tst_packagemanagercore.cpp b/tests/auto/installer/packagemanagercore/tst_packagemanagercore.cpp index 47fb41bf1..475c4d8b4 100644 --- a/tests/auto/installer/packagemanagercore/tst_packagemanagercore.cpp +++ b/tests/auto/installer/packagemanagercore/tst_packagemanagercore.cpp @@ -34,6 +34,7 @@ #include #include +#include #include #include @@ -257,6 +258,58 @@ private slots: core.calculateComponentsToInstall(); QCOMPARE(core.requiredDiskSpace(), 250ULL); } + + void testDirectoryWritable() + { + PackageManagerCore core; + + const QString testDirectory = QInstaller::generateTemporaryFileName(); + QVERIFY(QDir().mkpath(testDirectory)); + QVERIFY(QDir(testDirectory).exists()); + + // should be writable + QVERIFY(core.directoryWritable(testDirectory)); + +#if defined(Q_OS_LINUX) || defined(Q_OS_MACOS) + QFile dirDevice(testDirectory); + dirDevice.setPermissions(QFileDevice::ReadOwner | QFileDevice::ExeOwner); + + // should not be writable + QVERIFY(!core.directoryWritable(testDirectory)); + + dirDevice.setPermissions(QFileDevice::ReadOwner | QFileDevice::WriteOwner | QFileDevice::ExeOwner); +#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)); + } }; -- cgit v1.2.3