Merge lp:~nataliabidart/ubuntuone-client/fix-sso-signal-handling into lp:ubuntuone-client

Proposed by Natalia Bidart on 2010-08-25
Status: Merged
Approved by: Natalia Bidart on 2010-08-25
Approved revision: 653
Merged at revision: 653
Proposed branch: lp:~nataliabidart/ubuntuone-client/fix-sso-signal-handling
Merge into: lp:ubuntuone-client
Diff against target: 163 lines (+85/-11)
3 files modified
contrib/testing/testcase.py (+2/-1)
tests/syncdaemon/test_dbus.py (+64/-3)
ubuntuone/syncdaemon/dbus_interface.py (+19/-7)
To merge this branch: bzr merge lp:~nataliabidart/ubuntuone-client/fix-sso-signal-handling
Reviewer Review Type Date Requested Status
Facundo Batista Approve on 2010-08-25
Rodrigo Moya (community) Approve on 2010-08-25
John Lenton 2010-08-25 Approve on 2010-08-25
Review via email: mp+33636@code.launchpad.net

Commit message

  Signals from SSO dbus service are correctly processed now (using the latest
  API). Also, if the app_name that the signal is sending doesn't match U1
  app_name, the signal is ignored (LP: #623447).

Description of the change

  Signals from SSO dbus service are correctly processed now (using the latest
  API). Also, if the app_name that the signal is sending doesn't match U1
  app_name, the signal is ignored.

To post a comment you must log in.
John Lenton (chipaca) wrote :

This works like a charm.

review: Approve
Rodrigo Moya (rodrigo-moya) wrote :

Fixed the problems I was having with syncdaemon not connecting

review: Approve
Facundo Batista (facundo) wrote :

Like it!

review: Approve
dobey (dobey) wrote :
Download full text (15.5 KiB)

The attempt to merge lp:~nataliabidart/ubuntuone-client/fix-sso-signal-handling into lp:ubuntuone-client failed.Below is the output from the failed tests.

/usr/bin/gnome-autogen.sh
checking for autoconf >= 2.53...
  testing autoconf2.50... not found.
  testing autoconf... found 2.67
checking for automake >= 1.10...
  testing automake-1.11... found 1.11.1
checking for libtool >= 1.5...
  testing libtoolize... found 2.2.6b
checking for intltool >= 0.30...
  testing intltoolize... found 0.41.1
checking for pkg-config >= 0.14.0...
  testing pkg-config... found 0.25
Checking for required M4 macros...
Checking for forbidden M4 macros...
Processing ./configure.ac
Running libtoolize...
libtoolize: putting auxiliary files in `.'.
libtoolize: copying file `./ltmain.sh'
libtoolize: putting macros in AC_CONFIG_MACRO_DIR, `m4'.
libtoolize: copying file `m4/libtool.m4'
libtoolize: copying file `m4/ltoptions.m4'
libtoolize: copying file `m4/ltsugar.m4'
libtoolize: copying file `m4/ltversion.m4'
libtoolize: copying file `m4/lt~obsolete.m4'
Running intltoolize...
Running aclocal-1.11...
Running autoconf...
Running autoheader...
Running automake-1.11...
Running ./configure --with-protocol=/var/cache/tarmac/ubuntuone-storage-protocol/trunk ...
checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
checking for a thread-safe mkdir -p... /bin/mkdir -p
checking for gawk... gawk
checking whether make sets $(MAKE)... yes
checking for style of include used by make... GNU
checking for gcc... gcc
checking whether the C compiler works... yes
checking for C compiler default output file name... a.out
checking for suffix of executables...
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether we are using the GNU C compiler... yes
checking whether gcc accepts -g... yes
checking for gcc option to accept ISO C89... none needed
checking dependency style of gcc... gcc3
checking for library containing strerror... none required
checking for gcc... (cached) gcc
checking whether we are using the GNU C compiler... (cached) yes
checking whether gcc accepts -g... (cached) yes
checking for gcc option to accept ISO C89... (cached) none needed
checking dependency style of gcc... (cached) gcc3
checking build system type... i686-pc-linux-gnu
checking host system type... i686-pc-linux-gnu
checking for a sed that does not truncate output... /bin/sed
checking for grep that handles long lines and -e... /bin/grep
checking for egrep... /bin/grep -E
checking for fgrep... /bin/grep -F
checking for ld used by gcc... /usr/bin/ld
checking if the linker (/usr/bin/ld) is GNU ld... yes
checking for BSD- or MS-compatible name lister (nm)... /usr/bin/nm -B
checking the name lister (/usr/bin/nm -B) interface... BSD nm
checking whether ln -s works... yes
checking the maximum length of command line arguments... 1572864
checking whether the shell understands some XSI constructs... yes
checking whether the shell understands "+="... yes
checking for /usr/bin/ld option to reload object files... -r
checking for objdump... objdump
checking how to recognize dependent libraries... pass_all
checking for ar... a...

dobey (dobey) wrote :
Download full text (15.2 KiB)

The attempt to merge lp:~nataliabidart/ubuntuone-client/fix-sso-signal-handling into lp:ubuntuone-client failed.Below is the output from the failed tests.

/usr/bin/gnome-autogen.sh
checking for autoconf >= 2.53...
  testing autoconf2.50... not found.
  testing autoconf... found 2.67
checking for automake >= 1.10...
  testing automake-1.11... found 1.11.1
checking for libtool >= 1.5...
  testing libtoolize... found 2.2.6b
checking for intltool >= 0.30...
  testing intltoolize... found 0.41.1
checking for pkg-config >= 0.14.0...
  testing pkg-config... found 0.25
Checking for required M4 macros...
Checking for forbidden M4 macros...
Processing ./configure.ac
Running libtoolize...
libtoolize: putting auxiliary files in `.'.
libtoolize: copying file `./ltmain.sh'
libtoolize: putting macros in AC_CONFIG_MACRO_DIR, `m4'.
libtoolize: copying file `m4/libtool.m4'
libtoolize: copying file `m4/ltoptions.m4'
libtoolize: copying file `m4/ltsugar.m4'
libtoolize: copying file `m4/ltversion.m4'
libtoolize: copying file `m4/lt~obsolete.m4'
Running intltoolize...
Running aclocal-1.11...
Running autoconf...
Running autoheader...
Running automake-1.11...
Running ./configure --with-protocol=/var/cache/tarmac/ubuntuone-storage-protocol/trunk ...
checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
checking for a thread-safe mkdir -p... /bin/mkdir -p
checking for gawk... gawk
checking whether make sets $(MAKE)... yes
checking for style of include used by make... GNU
checking for gcc... gcc
checking whether the C compiler works... yes
checking for C compiler default output file name... a.out
checking for suffix of executables...
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether we are using the GNU C compiler... yes
checking whether gcc accepts -g... yes
checking for gcc option to accept ISO C89... none needed
checking dependency style of gcc... gcc3
checking for library containing strerror... none required
checking for gcc... (cached) gcc
checking whether we are using the GNU C compiler... (cached) yes
checking whether gcc accepts -g... (cached) yes
checking for gcc option to accept ISO C89... (cached) none needed
checking dependency style of gcc... (cached) gcc3
checking build system type... i686-pc-linux-gnu
checking host system type... i686-pc-linux-gnu
checking for a sed that does not truncate output... /bin/sed
checking for grep that handles long lines and -e... /bin/grep
checking for egrep... /bin/grep -E
checking for fgrep... /bin/grep -F
checking for ld used by gcc... /usr/bin/ld
checking if the linker (/usr/bin/ld) is GNU ld... yes
checking for BSD- or MS-compatible name lister (nm)... /usr/bin/nm -B
checking the name lister (/usr/bin/nm -B) interface... BSD nm
checking whether ln -s works... yes
checking the maximum length of command line arguments... 1572864
checking whether the shell understands some XSI constructs... yes
checking whether the shell understands "+="... yes
checking for /usr/bin/ld option to reload object files... -r
checking for objdump... objdump
checking how to recognize dependent libraries... pass_all
checking for ar... a...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'contrib/testing/testcase.py'
2--- contrib/testing/testcase.py 2010-08-16 22:14:19 +0000
3+++ contrib/testing/testcase.py 2010-08-25 17:46:40 +0000
4@@ -53,7 +53,8 @@
5 FAKED_CREDENTIALS = {'consumer_key': 'faked_consumer_key',
6 'consumer_secret': 'faked_consumer_secret',
7 'token': 'faked_token',
8- 'token_secret': 'faked_token_secret'}
9+ 'token_secret': 'faked_token_secret',
10+ 'token_name': 'Test me please'}
11
12
13 @contextlib.contextmanager
14
15=== modified file 'tests/syncdaemon/test_dbus.py'
16--- tests/syncdaemon/test_dbus.py 2010-08-18 17:56:57 +0000
17+++ tests/syncdaemon/test_dbus.py 2010-08-25 17:46:40 +0000
18@@ -30,6 +30,7 @@
19 from contrib.testing.testcase import (
20 BaseTwistedTestCase, DBusTwistedTestCase, FakeMain, FAKED_CREDENTIALS,
21 MementoHandler)
22+from ubuntuone.clientdefs import APP_NAME
23 from ubuntuone.storageprotocol.sharersp import NotifyShareHolder
24 from ubuntuone.syncdaemon import event_queue, states, config
25 from ubuntuone.syncdaemon.dbus_interface import (
26@@ -2218,7 +2219,7 @@
27
28 def f(*a, **kw):
29 """Receive credentials."""
30- self.dbus_iface._signal_handler(FAKED_CREDENTIALS,
31+ self.dbus_iface._signal_handler(APP_NAME, FAKED_CREDENTIALS,
32 member='CredentialsFound')
33
34 self.patch(FakedSSOBackend, 'login_or_register_to_get_credentials', f)
35@@ -2232,7 +2233,9 @@
36
37 def f(*a, **kw):
38 """Receive error signal."""
39- self.dbus_iface._signal_handler(member='CredentialsError')
40+ self.dbus_iface._signal_handler(APP_NAME, 'Error description',
41+ 'Detailed error',
42+ member='CredentialsError')
43
44 self.patch(FakedSSOBackend, 'login_or_register_to_get_credentials', f)
45 d = self.dbus_iface.connect()
46@@ -2246,7 +2249,8 @@
47
48 def f(*a, **kw):
49 """Receive error signal."""
50- self.dbus_iface._signal_handler(member='AuthorizationDenied')
51+ self.dbus_iface._signal_handler(APP_NAME,
52+ member='AuthorizationDenied')
53
54 self.patch(FakedSSOBackend, 'login_or_register_to_get_credentials', f)
55 d = self.dbus_iface.connect()
56@@ -2303,6 +2307,63 @@
57 self.assertEqual(self.events, [('SYS_USER_CONNECT', (),
58 {'access_token': expected})])
59
60+ def test_only_log_if_app_name_is_not_correct_when_credentials_found(self):
61+ """If the app_name is not ours, just log an INFO message."""
62+
63+ def f(*a, **kw):
64+ """Receive wrong app_name."""
65+ self.dbus_iface._signal_handler(APP_NAME * 2,
66+ member='CredentialsFound')
67+
68+ self.patch(FakedSSOBackend, 'login_or_register_to_get_credentials', f)
69+ self.dbus_iface._request_token()
70+
71+ self.assertFalse(self.dbus_iface._deferred.called)
72+ self.assertTrue(self.memento.check_info('CredentialsFound',
73+ APP_NAME * 2))
74+
75+ def test_only_log_if_app_name_is_not_correct_when_credentials_error(self):
76+ """If the app_name is not ours, just log an INFO message."""
77+
78+ def f(*a, **kw):
79+ """Receive a wrong app_name."""
80+ self.dbus_iface._signal_handler(APP_NAME * 2,
81+ member='CredentialsError')
82+
83+ self.patch(FakedSSOBackend, 'login_or_register_to_get_credentials', f)
84+ self.dbus_iface._request_token()
85+
86+ self.assertFalse(self.dbus_iface._deferred.called)
87+ self.assertTrue(self.memento.check_info('CredentialsError',
88+ APP_NAME * 2))
89+
90+ def test_only_log_if_app_name_is_not_correct_when_authorization_denied(self):
91+ """If the app_name is not ours, just log an INFO message."""
92+
93+ def f(*a, **kw):
94+ """Receive a wrong app_name."""
95+ self.dbus_iface._signal_handler(APP_NAME * 2,
96+ member='AuthorizationDenied')
97+
98+ self.patch(FakedSSOBackend, 'login_or_register_to_get_credentials', f)
99+ self.dbus_iface._request_token()
100+
101+ self.assertFalse(self.dbus_iface._deferred.called)
102+ self.assertTrue(self.memento.check_info('AuthorizationDenied',
103+ APP_NAME * 2))
104+
105+ def test_signal_handler_remains_generic(self):
106+ """The signal handler function should be generic."""
107+ self.dbus_iface._signal_handler()
108+ # no failure
109+ self.assertTrue(self.memento.check_debug('member: None',
110+ 'app_name: None'))
111+
112+ self.dbus_iface._signal_handler(no_member_kwarg='Test')
113+ # no failure
114+ self.assertTrue(self.memento.check_debug('member: None',
115+ 'app_name: None'))
116+
117
118 class FolderTests(DBusTwistedTestCase):
119 """Tests for the Folder object exposed via dbus."""
120
121=== modified file 'ubuntuone/syncdaemon/dbus_interface.py'
122--- ubuntuone/syncdaemon/dbus_interface.py 2010-08-20 21:09:00 +0000
123+++ ubuntuone/syncdaemon/dbus_interface.py 2010-08-25 17:46:40 +0000
124@@ -1871,20 +1871,32 @@
125
126 def _signal_handler(self, *args, **kwargs):
127 """Generic signal handler."""
128+ member = kwargs.get('member', None)
129+ app_name = args[0] if len(args) > 0 else None
130 d = self._deferred
131- member = kwargs.get('member', None)
132- logger.debug('Handling DBus signals for member %r, with args %r ' \
133- 'and kwargs %r.', member, args, kwargs)
134+ logger.debug('Handling DBus signal for member: %r, app_name: %r.',
135+ member, app_name)
136+
137+ if app_name != APP_NAME:
138+ logger.info('Received %s but app_name %s does not match %s, ' \
139+ 'exiting.', member, app_name, APP_NAME)
140+ return
141+
142 if member in ('CredentialsError', 'AuthorizationDenied'):
143+ logger.warning('%r: %r %r', member, args, kwargs)
144 if not args:
145 d.errback(Failure(NoAccessToken(member)))
146 else:
147- d.errback(Failure(NoAccessToken("%s: %s" %
148- (member, args[0]))))
149+ d.errback(Failure(NoAccessToken("%s: %s %s" %
150+ (member, args, kwargs))))
151 elif member == 'CredentialsFound' and not d.called:
152- d.callback(args[0])
153+ credentials = args[1]
154+ logger.info('%r: callbacking with credentials (token_name: %r).',
155+ member, None) # fill token_name later, see bug #623604
156+ d.callback(credentials)
157 else:
158- logger.debug('_signal_handler: member %r not used.', member)
159+ logger.debug('_signal_handler: member %r not used or deferred '
160+ 'already called? %r.', member, d.called)
161
162 def _request_token(self):
163 """Request to SSO auth service to fetch the token."""

Subscribers

People subscribed via source and target branches