ensure user details are always in the session after entering password

We signal that we're mid-way through the sign-in flow by adding a
`user_details` dict to the session.

previously, we'd only put a user's details in the session in `User.sign_in`,
just before sending any 2fa prompt and redirecting to the two factor
pages.

However, we found a bug where a user with no session (eg, using a fresh
browser) tried to log in, but they had never clicked the link to
validate their email address when registering. Their user's state was
still in "pending", so we redirected to `main.resend_email_verification`
as intended - however, they didn't have anything in the session and the
resend page expected to get the email address to resend to out of that.

To be safe, as soon as we've confirmed the user has entered their
password correctly, lets save the session data at that point. That way
any redirects will be fine.
This commit is contained in:
Leo Hemsted
2021-06-29 18:13:25 +01:00
parent 5534ecb5a4
commit 7b3751240c
3 changed files with 37 additions and 33 deletions
+24 -17
View File
@@ -33,24 +33,31 @@ def sign_in():
form.email_address.data, form.password.data form.email_address.data, form.password.data
) )
if user and user.state == 'pending': if user:
return redirect(url_for('main.resend_email_verification', next=redirect_url)) # add user to session to mark us as in the process of signing the user in
session['user_details'] = {"email": user.email_address, "id": user.id}
if user and session.get('invited_user_id'): if user.state == 'pending':
invited_user = InvitedUser.from_session() return redirect(url_for('main.resend_email_verification', next=redirect_url))
if user.email_address.lower() != invited_user.email_address.lower():
flash("You cannot accept an invite for another person.") if user.is_active:
session.pop('invited_user_id', None) if session.get('invited_user_id'):
abort(403) invited_user = InvitedUser.from_session()
else: if user.email_address.lower() != invited_user.email_address.lower():
invited_user.accept_invite() flash("You cannot accept an invite for another person.")
if user and user.sign_in(): session.pop('invited_user_id', None)
if user.sms_auth: abort(403)
return redirect(url_for('.two_factor_sms', next=redirect_url)) else:
if user.email_auth: invited_user.accept_invite()
return redirect(url_for('.two_factor_email_sent', next=redirect_url))
if user.webauthn_auth: user.send_login_code()
return redirect(url_for('.two_factor_webauthn', next=redirect_url))
if user.sms_auth:
return redirect(url_for('.two_factor_sms', next=redirect_url))
if user.email_auth:
return redirect(url_for('.two_factor_email_sent', next=redirect_url))
if user.webauthn_auth:
return redirect(url_for('.two_factor_webauthn', next=redirect_url))
# Vague error message for login in case of user not known, locked, inactive or password not verified # Vague error message for login in case of user not known, locked, inactive or password not verified
flash(Markup( flash(Markup(
+1 -9
View File
@@ -142,20 +142,12 @@ class User(JSONModel, UserMixin):
login_user(self) login_user(self)
session['user_id'] = self.id session['user_id'] = self.id
def sign_in(self): def send_login_code(self):
session['user_details'] = {"email": self.email_address, "id": self.id}
if not self.is_active:
return False
if self.email_auth: if self.email_auth:
user_api_client.send_verify_code(self.id, 'email', None, request.args.get('next')) user_api_client.send_verify_code(self.id, 'email', None, request.args.get('next'))
if self.sms_auth: if self.sms_auth:
user_api_client.send_verify_code(self.id, 'sms', self.mobile_number) user_api_client.send_verify_code(self.id, 'sms', self.mobile_number)
return True
def sign_out(self): def sign_out(self):
session.clear() session.clear()
# Update the db so the server also knows the user is logged out. # Update the db so the server also knows the user is logged out.
+12 -7
View File
@@ -1,7 +1,6 @@
import uuid import uuid
import pytest import pytest
from bs4 import BeautifulSoup
from flask import url_for from flask import url_for
from app.models.user import User from app.models.user import User
@@ -215,16 +214,22 @@ def test_should_return_200_when_user_does_not_exist(
def test_should_return_redirect_when_user_is_pending( def test_should_return_redirect_when_user_is_pending(
client, client,
mock_get_user_by_email_pending, mock_get_user_by_email_pending,
api_user_pending,
mock_verify_password, mock_verify_password,
): ):
response = client.post( response = client.post(
url_for('main.sign_in'), data={ url_for('main.sign_in'),
data={
'email_address': 'pending_user@example.gov.uk', 'email_address': 'pending_user@example.gov.uk',
'password': 'val1dPassw0rd!'}, follow_redirects=True) 'password': 'val1dPassw0rd!'
}
page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') )
assert page.h1.string == 'Sign in' assert response.location == url_for('main.resend_email_verification', _external=True)
assert response.status_code == 200 with client.session_transaction() as s:
assert s['user_details'] == {
'email': api_user_pending['email_address'],
'id': api_user_pending['id']
}
@pytest.mark.parametrize('redirect_url', [ @pytest.mark.parametrize('redirect_url', [