Merge lp:~mardy/ubuntu-system-settings-online-accounts/lp1376445 into lp:~online-accounts/ubuntu-system-settings-online-accounts/master

[Alberto Mardegan]

Handle requests from unconfined applications

When an unconfined process is requesting access for a specific application ID, the profile to be added to the ACL must be the one find in the .application file beloging to t
he given application ID, and not "unconfined.
Add a test case to catch regressions.

Handle requests from unconfined applications

When an unconfined process is requesting access for a specific application ID, the profile to be added to the ACL must be the one find in the .application file beloging to t
he given application ID, and not "unconfined.
Add a test case to catch regressions.

[Alexandre Abreu (abreu-alexandre)]

seem comment inline

review: Needs Information

[lp:~mardy/ubuntu-system-settings-online-accounts/lp1376445]

209. By Alberto Mardegan on 2014-11-07

Make sure we don't use an empty profile

[Alberto Mardegan (mardy)]

> seem comment inline

Good catch! I think it's now fixed, and I've added a comment explaining what we are doing there, and one more test for this case.

[Alexandre Abreu (abreu-alexandre)]

Looks good now

review: Approve

=== modified file 'plugins/OnlineAccountsPlugin/application-manager.cpp'
--- plugins/OnlineAccountsPlugin/application-manager.cpp	2014-10-06 09:33:22 +0000
+++ plugins/OnlineAccountsPlugin/application-manager.cpp	2014-11-07 08:34:30 +0000
@@ -158,7 +158,17 @@
     app.insert(QStringLiteral("id"), applicationId);
     app.insert(QStringLiteral("displayName"), application.displayName());
     app.insert(QStringLiteral("icon"), application.iconName());
-    app.insert(QStringLiteral("profile"), profile);
+    /* The applicationMatchesProfile() test above ensures that either the peer
+     * is unconfined, or the profile in the .application file matches the one
+     * we see from our peer.
+     * In the first case, what we really want is the profile from the
+     * .application file (if that's set), to cover the case where an unconfined
+     * process is asking authorization on behalf of a confined app. */
+    QString targetProfile = d->applicationProfile(application.name());
+    if (targetProfile.isEmpty()) {
+        targetProfile = profile;
+    }
+    app.insert(QStringLiteral("profile"), targetProfile);
 
     /* List all the services supported by this application */
     QVariantList serviceIds;

=== modified file 'tests/plugin/tst_application_manager.cpp'
--- tests/plugin/tst_application_manager.cpp	2014-10-06 09:33:22 +0000
+++ tests/plugin/tst_application_manager.cpp	2014-11-07 08:34:30 +0000
@@ -190,7 +190,8 @@
 {
     QTest::addColumn<QString>("applicationId");
     QTest::addColumn<QString>("contents");
-    QTest::addColumn<QString>("profile");
+    QTest::addColumn<QString>("inputProfile");
+    QTest::addColumn<QString>("expectedProfile");
     QTest::addColumn<QStringList>("services");
 
     QTest::newRow("no-services") <<
@@ -201,6 +202,7 @@
         "  <profile>com.ubuntu.test_MyApp_0.2</profile>\n"
         "</application>" <<
         "com.ubuntu.test_MyApp_0.2" <<
+        "com.ubuntu.test_MyApp_0.2" <<
         QStringList();
 
     QTest::newRow("no-valid-services") <<
@@ -214,6 +216,7 @@
         "  <profile>com.ubuntu.test_MyApp2_0.2</profile>\n"
         "</application>" <<
         "com.ubuntu.test_MyApp2_0.2" <<
+        "com.ubuntu.test_MyApp2_0.2" <<
         QStringList();
 
     QTest::newRow("email-services") <<
@@ -229,6 +232,7 @@
         "  <profile>com.ubuntu.test_MyApp3_0.2</profile>\n"
         "</application>" <<
         "com.ubuntu.test_MyApp3_0.2" <<
+        "com.ubuntu.test_MyApp3_0.2" <<
         (QStringList() << "cool-mail" << "bad-mail");
 
     QTest::newRow("cool-services") <<
@@ -247,7 +251,39 @@
         "  <profile>com.ubuntu.test_MyApp4_0.2</profile>\n"
         "</application>" <<
         "com.ubuntu.test_MyApp4_0.2" <<
+        "com.ubuntu.test_MyApp4_0.2" <<
         (QStringList() << "cool-mail" << "cool-sharing");
+
+    QTest::newRow("unconfined app") <<
+        "com.ubuntu.test_UnconfinedApp" <<
+        "<?xml version=\"1.0\" encoding=\"UTF-8\" ?>\n"
+        "<application id=\"com.ubuntu.test_UnconfinedApp\">\n"
+        "  <description>My application 5</description>\n"
+        "  <services>\n"
+        "    <service id=\"cool-mail\">\n"
+        "      <description>Send email</description>\n"
+        "    </service>\n"
+        "  </services>\n"
+        "  <profile>com.ubuntu.test_UnconfinedApp_0.2</profile>\n"
+        "</application>" <<
+        "unconfined" <<
+        "com.ubuntu.test_UnconfinedApp_0.2" <<
+        (QStringList() << "cool-mail");
+
+    QTest::newRow("unconfined app, no profile") <<
+        "com.ubuntu.test_UnconfinedApp2" <<
+        "<?xml version=\"1.0\" encoding=\"UTF-8\" ?>\n"
+        "<application id=\"com.ubuntu.test_UnconfinedApp2\">\n"
+        "  <description>My application 6</description>\n"
+        "  <services>\n"
+        "    <service id=\"cool-mail\">\n"
+        "      <description>Send email</description>\n"
+        "    </service>\n"
+        "  </services>\n"
+        "</application>" <<
+        "unconfined" <<
+        "unconfined" <<
+        (QStringList() << "cool-mail");
 }
 
 void ApplicationManagerTest::testApplicationInfo()
@@ -256,16 +292,17 @@
 
     QFETCH(QString, applicationId);
     QFETCH(QString, contents);
-    QFETCH(QString, profile);
+    QFETCH(QString, inputProfile);
+    QFETCH(QString, expectedProfile);
     QFETCH(QStringList, services);
 
     writeAccountsFile(applicationId + ".application", contents);
 
     ApplicationManager manager;
 
-    QVariantMap info = manager.applicationInfo(applicationId, profile);
+    QVariantMap info = manager.applicationInfo(applicationId, inputProfile);
     QCOMPARE(info.value("id").toString(), applicationId);
-    QCOMPARE(info.value("profile").toString(), profile);
+    QCOMPARE(info.value("profile").toString(), expectedProfile);
     QCOMPARE(info.value("services").toStringList().toSet(), services.toSet());
 }