Indicator Display

Merge lp://staging/~charlesk/indicator-display/adb-key-auth into lp://staging/indicator-display/15.10

  1. adb-key-auth
  2. Merge into trunk.15.10
Proposed by Charles Kerr
Status: Merged
Approved by: Xavi Garcia
Approved revision: 42
Merged at revision: 18
Proposed branch: lp://staging/~charlesk/indicator-display/adb-key-auth
Merge into: lp://staging/indicator-display/15.10
Diff against target: 3348 lines (+2613/-310)
40 files modified
.bzr-builddeb/default.conf (+0/-2)
CMakeLists.txt (+76/-60)
cmake/FindGMock.cmake (+0/-10)
cmake/GCov.cmake (+0/-51)
cmake/GdbusCodegen.cmake (+0/-36)
debian/control (+6/-0)
po/POTFILES.in (+1/-0)
src/CMakeLists.txt (+36/-28)
src/adbd-client.cpp (+303/-0)
src/adbd-client.h (+74/-0)
src/dbus-names.h (+60/-0)
src/greeter.cpp (+161/-0)
src/greeter.h (+47/-0)
src/indicator.cpp (+37/-0)
src/indicator.h (+6/-7)
src/main.cpp (+13/-0)
src/rotation-lock.cpp (+1/-0)
src/usb-manager.cpp (+180/-0)
src/usb-manager.h (+48/-0)
src/usb-monitor.cpp (+81/-0)
src/usb-monitor.h (+52/-0)
src/usb-snap.cpp (+250/-0)
src/usb-snap.h (+42/-0)
tests/CMakeLists.txt (+33/-33)
tests/integration/CMakeLists.txt (+24/-0)
tests/integration/usb-manager-test.cpp (+226/-0)
tests/unit/CMakeLists.txt (+34/-0)
tests/unit/adbd-client-test.cpp (+95/-0)
tests/unit/rotation-lock-test.cpp (+3/-3)
tests/unit/usb-snap-test.cpp (+143/-0)
tests/utils/CMakeLists.txt (+17/-0)
tests/utils/adbd-server.h (+150/-0)
tests/utils/dbus-types.h (+42/-0)
tests/utils/glib-fixture.h (+115/-64)
tests/utils/gtest-qt-print-helpers.h (+45/-0)
tests/utils/mock-greeter.h (+32/-0)
tests/utils/mock-usb-monitor.h (+32/-0)
tests/utils/qmain.cpp (+60/-0)
tests/utils/qt-fixture.h (+74/-0)
tests/utils/test-dbus-fixture.h (+14/-16)
To merge this branch: bzr merge lp://staging/~charlesk/indicator-display/adb-key-auth
Reviewer Review Type Date Requested Status
Charles Kerr (community) Approve
Matthew Paul Thomas (community) design Needs Fixing
Xavi Garcia Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+289126@code.staging.launchpad.net

Commit message

When a new device appears to ADB, prompt the user whether or not to allow the connection.

Description of the change

When a new device appears to ADB, prompt the user whether or not to allow the connection.

Reference java implementation @ http://androidxref.com/5.1.1_r6/xref/frameworks/base/services/usb/java/com/android/server/usb/UsbDebuggingManager.java#59

Basically, the ADBD daemon talks to the indicator via a local domain socket. ADBD sends a request string "PK" + the public key. The indicator's job is to decode the public key and present the resulting fingerprint to the user to allow or deny. Reference Android prompt: http://i.stack.imgur.com/MPh73.png

The indicator returns the strings 'OK' or 'NO' back down the socket depending on the user's choice. If the user clicked the 'remember this' checkbox, we'll also append the public key on its own new line to the textfile "/data/misc/adb/adb_keys".

NB 1: unity-notifications doesn't yet have a checkbox action, so this is omitted from this MP. To honor the most common use case first, this first cut remembers the public key if the user allows the connection.

NB 2: this code is currently being used for testing. The single line in main.cpp 'g_setenv("G_MESSAGES_DEBUG", "all", true);' is to enable verbose logging in ~/.cache/upstart/indicator-display.log and will be removed before landing.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:37
http://jenkins.qa.ubuntu.com/job/indicator-display-ci/22/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/indicator-display-wily-amd64-ci/5/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/indicator-display-wily-armhf-ci/5/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/indicator-display-ci/22/rebuild

review: Needs Fixing (continuous-integration)
lp://staging/~charlesk/indicator-display/adb-key-auth updated
38. By Charles Kerr

Remove the minimum version of 0.4 for libqtdbusmock1-dev, it's causing a failure on Jenkins wily and was only included due to copy-paste from indicator-network. So let's see how Wily goes without it.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:38
http://jenkins.qa.ubuntu.com/job/indicator-display-ci/23/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/indicator-display-wily-amd64-ci/6/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/indicator-display-wily-armhf-ci/6/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/indicator-display-ci/23/rebuild

