diff --git a/app/dao/users_dao.py b/app/dao/users_dao.py index 5b389e6b7..db23fbe37 100644 --- a/app/dao/users_dao.py +++ b/app/dao/users_dao.py @@ -8,7 +8,8 @@ from app.models import User def save_model_user(usr, update_dict={}): if update_dict: - del update_dict['id'] + if update_dict.get('id'): + del update_dict['id'] db.session.query(User).filter_by(id=usr.id).update(update_dict) else: db.session.add(usr) diff --git a/app/encryption.py b/app/encryption.py new file mode 100644 index 000000000..51caaab72 --- /dev/null +++ b/app/encryption.py @@ -0,0 +1,10 @@ +from flask.ext.bcrypt import generate_password_hash, check_password_hash + + +def hashpw(password): + return generate_password_hash(password.encode('UTF-8'), 10) + + +def check_hash(password, hashed_password): + # If salt is invalid throws a 500 should add try/catch here + return check_password_hash(hashed_password, password) diff --git a/app/models.py b/app/models.py index 2c6b8be1d..745e4ef0e 100644 --- a/app/models.py +++ b/app/models.py @@ -2,6 +2,10 @@ from . import db import datetime from sqlalchemy.dialects.postgresql import UUID +from app.encryption import ( + hashpw, + check_hash +) def filter_null_value_fields(obj): @@ -14,6 +18,7 @@ class User(db.Model): __tablename__ = 'users' id = db.Column(db.Integer, primary_key=True) + name = db.Column(db.String, nullable=False, index=True, unique=False) email_address = db.Column(db.String(255), nullable=False, index=True, unique=True) created_at = db.Column( db.DateTime, @@ -27,6 +32,23 @@ class User(db.Model): unique=False, nullable=True, onupdate=datetime.datetime.now) + _password = db.Column(db.String, index=False, unique=False, nullable=False) + mobile_number = db.Column(db.String, index=False, unique=False, nullable=False) + password_changed_at = db.Column(db.DateTime, index=False, unique=False, nullable=True) + logged_in_at = db.Column(db.DateTime, nullable=True) + failed_login_count = db.Column(db.Integer, nullable=False, default=0) + state = db.Column(db.String, nullable=False, default='pending') + + @property + def password(self): + raise AttributeError("Password not readable") + + @password.setter + def password(self, password): + self._password = hashpw(password) + + def check_password(self, password): + return check_hash(password, self._password) user_to_service = db.Table( diff --git a/app/schemas.py b/app/schemas.py index f5d9a0349..542144ea7 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -11,7 +11,7 @@ from marshmallow import post_load class UserSchema(ma.ModelSchema): class Meta: model = models.User - exclude = ("updated_at", "created_at", "user_to_service") + exclude = ("updated_at", "created_at", "user_to_service", "_password") # TODO process users list, to return a list of user.id diff --git a/app/user/rest.py b/app/user/rest.py index 2731712a7..9e1fbb6d0 100644 --- a/app/user/rest.py +++ b/app/user/rest.py @@ -17,8 +17,14 @@ user = Blueprint('user', __name__) @user.route('', methods=['POST']) def create_user(): user, errors = user_schema.load(request.get_json()) + req_json = request.get_json() + if not req_json.get('password'): + errors = {'password': ['Missing data for required field.']} + return jsonify(result="error", message=errors), 400 if errors: return jsonify(result="error", message=errors), 400 + + user.password = req_json.get('password') save_model_user(user) return jsonify(data=user_schema.dump(user).data), 201 @@ -36,16 +42,11 @@ def update_user(user_id): status_code = 202 delete_model_user(user) else: + # TODO removed some validation checking by using load + # which will need to be done in another way status_code = 200 - # TODO there has got to be a better way to do the next three lines - update_user, errors = user_schema.load(request.get_json()) - if errors: - return jsonify(result="error", message=errors), 400 - update_dict, errors = user_schema.dump(update_user) - # TODO FIX ME - # Remove update_service model which is added to db.session db.session.rollback() - save_model_user(user, update_dict=update_dict) + save_model_user(user, update_dict=request.get_json()) return jsonify(data=user_schema.dump(user).data), status_code diff --git a/migrations/versions/0006_add_user_details.py b/migrations/versions/0006_add_user_details.py new file mode 100644 index 000000000..a789a348c --- /dev/null +++ b/migrations/versions/0006_add_user_details.py @@ -0,0 +1,40 @@ +"""empty message + +Revision ID: 0006_add_user_details +Revises: 0005_add_job_details +Create Date: 2016-01-19 11:16:06.518285 + +""" + +# revision identifiers, used by Alembic. +revision = '0006_add_user_details' +down_revision = '0005_add_job_details' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + ### commands auto generated by Alembic - please adjust! ### + op.add_column('users', sa.Column('_password', sa.String(), nullable=False)) + op.add_column('users', sa.Column('failed_login_count', sa.Integer(), nullable=False)) + op.add_column('users', sa.Column('logged_in_at', sa.DateTime(), nullable=True)) + op.add_column('users', sa.Column('mobile_number', sa.String(), nullable=False)) + op.add_column('users', sa.Column('name', sa.String(), nullable=False)) + op.add_column('users', sa.Column('password_changed_at', sa.DateTime(), nullable=True)) + op.add_column('users', sa.Column('state', sa.String(), nullable=False)) + op.create_index(op.f('ix_users_name'), 'users', ['name'], unique=False) + ### end Alembic commands ### + + +def downgrade(): + ### commands auto generated by Alembic - please adjust! ### + op.drop_index(op.f('ix_users_name'), table_name='users') + op.drop_column('users', 'state') + op.drop_column('users', 'password_changed_at') + op.drop_column('users', 'name') + op.drop_column('users', 'mobile_number') + op.drop_column('users', 'logged_in_at') + op.drop_column('users', 'failed_login_count') + op.drop_column('users', '_password') + ### end Alembic commands ### diff --git a/requirements.txt b/requirements.txt index 5112e5b2f..d76120cfd 100644 --- a/requirements.txt +++ b/requirements.txt @@ -10,6 +10,7 @@ marshmallow==2.4.2 marshmallow-sqlalchemy==0.8.0 flask-marshmallow==0.6.2 itsdangerous==0.24 +Flask-Bcrypt==0.6.2 git+https://github.com/alphagov/notifications-python-client.git@0.1.5#egg=notifications-python-client==0.1.5 diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 79cf576c2..e8854a57a 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -12,7 +12,14 @@ import uuid def sample_user(notify_db, notify_db_session, email="notify@digital.cabinet-office.gov.uk"): - user = User(**{'email_address': email}) + data = { + 'name': 'Test User', + 'email_address': email, + 'password': 'password', + 'mobile_number': '+44 7700 900986', + 'state': 'active' + } + user = User(**data) save_model_user(user) return user diff --git a/tests/app/dao/test_users_dao.py b/tests/app/dao/test_users_dao.py index 6190ab20b..a8b15c603 100644 --- a/tests/app/dao/test_users_dao.py +++ b/tests/app/dao/test_users_dao.py @@ -8,7 +8,13 @@ from app.models import User def test_create_user(notify_api, notify_db, notify_db_session): email = 'notify@digital.cabinet-office.gov.uk' - user = User(**{'email_address': email}) + data = { + 'name': 'Test User', + 'email_address': email, + 'password': 'password', + 'mobile_number': '+44 7700 900986' + } + user = User(**data) save_model_user(user) assert User.query.count() == 1 assert User.query.first().email_address == email diff --git a/tests/app/user/test_rest.py b/tests/app/user/test_rest.py index 7a681af08..e6cf0ee3b 100644 --- a/tests/app/user/test_rest.py +++ b/tests/app/user/test_rest.py @@ -19,7 +19,17 @@ def test_get_user_list(notify_api, notify_db, notify_db_session, sample_user, sa assert response.status_code == 200 json_resp = json.loads(response.get_data(as_text=True)) assert len(json_resp['data']) == 2 - assert {"email_address": sample_user.email_address, "id": sample_user.id} in json_resp['data'] + expected = { + "name": "Test User", + "email_address": sample_user.email_address, + "id": sample_user.id, + "mobile_number": "+44 7700 900986", + "password_changed_at": None, + "logged_in_at": None, + "state": "active", + "failed_login_count": 0 + } + assert expected in json_resp['data'] def test_get_user(notify_api, notify_db, notify_db_session, sample_user, sample_admin_service_id): @@ -36,7 +46,17 @@ def test_get_user(notify_api, notify_db, notify_db_session, sample_user, sample_ headers=[header]) assert resp.status_code == 200 json_resp = json.loads(resp.get_data(as_text=True)) - assert json_resp['data'] == {"email_address": sample_user.email_address, "id": sample_user.id} + expected = { + "name": "Test User", + "email_address": sample_user.email_address, + "id": sample_user.id, + "mobile_number": "+44 7700 900986", + "password_changed_at": None, + "logged_in_at": None, + "state": "active", + "failed_login_count": 0 + } + assert json_resp['data'] == expected def test_post_user(notify_api, notify_db, notify_db_session, sample_admin_service_id): @@ -46,8 +66,16 @@ def test_post_user(notify_api, notify_db, notify_db_session, sample_admin_servic with notify_api.test_request_context(): with notify_api.test_client() as client: assert User.query.count() == 1 - data = {'email_address': 'user@digital.cabinet-office.gov.uk'} - + data = { + "name": "Test User", + "email_address": "user@digital.cabinet-office.gov.uk", + "password": "password", + "mobile_number": "+44 7700 900986", + "password_changed_at": None, + "logged_in_at": None, + "state": "active", + "failed_login_count": 0 + } auth_header = create_authorization_header(service_id=sample_admin_service_id, path=url_for('user.create_user'), method='POST', @@ -73,7 +101,14 @@ def test_post_user_missing_attribute_email(notify_api, notify_db, notify_db_sess with notify_api.test_client() as client: assert User.query.count() == 1 data = { - 'blah': 'blah.blah'} + "name": "Test User", + "password": "password", + "mobile_number": "+44 7700 900986", + "password_changed_at": None, + "logged_in_at": None, + "state": "active", + "failed_login_count": 0 + } auth_header = create_authorization_header(service_id=sample_admin_service_id, path=url_for('user.create_user'), method='POST', @@ -89,6 +124,37 @@ def test_post_user_missing_attribute_email(notify_api, notify_db, notify_db_sess assert {'email_address': ['Missing data for required field.']} == json_resp['message'] +def test_post_user_missing_attribute_password(notify_api, notify_db, notify_db_session, sample_admin_service_id): + """ + Tests POST endpoint '/' missing attribute password. + """ + with notify_api.test_request_context(): + with notify_api.test_client() as client: + assert User.query.count() == 1 + data = { + "name": "Test User", + "email_address": "user@digital.cabinet-office.gov.uk", + "mobile_number": "+44 7700 900986", + "password_changed_at": None, + "logged_in_at": None, + "state": "active", + "failed_login_count": 0 + } + auth_header = create_authorization_header(service_id=sample_admin_service_id, + path=url_for('user.create_user'), + method='POST', + request_body=json.dumps(data)) + headers = [('Content-Type', 'application/json'), auth_header] + resp = client.post( + url_for('user.create_user'), + data=json.dumps(data), + headers=headers) + assert resp.status_code == 400 + assert User.query.count() == 1 + json_resp = json.loads(resp.get_data(as_text=True)) + assert {'password': ['Missing data for required field.']} == json_resp['message'] + + def test_put_user(notify_api, notify_db, notify_db_session, sample_user, sample_admin_service_id): """ Tests PUT endpoint '/' to update a user. @@ -98,7 +164,8 @@ def test_put_user(notify_api, notify_db, notify_db_session, sample_user, sample_ assert User.query.count() == 2 new_email = 'new@digital.cabinet-office.gov.uk' data = { - 'email_address': new_email} + 'email_address': new_email + } auth_header = create_authorization_header(service_id=sample_admin_service_id, path=url_for('user.update_user', user_id=sample_user.id), method='PUT', @@ -112,9 +179,18 @@ def test_put_user(notify_api, notify_db, notify_db_session, sample_user, sample_ assert User.query.count() == 2 user = User.query.filter_by(email_address=new_email).first() json_resp = json.loads(resp.get_data(as_text=True)) - assert json_resp['data'] == {'email_address': new_email, 'id': user.id} + expected = { + "name": "Test User", + "email_address": new_email, + "mobile_number": "+44 7700 900986", + "password_changed_at": None, + "id": user.id, + "logged_in_at": None, + "state": "active", + "failed_login_count": 0 + } + assert json_resp['data'] == expected assert json_resp['data']['email_address'] == new_email - assert json_resp['data']['id'] == user.id def test_put_user_not_exists(notify_api, notify_db, notify_db_session, sample_user, sample_admin_service_id): @@ -144,33 +220,6 @@ def test_put_user_not_exists(notify_api, notify_db, notify_db_session, sample_us assert user.email_address != new_email -def test_put_user_missing_email(notify_api, notify_db, notify_db_session, sample_user, sample_admin_service_id): - """ - Tests PUT endpoint '/' missing attribute email. - """ - with notify_api.test_request_context(): - with notify_api.test_client() as client: - assert User.query.count() == 2 - new_email = 'new@digital.cabinet-office.gov.uk' - data = { - 'blah': new_email} - auth_header = create_authorization_header(service_id=sample_admin_service_id, - path=url_for('user.update_user', user_id=sample_user.id), - method='PUT', - request_body=json.dumps(data)) - headers = [('Content-Type', 'application/json'), auth_header] - resp = client.put( - url_for('user.update_user', user_id=sample_user.id), - data=json.dumps(data), - headers=headers) - assert resp.status_code == 400 - assert User.query.count() == 2 - user = User.query.get(sample_user.id) - json_resp = json.loads(resp.get_data(as_text=True)) - assert user.email_address == sample_user.email_address - assert {'email_address': ['Missing data for required field.']} == json_resp['message'] - - def test_get_user_services(notify_api, notify_db, notify_db_session, sample_service, sample_admin_service_id): """ Tests GET endpoint "//service/" to retrieve services for a user. @@ -280,7 +329,18 @@ def test_delete_user(notify_api, notify_db, notify_db_session, sample_user, samp assert resp.status_code == 202 json_resp = json.loads(resp.get_data(as_text=True)) assert User.query.count() == 1 - assert json_resp['data'] == {'id': sample_user.id, 'email_address': sample_user.email_address} + expected = { + "name": "Test User", + "email_address": sample_user.email_address, + "mobile_number": "+44 7700 900986", + "password_changed_at": None, + "id": sample_user.id, + "logged_in_at": None, + "state": "active", + "failed_login_count": 0 + + } + assert json_resp['data'] == expected def test_delete_user_not_exists(notify_api, notify_db, notify_db_session, sample_user, sample_admin_service_id):