dbus: do not silently ignore invalid parametersHEADdev

Requesting to stop an application with an unknown id would just result
in the appman-controller exiting with code '0', instead of complaining.

This patch fixes all those cases, where an invalid id parameter or even
a missing installer sub-system would silently be accepted on a DBus
call.

Change-Id: I1ffc8cec970a79c23fde82c7523b2a69519a5583
Pick-to: 6.10
Reviewed-by: Dominik Holland <[email protected]>

7 files changed, 122 insertions, 28 deletions

diff --git a/src/dbus-interfaces/io.qt.packagemanager.xml b/src/dbus-interfaces/io.qt.packagemanager.xml
index d40c6970..2e9d49b0 100644
--- a/src/dbus-interfaces/io.qt.packagemanager.xml
+++ b/src/dbus-interfaces/io.qt.packagemanager.xml
@@ -2,6 +2,7 @@
 <node>
   <interface name="io.qt.PackageManager">
     <property name="count" type="i" access="read"/>
+    <property name="installationEnabled" type="b" access="read"/>
     <property name="allowInstallationOfUnsignedPackages" type="b" access="read"/>
     <property name="developmentMode" type="b" access="read"/>
     <property name="hardwareId" type="s" access="read"/>
diff --git a/src/dbus-lib/dbuscontextadaptor.cpp b/src/dbus-lib/dbuscontextadaptor.cpp
index 8ee341b6..31b2f90d 100644
--- a/src/dbus-lib/dbuscontextadaptor.cpp
+++ b/src/dbus-lib/dbuscontextadaptor.cpp
@@ -27,6 +27,12 @@ QDBusContext *QtAM::DBusContextAdaptor::dbusContextFor(QDBusAbstractAdaptor *ada
     return adaptor ? qobject_cast<DBusContextAdaptor *>(adaptor->parent()) : nullptr;
 }
 
+void DBusContextAdaptor::sendErrorReply(QDBusAbstractAdaptor *adaptor, const QString &errorString)
+{
+    if (auto *ctxt = dbusContextFor(adaptor))
+        ctxt->sendErrorReply(QDBusError::Failed, errorString);
+}
+
 QT_END_NAMESPACE_AM
 
 #include "moc_dbuscontextadaptor.cpp"
diff --git a/src/dbus-lib/dbuscontextadaptor.h b/src/dbus-lib/dbuscontextadaptor.h
index 7ee2896d..2fa9ce19 100644
--- a/src/dbus-lib/dbuscontextadaptor.h
+++ b/src/dbus-lib/dbuscontextadaptor.h
@@ -26,6 +26,7 @@ public:
     bool isRegistered() const;
 
     static QDBusContext *dbusContextFor(QDBusAbstractAdaptor *adaptor);
+    static void sendErrorReply(QDBusAbstractAdaptor *adaptor, const QString &errorString);
 
     template<typename T>
     T *generatedAdaptor() { return qobject_cast<T *>(m_adaptor); }
diff --git a/src/main-lib/applicationmanageradaptor_dbus.cpp b/src/main-lib/applicationmanageradaptor_dbus.cpp
index 5802a89c..15de90f8 100644
--- a/src/main-lib/applicationmanageradaptor_dbus.cpp
+++ b/src/main-lib/applicationmanageradaptor_dbus.cpp
@@ -25,6 +25,14 @@ using namespace Qt::StringLiterals;
 
 QT_USE_NAMESPACE_AM
 
