diff --git a/app/authentication/auth.py b/app/authentication/auth.py index 357151fba..208d9117e 100644 --- a/app/authentication/auth.py +++ b/app/authentication/auth.py @@ -16,10 +16,15 @@ class AuthError(Exception): self.code = code def to_dict_v2(self): - return {'code': self.code, - 'message': self.short_message, - 'fields': self.message, - 'link': 'link to docs'} + return { + 'status_code': self.code, + "errors": [ + { + "error": "AuthError", + "message": self.short_message + } + ] + } def get_auth_token(req): @@ -48,10 +53,19 @@ def requires_auth(): return handle_admin_key(auth_token, current_app.config.get('ADMIN_CLIENT_SECRET')) try: - api_keys = get_model_api_keys(client) + service = dao_fetch_service_by_id(client) except DataError: raise AuthError("Invalid token: service id is not the right data type", 403) - for api_key in api_keys: + except NoResultFound: + raise AuthError("Invalid token: service not found", 403) + + if not service.api_keys: + raise AuthError("Invalid token: service has no API keys", 403) + + if not service.active: + raise AuthError("Invalid token: service is archived", 403) + + for api_key in service.api_keys: try: get_decode_errors(auth_token, api_key.unsigned_secret) except TokenDecodeError: @@ -62,15 +76,8 @@ def requires_auth(): _request_ctx_stack.top.api_user = api_key return - - try: - dao_fetch_service_by_id(client) - except NoResultFound: - raise AuthError("Invalid token: service not found", 403) - - if not api_keys: - raise AuthError("Invalid token: service has no API keys", 403) else: + # service has API keys, but none matching the one the user provided raise AuthError("Invalid token: signature, api token is not valid", 403) diff --git a/app/celery/tasks.py b/app/celery/tasks.py index 7e29acc51..75109fd85 100644 --- a/app/celery/tasks.py +++ b/app/celery/tasks.py @@ -62,10 +62,7 @@ def process_job(job_id): 'job': str(job.id), 'to': recipient, 'row_number': row_number, - 'personalisation': { - key: personalisation.get(key) - for key in template.placeholders - } + 'personalisation': dict(personalisation) }) if template.template_type == SMS_TYPE: diff --git a/app/dao/dao_utils.py b/app/dao/dao_utils.py index bdaafc074..96628be06 100644 --- a/app/dao/dao_utils.py +++ b/app/dao/dao_utils.py @@ -9,7 +9,6 @@ def transactional(func): @wraps(func) def commit_or_rollback(*args, **kwargs): from flask import current_app - from app import db try: res = func(*args, **kwargs) db.session.commit() @@ -27,7 +26,6 @@ def version_class(model_class, history_cls=None): def versioned(func): @wraps(func) def record_version(*args, **kwargs): - from app import db func(*args, **kwargs) history_objects = [create_hist(obj) for obj in itertools.chain(db.session.new, db.session.dirty) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index c89886f1a..bead23a6e 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -1,5 +1,5 @@ import uuid -from datetime import date +from datetime import date, datetime from sqlalchemy import asc, func from sqlalchemy.orm import joinedload @@ -28,30 +28,68 @@ from app.models import ( from app.statsd_decorators import statsd -def dao_fetch_all_services(): - return Service.query.order_by( +def dao_fetch_all_services(only_active=False): + query = Service.query.order_by( asc(Service.created_at) ).options( joinedload('users') - ).all() + ) + + if only_active: + query = query.filter(Service.active) + + return query.all() -def dao_fetch_service_by_id(service_id): - return Service.query.filter_by( +def dao_fetch_service_by_id(service_id, only_active=False): + query = Service.query.filter_by( id=service_id ).options( joinedload('users') - ).one() + ) + + if only_active: + query = query.filter(Service.active) + + return query.one() -def dao_fetch_all_services_by_user(user_id): - return Service.query.filter( +def dao_fetch_all_services_by_user(user_id, only_active=False): + query = Service.query.filter( Service.users.any(id=user_id) ).order_by( asc(Service.created_at) ).options( joinedload('users') - ).all() + ) + + if only_active: + query = query.filter(Service.active) + + return query.all() + + +@transactional +@version_class(Service) +@version_class(Template, TemplateHistory) +@version_class(ApiKey) +def dao_deactive_service(service_id): + # have to eager load templates and api keys so that we don't flush when we loop through them + # to ensure that db.session still contains the models when it comes to creating history objects + service = Service.query.options( + joinedload('templates'), + joinedload('api_keys'), + ).filter(Service.id == service_id).one() + + service.active = False + service.name = '_archived_' + service.name + service.email_from = '_archived_' + service.email_from + + for template in service.templates: + template.archived = True + + for api_key in service.api_keys: + api_key.expiry_date = datetime.utcnow() def dao_fetch_service_by_id_and_user(service_id, user_id): @@ -70,6 +108,7 @@ def dao_create_service(service, user): service.users.append(user) permission_dao.add_default_service_permissions_for_user(user, service) service.id = uuid.uuid4() # must be set now so version history model can use same id + service.active = True service.research_mode = False db.session.add(service) diff --git a/app/dao/users_dao.py b/app/dao/users_dao.py index 1a240fecc..6654234a5 100644 --- a/app/dao/users_dao.py +++ b/app/dao/users_dao.py @@ -7,18 +7,26 @@ from app import db from app.models import (User, VerifyCode) +def _remove_values_for_keys_if_present(dict, keys): + for key in keys: + dict.pop(key, None) + + def create_secret_code(): return ''.join(map(str, random.sample(range(9), 5))) +def save_user_attribute(usr, update_dict={}): + db.session.query(User).filter_by(id=usr.id).update(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() if update_dict: - if update_dict.get('id'): - del update_dict['id'] - update_dict.pop('password_changed_at') + _remove_values_for_keys_if_present(update_dict, ['id', 'password_changed_at']) db.session.query(User).filter_by(id=usr.id).update(update_dict) else: db.session.add(usr) @@ -74,7 +82,7 @@ def delete_user_verify_codes(user): db.session.commit() -def get_model_users(user_id=None): +def get_user_by_id(user_id=None): if user_id: return User.query.filter_by(id=user_id).one() return User.query.filter_by().all() diff --git a/app/errors.py b/app/errors.py index 8b027ba60..546f75f0c 100644 --- a/app/errors.py +++ b/app/errors.py @@ -10,7 +10,6 @@ from app.authentication.auth import AuthError class InvalidRequest(Exception): code = None - link = None fields = [] def __init__(self, message, status_code): @@ -26,10 +25,13 @@ class InvalidRequest(Exception): Version 2 of the public api error response. ''' return { - "code": self.code, - "message": self.message, - "link": self.link, - "fields": self.fields + "status_code": self.status_code, + "errors": [ + { + "error": self.__class__.__name__, + "message": self.message + } + ] } def __str__(self): diff --git a/app/models.py b/app/models.py index fadd057d3..5fa8f6a98 100644 --- a/app/models.py +++ b/app/models.py @@ -115,7 +115,7 @@ class Service(db.Model, Versioned): unique=False, nullable=True, onupdate=datetime.datetime.utcnow) - active = db.Column(db.Boolean, index=False, unique=False, nullable=False) + active = db.Column(db.Boolean, index=False, unique=False, nullable=False, default=True) message_limit = db.Column(db.BigInteger, index=False, unique=False, nullable=False) users = db.relationship( 'User', @@ -188,7 +188,7 @@ class ApiKey(db.Model, Versioned): name = db.Column(db.String(255), nullable=False) secret = db.Column(db.String(255), unique=True, nullable=False) service_id = db.Column(UUID(as_uuid=True), db.ForeignKey('services.id'), index=True, nullable=False) - service = db.relationship('Service', backref=db.backref('api_keys', lazy='dynamic')) + service = db.relationship('Service', backref='api_keys') key_type = db.Column(db.String(255), db.ForeignKey('key_types.name'), index=True, nullable=False) expiry_date = db.Column(db.DateTime) created_at = db.Column( @@ -275,7 +275,7 @@ class Template(db.Model): content = db.Column(db.Text, index=False, unique=False, nullable=False) archived = db.Column(db.Boolean, index=False, nullable=False, default=False) service_id = db.Column(UUID(as_uuid=True), db.ForeignKey('services.id'), index=True, unique=False, nullable=False) - service = db.relationship('Service', backref=db.backref('templates', lazy='dynamic')) + service = db.relationship('Service', backref='templates') subject = db.Column(db.Text, index=False, unique=False, nullable=True) created_by_id = db.Column(UUID(as_uuid=True), db.ForeignKey('users.id'), index=True, nullable=False) created_by = db.relationship('User') diff --git a/app/schema_validation/__init__.py b/app/schema_validation/__init__.py index 38837b467..71dd37c18 100644 --- a/app/schema_validation/__init__.py +++ b/app/schema_validation/__init__.py @@ -1,4 +1,5 @@ import json + from jsonschema import Draft4Validator, ValidationError @@ -16,13 +17,11 @@ def build_error_message(errors, schema): field = "'{}' {}".format(e.path[0], e.schema.get('validationMessage')) if e.schema.get( 'validationMessage') else e.message s = field.split("'") - field = {s[1]: s[2].strip()} + field = {"error": "ValidationError", "message": "{}{}".format(s[1], s[2])} fields.append(field) message = { - "code": "1001", - "message": "Validation error occurred - {}".format(schema['title']), - "link": "link to error documentation (not yet implemented)", - "fields": fields + "status_code": 400, + "errors": fields } return json.dumps(message) diff --git a/app/schema_validation/definitions.py b/app/schema_validation/definitions.py index e65566d7b..3841b64a8 100644 --- a/app/schema_validation/definitions.py +++ b/app/schema_validation/definitions.py @@ -6,7 +6,7 @@ If the definition is specific to a version put it in a definition file in the ve uuid = { "type": "string", "pattern": "^[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}$", - "validationMessage": "not a valid UUID", + "validationMessage": "is not a valid UUID", "code": "1001", # yet to be implemented "link": "link to our error documentation not yet implemented" } diff --git a/app/schemas.py b/app/schemas.py index 6f5934114..7f52b9541 100644 --- a/app/schemas.py +++ b/app/schemas.py @@ -105,6 +105,42 @@ class UserSchema(BaseSchema): strict = True +class UserUpdateAttributeSchema(BaseSchema): + + class Meta: + model = models.User + exclude = ( + 'id', 'updated_at', 'created_at', 'user_to_service', + '_password', 'verify_codes', 'logged_in_at', 'password_changed_at', + 'failed_login_count', 'state', 'platform_admin') + strict = True + + @validates('name') + def validate_name(self, value): + if not value: + raise ValidationError('Invalid name') + + @validates('email_address') + def validate_email_address(self, value): + try: + validate_email_address(value) + except InvalidEmailError as e: + raise ValidationError(e.message) + + @validates('mobile_number') + def validate_mobile_number(self, value): + try: + validate_phone_number(value) + except InvalidPhoneError as error: + raise ValidationError('Invalid phone number: {}'.format(error)) + + @validates_schema(pass_original=True) + def check_unknown_fields(self, data, original_data): + for key in original_data: + if key not in self.fields: + raise ValidationError('Unknown field name {}'.format(key)) + + class ProviderDetailsSchema(BaseSchema): class Meta: model = models.ProviderDetails @@ -529,6 +565,7 @@ class UnarchivedTemplateSchema(BaseSchema): user_schema = UserSchema() user_schema_load_json = UserSchema(load_json=True) +user_update_schema_load_json = UserUpdateAttributeSchema(load_json=True, partial=True) service_schema = ServiceSchema() service_schema_load_json = ServiceSchema(load_json=True) detailed_service_schema = DetailedServiceSchema() diff --git a/app/service/rest.py b/app/service/rest.py index 2cc7dab90..75c68fe1f 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -25,7 +25,8 @@ from app.dao.services_dao import ( dao_fetch_stats_for_service, dao_fetch_todays_stats_for_service, dao_fetch_weekly_historical_stats_for_service, - dao_fetch_todays_stats_for_all_services + dao_fetch_todays_stats_for_all_services, + dao_deactive_service ) from app.dao.service_whitelist_dao import ( dao_fetch_service_whitelist, @@ -34,7 +35,7 @@ from app.dao.service_whitelist_dao import ( ) from app.dao import notifications_dao from app.dao.provider_statistics_dao import get_fragment_count -from app.dao.users_dao import get_model_users +from app.dao.users_dao import get_user_by_id from app.errors import ( register_errors, InvalidRequest @@ -58,13 +59,16 @@ register_errors(service_blueprint) @service_blueprint.route('', methods=['GET']) def get_services(): + only_active = request.args.get('only_active') == 'True' + detailed = request.args.get('detailed') == 'True' user_id = request.args.get('user_id', None) + if user_id: - services = dao_fetch_all_services_by_user(user_id) - elif request.args.get('detailed') == 'True': - return jsonify(data=get_detailed_services()) + services = dao_fetch_all_services_by_user(user_id, only_active) + elif detailed: + return jsonify(data=get_detailed_services(only_active)) else: - services = dao_fetch_all_services() + services = dao_fetch_all_services(only_active) data = service_schema.dump(services, many=True).data return jsonify(data=data) @@ -88,7 +92,7 @@ def create_service(): errors = {'user_id': ['Missing data for required field.']} raise InvalidRequest(errors, status_code=400) - user = get_model_users(data['user_id']) + user = get_user_by_id(data['user_id']) data.pop('user_id', None) valid_service = service_schema.load(request.get_json()).data dao_create_service(valid_service, user) @@ -148,7 +152,7 @@ def get_users_for_service(service_id): @service_blueprint.route('//users/', methods=['POST']) def add_user_to_service(service_id, user_id): service = dao_fetch_service_by_id(service_id) - user = get_model_users(user_id=user_id) + user = get_user_by_id(user_id=user_id) if user in service.users: error = 'User id: {} already part of service id: {}'.format(user_id, service_id) @@ -163,7 +167,7 @@ def add_user_to_service(service_id, user_id): @service_blueprint.route('//users/', methods=['DELETE']) def remove_user_from_service(service_id, user_id): service = dao_fetch_service_by_id(service_id) - user = get_model_users(user_id=user_id) + user = get_user_by_id(user_id=user_id) if user not in service.users: error = 'User not found' raise InvalidRequest(error, status_code=404) @@ -264,8 +268,8 @@ def get_detailed_service(service_id, today_only=False): return detailed_service_schema.dump(service).data -def get_detailed_services(): - services = {service.id: service for service in dao_fetch_all_services()} +def get_detailed_services(only_active=False): + services = {service.id: service for service in dao_fetch_all_services(only_active)} stats = dao_fetch_todays_stats_for_all_services() for service_id, rows in itertools.groupby(stats, lambda x: x.service_id): @@ -312,6 +316,19 @@ def update_whitelist(service_id): return '', 204 +@service_blueprint.route('//deactivate', methods=['POST']) +def deactivate_service(service_id): + service = dao_fetch_service_by_id(service_id) + + if not service.active: + # assume already inactive, don't change service name + return '', 204 + + dao_deactive_service(service.id) + + return '', 204 + + @service_blueprint.route('//billable-units') def get_billable_unit_count(service_id): try: diff --git a/app/user/rest.py b/app/user/rest.py index 2e6e6b021..a188f162e 100644 --- a/app/user/rest.py +++ b/app/user/rest.py @@ -4,7 +4,7 @@ from datetime import datetime from flask import (jsonify, request, Blueprint, current_app) from app import encryption, DATETIME_FORMAT from app.dao.users_dao import ( - get_model_users, + get_user_by_id, save_model_user, create_user_code, get_user_code, @@ -12,7 +12,8 @@ from app.dao.users_dao import ( increment_failed_login_count, reset_failed_login_count, get_user_by_email, - create_secret_code + create_secret_code, + save_user_attribute ) from app.dao.permissions_dao import permission_dao from app.dao.services_dao import dao_fetch_service_by_id @@ -22,8 +23,10 @@ from app.schemas import ( email_data_request_schema, user_schema, request_verify_code_schema, + permission_schema, user_schema_load_json, - permission_schema) + user_update_schema_load_json +) from app.celery.tasks import ( send_sms, @@ -53,7 +56,7 @@ def create_user(): @user.route('/', methods=['PUT']) def update_user(user_id): - user_to_update = get_model_users(user_id=user_id) + user_to_update = get_user_by_id(user_id=user_id) req_json = request.get_json() update_dct, errors = user_schema_load_json.load(req_json) pwd = req_json.get('password', None) @@ -66,9 +69,20 @@ def update_user(user_id): return jsonify(data=user_schema.dump(user_to_update).data), 200 +@user.route('/', methods=['POST']) +def update_user_attribute(user_id): + user_to_update = get_user_by_id(user_id=user_id) + req_json = request.get_json() + update_dct, errors = user_update_schema_load_json.load(req_json) + if errors: + raise InvalidRequest(errors, status_code=400) + save_user_attribute(user_to_update, update_dict=update_dct) + return jsonify(data=user_schema.dump(user_to_update).data), 200 + + @user.route('//verify/password', methods=['POST']) def verify_user_password(user_id): - user_to_verify = get_model_users(user_id=user_id) + user_to_verify = get_user_by_id(user_id=user_id) txt_pwd = None try: @@ -92,7 +106,7 @@ def verify_user_password(user_id): @user.route('//verify/code', methods=['POST']) def verify_user_code(user_id): - user_to_verify = get_model_users(user_id=user_id) + user_to_verify = get_user_by_id(user_id=user_id) txt_code = None resp_json = request.get_json() @@ -120,7 +134,7 @@ def verify_user_code(user_id): @user.route('//sms-code', methods=['POST']) def send_user_sms_code(user_id): - user_to_send_to = get_model_users(user_id=user_id) + user_to_send_to = get_user_by_id(user_id=user_id) verify_code, errors = request_verify_code_schema.load(request.get_json()) secret_code = create_secret_code() @@ -149,7 +163,7 @@ def send_user_sms_code(user_id): @user.route('//change-email-verification', methods=['POST']) def send_user_confirm_new_email(user_id): - user_to_send_to = get_model_users(user_id=user_id) + user_to_send_to = get_user_by_id(user_id=user_id) email, errors = email_data_request_schema.load(request.get_json()) if errors: raise InvalidRequest(message=errors, status_code=400) @@ -178,7 +192,7 @@ def send_user_confirm_new_email(user_id): @user.route('//email-verification', methods=['POST']) def send_user_email_verification(user_id): - user_to_send_to = get_model_users(user_id=user_id) + user_to_send_to = get_user_by_id(user_id=user_id) secret_code = create_secret_code() create_user_code(user_to_send_to, secret_code, 'email') @@ -230,7 +244,7 @@ def send_already_registered_email(user_id): @user.route('/', methods=['GET']) @user.route('', methods=['GET']) def get_user(user_id=None): - users = get_model_users(user_id=user_id) + users = get_user_by_id(user_id=user_id) result = user_schema.dump(users, many=True) if isinstance(users, list) else user_schema.dump(users) return jsonify(data=result.data) @@ -239,7 +253,7 @@ def get_user(user_id=None): def set_permissions(user_id, service_id): # TODO fix security hole, how do we verify that the user # who is making this request has permission to make the request. - user = get_model_users(user_id=user_id) + user = get_user_by_id(user_id=user_id) service = dao_fetch_service_by_id(service_id=service_id) permissions, errors = permission_schema.load(request.get_json(), many=True) diff --git a/app/v2/errors.py b/app/v2/errors.py index b7bd5f586..26cc3b36e 100644 --- a/app/v2/errors.py +++ b/app/v2/errors.py @@ -1,19 +1,14 @@ import json - from flask import jsonify, current_app from jsonschema import ValidationError from sqlalchemy.exc import DataError from sqlalchemy.orm.exc import NoResultFound - from app.authentication.auth import AuthError from app.errors import InvalidRequest class TooManyRequestsError(InvalidRequest): status_code = 429 - # code and link will be in a static file - code = "10429" - link = "link to docs" message_template = 'Exceeded send limits ({}) for today' def __init__(self, sending_limit): @@ -22,8 +17,6 @@ class TooManyRequestsError(InvalidRequest): class BadRequestError(InvalidRequest): status_code = 400 - code = 10400 - link = "link to documentation" message = "An error occurred" def __init__(self, fields=[], message=None): @@ -47,7 +40,8 @@ def register_errors(blueprint): @blueprint.errorhandler(DataError) def no_result_found(e): current_app.logger.exception(e) - return jsonify(message="No result found"), 404 + return jsonify(status_code=404, + errors=[{"error": e.__class__.__name__, "message": "No result found"}]), 404 @blueprint.errorhandler(AuthError) def auth_error(error): @@ -56,4 +50,5 @@ def register_errors(blueprint): @blueprint.errorhandler(Exception) def internal_server_error(error): current_app.logger.exception(error) - return jsonify(message='Internal server error'), 500 + return jsonify(status_code=500, + errors=[{"error": error.__class__.__name__, "message": 'Internal server error'}]), 500 diff --git a/migrations/versions/0059_set_services_to_active.py b/migrations/versions/0059_set_services_to_active.py new file mode 100644 index 000000000..f402d1a70 --- /dev/null +++ b/migrations/versions/0059_set_services_to_active.py @@ -0,0 +1,23 @@ +""" +we weren't previously using the services.active column , and by default it was set to false. lets set all services to +active, so that in the future we can turn it off to signify deactivating a service + +Revision ID: 0059_set_services_to_active +Revises: 0058_add_letters_flag +Create Date: 2016-10-31 15:17:16.716450 + +""" + +# revision identifiers, used by Alembic. +revision = '0059_set_services_to_active' +down_revision = '0058_add_letters_flag' + +from alembic import op + + +def upgrade(): + op.execute('UPDATE services SET active = TRUE') + + +def downgrade(): + op.execute('UPDATE services SET active = FALSE') diff --git a/migrations/versions/0060_add_letter_template_type.py b/migrations/versions/0060_add_letter_template_type.py new file mode 100644 index 000000000..cf5057de4 --- /dev/null +++ b/migrations/versions/0060_add_letter_template_type.py @@ -0,0 +1,56 @@ +"""empty message + +Revision ID: f266fb67597a +Revises: 0059_set_services_to_active +Create Date: 2016-11-07 16:13:18.961527 + +""" + +from alembic import op +import sqlalchemy as sa + +# revision identifiers, used by Alembic. +revision = '0060_add_letter_template_type' +down_revision = '0059_set_services_to_active' + + +name = 'template_type' +tmp_name = 'tmp_' + name + +old_options = ('sms', 'email') +new_options = old_options + ('letter',) + +new_type = sa.Enum(*new_options, name=name) +old_type = sa.Enum(*old_options, name=name) + +tcr = sa.sql.table( + 'templates', + sa.Column('template_type', new_type, nullable=False) +) + + +def upgrade(): + op.execute('ALTER TYPE ' + name + ' RENAME TO ' + tmp_name) + + new_type.create(op.get_bind()) + op.execute( + 'ALTER TABLE templates ALTER COLUMN template_type ' + + 'TYPE ' + name + ' USING template_type::text::' + name + ) + op.execute('DROP TYPE ' + tmp_name) + + +def downgrade(): + # Convert 'letter' template into 'email' + op.execute( + tcr.update().where(tcr.c.template_type=='letter').values(template_type='email') + ) + + op.execute('ALTER TYPE ' + name + ' RENAME TO ' + tmp_name) + + old_type.create(op.get_bind()) + op.execute( + 'ALTER TABLE templates ALTER COLUMN template_type ' + + 'TYPE ' + name + ' USING template_type::text::' + name + ) + op.execute('DROP TYPE ' + tmp_name) diff --git a/requirements.txt b/requirements.txt index 594f12317..d6166c2d5 100644 --- a/requirements.txt +++ b/requirements.txt @@ -21,6 +21,6 @@ jsonschema==2.5.1 git+https://github.com/alphagov/notifications-python-client.git@2.0.0#egg=notifications-python-client==2.0.0 -git+https://github.com/alphagov/notifications-utils.git@9.1.1#egg=notifications-utils==9.1.1 +git+https://github.com/alphagov/notifications-utils.git@9.2.1#egg=notifications-utils==9.2.1 git+https://github.com/alphagov/boto.git@2.43.0-patch3#egg=boto==2.43.0-patch3 diff --git a/tests/__init__.py b/tests/__init__.py index 526e0efc1..a40d209cf 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -30,3 +30,12 @@ def create_authorization_header(service_id=None, key_type=KEY_TYPE_NORMAL): token = create_jwt_token(secret=secret, client_id=client_id) return 'Authorization', 'Bearer {}'.format(token) + + +def unwrap_function(fn): + """ + Given a function, returns its undecorated original. + """ + while hasattr(fn, '__wrapped__'): + fn = fn.__wrapped__ + return fn diff --git a/tests/app/authentication/test_authentication.py b/tests/app/authentication/test_authentication.py index be7217b07..b506c6409 100644 --- a/tests/app/authentication/test_authentication.py +++ b/tests/app/authentication/test_authentication.py @@ -266,6 +266,17 @@ def test_authentication_returns_error_when_service_doesnt_exit( assert error_message['message'] == {'token': ['Invalid token: service not found']} +def test_authentication_returns_error_when_service_inactive(client, sample_api_key): + sample_api_key.service.active = False + token = create_jwt_token(secret=str(sample_api_key.id), client_id=str(sample_api_key.service_id)) + + response = client.get('/service', headers={'Authorization': 'Bearer {}'.format(token)}) + + assert response.status_code == 403 + error_message = json.loads(response.get_data()) + assert error_message['message'] == {'token': ['Invalid token: service is archived']} + + def test_authentication_returns_error_when_service_has_no_secrets(notify_api, sample_service, fake_uuid): diff --git a/tests/app/conftest.py b/tests/app/conftest.py index 002c7c13c..435ea3bf5 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -129,6 +129,7 @@ def sample_service(notify_db, notify_db_session, service_name="Sample service", user=None, + active=True, restricted=False, limit=1000, email_from=None): @@ -139,7 +140,7 @@ def sample_service(notify_db, data = { 'name': service_name, 'message_limit': limit, - 'active': False, + 'active': active, 'restricted': restricted, 'email_from': email_from, 'created_by': user @@ -190,7 +191,8 @@ def sample_template(notify_db, @pytest.fixture(scope='function') def sample_template_with_placeholders(notify_db, notify_db_session): - return sample_template(notify_db, notify_db_session, content="Hello ((name))\nYour thing is due soon") + # deliberate space and title case in placeholder + return sample_template(notify_db, notify_db_session, content="Hello (( Name))\nYour thing is due soon") @pytest.fixture(scope='function') diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index a24f26c0b..6140231e4 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -58,7 +58,6 @@ def test_create_service(sample_user): service = Service(name="service_name", email_from="email_from", message_limit=1000, - active=True, restricted=False, created_by=sample_user) dao_create_service(service, sample_user) @@ -68,7 +67,8 @@ def test_create_service(sample_user): assert service_db.name == "service_name" assert service_db.id == service.id assert service_db.branding == BRANDING_GOVUK - assert not service_db.research_mode + assert service_db.research_mode is False + assert service.active is True assert sample_user in service_db.users @@ -77,14 +77,12 @@ def test_cannot_create_two_services_with_same_name(sample_user): service1 = Service(name="service_name", email_from="email_from1", message_limit=1000, - active=True, restricted=False, created_by=sample_user) service2 = Service(name="service_name", email_from="email_from2", message_limit=1000, - active=True, restricted=False, created_by=sample_user) with pytest.raises(IntegrityError) as excinfo: @@ -98,13 +96,11 @@ def test_cannot_create_two_services_with_same_email_from(sample_user): service1 = Service(name="service_name1", email_from="email_from", message_limit=1000, - active=True, restricted=False, created_by=sample_user) service2 = Service(name="service_name2", email_from="email_from", message_limit=1000, - active=True, restricted=False, created_by=sample_user) with pytest.raises(IntegrityError) as excinfo: @@ -118,7 +114,6 @@ def test_cannot_create_service_with_no_user(notify_db_session, sample_user): service = Service(name="service_name", email_from="email_from", message_limit=1000, - active=True, restricted=False, created_by=sample_user) with pytest.raises(FlushError) as excinfo: @@ -130,7 +125,6 @@ def test_should_add_user_to_service(sample_user): service = Service(name="service_name", email_from="email_from", message_limit=1000, - active=True, restricted=False, created_by=sample_user) dao_create_service(service, sample_user) @@ -150,7 +144,6 @@ def test_should_remove_user_from_service(sample_user): service = Service(name="service_name", email_from="email_from", message_limit=1000, - active=True, restricted=False, created_by=sample_user) dao_create_service(service, sample_user) @@ -244,7 +237,6 @@ def test_create_service_creates_a_history_record_with_current_data(sample_user): service = Service(name="service_name", email_from="email_from", message_limit=1000, - active=True, restricted=False, created_by=sample_user) dao_create_service(service, sample_user) @@ -270,7 +262,6 @@ def test_update_service_creates_a_history_record_with_current_data(sample_user): service = Service(name="service_name", email_from="email_from", message_limit=1000, - active=True, restricted=False, created_by=sample_user) dao_create_service(service, sample_user) @@ -299,7 +290,6 @@ def test_create_service_and_history_is_transactional(sample_user): service = Service(name=None, email_from="email_from", message_limit=1000, - active=True, restricted=False, created_by=sample_user) @@ -348,7 +338,6 @@ def test_add_existing_user_to_another_service_doesnot_change_old_permissions(sam service_one = Service(name="service_one", email_from="service_one", message_limit=1000, - active=True, restricted=False, created_by=sample_user) @@ -367,7 +356,6 @@ def test_add_existing_user_to_another_service_doesnot_change_old_permissions(sam service_two = Service(name="service_two", email_from="service_two", message_limit=1000, - active=True, restricted=False, created_by=other_user) dao_create_service(service_two, other_user) @@ -397,7 +385,6 @@ def test_fetch_stats_filters_on_service(sample_notification): service_two = Service(name="service_two", created_by=sample_notification.service.created_by, email_from="hello", - active=False, restricted=False, message_limit=1000) dao_create_service(service_two, sample_notification.service.created_by) diff --git a/tests/app/dao/test_users_dao.py b/tests/app/dao/test_users_dao.py index 7afcf72ce..cc814b160 100644 --- a/tests/app/dao/test_users_dao.py +++ b/tests/app/dao/test_users_dao.py @@ -7,12 +7,13 @@ import pytest from app.dao.users_dao import ( save_model_user, - get_model_users, + save_user_attribute, + get_user_by_id, delete_model_user, increment_failed_login_count, reset_failed_login_count, get_user_by_email, - delete_codes_older_created_more_than_a_day_ago + delete_codes_older_created_more_than_a_day_ago, ) from tests.app.conftest import sample_user as create_sample_user @@ -37,13 +38,13 @@ def test_create_user(notify_api, notify_db, notify_db_session): def test_get_all_users(notify_api, notify_db, notify_db_session, sample_user): assert User.query.count() == 1 - assert len(get_model_users()) == 1 + assert len(get_user_by_id()) == 1 email = "another.notify@digital.cabinet-office.gov.uk" another_user = create_sample_user(notify_db, notify_db_session, email=email) assert User.query.count() == 2 - assert len(get_model_users()) == 2 + assert len(get_user_by_id()) == 2 def test_get_user(notify_api, notify_db, notify_db_session): @@ -51,23 +52,20 @@ def test_get_user(notify_api, notify_db, notify_db_session): another_user = create_sample_user(notify_db, notify_db_session, email=email) - assert get_model_users(user_id=another_user.id).email_address == email + assert get_user_by_id(user_id=another_user.id).email_address == email def test_get_user_not_exists(notify_api, notify_db, notify_db_session, fake_uuid): try: - get_model_users(user_id=fake_uuid) + get_user_by_id(user_id=fake_uuid) pytest.fail("NoResultFound exception not thrown.") except NoResultFound as e: pass def test_get_user_invalid_id(notify_api, notify_db, notify_db_session): - try: - get_model_users(user_id="blah") - pytest.fail("DataError exception not thrown.") - except DataError: - pass + with pytest.raises(DataError): + get_user_by_id(user_id="blah") def test_delete_users(notify_api, notify_db, notify_db_session, sample_user): @@ -131,3 +129,17 @@ def make_verify_code(user, age=timedelta(hours=0), code="12335"): ) db.session.add(verify_code) db.session.commit() + + +@pytest.mark.parametrize('user_attribute, user_value', [ + ('name', 'New User'), + ('email_address', 'newuser@mail.com'), + ('mobile_number', '+4407700900460') +]) +def test_update_user_attribute(client, sample_user, user_attribute, user_value): + assert getattr(sample_user, user_attribute) != user_value + update_dict = { + user_attribute: user_value + } + save_user_attribute(sample_user, update_dict) + assert getattr(sample_user, user_attribute) == user_value diff --git a/tests/app/invite/test_invite_rest.py b/tests/app/invite/test_invite_rest.py index 301bf6e69..9cf9c2f13 100644 --- a/tests/app/invite/test_invite_rest.py +++ b/tests/app/invite/test_invite_rest.py @@ -85,7 +85,7 @@ def test_create_invited_user_invalid_email(notify_api, sample_service, mocker): assert response.status_code == 400 json_resp = json.loads(response.get_data(as_text=True)) assert json_resp['result'] == 'error' - assert json_resp['message'] == {'email_address': ['Not a valid email address']} + assert json_resp['message'] == {'email_address': ['Not a valid email address.']} app.celery.tasks.send_email.apply_async.assert_not_called() diff --git a/tests/app/notifications/rest/test_send_notification.py b/tests/app/notifications/rest/test_send_notification.py index 78c45b748..74b6fbdee 100644 --- a/tests/app/notifications/rest/test_send_notification.py +++ b/tests/app/notifications/rest/test_send_notification.py @@ -306,7 +306,7 @@ def test_should_reject_email_notification_with_bad_email(notify_api, sample_emai mocked.apply_async.assert_not_called() assert response.status_code == 400 assert data['result'] == 'error' - assert data['message']['to'][0] == 'Not a valid email address' + assert data['message']['to'][0] == 'Not a valid email address.' @freeze_time("2016-01-01 11:09:00.061258") @@ -914,7 +914,7 @@ def test_create_template_raises_invalid_request_exception_with_missing_personali from app.notifications.rest import create_template_object_for_notification with pytest.raises(InvalidRequest) as e: create_template_object_for_notification(template, {}) - assert {'template': ['Missing personalisation: name']} == e.value.message + assert {'template': ['Missing personalisation: Name']} == e.value.message def test_create_template_raises_invalid_request_exception_with_too_much_personalisation_data( diff --git a/tests/app/notifications/test_validators.py b/tests/app/notifications/test_validators.py index e41d5f81c..03781bcb8 100644 --- a/tests/app/notifications/test_validators.py +++ b/tests/app/notifications/test_validators.py @@ -28,7 +28,6 @@ def test_check_service_message_limit_over_message_limit_fails(key_type, notify_d with pytest.raises(TooManyRequestsError) as e: check_service_message_limit(key_type, service) assert e.value.status_code == 429 - assert e.value.code == '10429' assert e.value.message == 'Exceeded send limits (4) for today' assert e.value.fields == [] @@ -49,10 +48,9 @@ def test_check_template_is_for_notification_type_fails_when_template_type_does_n with pytest.raises(BadRequestError) as e: check_template_is_for_notification_type(notification_type=notification_type, template_type=template_type) - assert e.value.code == 10400 + assert e.value.status_code == 400 error_message = '{0} template is not suitable for {1} notification'.format(template_type, notification_type) assert e.value.message == error_message - assert e.value.link == 'link to documentation' assert e.value.fields == [{'template': error_message}] @@ -67,9 +65,7 @@ def test_check_template_is_active_fails(sample_template): with pytest.raises(BadRequestError) as e: check_template_is_active(sample_template) assert e.value.status_code == 400 - assert e.value.code == 10400 assert e.value.message == 'Template has been deleted' - assert e.value.link == "link to documentation" assert e.value.fields == [{'template': 'Template has been deleted'}] @@ -122,9 +118,7 @@ def test_service_can_send_to_recipient_fails_when_recipient_is_not_on_team(recip key_type, trial_mode_service) assert exec_info.value.status_code == 400 - assert exec_info.value.code == 10400 assert exec_info.value.message == error_message - assert exec_info.value.link == 'link to documentation' assert exec_info.value.fields == [] @@ -135,9 +129,7 @@ def test_service_can_send_to_recipient_fails_when_mobile_number_is_not_on_team(n 'team', live_service) assert e.value.status_code == 400 - assert e.value.code == 10400 assert e.value.message == 'Can’t send to this recipient using a team-only API key' - assert e.value.link == 'link to documentation' assert e.value.fields == [] @@ -151,8 +143,6 @@ def test_check_sms_content_char_count_fails(char_count, notify_api): with pytest.raises(BadRequestError) as e: check_sms_content_char_count(char_count) assert e.value.status_code == 400 - assert e.value.code == 10400 assert e.value.message == 'Content for template has a character count greater than the limit of {}'.format( notify_api.config['SMS_CHAR_COUNT_LIMIT']) - assert e.value.link == 'link to documentation' assert e.value.fields == [] diff --git a/tests/app/service/test_deactivate.py b/tests/app/service/test_deactivate.py new file mode 100644 index 000000000..0f9a3dcce --- /dev/null +++ b/tests/app/service/test_deactivate.py @@ -0,0 +1,89 @@ +import uuid +from unittest import mock + +import pytest + +from app import db +from app.models import Service, TemplateHistory, ApiKey +from app.dao.services_dao import dao_deactive_service + +from tests import create_authorization_header, unwrap_function +from tests.app.conftest import ( + sample_template as create_template, + sample_api_key as create_api_key +) + + +def test_deactivate_only_allows_post(client): + auth_header = create_authorization_header() + response = client.get('/service/{}/deactivate'.format(uuid.uuid4()), headers=[auth_header]) + assert response.status_code == 405 + + +def test_deactivate_service_errors_with_bad_service_id(client): + auth_header = create_authorization_header() + response = client.post('/service/{}/deactivate'.format(uuid.uuid4()), headers=[auth_header]) + assert response.status_code == 404 + + +def test_deactivating_inactive_service_does_nothing(client, sample_service): + auth_header = create_authorization_header() + sample_service.active = False + response = client.post('/service/{}/deactivate'.format(sample_service.id), headers=[auth_header]) + assert response.status_code == 204 + assert sample_service.name == 'Sample service' + + +@pytest.fixture +def deactivated_service(client, notify_db, notify_db_session, sample_service): + create_template(notify_db, notify_db_session, template_name='a') + create_template(notify_db, notify_db_session, template_name='b') + create_api_key(notify_db, notify_db_session) + create_api_key(notify_db, notify_db_session) + + auth_header = create_authorization_header() + response = client.post('/service/{}/deactivate'.format(sample_service.id), headers=[auth_header]) + assert response.status_code == 204 + assert response.data == b'' + return sample_service + + +def test_deactivating_service_changes_name_and_email(deactivated_service): + assert deactivated_service.name == '_archived_Sample service' + assert deactivated_service.email_from == '_archived_sample.service' + + +def test_deactivating_service_revokes_api_keys(deactivated_service): + assert len(deactivated_service.api_keys) == 2 + for key in deactivated_service.api_keys: + assert key.expiry_date is not None + assert key.version == 2 + + +def test_deactivating_service_archives_templates(deactivated_service): + assert len(deactivated_service.templates) == 2 + for template in deactivated_service.templates: + assert template.archived is True + assert template.version == 2 + + +def test_deactivating_service_creates_history(deactivated_service): + ServiceHistory = Service.get_history_model() + history = ServiceHistory.query.filter_by( + id=deactivated_service.id + ).order_by( + ServiceHistory.version.desc() + ).first() + + assert history.version == 2 + assert history.active is False + + +def test_deactivating_service_rolls_back_everything_on_error(sample_service, sample_api_key, sample_template): + unwrapped_deactive_service = unwrap_function(dao_deactive_service) + + unwrapped_deactive_service(sample_service.id) + + assert sample_service in db.session.dirty + assert sample_api_key in db.session.dirty + assert sample_template in db.session.dirty diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 3959e401d..3b3bdc744 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -11,8 +11,8 @@ from app.dao.services_dao import dao_remove_user_from_service from app.models import User, Organisation from tests import create_authorization_header from tests.app.conftest import ( - sample_service as create_sample_service, - sample_service_permission as create_sample_service_permission, + sample_service as create_service, + sample_service_permission as create_service_permission, sample_user as create_sample_user, sample_notification as create_sample_notification, sample_notification_with_job) @@ -22,9 +22,9 @@ from app.models import KEY_TYPE_TEST def test_get_service_list(notify_api, service_factory): with notify_api.test_request_context(): with notify_api.test_client() as client: - service_factory.get('one', email_from='one') - service_factory.get('two', email_from='two') - service_factory.get('three', email_from='three') + service_factory.get('one') + service_factory.get('two') + service_factory.get('three') auth_header = create_authorization_header() response = client.get( '/service', @@ -38,50 +38,84 @@ def test_get_service_list(notify_api, service_factory): assert json_resp['data'][2]['name'] == 'three' -def test_get_service_list_by_user(notify_api, sample_user, service_factory): +def test_get_service_list_with_only_active_flag(client, service_factory): + inactive = service_factory.get('one') + active = service_factory.get('two') - with notify_api.test_request_context(): - with notify_api.test_client() as client: - service_factory.get('one', sample_user, email_from='one') - service_factory.get('two', sample_user, email_from='two') - service_factory.get('three', sample_user, email_from='three') + inactive.active = False - auth_header = create_authorization_header() - response = client.get( - '/service?user_id='.format(sample_user.id), - headers=[auth_header] - ) - json_resp = json.loads(response.get_data(as_text=True)) - assert response.status_code == 200 - assert len(json_resp['data']) == 3 - assert json_resp['data'][0]['name'] == 'one' - assert json_resp['data'][1]['name'] == 'two' - assert json_resp['data'][2]['name'] == 'three' + auth_header = create_authorization_header() + response = client.get( + '/service?only_active=True', + headers=[auth_header] + ) + assert response.status_code == 200 + json_resp = json.loads(response.get_data(as_text=True)) + assert len(json_resp['data']) == 1 + assert json_resp['data'][0]['id'] == str(active.id) -def test_get_service_list_by_user_should_return_empty_list_if_no_services(notify_api, service_factory, sample_user): - with notify_api.test_request_context(): - with notify_api.test_client() as client: - new_user = User( - name='Test User', - email_address='new_user@digital.cabinet-office.gov.uk', - password='password', - mobile_number='+447700900986' - ) - save_model_user(new_user) +def test_get_service_list_with_user_id_and_only_active_flag( + notify_db, + notify_db_session, + client, + sample_user, + service_factory +): + other_user = create_sample_user(notify_db, notify_db_session, email='foo@bar.gov.uk') - service_factory.get('one', sample_user, email_from='one') - service_factory.get('two', sample_user, email_from='two') - service_factory.get('three', sample_user, email_from='three') + inactive = service_factory.get('one', user=sample_user) + active = service_factory.get('two', user=sample_user) + from_other_user = service_factory.get('three', user=other_user) - auth_header = create_authorization_header() - response = client.get( - '/service?user_id={}'.format(new_user.id), - headers=[auth_header] - ) - json_resp = json.loads(response.get_data(as_text=True)) - assert response.status_code == 200 - assert len(json_resp['data']) == 0 + inactive.active = False + + auth_header = create_authorization_header() + response = client.get( + '/service?user_id={}&only_active=True'.format(sample_user.id), + headers=[auth_header] + ) + assert response.status_code == 200 + json_resp = json.loads(response.get_data(as_text=True)) + assert len(json_resp['data']) == 1 + assert json_resp['data'][0]['id'] == str(active.id) + + +def test_get_service_list_by_user(notify_db, notify_db_session, client, sample_user, service_factory): + other_user = create_sample_user(notify_db, notify_db_session, email='foo@bar.gov.uk') + service_factory.get('one', sample_user) + service_factory.get('two', sample_user) + service_factory.get('three', other_user) + + auth_header = create_authorization_header() + response = client.get( + '/service?user_id={}'.format(sample_user.id), + headers=[auth_header] + ) + json_resp = json.loads(response.get_data(as_text=True)) + assert response.status_code == 200 + assert len(json_resp['data']) == 2 + assert json_resp['data'][0]['name'] == 'one' + assert json_resp['data'][1]['name'] == 'two' + + +def test_get_service_list_by_user_should_return_empty_list_if_no_services( + notify_db, + notify_db_session, + client, + sample_service +): + # service is already created by sample user + new_user = create_sample_user(notify_db, notify_db_session, email='foo@bar.gov.uk') + + auth_header = create_authorization_header() + response = client.get( + '/service?user_id={}'.format(new_user.id), + headers=[auth_header] + ) + json_resp = json.loads(response.get_data(as_text=True)) + assert response.status_code == 200 + assert len(json_resp['data']) == 0 def test_get_service_list_should_return_empty_list_if_no_services(notify_api, notify_db, notify_db_session): @@ -132,7 +166,7 @@ def test_get_service_by_id_should_404_if_no_service(notify_api, notify_db): def test_get_service_by_id_and_user(notify_api, service_factory, sample_user): with notify_api.test_request_context(): with notify_api.test_client() as client: - service = service_factory.get('new service', sample_user, email_from='new.service') + service = service_factory.get('new.service', sample_user) auth_header = create_authorization_header() resp = client.get( '/service/{}?user_id={}'.format(service.id, sample_user.id), @@ -285,7 +319,6 @@ def test_should_not_create_service_if_missing_data(notify_api, sample_user): assert resp.status_code == 400 assert json_resp['result'] == 'error' assert 'Missing data for required field.' in json_resp['message']['name'] - assert 'Missing data for required field.' in json_resp['message']['active'] assert 'Missing data for required field.' in json_resp['message']['message_limit'] assert 'Missing data for required field.' in json_resp['message']['restricted'] @@ -446,7 +479,7 @@ def test_should_not_update_service_with_duplicate_name(notify_api, with notify_api.test_request_context(): with notify_api.test_client() as client: service_name = "another name" - service = create_sample_service( + service = create_service( notify_db, notify_db_session, service_name=service_name, @@ -479,7 +512,7 @@ def test_should_not_update_service_with_duplicate_email_from(notify_api, with notify_api.test_client() as client: email_from = "duplicate.name" service_name = "duplicate name" - service = create_sample_service( + service = create_service( notify_db, notify_db_session, service_name=service_name, @@ -915,7 +948,7 @@ def test_remove_user_from_service(notify_api, notify_db, notify_db_session, samp notify_db_session, email="new@digital.cabinet-office.gov.uk") # Simulates successfully adding a user to the service - second_permission = create_sample_service_permission( + second_permission = create_service_permission( notify_db, notify_db_session, user=second_user) @@ -1019,8 +1052,8 @@ def test_set_reply_to_email_for_service(notify_api, sample_service): def test_get_all_notifications_for_service_in_order(notify_api, notify_db, notify_db_session): with notify_api.test_request_context(), notify_api.test_client() as client: - service_1 = create_sample_service(notify_db, notify_db_session, service_name="1", email_from='1') - service_2 = create_sample_service(notify_db, notify_db_session, service_name="2", email_from='2') + service_1 = create_service(notify_db, notify_db_session, service_name="1", email_from='1') + service_2 = create_service(notify_db, notify_db_session, service_name="2", email_from='2') create_sample_notification(notify_db, notify_db_session, service=service_2) @@ -1240,8 +1273,8 @@ def test_get_services_with_detailed_flag(notify_api, notify_db, notify_db_sessio def test_get_detailed_services_groups_by_service(notify_db, notify_db_session): from app.service.rest import get_detailed_services - service_1 = create_sample_service(notify_db, notify_db_session, service_name="1", email_from='1') - service_2 = create_sample_service(notify_db, notify_db_session, service_name="2", email_from='2') + service_1 = create_service(notify_db, notify_db_session, service_name="1", email_from='1') + service_2 = create_service(notify_db, notify_db_session, service_name="2", email_from='2') create_sample_notification(notify_db, notify_db_session, service=service_1, status='created') create_sample_notification(notify_db, notify_db_session, service=service_2, status='created') @@ -1267,8 +1300,8 @@ def test_get_detailed_services_groups_by_service(notify_db, notify_db_session): def test_get_detailed_services_includes_services_with_no_notifications(notify_db, notify_db_session): from app.service.rest import get_detailed_services - service_1 = create_sample_service(notify_db, notify_db_session, service_name="1", email_from='1') - service_2 = create_sample_service(notify_db, notify_db_session, service_name="2", email_from='2') + service_1 = create_service(notify_db, notify_db_session, service_name="1", email_from='1') + service_2 = create_service(notify_db, notify_db_session, service_name="2", email_from='2') create_sample_notification(notify_db, notify_db_session, service=service_1) diff --git a/tests/app/test_schemas.py b/tests/app/test_schemas.py index 4ba1c98b5..1d66f3290 100644 --- a/tests/app/test_schemas.py +++ b/tests/app/test_schemas.py @@ -1,3 +1,8 @@ +import pytest + +from marshmallow import ValidationError + + def test_job_schema_doesnt_return_notifications(sample_notification_with_job): from app.schemas import job_schema @@ -22,3 +27,51 @@ def test_notification_schema_adds_api_key_name(sample_notification_with_api_key) data = notification_with_template_schema.dump(sample_notification_with_api_key).data assert data['key_name'] == 'Test key' + + +@pytest.mark.parametrize('user_attribute, user_value', [ + ('name', 'New User'), + ('email_address', 'newuser@mail.com'), + ('mobile_number', '+4407700900460') +]) +def test_user_update_schema_accepts_valid_attribute_pairs(user_attribute, user_value): + update_dict = { + user_attribute: user_value + } + from app.schemas import user_update_schema_load_json + + data, errors = user_update_schema_load_json.load(update_dict) + assert not errors + + +@pytest.mark.parametrize('user_attribute, user_value', [ + ('name', None), + ('name', ''), + ('email_address', 'bademail@...com'), + ('mobile_number', '+44077009') +]) +def test_user_update_schema_rejects_invalid_attribute_pairs(user_attribute, user_value): + from app.schemas import user_update_schema_load_json + update_dict = { + user_attribute: user_value + } + + with pytest.raises(ValidationError): + data, errors = user_update_schema_load_json.load(update_dict) + + +@pytest.mark.parametrize('user_attribute', [ + 'id', 'updated_at', 'created_at', 'user_to_service', + '_password', 'verify_codes', 'logged_in_at', 'password_changed_at', + 'failed_login_count', 'state', 'platform_admin' +]) +def test_user_update_schema_rejects_disallowed_attribute_keys(user_attribute): + update_dict = { + user_attribute: 'not important' + } + from app.schemas import user_update_schema_load_json + + with pytest.raises(ValidationError) as excinfo: + data, errors = user_update_schema_load_json.load(update_dict) + + assert excinfo.value.messages['_schema'][0] == 'Unknown field name {}'.format(user_attribute) diff --git a/tests/app/user/test_rest.py b/tests/app/user/test_rest.py index 14b26d387..8c87893aa 100644 --- a/tests/app/user/test_rest.py +++ b/tests/app/user/test_rest.py @@ -1,4 +1,5 @@ import json +import pytest from flask import url_for, current_app from freezegun import freeze_time @@ -180,6 +181,29 @@ def test_put_user(notify_api, notify_db, notify_db_session, sample_service): assert sorted(expected_permissions) == sorted(fetched['permissions'][str(sample_service.id)]) +@pytest.mark.parametrize('user_attribute, user_value', [ + ('name', 'New User'), + ('email_address', 'newuser@mail.com'), + ('mobile_number', '+4407700900460') +]) +def test_post_user_attribute(client, sample_user, user_attribute, user_value): + assert getattr(sample_user, user_attribute) != user_value + update_dict = { + user_attribute: user_value + } + auth_header = create_authorization_header() + headers = [('Content-Type', 'application/json'), auth_header] + + resp = client.post( + url_for('user.update_user_attribute', user_id=sample_user.id), + data=json.dumps(update_dict), + headers=headers) + + assert resp.status_code == 200 + json_resp = json.loads(resp.get_data(as_text=True)) + assert json_resp['data'][user_attribute] == user_value + + def test_put_user_update_password(notify_api, notify_db, notify_db_session, @@ -470,7 +494,7 @@ def test_send_user_reset_password_should_return_400_when_data_is_not_email_addre headers=[('Content-Type', 'application/json'), auth_header]) assert resp.status_code == 400 - assert json.loads(resp.get_data(as_text=True))['message'] == {'email': ['Not a valid email address']} + assert json.loads(resp.get_data(as_text=True))['message'] == {'email': ['Not a valid email address.']} @freeze_time("2016-01-01 11:09:00.061258") diff --git a/tests/app/v2/notifications/test_notification_schemas.py b/tests/app/v2/notifications/test_notification_schemas.py index 96f9e1284..74c734c2f 100644 --- a/tests/app/v2/notifications/test_notification_schemas.py +++ b/tests/app/v2/notifications/test_notification_schemas.py @@ -20,7 +20,7 @@ valid_json_with_optionals = { @pytest.mark.parametrize("input", [valid_json, valid_json_with_optionals]) def test_post_sms_schema_is_valid(input): - validate(input, post_sms_request) + assert validate(input, post_sms_request) == input def test_post_sms_json_schema_bad_uuid_and_missing_phone_number(): @@ -28,12 +28,13 @@ def test_post_sms_json_schema_bad_uuid_and_missing_phone_number(): with pytest.raises(ValidationError) as e: validate(j, post_sms_request) error = json.loads(e.value.message) - assert "POST v2/notifications/sms" in error['message'] - assert len(error.get('fields')) == 2 - assert {"phone_number": "is a required property"} in error['fields'] - assert {"template_id": "not a valid UUID"} in error['fields'] - assert error.get('code') == '1001' - assert error.get('link', None) is not None + assert len(error.keys()) == 2 + assert error.get('status_code') == 400 + assert len(error.get('errors')) == 2 + assert {'error': 'ValidationError', + 'message': "phone_number is a required property"} in error['errors'] + assert {'error': 'ValidationError', + 'message': "template_id is not a valid UUID"} in error['errors'] def test_post_sms_schema_with_personalisation_that_is_not_a_dict(): @@ -46,11 +47,11 @@ def test_post_sms_schema_with_personalisation_that_is_not_a_dict(): with pytest.raises(ValidationError) as e: validate(j, post_sms_request) error = json.loads(e.value.message) - assert "POST v2/notifications/sms" in error['message'] - assert len(error.get('fields')) == 1 - assert error['fields'][0] == {"personalisation": "should contain key value pairs"} - assert error.get('code') == '1001' - assert error.get('link', None) == 'link to error documentation (not yet implemented)' + assert len(error.get('errors')) == 1 + assert error['errors'] == [{'error': 'ValidationError', + 'message': "personalisation should contain key value pairs"}] + assert error.get('status_code') == 400 + assert len(error.keys()) == 2 valid_response = { @@ -77,7 +78,7 @@ valid_response_with_optionals = { @pytest.mark.parametrize('input', [valid_response]) def test_post_sms_response_schema_is_valid(input): - validate(input, post_sms_response) + assert validate(input, post_sms_response) == input def test_post_sms_response_schema_missing_uri(): @@ -86,7 +87,6 @@ def test_post_sms_response_schema_missing_uri(): with pytest.raises(ValidationError) as e: validate(j, post_sms_response) error = json.loads(e.value.message) - assert '1001' == error['code'] - assert 'link to error documentation (not yet implemented)' == error['link'] - assert 'Validation error occurred - response v2/notifications/sms' == error['message'] - assert [{"uri": "is a required property"}] == error['fields'] + assert error['status_code'] == 400 + assert error['errors'] == [{'error': 'ValidationError', + 'message': "uri is a required property"}] diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index bbc07e309..c02e2d8e9 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -55,10 +55,9 @@ def test_post_sms_notification_returns_404_and_missing_template(notify_api, samp assert response.headers['Content-type'] == 'application/json' error_json = json.loads(response.get_data(as_text=True)) - assert error_json['code'] == 10400 - assert error_json['message'] == 'Template not found' - assert error_json['fields'] == [{'template': 'Template not found'}] - assert error_json['link'] == 'link to documentation' + assert error_json['status_code'] == 400 + assert error_json['errors'] == [{"error": "BadRequestError", + "message": 'Template not found'}] def test_post_sms_notification_returns_403_and_well_formed_auth_error(notify_api, sample_template): @@ -77,10 +76,9 @@ def test_post_sms_notification_returns_403_and_well_formed_auth_error(notify_api assert response.status_code == 401 assert response.headers['Content-type'] == 'application/json' error_resp = json.loads(response.get_data(as_text=True)) - assert error_resp['code'] == 401 - assert error_resp['message'] == 'Unauthorized, authentication token must be provided' - assert error_resp['fields'] == {'token': ['Unauthorized, authentication token must be provided']} - assert error_resp['link'] == 'link to docs' + assert error_resp['status_code'] == 401 + assert error_resp['errors'] == [{'error': "AuthError", + 'message': 'Unauthorized, authentication token must be provided'}] def test_post_sms_notification_returns_400_and_for_schema_problems(notify_api, sample_template): @@ -100,7 +98,7 @@ def test_post_sms_notification_returns_400_and_for_schema_problems(notify_api, s assert response.status_code == 400 assert response.headers['Content-type'] == 'application/json' error_resp = json.loads(response.get_data(as_text=True)) - assert error_resp['code'] == '1001' - assert error_resp['message'] == 'Validation error occurred - POST v2/notifications/sms' - assert error_resp['link'] == "link to error documentation (not yet implemented)" - assert error_resp['fields'] == [{"template_id": "is a required property"}] + assert error_resp['status_code'] == 400 + assert error_resp['errors'] == [{'error': 'ValidationError', + 'message': "template_id is a required property" + }] diff --git a/tests/app/v2/test_errors.py b/tests/app/v2/test_errors.py new file mode 100644 index 000000000..5f0ed0f81 --- /dev/null +++ b/tests/app/v2/test_errors.py @@ -0,0 +1,118 @@ +import json +import pytest +from flask import url_for +from sqlalchemy.exc import DataError + + +@pytest.fixture(scope='function') +def app_for_test(mocker): + import flask + from flask import Blueprint + from app.authentication.auth import AuthError + from app.v2.errors import BadRequestError, TooManyRequestsError + + app = flask.Flask(__name__) + app.config['TESTING'] = True + + from app.v2.errors import register_errors + blue = Blueprint("v2_under_test", __name__, url_prefix='/v2/under_test') + + @blue.route("/raise_auth_error", methods=["GET"]) + def raising_auth_error(): + raise AuthError("some message", 403) + + @blue.route("/raise_bad_request", methods=["GET"]) + def raising_bad_request(): + raise BadRequestError(message="you forgot the thing") + + @blue.route("/raise_too_many_requests", methods=["GET"]) + def raising_too_many_requests(): + raise TooManyRequestsError(sending_limit="452") + + @blue.route("/raise_validation_error", methods=["GET"]) + def raising_validation_error(): + from app.schema_validation import validate + from app.v2.notifications.notification_schemas import post_sms_request + validate({"template_id": "bad_uuid"}, post_sms_request) + + @blue.route("raise_data_error", methods=["GET"]) + def raising_data_error(): + raise DataError("There was a db problem", "params", "orig") + + @blue.route("raise_exception", methods=["GET"]) + def raising_exception(): + raise AssertionError("Raising any old exception") + + register_errors(blue) + app.register_blueprint(blue) + + return app + + +def test_auth_error(app_for_test): + with app_for_test.test_request_context(): + with app_for_test.test_client() as client: + response = client.get(url_for('v2_under_test.raising_auth_error')) + assert response.status_code == 403 + error = json.loads(response.get_data(as_text=True)) + assert error == {"status_code": 403, + "errors": [{"error": "AuthError", + "message": "some message"}]} + + +def test_bad_request_error(app_for_test): + with app_for_test.test_request_context(): + with app_for_test.test_client() as client: + response = client.get(url_for('v2_under_test.raising_bad_request')) + assert response.status_code == 400 + error = json.loads(response.get_data(as_text=True)) + assert error == {"status_code": 400, + "errors": [{"error": "BadRequestError", + "message": "you forgot the thing"}]} + + +def test_too_many_requests_error(app_for_test): + with app_for_test.test_request_context(): + with app_for_test.test_client() as client: + response = client.get(url_for('v2_under_test.raising_too_many_requests')) + assert response.status_code == 429 + error = json.loads(response.get_data(as_text=True)) + assert error == {"status_code": 429, + "errors": [{"error": "TooManyRequestsError", + "message": "Exceeded send limits (452) for today"}]} + + +def test_validation_error(app_for_test): + with app_for_test.test_request_context(): + with app_for_test.test_client() as client: + response = client.get(url_for('v2_under_test.raising_validation_error')) + assert response.status_code == 400 + error = json.loads(response.get_data(as_text=True)) + print(error) + assert len(error.keys()) == 2 + assert error['status_code'] == 400 + assert len(error['errors']) == 2 + assert {'error': 'ValidationError', + 'message': "phone_number is a required property"} in error['errors'] + assert {'error': 'ValidationError', + 'message': "template_id is not a valid UUID"} in error['errors'] + + +def test_data_errors(app_for_test): + with app_for_test.test_request_context(): + with app_for_test.test_client() as client: + response = client.get(url_for('v2_under_test.raising_data_error')) + assert response.status_code == 404 + error = json.loads(response.get_data(as_text=True)) + assert error == {"status_code": 404, + "errors": [{"error": "DataError", "message": "No result found"}]} + + +def test_internal_server_error_handler(app_for_test): + with app_for_test.test_request_context(): + with app_for_test.test_client() as client: + response = client.get(url_for("v2_under_test.raising_exception")) + assert response.status_code == 500 + error = json.loads(response.get_data(as_text=True)) + assert error == {"status_code": 500, + "errors": [{"error": "AssertionError", "message": "Internal server error"}]} diff --git a/tests/conftest.py b/tests/conftest.py index 36ad8c58b..17beb8a01 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -47,6 +47,7 @@ def client(notify_api): @pytest.fixture(scope='session') def notify_db(notify_api): + assert db.engine.url.database != 'notification_api', 'dont run tests against main db' Migrate(notify_api, db) Manager(db, MigrateCommand) BASE_DIR = os.path.dirname(os.path.dirname(__file__))