From 7f96ef5a25203f89a75f74622ebe89a76e298958 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Fri, 27 Nov 2015 09:47:29 +0000 Subject: [PATCH 1/9] 108536490: Initial effort to implement log in Add endpoint for post to /sign-in Initialise role data --- app/main/__init__.py | 2 +- app/main/dao/users_dao.py | 6 ++++ app/main/encryption.py | 7 ++++ app/main/forms.py | 14 ++++++++ app/main/views/index.py | 5 --- app/main/views/sign_in.py | 43 +++++++++++++++++++++++ app/templates/signin.html | 31 +++++++++------- config.py | 1 + migrations/versions/20_initialise_data.py | 16 +++++++++ requirements.txt | 2 ++ tests/app/main/dao/test_get_all_users.py | 32 ----------------- tests/app/main/dao/test_roles_dao.py | 9 +++-- tests/app/main/dao/test_users_dao.py | 41 +++++++++++++++++++++ tests/app/main/test_encyption.py | 9 +++++ tests/app/main/views/__init__.py | 0 tests/app/main/views/test_sign_in.py | 17 +++++++++ tests/conftest.py | 4 +-- 17 files changed, 183 insertions(+), 56 deletions(-) create mode 100644 app/main/encryption.py create mode 100644 app/main/forms.py create mode 100644 app/main/views/sign_in.py create mode 100644 migrations/versions/20_initialise_data.py delete mode 100644 tests/app/main/dao/test_get_all_users.py create mode 100644 tests/app/main/test_encyption.py create mode 100644 tests/app/main/views/__init__.py create mode 100644 tests/app/main/views/test_sign_in.py diff --git a/app/main/__init__.py b/app/main/__init__.py index e3a397f4d..5cf60b5fe 100644 --- a/app/main/__init__.py +++ b/app/main/__init__.py @@ -3,4 +3,4 @@ from flask import Blueprint main = Blueprint('main', __name__) -from app.main.views import index +from app.main.views import index, sign_in diff --git a/app/main/dao/users_dao.py b/app/main/dao/users_dao.py index 97bdfb9f3..95d68eafa 100644 --- a/app/main/dao/users_dao.py +++ b/app/main/dao/users_dao.py @@ -1,8 +1,10 @@ from app import db from app.models import Users +from app.main.encryption import encrypt def insert_user(user): + user.password = encrypt(user.password) db.session.add(user) db.session.commit() @@ -13,3 +15,7 @@ def get_user_by_id(id): def get_all_users(): return Users.query.all() + + +def get_user_by_email(email_address): + return Users.query.filter_by(email_address=email_address).first() diff --git a/app/main/encryption.py b/app/main/encryption.py new file mode 100644 index 000000000..e42a42f22 --- /dev/null +++ b/app/main/encryption.py @@ -0,0 +1,7 @@ +import hashlib +from flask import current_app + + +def encrypt(value): + key = current_app.config['SECRET_KEY'] + return hashlib.sha256((key + value).encode('UTF-8')).hexdigest() diff --git a/app/main/forms.py b/app/main/forms.py new file mode 100644 index 000000000..a2cf5d9e9 --- /dev/null +++ b/app/main/forms.py @@ -0,0 +1,14 @@ +from flask_wtf import Form +from wtforms import StringField, PasswordField +from wtforms.validators import DataRequired, Email, Length + + +class LoginForm(Form): + email_address = StringField('Email address', validators=[ + Length(255), + DataRequired(message='Email cannot be empty'), + Email(message='Please enter a valid email address') + ]) + password = PasswordField('Password', validators=[ + DataRequired(message='Please enter your password') + ]) diff --git a/app/main/views/index.py b/app/main/views/index.py index 34cbd3e4c..de48cb9e6 100644 --- a/app/main/views/index.py +++ b/app/main/views/index.py @@ -43,11 +43,6 @@ def dashboard(): return render_template('dashboard.html') -@main.route("/sign-in") -def signin(): - return render_template('signin.html') - - @main.route("/add-service") def addservice(): return render_template('add-service.html') diff --git a/app/main/views/sign_in.py b/app/main/views/sign_in.py new file mode 100644 index 000000000..e95f661bf --- /dev/null +++ b/app/main/views/sign_in.py @@ -0,0 +1,43 @@ +from datetime import datetime + +from flask import render_template, redirect, url_for, jsonify +from flask_login import login_user + +from app.main import main +from app.main.forms import LoginForm +from app.main.dao import users_dao +from app.models import Users +from app.main.encryption import encrypt + + +@main.route("/sign-in", methods=(['GET'])) +def render_sign_in(): + return render_template('signin.html', form=LoginForm()) + + +@main.route('/sign-in', methods=(['POST'])) +def process_sign_in(): + form = LoginForm() + if form.validate_on_submit(): + user = users_dao.get_user_by_email(form.email_address) + if user is None: + return jsonify(authorization=False), 404 + if user.password == encrypt(form.password): + login_user(user) + else: + return jsonify(authorization=False), 404 + + return redirect('/two-factor') + + +@main.route('/create_user', methods=(['POST'])) +def create_user_for_test(): + form = LoginForm() + user = Users(email_address=form.email_address, + name=form.email_address, + password=form.password, + created_at=datetime.now(), + role_id=1) + users_dao.insert_user(user) + + return 'created' diff --git a/app/templates/signin.html b/app/templates/signin.html index 3d6609010..7aea22245 100644 --- a/app/templates/signin.html +++ b/app/templates/signin.html @@ -1,7 +1,7 @@ {% extends "admin_template.html" %} {% block page_title %} -Hello world! +Sign in {% endblock %} {% block content %} @@ -12,19 +12,24 @@ Hello world!

If you do not have an account, you can register.

-

- -
-

-

- -
- Forgotten password? -

+
-

- Continue -

+

+ + {{ form.email_address(class="form-control-2-3", autocomplete="off") }}
+

+

+ + {{ form.password(class="form-control-1-4", autocomplete="off") }}
+

+

+ Forgotten password? +

+ +

+ Continue +

