diff options
author | Arttu Tarkiainen <arttu.tarkiainen@qt.io> | 2019-06-05 16:55:04 +0300 |
---|---|---|
committer | Arttu Tarkiainen <arttu.tarkiainen@qt.io> | 2019-06-14 08:10:26 +0000 |
commit | 89f772f819178ee2502768c3d259d22ecb910fbe (patch) | |
tree | 2b8abbfe061613b8f16a2af90c5e044ee526d5d4 | |
parent | 8d020f1b19b53bb265e93097bad1ee55d9df113f (diff) |
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 <RepositoryUpdate> 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 <katja.marttila@qt.io>
-rw-r--r-- | src/libs/installer/metadatajob.cpp | 20 | ||||
-rw-r--r-- | src/libs/installer/packagemanagercore.cpp | 17 | ||||
-rw-r--r-- | src/libs/installer/packagemanagercore.h | 3 | ||||
-rw-r--r-- | src/libs/installer/packagemanagercore_p.cpp | 27 | ||||
-rw-r--r-- | src/libs/installer/packagemanagercore_p.h | 3 | ||||
-rw-r--r-- | tests/auto/installer/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<FileTaskResult> &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<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 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<Component *> 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 <progresscoordinator.h> #include <QDir> +#include <QFile> #include <QTemporaryFile> #include <QTest> @@ -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)); + } }; |