diff --git a/app/main/views/sign_in.py b/app/main/views/sign_in.py index a119faada..f313c0855 100644 --- a/app/main/views/sign_in.py +++ b/app/main/views/sign_in.py @@ -33,24 +33,31 @@ def sign_in(): form.email_address.data, form.password.data ) - if user and user.state == 'pending': - return redirect(url_for('main.resend_email_verification', next=redirect_url)) + if user: + # 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'): - invited_user = InvitedUser.from_session() - if user.email_address.lower() != invited_user.email_address.lower(): - flash("You cannot accept an invite for another person.") - session.pop('invited_user_id', None) - abort(403) - else: - invited_user.accept_invite() - if user and user.sign_in(): - 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)) + if user.state == 'pending': + return redirect(url_for('main.resend_email_verification', next=redirect_url)) + + if user.is_active: + if session.get('invited_user_id'): + invited_user = InvitedUser.from_session() + if user.email_address.lower() != invited_user.email_address.lower(): + flash("You cannot accept an invite for another person.") + session.pop('invited_user_id', None) + abort(403) + else: + invited_user.accept_invite() + + user.send_login_code() + + 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 flash(Markup( diff --git a/app/models/user.py b/app/models/user.py index 0d95e1624..e52661b2b 100644 --- a/app/models/user.py +++ b/app/models/user.py @@ -142,20 +142,12 @@ class User(JSONModel, UserMixin): login_user(self) session['user_id'] = self.id - def sign_in(self): - - session['user_details'] = {"email": self.email_address, "id": self.id} - - if not self.is_active: - return False - + def send_login_code(self): if self.email_auth: user_api_client.send_verify_code(self.id, 'email', None, request.args.get('next')) if self.sms_auth: user_api_client.send_verify_code(self.id, 'sms', self.mobile_number) - return True - def sign_out(self): session.clear() # Update the db so the server also knows the user is logged out. diff --git a/tests/app/main/views/test_sign_in.py b/tests/app/main/views/test_sign_in.py index a96fa0ad5..c177ca974 100644 --- a/tests/app/main/views/test_sign_in.py +++ b/tests/app/main/views/test_sign_in.py @@ -1,7 +1,6 @@ import uuid import pytest -from bs4 import BeautifulSoup from flask import url_for 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( client, mock_get_user_by_email_pending, + api_user_pending, mock_verify_password, ): response = client.post( - url_for('main.sign_in'), data={ + url_for('main.sign_in'), + data={ 'email_address': 'pending_user@example.gov.uk', - 'password': 'val1dPassw0rd!'}, follow_redirects=True) - - page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser') - assert page.h1.string == 'Sign in' - assert response.status_code == 200 + 'password': 'val1dPassw0rd!' + } + ) + assert response.location == url_for('main.resend_email_verification', _external=True) + 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', [