From a741c128da0217ee3109baf0cb84fb92c8aa11ad Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Fri, 4 Dec 2015 14:40:16 +0000 Subject: [PATCH] 108537814: Implementation of 3 factor authentication. The post register endpoint will send a random 5 digit code via sms and another via email. If either code fails to send, the user will not be created and the person can register again. The codes are saved to the session cookie, and expire in 1 hour. Another iteration of this story will save the codes to a database. --- app/__init__.py | 4 +++ app/main/exceptions.py | 5 +++ app/main/views/index.py | 4 --- app/main/views/register.py | 51 +++++++++++++++++++++++---- app/notify_client/__init__.py | 0 app/notify_client/api_client.py | 8 +++++ config.py | 10 ++++++ requirements.txt | 4 ++- requirements_for_test.txt | 3 +- tests/app/main/views/test_register.py | 31 +++++++++++++--- 10 files changed, 103 insertions(+), 17 deletions(-) create mode 100644 app/main/exceptions.py create mode 100644 app/notify_client/__init__.py create mode 100644 app/notify_client/api_client.py diff --git a/app/__init__.py b/app/__init__.py index c0c0cd758..98fd560b9 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -9,6 +9,7 @@ from flask_wtf import CsrfProtect from webassets.filter import get_filter from werkzeug.exceptions import abort +from app.notify_client.api_client import AdminAPIClient from app.its_dangerous_session import ItsdangerousSessionInterface import app.proxy_fix from config import configs @@ -18,6 +19,8 @@ db = SQLAlchemy() login_manager = LoginManager() csrf = CsrfProtect() +admin_api_client = AdminAPIClient() + def create_app(config_name): application = Flask(__name__) @@ -37,6 +40,7 @@ def create_app(config_name): proxy_fix.init_app(application) application.session_interface = ItsdangerousSessionInterface() + admin_api_client.init_app(application) return application diff --git a/app/main/exceptions.py b/app/main/exceptions.py new file mode 100644 index 000000000..45f0515c0 --- /dev/null +++ b/app/main/exceptions.py @@ -0,0 +1,5 @@ + + +class AdminApiClientException(Exception): + def __init__(self, message): + self.value = message diff --git a/app/main/views/index.py b/app/main/views/index.py index 2ffe73da4..f664ee835 100644 --- a/app/main/views/index.py +++ b/app/main/views/index.py @@ -15,19 +15,16 @@ def govuk(): @main.route("/register-from-invite") -@login_required def registerfrominvite(): return render_template('register-from-invite.html') @main.route("/verify") -@login_required def verify(): return render_template('verify.html') @main.route("/verify-mobile") -@login_required def verifymobile(): return render_template('verify-mobile.html') @@ -50,7 +47,6 @@ def addservice(): @main.route("/two-factor") -@login_required def twofactor(): return render_template('two-factor.html') diff --git a/app/main/views/register.py b/app/main/views/register.py index 1f426de40..34f781157 100644 --- a/app/main/views/register.py +++ b/app/main/views/register.py @@ -1,10 +1,14 @@ -from datetime import datetime +from datetime import datetime, timedelta +from random import randint -from flask import render_template, redirect, jsonify -from flask_login import login_user +from flask import render_template, redirect, jsonify, session +from sqlalchemy.exc import SQLAlchemyError +from app import admin_api_client from app.main import main from app.main.dao import users_dao +from app.main.encryption import hashpw +from app.main.exceptions import AdminApiClientException from app.main.forms import RegisterUserForm from app.models import User @@ -26,10 +30,43 @@ def process_register(): created_at=datetime.now(), role_id=1) try: + sms_code = send_sms_code(form.mobile_number.data) + email_code = send_email_code(form.email_address.data) + session['sms_code'] = hashpw(sms_code) + session['email_code'] = hashpw(email_code) + session['expiry_date'] = str(datetime.now() + timedelta(hours=2)) users_dao.insert_user(user) - login_user(user) - return redirect('/two-factor') - except Exception as e: - return jsonify(database_error=e.message), 400 + except AdminApiClientException as e: + return jsonify(admin_api_client_error=e.value) + except SQLAlchemyError: + return jsonify(database_error='encountered database error'), 400 else: return jsonify(form.errors), 400 + return redirect('/verify') + + +def send_sms_code(mobile_number): + sms_code = _create_code() + try: + admin_api_client.send_sms(mobile_number, message=sms_code, token=admin_api_client.auth_token) + except: + raise AdminApiClientException('Exception when sending sms.') + return sms_code + + +def send_email_code(email): + email_code = _create_code() + try: + admin_api_client.send_email(email_address=email, + from_str='notify@digital.cabinet-office.gov.uk', + message=email_code, + subject='Verification code', + token=admin_api_client.auth_token) + except: + raise AdminApiClientException('Exception when sending email.') + + return email_code + + +def _create_code(): + return ''.join(["%s" % randint(0, 9) for _ in range(0, 5)]) diff --git a/app/notify_client/__init__.py b/app/notify_client/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/app/notify_client/api_client.py b/app/notify_client/api_client.py new file mode 100644 index 000000000..e32e8e37e --- /dev/null +++ b/app/notify_client/api_client.py @@ -0,0 +1,8 @@ +from __future__ import unicode_literals +from notify_client import NotifyAPIClient + + +class AdminAPIClient(NotifyAPIClient): + def init_app(self, app): + self.base_url = app.config['NOTIFY_DATA_API_URL'] + self.auth_token = app.config['NOTIFY_DATA_API_AUTH_TOKEN'] diff --git a/config.py b/config.py index 6c799b561..f373ed27a 100644 --- a/config.py +++ b/config.py @@ -1,3 +1,5 @@ +import os + class Config(object): DEBUG = False @@ -11,6 +13,14 @@ class Config(object): MAX_FAILED_LOGIN_COUNT = 10 PASS_SECRET_KEY = 'secret-key-unique-changeme' + SESSION_COOKIE_NAME = 'notify_admin_session' + SESSION_COOKIE_PATH = '/admin' + SESSION_COOKIE_HTTPONLY = True + SESSION_COOKIE_SECURE = True + + NOTIFY_DATA_API_URL = os.getenv('NOTIFY_API_URL', "http://localhost:6001") + NOTIFY_DATA_API_AUTH_TOKEN = os.getenv('NOTIFY_API_TOKEN', "pLuj5kat5auC9Ve") + WTF_CSRF_ENABLED = True SECRET_KEY = 'secret-key' HTTP_PROTOCOL = 'http' diff --git a/requirements.txt b/requirements.txt index 9ac4a3971..aaa53cebc 100644 --- a/requirements.txt +++ b/requirements.txt @@ -8,4 +8,6 @@ SQLAlchemy==1.0.5 SQLAlchemy-Utils==0.30.5 Flask-WTF==0.11 Flask-Login==0.2.11 -Flask-Bcrypt==0.6.2 \ No newline at end of file +Flask-Bcrypt==0.6.2 + +git+https://github.com/alphagov/notify-api-client.git@0.1.4#egg=notify-api-client==0.1.4 \ No newline at end of file diff --git a/requirements_for_test.txt b/requirements_for_test.txt index f8aeacc70..ebea6362b 100644 --- a/requirements_for_test.txt +++ b/requirements_for_test.txt @@ -1,3 +1,4 @@ -r requirements.txt pep8==1.5.7 -pytest==2.8.1 \ No newline at end of file +pytest==2.8.1 +pytest-mock==0.8.1 diff --git a/tests/app/main/views/test_register.py b/tests/app/main/views/test_register.py index 32da075c2..a40b93481 100644 --- a/tests/app/main/views/test_register.py +++ b/tests/app/main/views/test_register.py @@ -7,17 +7,22 @@ def test_render_register_returns_template_with_form(notifications_admin, notific assert 'Create an account' in response.get_data(as_text=True) -def test_process_register_creates_new_user(notifications_admin, notifications_admin_db): +def test_process_register_creates_new_user(notifications_admin, notifications_admin_db, mocker): + _set_up_mocker(mocker) + response = notifications_admin.test_client().post('/register', data={'name': 'Some One Valid', 'email_address': 'someone@example.gov.uk', 'mobile_number': '+441231231231', 'password': 'validPassword!'}) assert response.status_code == 302 - assert response.location == 'http://localhost/two-factor' + assert response.location == 'http://localhost/verify' -def test_process_register_returns_400_when_mobile_number_is_invalid(notifications_admin, notifications_admin_db): +def test_process_register_returns_400_when_mobile_number_is_invalid(notifications_admin, + notifications_admin_db, + mocker): + _set_up_mocker(mocker) response = notifications_admin.test_client().post('/register', data={'name': 'Bad Mobile', 'email_address': 'bad_mobile@example.gov.uk', @@ -28,7 +33,8 @@ def test_process_register_returns_400_when_mobile_number_is_invalid(notification assert 'Please enter a +44 mobile number' in response.get_data(as_text=True) -def test_should_return_400_when_email_is_not_gov_uk(notifications_admin, notifications_admin_db): +def test_should_return_400_when_email_is_not_gov_uk(notifications_admin, notifications_admin_db, mocker): + _set_up_mocker(mocker) response = notifications_admin.test_client().post('/register', data={'name': 'Bad Mobile', 'email_address': 'bad_mobile@example.not.right', @@ -39,6 +45,23 @@ def test_should_return_400_when_email_is_not_gov_uk(notifications_admin, notific assert 'Please enter a gov.uk email address' in response.get_data(as_text=True) +def test_should_add_verify_codes_on_session(notifications_admin, notifications_admin_db, mocker): + _set_up_mocker(mocker) + with notifications_admin.test_client() as client: + response = client.post('/register', + data={'name': 'Test Codes', + 'email_address': 'test_codes@example.gov.uk', + 'mobile_number': '+441234567890', + 'password': 'validPassword!'}) + assert response.status_code == 302 + assert 'notify_admin_session' in response.headers.get('Set-Cookie') + + +def _set_up_mocker(mocker): + mocker.patch("app.admin_api_client.send_sms") + mocker.patch("app.admin_api_client.send_email") + + def test_should_return_400_if_password_is_blacklisted(notifications_admin, notifications_admin_db): response = notifications_admin.test_client().post('/register', data={'name': 'Bad Mobile',