+static bool checkApplicationId(ApplicationManagerAdaptor *a, const QString &id)
+{
+    if (ApplicationManager::instance()->application(id))
+        return true;
+    DBusContextAdaptor::sendErrorReply(a, u"Unknown application id '%1'"_s.arg(id));
+    return false;
+}
+
 ApplicationManagerAdaptor::ApplicationManagerAdaptor(QObject *parent)
     : QDBusAbstractAdaptor(parent)
 {
@@ -92,13 +100,19 @@ QStringList ApplicationManagerAdaptor::applicationIds()
 uint ApplicationManagerAdaptor::applicationRunState(const QString &id)
 {
     QT_AM_AUTHENTICATE_DBUS(uint)
-    return ApplicationManager::instance()->applicationRunState(id);
+    if (checkApplicationId(this, id))
+        return ApplicationManager::instance()->applicationRunState(id);
+    else
+        return { };
 }
 
 QStringList ApplicationManagerAdaptor::capabilities(const QString &id)
 {
     QT_AM_AUTHENTICATE_DBUS(QStringList)
-    return ApplicationManager::instance()->capabilities(id);
+    if (checkApplicationId(this, id))
+        return ApplicationManager::instance()->capabilities(id);
+    else
+        return { };
 }
 
 bool ApplicationManagerAdaptor::debugApplication(const QString &id, const QString &debugWrapper)
@@ -139,8 +153,7 @@ bool ApplicationManagerAdaptor::debugApplication(const QString &id, const QStrin
                                             std::move(stdioRedirections));
     } catch (const Exception &e) {
         qCWarning(LogSystem) << e.what();
-        if (auto *ctxt = DBusContextAdaptor::dbusContextFor(this))
-            ctxt->sendErrorReply(QDBusError::Failed, e.errorString());
+        DBusContextAdaptor::sendErrorReply(this, e.errorString());
         return false;
     }
 }
@@ -148,10 +161,14 @@ bool ApplicationManagerAdaptor::debugApplication(const QString &id, const QStrin
 QVariantMap ApplicationManagerAdaptor::get(const QString &id)
 {
     QT_AM_AUTHENTICATE_DBUS(QVariantMap)
-    auto map = ApplicationManager::instance()->get(id);
-    map.remove(u"application"_s);       // cannot marshall QObject *
-    map.remove(u"applicationObject"_s); // cannot marshall QObject *
-    return convertToDBusVariant(map).toMap();
+    if (checkApplicationId(this, id)) {
+        auto map = ApplicationManager::instance()->get(id);
+        map.remove(u"application"_s);       // cannot marshall QObject *
+        map.remove(u"applicationObject"_s); // cannot marshall QObject *
+        return convertToDBusVariant(map).toMap();
+    } else {
+        return { };
+    }
 }
 
 QString ApplicationManagerAdaptor::identifyApplication(qlonglong pid)
@@ -210,8 +227,7 @@ bool ApplicationManagerAdaptor::startApplication(const QString &id, const QtAM::
                                             std::move(stdioRedirections));
     } catch (const Exception &e) {
         qCWarning(LogSystem) << e.what();
-        if (auto *ctxt = DBusContextAdaptor::dbusContextFor(this))
-            ctxt->sendErrorReply(QDBusError::Failed, e.errorString());
+        DBusContextAdaptor::sendErrorReply(this, e.errorString());
         return false;
     }
 }
