Merge pull request #936 from alphagov/fix-forgot-password-empty-user-issue

Fix exception raised when a pending user attempts to complete 'forgotten password' flow

app/main/views/sign_in.py

@@ -28,7 +28,6 @@ from app.main.forms import LoginForm
 
 @main.route('/sign-in', methods=(['GET', 'POST']))
 def sign_in():
-
     if current_user and current_user.is_authenticated:
         return redirect(url_for('main.choose_service'))
 

app/main/views/two_factor.py

@@ -32,7 +32,8 @@ def two_factor():
                 user.set_password(session['user_details']['password'])
                 user.reset_failed_login_count()
                 user_api_client.update_user(user)
-            login_user(user, remember=True)
+            activated_user = user_api_client.activate_user(user)
+            login_user(activated_user, remember=True)
         finally:
             del session['user_details']
 

app/notify_client/user_api_client.py

@@ -122,5 +122,8 @@ class UserApiClient(BaseAPIClient):
         return True
 
     def activate_user(self, user):
-        user.state = 'active'
-        return self.update_user(user)
+        if user.state == 'pending':
+            user.state = 'active'
+            return self.update_user(user)
+        else:
+            return user

tests/app/main/views/test_two_factor.py

@@ -222,3 +222,24 @@ def test_two_factor_should_redirect_to_sign_in_if_user_not_in_session(app_,
                                    data={'sms_code': '12345'})
             assert response.status_code == 302
             assert response.location == url_for('main.sign_in', _external=True)
+
+
+def test_two_factor_should_activate_pending_user(app_,
+                                                 mocker,
+                                                 api_user_pending,
+                                                 mock_check_verify_code,
+                                                 mock_update_user
+                                                 ):
+    mocker.patch('app.user_api_client.get_user', return_value=api_user_pending)
+    mocker.patch('app.service_api_client.get_services', return_value={'data': []})
+    with app_.test_request_context():
+        with app_.test_client() as client:
+            with client.session_transaction() as session:
+                session['user_details'] = {
+                    'id': api_user_pending.id,
+                    'email_address': api_user_pending.email_address
+                }
+            client.post(url_for('main.two_factor'), data={'sms_code': '12345'})
+
+            assert mock_update_user.called
+            assert api_user_pending.is_active

tests/app/main/views/test_verify.py

@@ -39,15 +39,16 @@ def test_should_redirect_to_add_service_when_sms_code_is_correct(app_,
 
 
 def test_should_activate_user_after_verify(app_,
-                                           api_user_active,
-                                           mock_get_user,
+                                           mocker,
+                                           api_user_pending,
                                            mock_send_verify_code,
                                            mock_check_verify_code,
                                            mock_update_user):
+    mocker.patch('app.user_api_client.get_user', return_value=api_user_pending)
     with app_.test_request_context():
         with app_.test_client() as client:
             with client.session_transaction() as session:
-                session['user_details'] = {'email_address': api_user_active.email_address, 'id': api_user_active.id}
+                session['user_details'] = {'email_address': api_user_pending.email_address, 'id': api_user_pending.id}
             client.post(url_for('main.verify'),
                         data={'sms_code': '12345'})
             assert mock_update_user.called