Merge lp:~nataliabidart/ubuntu-sso-client/handle-user-not-validated-stable into lp:ubuntu-sso-client/stable-1-0
- handle-user-not-validated-stable
- Merge into stable-1-0
Proposed by
Natalia Bidart
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Natalia Bidart | ||||
Approved revision: | 637 | ||||
Merged at revision: | 637 | ||||
Proposed branch: | lp:~nataliabidart/ubuntu-sso-client/handle-user-not-validated-stable | ||||
Merge into: | lp:ubuntu-sso-client/stable-1-0 | ||||
Diff against target: |
449 lines (+173/-34) 4 files modified
ubuntu_sso/gui.py (+19/-6) ubuntu_sso/main.py (+57/-24) ubuntu_sso/tests/test_gui.py (+26/-1) ubuntu_sso/tests/test_main.py (+71/-3) |
||||
To merge this branch: | bzr merge lp:~nataliabidart/ubuntu-sso-client/handle-user-not-validated-stable | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Alejandro J. Cura (community) | Approve | ||
Roman Yepishev (community) | fieldtest | Approve | |
Review via email: mp+39786@code.launchpad.net |
Commit message
* Added a new DBus signal UserNotValidated to indicate when a user is registered but not validated (LP: #667899).
* Added new workflow so email validation is requested if necessary.
Description of the change
To test, follow the same procedure as in
https:/
but use instead the ApplicationCred
'Ubuntu One', '', '', 0
The rest of the workflow is the same.
To post a comment you must log in.
Revision history for this message
Alejandro J. Cura (alecu) wrote : | # |
I approve of this branch.
review:
Approve
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'ubuntu_sso/gui.py' |
2 | --- ubuntu_sso/gui.py 2010-10-01 13:14:34 +0000 |
3 | +++ ubuntu_sso/gui.py 2010-11-01 21:13:47 +0000 |
4 | @@ -252,6 +252,8 @@ |
5 | self.tc_uri = tc_uri |
6 | self.help_text = help_text |
7 | self.close_callback = close_callback |
8 | + self.user_email = None |
9 | + self.user_password = None |
10 | |
11 | ui_filename = get_data_file('ui.glade') |
12 | builder = gtk.Builder() |
13 | @@ -357,6 +359,8 @@ |
14 | self._filter_by_app_name(self.on_logged_in), |
15 | 'LoginError': |
16 | self._filter_by_app_name(self.on_login_error), |
17 | + 'UserNotValidated': |
18 | + self._filter_by_app_name(self.on_user_not_validated), |
19 | 'PasswordResetTokenSent': |
20 | self._filter_by_app_name(self.on_password_reset_token_sent), |
21 | 'PasswordResetError': |
22 | @@ -417,8 +421,8 @@ |
23 | |
24 | match = self.bus.add_signal_receiver(method, signal_name=signal, |
25 | dbus_interface=iface) |
26 | - logger.info('Connecting signal %r with method %r at iface %r.' \ |
27 | - 'Match: %r', signal, method, iface, match) |
28 | + logger.debug('Connecting signal %r with method %r at iface %r.' \ |
29 | + 'Match: %r', signal, method, iface, match) |
30 | self._signals_receivers[(iface, signal)] = method |
31 | |
32 | def _debug(self, *args, **kwargs): |
33 | @@ -725,8 +729,8 @@ |
34 | remove = self.bus.remove_signal_receiver |
35 | for (iface, signal) in self._signals_receivers.keys(): |
36 | method = self._signals_receivers.pop((iface, signal)) |
37 | - logger.info('Removing signal %r with method %r at iface %r.', |
38 | - signal, method, iface) |
39 | + logger.debug('Removing signal %r with method %r at iface %r.', |
40 | + signal, method, iface) |
41 | remove(method, signal_name=signal, dbus_interface=iface) |
42 | |
43 | # hide the main window |
44 | @@ -804,6 +808,8 @@ |
45 | return |
46 | |
47 | self._set_current_page(self.processing_vbox) |
48 | + self.user_email = email1 |
49 | + self.user_password = password1 |
50 | |
51 | logger.info('Calling register_user with email %r, password <hidden>,' \ |
52 | ' captcha_id %r and captcha_solution %r.', email1, |
53 | @@ -832,8 +838,8 @@ |
54 | self.email_token_entry.set_warning(self.FIELD_REQUIRED) |
55 | return |
56 | |
57 | - email = self.email1_entry.get_text() |
58 | - password = self.password1_entry.get_text() |
59 | + email = self.user_email |
60 | + password = self.user_password |
61 | f = self.backend.validate_email |
62 | logger.info('Calling validate_email with email %r, password <hidden>' \ |
63 | ', app_name %r and email_token %r.', email, self.app_name, |
64 | @@ -871,6 +877,8 @@ |
65 | reply_handler=NO_OP, error_handler=NO_OP) |
66 | |
67 | self._set_current_page(self.processing_vbox) |
68 | + self.user_email = email |
69 | + self.user_password = password |
70 | |
71 | def on_login_back_button_clicked(self, *args, **kwargs): |
72 | """User wants to go to the previous page.""" |
73 | @@ -1061,6 +1069,11 @@ |
74 | msg)) |
75 | |
76 | @log_call |
77 | + def on_user_not_validated(self, app_name, email, *args, **kwargs): |
78 | + """User was not validated.""" |
79 | + self.on_user_registered(app_name, email) |
80 | + |
81 | + @log_call |
82 | def on_password_reset_token_sent(self, app_name, email, *args, **kwargs): |
83 | """Password reset token was successfully sent.""" |
84 | msg = self.SET_NEW_PASSWORD_LABEL % {'email': email} |
85 | |
86 | === modified file 'ubuntu_sso/main.py' |
87 | --- ubuntu_sso/main.py 2010-10-18 17:34:04 +0000 |
88 | +++ ubuntu_sso/main.py 2010-11-01 21:13:47 +0000 |
89 | @@ -209,6 +209,27 @@ |
90 | 'token_name: %r', credentials['consumer_key'], token_name) |
91 | return credentials |
92 | |
93 | + def is_validated(self, email, password, token_name, sso_service=None): |
94 | + """Return if user with 'email' and 'password' is validated.""" |
95 | + if sso_service is None: |
96 | + token = self.login(email=email, password=password, |
97 | + token_name=token_name) |
98 | + |
99 | + oauth_token = oauth.OAuthToken(token['token'], |
100 | + token['token_secret']) |
101 | + authorizer = OAuthAuthorizer(token['consumer_key'], |
102 | + token['consumer_secret'], |
103 | + oauth_token) |
104 | + sso_service = self.sso_service_class(authorizer, self.service_url) |
105 | + |
106 | + me_info = sso_service.accounts.me() |
107 | + key = 'preferred_email' |
108 | + result = key in me_info and me_info[key] != None |
109 | + |
110 | + logger.debug('is_validated: email: %r token_name: %r, result: %r.', |
111 | + email, token_name, result) |
112 | + return result |
113 | + |
114 | def validate_email(self, email, password, email_token, token_name): |
115 | """Validate an email token for user with 'email' and 'password'.""" |
116 | logger.debug('validate_email: email: %r password: <hidden>, ' |
117 | @@ -311,12 +332,8 @@ |
118 | dbus.service.Object.__init__(self, object_path="/sso", |
119 | bus_name=bus_name) |
120 | self.sso_login_processor_class = sso_login_processor_class |
121 | - self.sso_service_class = sso_service_class |
122 | - |
123 | - def processor(self): |
124 | - """Create a login processor with the given class and service class.""" |
125 | - return self.sso_login_processor_class( |
126 | - sso_service_class=self.sso_service_class) |
127 | + self.processor = self.sso_login_processor_class( |
128 | + sso_service_class=sso_service_class) |
129 | |
130 | # generate_capcha signals |
131 | @dbus.service.signal(DBUS_IFACE_USER_NAME, signature="ss") |
132 | @@ -337,7 +354,7 @@ |
133 | """Call the matching method in the processor.""" |
134 | def f(): |
135 | """Inner function that will be run in a thread.""" |
136 | - return self.processor().generate_captcha(filename) |
137 | + return self.processor.generate_captcha(filename) |
138 | blocking(f, app_name, self.CaptchaGenerated, |
139 | self.CaptchaGenerationError) |
140 | |
141 | @@ -361,8 +378,8 @@ |
142 | """Call the matching method in the processor.""" |
143 | def f(): |
144 | """Inner function that will be run in a thread.""" |
145 | - return self.processor().register_user(email, password, |
146 | - captcha_id, captcha_solution) |
147 | + return self.processor.register_user(email, password, |
148 | + captcha_id, captcha_solution) |
149 | blocking(f, app_name, self.UserRegistered, self.UserRegistrationError) |
150 | |
151 | # login signals |
152 | @@ -378,6 +395,12 @@ |
153 | logger.debug('SSOLogin: emitting LoginError with ' |
154 | 'app_name "%s" and error %r', app_name, error) |
155 | |
156 | + @dbus.service.signal(DBUS_IFACE_USER_NAME, signature="ss") |
157 | + def UserNotValidated(self, app_name, result): |
158 | + """Signal thrown when the user is not validated.""" |
159 | + logger.debug('SSOLogin: emitting UserNotValidated with app_name "%s" ' |
160 | + 'and result %r', app_name, result) |
161 | + |
162 | @dbus.service.method(dbus_interface=DBUS_IFACE_USER_NAME, |
163 | in_signature='sss') |
164 | def login(self, app_name, email, password): |
165 | @@ -387,13 +410,23 @@ |
166 | token_name = get_token_name(app_name) |
167 | logger.debug('login: token_name %r, email %r, password <hidden>.', |
168 | token_name, email) |
169 | - credentials = self.processor().login(email, password, token_name) |
170 | + credentials = self.processor.login(email, password, token_name) |
171 | logger.debug('login returned not None credentials? %r.', |
172 | credentials is not None) |
173 | - assert credentials is not None |
174 | - keyring_store_credentials(app_name, credentials) |
175 | - return email |
176 | - blocking(f, app_name, self.LoggedIn, self.LoginError) |
177 | + return credentials |
178 | + |
179 | + def success_cb(app_name, credentials): |
180 | + """Login finished successfull.""" |
181 | + token_name = get_token_name(app_name) |
182 | + is_validated = self.processor.is_validated(email, password, |
183 | + token_name) |
184 | + logger.debug('user is validated? %r.', is_validated) |
185 | + if is_validated: |
186 | + keyring_store_credentials(app_name, credentials) |
187 | + self.LoggedIn(app_name, email) |
188 | + else: |
189 | + self.UserNotValidated(app_name, email) |
190 | + blocking(f, app_name, success_cb, self.LoginError) |
191 | |
192 | # validate_email signals |
193 | @dbus.service.signal(DBUS_IFACE_USER_NAME, signature="ss") |
194 | @@ -415,8 +448,8 @@ |
195 | def f(): |
196 | """Inner function that will be run in a thread.""" |
197 | token_name = get_token_name(app_name) |
198 | - credentials = self.processor().validate_email(email, password, |
199 | - email_token, token_name) |
200 | + credentials = self.processor.validate_email(email, password, |
201 | + email_token, token_name) |
202 | keyring_store_credentials(app_name, credentials) |
203 | return email |
204 | blocking(f, app_name, self.EmailValidated, self.EmailValidationError) |
205 | @@ -440,7 +473,7 @@ |
206 | """Call the matching method in the processor.""" |
207 | def f(): |
208 | """Inner function that will be run in a thread.""" |
209 | - return self.processor().request_password_reset_token(email) |
210 | + return self.processor.request_password_reset_token(email) |
211 | blocking(f, app_name, self.PasswordResetTokenSent, |
212 | self.PasswordResetError) |
213 | |
214 | @@ -463,8 +496,8 @@ |
215 | """Call the matching method in the processor.""" |
216 | def f(): |
217 | """Inner function that will be run in a thread.""" |
218 | - return self.processor().set_new_password(email, token, |
219 | - new_password) |
220 | + return self.processor.set_new_password(email, token, |
221 | + new_password) |
222 | blocking(f, app_name, self.PasswordChanged, self.PasswordChangeError) |
223 | |
224 | |
225 | @@ -481,19 +514,19 @@ |
226 | @dbus.service.signal(DBUS_IFACE_CRED_NAME, signature="s") |
227 | def AuthorizationDenied(self, app_name): |
228 | """Signal thrown when the user denies the authorization.""" |
229 | - logger.info('SSOLogin: emitting AuthorizationDenied with app_name ' |
230 | - '"%s"', app_name) |
231 | + logger.info('SSOCredentials: emitting AuthorizationDenied with ' |
232 | + 'app_name "%s"', app_name) |
233 | |
234 | @dbus.service.signal(DBUS_IFACE_CRED_NAME, signature="sa{ss}") |
235 | def CredentialsFound(self, app_name, credentials): |
236 | """Signal thrown when the credentials are found.""" |
237 | - logger.info('SSOLogin: emitting CredentialsFound with app_name "%s"', |
238 | - app_name) |
239 | + logger.info('SSOCredentials: emitting CredentialsFound with ' |
240 | + 'app_name "%s"', app_name) |
241 | |
242 | @dbus.service.signal(DBUS_IFACE_CRED_NAME, signature="sss") |
243 | def CredentialsError(self, app_name, error_message, detailed_error): |
244 | """Signal thrown when there is a problem finding the credentials.""" |
245 | - logger.debug('SSOCredentials: emitting CredentialsError with app_name ' |
246 | + logger.error('SSOCredentials: emitting CredentialsError with app_name ' |
247 | '"%s" and error_message %r', app_name, error_message) |
248 | |
249 | @dbus.service.method(dbus_interface=DBUS_IFACE_CRED_NAME, |
250 | |
251 | === modified file 'ubuntu_sso/tests/test_gui.py' |
252 | --- ubuntu_sso/tests/test_gui.py 2010-09-07 13:57:35 +0000 |
253 | +++ ubuntu_sso/tests/test_gui.py 2010-11-01 21:13:47 +0000 |
254 | @@ -856,6 +856,12 @@ |
255 | self.ui.join_ok_button.clicked() |
256 | self.assertTrue(self._called) |
257 | |
258 | + def test_user_and_pass_are_cached(self): |
259 | + """Username and password are temporarly cached for further use.""" |
260 | + self.click_join_with_valid_data() |
261 | + self.assertEqual(self.ui.user_email, EMAIL) |
262 | + self.assertEqual(self.ui.user_password, PASSWORD) |
263 | + |
264 | |
265 | class NoTermsAndConditionsTestCase(UbuntuSSOClientTestCase): |
266 | """Test suite for the user registration (with no t&c link).""" |
267 | @@ -1328,6 +1334,12 @@ |
268 | self.ui.on_login_error(app_name=APP_NAME, error=self.error) |
269 | self.assert_pages_visibility(login=True) |
270 | |
271 | + def test_on_user_not_validated_morphs_to_verify_page(self): |
272 | + """On user not validated, the verify page is shown.""" |
273 | + self.click_connect_with_valid_data() |
274 | + self.ui.on_user_not_validated(app_name=APP_NAME, email=EMAIL) |
275 | + self.assert_pages_visibility(verify_email=True) |
276 | + |
277 | def test_on_login_error_a_warning_is_shown(self): |
278 | """On user login error, a warning is shown with proper wording.""" |
279 | self.click_connect_with_valid_data() |
280 | @@ -1377,6 +1389,12 @@ |
281 | self.ui.login_ok_button.clicked() |
282 | self.assertTrue(self._called) |
283 | |
284 | + def test_user_and_pass_are_cached(self): |
285 | + """Username and password are temporarly cached for further use.""" |
286 | + self.click_connect_with_valid_data() |
287 | + self.assertEqual(self.ui.user_email, EMAIL) |
288 | + self.assertEqual(self.ui.user_password, PASSWORD) |
289 | + |
290 | |
291 | class LoginValidationTestCase(UbuntuSSOClientTestCase): |
292 | """Test suite for the user login validation.""" |
293 | @@ -1782,7 +1800,7 @@ |
294 | """All the backend signals are listed to be binded.""" |
295 | for sig in ('CaptchaGenerated', 'CaptchaGenerationError', |
296 | 'UserRegistered', 'UserRegistrationError', |
297 | - 'LoggedIn', 'LoginError', |
298 | + 'LoggedIn', 'LoginError', 'UserNotValidated', |
299 | 'EmailValidated', 'EmailValidationError', |
300 | 'PasswordResetTokenSent', 'PasswordResetError', |
301 | 'PasswordChanged', 'PasswordChangeError'): |
302 | @@ -1864,6 +1882,13 @@ |
303 | self.ui._signals['LoginError'](mismatch_app_name, 'dummy') |
304 | self.assertFalse(self._called) |
305 | |
306 | + def test_on_user_not_validated_is_not_called(self): |
307 | + """on_user_not_validated is not called if incorrect app_name.""" |
308 | + self.patch(self.ui, 'on_user_not_validated', self._set_called) |
309 | + mismatch_app_name = self.ui.app_name * 2 |
310 | + self.ui._signals['UserNotValidated'](mismatch_app_name, 'dummy') |
311 | + self.assertFalse(self._called) |
312 | + |
313 | def test_on_password_reset_token_sent_is_not_called(self): |
314 | """on_password_reset_token_sent is not called if incorrect app_name.""" |
315 | self.patch(self.ui, 'on_password_reset_token_sent', self._set_called) |
316 | |
317 | === modified file 'ubuntu_sso/tests/test_main.py' |
318 | --- ubuntu_sso/tests/test_main.py 2010-10-18 17:34:04 +0000 |
319 | +++ ubuntu_sso/tests/test_main.py 2010-11-01 21:13:47 +0000 |
320 | @@ -152,6 +152,9 @@ |
321 | class FakedAccounts(object): |
322 | """Fake the accounts service.""" |
323 | |
324 | + def __init__(self): |
325 | + self.preferred_email = EMAIL |
326 | + |
327 | def validate_email(self, email_token): |
328 | """Fake the email validation. Return a fix result.""" |
329 | if email_token is None: |
330 | @@ -164,6 +167,17 @@ |
331 | else: |
332 | return STATUS_EMAIL_OK |
333 | |
334 | + # pylint: disable=E0202, C0103 |
335 | + |
336 | + def me(self): |
337 | + """Fake the 'me' information.""" |
338 | + return {u'username': u'Wh46bKY', |
339 | + u'preferred_email': self.preferred_email, |
340 | + u'displayname': u'', |
341 | + u'unverified_emails': [u'aaaaaa@example.com'], |
342 | + u'verified_emails': [], |
343 | + u'openid_identifier': u'Wh46bKY'} |
344 | + |
345 | |
346 | class FakedSSOServer(object): |
347 | """Fake an SSO server.""" |
348 | @@ -279,6 +293,29 @@ |
349 | result = self.processor.login(**self.login_kwargs) |
350 | self.assertEqual(TOKEN, result, 'authentication was successful.') |
351 | |
352 | + # is_validated |
353 | + |
354 | + def test_is_validated(self): |
355 | + """If preferred email is not None, user is validated.""" |
356 | + result = self.processor.is_validated(**self.login_kwargs) |
357 | + self.assertTrue(result, 'user must be validated.') |
358 | + |
359 | + def test_is_not_validated(self): |
360 | + """If preferred email is None, user is not validated.""" |
361 | + service = FakedSSOServer(None, None) |
362 | + service.accounts.preferred_email = None |
363 | + result = self.processor.is_validated(sso_service=service, |
364 | + **self.login_kwargs) |
365 | + self.assertFalse(result, 'user must not be validated.') |
366 | + |
367 | + def test_is_not_validated_empty_result(self): |
368 | + """If preferred email is None, user is not validated.""" |
369 | + service = FakedSSOServer(None, None) |
370 | + service.accounts.me = lambda: {} |
371 | + result = self.processor.is_validated(sso_service=service, |
372 | + **self.login_kwargs) |
373 | + self.assertFalse(result, 'user must not be validated.') |
374 | + |
375 | # validate_email |
376 | |
377 | def test_validate_email_if_status_ok(self): |
378 | @@ -519,8 +556,11 @@ |
379 | def test_login(self): |
380 | """Test that the login method works ok.""" |
381 | d = Deferred() |
382 | - self.create_mock_processor().login(EMAIL, PASSWORD, TOKEN_NAME) |
383 | + processor = self.create_mock_processor() |
384 | + processor.login(EMAIL, PASSWORD, TOKEN_NAME) |
385 | self.mocker.result(TOKEN) |
386 | + processor.is_validated(EMAIL, PASSWORD, TOKEN_NAME) |
387 | + self.mocker.result(True) |
388 | self.patch(ubuntu_sso.main, "blocking", self.fake_ok_blocking) |
389 | self.mocker.replay() |
390 | |
391 | @@ -535,13 +575,40 @@ |
392 | sso_login_processor_class=self.mockprocessorclass) |
393 | self.patch(client, "LoggedIn", verify) |
394 | self.patch(client, "LoginError", d.errback) |
395 | + self.patch(client, "UserNotValidated", d.errback) |
396 | + client.login(APP_NAME, EMAIL, PASSWORD) |
397 | + return d |
398 | + |
399 | + def test_login_user_not_validated(self): |
400 | + """Test that the login sends EmailNotValidated signal.""" |
401 | + d = Deferred() |
402 | + processor = self.create_mock_processor() |
403 | + processor.login(EMAIL, PASSWORD, TOKEN_NAME) |
404 | + self.mocker.result(TOKEN) |
405 | + processor.is_validated(EMAIL, PASSWORD, TOKEN_NAME) |
406 | + self.mocker.result(False) |
407 | + self.patch(ubuntu_sso.main, "blocking", self.fake_ok_blocking) |
408 | + self.mocker.replay() |
409 | + |
410 | + def verify(app_name, email): |
411 | + """The actual test.""" |
412 | + self.assertEqual(app_name, APP_NAME) |
413 | + self.assertEqual(email, EMAIL) |
414 | + self.assertFalse(self.keyring_was_set, "Keyring should not be set") |
415 | + d.callback("Ok") |
416 | + |
417 | + client = SSOLogin(self.mockbusname, |
418 | + sso_login_processor_class=self.mockprocessorclass) |
419 | + self.patch(client, "LoggedIn", d.errback) |
420 | + self.patch(client, "LoginError", d.errback) |
421 | + self.patch(client, "UserNotValidated", verify) |
422 | client.login(APP_NAME, EMAIL, PASSWORD) |
423 | return d |
424 | |
425 | def test_login_error(self): |
426 | """Test that the login method fails as expected.""" |
427 | d = Deferred() |
428 | - self.mockprocessorclass = self.mocker.mock() |
429 | + self.create_mock_processor() |
430 | self.patch(ubuntu_sso.main, "blocking", self.fake_err_blocking) |
431 | |
432 | def fake_gtn(*args): |
433 | @@ -562,6 +629,7 @@ |
434 | sso_login_processor_class=self.mockprocessorclass) |
435 | self.patch(client, "LoggedIn", d.errback) |
436 | self.patch(client, "LoginError", verify) |
437 | + self.patch(client, "UserNotValidated", d.errback) |
438 | client.login(APP_NAME, EMAIL, PASSWORD) |
439 | return d |
440 | |
441 | @@ -591,7 +659,7 @@ |
442 | def test_validate_email_error(self): |
443 | """Test that the validate_email method fails as expected.""" |
444 | d = Deferred() |
445 | - self.mockprocessorclass = self.mocker.mock() |
446 | + self.create_mock_processor() |
447 | self.patch(ubuntu_sso.main, "blocking", self.fake_err_blocking) |
448 | |
449 | def fake_gtn(*args): |
Works as advertised. This branch is awesome too.