review: Needs Fixing (continuous-integration)
lp://staging/~charlesk/indicator-display/adb-key-auth updated
39. By Charles Kerr

use cmake-extras' EnableCoverageReport instead of home-rolled rules

40. By Charles Kerr

in POTFILES.in, fix copy-paste typo

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:39
http://jenkins.qa.ubuntu.com/job/indicator-display-ci/24/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-display-wily-amd64-ci/7
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-display-wily-armhf-ci/7
        deb: http://jenkins.qa.ubuntu.com/job/indicator-display-wily-armhf-ci/7/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/indicator-display-ci/24/rebuild

review: Approve (continuous-integration)
lp://staging/~charlesk/indicator-display/adb-key-auth updated
41. By Charles Kerr

in usb-manager.cpp, remove the remember_choice workaround since is already working around it

42. By Charles Kerr

tweak class description comments

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:42
http://jenkins.qa.ubuntu.com/job/indicator-display-ci/25/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-display-wily-amd64-ci/8
    SUCCESS: http://jenkins.qa.ubuntu.com/job/indicator-display-wily-armhf-ci/8
        deb: http://jenkins.qa.ubuntu.com/job/indicator-display-wily-armhf-ci/8/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/indicator-display-ci/25/rebuild

review: Approve (continuous-integration)
Revision history for this message
Xavi Garcia (xavi-garcia-mena) wrote :

Looks good to me...

Just a minor comment that don't prevent this to be approved IMHO:

Would it make sense to have qDBusArgumentToMap and WAIT_FOR_SIGNALS defined in a common place?
I see both are defined twice and the code is the same...
In fact this is something I'm using in my tests and that maybe would be good to have in a common test-utils library or something... (maybe something to be done in the next future)

Revision history for this message
Xavi Garcia (xavi-garcia-mena) :
review: Approve
lp://staging/~charlesk/indicator-display/adb-key-auth updated
43. By Charles Kerr

add tests/utils/qdbus-helpers.h so that we only define qDBusArgumentToMap() in one place

44. By Charles Kerr

de-dupe use of dbus names

45. By Charles Kerr

introduce a QtFixture gtest base class to reduce redundancy in test fixtures' helper/util code

46. By Charles Kerr

sync with lp:~ubuntu-branches/ubuntu/wily/indicator-display/wily/ to pick up 0.1+15.10.20150727-0ubuntu2~gcc5.1

47. By Charles Kerr

fix warning message

48. By Charles Kerr

sync with trunk

49. By Charles Kerr

re-add .bzr-builddeb, which got removed by accident

50. By Charles Kerr

add some extra debug statements to usb-manager.cpp to track user response and the act of writing the pk out to disk

Revision history for this message
Matthew Paul Thomas (mpt) wrote :

Please change “Deny” to “Don’t Allow” for consistency with other permission prompts.
<https://wiki.ubuntu.com/SecurityPermissions#permission-prompt>

review: Needs Fixing (design)
lp://staging/~charlesk/indicator-display/adb-key-auth updated
51. By Charles Kerr

turn off verbose debugging

52. By Charles Kerr

if our USB device is disconnected while prompting the user for ADBD, cancel the prompt.

53. By Charles Kerr

in UsbManager, reset AdbdClient on usb disconnect

54. By Charles Kerr

add Greeter skeleton

55. By Charles Kerr

get greeter's IsActive property working

56. By Charles Kerr

fix typo

57. By Charles Kerr

make wait_for_signals() a macro again so that the GTest log messages will give the test file and line number

58. By Charles Kerr

in the mock ADB server, keep making a request until a response is received.

59. By Charles Kerr

add multiple usb requests + disconnects to confirm the notification appears on subsequent requests

60. By Charles Kerr

don't show the snap decision until we're out of the greeter

61. By Charles Kerr

add tests for not showing snap decisions in greeter mode

62. By Charles Kerr

replace text 'Deny' with 'Don't Allow' for consistency with other permission prompts

63. By Charles Kerr

keep the adbd socket open even when the lockscreen is closed. hold the pkrequest state in USBManager until the screen's unlocked.

64. By Charles Kerr

fix missing field initialization compiler warning

65. By Charles Kerr

fix UsbManager dtor issue found by valgrind

Revision history for this message
Charles Kerr (charlesk) wrote :

re-approving up to r65 from ondra revie

review: Approve
lp://staging/~charlesk/indicator-display/adb-key-auth updated
66. By Charles Kerr

add tracer g_debug() calls for the benefit of the integration tests

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches