Merge lp:~mardy/ubuntu-system-settings/rtm-lp1387734 into lp:ubuntu-system-settings/rtm-14.09

[Alberto Mardegan]

Security: ignore older requests when building the applications' grants

Also update the unit tests to ensure that we'll compute the same results no matter what the order of the rows is.

Security: ignore older requests when building the applications' grants

Also update the unit tests to ensure that we'll compute the same results no matter what the order of the rows is.

[David Barth (dbarth)]

Same as the vivid branch which was tested to work fine

review: Approve

[Sebastien Bacher (seb128)]

thanks

review: Approve

=== modified file 'plugins/security-privacy/trust-store-model.cpp'
--- plugins/security-privacy/trust-store-model.cpp	2014-09-29 11:02:13 +0000
+++ plugins/security-privacy/trust-store-model.cpp	2015-01-27 13:47:59 +0000
@@ -35,6 +35,11 @@
 class Application
 {
 public:
+    struct GrantData {
+        bool granted{false};
+        std::chrono::system_clock::time_point timestamp; // initialized with the epoch
+    };
+
     Application() {}
 
     void setId(const QString &id) {
@@ -125,19 +130,25 @@
     }
 
     void addRequest(const core::trust::Request &request) {
-        if (request.answer == core::trust::Request::Answer::granted) {
-            grantedFeatures.insert(request.feature.value);
-        } else {
-            grantedFeatures.remove(request.feature.value);
+        GrantData &data = grantedFeatures[request.feature.value];
+        /* Ignore older requests */
+        if (request.when <= data.timestamp) return;
+
+        data.granted = (request.answer == core::trust::Request::Answer::granted);
+        data.timestamp = request.when;
+    }
+
+    bool hasGrants() const {
+        Q_FOREACH(const GrantData &data, grantedFeatures) {
+            if (data.granted) return true;
         }
+        return false;
     }
 
-    bool hasGrants() const { return !grantedFeatures.isEmpty(); }
-
     QString id;
     QString displayName;
     QString iconName;
-    QSet<std::uint64_t> grantedFeatures;
+    QHash<std::uint64_t,GrantData> grantedFeatures;
 };
 
 class TrustStoreModelPrivate: public QObject
@@ -355,7 +366,7 @@
 
     /* When disabling, we must disable all the features */
     if (!enabled) {
-        Q_FOREACH(std::int64_t feature, app.grantedFeatures) {
+        Q_FOREACH(std::int64_t feature, app.grantedFeatures.keys()) {
             /* Skip the default feature, we already disabled it */
             if (feature == core::trust::Request::default_feature) continue;
 

=== modified file 'tests/plugins/security-privacy/tst_trust_store_model.cpp'
--- tests/plugins/security-privacy/tst_trust_store_model.cpp	2014-08-11 09:38:00 +0000
+++ tests/plugins/security-privacy/tst_trust_store_model.cpp	2015-01-27 13:47:59 +0000
@@ -32,11 +32,20 @@
 {
 struct Store: public core::trust::Store
 {
+    enum SortOrder {
+        Unsorted = 0,
+        TimeAsc,
+        TimeDesc,
+        LastSortOrder
+    };
+
     std::string m_name;
     std::list<core::trust::Request> m_allRequests;
+    SortOrder m_sortOrder;
     static std::shared_ptr<mock::Store> m_instance;
 
-    Store()
+    Store():
+        m_sortOrder(Unsorted)
     {
     }
 
@@ -48,6 +57,22 @@
         m_instance = store;
     }
 
+    void setSortOrder(SortOrder order) {
+        m_sortOrder = order;
+    }
+
+    static bool requestCompareTimeAsc(const core::trust::Request &left,
+                                      const core::trust::Request &right)
+    {
+        return left.when < right.when;
+    }
+
+    static bool requestCompareTimeDesc(const core::trust::Request &left,
+                                       const core::trust::Request &right)
+    {
+        return left.when > right.when;
+    }
+
     struct Query: public core::trust::Store::Query
     {
         mock::Store *m_store;
@@ -98,6 +123,12 @@
                     }
                 }
             }
+            // sort the results
+            if (m_store->m_sortOrder != Unsorted) {
+                m_requests.sort(m_store->m_sortOrder == TimeAsc ?
+                                requestCompareTimeAsc : requestCompareTimeDesc);
+            }
+
             m_it = m_requests.begin();
             m_status = (m_it == m_requests.end()) ?
                 core::trust::Store::Query::Status::eor :
@@ -263,13 +294,17 @@
     QFETCH(QStringList, expectedAppIds);
     QFETCH(QList<bool>, expectedAppGrants);
 
+    std::chrono::system_clock::time_point requestTime =
+        std::chrono::system_clock::now();
+
     for (int i = 0; i < appIds.count(); i++) {
         core::trust::Request r;
         r.from = appIds[i].toStdString();
         r.feature = core::trust::Feature(core::trust::Request::default_feature);
         r.answer = appGrants[i] ?
             core::trust::Request::Answer::granted : core::trust::Request::Answer::denied;
-        r.when = std::chrono::system_clock::now();
+        // make sure all times are monotonically increasing
+        r.when = requestTime + std::chrono::duration<int>(i);
         m_store->add(r);
     }
 
@@ -280,16 +315,22 @@
                       "  serviceName: \"storeTest\"\n"
                       "}",
                       QUrl());
-    QObject *object = component.create();
-    QVERIFY(object != 0);
-    QAbstractListModel *model = qobject_cast<QAbstractListModel*>(object);
-    QVERIFY(model != 0);
-
-    QCOMPARE(model->rowCount(), expectedAppIds.count());
-
-    for (int i = 0; i < model->rowCount(); i++) {
-        QCOMPARE(get(model, i, "applicationId").toString(), expectedAppIds[i]);
-        QCOMPARE(get(model, i, "granted").toBool(), expectedAppGrants[i]);
+    // Test unsorted, sorted asc and sorted desc
+    for (int i_order = 0; i_order < mock::Store::LastSortOrder; i_order++) {
+        m_store->setSortOrder(mock::Store::SortOrder(i_order));
+        QObject *object = component.create();
+        QVERIFY(object != 0);
+        QAbstractListModel *model = qobject_cast<QAbstractListModel*>(object);
+        QVERIFY(model != 0);
+
+        QCOMPARE(model->rowCount(), expectedAppIds.count());
+
+        for (int i = 0; i < model->rowCount(); i++) {
+            QCOMPARE(get(model, i, "applicationId").toString(), expectedAppIds[i]);
+            QCOMPARE(get(model, i, "granted").toBool(), expectedAppGrants[i]);
+        }
+
+        delete object;
     }
 }