diff --git a/app/errors.py b/app/errors.py index 3deac5600..da3b719ca 100644 --- a/app/errors.py +++ b/app/errors.py @@ -2,6 +2,8 @@ from flask import ( jsonify, current_app ) +from sqlalchemy.exc import SQLAlchemyError, DataError +from sqlalchemy.orm.exc import NoResultFound def register_errors(blueprint): @@ -32,3 +34,18 @@ def register_errors(blueprint): def internal_server_error(e): current_app.logger.exception(e) return jsonify(error="Internal error"), 500 + + @blueprint.app_errorhandler(NoResultFound) + def no_result_found(e): + current_app.logger.error(e) + return jsonify(error="No result found"), 404 + + @blueprint.app_errorhandler(DataError) + def no_result_found(e): + current_app.logger.error(e) + return jsonify(error="No result found"), 404 + + @blueprint.app_errorhandler(SQLAlchemyError) + def db_error(e): + current_app.logger.error(e) + return jsonify(error="Database error occurred"), 500 diff --git a/app/schemas.py b/app/schemas.py index 18007f91d..0d8724e00 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -62,7 +62,8 @@ class JobSchema(BaseSchema): model = models.Job -class RequestVerifyCodeSchema(ma.Schema): +# TODO: Remove this schema once the admin app has stopped using the /user/code endpoint +class OldRequestVerifyCodeSchema(ma.Schema): code_type = fields.Str(required=True) to = fields.Str(required=False) @@ -73,6 +74,10 @@ class RequestVerifyCodeSchema(ma.Schema): raise ValidationError('Invalid code type') +class RequestVerifyCodeSchema(ma.Schema): + to = fields.Str(required=False) + + # TODO main purpose to be added later # when processing templates, template will be # common for all notifications. @@ -154,6 +159,8 @@ api_keys_schema = ApiKeySchema(many=True) job_schema = JobSchema() job_schema_load_json = JobSchema(load_json=True) jobs_schema = JobSchema(many=True) +# TODO: Remove this schema once the admin app has stopped using the /user/code endpoint +old_request_verify_code_schema = OldRequestVerifyCodeSchema() request_verify_code_schema = RequestVerifyCodeSchema() sms_admin_notification_schema = SmsAdminNotificationSchema() sms_template_notification_schema = SmsTemplateNotificationSchema() diff --git a/app/user/rest.py b/app/user/rest.py index 878176461..2a885d63c 100644 --- a/app/user/rest.py +++ b/app/user/rest.py @@ -1,7 +1,5 @@ from datetime import datetime from flask import (jsonify, request, abort, Blueprint, current_app) -from sqlalchemy.exc import DataError -from sqlalchemy.orm.exc import NoResultFound from app import encryption from app.dao.services_dao import get_model_services @@ -17,7 +15,8 @@ from app.dao.users_dao import ( ) from app.schemas import ( user_schema, users_schema, service_schema, services_schema, - request_verify_code_schema, user_schema_load_json) + old_request_verify_code_schema, user_schema_load_json, + request_verify_code_schema) from app.celery.tasks import (send_sms_code, send_email_code) user = Blueprint('user', __name__) @@ -42,15 +41,11 @@ def create_user(): @user.route('/', methods=['PUT', 'DELETE']) def update_user(user_id): - try: - user = get_model_users(user_id=user_id) - except DataError: - return jsonify(result="error", message="Invalid user id"), 400 - except NoResultFound: - return jsonify(result="error", message="User not found"), 404 + user_to_update = get_model_users(user_id=user_id) + if request.method == 'DELETE': status_code = 202 - delete_model_user(user) + delete_model_user(user_to_update) else: req_json = request.get_json() update_dct, errors = user_schema_load_json.load(req_json) @@ -62,18 +57,14 @@ def update_user(user_id): if errors: return jsonify(result="error", message=errors), 400 status_code = 200 - save_model_user(user, update_dict=update_dct, pwd=pwd) - return jsonify(data=user_schema.dump(user).data), status_code + save_model_user(user_to_update, update_dict=update_dct, pwd=pwd) + return jsonify(data=user_schema.dump(user_to_update).data), status_code @user.route('//verify/password', methods=['POST']) def verify_user_password(user_id): - try: - user = get_model_users(user_id=user_id) - except DataError: - return jsonify(result="error", message="Invalid user id"), 400 - except NoResultFound: - return jsonify(result="error", message="User not found"), 404 + user_to_verify = get_model_users(user_id=user_id) + txt_pwd = None try: txt_pwd = request.get_json()['password'] @@ -81,22 +72,18 @@ def verify_user_password(user_id): return jsonify( result="error", message={'password': ['Required field missing data']}), 400 - if user.check_password(txt_pwd): - reset_failed_login_count(user) + if user_to_verify.check_password(txt_pwd): + reset_failed_login_count(user_to_verify) return jsonify({}), 204 else: - increment_failed_login_count(user) + increment_failed_login_count(user_to_verify) return jsonify(result='error', message={'password': ['Incorrect password']}), 400 @user.route('//verify/code', methods=['POST']) def verify_user_code(user_id): - try: - user = get_model_users(user_id=user_id) - except DataError: - return jsonify(result="error", message="Invalid user id"), 400 - except NoResultFound: - return jsonify(result="error", message="User not found"), 404 + user_to_verify = get_model_users(user_id=user_id) + txt_code = None resp_json = request.get_json() txt_type = None @@ -111,7 +98,7 @@ def verify_user_code(user_id): errors.update({'code_type': ['Required field missing data']}) if errors: return jsonify(result="error", message=errors), 400 - code = get_user_code(user, txt_code, txt_type) + code = get_user_code(user_to_verify, txt_code, txt_type) if not code: return jsonify(result="error", message="Code not found"), 404 if datetime.now() > code.expiry_datetime or code.code_used: @@ -120,14 +107,9 @@ def verify_user_code(user_id): return jsonify({}), 204 -@user.route('//code', methods=['POST']) -def send_user_code(user_id): - try: - user = get_model_users(user_id=user_id) - except DataError: - return jsonify(result="error", message="Invalid user id"), 400 - except NoResultFound: - return jsonify(result="error", message="User not found"), 404 +@user.route('//sms-code', methods=['POST']) +def send_user_sms_code(user_id): + user_to_send_to = get_model_users(user_id=user_id) verify_code, errors = request_verify_code_schema.load(request.get_json()) if errors: @@ -135,13 +117,54 @@ def send_user_code(user_id): from app.dao.users_dao import create_secret_code secret_code = create_secret_code() - create_user_code(user, secret_code, verify_code.get('code_type')) + create_user_code(user_to_send_to, secret_code, 'sms') + + mobile = user_to_send_to.mobile_number if verify_code.get('to', None) is None else verify_code.get('to') + verification_message = {'to': mobile, 'secret_code': secret_code} + + send_sms_code.apply_async([encryption.encrypt(verification_message)], queue='sms-code') + + return jsonify({}), 204 + + +@user.route('//email-code', methods=['POST']) +def send_user_email_code(user_id): + user_to_send_to = get_model_users(user_id=user_id) + + verify_code, errors = request_verify_code_schema.load(request.get_json()) + if errors: + return jsonify(result="error", message=errors), 400 + + from app.dao.users_dao import create_secret_code + secret_code = create_secret_code() + create_user_code(user_to_send_to, secret_code, 'email') + + email = user_to_send_to.email_address if verify_code.get('to', None) is None else verify_code.get('to') + verification_message = {'to': email, 'secret_code': secret_code} + + send_email_code.apply_async([encryption.encrypt(verification_message)], queue='email-code') + + return jsonify({}), 204 + + +# TODO: Remove this method once the admin app has stopped using it. +@user.route('//code', methods=['POST']) +def send_user_code(user_id): + user_to_send_to = get_model_users(user_id=user_id) + + verify_code, errors = old_request_verify_code_schema.load(request.get_json()) + if errors: + return jsonify(result="error", message=errors), 400 + + from app.dao.users_dao import create_secret_code + secret_code = create_secret_code() + create_user_code(user_to_send_to, secret_code, verify_code.get('code_type')) if verify_code.get('code_type') == 'sms': - mobile = user.mobile_number if verify_code.get('to', None) is None else verify_code.get('to') + mobile = user_to_send_to.mobile_number if verify_code.get('to', None) is None else verify_code.get('to') verification_message = {'to': mobile, 'secret_code': secret_code} send_sms_code.apply_async([encryption.encrypt(verification_message)], queue='sms-code') elif verify_code.get('code_type') == 'email': - email = user.email_address if verify_code.get('to', None) is None else verify_code.get('to') + email = user_to_send_to.email_address if verify_code.get('to', None) is None else verify_code.get('to') verification_message = { 'to_address': email, 'from_address': current_app.config['VERIFY_CODE_FROM_EMAIL_ADDRESS'], @@ -156,12 +179,8 @@ def send_user_code(user_id): @user.route('/', methods=['GET']) @user.route('', methods=['GET']) def get_user(user_id=None): - try: - users = get_model_users(user_id=user_id) - except DataError: - return jsonify(result="error", message="Invalid user id"), 400 - except NoResultFound: - return jsonify(result="error", message="User not found"), 404 + users = get_model_users(user_id=user_id) + result = users_schema.dump(users) if isinstance(users, list) else user_schema.dump(users) return jsonify(data=result.data) @@ -169,18 +188,9 @@ def get_user(user_id=None): @user.route('//service', methods=['GET']) @user.route('//service/', methods=['GET']) def get_service_by_user_id(user_id, service_id=None): - try: - user = get_model_users(user_id=user_id) - except DataError: - return jsonify(result="error", message="Invalid user id"), 400 - except NoResultFound: - return jsonify(result="error", message="User not found"), 404 + ret_user = get_model_users(user_id=user_id) + + services = get_model_services(user_id=ret_user.id, service_id=service_id) - try: - services = get_model_services(user_id=user.id, service_id=service_id) - except DataError: - return jsonify(result="error", message="Invalid service id"), 400 - except NoResultFound: - return jsonify(result="error", message="Service not found"), 404 services, errors = services_schema.dump(services) if isinstance(services, list) else service_schema.dump(services) return jsonify(data=services) diff --git a/tests/app/user/test_rest.py b/tests/app/user/test_rest.py index 008da0558..97f55f83b 100644 --- a/tests/app/user/test_rest.py +++ b/tests/app/user/test_rest.py @@ -261,7 +261,7 @@ def test_put_user_not_exists(notify_api, notify_db, notify_db_session, sample_us assert User.query.count() == 2 user = User.query.filter_by(id=sample_user.id).first() json_resp = json.loads(resp.get_data(as_text=True)) - assert json_resp == {'result': 'error', 'message': 'User not found'} + assert json_resp == {'error': 'No result found'} assert user == sample_user assert user.email_address != new_email @@ -318,8 +318,7 @@ def test_get_user_service(notify_api, notify_db, notify_db_session, sample_servi assert json_resp['data']['id'] == str(another_service.id) -def test_get_user_service_user_not_exists(notify_api, notify_db, notify_db_session, sample_service, - sample_admin_service_id): +def test_get_user_service_user_not_exists(notify_api, sample_service, sample_admin_service_id): """ Tests GET endpoint "//service/" 404 is returned for invalid user. """ @@ -330,28 +329,25 @@ def test_get_user_service_user_not_exists(notify_api, notify_db, notify_db_sessi path=url_for('user.get_service_by_user_id', user_id="123423", service_id=sample_service.id), method='GET') - print('** service users{}'.format(sample_service.users[0].id)) resp = client.get( url_for('user.get_service_by_user_id', user_id="123423", service_id=sample_service.id), headers=[('Content-Type', 'application/json'), auth_header]) assert resp.status_code == 404 json_resp = json.loads(resp.get_data(as_text=True)) - assert "User not found" in json_resp['message'] + assert json_resp == {'error': 'No result found'} -def test_get_user_service_service_not_exists(notify_api, notify_db, notify_db_session, sample_service, - sample_admin_service_id): +def test_get_user_service_service_not_exists(notify_api, sample_service): """ Tests GET endpoint "//service/" 404 is returned for invalid service. """ with notify_api.test_request_context(): with notify_api.test_client() as client: user = User.query.first() - assert Service.query.count() == 2 + assert Service.query.count() == 1 import uuid missing_service_id = uuid.uuid4() - auth_header = create_authorization_header(service_id=sample_admin_service_id, - path=url_for('user.get_service_by_user_id', user_id=user.id, + auth_header = create_authorization_header(path=url_for('user.get_service_by_user_id', user_id=user.id, service_id=missing_service_id), method='GET') resp = client.get( @@ -359,10 +355,10 @@ def test_get_user_service_service_not_exists(notify_api, notify_db, notify_db_se headers=[('Content-Type', 'application/json'), auth_header]) assert resp.status_code == 404 json_resp = json.loads(resp.get_data(as_text=True)) - assert "Service not found" in json_resp['message'] + assert json_resp == {'error': 'No result found'} -def test_delete_user(notify_api, notify_db, notify_db_session, sample_user, sample_admin_service_id): +def test_delete_user(notify_api, sample_user, sample_admin_service_id): """ Tests DELETE endpoint '/' delete user. """ @@ -407,3 +403,5 @@ def test_delete_user_not_exists(notify_api, notify_db, notify_db_session, sample headers=[('Content-Type', 'application/json'), auth_header]) assert resp.status_code == 404 assert User.query.count() == 2 + json_resp = json.loads(resp.get_data(as_text=True)) + assert json_resp == {'error': 'No result found'} diff --git a/tests/app/user/test_rest_verify.py b/tests/app/user/test_rest_verify.py index e6759f45b..694cc703c 100644 --- a/tests/app/user/test_rest_verify.py +++ b/tests/app/user/test_rest_verify.py @@ -2,9 +2,7 @@ import json import moto from datetime import (datetime, timedelta) from flask import url_for - from app.models import (VerifyCode) - import app.celery.tasks from app import db, encryption from tests import create_authorization_header @@ -191,7 +189,6 @@ def test_user_verify_password_valid_password_resets_failed_logins(notify_api, notify_db, notify_db_session, sample_user): - with notify_api.test_request_context(): with notify_api.test_client() as client: data = json.dumps({'password': 'bad password'}) @@ -249,9 +246,10 @@ def test_user_verify_password_missing_password(notify_api, assert 'Required field missing data' in json_resp['message']['password'] +# TODO: Remove this test once the admin app has stopped using it. def test_send_user_code_for_sms(notify_api, sample_sms_code, - mock_secret_code, + mock_encryption, mock_celery_send_sms_code): """ Tests POST endpoint '//code' successful sms @@ -269,35 +267,11 @@ def test_send_user_code_for_sms(notify_api, headers=[('Content-Type', 'application/json'), auth_header]) assert resp.status_code == 204 - encrpyted = encryption.encrypt({'to': sample_sms_code.user.mobile_number, 'secret_code': '11111'}) - app.celery.tasks.send_sms_code.apply_async.assert_called_once_with([encrpyted], queue='sms-code') - - -def test_send_user_code_for_sms_with_optional_to_field(notify_api, - sample_sms_code, - mock_secret_code, - mock_celery_send_sms_code): - """ - Tests POST endpoint '//code' successful sms with optional to field - """ - with notify_api.test_request_context(): - with notify_api.test_client() as client: - - data = json.dumps({'code_type': 'sms', 'to': '+441119876757'}) - auth_header = create_authorization_header( - path=url_for('user.send_user_code', user_id=sample_sms_code.user.id), - method='POST', - request_body=data) - resp = client.post( - url_for('user.send_user_code', user_id=sample_sms_code.user.id), - data=data, - headers=[('Content-Type', 'application/json'), auth_header]) - - assert resp.status_code == 204 - encrypted = encryption.encrypt({'to': '+441119876757', 'secret_code': '11111'}) - app.celery.tasks.send_sms_code.apply_async.assert_called_once_with([encrypted], queue='sms-code') + app.celery.tasks.send_sms_code.apply_async.assert_called_once_with(['something_encrypted'], + queue='sms-code') +# TODO: Remove this test once the admin app has stopped using it. def test_send_user_code_for_email(notify_api, sample_email_code, mock_secret_code, @@ -323,6 +297,7 @@ def test_send_user_code_for_email(notify_api, queue='email-code') +# TODO: Remove this test once the admin app has stopped using it. def test_send_user_code_for_email_uses_optional_to_field(notify_api, sample_email_code, mock_secret_code, @@ -348,16 +323,130 @@ def test_send_user_code_for_email_uses_optional_to_field(notify_api, queue='email-code') -def test_request_verify_code_schema_invalid_code_type(notify_api, notify_db, notify_db_session, sample_user): - from app.schemas import request_verify_code_schema +# TODO: Remove this test once the admin app has stopped using it. +def test_request_verify_code_schema_invalid_code_type(notify_api, sample_user): + from app.schemas import old_request_verify_code_schema data = json.dumps({'code_type': 'not_sms'}) - code, error = request_verify_code_schema.loads(data) + code, error = old_request_verify_code_schema.loads(data) assert error == {'code_type': ['Invalid code type']} -def test_request_verify_code_schema_with_to(notify_api, notify_db, notify_db_session, sample_user): - from app.schemas import request_verify_code_schema +# TODO: Remove this method once the admin app has stopped using it. +def test_request_verify_code_schema_with_to(notify_api, sample_user): + from app.schemas import old_request_verify_code_schema data = json.dumps({'code_type': 'sms', 'to': 'some@one.gov.uk'}) - code, error = request_verify_code_schema.loads(data) + code, error = old_request_verify_code_schema.loads(data) assert code == {'code_type': 'sms', 'to': 'some@one.gov.uk'} assert error == {} + + +def test_send_user_sms_code(notify_api, + sample_sms_code, + mock_celery_send_sms_code, + mock_encryption): + """ + Tests POST endpoint /user//sms-code + """ + with notify_api.test_request_context(): + with notify_api.test_client() as client: + data = json.dumps({}) + auth_header = create_authorization_header( + path=url_for('user.send_user_sms_code', user_id=sample_sms_code.user.id), + method='POST', + request_body=data) + resp = client.post( + url_for('user.send_user_sms_code', user_id=sample_sms_code.user.id), + data=data, + headers=[('Content-Type', 'application/json'), auth_header]) + print(resp.get_data(as_text=True)) + assert resp.status_code == 204 + app.celery.tasks.send_sms_code.apply_async.assert_called_once_with(['something_encrypted'], + queue='sms-code') + + +def test_send_user_code_for_sms_with_optional_to_field(notify_api, + sample_sms_code, + mock_secret_code, + mock_celery_send_sms_code): + """ + Tests POST endpoint '//code' successful sms with optional to field + """ + with notify_api.test_request_context(): + with notify_api.test_client() as client: + data = json.dumps({'code_type': 'sms', 'to': '+441119876757'}) + auth_header = create_authorization_header( + path=url_for('user.send_user_sms_code', user_id=sample_sms_code.user.id), + method='POST', + request_body=data) + resp = client.post( + url_for('user.send_user_sms_code', user_id=sample_sms_code.user.id), + data=data, + headers=[('Content-Type', 'application/json'), auth_header]) + + assert resp.status_code == 204 + encrypted = encryption.encrypt({'to': '+441119876757', 'secret_code': '11111'}) + app.celery.tasks.send_sms_code.apply_async.assert_called_once_with([encrypted], queue='sms-code') + + +def test_send_sms_code_returns_404_for_bad_input_data(notify_api, notify_db, notify_db_session): + """ + Tests POST endpoint /user//sms-code return 404 for bad input data + """ + with notify_api.test_request_context(): + with notify_api.test_client() as client: + data = json.dumps({}) + import uuid + uuid_ = uuid.uuid4() + auth_header = create_authorization_header( + path=url_for('user.send_user_sms_code', user_id=uuid_), + method='POST', + request_body=data) + resp = client.post( + url_for('user.send_user_sms_code', user_id=uuid_), + data=data, + headers=[('Content-Type', 'application/json'), auth_header]) + assert resp.status_code == 404 + assert json.loads(resp.get_data(as_text=True))['error'] == 'No result found' + + +def test_send_user_email_code(notify_api, + sample_email_code, + mock_celery_send_email_code, + mock_encryption): + """ + Tests POST endpoint /user//email-code + """ + with notify_api.test_request_context(): + with notify_api.test_client() as client: + data = json.dumps({}) + auth_header = create_authorization_header( + path=url_for('user.send_user_email_code', user_id=sample_email_code.user.id), + method='POST', + request_body=data) + resp = client.post( + url_for('user.send_user_email_code', user_id=sample_email_code.user.id), + data=data, + headers=[('Content-Type', 'application/json'), auth_header]) + print(resp.get_data(as_text=True)) + assert resp.status_code == 204 + app.celery.tasks.send_email_code.apply_async.assert_called_once_with(['something_encrypted'], + queue='email-code') + + +def test_send_user_email_code_returns_404_for_when_user_does_not_exist(notify_api, notify_db, notify_db_session): + """ + Tests POST endpoint /user//email-code return 404 for missing user + """ + with notify_api.test_request_context(): + with notify_api.test_client() as client: + data = json.dumps({}) + auth_header = create_authorization_header( + path=url_for('user.send_user_email_code', user_id=1), + method='POST', + request_body=data) + resp = client.post( + url_for('user.send_user_email_code', user_id=1), + data=data, + headers=[('Content-Type', 'application/json'), auth_header]) + assert resp.status_code == 404 + assert json.loads(resp.get_data(as_text=True))['error'] == 'No result found'