Merge remote-tracking branch 'origin/master' into implement_send_sms_and_email

app/__init__.py

@@ -8,6 +8,7 @@ from flask_login import LoginManager
 from flask_wtf import CsrfProtect
 from werkzeug.exceptions import abort
 from app.notify_client.api_client import NotificationsAdminAPIClient
+from app.notify_client.user_api_client import UserApiClient
 from app.its_dangerous_session import ItsdangerousSessionInterface
 import app.proxy_fix
 from config import configs
@@ -18,6 +19,7 @@ login_manager = LoginManager()
 csrf = CsrfProtect()
 
 notifications_api_client = NotificationsAdminAPIClient()
+user_api_client = UserApiClient()
 
 
 def create_app(config_name, config_overrides=None):
@@ -31,6 +33,7 @@ def create_app(config_name, config_overrides=None):
     init_csrf(application)
 
     notifications_api_client.init_app(application)
+    user_api_client.init_app(application)
 
     login_manager.init_app(application)
     login_manager.login_view = 'main.sign_in'

app/main/dao/verify_codes_dao.py

@@ -13,7 +13,7 @@ def add_code(user_id, code, code_type):
 
     db.session.add(code)
     db.session.commit()
-    return code.id
+    return code
 
 
 def get_codes(user_id, code_type=None):
@@ -23,7 +23,7 @@ def get_codes(user_id, code_type=None):
 
 
 def get_code_by_code(user_id, code, code_type):
-    return VerifyCodes.query.filter_by(user_id=user_id, code=code, code_type=code_type).first()
+    return VerifyCodes.query.filter_by(user_id=user_id, code=hashpw(code), code_type=code_type).first()
 
 
 def use_code(id):

app/main/views/register.py

@@ -15,7 +15,7 @@ from app.models import User
 from app.main.dao import users_dao
 from app.main.forms import RegisterUserForm
 
-from app.notify_client.user_api_client import UserApiClient
+from app import user_api_client
 
 # TODO how do we handle duplicate unverifed email addresses?
 # malicious or otherwise.
@@ -28,23 +28,11 @@ def register():
 
     if form.validate_on_submit():
 
-        #  TODO remove once all api integrations done
-        user = User(name=form.name.data,
-                    email_address=form.email_address.data,
-                    mobile_number=form.mobile_number.data,
-                    password=form.password.data,
-                    created_at=datetime.now(),
-                    role_id=1)
-        users_dao.insert_user(user)
-
-        user_api_client = UserApiClient(current_app.config['API_HOST_NAME'],
-                                        current_app.config['ADMIN_CLIENT_USER_NAME'],
-                                        current_app.config['ADMIN_CLIENT_SECRET'])
         try:
-            user_api_client.register_user(form.name.data,
+            user = user_api_client.register_user(form.name.data,
-                                          form.email_address.data,
+                                                 form.email_address.data,
-                                          form.mobile_number.data,
+                                                 form.mobile_number.data,
-                                          form.password.data)
+                                                 form.password.data)
         except HTTPError as e:
             if e.status_code == 404:
                 abort(404)
@@ -56,10 +44,10 @@ def register():
         # How do we report to the user there is a problem with
         # sending codes apart from service unavailable?
         # at the moment i believe http 500 is fine.
-        send_sms_code(user_id=user.id, mobile_number=form.mobile_number.data)
+        send_sms_code(user_id=user.id, mobile_number=user.mobile_number)
-        send_email_code(user_id=user.id, email=form.email_address.data)
+        send_email_code(user_id=user.id, email=user.email_address)
         session['expiry_date'] = str(datetime.now() + timedelta(hours=1))
-        session['user_email'] = user.email_address
+        session['user_details'] = {"email": user.email_address, "id": user.id}
         return redirect('/verify')
 
     return render_template('views/register.html', form=form)

app/main/views/verify.py

@@ -1,5 +1,10 @@
 from flask import (
-    render_template, redirect, jsonify, session, url_for)
+    render_template,
+    redirect,
+    jsonify,
+    session,
+    url_for
+)
 
 from flask_login import login_user
 
