Merge pull request #122 from alphagov/sign_in_fixes

Sign in fixes
This commit is contained in:
Rebecca Law
2016-01-28 16:47:20 +00:00
6 changed files with 53 additions and 64 deletions

View File

@@ -105,9 +105,6 @@ class LoginForm(Form):
class RegisterUserForm(Form): class RegisterUserForm(Form):
def __init__(self, unique_email_func, *args, **kwargs):
self.unique_email_func = unique_email_func
super(RegisterUserForm, self).__init__(*args, **kwargs)
name = StringField('Full name', name = StringField('Full name',
validators=[DataRequired(message='Name can not be empty')]) validators=[DataRequired(message='Name can not be empty')])
@@ -115,11 +112,6 @@ class RegisterUserForm(Form):
mobile_number = mobile_number() mobile_number = mobile_number()
password = password() password = password()
def validate_email_address(self, field):
# Validate email address is unique.
if not self.unique_email_func(field.data):
raise ValidationError('Email address already exists')
class TwoFactorForm(Form): class TwoFactorForm(Form):
def __init__(self, validate_code_func, *args, **kwargs): def __init__(self, validate_code_func, *args, **kwargs):
@@ -221,17 +213,8 @@ class TemplateForm(Form):
class ForgotPasswordForm(Form): class ForgotPasswordForm(Form):
def __init__(self, user_email_exists_func, *args, **kwargs):
self._user_email_exists_func = user_email_exists_func
super(ForgotPasswordForm, self).__init__(*args, **kwargs)
email_address = email_address() email_address = email_address()
def validate_email_address(self, field):
if not self._user_email_exists_func(field.data):
raise ValidationError('The email is not registered on our system')
class NewPasswordForm(Form): class NewPasswordForm(Form):
new_password = password() new_password = password()

View File

@@ -8,14 +8,13 @@ from app.notify_client.sender import send_change_password_email
@main.route('/forgot-password', methods=['GET', 'POST']) @main.route('/forgot-password', methods=['GET', 'POST'])
def forgot_password(): def forgot_password():
def _email_exists(email): form = ForgotPasswordForm()
return not users_dao.is_email_unique(email)
form = ForgotPasswordForm(_email_exists)
if form.validate_on_submit(): if form.validate_on_submit():
user = users_dao.get_user_by_email(form.email_address.data) if not users_dao.is_email_unique(form.email_address.data):
users_dao.request_password_reset(user) user = users_dao.get_user_by_email(form.email_address.data)
send_change_password_email(form.email_address.data) users_dao.request_password_reset(user)
return render_template('views/password-reset-sent.html') send_change_password_email(form.email_address.data)
return render_template('views/password-reset-sent.html')
flash('There was an error processing your request')
return render_template('views/forgot-password.html', form=form) return render_template('views/forgot-password.html', form=form)

View File

@@ -5,7 +5,8 @@ from flask import (
redirect, redirect,
session, session,
abort, abort,
url_for url_for,
flash
) )
from flask.ext.login import current_user from flask.ext.login import current_user
@@ -24,29 +25,32 @@ def register():
if current_user and current_user.is_authenticated(): if current_user and current_user.is_authenticated():
return redirect(url_for('main.choose_service')) return redirect(url_for('main.choose_service'))
form = RegisterUserForm(users_dao.is_email_unique) form = RegisterUserForm()
if form.validate_on_submit(): if form.validate_on_submit():
try: if users_dao.is_email_unique(form.email_address.data):
user = user_api_client.register_user(form.name.data, try:
form.email_address.data, user = user_api_client.register_user(form.name.data,
form.mobile_number.data, form.email_address.data,
form.password.data) form.mobile_number.data,
except HTTPError as e: form.password.data)
if e.status_code == 404: except HTTPError as e:
abort(404) if e.status_code == 404:
else: abort(404)
raise e else:
raise e
# TODO possibly there should be some exception handling # TODO possibly there should be some exception handling
# for sending sms and email codes. # for sending sms and email codes.
# How do we report to the user there is a problem with # How do we report to the user there is a problem with
# sending codes apart from service unavailable? # sending codes apart from service unavailable?
# at the moment i believe http 500 is fine. # at the moment i believe http 500 is fine.
users_dao.send_verify_code(user.id, 'sms') users_dao.send_verify_code(user.id, 'sms')
users_dao.send_verify_code(user.id, 'email') users_dao.send_verify_code(user.id, 'email')
session['expiry_date'] = str(datetime.now() + timedelta(hours=1)) session['expiry_date'] = str(datetime.now() + timedelta(hours=1))
session['user_details'] = {"email": user.email_address, "id": user.id} session['user_details'] = {"email": user.email_address, "id": user.id}
return redirect(url_for('main.verify')) return redirect(url_for('main.verify'))
else:
flash('There was an error registering your account')
return render_template('views/register.html', form=form) return render_template('views/register.html', form=form)

