ubuntu-system-settings

Merge lp:~dbarth/ubuntu-system-settings/location-apps into lp:ubuntu-system-settings

location-apps
Merge into trunk

Proposed by David Barth on 2014-08-28

Status:	Merged
Approved by:	Sebastien Bacher on 2014-09-05
Approved revision:	952
Merged at revision:	1001
Proposed branch:	lp:~dbarth/ubuntu-system-settings/location-apps
Merge into:	lp:ubuntu-system-settings
Diff against target:	142 lines (+63/-15) 3 files modified plugins/security-privacy/Location.qml (+17/-6) plugins/security-privacy/PageComponent.qml (+14/-4) plugins/security-privacy/trust-store-model.cpp (+32/-5)
To merge this branch:	bzr merge lp:~dbarth/ubuntu-system-settings/location-apps
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot	continuous-integration		Needs Fixing on 2014-09-05
Sebastien Bacher (community)			Approve on 2014-09-05
Alberto Mardegan		2014-08-28	Approve on 2014-09-04
Alexandre Abreu (community)			Approve on 2014-09-04
Review via email: mp+232575@code.launchpad.net

Commit message

Enable location panel, display contols for authorized applications.

Description of the change

Enable location panel, display contols for authorized applications.

Note: icon files and application names are not always working; there is a bug in the trust-store-model access code.

lp:~dbarth/ubuntu-system-settings/location-apps updated on 2014-08-28

947. By David Barth on 2014-08-28: comment

Revision history for this message

Alberto Mardegan (mardy) wrote on 2014-08-28:

I didn't test it, but looks good!

review: Approve

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-08-28:

FAILED: Continuous integration, rev:947
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~dbarth/ubuntu-system-settings/location-apps/+merge/232575/+edit-commit-message

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-system-settings-ci/1328/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Sebastien Bacher (seb128) wrote on 2014-08-29:

Thanks, I've one comment inline, also is there a bug about the icons issue? It seems like something to fix before enabling the feature, the dialog looks quite buggy without those

review: Needs Fixing

Revision history for this message

Sebastien Bacher (seb128) wrote on 2014-08-29:

