Merge lp:~roadmr/checkbox/1097816-pkexec into lp:checkbox

[Daniel Manrique]

This removes the deprecated gksu privileged execution mechanism, and replaces it with pkexec, along with a policy file so the backend can launch graphical programs and ask for proper authorization.

This removes the deprecated gksu privileged execution mechanism, and replaces it with pkexec, along with a policy file so the backend can launch graphical programs and ask for proper authorization.

An instance of sudo (not gksudo) is kept as a desperate measure in case pkexec is not present.

Note that if you want to test this from a source tree, the policy file needs to be copied to /usr/share/polkit-1/actions, and edited so the path points to the source tree's location.

Then run checkbox specifying a full path for CHECKBOX_SHARE, like:

CHECKBOX_SHARE=`pwd` bin/checkbox-qt

this is because full paths are needed for things to work correctly. Optionally, just copy the policy file to the above location, and copy the updated plugins/backend_info.py to /usr/share/checkbox/plugins.

An important part of this set of changes is changing the policykit dependency and installing the policy file, for this a package can be built and installed in a VM. I tested this and it works, although I tested only on a raring VM. This mechanism would need to work on Precise, Quantal, Raring and Saucy, though I'm confident it will as all the underlying components are present in those.

Finally, this could potentially need a change in our preseeds for certification, to avoid asking for the password when it's run. I'm thinking this may not be so necessary anymore, since plainbox (which already has sensible policy packages for this) is used for automated (SRU) testing, and on a manual certification run inputting the password may not be such a bother. If needed, however, the preseeds can simply be modified with a s/auth_admin/yes/ on the policy file that checkbox will now install.

[Zygmunt Krynicki (zyga)]

+1

review: Approve

[Zygmunt Krynicki (zyga)]

BTW: I'd love if you could put that awesome branch description into your commit messages! That way it's not lost and it's really the quality that we are after!

[Daniel Manrique (roadmr)]

Hi, I was under the assumption that the commit message has to be short-ish since it'll be in bzr/git. I can try putting a short one-line description and then the long explanation, in the commit message as well. I'll do this in future merge requests.

[Zygmunt Krynicki (zyga)]

The first line needs to be short

The rest can be as long as you want!

On Tue, Jul 2, 2013 at 4:18 PM, Daniel Manrique <
<email address hidden>> wrote:

> Hi, I was under the assumption that the commit message has to be short-ish
> since it'll be in bzr/git. I can try putting a short one-line description
> and then the long explanation, in the commit message as well. I'll do this
> in future merge requests.
> --
> https://code.launchpad.net/~roadmr/checkbox/1097816-pkexec/+merge/171930
> You are reviewing the proposed merge of lp:~roadmr/checkbox/1097816-pkexec
> into lp:checkbox.
>

