From e000552e56a0a41cc99a88044f0ee520d27acd0b Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Mon, 19 Mar 2018 16:38:57 +0000 Subject: [PATCH] redirect to show_accounts_or_dashboard on login show_accounts_or_dashboard has logic about where you should redirect to. If we let it do this, then that's nicer than duplicating its logic. We found that it wasn't accounting for orgs in redirects properly. --- app/main/views/two_factor.py | 9 +--- tests/app/main/views/test_new_password.py | 2 +- tests/app/main/views/test_two_factor.py | 50 ++++------------------- 3 files changed, 10 insertions(+), 51 deletions(-) diff --git a/app/main/views/two_factor.py b/app/main/views/two_factor.py index cf2dab0c2..0d311ac6b 100644 --- a/app/main/views/two_factor.py +++ b/app/main/views/two_factor.py @@ -13,7 +13,7 @@ from flask_login import current_user, login_user from itsdangerous import SignatureExpired from notifications_utils.url_safe_token import check_token -from app import service_api_client, user_api_client +from app import user_api_client from app.main import main from app.main.forms import TwoFactorForm from app.utils import redirect_to_sign_in @@ -112,9 +112,4 @@ def redirect_when_logged_in(user_id): if current_user.platform_admin: return redirect(url_for('main.platform_admin')) - services = service_api_client.get_active_services({'user_id': str(user_id)}).get('data', []) - - if len(services) == 1: - return redirect(url_for('main.service_dashboard', service_id=services[0]['id'])) - else: - return redirect(url_for('main.choose_account')) + return redirect(url_for('main.show_accounts_or_dashboard')) diff --git a/tests/app/main/views/test_new_password.py b/tests/app/main/views/test_new_password.py index 93b5a425f..d545638b3 100644 --- a/tests/app/main/views/test_new_password.py +++ b/tests/app/main/views/test_new_password.py @@ -100,7 +100,7 @@ def test_should_sign_in_when_password_reset_is_successful_for_email_auth( response = client.post(url_for('.new_password', token=token), data={'new_password': 'a-new_password'}) assert response.status_code == 302 - assert response.location == url_for('.choose_account', _external=True) + assert response.location == url_for('.show_accounts_or_dashboard', _external=True) assert mock_get_user_by_email_request_password_reset.called assert mock_reset_failed_login_count.called diff --git a/tests/app/main/views/test_two_factor.py b/tests/app/main/views/test_two_factor.py index 747dff750..b17b5bc55 100644 --- a/tests/app/main/views/test_two_factor.py +++ b/tests/app/main/views/test_two_factor.py @@ -1,5 +1,3 @@ -from unittest.mock import ANY - from bs4 import BeautifulSoup from flask import url_for from tests.conftest import SERVICE_ONE_ID, normalize_spaces, set_config @@ -21,38 +19,12 @@ def test_should_render_two_factor_page( assert '''We’ve sent you a text message with a security code.''' in response.get_data(as_text=True) -def test_should_login_user_and_redirect_to_service_dashboard( - client, - api_user_active, - mock_get_user, - mock_get_user_by_email, - mock_check_verify_code, - mock_get_services_with_one_service, - mock_events, -): - with client.session_transaction() as session: - session['user_details'] = { - 'id': api_user_active.id, - 'email': api_user_active.email_address} - response = client.post(url_for('main.two_factor'), - data={'sms_code': '12345'}) - assert response.status_code == 302 - assert response.location == url_for( - 'main.service_dashboard', - service_id=SERVICE_ONE_ID, - _external=True - ) - - mock_events.assert_called_with('sucessful_login', ANY) - - def test_should_login_user_and_should_redirect_to_next_url( client, api_user_active, mock_get_user, mock_get_user_by_email, mock_check_verify_code, - mock_get_services, ): with client.session_transaction() as session: session['user_details'] = { @@ -83,20 +55,15 @@ def test_should_login_user_and_not_redirect_to_external_url( response = client.post(url_for('main.two_factor', next='http://www.google.com'), data={'sms_code': '12345'}) assert response.status_code == 302 - assert response.location == url_for( - 'main.service_dashboard', - service_id=SERVICE_ONE_ID, - _external=True - ) + assert response.location == url_for('main.show_accounts_or_dashboard', _external=True) -def test_should_login_user_and_redirect_to_choose_accounts( +def test_should_login_user_and_redirect_to_show_accounts( client, api_user_active, mock_get_user, mock_get_user_by_email, mock_check_verify_code, - mock_get_services, ): with client.session_transaction() as session: session['user_details'] = { @@ -106,7 +73,7 @@ def test_should_login_user_and_redirect_to_choose_accounts( data={'sms_code': '12345'}) assert response.status_code == 302 - assert response.location == url_for('main.choose_account', _external=True) + assert response.location == url_for('main.show_accounts_or_dashboard', _external=True) def test_should_return_200_with_sms_code_error_when_sms_code_is_wrong( @@ -159,11 +126,8 @@ def test_two_factor_should_set_password_when_new_password_exists_in_session( response = client.post(url_for('main.two_factor'), data={'sms_code': '12345'}) assert response.status_code == 302 - assert response.location == url_for( - 'main.service_dashboard', - service_id=SERVICE_ONE_ID, - _external=True - ) + assert response.location == url_for('main.show_accounts_or_dashboard', _external=True) + mock_update_user_password.assert_called_once_with(api_user_active.id, password='changedpassword') @@ -230,7 +194,7 @@ def test_valid_two_factor_email_link_logs_in_user( ) assert response.status_code == 302 - assert response.location == url_for('main.service_dashboard', service_id=SERVICE_ONE_ID, _external=True) + assert response.location == url_for('main.show_accounts_or_dashboard', _external=True) def test_two_factor_email_link_has_expired( @@ -321,4 +285,4 @@ def test_two_factor_email_link_used_when_user_already_logged_in( url_for('main.two_factor_email', token=valid_token) ) assert response.status_code == 302 - assert response.location == url_for('main.choose_account', _external=True) + assert response.location == url_for('main.show_accounts_or_dashboard', _external=True)