(the titles are also missing, which gives 3 empty lines and 1 for camera, it's not really usable)

Revision history for this message

Sebastien Bacher (seb128) wrote on 2014-08-29:

could you also look at enabling/displaying th caption described in the design?

lp:~dbarth/ubuntu-system-settings/location-apps updated on 2014-09-01

948. By David Barth on 2014-09-01: display all text in debug mode

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-09-01:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-system-settings-ci/1355/rebuild

review: Approve (continuous-integration)

lp:~dbarth/ubuntu-system-settings/location-apps updated on 2014-09-04

949. By David Barth on 2014-09-04: filter out unconfined apps; lookup desktop file for apps without version numbers
950. By David Barth on 2014-09-04: improve rendering of the location entry

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-09-04:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-system-settings-ci/1378/rebuild

review: Approve (continuous-integration)

Revision history for this message

Alberto Mardegan (mardy) wrote on 2014-09-04:

A few inline comments, nothing major.

review: Needs Fixing

lp:~dbarth/ubuntu-system-settings/location-apps updated on 2014-09-04

951. By David Barth on 2014-09-04: cleanups

Revision history for this message

Alexandre Abreu (abreu-alexandre) on 2014-09-04:

review: Approve

Revision history for this message

Alberto Mardegan (mardy) wrote on 2014-09-04:

Looks good to me!

review: Approve

Revision history for this message

Sebastien Bacher (seb128) wrote on 2014-09-04:

David, testing the debs from the C.I run from earlier today, on a krillin device, I still get camera and 3 rows without a title nor an icon... what informations do you need to debug those empty row issues?

review: Needs Fixing

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-09-04:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-system-settings-ci/1379/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-09-04:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-system-settings-ci/1385/rebuild

review: Approve (continuous-integration)

Revision history for this message

David Barth (dbarth) wrote on 2014-09-04:

You should try the packages from the silo, ie https://launchpad.net/~ci-train-ppa-service/+archive/ubuntu/landing-017 and that package version number: 0.3+14.10.20140904.2-0ubuntu1

If the problem persists, please email me the trustdb in:
~/.local/share/UbuntuLocationService/
and all desktop files in ~/.local/share/applications

Revision history for this message

Pat McGowan (pat-mcgowan) wrote on 2014-09-04:

tested the debs in the silo

Approved location access for camera and it showed up in the list as the only entry
Approved access for the browser and it does not show up in the list
Approved access for openstreetmap and it showed in the list
I ran clock and weather but they did not prompt for access and they do not show up in the list. I even scanned for location in weather.

When I disabled the camera access I saw this message on the terminal output
2014-09-04 18:03:40,782 - WARNING - file:///usr/share/ubuntu/settings/system/qml-plugins/security-privacy/Location.qml:106: TypeError: Property 'setEnabled' of object QQmlDMAbstractItemModelData(0x91c990) is not a function

Restarted settings and camera was still enabled

Revision history for this message

David Barth (dbarth) wrote on 2014-09-05:

The applications which do not appear on the list are 'unconfined'
applications. They can't be controled as they have all privileges
potentially. If we want to control the browser, we need to fix apparmor,
trust-store and then update webbrowser-app.

The function issue is a bug, re-factoring: thanks for spotting that one.
It's fixed in the new rev.

But in geneal, the main problem is going to be that apps have the logic to
request authorizations, but may not have the right logic to react whn the
authorization is removed.

I think this is independent of the settings, as technically the trust store
db is updated to reflect the authorization status.

On Fri, Sep 5, 2014 at 12:11 AM, Pat McGowan <email address hidden>
wrote:

> tested the debs in the silo
>
> Approved location access for camera and it showed up in the list as the
> only entry
> Approved access for the browser and it does not show up in the list
> Approved access for openstreetmap and it showed in the list
> I ran clock and weather but they did not prompt for access and they do not
> show up in the list. I even scanned for location in weather.
>
> When I disabled the camera access I saw this message on the terminal output
> 2014-09-04 18:03:40,782 - WARNING -
> file:///usr/share/ubuntu/settings/system/qml-plugins/security-privacy/Location.qml:106:
> TypeError: Property 'setEnabled' of object
> QQmlDMAbstractItemModelData(0x91c990) is not a function
>
> Restarted settings and camera was still enabled
>
>
> --
>
> https://code.launchpad.net/~dbarth/ubuntu-system-settings/location-apps/+merge/232575
> You are the owner of lp:~dbarth/ubuntu-system-settings/location-apps.
>

lp:~dbarth/ubuntu-system-settings/location-apps updated on 2014-09-05

952. By David Barth on 2014-09-05: call the right model to toggle the setting

Revision history for this message

Sebastien Bacher (seb128) wrote on 2014-09-05:

Ok, looks fine to me after clearing out ~/.local/share/UbuntuLocationService, I guess I had an old configuration from previous tests there.

review: Approve

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-09-05:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-system-settings-ci/1390/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-09-05:

FAILED: Continuous integration, rev:952
http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-ci/1397/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/4529/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/3361
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-i386-ci/589
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/4311/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5781
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5781/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/12768
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/2767
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/3663
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/3663/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-system-settings-ci/1397/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-09-05:

FAILED: Continuous integration, rev:952
http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-ci/1398/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/4531/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/3363
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-system-settings-utopic-i386-ci/590
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/4312/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5783
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5783/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/12769
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/2769
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/3665
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/3665/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-system-settings-ci/1398/rebuild

review: Needs Fixing (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

David Barth

Ubuntu Touch System Settings

 === modified file 'plugins/security-privacy/Location.qml'
 --- plugins/security-privacy/Location.qml	2013-09-24 12:33:58 +0000
 +++ plugins/security-privacy/Location.qml	2014-09-05 08:59:08 +0000
@@ -23,6 +23,7 @@
  import QtQuick 2.0
  import Ubuntu.Components 0.1
  import Ubuntu.Components.ListItems 0.1 as ListItem
++import Ubuntu.SystemSettings.SecurityPrivacy 1.0
  import SystemSettings 1.0
  ItemPage {
@@ -82,21 +83,31 @@
                      return i18n.tr("Uses wi-fi (currently off), cell tower locations (no current cellular connection), and GPS to detect your rough location. Turning off location detection saves battery.")
+             }
--            visible: showAllUI
++            visible: showAllUI /* hide until the information is real */
+         }
          ListItem.Standard {
              text: i18n.tr("Allow access to location:")
--            visible: showAllUI && locationOn.checked
++            visible: locationOn.checked
++        }
++
++        TrustStoreModel {
++            id: trustStoreModel
++            serviceName: "UbuntuLocationService"
+         }
          Repeater {
--            model: ["Browser", "Camera", "Clock", "Weather"]
++            model: trustStoreModel
              ListItem.Standard {
--                text: modelData
--                control: Switch { checked: true; enabled: false}
--                visible: showAllUI && locationOn.checked
++                text: model.applicationName
++                iconSource: model.iconName
++                control: Switch {
++                    checked: model.granted
++                    onClicked: trustStoreModel.setEnabled(index, !model.granted)
++                }
++                visible: locationOn.checked
+             }
+         }
++
+     }
+ }
 === modified file 'plugins/security-privacy/PageComponent.qml'
 --- plugins/security-privacy/PageComponent.qml	2014-08-28 18:11:36 +0000
 +++ plugins/security-privacy/PageComponent.qml	2014-09-05 08:59:08 +0000