=== modified file 'checkbox-old/debian/checkbox.install'
--- checkbox-old/debian/checkbox.install	2013-06-21 20:43:39 +0000
+++ checkbox-old/debian/checkbox.install	2013-06-27 23:07:33 +0000
@@ -7,6 +7,7 @@
 usr/share/checkbox/data/*
 usr/share/checkbox/examples/checkbox.ini
 usr/share/checkbox/examples/network.cfg
+usr/share/checkbox/examples/org.freedesktop.policykit.checkbox.policy usr/share/polkit-1/actions/
 usr/share/checkbox/install/*
 usr/share/checkbox/jobs/*
 usr/share/checkbox/patches/*

=== modified file 'checkbox-old/debian/control'
--- checkbox-old/debian/control	2013-06-21 20:43:39 +0000
+++ checkbox-old/debian/control	2013-06-27 23:07:33 +0000
@@ -11,34 +11,34 @@
                python,
                python-distutils-extra,
                python-setuptools,
+               python3-setuptools,
                python3-all,
                python3-distutils-extra,
                python3-gi,
                python3-lxml,
                python3-mock,
                python3-pkg-resources,
-               python3-setuptools,
                qt4-qmake
-Vcs-Bzr: https://code.launchpad.net/~hardware-certification/checkbox/trunk
+Vcs-Bzr: https://code.launchpad.net/~checkbox-dev/checkbox/trunk
 
 Package: checkbox
 Section: python
 Architecture: any
 Depends: debconf,
-         gir1.2-gudev-1.0,
          python3-lxml,
+         policykit-1,
          udev,
+         gir1.2-gudev-1.0,
          udisks2 | udisks,
          ${misc:Depends},
          ${python3:Depends},
          ${shlibs:Depends}
-Recommends: alsa-base,
-            dpkg (>= 1.13),
-            gir1.2-gst-plugins-base-0.10 | gir1.2-gst-plugins-base-1.0,
-            gir1.2-gstreamer-1.0,
-            gstreamer0.10-gconf | gstreamer1.0-plugins-good,
-            gstreamer1.0-pulseaudio,
-            libgstreamer1.0-0,
+Recommends: dpkg (>= 1.13),
+            gir1.2-gst-plugins-base-1.0 | gir1.2-gst-plugins-base-0.10,
+            gir1.2-gstreamer-1.0 | gir1.2-gstreamer-0.10,
+            gstreamer1.0-plugins-good | gstreamer0.10-gconf,
+            gstreamer1.0-pulseaudio | gstreamer0.10-pulseaudio,
+            libgstreamer1.0-0 | libgstreamer0.10-0,
             lsb-release,
             perl,
             pm-utils,
@@ -95,7 +95,6 @@
 Architecture: all
 Depends: checkbox (>= ${source:Version}),
          gir1.2-gtk-3.0,
-         gksu,
          python3-gi,
          python3-gi-cairo,
          ${misc:Depends}
@@ -121,7 +120,8 @@
 
 Package: checkbox-hw-collection
 Architecture: any
-Depends: checkbox (>= ${source:Version}), ${misc:Depends}
+Depends: checkbox (>= ${source:Version}),
+         ${misc:Depends}
 Description: CLI tool for collecting HW information from a system
  .
  This package provides a tool for collecting hardware information from

=== added file 'checkbox-old/examples/org.freedesktop.policykit.checkbox.policy'
--- checkbox-old/examples/org.freedesktop.policykit.checkbox.policy	1970-01-01 00:00:00 +0000
+++ checkbox-old/examples/org.freedesktop.policykit.checkbox.policy	2013-06-27 23:07:33 +0000
@@ -0,0 +1,31 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE policyconfig PUBLIC
+ "-//freedesktop//DTD PolicyKit Policy Configuration 1.0//EN"
+ "http://www.freedesktop.org/standards/PolicyKit/1/policyconfig.dtd">
+<policyconfig>
+
+  <!--
+    Policy definitions for Checkbox system runner backend.
+    (C) 2013 Canonical Ltd.
+    Authors: Sylvain Pineau <[e-mail]>
+             Daniel Manrique <[e-mail]>
+  -->
+
+  <vendor>Checkbox</vendor>
+  <vendor_url>https://launchpad.net/checkbox</vendor_url>
+  <icon_name>checkbox</icon_name>
+
+  <action id="org.freedesktop.policykit.pkexec.run-checkbox-backend">
+      <description>Checkbox privileged command runner</description>
+      <message>SYSTEM TESTING: Please enter your password. Some tests require root access to run properly. Your password will never be stored and will never be submitted with test results.</message>
+    <defaults>
+      <allow_any>auth_admin</allow_any>
+      <allow_inactive>auth_admin</allow_inactive>
+      <allow_active>auth_admin</allow_active>
+    </defaults>
+    <annotate key="org.freedesktop.policykit.exec.path">/usr/share/checkbox/backend</annotate>
+    <annotate key="org.freedesktop.policykit.exec.allow_gui">TRUE</annotate>
+  </action>
+
+</policyconfig>
+

=== modified file 'checkbox-old/plugins/backend_info.py'
--- checkbox-old/plugins/backend_info.py	2013-05-29 07:50:30 +0000
+++ checkbox-old/plugins/backend_info.py	2013-06-27 23:07:33 +0000
@@ -94,20 +94,16 @@
                           "Your password will never be stored and will never "
                           "be submitted with test results.")
         password_prompt = _("PASSWORD: ")
+
         if uid == 0:
             prefix = []
-        elif os.getenv("DISPLAY") and \
-                call(["which", "kdesudo"],
-                    stdout=PIPE, stderr=PIPE) == 0 and \
-                call(["pgrep", "-x", "-u", str(uid), "ksmserver"],
-                    stdout=PIPE, stderr=PIPE) == 0:
-            prefix = ["kdesudo", "--comment", password_text, "-d", "--"]
-        elif os.getenv("DISPLAY") and \
-                call(["which", "gksu"],
-                    stdout=PIPE, stderr=PIPE) == 0 and \
-                call(["pgrep", "-x", "-u", str(uid), "gnome-panel|gconfd-2"],
-                    stdout=PIPE, stderr=PIPE) == 0:
-            prefix = ["gksu", "--message", password_text, "--"]
+        #We will try to use pkexec, should be installed by default on desktop
+        #installs and is likely to be there for servers too.
+        elif call(["which", "pkexec"],
+                    stdout=PIPE, stderr=PIPE) == 0:
+            prefix = ["pkexec"]
+        #We fall back to good old sudo if pkexec is not present. Sudo
+        #*should* be present in any Ubuntu installation.
         else:
             prefix = ["sudo", "-p", password_text + " " + password_prompt]