Fix session bug when changing user’s email address

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.
This commit is contained in:
Chris Hill-Scott
2020-01-13 12:03:39 +00:00
parent c391729dc0
commit 72eb6ef99a
2 changed files with 18 additions and 6 deletions

View File

@@ -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',

View File

@@ -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',