diff --git a/app/authentication/auth.py b/app/authentication/auth.py index 1b7a286ac..927fd197e 100644 --- a/app/authentication/auth.py +++ b/app/authentication/auth.py @@ -8,7 +8,13 @@ from sqlalchemy.exc import DataError from sqlalchemy.orm.exc import NoResultFound from gds_metrics import Histogram -from app.dao.services_dao import dao_fetch_service_by_id_with_api_keys +from app.dao.services_dao import dao_fetch_service_by_id +from app.dao.api_key_dao import get_model_api_keys +from app.serialised_models import ( + SerialisedAPIKey, + SerialisedAPIKeyCollection, + SerialisedService, +) GENERAL_TOKEN_ERROR_MESSAGE = 'Invalid token: make sure your API token matches the example at https://docs.notifications.service.gov.uk/rest-api.html#authorisation-header' # noqa @@ -86,6 +92,28 @@ def requires_admin_auth(): raise AuthError('Unauthorized: admin authentication token required', 401) +def get_service_dict(issuer): + from app.schemas import service_schema + with AUTH_DB_CONNECTION_DURATION_SECONDS.time(): + fetched = dao_fetch_service_by_id(issuer) + return service_schema.dump(fetched).data + + +def get_service_model(issuer): + return SerialisedService(get_service_dict(issuer)) + + +def get_api_keys_dict(issuer): + return [ + {k: getattr(key, k) for k in SerialisedAPIKey.ALLOWED_PROPERTIES} + for key in get_model_api_keys(issuer) + ] + + +def get_api_keys_models(issuer): + return SerialisedAPIKeyCollection(get_api_keys_dict(issuer)) + + def requires_auth(): request_helper.check_proxy_header_before_request() @@ -93,8 +121,8 @@ def requires_auth(): issuer = __get_token_issuer(auth_token) # ie the `iss` claim which should be a service ID try: - with AUTH_DB_CONNECTION_DURATION_SECONDS.time(): - service = dao_fetch_service_by_id_with_api_keys(issuer) + service = get_service_model(issuer) + service.api_keys = get_api_keys_models(issuer) except DataError: raise AuthError("Invalid token: service id is not the right data type", 403) except NoResultFound: @@ -129,7 +157,7 @@ def requires_auth(): if api_key.expiry_date: raise AuthError("Invalid token: API key revoked", 403, service_id=service.id, api_key_id=api_key.id) - g.service_id = api_key.service_id + g.service_id = service.id _request_ctx_stack.top.authenticated_service = service _request_ctx_stack.top.api_user = api_key diff --git a/app/notifications/process_notifications.py b/app/notifications/process_notifications.py index 826177bd3..d8a5c5b9a 100644 --- a/app/notifications/process_notifications.py +++ b/app/notifications/process_notifications.py @@ -122,7 +122,6 @@ def persist_notification( template_version=template_version, to=recipient, service_id=service.id, - service=service, personalisation=personalisation, notification_type=notification_type, api_key_id=api_key_id, diff --git a/app/notifications/rest.py b/app/notifications/rest.py index 1afe2fa72..05706f561 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -164,7 +164,7 @@ def _service_can_send_internationally(service, number): international_phone_info = get_international_phone_info(number) if international_phone_info.international and \ - INTERNATIONAL_SMS_TYPE not in [p.permission for p in service.permissions]: + INTERNATIONAL_SMS_TYPE not in service.permissions: raise InvalidRequest( {'to': ["Cannot send to international mobile numbers"]}, status_code=400 diff --git a/app/notifications/validators.py b/app/notifications/validators.py index ae33f06c0..40c14b86d 100644 --- a/app/notifications/validators.py +++ b/app/notifications/validators.py @@ -90,7 +90,7 @@ def service_can_send_to_recipient(send_to, key_type, service, allow_whitelisted_ def service_has_permission(notify_type, permissions): - return notify_type in [p.permission for p in permissions] + return notify_type in permissions def check_service_has_permission(notify_type, permissions): @@ -124,7 +124,7 @@ def validate_and_format_recipient(send_to, key_type, service, notification_type, international_phone_info = get_international_phone_info(send_to) if international_phone_info.international and \ - INTERNATIONAL_SMS_TYPE not in [p.permission for p in service.permissions]: + INTERNATIONAL_SMS_TYPE not in service.permissions: raise BadRequestError(message="Cannot send to international mobile numbers") return validate_and_format_phone_number( diff --git a/app/serialised_models.py b/app/serialised_models.py index beeea3b1a..37cf4e363 100644 --- a/app/serialised_models.py +++ b/app/serialised_models.py @@ -24,6 +24,29 @@ class SerialisedModel(ABC): return super().__dir__() + list(sorted(self.ALLOWED_PROPERTIES)) +class SerialisedModelCollection(ABC): + + """ + A SerialisedModelCollection takes a list of dictionaries, typically + created by serialising database objects. When iterated over it + returns a SerialisedModel instance for each of the items in the list. + """ + + @property + @abstractmethod + def model(self): + pass + + def __init__(self, items): + self.items = items + + def __bool__(self): + return bool(self.items) + + def __getitem__(self, index): + return self.model(self.items[index]) + + class SerialisedTemplate(SerialisedModel): ALLOWED_PROPERTIES = { 'archived', @@ -36,3 +59,28 @@ class SerialisedTemplate(SerialisedModel): 'template_type', 'version', } + + +class SerialisedService(SerialisedModel): + ALLOWED_PROPERTIES = { + 'id', + 'active', + 'contact_link', + 'email_from', + 'permissions', + 'research_mode', + 'restricted', + } + + +class SerialisedAPIKey(SerialisedModel): + ALLOWED_PROPERTIES = { + 'id', + 'secret', + 'expiry_date', + 'key_type', + } + + +class SerialisedAPIKeyCollection(SerialisedModelCollection): + model = SerialisedAPIKey diff --git a/app/service/send_notification.py b/app/service/send_notification.py index a7886b624..10ed42ab7 100644 --- a/app/service/send_notification.py +++ b/app/service/send_notification.py @@ -137,7 +137,9 @@ def get_reply_to_text(notification_type, sender_id, service, template): def send_pdf_letter_notification(service_id, post_data): service = dao_fetch_service_by_id(service_id) - check_service_has_permission(LETTER_TYPE, service.permissions) + check_service_has_permission(LETTER_TYPE, [ + p.permission for p in service.permissions + ]) check_service_over_daily_message_limit(KEY_TYPE_NORMAL, service) validate_created_by(service, post_data['created_by']) validate_and_format_recipient( diff --git a/app/service/utils.py b/app/service/utils.py index 16521fbbd..c6e0e0476 100644 --- a/app/service/utils.py +++ b/app/service/utils.py @@ -7,6 +7,8 @@ from app.models import ( MOBILE_TYPE, EMAIL_TYPE, KEY_TYPE_TEST, KEY_TYPE_TEAM, KEY_TYPE_NORMAL) +from app.dao.services_dao import dao_fetch_service_by_id + def get_recipients_from_request(request_json, key, type): return [(type, recipient) for recipient in request_json.get(key)] @@ -33,6 +35,10 @@ def service_allowed_to_send_to(recipient, service, key_type, allow_whitelisted_r if key_type == KEY_TYPE_NORMAL and not service.restricted: return True + # Revert back to the ORM model here so we can get some things which + # aren’t in the serialised model + service = dao_fetch_service_by_id(service.id) + team_members = itertools.chain.from_iterable( [user.mobile_number, user.email_address] for user in service.users ) diff --git a/app/template/rest.py b/app/template/rest.py index 80e33ff9c..4d116210f 100644 --- a/app/template/rest.py +++ b/app/template/rest.py @@ -69,7 +69,9 @@ def validate_parent_folder(template_json): def create_template(service_id): fetched_service = dao_fetch_service_by_id(service_id=service_id) # permissions needs to be placed here otherwise marshmallow will interfere with versioning - permissions = fetched_service.permissions + permissions = [ + p.permission for p in fetched_service.permissions + ] template_json = validate(request.get_json(), post_create_template_schema) folder = validate_parent_folder(template_json=template_json) new_template = Template.from_json(template_json, folder) @@ -102,7 +104,12 @@ def create_template(service_id): def update_template(service_id, template_id): fetched_template = dao_get_template_by_id_and_service_id(template_id=template_id, service_id=service_id) - if not service_has_permission(fetched_template.template_type, fetched_template.service.permissions): + if not service_has_permission( + fetched_template.template_type, + [ + p.permission for p in fetched_template.service.permissions + ] + ): message = "Updating {} templates is not allowed".format( get_public_notify_type_text(fetched_template.template_type)) errors = {'template_type': [message]} diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index c8559164d..d9a637432 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -343,7 +343,7 @@ def process_letter_notification( if api_key.key_type == KEY_TYPE_TEAM: raise BadRequestError(message='Cannot send letters with a team api key', status_code=403) - if not api_key.service.research_mode and api_key.service.restricted and api_key.key_type != KEY_TYPE_TEST: + if not service.research_mode and service.restricted and api_key.key_type != KEY_TYPE_TEST: raise BadRequestError(message='Cannot send letters when service is in trial mode', status_code=403) if precompiled: @@ -353,7 +353,7 @@ def process_letter_notification( template=template, reply_to_text=reply_to_text) - validate_address(api_key, letter_data) + validate_address(service, letter_data) test_key = api_key.key_type == KEY_TYPE_TEST @@ -399,10 +399,10 @@ def process_letter_notification( return resp -def validate_address(api_key, letter_data): +def validate_address(service, letter_data): address = PostalAddress.from_personalisation( letter_data['personalisation'], - allow_international_letters=api_key.service.has_permission(INTERNATIONAL_LETTERS), + allow_international_letters=(INTERNATIONAL_LETTERS in service.permissions), ) if not address.has_enough_lines: raise ValidationError( diff --git a/tests/app/authentication/test_authentication.py b/tests/app/authentication/test_authentication.py index 3b6b086aa..4cae2acd1 100644 --- a/tests/app/authentication/test_authentication.py +++ b/tests/app/authentication/test_authentication.py @@ -300,7 +300,7 @@ def test_authentication_returns_token_expired_when_service_uses_expired_key_and_ with pytest.raises(AuthError) as exc: requires_auth() assert exc.value.short_message == 'Invalid token: API key revoked' - assert exc.value.service_id == expired_api_key.service_id + assert exc.value.service_id == str(expired_api_key.service_id) assert exc.value.api_key_id == expired_api_key.id @@ -376,7 +376,7 @@ def test_authentication_returns_error_when_service_has_no_secrets(client, with pytest.raises(AuthError) as exc: requires_auth() assert exc.value.short_message == 'Invalid token: service has no API keys' - assert exc.value.service_id == sample_service.id + assert exc.value.service_id == str(sample_service.id) def test_should_attach_the_current_api_key_to_current_app(notify_api, sample_service, sample_api_key): @@ -387,7 +387,7 @@ def test_should_attach_the_current_api_key_to_current_app(notify_api, sample_ser headers={'Authorization': 'Bearer {}'.format(token)} ) assert response.status_code == 200 - assert api_user == sample_api_key + assert str(api_user.id) == str(sample_api_key.id) def test_should_return_403_when_token_is_expired(client, @@ -399,8 +399,8 @@ def test_should_return_403_when_token_is_expired(client, request.headers = {'Authorization': 'Bearer {}'.format(token)} requires_auth() assert exc.value.short_message == 'Error: Your system clock must be accurate to within 30 seconds' - assert exc.value.service_id == sample_api_key.service_id - assert exc.value.api_key_id == sample_api_key.id + assert exc.value.service_id == str(sample_api_key.service_id) + assert str(exc.value.api_key_id) == str(sample_api_key.id) def __create_token(service_id): diff --git a/tests/app/notifications/test_validators.py b/tests/app/notifications/test_validators.py index 49ad9615b..be9b6c65c 100644 --- a/tests/app/notifications/test_validators.py +++ b/tests/app/notifications/test_validators.py @@ -4,6 +4,7 @@ from flask import current_app from notifications_utils import SMS_CHAR_COUNT_LIMIT import app +from app.authentication.auth import get_service_model from app.dao import templates_dao from app.models import SMS_TYPE, EMAIL_TYPE, LETTER_TYPE from app.notifications.process_notifications import create_content_for_notification @@ -439,8 +440,9 @@ def test_rejects_api_calls_with_international_numbers_if_service_does_not_allow_ notify_db_session, ): service = create_service(service_permissions=[SMS_TYPE]) + service_model = get_service_model(service.id) with pytest.raises(BadRequestError) as e: - validate_and_format_recipient('20-12-1234-1234', key_type, service, SMS_TYPE) + validate_and_format_recipient('20-12-1234-1234', key_type, service_model, SMS_TYPE) assert e.value.status_code == 400 assert e.value.message == 'Cannot send to international mobile numbers' assert e.value.fields == [] @@ -449,7 +451,8 @@ def test_rejects_api_calls_with_international_numbers_if_service_does_not_allow_ @pytest.mark.parametrize('key_type', ['test', 'normal']) def test_allows_api_calls_with_international_numbers_if_service_does_allow_int_sms( key_type, sample_service_full_permissions): - result = validate_and_format_recipient('20-12-1234-1234', key_type, sample_service_full_permissions, SMS_TYPE) + service_model = get_service_model(sample_service_full_permissions.id) + result = validate_and_format_recipient('20-12-1234-1234', key_type, service_model, SMS_TYPE) assert result == '201212341234' diff --git a/tests/app/v2/notifications/test_post_notifications.py b/tests/app/v2/notifications/test_post_notifications.py index 7fe8acc0a..990726cbb 100644 --- a/tests/app/v2/notifications/test_post_notifications.py +++ b/tests/app/v2/notifications/test_post_notifications.py @@ -818,8 +818,8 @@ def test_post_notification_with_document_upload(client, notify_db_session, mocke assert validate(resp_json, post_email_response) == resp_json assert document_download_mock.upload_document.call_args_list == [ - call(service.id, 'abababab', csv_param.get('is_csv')), - call(service.id, 'cdcdcdcd', csv_param.get('is_csv')) + call(str(service.id), 'abababab', csv_param.get('is_csv')), + call(str(service.id), 'cdcdcdcd', csv_param.get('is_csv')) ] notification = Notification.query.one()