From 72eb6ef99a81409165d811402ea140eb8f678ab7 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 13 Jan 2020 12:03:39 +0000 Subject: [PATCH] =?UTF-8?q?Fix=20session=20bug=20when=20changing=20user?= =?UTF-8?q?=E2=80=99s=20email=20address?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The session key we use is global. This means if you open the edit page for two different users in two different tabs the session for the first tab is overwritten with the session from the second tab. This means the two users are both set to the same email address, which causes an exception (email addresses are unique). This commit fixes that bug by including the user ID in the session ID. --- app/main/views/manage_users.py | 10 ++++++---- tests/app/main/views/test_manage_users.py | 14 ++++++++++++-- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/app/main/views/manage_users.py b/app/main/views/manage_users.py index 5499e0907..5f977bf7c 100644 --- a/app/main/views/manage_users.py +++ b/app/main/views/manage_users.py @@ -144,6 +144,7 @@ def remove_user_from_service(service_id, user_id): def edit_user_email(service_id, user_id): user = current_service.get_team_member(user_id) user_email = user.email_address + session_key = 'team_member_email_change-{}'.format(user_id) if is_gov_user(user_email): form = ChangeEmailForm(User.already_registered, email_address=user_email) @@ -154,7 +155,7 @@ def edit_user_email(service_id, user_id): return redirect(url_for('.manage_users', service_id=current_service.id)) if form.validate_on_submit(): - session['team_member_email_change'] = form.email_address.data + session[session_key] = form.email_address.data return redirect(url_for('.confirm_edit_user_email', user_id=user.id, service_id=service_id)) @@ -170,8 +171,9 @@ def edit_user_email(service_id, user_id): @user_has_permissions('manage_service') def confirm_edit_user_email(service_id, user_id): user = current_service.get_team_member(user_id) - if 'team_member_email_change' in session: - new_email = session['team_member_email_change'] + session_key = 'team_member_email_change-{}'.format(user_id) + if session_key in session: + new_email = session[session_key] else: return redirect(url_for( '.edit_user_email', @@ -186,7 +188,7 @@ def confirm_edit_user_email(service_id, user_id): else: create_email_change_event(user.id, current_user.id, user.email_address, new_email) finally: - session.pop('team_member_email_change', None) + session.pop(session_key, None) return redirect(url_for( '.manage_users', diff --git a/tests/app/main/views/test_manage_users.py b/tests/app/main/views/test_manage_users.py index acf3589ac..07f3ec1b6 100644 --- a/tests/app/main/views/test_manage_users.py +++ b/tests/app/main/views/test_manage_users.py @@ -1079,6 +1079,10 @@ def test_edit_user_email_redirects_to_confirmation( _external=True, ), ) + with client_request.session_transaction() as session: + assert session[ + 'team_member_email_change-{}'.format(active_user_with_permissions['id']) + ] == 'test@user.gov.uk' def test_edit_user_email_without_changing_goes_back_to_team_members( @@ -1179,6 +1183,8 @@ def test_edit_user_email_cannot_change_a_gov_email_address_to_a_non_gov_email_ad _expected_status=200, ) assert 'Enter a government email address.' in page.find('span', class_='error-message').text + with client_request.session_transaction() as session: + assert 'team_member_email_change-'.format(active_user_with_permissions['id']) not in session def test_confirm_edit_user_email_page( @@ -1189,7 +1195,9 @@ def test_confirm_edit_user_email_page( ): new_email = 'new_email@gov.uk' with client_request.session_transaction() as session: - session['team_member_email_change'] = new_email + session[ + 'team_member_email_change-{}'.format(active_user_with_permissions['id']) + ] = new_email page = client_request.get( 'main.confirm_edit_user_email', @@ -1253,7 +1261,9 @@ def test_confirm_edit_user_email_changes_user_email( new_email = 'new_email@gov.uk' with client_request.session_transaction() as session: - session['team_member_email_change'] = new_email + session[ + 'team_member_email_change-{}'.format(api_user_active['id']) + ] = new_email client_request.post( 'main.confirm_edit_user_email',