View File

@@ -3,7 +3,8 @@ from flask import (
redirect, redirect,
url_for, url_for,
session, session,
abort abort,
flash
) )
from flask.ext.login import current_user from flask.ext.login import current_user
@@ -19,26 +20,26 @@ def sign_in():
return redirect(url_for('main.choose_service')) return redirect(url_for('main.choose_service'))
form = LoginForm() form = LoginForm()
if form.validate_on_submit(): if form.validate_on_submit():
user = _get_and_verify_user(form.email_address.data, form.password.data) user = users_dao.get_user_by_email(form.email_address.data)
user = _get_and_verify_user(user, form.password.data)
if user: if user:
users_dao.send_verify_code(user.id, 'sms')
session['user_details'] = {"email": user.email_address, "id": user.id} session['user_details'] = {"email": user.email_address, "id": user.id}
return redirect(url_for('.two_factor')) if user.state == 'pending':
else: return redirect(url_for('.verify'))
# Vague error message for login in case of user not known, locked, inactive or password not verified elif user.is_active():
form.password.errors.append('Username or password is incorrect') users_dao.send_verify_code(user.id, 'sms')
return redirect(url_for('.two_factor'))
# Vague error message for login in case of user not known, locked, inactive or password not verified
flash('Username or password is incorrect')
return render_template('views/signin.html', form=form) return render_template('views/signin.html', form=form)
def _get_and_verify_user(email_address, password): def _get_and_verify_user(user, password):
user = users_dao.get_user_by_email(email_address)
if not user: if not user:
return None return None
elif user.is_locked(): elif user.is_locked():
return None return None
elif not user.is_active():
return None
elif not users_dao.verify_password(user.id, password): elif not users_dao.verify_password(user.id, password):
return None return None
else: else:

View File

@@ -3,7 +3,7 @@ from app.main.forms import RegisterUserForm
def test_should_raise_validation_error_for_password(app_, mock_get_user_by_email): def test_should_raise_validation_error_for_password(app_, mock_get_user_by_email):
form = RegisterUserForm(users_dao.get_user_by_email) form = RegisterUserForm()
form.name.data = 'test' form.name.data = 'test'
form.email_address.data = 'teset@example.gov.uk' form.email_address.data = 'teset@example.gov.uk'
form.mobile_number.data = '+441231231231' form.mobile_number.data = '+441231231231'

View File

@@ -42,8 +42,8 @@ def test_process_sign_in_return_2fa_template(app_,
url_for('main.sign_in'), data={ url_for('main.sign_in'), data={
'email_address': 'valid@example.gov.uk', 'email_address': 'valid@example.gov.uk',
'password': 'val1dPassw0rd!'}) 'password': 'val1dPassw0rd!'})
assert response.status_code == 302 assert response.status_code == 302
assert response.location == 'http://localhost/two-factor' assert response.location == url_for('.two_factor', _external=True)
mock_verify_password.assert_called_with(api_user_active.id, 'val1dPassw0rd!') mock_verify_password.assert_called_with(api_user_active.id, 'val1dPassw0rd!')
@@ -80,11 +80,13 @@ def test_should_return_200_when_user_does_not_exist(app_, mock_get_user_by_email
assert 'Username or password is incorrect' in response.get_data(as_text=True) assert 'Username or password is incorrect' in response.get_data(as_text=True)
def test_should_return_200_when_user_is_pending(app_, mock_get_user_by_email_pending): def test_should_return_redirect_when_user_is_pending(app_,
mock_get_user_by_email_pending,
mock_verify_password):
with app_.test_request_context(): with app_.test_request_context():
response = app_.test_client().post( response = app_.test_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!'}) 'password': 'val1dPassw0rd!'})
assert response.status_code == 200 assert response.status_code == 302
assert 'Username or password is incorrect' in response.get_data(as_text=True) assert response.location == url_for('main.verify', _external=True)