+
diff --git a/config.py b/config.py index 002f99162..5c1ecfea1 100644 --- a/config.py +++ b/config.py @@ -9,6 +9,7 @@ class Config(object): SQLALCHEMY_RECORD_QUERIES = True SQLALCHEMY_DATABASE_URI = 'postgresql://localhost/notifications_admin' MAX_FAILED_LOGIN_COUNT = 10 + SECRET_KEY = 'secret-key-unique-changeme' class Development(Config): diff --git a/migrations/versions/20_initialise_data.py b/migrations/versions/20_initialise_data.py new file mode 100644 index 000000000..4784310e6 --- /dev/null +++ b/migrations/versions/20_initialise_data.py @@ -0,0 +1,16 @@ +# revision identifiers, used by Alembic. +revision = '20_initialise_data' +down_revision = None + +from alembic import op + +def upgrade(): + op.bulk_insert('roles', + [ + {'role': 'plaform_admin'}, + {'role': 'service_user'} + ]) + +def downgrade(): + op.drop_table('users') + op.drop_table('roles') diff --git a/requirements.txt b/requirements.txt index b415c290c..b66dabd3e 100644 --- a/requirements.txt +++ b/requirements.txt @@ -6,3 +6,5 @@ Flask-SQLAlchemy==2.0 psycopg2==2.6.1 SQLAlchemy==1.0.5 SQLAlchemy-Utils==0.30.5 +Flask-WTF==0.11 +Flask-Login==0.2.11 \ No newline at end of file diff --git a/tests/app/main/dao/test_get_all_users.py b/tests/app/main/dao/test_get_all_users.py deleted file mode 100644 index 96229db72..000000000 --- a/tests/app/main/dao/test_get_all_users.py +++ /dev/null @@ -1,32 +0,0 @@ -from datetime import datetime - -from app.main.dao import users_dao -from app.models import Users - - -def test_get_all_users_returns_all_users(notifications_admin, notifications_admin_db): - user1 = Users(name='test one', - password='somepassword', - email_address='test1@get_all.gov.uk', - mobile_number='+441234123412', - created_at=datetime.now(), - role_id=1) - user2 = Users(name='test two', - password='some2ndpassword', - email_address='test2@get_all.gov.uk', - mobile_number='+441234123412', - created_at=datetime.now(), - role_id=1) - user3 = Users(name='test three', - password='some2ndpassword', - email_address='test2@get_all.gov.uk', - mobile_number='+441234123412', - created_at=datetime.now(), - role_id=1) - - users_dao.insert_user(user1) - users_dao.insert_user(user2) - users_dao.insert_user(user3) - users = users_dao.get_all_users() - assert len(users) == 3 - assert users == [user1, user2, user3] diff --git a/tests/app/main/dao/test_roles_dao.py b/tests/app/main/dao/test_roles_dao.py index ac2ce2193..14d28102d 100644 --- a/tests/app/main/dao/test_roles_dao.py +++ b/tests/app/main/dao/test_roles_dao.py @@ -13,9 +13,12 @@ def test_insert_role_should_be_able_to_get_role(notifications_admin, notificatio assert saved_role == role -def test_insert_role_will_throw_error_if_role_already_exists(): +def test_insert_role_will_throw_error_if_role_already_exists(notifications_admin, notifications_admin_db): + role1 = roles_dao.get_role_by_id(1) + assert role1.id == 1 + role = Roles(id=1, role='cannot create a duplicate') - with pytest.raises(sqlalchemy.exc.IntegrityError) as error: + with pytest.raises(sqlalchemy.orm.exc.FlushError) as error: roles_dao.insert_role(role) - assert 'duplicate key value violates unique constraint "roles_pkey"' in str(error.value) + assert 'conflicts with persistent instance' in str(error.value) diff --git a/tests/app/main/dao/test_users_dao.py b/tests/app/main/dao/test_users_dao.py index a4b8a910c..3f734ff87 100644 --- a/tests/app/main/dao/test_users_dao.py +++ b/tests/app/main/dao/test_users_dao.py @@ -30,3 +30,44 @@ def test_insert_user_with_role_that_does_not_exist_fails(notifications_admin, no with pytest.raises(sqlalchemy.exc.IntegrityError) as error: users_dao.insert_user(user) assert 'insert or update on table "users" violates foreign key constraint "users_role_id_fkey"' in str(error.value) + + +def test_get_user_by_email(notifications_admin, notifications_admin_db): + user = Users(name='test_get_by_email', + password='somepassword', + email_address='email@example.gov.uk', + mobile_number='+441234153412', + created_at=datetime.now(), + role_id=1) + + users_dao.insert_user(user) + retrieved = users_dao.get_user_by_email(user.email_address) + assert retrieved == user + + +def test_get_all_users_returns_all_users(notifications_admin, notifications_admin_db): + user1 = Users(name='test one', + password='somepassword', + email_address='test1@get_all.gov.uk', + mobile_number='+441234123412', + created_at=datetime.now(), + role_id=1) + user2 = Users(name='test two', + password='some2ndpassword', + email_address='test2@get_all.gov.uk', + mobile_number='+441234123412', + created_at=datetime.now(), + role_id=1) + user3 = Users(name='test three', + password='some2ndpassword', + email_address='test2@get_all.gov.uk', + mobile_number='+441234123412', + created_at=datetime.now(), + role_id=1) + + users_dao.insert_user(user1) + users_dao.insert_user(user2) + users_dao.insert_user(user3) + users = users_dao.get_all_users() + assert len(users) == 3 + assert users == [user1, user2, user3] diff --git a/tests/app/main/test_encyption.py b/tests/app/main/test_encyption.py new file mode 100644 index 000000000..a85aa59d9 --- /dev/null +++ b/tests/app/main/test_encyption.py @@ -0,0 +1,9 @@ +from app.main import encryption + + +def test_encryption(notifications_admin): + value = 's3curePassword!' + + encrypted = encryption.encrypt(value) + + assert encrypted == encryption.encrypt(value) diff --git a/tests/app/main/views/__init__.py b/tests/app/main/views/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/app/main/views/test_sign_in.py b/tests/app/main/views/test_sign_in.py new file mode 100644 index 000000000..66e2ebaa2 --- /dev/null +++ b/tests/app/main/views/test_sign_in.py @@ -0,0 +1,17 @@ + + +def test_render_sign_in_returns_sign_in_template(notifications_admin): + response = notifications_admin.test_client().get('/sign-in') + assert response.status_code == 200 + assert 'Sign in' in response.get_data(as_text=True) + assert 'Email address' in response.get_data(as_text=True) + assert 'Password' in response.get_data(as_text=True) + assert 'Forgotten password?' in response.get_data(as_text=True) + + +def test_process_sign_in_return_2fa_template(notifications_admin): + response = notifications_admin.test_client().post('/sign-in', + data={'email_address': 'valid@example.gov.uk', + 'password': 'val1dPassw0rd!'}) + assert response.status_code == 302 + assert response.location == 'http://localhost/two-factor' diff --git a/tests/conftest.py b/tests/conftest.py index faedd70b1..338e4d63b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -5,7 +5,7 @@ from app import create_app, db from app.models import Roles -@pytest.fixture(scope='module') +@pytest.fixture(scope='function') def notifications_admin(request): app = create_app('test') ctx = app.app_context() @@ -18,7 +18,7 @@ def notifications_admin(request): return app -@pytest.fixture(scope='module') +@pytest.fixture(scope='function') def notifications_admin_db(notifications_admin, request): metadata = MetaData(db.engine) metadata.reflect() From 48b7a7dc373a7ce9739e16fe48bf4b1aca60a730 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Fri, 27 Nov 2015 16:25:56 +0000 Subject: [PATCH 2/9] 108536490: Adding the login manager and csrf token. Still need to figure out how to override the load_user method, currently it is not working. --- app/__init__.py | 6 ++++ app/main/encryption.py | 2 +- app/main/forms.py | 2 +- app/main/views/index.py | 8 ++++++ app/main/views/sign_in.py | 35 +++++++++++++++-------- app/models.py | 24 ++++++++++++++++ app/templates/signin.html | 4 +-- app/templates/temp-create-users.html | 33 +++++++++++++++++++++ config.py | 7 +++-- migrations/versions/10_create_users.py | 3 +- migrations/versions/20_initialise_data.py | 12 ++++---- tests/app/main/views/test_sign_in.py | 21 +++++++++++++- 12 files changed, 130 insertions(+), 27 deletions(-) create mode 100644 app/templates/temp-create-users.html diff --git a/app/__init__.py b/app/__init__.py index 011e4cc2a..17f2828d0 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -4,11 +4,15 @@ from flask import Flask from flask._compat import string_types from flask.ext import assets from flask.ext.sqlalchemy import SQLAlchemy +from flask_login import LoginManager +from flask_wtf import CsrfProtect from webassets.filter import get_filter from config import configs db = SQLAlchemy() +login_manager = LoginManager() +csrf = CsrfProtect() def create_app(config_name): @@ -18,6 +22,8 @@ def create_app(config_name): application.config.from_object(configs[config_name]) db.init_app(application) init_app(application) + csrf.init_app(application) + login_manager.init_app(application) from app.main import main as main_blueprint application.register_blueprint(main_blueprint) diff --git a/app/main/encryption.py b/app/main/encryption.py index e42a42f22..b070fe4aa 100644 --- a/app/main/encryption.py +++ b/app/main/encryption.py @@ -3,5 +3,5 @@ from flask import current_app def encrypt(value): - key = current_app.config['SECRET_KEY'] + key = current_app.config['PASS_SECRET_KEY'] return hashlib.sha256((key + value).encode('UTF-8')).hexdigest() diff --git a/app/main/forms.py b/app/main/forms.py index a2cf5d9e9..6b7119817 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -5,7 +5,7 @@ from wtforms.validators import DataRequired, Email, Length class LoginForm(Form): email_address = StringField('Email address', validators=[ - Length(255), + Length(min=5, max=255), DataRequired(message='Email cannot be empty'), Email(message='Please enter a valid email address') ]) diff --git a/app/main/views/index.py b/app/main/views/index.py index de48cb9e6..ff2c53bfa 100644 --- a/app/main/views/index.py +++ b/app/main/views/index.py @@ -1,4 +1,5 @@ from flask import render_template +from flask_login import login_required from app.main import main @@ -19,36 +20,43 @@ def helloworld(): @main.route("/register") +@login_required def register(): return render_template('register.html') @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') @main.route("/dashboard") +@login_required def dashboard(): return render_template('dashboard.html') @main.route("/add-service") +@login_required def addservice(): return render_template('add-service.html') @main.route("/two-factor") +@login_required def twofactor(): return render_template('two-factor.html') diff --git a/app/main/views/sign_in.py b/app/main/views/sign_in.py index e95f661bf..6ffa19144 100644 --- a/app/main/views/sign_in.py +++ b/app/main/views/sign_in.py @@ -1,6 +1,6 @@ from datetime import datetime -from flask import render_template, redirect, url_for, jsonify +from flask import render_template, redirect, jsonify from flask_login import login_user from app.main import main @@ -19,25 +19,36 @@ def render_sign_in(): def process_sign_in(): form = LoginForm() if form.validate_on_submit(): - user = users_dao.get_user_by_email(form.email_address) + user = users_dao.get_user_by_email(form.email_address.data) if user is None: return jsonify(authorization=False), 404 - if user.password == encrypt(form.password): + if user.password == encrypt(form.password.data): login_user(user) else: return jsonify(authorization=False), 404 - + else: + return jsonify(form.errors), 404 return redirect('/two-factor') -@main.route('/create_user', methods=(['POST'])) +@main.route('/temp-create-users', methods=(['GET'])) +def render_create_user(): + return render_template('temp-create-users.html', form=LoginForm()) + + +@main.route('/temp-create-users', methods=(['POST'])) def create_user_for_test(): form = LoginForm() - user = Users(email_address=form.email_address, - name=form.email_address, - password=form.password, - created_at=datetime.now(), - role_id=1) - users_dao.insert_user(user) + if form.validate_on_submit(): + user = Users(email_address=form.email_address.data, + name=form.email_address.data, + password=form.password.data, + created_at=datetime.now(), + mobile_number='+447651234534', + role_id=1) + users_dao.insert_user(user) - return 'created' + return redirect('/sign-in') + else: + print(form.errors) + return redirect(form.errors), 400 diff --git a/app/models.py b/app/models.py index 56c65e7f9..08068b978 100644 --- a/app/models.py +++ b/app/models.py @@ -43,6 +43,30 @@ class Users(db.Model): return filter_null_value_fields(serialized) + def is_authenticated(self): + return True + + def is_active(self): + return True + + def is_locked(self): + if self.failed_login_count <= current_app.config['MAX_FAILED_LOGIN_COUNT']: + return False + else: + return True + + def is_anonymous(self): + return False + + def get_id(self): + return self.id + + @staticmethod + def load_user(user_id): + user = Users.query.filter_by(id=user_id).first() + if user.is_active(): + return user + def filter_null_value_fields(obj): return dict( diff --git a/app/templates/signin.html b/app/templates/signin.html index 7aea22245..b47f53475 100644 --- a/app/templates/signin.html +++ b/app/templates/signin.html @@ -13,7 +13,7 @@ Sign in