@@ -12,13 +17,16 @@ from app.main.forms import VerifyForm
 def verify():
     # TODO there needs to be a way to regenerate a session id
     # or handle gracefully.
-    user = users_dao.get_user_by_email(session['user_email'])
+    user_id = session['user_details']['id']
-    codes = verify_codes_dao.get_codes(user.id)
+    codes = verify_codes_dao.get_codes(user_id)
     form = VerifyForm(codes)
     if form.validate_on_submit():
-        verify_codes_dao.use_code_for_user_and_type(user_id=user.id, code_type='email')
+        verify_codes_dao.use_code_for_user_and_type(user_id=user_id, code_type='email')
-        verify_codes_dao.use_code_for_user_and_type(user_id=user.id, code_type='sms')
+        verify_codes_dao.use_code_for_user_and_type(user_id=user_id, code_type='sms')
-        users_dao.activate_user(user.id)
+
-        login_user(user)
+        # TODO complete verify and login flow
+        # users_dao.activate_user(user.id)
+        # login_user(user)
+
         return redirect(url_for('.add_service', first='first'))
     return render_template('views/verify.html', form=form)

app/models.py

@@ -12,7 +12,7 @@ class VerifyCodes(db.Model):
     code_types = ['email', 'sms']
 
     id = db.Column(db.Integer, primary_key=True)
-    user_id = db.Column(db.Integer, db.ForeignKey('users.id'), index=True, unique=False, nullable=False)
+    user_id = db.Column(db.Integer, index=True, unique=False, nullable=False)
     code = db.Column(db.String, nullable=False)
     code_type = db.Column(db.Enum(code_types, name='verify_code_types'), index=False, unique=False, nullable=False)
     expiry_datetime = db.Column(db.DateTime, nullable=False)

app/notify_client/user_api_client.py

@@ -3,16 +3,72 @@ from client.notifications import BaseAPIClient
 
 class UserApiClient(BaseAPIClient):
 
-    def __init__(self, base_url, client_id, secret):
+    def __init__(self, base_url=None, client_id=None, secret=None):
-        super(self.__class__, self).__init__(base_url=base_url,
+        super(self.__class__, self).__init__(base_url=base_url or 'base_url',
-                                             client_id=client_id,
+                                             client_id=client_id or 'client_id',
-                                             secret=secret)
+                                             secret=secret or 'secret')
+
+    def init_app(self, app):
+        self.base_url = app.config['API_HOST_NAME']
+        self.client_id = app.config['ADMIN_CLIENT_USER_NAME']
+        self.secret = app.config['ADMIN_CLIENT_SECRET']
+        self.user_max_failed_login_count = app.config["MAX_FAILED_LOGIN_COUNT"]
 
     def register_user(self, name, email_address,  mobile_number, password):
         data = {
             "name": name,
             "email_address": email_address,
             "mobile_number": mobile_number,
-            "password": password}
+            "password": password
+        }
+        user_data = self.post("/user", data)
+        return User(user_data, max_failed_login_count=self.user_max_failed_login_count)
 
-        return self.post("/user", data)
+
+class User(object):
+
+    def __init__(self, fields, max_failed_login_count=3):
+        self.fields = fields
+        self.max_failed_login_count = max_failed_login_count
+
+    @property
+    def id(self):
+        return self.fields.get('id')
+
+    @property
+    def name(self):
+        return self.fields.get('name')
+
+    @property
+    def email_address(self):
+        return self.fields.get('email_address')
+
+    @property
+    def mobile_number(self):
+        return self.fields.get('mobile_number')
+
+    @property
+    def password_changed_at(self):
+        return self.fields.get('password_changed_at')
+
+    @property
+    def is_authenticated(self):
+        return self.fields.get('is_authenticated')
+
+    @property
+    def is_active(self):
+        if self.fields.get('state') != 'active':
+            return False
+        else:
+            return True
+
+    @property
+    def is_anonymous(self):
+        return False
+
+    @property
+    def is_locked(self):
+        if self.fields.get('failed_login_count') < self.max_failed_login_count:
+            return False
+        else:
+            return True