@@ -231,13 +231,23 @@
                  Component.onCompleted: start()
+             }
              ListItem.SingleValue {
++                id: locationItem
                  text: i18n.tr("Location access")
--                value: locationActionGroup.enabled.state ?
--                           i18n.tr("On") : i18n.tr("Off")
++                value: ""
                  progression: true
                  onClicked: pageStack.push(Qt.resolvedUrl("Location.qml"))
--                visible: showAllUI && // Hidden until the indicator works
--                         locationActionGroup.enabled.state !== undefined
++                visible: true
++                enabled: true
++                property variant locationEnabled
++                onLocationEnabledChanged: {
++                    value = locationEnabled ?
++                       i18n.tr("On") : i18n.tr("Off")
++                }
++            }
++            Binding {
++                target: locationItem
++                property: "locationEnabled"
++                value: locationActionGroup.enabled.state
+             }
              ListItem.SingleValue {
                  text: i18n.tr("Other app access")
 === modified file 'plugins/security-privacy/trust-store-model.cpp'
 --- plugins/security-privacy/trust-store-model.cpp	2014-08-20 12:50:09 +0000
 +++ plugins/security-privacy/trust-store-model.cpp	2014-09-05 08:59:08 +0000
@@ -39,17 +39,38 @@
      void setId(const QString &id) {
          this->id = id;
--        QString localShare =
--            QStandardPaths::writableLocation(QStandardPaths::GenericDataLocation);
--        QSettings desktopFile(QString("%1/applications/%2.desktop").
--                              arg(localShare).arg(id),
--                              QSettings::IniFormat);
++        QString desktopFilename = resolveDesktopFilename(id);
++        QSettings desktopFile(desktopFilename, QSettings::IniFormat);
          desktopFile.beginGroup("Desktop Entry");
          displayName = desktopFile.value("Name").toString();
          iconName = resolveIcon(desktopFile.value("Icon").toString(),
                                 desktopFile.value("Path").toString());
+     }
++    QString resolveDesktopFilename(const QString &id) {
++        QString localShare =
++            QStandardPaths::writableLocation(QStandardPaths::GenericDataLocation);
++        QString desktopFilename(QString("%1/applications/%2.desktop").
++                                arg(localShare).arg(id));
++        if (QFile(desktopFilename).exists())
++            return desktopFilename;
++
++        /* search the directory for a matching filename */
++        QDir dir(QString("%1/applications").arg(localShare));
++        dir.setFilter(QDir::Files);
++        QStringList fileList = dir.entryList();
++        QString pattern = QString("%1*.desktop").arg(id);
++        for (int i = 0; i < fileList.count(); i++) {
++            /* stop at the first match */
++            if (QDir::match(pattern, fileList[i])) {
++                return QString("%1/applications/%2").arg(localShare).arg(fileList[i]);
++            }
++        }
++
++        qWarning() << "No desktop file found for app id: " << id;
++        return QString();
++    }
++
      QString resolveIcon(const QString &iconName, const QString &basePath) {
          /* If iconName points to a valid file, use it */
          if (QFile::exists(iconName)) {
@@ -160,6 +181,12 @@
              QString applicationId = QString::fromStdString(r.from);
++            /* filter out unconfined apps, they can access everything anyway */
++            if (applicationId == "unconfined") {
++                query->next();
++                continue;
++            }
++
              Application &app = appMap[applicationId];
              app.setId(applicationId);
              app.addRequest(r);