If you do not have an account, you can register.

- + {{ form.hidden_tag() }}

{{ form.email_address(class="form-control-2-3", autocomplete="off") }}
@@ -27,7 +27,7 @@ Sign in

- Continue +

diff --git a/app/templates/temp-create-users.html b/app/templates/temp-create-users.html new file mode 100644 index 000000000..e99abfed4 --- /dev/null +++ b/app/templates/temp-create-users.html @@ -0,0 +1,33 @@ +{% extends "admin_template.html" %} + +{% block page_title %} +Temp create users +{% endblock %} + +{% block content %} + +
+
+

Temporary page to create user

+ +

This is a temporary page to create users, the name will be the same as the email address.

+ +
+ {{ form.hidden_tag() }} +

+ + {{ form.email_address(class="form-control-2-3", autocomplete="off") }}
+

+

+ + {{ form.password(class="form-control-1-4", autocomplete="off") }}
+

+ +

+ +

+
+
+
+ +{% endblock %} \ No newline at end of file diff --git a/config.py b/config.py index 5c1ecfea1..f0558df21 100644 --- a/config.py +++ b/config.py @@ -9,7 +9,10 @@ class Config(object): SQLALCHEMY_RECORD_QUERIES = True SQLALCHEMY_DATABASE_URI = 'postgresql://localhost/notifications_admin' MAX_FAILED_LOGIN_COUNT = 10 - SECRET_KEY = 'secret-key-unique-changeme' + PASS_SECRET_KEY = 'secret-key-unique-changeme' + + WTF_CSRF_ENABLED = True + SECRET_KEY = 'secret-key' class Development(Config): @@ -19,7 +22,7 @@ class Development(Config): class Test(Config): DEBUG = False SQLALCHEMY_DATABASE_URI = 'postgresql://localhost/test_notifications_admin' - + WTF_CSRF_ENABLED = False configs = { 'development': Development, diff --git a/migrations/versions/10_create_users.py b/migrations/versions/10_create_users.py index 4a1a0fe1d..fbd32f153 100644 --- a/migrations/versions/10_create_users.py +++ b/migrations/versions/10_create_users.py @@ -1,6 +1,6 @@ """empty message -Revision ID: create_users +Revision ID: 10_create_users Revises: None Create Date: 2015-11-24 10:39:19.827534 @@ -13,6 +13,7 @@ down_revision = None from alembic import op import sqlalchemy as sa + def upgrade(): op.create_table('roles', sa.Column('id', sa.Integer, primary_key=True), diff --git a/migrations/versions/20_initialise_data.py b/migrations/versions/20_initialise_data.py index 4784310e6..ce5726cfc 100644 --- a/migrations/versions/20_initialise_data.py +++ b/migrations/versions/20_initialise_data.py @@ -1,15 +1,13 @@ # revision identifiers, used by Alembic. revision = '20_initialise_data' -down_revision = None - +down_revision = '10_create_users' +from app.models import Roles from alembic import op def upgrade(): - op.bulk_insert('roles', - [ - {'role': 'plaform_admin'}, - {'role': 'service_user'} - ]) + op.execute("insert into roles(role) values('platform_admin')") + op.execute("insert into roles(role) values('service_user')") + def downgrade(): op.drop_table('users') diff --git a/tests/app/main/views/test_sign_in.py b/tests/app/main/views/test_sign_in.py index 66e2ebaa2..90b6b1af0 100644 --- a/tests/app/main/views/test_sign_in.py +++ b/tests/app/main/views/test_sign_in.py @@ -1,3 +1,7 @@ +from datetime import datetime + +from app.main.dao import users_dao +from app.models import Users def test_render_sign_in_returns_sign_in_template(notifications_admin): @@ -9,9 +13,24 @@ def test_render_sign_in_returns_sign_in_template(notifications_admin): assert 'Forgotten password?' in response.get_data(as_text=True) -def test_process_sign_in_return_2fa_template(notifications_admin): +def test_process_sign_in_return_2fa_template(notifications_admin, notifications_admin_db): + user = Users(email_address='valid@example.gov.uk', + password='val1dPassw0rd!', + mobile_number='+441234123123', + name='valid', + created_at=datetime.now(), + role_id=1) + users_dao.insert_user(user) response = notifications_admin.test_client().post('/sign-in', data={'email_address': 'valid@example.gov.uk', 'password': 'val1dPassw0rd!'}) assert response.status_code == 302 assert response.location == 'http://localhost/two-factor' + + +def test_temp_create_user(notifications_admin, notifications_admin_db): + response = notifications_admin.test_client().post('/temp-create-users', + data={'email_address': 'testing@example.gov.uk', + 'password': 'val1dPassw0rd!'}) + + assert response.status_code == 302 From 6f61906fd4deda9c545bd40041b9c4ebc5190776 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 30 Nov 2015 11:21:51 +0000 Subject: [PATCH 3/9] 108536490: Implement LoginManager for the admin app. Also added csrf error handler, will make the session unauthorized if the csrf token is invalid. --- app/__init__.py | 26 +++++++++- app/main/dao/users_dao.py | 8 +-- app/main/views/sign_in.py | 20 +++++--- app/models.py | 4 +- tests/app/main/dao/test_users_dao.py | 74 ++++++++++++++-------------- tests/app/main/views/test_sign_in.py | 14 +++--- 6 files changed, 87 insertions(+), 59 deletions(-) diff --git a/app/__init__.py b/app/__init__.py index 17f2828d0..007b52aee 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -1,12 +1,13 @@ import os -from flask import Flask +from flask import Flask, session from flask._compat import string_types from flask.ext import assets from flask.ext.sqlalchemy import SQLAlchemy from flask_login import LoginManager from flask_wtf import CsrfProtect from webassets.filter import get_filter +from werkzeug.exceptions import abort from config import configs @@ -22,8 +23,10 @@ def create_app(config_name): application.config.from_object(configs[config_name]) db.init_app(application) init_app(application) - csrf.init_app(application) + init_csrf(application) + login_manager.init_app(application) + login_manager.login_view = 'main.sign_in.render_sign_in' from app.main import main as main_blueprint application.register_blueprint(main_blueprint) @@ -31,6 +34,25 @@ def create_app(config_name): return application +def init_csrf(application): + csrf.init_app(application) + + @csrf.error_handler + def csrf_handler(reason): + if 'user_id' not in session: + application.logger.info( + u'csrf.session_expired: Redirecting user to log in page' + ) + + return application.login_manager.unauthorized() + + application.logger.info( + u'csrf.invalid_token: Aborting request, user_id: {user_id}', + extra={'user_id': session['user_id']}) + + abort(400, reason) + + def init_app(app): for key, value in app.config.items(): if key in os.environ: diff --git a/app/main/dao/users_dao.py b/app/main/dao/users_dao.py index 95d68eafa..1cb7aeea2 100644 --- a/app/main/dao/users_dao.py +++ b/app/main/dao/users_dao.py @@ -1,5 +1,5 @@ from app import db -from app.models import Users +from app.models import User from app.main.encryption import encrypt @@ -10,12 +10,12 @@ def insert_user(user): def get_user_by_id(id): - return Users.query.filter_by(id=id).first() + return User.query.filter_by(id=id).first() def get_all_users(): - return Users.query.all() + return User.query.all() def get_user_by_email(email_address): - return Users.query.filter_by(email_address=email_address).first() + return User.query.filter_by(email_address=email_address).first() diff --git a/app/main/views/sign_in.py b/app/main/views/sign_in.py index 6ffa19144..abf14e4b4 100644 --- a/app/main/views/sign_in.py +++ b/app/main/views/sign_in.py @@ -3,13 +3,19 @@ from datetime import datetime from flask import render_template, redirect, jsonify from flask_login import login_user +from app import login_manager from app.main import main from app.main.forms import LoginForm from app.main.dao import users_dao -from app.models import Users +from app.models import User from app.main.encryption import encrypt +@login_manager.user_loader +def load_user(user_id): + return users_dao.get_user_by_id(user_id) + + @main.route("/sign-in", methods=(['GET'])) def render_sign_in(): return render_template('signin.html', form=LoginForm()) @@ -40,12 +46,12 @@ def render_create_user(): def create_user_for_test(): form = LoginForm() if form.validate_on_submit(): - user = Users(email_address=form.email_address.data, - name=form.email_address.data, - password=form.password.data, - created_at=datetime.now(), - mobile_number='+447651234534', - role_id=1) + user = User(email_address=form.email_address.data, + name=form.email_address.data, + password=form.password.data, + created_at=datetime.now(), + mobile_number='+447651234534', + role_id=1) users_dao.insert_user(user) return redirect('/sign-in') diff --git a/app/models.py b/app/models.py index 08068b978..2ed53c257 100644 --- a/app/models.py +++ b/app/models.py @@ -12,7 +12,7 @@ class Roles(db.Model): role = db.Column(db.String, nullable=False, unique=True) -class Users(db.Model): +class User(db.Model): __tablename__ = 'users' id = db.Column(db.Integer, primary_key=True) @@ -63,7 +63,7 @@ class Users(db.Model): @staticmethod def load_user(user_id): - user = Users.query.filter_by(id=user_id).first() + user = User.query.filter_by(id=user_id).first() if user.is_active(): return user diff --git a/tests/app/main/dao/test_users_dao.py b/tests/app/main/dao/test_users_dao.py index 3f734ff87..694a0fe3b 100644 --- a/tests/app/main/dao/test_users_dao.py +++ b/tests/app/main/dao/test_users_dao.py @@ -3,17 +3,17 @@ from datetime import datetime import pytest import sqlalchemy -from app.models import Users +from app.models import User from app.main.dao import users_dao def test_insert_user_should_add_user(notifications_admin, notifications_admin_db): - user = Users(name='test insert', - password='somepassword', - email_address='test@insert.gov.uk', - mobile_number='+441234123412', - created_at=datetime.now(), - role_id=1) + user = User(name='test insert', + password='somepassword', + email_address='test@insert.gov.uk', + mobile_number='+441234123412', + created_at=datetime.now(), + role_id=1) users_dao.insert_user(user) saved_user = users_dao.get_user_by_id(user.id) @@ -21,24 +21,24 @@ def test_insert_user_should_add_user(notifications_admin, notifications_admin_db def test_insert_user_with_role_that_does_not_exist_fails(notifications_admin, notifications_admin_db): - user = Users(name='role does not exist', - password='somepassword', - email_address='test@insert.gov.uk', - mobile_number='+441234123412', - created_at=datetime.now(), - role_id=100) + user = User(name='role does not exist', + password='somepassword', + email_address='test@insert.gov.uk', + mobile_number='+441234123412', + created_at=datetime.now(), + role_id=100) with pytest.raises(sqlalchemy.exc.IntegrityError) as error: users_dao.insert_user(user) assert 'insert or update on table "users" violates foreign key constraint "users_role_id_fkey"' in str(error.value) def test_get_user_by_email(notifications_admin, notifications_admin_db): - user = Users(name='test_get_by_email', - password='somepassword', - email_address='email@example.gov.uk', - mobile_number='+441234153412', - created_at=datetime.now(), - role_id=1) + user = User(name='test_get_by_email', + password='somepassword', + email_address='email@example.gov.uk', + mobile_number='+441234153412', + created_at=datetime.now(), + role_id=1) users_dao.insert_user(user) retrieved = users_dao.get_user_by_email(user.email_address) @@ -46,24 +46,24 @@ def test_get_user_by_email(notifications_admin, notifications_admin_db): def test_get_all_users_returns_all_users(notifications_admin, notifications_admin_db): - user1 = Users(name='test one', - password='somepassword', - email_address='test1@get_all.gov.uk', - mobile_number='+441234123412', - created_at=datetime.now(), - role_id=1) - user2 = Users(name='test two', - password='some2ndpassword', - email_address='test2@get_all.gov.uk', - mobile_number='+441234123412', - created_at=datetime.now(), - role_id=1) - user3 = Users(name='test three', - password='some2ndpassword', - email_address='test2@get_all.gov.uk', - mobile_number='+441234123412', - created_at=datetime.now(), - role_id=1) + user1 = User(name='test one', + password='somepassword', + email_address='test1@get_all.gov.uk', + mobile_number='+441234123412', + created_at=datetime.now(), + role_id=1) + user2 = User(name='test two', + password='some2ndpassword', + email_address='test2@get_all.gov.uk', + mobile_number='+441234123412', + created_at=datetime.now(), + role_id=1) + user3 = User(name='test three', + password='some2ndpassword', + email_address='test2@get_all.gov.uk', + mobile_number='+441234123412', + created_at=datetime.now(), + role_id=1) users_dao.insert_user(user1) users_dao.insert_user(user2) diff --git a/tests/app/main/views/test_sign_in.py b/tests/app/main/views/test_sign_in.py index 90b6b1af0..255c9d90c 100644 --- a/tests/app/main/views/test_sign_in.py +++ b/tests/app/main/views/test_sign_in.py @@ -1,7 +1,7 @@ from datetime import datetime from app.main.dao import users_dao -from app.models import Users +from app.models import User def test_render_sign_in_returns_sign_in_template(notifications_admin): @@ -14,12 +14,12 @@ def test_render_sign_in_returns_sign_in_template(notifications_admin): def test_process_sign_in_return_2fa_template(notifications_admin, notifications_admin_db): - user = Users(email_address='valid@example.gov.uk', - password='val1dPassw0rd!', - mobile_number='+441234123123', - name='valid', - created_at=datetime.now(), - role_id=1) + user = User(email_address='valid@example.gov.uk', + password='val1dPassw0rd!', + mobile_number='+441234123123', + name='valid', + created_at=datetime.now(), + role_id=1) users_dao.insert_user(user) response = notifications_admin.test_client().post('/sign-in', data={'email_address': 'valid@example.gov.uk', From af382885d3dad8fee12470a0a726284f15475b06 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 30 Nov 2015 12:38:02 +0000 Subject: [PATCH 4/9] 108536490: Use ItsDangerousSessionInterface in the app. Start using http://flask.pocoo.org/snippets/51/ --- app/__init__.py | 2 ++ app/its_dangerous_session.py | 50 ++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) create mode 100644 app/its_dangerous_session.py diff --git a/app/__init__.py b/app/__init__.py index 007b52aee..d88b74707 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.its_dangerous_session import ItsdangerousSessionInterface from config import configs db = SQLAlchemy() @@ -31,6 +32,7 @@ def create_app(config_name): from app.main import main as main_blueprint application.register_blueprint(main_blueprint) + application.session_interface = ItsdangerousSessionInterface() return application diff --git a/app/its_dangerous_session.py b/app/its_dangerous_session.py new file mode 100644 index 000000000..ab644f057 --- /dev/null +++ b/app/its_dangerous_session.py @@ -0,0 +1,50 @@ +from werkzeug.datastructures import CallbackDict +from flask.sessions import SessionInterface, SessionMixin +from itsdangerous import URLSafeTimedSerializer, BadSignature + + +class ItsdangerousSession(CallbackDict, SessionMixin): + + def __init__(self, initial=None): + def on_update(self): + self.modified = True + CallbackDict.__init__(self, initial, on_update) + self.modified = False + + +class ItsdangerousSessionInterface(SessionInterface): + salt = 'cookie-session' + session_class = ItsdangerousSession + + def get_serializer(self, app): + if not app.secret_key: + return None + return URLSafeTimedSerializer(app.secret_key, + salt=self.salt) + + def open_session(self, app, request): + s = self.get_serializer(app) + if s is None: + return None + val = request.cookies.get(app.session_cookie_name) + if not val: + return self.session_class() + max_age = app.permanent_session_lifetime.total_seconds() + try: + data = s.loads(val, max_age=max_age) + return self.session_class(data) + except BadSignature: + return self.session_class() + + def save_session(self, app, session, response): + domain = self.get_cookie_domain(app) + if not session: + if session.modified: + response.delete_cookie(app.session_cookie_name, + domain=domain) + return + expires = self.get_expiration_time(app, session) + val = self.get_serializer(app).dumps(dict(session)) + response.set_cookie(app.session_cookie_name, val, + expires=expires, httponly=True, + domain=domain) From 3f017b30f2a4235028346c99f6917b9ca18735ca Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 30 Nov 2015 14:32:58 +0000 Subject: [PATCH 5/9] 108536490: add the proxy_fix --- .travis.yml | 2 ++ app/__init__.py | 5 +++++ app/its_dangerous_session.py | 4 ++-- app/main/dao/users_dao.py | 7 ++++++- app/main/views/sign_in.py | 12 +++--------- app/models.py | 16 +++++----------- app/proxy_fix.py | 17 +++++++++++++++++ config.py | 7 +++++++ requirements.txt | 2 +- 9 files changed, 48 insertions(+), 24 deletions(-) create mode 100644 app/proxy_fix.py diff --git a/.travis.yml b/.travis.yml index 4c7783f29..912413eb2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -27,3 +27,5 @@ deploy: app: notifications-admin on: repo: alphagov/notifications-admin + run: + - python app.py db upgrade diff --git a/app/__init__.py b/app/__init__.py index d88b74707..fc5c73b8e 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -10,8 +10,10 @@ from webassets.filter import get_filter from werkzeug.exceptions import abort from app.its_dangerous_session import ItsdangerousSessionInterface +import app.proxy_fix from config import configs + db = SQLAlchemy() login_manager = LoginManager() csrf = CsrfProtect() @@ -32,7 +34,10 @@ def create_app(config_name): from app.main import main as main_blueprint application.register_blueprint(main_blueprint) + proxy_fix.init_app(application) + application.session_interface = ItsdangerousSessionInterface() + return application diff --git a/app/its_dangerous_session.py b/app/its_dangerous_session.py index ab644f057..d281b32d9 100644 --- a/app/its_dangerous_session.py +++ b/app/its_dangerous_session.py @@ -13,14 +13,14 @@ class ItsdangerousSession(CallbackDict, SessionMixin): class ItsdangerousSessionInterface(SessionInterface): - salt = 'cookie-session' session_class = ItsdangerousSession def get_serializer(self, app): + salt = app.config.get('DANGEROUS_SALT') if not app.secret_key: return None return URLSafeTimedSerializer(app.secret_key, - salt=self.salt) + salt=salt) def open_session(self, app, request): s = self.get_serializer(app) diff --git a/app/main/dao/users_dao.py b/app/main/dao/users_dao.py index 1cb7aeea2..0ebb54b6d 100644 --- a/app/main/dao/users_dao.py +++ b/app/main/dao/users_dao.py @@ -1,8 +1,13 @@ -from app import db +from app import db, login_manager from app.models import User from app.main.encryption import encrypt +@login_manager.user_loader +def load_user(user_id): + return get_user_by_id(user_id) + + def insert_user(user): user.password = encrypt(user.password) db.session.add(user) diff --git a/app/main/views/sign_in.py b/app/main/views/sign_in.py index abf14e4b4..78011390f 100644 --- a/app/main/views/sign_in.py +++ b/app/main/views/sign_in.py @@ -3,7 +3,6 @@ from datetime import datetime from flask import render_template, redirect, jsonify from flask_login import login_user -from app import login_manager from app.main import main from app.main.forms import LoginForm from app.main.dao import users_dao @@ -11,11 +10,6 @@ from app.models import User from app.main.encryption import encrypt -@login_manager.user_loader -def load_user(user_id): - return users_dao.get_user_by_id(user_id) - - @main.route("/sign-in", methods=(['GET'])) def render_sign_in(): return render_template('signin.html', form=LoginForm()) @@ -27,13 +21,13 @@ def process_sign_in(): if form.validate_on_submit(): user = users_dao.get_user_by_email(form.email_address.data) if user is None: - return jsonify(authorization=False), 404 + return jsonify(authorization=False), 401 if user.password == encrypt(form.password.data): login_user(user) else: - return jsonify(authorization=False), 404 + return jsonify(authorization=False), 401 else: - return jsonify(form.errors), 404 + return jsonify(form.errors), 400 return redirect('/two-factor') diff --git a/app/models.py b/app/models.py index 2ed53c257..7767c91a8 100644 --- a/app/models.py +++ b/app/models.py @@ -49,23 +49,17 @@ class User(db.Model): def is_active(self): return True - def is_locked(self): - if self.failed_login_count <= current_app.config['MAX_FAILED_LOGIN_COUNT']: - return False - else: - return True - def is_anonymous(self): return False def get_id(self): return self.id - @staticmethod - def load_user(user_id): - user = User.query.filter_by(id=user_id).first() - if user.is_active(): - return user + def is_locked(self): + if self.failed_login_count <= current_app.config['MAX_FAILED_LOGIN_COUNT']: + return False + else: + return True def filter_null_value_fields(obj): diff --git a/app/proxy_fix.py b/app/proxy_fix.py new file mode 100644 index 000000000..a31496411 --- /dev/null +++ b/app/proxy_fix.py @@ -0,0 +1,17 @@ +from werkzeug.contrib.fixers import ProxyFix + + +class CustomProxyFix(object): + def __init__(self, app, forwarded_proto): + self.app = ProxyFix(app) + self.forwarded_proto = forwarded_proto + + def __call__(self, environ, start_response): + environ.update({ + "HTTP_X_FORWARDED_PROTO": self.forwarded_proto + }) + return self.app(environ, start_response) + + +def init_app(app): + app.wsgi_app = CustomProxyFix(app.wsgi_app, app.config.get('HTTP_PROTOCOL', 'http')) \ No newline at end of file diff --git a/config.py b/config.py index f0558df21..6c799b561 100644 --- a/config.py +++ b/config.py @@ -13,6 +13,8 @@ class Config(object): WTF_CSRF_ENABLED = True SECRET_KEY = 'secret-key' + HTTP_PROTOCOL = 'http' + DANGEROUS_SALT = 'itsdangeroussalt' class Development(Config): @@ -24,6 +26,11 @@ class Test(Config): SQLALCHEMY_DATABASE_URI = 'postgresql://localhost/test_notifications_admin' WTF_CSRF_ENABLED = False + +class Live(Config): + DEBUG = False + HTTP_PROTOCOL = 'https' + configs = { 'development': Development, 'test': Test diff --git a/requirements.txt b/requirements.txt index b66dabd3e..379db7cd1 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,7 +3,7 @@ Flask-Script==2.0.5 Flask-Assets==0.11 Flask-Migrate==1.3.1 Flask-SQLAlchemy==2.0 -psycopg2==2.6.1 +psycopg2==2.6.2 SQLAlchemy==1.0.5 SQLAlchemy-Utils==0.30.5 Flask-WTF==0.11 From ff9e98907e075ece02904863d5e17b3fd037b785 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 30 Nov 2015 15:33:40 +0000 Subject: [PATCH 6/9] 108536490: Update encryption for password --- app/main/dao/users_dao.py | 4 ++-- app/main/encryption.py | 12 +++++++----- app/main/views/sign_in.py | 4 ++-- app/proxy_fix.py | 2 +- requirements.txt | 5 +++-- tests/app/main/test_encyption.py | 16 ++++++++++++---- 6 files changed, 27 insertions(+), 16 deletions(-) diff --git a/app/main/dao/users_dao.py b/app/main/dao/users_dao.py index 0ebb54b6d..59a23568c 100644 --- a/app/main/dao/users_dao.py +++ b/app/main/dao/users_dao.py @@ -1,6 +1,6 @@ from app import db, login_manager from app.models import User -from app.main.encryption import encrypt +from app.main.encryption import hashpw @login_manager.user_loader @@ -9,7 +9,7 @@ def load_user(user_id): def insert_user(user): - user.password = encrypt(user.password) + user.password = hashpw(user.password) db.session.add(user) db.session.commit() diff --git a/app/main/encryption.py b/app/main/encryption.py index b070fe4aa..27aff9e25 100644 --- a/app/main/encryption.py +++ b/app/main/encryption.py @@ -1,7 +1,9 @@ -import hashlib -from flask import current_app +from flask.ext.bcrypt import generate_password_hash, check_password_hash -def encrypt(value): - key = current_app.config['PASS_SECRET_KEY'] - return hashlib.sha256((key + value).encode('UTF-8')).hexdigest() +def hashpw(password): + return generate_password_hash(password.encode('UTF-8'), 10) + + +def checkpw(password, hashed_password): + return check_password_hash(hashed_password, password) diff --git a/app/main/views/sign_in.py b/app/main/views/sign_in.py index 78011390f..d5d3677c2 100644 --- a/app/main/views/sign_in.py +++ b/app/main/views/sign_in.py @@ -7,7 +7,7 @@ from app.main import main from app.main.forms import LoginForm from app.main.dao import users_dao from app.models import User -from app.main.encryption import encrypt +from app.main.encryption import checkpw @main.route("/sign-in", methods=(['GET'])) @@ -22,7 +22,7 @@ def process_sign_in(): user = users_dao.get_user_by_email(form.email_address.data) if user is None: return jsonify(authorization=False), 401 - if user.password == encrypt(form.password.data): + if checkpw(form.password.data, user.password): login_user(user) else: return jsonify(authorization=False), 401 diff --git a/app/proxy_fix.py b/app/proxy_fix.py index a31496411..a572672d7 100644 --- a/app/proxy_fix.py +++ b/app/proxy_fix.py @@ -14,4 +14,4 @@ class CustomProxyFix(object): def init_app(app): - app.wsgi_app = CustomProxyFix(app.wsgi_app, app.config.get('HTTP_PROTOCOL', 'http')) \ No newline at end of file + app.wsgi_app = CustomProxyFix(app.wsgi_app, app.config.get('HTTP_PROTOCOL', 'http')) diff --git a/requirements.txt b/requirements.txt index 379db7cd1..9ac4a3971 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,8 +3,9 @@ Flask-Script==2.0.5 Flask-Assets==0.11 Flask-Migrate==1.3.1 Flask-SQLAlchemy==2.0 -psycopg2==2.6.2 +psycopg2==2.6.1 SQLAlchemy==1.0.5 SQLAlchemy-Utils==0.30.5 Flask-WTF==0.11 -Flask-Login==0.2.11 \ No newline at end of file +Flask-Login==0.2.11 +Flask-Bcrypt==0.6.2 \ No newline at end of file diff --git a/tests/app/main/test_encyption.py b/tests/app/main/test_encyption.py index a85aa59d9..9339dd1e3 100644 --- a/tests/app/main/test_encyption.py +++ b/tests/app/main/test_encyption.py @@ -1,9 +1,17 @@ -from app.main import encryption +from app.main.encryption import hashpw, checkpw -def test_encryption(notifications_admin): +def test_should_hash_password(): + password = 'passwordToHash' + assert password != hashpw(password) + + +def test_should_check_password(): value = 's3curePassword!' + encrypted = hashpw(value) + assert checkpw(value, encrypted) is True - encrypted = encryption.encrypt(value) - assert encrypted == encryption.encrypt(value) +def test_checkpw_should_fail_when_pw_does_not_match(): + value = hashpw('somePassword') + assert checkpw('somethingDifferent', value) is False From 3b27db98ff6dac10876593aea213ba5400294f23 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 30 Nov 2015 16:33:45 +0000 Subject: [PATCH 7/9] 108536490: Implement locked out function. User is locked if they fail to login 10 times or more. --- app/main/dao/users_dao.py | 6 +++++ app/main/views/sign_in.py | 3 +++ app/models.py | 2 +- tests/app/main/dao/test_users_dao.py | 34 ++++++++++++++++++++++++++++ tests/app/main/views/test_sign_in.py | 21 +++++++++++++++++ 5 files changed, 65 insertions(+), 1 deletion(-) diff --git a/app/main/dao/users_dao.py b/app/main/dao/users_dao.py index 59a23568c..d4d3a0b42 100644 --- a/app/main/dao/users_dao.py +++ b/app/main/dao/users_dao.py @@ -24,3 +24,9 @@ def get_all_users(): def get_user_by_email(email_address): return User.query.filter_by(email_address=email_address).first() + + +def increment_failed_login_count(id): + user = User.query.filter_by(id=id).first() + user.failed_login_count += 1 + db.session.commit() diff --git a/app/main/views/sign_in.py b/app/main/views/sign_in.py index d5d3677c2..492b3225c 100644 --- a/app/main/views/sign_in.py +++ b/app/main/views/sign_in.py @@ -20,11 +20,14 @@ def process_sign_in(): form = LoginForm() if form.validate_on_submit(): user = users_dao.get_user_by_email(form.email_address.data) + if user.is_locked(): + return jsonify(locked_out=True), 401 if user is None: return jsonify(authorization=False), 401 if checkpw(form.password.data, user.password): login_user(user) else: + users_dao.increment_failed_login_count(user.id) return jsonify(authorization=False), 401 else: return jsonify(form.errors), 400 diff --git a/app/models.py b/app/models.py index 7767c91a8..c7a388423 100644 --- a/app/models.py +++ b/app/models.py @@ -56,7 +56,7 @@ class User(db.Model): return self.id def is_locked(self): - if self.failed_login_count <= current_app.config['MAX_FAILED_LOGIN_COUNT']: + if self.failed_login_count < current_app.config['MAX_FAILED_LOGIN_COUNT']: return False else: return True diff --git a/tests/app/main/dao/test_users_dao.py b/tests/app/main/dao/test_users_dao.py index 694a0fe3b..55b64e508 100644 --- a/tests/app/main/dao/test_users_dao.py +++ b/tests/app/main/dao/test_users_dao.py @@ -71,3 +71,37 @@ def test_get_all_users_returns_all_users(notifications_admin, notifications_admi users = users_dao.get_all_users() assert len(users) == 3 assert users == [user1, user2, user3] + + +def test_increment_failed_lockout_count_should_increade_count_by_1(notifications_admin, notifications_admin_db): + user = User(name='cannot remember password', + password='somepassword', + email_address='test1@get_all.gov.uk', + mobile_number='+441234123412', + created_at=datetime.now(), + role_id=1) + users_dao.insert_user(user) + + savedUser = users_dao.get_user_by_id(user.id) + assert savedUser.failed_login_count == 0 + users_dao.increment_failed_login_count(user.id) + assert users_dao.get_user_by_id(user.id).failed_login_count == 1 + + +def test_user_is_locked_if_failed_login_count_is_10_or_greater(notifications_admin, notifications_admin_db): + user = User(name='cannot remember password', + password='somepassword', + email_address='test1@get_all.gov.uk', + mobile_number='+441234123412', + created_at=datetime.now(), + role_id=1) + users_dao.insert_user(user) + saved_user = users_dao.get_user_by_id(user.id) + assert saved_user.is_locked() is False + + for _ in range(10): + users_dao.increment_failed_login_count(user.id) + + saved_user = users_dao.get_user_by_id(user.id) + assert saved_user.failed_login_count == 10 + assert saved_user.is_locked() is True diff --git a/tests/app/main/views/test_sign_in.py b/tests/app/main/views/test_sign_in.py index 255c9d90c..1c84650c3 100644 --- a/tests/app/main/views/test_sign_in.py +++ b/tests/app/main/views/test_sign_in.py @@ -34,3 +34,24 @@ def test_temp_create_user(notifications_admin, notifications_admin_db): 'password': 'val1dPassw0rd!'}) assert response.status_code == 302 + + +def test_should_return_locked_out_true_when_user_is_locked(notifications_admin, notifications_admin_db): + user = User(email_address='valid@example.gov.uk', + password='val1dPassw0rd!', + mobile_number='+441234123123', + name='valid', + created_at=datetime.now(), + role_id=1) + users_dao.insert_user(user) + for _ in range(10): + notifications_admin.test_client().post('/sign-in', + data={'email_address': 'valid@example.gov.uk', + 'password': 'whatIsMyPassword!'}) + + response = notifications_admin.test_client().post('/sign-in', + data={'email_address': 'valid@example.gov.uk', + 'password': 'val1dPassw0rd!'}) + + assert response.status_code == 401 + assert '"locked_out": true' in response.get_data(as_text=True) From edfc1d6efc637b27d9979d0306113a6578f9ee1c Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 30 Nov 2015 16:44:59 +0000 Subject: [PATCH 8/9] 108536490: Implement User.is_active() If the state of the user is inactive the user.is_active() returns false. --- app/main/views/sign_in.py | 2 ++ app/models.py | 5 ++++- tests/app/main/dao/test_users_dao.py | 14 ++++++++++++++ tests/app/main/views/test_sign_in.py | 18 ++++++++++++++++++ 4 files changed, 38 insertions(+), 1 deletion(-) diff --git a/app/main/views/sign_in.py b/app/main/views/sign_in.py index 492b3225c..e2d37a9c9 100644 --- a/app/main/views/sign_in.py +++ b/app/main/views/sign_in.py @@ -22,6 +22,8 @@ def process_sign_in(): user = users_dao.get_user_by_email(form.email_address.data) if user.is_locked(): return jsonify(locked_out=True), 401 + if not user.is_active(): + return jsonify(active_user=False), 401 if user is None: return jsonify(authorization=False), 401 if checkpw(form.password.data, user.password): diff --git a/app/models.py b/app/models.py index c7a388423..21d0aca52 100644 --- a/app/models.py +++ b/app/models.py @@ -47,7 +47,10 @@ class User(db.Model): return True def is_active(self): - return True + if self.state == 'inactive': + return False + else: + return True def is_anonymous(self): return False diff --git a/tests/app/main/dao/test_users_dao.py b/tests/app/main/dao/test_users_dao.py index 55b64e508..9fde6bd9b 100644 --- a/tests/app/main/dao/test_users_dao.py +++ b/tests/app/main/dao/test_users_dao.py @@ -105,3 +105,17 @@ def test_user_is_locked_if_failed_login_count_is_10_or_greater(notifications_adm saved_user = users_dao.get_user_by_id(user.id) assert saved_user.failed_login_count == 10 assert saved_user.is_locked() is True + + +def test_user_is_active_is_false_if_state_is_inactive(notifications_admin, notifications_admin_db): + user = User(name='inactive user', + password='somepassword', + email_address='test1@get_all.gov.uk', + mobile_number='+441234123412', + created_at=datetime.now(), + role_id=1, + state='inactive') + users_dao.insert_user(user) + + saved_user = users_dao.get_user_by_id(user.id) + assert saved_user.is_active() is False diff --git a/tests/app/main/views/test_sign_in.py b/tests/app/main/views/test_sign_in.py index 1c84650c3..07d48c60a 100644 --- a/tests/app/main/views/test_sign_in.py +++ b/tests/app/main/views/test_sign_in.py @@ -55,3 +55,21 @@ def test_should_return_locked_out_true_when_user_is_locked(notifications_admin, assert response.status_code == 401 assert '"locked_out": true' in response.get_data(as_text=True) + + +def test_should_return_active_user_is_false_if_user_is_inactive(notifications_admin, notifications_admin_db): + user = User(email_address='inactive_user@example.gov.uk', + password='val1dPassw0rd!', + mobile_number='+441234123123', + name='inactive user', + created_at=datetime.now(), + role_id=1, + state='inactive') + users_dao.insert_user(user) + + response = notifications_admin.test_client().post('/sign-in', + data={'email_address': 'inactive_user@example.gov.uk', + 'password': 'val1dPassw0rd!'}) + + assert response.status_code == 401 + assert '"active_user": false' in response.get_data(as_text=True) From de0efcb5085c14eb84bd43b415b3ec9e9e4d5e95 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 30 Nov 2015 16:52:28 +0000 Subject: [PATCH 9/9] 108536490: Add test that post with bad password when account is locked results in 401 --- tests/app/main/views/test_sign_in.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/app/main/views/test_sign_in.py b/tests/app/main/views/test_sign_in.py index 07d48c60a..2f55ba8cf 100644 --- a/tests/app/main/views/test_sign_in.py +++ b/tests/app/main/views/test_sign_in.py @@ -56,6 +56,12 @@ def test_should_return_locked_out_true_when_user_is_locked(notifications_admin, assert response.status_code == 401 assert '"locked_out": true' in response.get_data(as_text=True) + another_bad_attempt = notifications_admin.test_client().post('/sign-in', + data={'email_address': 'valid@example.gov.uk', + 'password': 'whatIsMyPassword!'}) + assert another_bad_attempt.status_code == 401 + assert '"locked_out": true' in response.get_data(as_text=True) + def test_should_return_active_user_is_false_if_user_is_inactive(notifications_admin, notifications_admin_db): user = User(email_address='inactive_user@example.gov.uk',