migrations/versions/90_remove_user_fk_verify_codes.py

@@ -0,0 +1,26 @@
+"""empty message
+
+Revision ID: 90_remove_user_fk_verify_codes
+Revises: 80_remove_services
+Create Date: 2016-01-19 20:57:41.986177
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '90_remove_user_fk_verify_codes'
+down_revision = '80_remove_services'
+
+from alembic import op
+import sqlalchemy as sa
+
+
+def upgrade():
+    ### commands auto generated by Alembic - please adjust! ###
+    op.drop_constraint('verify_codes_user_id_fkey', 'verify_codes', type_='foreignkey')
+    ### end Alembic commands ###
+
+
+def downgrade():
+    ### commands auto generated by Alembic - please adjust! ###
+    op.create_foreign_key('verify_codes_user_id_fkey', 'verify_codes', 'users', ['user_id'], ['id'])
+    ### end Alembic commands ###

tests/__init__.py

@@ -53,6 +53,19 @@ def create_test_user(state):
     return user
 
 
+def create_test_api_user(state):
+    from app.notify_client.user_api_client import User
+    user_data = {'id': 1,
+                 'name': 'Test User',
+                 'password': 'somepassword',
+                 'email_address': TEST_USER_EMAIL,
+                 'mobile_number': '+441234123412',
+                 'state': state
+                 }
+    user = User(user_data)
+    return user
+
+
 def create_another_test_user(state):
     user = User(name='Another Test User',
                 password='someOtherpassword',

tests/app/main/dao/test_verify_codes_dao.py

@@ -3,17 +3,15 @@ from pytest import fail
 
 from app.main.dao import verify_codes_dao
 from app.main.encryption import check_hash
-from tests import create_test_user, create_another_test_user
 
 
 def test_insert_new_code_and_get_it_back(app_, db_, db_session):
-    user = create_test_user('pending')
 
-    verify_codes_dao.add_code(user_id=user.id, code='12345', code_type='email')
+    verify_codes_dao.add_code(user_id=1, code='12345', code_type='email')
-    saved_codes = verify_codes_dao.get_codes(user_id=user.id, code_type='email')
+    saved_codes = verify_codes_dao.get_codes(user_id=1, code_type='email')
     assert len(saved_codes) == 1
     saved_code = saved_codes[0]
-    assert saved_code.user_id == user.id
+    assert saved_code.user_id == 1
     assert check_hash('12345', saved_code.code)
     assert saved_code.code_type == 'email'
     assert saved_code.code_used is False
@@ -22,65 +20,18 @@ def test_insert_new_code_and_get_it_back(app_, db_, db_session):
 def test_insert_new_code_should_thrw_exception_when_type_does_not_exist(app_,
                                                                         db_,
                                                                         db_session):
-    user = create_test_user('pending')
     try:
-        verify_codes_dao.add_code(user_id=user.id, code='23545', code_type='not_real')
+        verify_codes_dao.add_code(user_id=1, code='23545', code_type='not_real')
         fail('Should have thrown an exception')
     except sqlalchemy.exc.DataError as e:
         assert 'invalid input value for enum verify_code_types: "not_real"' in e.orig.pgerror
 
 
-def test_should_throw_exception_when_user_does_not_exist(app_,
-                                                         db_,
-                                                         db_session):
-    try:
-        verify_codes_dao.add_code(user_id=1, code='12345', code_type='email')
-        fail('Should throw exception')
-    except sqlalchemy.exc.IntegrityError as e:
-        assert 'ERROR:  insert or update on table "verify_codes" violates ' \
-               'foreign key constraint "verify_codes_user_id_fkey"' in e.orig.pgerror
-
-
 def test_should_return_none_if_code_is_used(app_,
                                             db_,
                                             db_session):
-    user = create_test_user('pending')
 
-    verify_codes_dao.add_code(user_id=user.id, code='12345', code_type='email')
+    code = verify_codes_dao.add_code(user_id=1, code='12345', code_type='email')
-    verify_codes_dao.use_code(user_id=user.id, code='12345', code_type='email')
+    verify_codes_dao.use_code(code.id)
-    saved_code = verify_codes_dao.get_code_by_code(user_id=user.id, code_type='email', code='12345')
+    saved_code = verify_codes_dao.get_code_by_code(user_id=1, code_type='email', code='12345')
-    assert saved_code.code_used is True
+    assert not saved_code
-
-
-def test_should_return_none_if_code_is_used(app_,
-                                            db_,
-                                            db_session):
-    user = create_test_user('pending')
-
-    verify_codes_dao.add_code(user_id=user.id, code='12345', code_type='sms')
-    code = verify_codes_dao.get_codes(user_id=user.id, code_type='sms')
-    verify_codes_dao.use_code(code[0].id)
-    used_code = verify_codes_dao.get_codes(user_id=user.id, code_type='sms')
-    assert used_code == []
-
-
-def test_should_return_all_unused_code_when_there_are_many(app_,
-                                                           db_,
-                                                           db_session):
-    user = create_test_user('pending')
-    another_user = create_another_test_user('active')
-    verify_codes_dao.add_code(user_id=user.id, code='12345', code_type='sms')
-    id = verify_codes_dao.add_code(user_id=user.id, code='09876', code_type='email')
-    verify_codes_dao.use_code(id)
-    verify_codes_dao.add_code(user_id=user.id, code='12345', code_type='email')
-    verify_codes_dao.add_code(user_id=user.id, code='23456', code_type='email')
-    verify_codes_dao.add_code(user_id=another_user.id, code='12345', code_type='sms')
-    verify_codes_dao.add_code(user_id=another_user.id, code='12345', code_type='email')
-
-    user_codes = verify_codes_dao.get_codes(user_id=user.id, code_type='email')
-    assert len(user_codes) == 2
-    s = sorted(user_codes, key=lambda r: r.expiry_datetime)
-    assert check_hash('12345', s[0].code)
-    assert check_hash('23456', s[1].code)
-    assert [(code.code_used, code.code_type, code.user_id) for code in user_codes] == \
-           [(False, 'email', user.id), (False, 'email', user.id)]

tests/app/main/views/test_register.py

@@ -1,6 +1,6 @@
 from flask import url_for
 
-from tests.conftest import mock_register_user
+from tests.conftest import mock_register_user as mock_user
 
 
 def test_render_register_returns_template_with_form(app_, db_, db_session):
@@ -16,7 +16,6 @@ def test_process_register_creates_new_user(app_,
                                            mock_send_sms,
                                            mock_send_email,
                                            mocker):
-
     user_data = {
         'name': 'Some One Valid',
         'email_address': 'someone@example.gov.uk',
@@ -24,7 +23,7 @@ def test_process_register_creates_new_user(app_,
         'password': 'validPassword!'
     }
 
-    mock_register_user(mocker, user_data)
+    mock_user(mocker, user_data)
 
     with app_.test_request_context():
         response = app_.test_client().post('/register',
@@ -76,7 +75,7 @@ def test_should_add_verify_codes_on_session(app_,
         'password': 'validPassword!'
     }
 
-    mock_register_user(mocker, user_data)
+    mock_user(mocker, user_data)
     with app_.test_client() as client:
         response = client.post('/register',
                                data=user_data)

tests/app/main/views/test_verify.py

@@ -1,6 +1,8 @@
 from flask import json, url_for
 from app.main.dao import users_dao, verify_codes_dao
-from tests import create_test_user
+from tests import create_test_api_user
+
+import pytest
 
 
 def test_should_return_verify_template(app_, db_, db_session):
@@ -9,8 +11,8 @@ def test_should_return_verify_template(app_, db_, db_session):
             # TODO this lives here until we work out how to
             # reassign the session after it is lost mid register process
             with client.session_transaction() as session:
-                user = create_test_user('pending')
+                user = create_test_api_user('pending')
-                session['user_email'] = user.email_address
+                session['user_details'] = {'email_address': user.email_address, 'id': user.id}
             response = client.get(url_for('main.verify'))
             assert response.status_code == 200
             assert (
@@ -24,8 +26,8 @@ def test_should_redirect_to_add_service_when_code_are_correct(app_,
     with app_.test_request_context():
         with app_.test_client() as client:
             with client.session_transaction() as session:
-                user = create_test_user('pending')
+                user = create_test_api_user('pending')
-                session['user_email'] = user.email_address
+                session['user_details'] = {'email_address': user.email_address, 'id': user.id}
             verify_codes_dao.add_code(user_id=user.id, code='12345', code_type='sms')
             verify_codes_dao.add_code(user_id=user.id, code='23456', code_type='email')
             response = client.post(url_for('main.verify'),
@@ -35,12 +37,13 @@ def test_should_redirect_to_add_service_when_code_are_correct(app_,
             assert response.location == url_for('main.add_service', first='first', _external=True)
 
 
+@pytest.mark.xfail(reason='Activation refactor to use api not completed')
 def test_should_activate_user_after_verify(app_, db_, db_session):
     with app_.test_request_context():
         with app_.test_client() as client:
             with client.session_transaction() as session:
-                user = create_test_user('pending')
+                user = create_test_api_user('pending')
-                session['user_email'] = user.email_address
+                session['user_details'] = {'email_address': user.email_address, 'id': user.id}
             verify_codes_dao.add_code(user_id=user.id, code='12345', code_type='sms')
             verify_codes_dao.add_code(user_id=user.id, code='23456', code_type='email')
             client.post(url_for('main.verify'),
@@ -55,8 +58,8 @@ def test_should_return_200_when_codes_are_wrong(app_, db_, db_session):
     with app_.test_request_context():
         with app_.test_client() as client:
             with client.session_transaction() as session:
-                user = create_test_user('pending')
+                user = create_test_api_user('pending')
-                session['user_email'] = user.email_address
+                session['user_details'] = {'email_address': user.email_address, 'id': user.id}
             verify_codes_dao.add_code(user_id=user.id, code='23345', code_type='sms')
             verify_codes_dao.add_code(user_id=user.id, code='98456', code_type='email')
             response = client.post(url_for('main.verify'),
@@ -68,14 +71,15 @@ def test_should_return_200_when_codes_are_wrong(app_, db_, db_session):
             assert resp_data.count('Code does not match') == 2
 
 
+@pytest.mark.xfail(reason='Activation refactor to use api not completed')
 def test_should_mark_all_codes_as_used_when_many_codes_exist(app_,
                                                              db_,
                                                              db_session):
     with app_.test_request_context():
         with app_.test_client() as client:
             with client.session_transaction() as session:
-                user = create_test_user('pending')
+                user = create_test_api_user('pending')
-                session['user_email'] = user.email_address
+                session['user_details'] = {'email_address': user.email_address, 'id': user.id}
             code1 = verify_codes_dao.add_code(user_id=user.id, code='23345', code_type='sms')
             code2 = verify_codes_dao.add_code(user_id=user.id, code='98456', code_type='email')
             code3 = verify_codes_dao.add_code(user_id=user.id, code='12345', code_type='sms')

tests/conftest.py

@@ -204,13 +204,9 @@ def mock_delete_service_template(mocker):
 
 
 def mock_register_user(mocker, user_data):
-    data = {
+    user_data['id'] = 1
-        "email_address": user_data['email_address'],
+    from app.notify_client.user_api_client import User
-        "failed_login_count": 0,
+    user = User(user_data)
-        "mobile_number": user_data['mobile_number'],
+    mock_class = mocker.patch('app.user_api_client.register_user')
-        "name": user_data['name'],
+    mock_class.return_value = user
-        "state": "pending"
-    }
-    mock_class = mocker.patch('app.main.views.register.UserApiClient')
-    mock_class.register_user.return_value = data
     return mock_class