@@ -235,7 +251,8 @@ void ApplicationManagerAdaptor::stopApplication(const QString &id)
 void ApplicationManagerAdaptor::stopApplication(const QString &id, bool forceKill)
 {
     QT_AM_AUTHENTICATE_DBUS(void)
-    ApplicationManager::instance()->stopApplication(id, forceKill);
+    if (checkApplicationId(this, id))
+        ApplicationManager::instance()->stopApplication(id, forceKill);
 }
 
 QString ApplicationManagerAdaptor::sendIntentRequestAs(const QString &requestingApplicationId,
diff --git a/src/main-lib/packagemanageradaptor_dbus.cpp b/src/main-lib/packagemanageradaptor_dbus.cpp
index 7ed62f6c..e39a837d 100644
--- a/src/main-lib/packagemanageradaptor_dbus.cpp
+++ b/src/main-lib/packagemanageradaptor_dbus.cpp
@@ -20,6 +20,22 @@ using namespace Qt::StringLiterals;
 
 QT_USE_NAMESPACE_AM
 
+static bool checkPackageId(PackageManagerAdaptor *a, const QString &id)
+{
+    if (PackageManager::instance()->package(id))
+        return true;
+    DBusContextAdaptor::sendErrorReply(a, u"Unknown package id '%1'"_s.arg(id));
+    return false;
+}
+
+static bool checkInstaller(PackageManagerAdaptor *a)
+{
+    if (PackageManager::instance()->installationEnabled())
+        return true;
+    DBusContextAdaptor::sendErrorReply(a, u"The application-manager was compiled without the installer part"_s);
+    return false;
+}
+
 static QString taskStateToString(AsynchronousTask::TaskState state)
 {
     const char *cstr = QMetaEnum::fromType<AsynchronousTask::TaskState>().valueToKey(state);
@@ -84,6 +100,11 @@ PackageManagerAdaptor::PackageManagerAdaptor(QObject *parent)
 PackageManagerAdaptor::~PackageManagerAdaptor()
 { }
 
+bool PackageManagerAdaptor::installationEnabled() const
+{
+    return PackageManager::instance()->installationEnabled();
+}
+
 bool PackageManagerAdaptor::allowInstallationOfUnsignedPackages() const
 {
     return PackageManager::instance()->allowInstallationOfUnsignedPackages();
@@ -127,13 +148,17 @@ QVariantMap PackageManagerAdaptor::documentLocation() const
 void PackageManagerAdaptor::acknowledgePackageInstallation(const QString &taskId)
 {
     QT_AM_AUTHENTICATE_DBUS(void)
-    PackageManager::instance()->acknowledgePackageInstallation(taskId);
+    if (checkInstaller(this))
+        PackageManager::instance()->acknowledgePackageInstallation(taskId);
 }
 
 bool PackageManagerAdaptor::cancelTask(const QString &taskId)
 {
     QT_AM_AUTHENTICATE_DBUS(bool)
-    return PackageManager::instance()->cancelTask(taskId);
+    if (checkInstaller(this))
+        return PackageManager::instance()->cancelTask(taskId);
+    else
+        return { };
 }
 
 int PackageManagerAdaptor::compareVersions(const QString &version1, const QString &version2)
@@ -151,30 +176,45 @@ QStringList PackageManagerAdaptor::packageIds()
 QVariantMap PackageManagerAdaptor::get(const QString &id)
 {
     QT_AM_AUTHENTICATE_DBUS(QVariantMap)
-    auto map = PackageManager::instance()->get(id);
-    map.remove(u"package"_s);       // cannot marshall QObject *
-    map.remove(u"packageObject"_s); // cannot marshall QObject *
-    return convertToDBusVariant(map).toMap();
+    if (checkPackageId(this, id)) {
+        auto map = PackageManager::instance()->get(id);
+        map.remove(u"package"_s);       // cannot marshall QObject *
+        map.remove(u"packageObject"_s); // cannot marshall QObject *
+        return convertToDBusVariant(map).toMap();
+    } else {
+        return { };
+    }
 }
 
 qlonglong PackageManagerAdaptor::installedPackageSize(const QString &packageId)
 {
     QT_AM_AUTHENTICATE_DBUS(qlonglong)
-    return PackageManager::instance()->installedPackageSize(packageId);
+    if (checkInstaller(this) && checkPackageId(this, packageId))
+        return PackageManager::instance()->installedPackageSize(packageId);
+    else
+        return { };
 }
 
 QVariantMap PackageManagerAdaptor::installedPackageExtraMetaData(const QString &packageId)
 {
     QT_AM_AUTHENTICATE_DBUS(QVariantMap)
-    const auto map = PackageManager::instance()->installedPackageExtraMetaData(packageId);
-    return convertToDBusVariant(map).toMap();
+    if (checkInstaller(this) && checkPackageId(this, packageId)) {
+        const auto map = PackageManager::instance()->installedPackageExtraMetaData(packageId);
+        return convertToDBusVariant(map).toMap();
+    } else {
+        return { };
+    }
 }
 
 QVariantMap PackageManagerAdaptor::installedPackageExtraSignedMetaData(const QString &packageId)
 {
     QT_AM_AUTHENTICATE_DBUS(QVariantMap)
-    const auto map = PackageManager::instance()->installedPackageExtraSignedMetaData(packageId);
-    return convertToDBusVariant(map).toMap();
+    if (checkInstaller(this) && checkPackageId(this, packageId)) {
+        const auto map = PackageManager::instance()->installedPackageExtraSignedMetaData(packageId);
+        return convertToDBusVariant(map).toMap();
+    } else {
+        return { };
+    }
 }
 
 QString PackageManagerAdaptor::removePackage(const QString &packageId, bool keepDocuments)
@@ -185,31 +225,46 @@ QString PackageManagerAdaptor::removePackage(const QString &packageId, bool keep
 QString PackageManagerAdaptor::removePackage(const QString &packageId, bool keepDocuments, bool force)
 {
     QT_AM_AUTHENTICATE_DBUS(QString)
-    return PackageManager::instance()->removePackage(packageId, keepDocuments, force);
+    if (checkInstaller(this) && checkPackageId(this, packageId))
+        return PackageManager::instance()->removePackage(packageId, keepDocuments, force);
+    else
+        return { };
 }
 
 QString PackageManagerAdaptor::startPackageInstallation(const QString &sourceUrl)
 {
     QT_AM_AUTHENTICATE_DBUS(QString)
-    return PackageManager::instance()->startPackageInstallation(sourceUrl);
+    if (checkInstaller(this))
+        return PackageManager::instance()->startPackageInstallation(sourceUrl);
+    else
+        return { };
 }
 
 QString PackageManagerAdaptor::taskState(const QString &taskId)
 {
     QT_AM_AUTHENTICATE_DBUS(QString)
-    return taskStateToString(PackageManager::instance()->taskState(taskId));
+    if (checkInstaller(this))
+        return taskStateToString(PackageManager::instance()->taskState(taskId));
+    else
+        return { };
 }
 
 QString PackageManagerAdaptor::taskPackageId(const QString &taskId)
 {
     QT_AM_AUTHENTICATE_DBUS(QString)
-    return PackageManager::instance()->taskPackageId(taskId);
+    if (checkInstaller(this))
+        return PackageManager::instance()->taskPackageId(taskId);
+    else
+        return { };
 }
 
 QStringList PackageManagerAdaptor::activeTaskIds()
 {
     QT_AM_AUTHENTICATE_DBUS(QStringList)
-    return PackageManager::instance()->activeTaskIds();
+    if (checkInstaller(this))
+        return PackageManager::instance()->activeTaskIds();
+    else
+        return { };
 }
 
 bool PackageManagerAdaptor::validateDnsName(const QString &name)
diff --git a/src/manager-lib/packagemanager.cpp b/src/manager-lib/packagemanager.cpp
index e8d1bfc5..b839f6a4 100644
--- a/src/manager-lib/packagemanager.cpp
+++ b/src/manager-lib/packagemanager.cpp
@@ -682,6 +682,18 @@ bool PackageManager::isReady() const
 }
 
 /*!
+    \qmlproperty bool PackageManager::installationEnabled
+    \readonly
+
+    Defaults to \c true. Only returns \c false, if the installer part of the application manager
+    has been \l{config disable installer}{disabled at compile time}.
+*/
+bool PackageManager::installationEnabled() const
+{
+    return d->enableInstaller;
+}
+
+/*!
     \qmlproperty bool PackageManager::developmentMode
     \readonly
 
@@ -699,7 +711,7 @@ void PackageManager::setDevelopmentMode(bool enable)
 }
 
 /*!
-    \qmlproperty string PackageManager::allowInstallationOfUnsignedPackages
+    \qmlproperty bool PackageManager::allowInstallationOfUnsignedPackages
     \readonly
 
     This readonly property reflects the \l{allow-unsigned-packages}{\c allowUnsignedPackages}
diff --git a/src/manager-lib/packagemanager.h b/src/manager-lib/packagemanager.h
index 82f619e0..ee8525d9 100644
--- a/src/manager-lib/packagemanager.h
+++ b/src/manager-lib/packagemanager.h
@@ -49,6 +49,7 @@ class PackageManager : public QAbstractListModel
     Q_CLASSINFO("D-Bus Interface", "io.qt.PackageManager")
     Q_PROPERTY(int count READ count NOTIFY countChanged FINAL)
     // these are const on purpose - these should never change in a running system
+    Q_PROPERTY(bool installationEnabled READ installationEnabled CONSTANT FINAL REVISION(6, 10))
     Q_PROPERTY(bool allowInstallationOfUnsignedPackages READ allowInstallationOfUnsignedPackages CONSTANT FINAL)
     Q_PROPERTY(bool developmentMode READ developmentMode CONSTANT FINAL)
     Q_PROPERTY(QString hardwareId READ hardwareId CONSTANT FINAL)
@@ -94,6 +95,7 @@ public:
 
     bool isReady() const;
 
+    bool installationEnabled() const;
     bool developmentMode() const;
     void setDevelopmentMode(bool enable);
     bool allowInstallationOfUnsignedPackages() const;