From 0eed4c99a7c5892e9d623ccb5a760dc62c91838d Mon Sep 17 00:00:00 2001 From: Pea Tyczynska Date: Fri, 24 Jan 2020 15:18:39 +0000 Subject: [PATCH] Add email_access_valdiated_at field to user table, populate it and update it when users have to use their email to interact with Notify service. Initial population: If user has email_auth, set last_validated_at to logged_in_at. If user has sms_auth, set it to created_at. Then: Update email_access_valdiated_at date when: - user with email_auth logs in - new user is created - user resets password when logged out, meaning we send them an email with a link they have to click to reset their password. --- app/dao/users_dao.py | 18 +++++--- app/models.py | 2 + app/schemas.py | 1 + app/user/rest.py | 9 ++-- .../0313_email_access_validated_at.py | 43 +++++++++++++++++++ tests/app/dao/test_services_dao.py | 8 ++-- tests/app/dao/test_users_dao.py | 26 ++++++++--- tests/app/db.py | 2 +- tests/app/service/test_rest.py | 12 +++--- tests/app/user/test_rest_verify.py | 5 +++ 10 files changed, 98 insertions(+), 28 deletions(-) create mode 100644 migrations/versions/0313_email_access_validated_at.py diff --git a/app/dao/users_dao.py b/app/dao/users_dao.py index 4706673ea..3e3924acd 100644 --- a/app/dao/users_dao.py +++ b/app/dao/users_dao.py @@ -28,15 +28,17 @@ def save_user_attribute(usr, update_dict={}): db.session.commit() -def save_model_user(usr, update_dict={}, pwd=None): - if pwd: - usr.password = pwd - usr.password_changed_at = datetime.utcnow() +def save_model_user(user, update_dict={}, password=None, validated_email_access=False): + if password: + user.password = password + user.password_changed_at = datetime.utcnow() + if validated_email_access: + user.email_access_validated_at = datetime.utcnow() if update_dict: _remove_values_for_keys_if_present(update_dict, ['id', 'password_changed_at']) - db.session.query(User).filter_by(id=usr.id).update(update_dict) + db.session.query(User).filter_by(id=user.id).update(update_dict) else: - db.session.add(usr) + db.session.add(user) db.session.commit() @@ -121,10 +123,12 @@ def reset_failed_login_count(user): db.session.commit() -def update_user_password(user, password): +def update_user_password(user, password, validated_email_access=False): # reset failed login count - they've just reset their password so should be fine user.password = password user.password_changed_at = datetime.utcnow() + if validated_email_access: + user.email_access_validated_at = datetime.utcnow() db.session.add(user) db.session.commit() diff --git a/app/models.py b/app/models.py index 7ab5c5e89..9f5b4dfdb 100644 --- a/app/models.py +++ b/app/models.py @@ -114,6 +114,7 @@ class User(db.Model): platform_admin = db.Column(db.Boolean, nullable=False, default=False) current_session_id = db.Column(UUID(as_uuid=True), nullable=True) auth_type = db.Column(db.String, db.ForeignKey('auth_type.name'), index=True, nullable=False, default=SMS_AUTH_TYPE) + email_access_validated_at = db.Column(db.DateTime, index=False, unique=False, nullable=False) # either email auth or a mobile number must be provided CheckConstraint("auth_type = 'email_auth' or mobile_number is not null") @@ -162,6 +163,7 @@ class User(db.Model): 'auth_type': self.auth_type, 'current_session_id': self.current_session_id, 'failed_login_count': self.failed_login_count, + 'email_access_validated_at': self.email_access_validated_at, 'logged_in_at': self.logged_in_at.strftime(DATETIME_FORMAT) if self.logged_in_at else None, 'mobile_number': self.mobile_number, 'organisations': [x.id for x in self.organisations if x.active], diff --git a/app/schemas.py b/app/schemas.py index 8659834c5..fc740c53e 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -101,6 +101,7 @@ class UserSchema(BaseSchema): model = models.User exclude = ( "updated_at", + "email_access_validated_at", "created_at", "user_to_service", "user_to_organisation", diff --git a/app/user/rest.py b/app/user/rest.py index 8a53d8b0d..7eed40c43 100644 --- a/app/user/rest.py +++ b/app/user/rest.py @@ -78,7 +78,7 @@ def create_user(): if not req_json.get('password', None): errors.update({'password': ['Missing data for required field.']}) raise InvalidRequest(errors, status_code=400) - save_model_user(user_to_create, pwd=req_json.get('password')) + save_model_user(user_to_create, password=req_json.get('password'), validated_email_access=True) result = user_to_create.serialize() return jsonify(data=result), 201 @@ -197,6 +197,8 @@ def verify_user_code(user_id): user_to_verify.current_session_id = str(uuid.uuid4()) user_to_verify.logged_in_at = datetime.utcnow() + if user_to_verify.auth_type == 'email_auth': + user_to_verify.email_access_validated_at = datetime.utcnow() user_to_verify.failed_login_count = 0 save_model_user(user_to_verify) @@ -459,11 +461,12 @@ def send_user_reset_password(): def update_password(user_id): user = get_user_by_id(user_id=user_id) req_json = request.get_json() - pwd = req_json.get('_password') + password = req_json.get('_password') + validated_email_access = req_json.get('validated_email_access') update_dct, errors = user_update_password_schema_load_json.load(req_json) if errors: raise InvalidRequest(errors, status_code=400) - update_user_password(user, pwd) + update_user_password(user, password, validated_email_access=validated_email_access) return jsonify(data=user.serialize()), 200 diff --git a/migrations/versions/0313_email_access_validated_at.py b/migrations/versions/0313_email_access_validated_at.py new file mode 100644 index 000000000..5f3f06e4f --- /dev/null +++ b/migrations/versions/0313_email_access_validated_at.py @@ -0,0 +1,43 @@ +""" + +Revision ID: 0313_email_access_validated_at +Revises: 0312_populate_returned_letters +Create Date: 2020-01-28 18:03:22.237386 + +""" +from alembic import op +import sqlalchemy as sa + + +revision = '0313_email_access_validated_at' +down_revision = '0312_populate_returned_letters' + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.add_column('users', sa.Column('email_access_validated_at', sa.DateTime(), nullable=True)) + # if user has email_auth, set email_access_validated_at on last login, else set it at user created_at date. + op.execute(""" + UPDATE + users + SET + email_access_validated_at = created_at + WHERE + auth_type = 'sms_auth' + """) + op.execute(""" + UPDATE + users + SET + email_access_validated_at = logged_in_at + WHERE + auth_type = 'email_auth' + """) + op.alter_column('users', 'email_access_validated_at', nullable=False) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_column('users', 'email_access_validated_at') + # ### end Alembic commands ### diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 1bdbd9358..60540ff0c 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -227,7 +227,7 @@ def test_should_add_user_to_service(notify_db_session): password='password', mobile_number='+447700900986' ) - save_model_user(new_user) + save_model_user(new_user, validated_email_access=True) dao_add_user_to_service(service, new_user) assert new_user in Service.query.first().users @@ -296,7 +296,7 @@ def test_should_remove_user_from_service(notify_db_session): password='password', mobile_number='+447700900986' ) - save_model_user(new_user) + save_model_user(new_user, validated_email_access=True) dao_add_user_to_service(service, new_user) assert new_user in Service.query.first().users dao_remove_user_from_service(service, new_user) @@ -397,7 +397,7 @@ def test_get_all_user_services_only_returns_services_user_has_access_to(notify_d password='password', mobile_number='+447700900986' ) - save_model_user(new_user) + save_model_user(new_user, validated_email_access=True) dao_add_user_to_service(service_3, new_user) assert len(dao_fetch_all_services_by_user(user.id)) == 3 assert dao_fetch_all_services_by_user(user.id)[0].name == 'service 1' @@ -710,7 +710,7 @@ def test_add_existing_user_to_another_service_doesnot_change_old_permissions(not password='password', mobile_number='+447700900987' ) - save_model_user(other_user) + save_model_user(other_user, validated_email_access=True) service_two = Service(name="service_two", email_from="service_two", message_limit=1000, diff --git a/tests/app/dao/test_users_dao.py b/tests/app/dao/test_users_dao.py index 43f37e24b..b56e204de 100644 --- a/tests/app/dao/test_users_dao.py +++ b/tests/app/dao/test_users_dao.py @@ -29,6 +29,7 @@ from app.models import EMAIL_AUTH_TYPE, User, VerifyCode from tests.app.db import create_permissions, create_service, create_template_folder, create_user +@freeze_time('2020-01-28T12:00:00') @pytest.mark.parametrize('phone_number', [ '+447700900986', '+1-800-555-5555', @@ -42,12 +43,14 @@ def test_create_user(notify_db_session, phone_number): 'mobile_number': phone_number } user = User(**data) - save_model_user(user) + save_model_user(user, password='password', validated_email_access=True) assert User.query.count() == 1 - assert User.query.first().email_address == email - assert User.query.first().id == user.id - assert User.query.first().mobile_number == phone_number - assert not user.platform_admin + user_query = User.query.first() + assert user_query.email_address == email + assert user_query.id == user.id + assert user_query.mobile_number == phone_number + assert user_query.email_access_validated_at == datetime.utcnow() + assert not user_query.platform_admin def test_get_all_users(notify_db_session): @@ -148,11 +151,20 @@ def test_update_user_attribute(client, sample_user, user_attribute, user_value): assert getattr(sample_user, user_attribute) == user_value -def test_update_user_password(notify_api, notify_db, notify_db_session, sample_user): +@freeze_time('2020-01-24T12:00:00') +@pytest.mark.parametrize('from_email', [True, False]) +def test_update_user_password(notify_api, notify_db, notify_db_session, sample_user, from_email): + sample_user.password_changed_at = datetime.utcnow() - timedelta(days=1) + sample_user.email_access_validated_at = datetime.utcnow() - timedelta(days=1) password = 'newpassword' assert not sample_user.check_password(password) - update_user_password(sample_user, password) + update_user_password(sample_user, password, validated_email_access=from_email) assert sample_user.check_password(password) + assert sample_user.password_changed_at == datetime.utcnow() + if from_email: + assert sample_user.email_access_validated_at == datetime.utcnow() + else: + assert sample_user.email_access_validated_at == datetime.utcnow() - timedelta(days=1) def test_count_user_verify_codes(sample_user): diff --git a/tests/app/db.py b/tests/app/db.py index f3540f8ac..57f3ac5a9 100644 --- a/tests/app/db.py +++ b/tests/app/db.py @@ -82,7 +82,7 @@ def create_user( user = User.query.filter_by(email_address=email).first() if not user: user = User(**data) - save_model_user(user) + save_model_user(user, validated_email_access=True) return user diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 42f86f27c..5d55b2572 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -1189,7 +1189,7 @@ def test_add_existing_user_to_another_service_with_all_permissions( mobile_number='+4477123456' ) # they must exist in db first - save_model_user(user_to_add) + save_model_user(user_to_add, validated_email_access=True) data = { "permissions": [ @@ -1253,7 +1253,7 @@ def test_add_existing_user_to_another_service_with_send_permissions(notify_api, password='password', mobile_number='+4477123456' ) - save_model_user(user_to_add) + save_model_user(user_to_add, validated_email_access=True) data = { "permissions": [ @@ -1301,7 +1301,7 @@ def test_add_existing_user_to_another_service_with_manage_permissions(notify_api password='password', mobile_number='+4477123456' ) - save_model_user(user_to_add) + save_model_user(user_to_add, validated_email_access=True) data = { "permissions": [ @@ -1348,7 +1348,7 @@ def test_add_existing_user_to_another_service_with_folder_permissions(notify_api password='password', mobile_number='+4477123456' ) - save_model_user(user_to_add) + save_model_user(user_to_add, validated_email_access=True) folder_1 = create_template_folder(sample_service) folder_2 = create_template_folder(sample_service) @@ -1389,7 +1389,7 @@ def test_add_existing_user_to_another_service_with_manage_api_keys(notify_api, password='password', mobile_number='+4477123456' ) - save_model_user(user_to_add) + save_model_user(user_to_add, validated_email_access=True) data = {"permissions": [{"permission": "manage_api_keys"}]} @@ -1428,7 +1428,7 @@ def test_add_existing_user_to_non_existing_service_returns404(notify_api, password='password', mobile_number='+4477123456' ) - save_model_user(user_to_add) + save_model_user(user_to_add, validated_email_access=True) incorrect_id = uuid.uuid4() diff --git a/tests/app/user/test_rest_verify.py b/tests/app/user/test_rest_verify.py index 9d8ccee94..2af880f5f 100644 --- a/tests/app/user/test_rest_verify.py +++ b/tests/app/user/test_rest_verify.py @@ -40,6 +40,7 @@ def test_user_verify_sms_code(client, sample_sms_code): assert resp.status_code == 204 assert VerifyCode.query.first().code_used assert sample_sms_code.user.logged_in_at == datetime.utcnow() + assert sample_sms_code.user.email_access_validated_at != datetime.utcnow() assert sample_sms_code.user.current_session_id is not None @@ -417,6 +418,9 @@ def test_send_email_code_returns_404_for_bad_input_data(admin_request): @freeze_time('2016-01-01T12:00:00') def test_user_verify_email_code(admin_request, sample_user): + sample_user.logged_in_at = datetime.utcnow() - timedelta(days=1) + sample_user.email_access_validated_at = datetime.utcnow() - timedelta(days=1) + sample_user.auth_type = "email_auth" magic_code = str(uuid.uuid4()) verify_code = create_user_code(sample_user, magic_code, EMAIL_TYPE) @@ -434,6 +438,7 @@ def test_user_verify_email_code(admin_request, sample_user): assert verify_code.code_used assert sample_user.logged_in_at == datetime.utcnow() + assert sample_user.email_access_validated_at == datetime.utcnow() assert sample_user.current_session_id is not None