Canonical SSO provider

Merge lp:~matiasb/canonical-identity-provider/validate-returns-is-verified into lp:canonical-identity-provider/release

  1. validate-returns-is-verified
  2. Merge into trunk
Proposed by Matias Bordese
Status: Merged
Approved by: Ricardo Kirkner
Approved revision: no longer in the source branch.
Merged at revision: 1144
Proposed branch: lp:~matiasb/canonical-identity-provider/validate-returns-is-verified
Merge into: lp:canonical-identity-provider/release
Diff against target: 271 lines (+66/-29)
2 files modified
src/api/v20/handlers.py (+6/-5)
src/api/v20/tests/test_handlers.py (+60/-24)
To merge this branch: bzr merge lp:~matiasb/canonical-identity-provider/validate-returns-is-verified
Reviewer Review Type Date Requested Status
Ricardo Kirkner (community) Approve
Review via email: mp+226035@code.launchpad.net

Commit message

Added account_verified details to validate_request API endpoint.

To post a comment you must log in.
Revision history for this message
Ricardo Kirkner (ricardokirkner) wrote :

LGTM

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/api/v20/handlers.py'
--- src/api/v20/handlers.py 2013-11-28 14:25:58 +0000
+++ src/api/v20/handlers.py 2014-07-08 20:32:06 +0000
@@ -458,7 +458,7 @@
458 if authorization is not None:458 if authorization is not None:
459 headers = {'Authorization': authorization}459 headers = {'Authorization': authorization}
460460
461 result = {'is_valid': False}461 result = dict(is_valid=False, account_verified=False)
462462
463 if not http_url or not http_method:463 if not http_url or not http_method:
464 return result464 return result
@@ -489,18 +489,19 @@
489 consumer, _, _ = oauth_server.verify_request(oauth_request)489 consumer, _, _ = oauth_server.verify_request(oauth_request)
490 except oauth.OAuthError:490 except oauth.OAuthError:
491 # do nothing special, signature was invalid491 # do nothing special, signature was invalid
492 valid = False492 pass
493 except Exception as e:493 except Exception as e:
494 result['error'] = self.serialize_error(e)494 result['error'] = self.serialize_error(e)
495 logger.exception(495 logger.exception(
496 'RequestsHandler.create: could not verify request:')496 'RequestsHandler.create: could not verify request:')
497 valid = False
498 else:497 else:
499 # signature is valid, need to also check the account exists498 # signature is valid, need to also check the account exists
500 # and is active499 # and is active
501 valid = Account.objects.active_by_openid(consumer.key) is not None500 account = Account.objects.active_by_openid(consumer.key)
501 result['is_valid'] = account is not None
502 result['account_verified'] = (
503 account is not None and account.is_verified)
502504
503 result['is_valid'] = valid
504 return result505 return result
505506
506507
507508
=== modified file 'src/api/v20/tests/test_handlers.py'
--- src/api/v20/tests/test_handlers.py 2014-01-27 19:18:59 +0000
+++ src/api/v20/tests/test_handlers.py 2014-07-08 20:32:06 +0000
@@ -333,7 +333,8 @@
333 )333 )
334 return data334 return data
335335
336 def assert_request_response(self, response, expected, error=None):336 def assert_request_response(self, response, is_valid, account_verified,
337 error=None):
337 self.assertEqual(response.status_code, 200)338 self.assertEqual(response.status_code, 200)
338 try:339 try:
339 response = json.loads(response.content)340 response = json.loads(response.content)
@@ -341,7 +342,8 @@
341 self.fail('Could not JSON-decode response, content is %r.' %342 self.fail('Could not JSON-decode response, content is %r.' %
342 response.content)343 response.content)
343 else:344 else:
344 expected = dict(is_valid=expected)345 expected = dict(is_valid=is_valid,
346 account_verified=account_verified)
345 if error:347 if error:
346 expected['error'] = error348 expected['error'] = error
347 self.assertEqual(response, expected)349 self.assertEqual(response, expected)
@@ -349,32 +351,37 @@
349 def test_invalid_if_not_method_given(self):351 def test_invalid_if_not_method_given(self):
350 self.data.pop('http_method')352 self.data.pop('http_method')
351 response = self.do_post()353 response = self.do_post()
352 self.assert_request_response(response, False)354 self.assert_request_response(
355 response, is_valid=False, account_verified=False)
353 self.assertFalse(self.mock_logger.exception.called)356 self.assertFalse(self.mock_logger.exception.called)
354357
355 def test_invalid_if_method_none(self):358 def test_invalid_if_method_none(self):
356 self.data['http_method'] = None359 self.data['http_method'] = None
357 response = self.do_post()360 response = self.do_post()
358 self.assert_request_response(response, False)361 self.assert_request_response(
362 response, is_valid=False, account_verified=False)
359 self.assertFalse(self.mock_logger.exception.called)363 self.assertFalse(self.mock_logger.exception.called)
360364
361 def test_invalid_if_not_url_given(self):365 def test_invalid_if_not_url_given(self):
362 self.data.pop('http_url')366 self.data.pop('http_url')
363 response = self.do_post()367 response = self.do_post()
364 self.assert_request_response(response, False)368 self.assert_request_response(
369 response, is_valid=False, account_verified=False)
365 self.assertFalse(self.mock_logger.exception.called)370 self.assertFalse(self.mock_logger.exception.called)
366371
367 def test_invalid_if_url_none(self):372 def test_invalid_if_url_none(self):
368 self.data['http_url'] = None373 self.data['http_url'] = None
369 response = self.do_post()374 response = self.do_post()
370 self.assert_request_response(response, False)375 self.assert_request_response(
376 response, is_valid=False, account_verified=False)
371 self.assertFalse(self.mock_logger.exception.called)377 self.assertFalse(self.mock_logger.exception.called)
372378
373 def test_url_encoded(self):379 def test_url_encoded(self):
374 self.data = self.build_data(external_url=u'http://foo/ñoño ñandú/')380 self.data = self.build_data(external_url=u'http://foo/ñoño ñandú/')
375 response = self.do_post()381 response = self.do_post()
376382
377 self.assert_request_response(response, True)383 self.assert_request_response(
384 response, is_valid=True, account_verified=True)
378 self.assertFalse(self.mock_logger.exception.called)385 self.assertFalse(self.mock_logger.exception.called)
379386
380 def test_error_in_response_is_robust_to_encodings(self):387 def test_error_in_response_is_robust_to_encodings(self):
@@ -388,7 +395,8 @@
388395
389 msg = ', '.join(repr(i) for i in exc.args)396 msg = ', '.join(repr(i) for i in exc.args)
390 self.assert_request_response(397 self.assert_request_response(
391 response, False, error="TypeError: %s" % msg)398 response, is_valid=False, account_verified=False,
399 error="TypeError: %s" % msg)
392400
393 def test_invalid_if_from_request_fails(self):401 def test_invalid_if_from_request_fails(self):
394 self._apply_patch(402 self._apply_patch(
@@ -397,7 +405,8 @@
397405
398 response = self.do_post()406 response = self.do_post()
399 self.assert_request_response(407 self.assert_request_response(
400 response, False, error="ValueError: 'foo \\xf1o\\xf1o'")408 response, is_valid=False, account_verified=False,
409 error="ValueError: 'foo \\xf1o\\xf1o'")
401 self.mock_logger.exception.assert_called_once_with(410 self.mock_logger.exception.assert_called_once_with(
402 'RequestsHandler.create: could not build OAuthRequest with the '411 'RequestsHandler.create: could not build OAuthRequest with the '
403 'given parameters %r:', self.data)412 'given parameters %r:', self.data)
@@ -409,26 +418,30 @@
409418
410 response = self.do_post()419 response = self.do_post()
411 self.assert_request_response(420 self.assert_request_response(
412 response, False, error="KeyError: u'\\xf1'")421 response, is_valid=False, account_verified=False,
422 error="KeyError: u'\\xf1'")
413 self.mock_logger.exception.assert_called_once_with(423 self.mock_logger.exception.assert_called_once_with(
414 'RequestsHandler.create: could not verify request:')424 'RequestsHandler.create: could not verify request:')
415425
416 def test_invalid_if_header_none(self):426 def test_invalid_if_header_none(self):
417 self.data['authorization'] = None427 self.data['authorization'] = None
418 response = self.do_post()428 response = self.do_post()
419 self.assert_request_response(response, False)429 self.assert_request_response(
430 response, is_valid=False, account_verified=False)
420 self.assertFalse(self.mock_logger.exception.called)431 self.assertFalse(self.mock_logger.exception.called)
421432
422 def test_invalid_if_not_header_given(self):433 def test_invalid_if_not_header_given(self):
423 self.data.pop('authorization')434 self.data.pop('authorization')
424 response = self.do_post()435 response = self.do_post()
425 self.assert_request_response(response, False)436 self.assert_request_response(
437 response, is_valid=False, account_verified=False)
426 self.assertFalse(self.mock_logger.exception.called)438 self.assertFalse(self.mock_logger.exception.called)
427439
428 def test_invalid_if_no_header_or_query_string(self):440 def test_invalid_if_no_header_or_query_string(self):
429 del self.data['authorization']441 del self.data['authorization']
430 response = self.do_post()442 response = self.do_post()
431 self.assert_request_response(response, False)443 self.assert_request_response(
444 response, is_valid=False, account_verified=False)
432 self.assertFalse(self.mock_logger.exception.called)445 self.assertFalse(self.mock_logger.exception.called)
433446
434 def test_invalid_if_bad_query_string(self):447 def test_invalid_if_bad_query_string(self):
@@ -436,7 +449,8 @@
436 # make sure there is no authorization header449 # make sure there is no authorization header
437 del self.data['authorization']450 del self.data['authorization']
438 response = self.do_post()451 response = self.do_post()
439 self.assert_request_response(response, False)452 self.assert_request_response(
453 response, is_valid=False, account_verified=False)
440 self.assertFalse(self.mock_logger.exception.called)454 self.assertFalse(self.mock_logger.exception.called)
441455
442 def test_invalid_if_query_string_invalid(self):456 def test_invalid_if_query_string_invalid(self):
@@ -451,7 +465,8 @@
451465
452 # do a post with correct params466 # do a post with correct params
453 response = self.do_post()467 response = self.do_post()
454 self.assert_request_response(response, False)468 self.assert_request_response(
469 response, is_valid=False, account_verified=False)
455 self.assertFalse(self.mock_logger.exception.called)470 self.assertFalse(self.mock_logger.exception.called)
456471
457 def test_url_mismatch(self):472 def test_url_mismatch(self):
@@ -460,14 +475,16 @@
460 self.data['http_url'] = new_url475 self.data['http_url'] = new_url
461 response = self.do_post()476 response = self.do_post()
462477
463 self.assert_request_response(response, False)478 self.assert_request_response(
479 response, is_valid=False, account_verified=False)
464 self.assertFalse(self.mock_logger.exception.called)480 self.assertFalse(self.mock_logger.exception.called)
465481
466 def test_method_mismatch(self):482 def test_method_mismatch(self):
467 self.data['http_method'] = self.default_http_method * 2483 self.data['http_method'] = self.default_http_method * 2
468 response = self.do_post()484 response = self.do_post()
469485
470 self.assert_request_response(response, False)486 self.assert_request_response(
487 response, is_valid=False, account_verified=False)
471 self.assertFalse(self.mock_logger.exception.called)488 self.assertFalse(self.mock_logger.exception.called)
472489
473 def test_valid_request(self):490 def test_valid_request(self):
@@ -475,7 +492,20 @@
475 response = self.do_post()492 response = self.do_post()
476493
477 self.assertEqual(response.status_code, 200)494 self.assertEqual(response.status_code, 200)
478 self.assert_request_response(response, True)495 self.assert_request_response(
496 response, is_valid=True, account_verified=True)
497 self.assertFalse(self.mock_logger.exception.called)
498
499 def test_valid_request_unverified_user(self):
500 self.account.verified_emails().delete()
501 assert not self.account.is_verified
502
503 # do a post with correct params
504 response = self.do_post()
505
506 self.assertEqual(response.status_code, 200)
507 self.assert_request_response(
508 response, is_valid=True, account_verified=False)
479 self.assertFalse(self.mock_logger.exception.called)509 self.assertFalse(self.mock_logger.exception.called)
480510
481 def test_valid_request_for_non_existent_account(self):511 def test_valid_request_for_non_existent_account(self):
@@ -483,7 +513,8 @@
483 # do a post with correct params for a non existent account513 # do a post with correct params for a non existent account
484 response = self.do_post()514 response = self.do_post()
485515
486 self.assert_request_response(response, False)516 self.assert_request_response(
517 response, is_valid=False, account_verified=False)
487518
488 def test_valid_request_for_inactive_account(self):519 def test_valid_request_for_inactive_account(self):
489 for status, name in AccountStatus._get_choices():520 for status, name in AccountStatus._get_choices():
@@ -499,7 +530,8 @@
499 # do a post with correct params for an inactive account530 # do a post with correct params for an inactive account
500 response = self.do_post()531 response = self.do_post()
501532
502 self.assert_request_response(response, False)533 self.assert_request_response(
534 response, is_valid=False, account_verified=False)
503 self.assertFalse(self.mock_logger.exception.called)535 self.assertFalse(self.mock_logger.exception.called)
504536
505 def test_valid_request_with_only_query_string(self):537 def test_valid_request_with_only_query_string(self):
@@ -511,7 +543,8 @@
511 response = self.do_post()543 response = self.do_post()
512544
513 self.assertEqual(response.status_code, 200)545 self.assertEqual(response.status_code, 200)
514 self.assert_request_response(response, True)546 self.assert_request_response(
547 response, is_valid=True, account_verified=True)
515 self.assertFalse(self.mock_logger.exception.called)548 self.assertFalse(self.mock_logger.exception.called)
516549
517 def test_query_string_ignored_if_authorization_header_found(self):550 def test_query_string_ignored_if_authorization_header_found(self):
@@ -521,15 +554,18 @@
521 response = self.do_post()554 response = self.do_post()
522555
523 self.assertEqual(response.status_code, 200)556 self.assertEqual(response.status_code, 200)
524 self.assert_request_response(response, True)557 self.assert_request_response(
558 response, is_valid=True, account_verified=True)
525 self.assertFalse(self.mock_logger.exception.called)559 self.assertFalse(self.mock_logger.exception.called)
526560
527 def test_replay_attack(self):561 def test_replay_attack(self):
528 response = self.do_post()562 response = self.do_post()
529 self.assert_request_response(response, True)563 self.assert_request_response(
564 response, is_valid=True, account_verified=True)
530565
531 response = self.do_post()566 response = self.do_post()
532 self.assert_request_response(response, False)567 self.assert_request_response(
568 response, is_valid=False, account_verified=False)
533569
534570
535class RequestsWithHEADTestCase(RequestsTestCase):571class RequestsWithHEADTestCase(RequestsTestCase):