Commit c80b5968 by John Cox

Fix two error message corner cases

parent ce0b6407
...@@ -244,11 +244,14 @@ def get_duplicate_provider(messages): ...@@ -244,11 +244,14 @@ def get_duplicate_provider(messages):
message there is a request to associate a social account S with an edX message there is a request to associate a social account S with an edX
account E if S is already associated with an edX account E'. account E if S is already associated with an edX account E'.
This messaging approach is stringly-typed and the particular string is
unfortunately not in a reusable constant.
Returns: Returns:
provider.BaseProvider child instance. The provider of the duplicate provider.BaseProvider child instance. The provider of the duplicate
account, or None if there is no duplicate (and hence no error). account, or None if there is no duplicate (and hence no error).
""" """
social_auth_messages = [m for m in messages if m.extra_tags.startswith('social-auth')] social_auth_messages = [m for m in messages if m.message.endswith('is already in use.')]
if not social_auth_messages: if not social_auth_messages:
return return
...@@ -348,7 +351,7 @@ def redirect_to_supplementary_form(strategy, details, response, uid, is_dashboar ...@@ -348,7 +351,7 @@ def redirect_to_supplementary_form(strategy, details, response, uid, is_dashboar
if is_dashboard: if is_dashboard:
return return
if is_login and user is None: if is_login and user is None or user and not user.is_active:
return redirect('/login', name='signin_user') return redirect('/login', name='signin_user')
if is_register and user is None: if is_register and user is None:
......
...@@ -210,6 +210,13 @@ class IntegrationTest(testutil.TestCase, test.TestCase): ...@@ -210,6 +210,13 @@ class IntegrationTest(testutil.TestCase, test.TestCase):
self.assertIn(argument_string, ['true', 'false']) self.assertIn(argument_string, ['true', 'false'])
self.assertEqual(boolean, True if argument_string == 'true' else False) self.assertEqual(boolean, True if argument_string == 'true' else False)
def assert_json_failure_response_is_inactive_account(self, response):
"""Asserts failure on /login for inactive account looks right."""
self.assertEqual(200, response.status_code) # Yes, it's a 200 even though it's a failure.
payload = json.loads(response.content)
self.assertFalse(payload.get('success'))
self.assertIn('This account has not been activated', payload.get('value'))
def assert_json_failure_response_is_missing_social_auth(self, response): def assert_json_failure_response_is_missing_social_auth(self, response):
"""Asserts failure on /login for missing social auth looks right.""" """Asserts failure on /login for missing social auth looks right."""
self.assertEqual(200, response.status_code) # Yes, it's a 200 even though it's a failure. self.assertEqual(200, response.status_code) # Yes, it's a 200 even though it's a failure.
...@@ -508,12 +515,13 @@ class IntegrationTest(testutil.TestCase, test.TestCase): ...@@ -508,12 +515,13 @@ class IntegrationTest(testutil.TestCase, test.TestCase):
# Monkey-patch storage for messaging; pylint: disable-msg=protected-access # Monkey-patch storage for messaging; pylint: disable-msg=protected-access
request._messages = fallback.FallbackStorage(request) request._messages = fallback.FallbackStorage(request)
middleware.ExceptionMiddleware().process_exception( middleware.ExceptionMiddleware().process_exception(
request, exceptions.AuthAlreadyAssociated(self.PROVIDER_CLASS.BACKEND_CLASS.name, 'duplicate')) request,
exceptions.AuthAlreadyAssociated(self.PROVIDER_CLASS.BACKEND_CLASS.name, 'account is already in use.'))
self.assert_dashboard_response_looks_correct( self.assert_dashboard_response_looks_correct(
student_views.dashboard(request), user, duplicate=True, linked=True) student_views.dashboard(request), user, duplicate=True, linked=True)
def test_full_pipeline_succeeds_for_signing_in_to_existing_account(self): def test_full_pipeline_succeeds_for_signing_in_to_existing_active_account(self):
# First, create, the request and strategy that store pipeline state, # First, create, the request and strategy that store pipeline state,
# configure the backend, and mock out wire traffic. # configure the backend, and mock out wire traffic.
request, strategy = self.get_request_and_strategy( request, strategy = self.get_request_and_strategy(
...@@ -522,6 +530,7 @@ class IntegrationTest(testutil.TestCase, test.TestCase): ...@@ -522,6 +530,7 @@ class IntegrationTest(testutil.TestCase, test.TestCase):
user = self.create_user_models_for_existing_account( user = self.create_user_models_for_existing_account(
strategy, 'user@example.com', 'password', self.get_username()) strategy, 'user@example.com', 'password', self.get_username())
self.assert_social_auth_exists_for_user(user, strategy) self.assert_social_auth_exists_for_user(user, strategy)
self.assertTrue(user.is_active)
# Begin! Ensure that the login form contains expected controls before # Begin! Ensure that the login form contains expected controls before
# the user starts the pipeline. # the user starts the pipeline.
...@@ -550,6 +559,17 @@ class IntegrationTest(testutil.TestCase, test.TestCase): ...@@ -550,6 +559,17 @@ class IntegrationTest(testutil.TestCase, test.TestCase):
actions.do_complete(strategy, social_views._do_login, user=user)) actions.do_complete(strategy, social_views._do_login, user=user))
self.assert_dashboard_response_looks_correct(student_views.dashboard(request), user) self.assert_dashboard_response_looks_correct(student_views.dashboard(request), user)
def test_signin_fails_if_account_not_active(self):
_, strategy = self.get_request_and_strategy(
auth_entry=pipeline.AUTH_ENTRY_LOGIN, redirect_uri='social:complete')
strategy.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy))
user = self.create_user_models_for_existing_account(strategy, 'user@example.com', 'password', self.get_username())
user.is_active = False
user.save()
self.assert_json_failure_response_is_inactive_account(student_views.login_user(strategy.request))
def test_signin_fails_if_no_account_associated(self): def test_signin_fails_if_no_account_associated(self):
_, strategy = self.get_request_and_strategy( _, strategy = self.get_request_and_strategy(
auth_entry=pipeline.AUTH_ENTRY_LOGIN, redirect_uri='social:complete') auth_entry=pipeline.AUTH_ENTRY_LOGIN, redirect_uri='social:complete')
...@@ -620,6 +640,14 @@ class IntegrationTest(testutil.TestCase, test.TestCase): ...@@ -620,6 +640,14 @@ class IntegrationTest(testutil.TestCase, test.TestCase):
created_user = self.get_user_by_email(strategy, email) created_user = self.get_user_by_email(strategy, email)
self.assert_password_overridden_by_pipeline(overridden_password, created_user.username) self.assert_password_overridden_by_pipeline(overridden_password, created_user.username)
# The user's account isn't created yet, so an attempt to complete the
# pipeline will error out on /login:
self.assert_redirect_to_login_looks_correct(
actions.do_complete(strategy, social_views._do_login, user=created_user))
# So we activate the account in order to verify the redirect to /dashboard:
created_user.is_active = True
created_user.save()
# Last step in the pipeline: we re-invoke the pipeline and expect to # Last step in the pipeline: we re-invoke the pipeline and expect to
# end up on /dashboard, with the correct social auth object now in the # end up on /dashboard, with the correct social auth object now in the
# backend and the correct user's data on display. # backend and the correct user's data on display.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment