From 5c64b47ca51492bdad68ef048e7ded6510ab64e9 Mon Sep 17 00:00:00 2001 From: Nikolai Efimov Date: Tue, 15 Aug 2023 07:16:00 -0700 Subject: [PATCH] 1344 - Authenticate via Reader instance (#1421) * read-db uri, engine binding, refactored service * add read-db for testing, formatting fixes, removed prints * added SQLALCHEMY_DATABASE_URI_READ to tasks def-n * adding .rate_limit and .service_sms_senders to AutheticatedServiceInfo * lintere fixes * added tasks for perf, prod and staging * improve readability of docker-compose * add message_limit * refactoring and clean up * lint fix * blank line fix * fixing unit tests init work * fixing unit tests aug2-3 part 1 * fixing unit tests aug 4 * fixed .has_permissions and service_has_permission * all unit tests are passing * cleaned up dev code and prints * additional clean up of unused code and comments * linter fixes * fixed "secret" violates not-null constraint" error * removed commented out import * add logger prints and move .close() * reverting * contextmanager and AuthenticatedServiceApiKey * clean up * clean up * added ticket number to TODO comments * docs for AuthenticatedServiceInfo and clean up * changed set to frozenset * added annotations * pr chage requests * new fixture, __eq__ for ServiceData, renamed AuthenticatedServiceInfo to ServiceData * readme update, moved context to dao utils, changed 403 test case to use sample_service_data_api_key * revert test changes * 403 fix and clean up * fix sample_service_data_api_key def and test_should_attach_the_current_api_key_to_current_app * use postgres 15 * fixed SQLALCHEMY_BINDS in main config * fixed SQLALCHEMY_BINDS * fixed _READ env variable * mod Staging and Production * rm extra line * add a log statement for SQLALCHEMY_BINDS * style fixes * logging * log in config.py * add _read to all task definitions, not only api * check BINDS instead of _READ directly --- .github/workflows/tests.yaml | 1 + app/__init__.py | 4 + app/cloudfoundry_config.py | 1 + app/config.py | 18 ++ app/dao/api_key_dao.py | 3 +- app/dao/dao_utils.py | 25 ++ app/dao/services_dao.py | 34 ++- app/models.py | 18 +- app/notifications/process_notifications.py | 1 - app/notifications/receive_notifications.py | 7 +- app/notifications/rest.py | 10 +- app/notifications/validators.py | 16 +- app/service/send_notification.py | 12 +- app/service/service_data.py | 215 ++++++++++++++++++ app/template/rest.py | 1 + app/v2/notifications/post_notifications.py | 21 +- app/v2/notifications/rest_push.py | 9 +- .../dev/vaec-api-task-definition.json | 4 + .../dev/vaec-celery-beat-task-definition.json | 4 + .../dev/vaec-celery-task-definition.json | 4 + .../vaec-db-downgrade-task-definition.json | 4 + .../vaec-db-migrations-task-definition.json | 4 + .../perf/vaec-api-task-definition.json | 4 + .../vaec-celery-beat-task-definition.json | 4 + .../perf/vaec-celery-task-definition.json | 4 + .../vaec-db-downgrade-task-definition.json | 4 + .../vaec-db-migrations-task-definition.json | 4 + .../prod/vaec-api-task-definition.json | 4 + .../vaec-celery-beat-task-definition.json | 4 + .../prod/vaec-celery-task-definition.json | 4 + .../vaec-db-downgrade-task-definition.json | 4 + .../vaec-db-migrations-task-definition.json | 4 + .../staging/vaec-api-task-definition.json | 4 + .../vaec-celery-beat-task-definition.json | 4 + .../staging/vaec-celery-task-definition.json | 4 + .../vaec-db-downgrade-task-definition.json | 4 + .../vaec-db-migrations-task-definition.json | 4 + ci/.docker-env.example | 3 +- ci/README.md | 119 ++++++++++ ci/docker-compose-local-readwrite.yml | 100 ++++++++ ci/docker-compose-local.yml | 8 +- ci/docker-compose-test.yml | 1 + ci/docker-compose.yml | 2 + tests/README.md | 6 +- .../app/authentication/test_authentication.py | 19 +- tests/app/conftest.py | 18 ++ .../test_receive_notification.py | 9 +- tests/app/test_cloudfoundry_config.py | 1 + tests/env_vars | 1 + 49 files changed, 696 insertions(+), 67 deletions(-) create mode 100644 app/service/service_data.py create mode 100644 ci/README.md create mode 100644 ci/docker-compose-local-readwrite.yml diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index fbb2cf1a3d..632c8af7f5 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -52,6 +52,7 @@ jobs: NOTIFY_ENVIRONMENT: test RETRY_SQS_URL: RETRY_SQS_URL SQLALCHEMY_DATABASE_URI: postgresql://postgres:postgres@localhost/test_notification_api + SQLALCHEMY_DATABASE_URI_READ: postgresql://postgres:postgres@localhost/test_notification_api TIMEOUT: 10 VA_PROFILE_DOMAIN: int.vaprofile.va.gov VETEXT_API_AUTH_SSM_PATH: test diff --git a/app/__init__.py b/app/__init__.py index 94f65d733e..53aee2f22b 100644 --- a/app/__init__.py +++ b/app/__init__.py @@ -97,8 +97,12 @@ def create_app(application, worker_id=None): if notify_environment == "test": assert worker_id is not None application.config["SQLALCHEMY_DATABASE_URI"] += f"_{worker_id}" + # set read-db to be the same as write/default instance for testing + application.config["SQLALCHEMY_BINDS"] = {"read-db": application.config["SQLALCHEMY_DATABASE_URI"]} assert "test_notification_api" in application.config["SQLALCHEMY_DATABASE_URI"], \ "Don't run tests against the main database." + assert "test_notification_api" in application.config["SQLALCHEMY_BINDS"]["read-db"], \ + "Don't run tests against the main database." application.config["NOTIFY_APP_NAME"] = application.name init_app(application) diff --git a/app/cloudfoundry_config.py b/app/cloudfoundry_config.py index 5cb7c437f4..0583a9eb1a 100644 --- a/app/cloudfoundry_config.py +++ b/app/cloudfoundry_config.py @@ -15,6 +15,7 @@ def extract_cloudfoundry_config(): def set_config_env_vars(vcap_services): # Postgres config os.environ['SQLALCHEMY_DATABASE_URI'] = vcap_services['postgres'][0]['credentials']['uri'] + os.environ['SQLALCHEMY_DATABASE_URI_READ'] = vcap_services['postgres'][0]['credentials']['uri'] vcap_application = json.loads(os.environ['VCAP_APPLICATION']) os.environ['NOTIFY_ENVIRONMENT'] = vcap_application['space_name'] diff --git a/app/config.py b/app/config.py index db787bc5d0..ba69b782e1 100644 --- a/app/config.py +++ b/app/config.py @@ -7,6 +7,7 @@ from fido2.server import Fido2Server from fido2.webauthn import PublicKeyCredentialRpEntity from kombu import Exchange, Queue +import logging load_dotenv() @@ -108,6 +109,7 @@ class Config(object): # DB conection string SQLALCHEMY_DATABASE_URI = os.getenv('SQLALCHEMY_DATABASE_URI') + SQLALCHEMY_BINDS = {"read-db": os.getenv('SQLALCHEMY_DATABASE_URI_READ')} # MMG API Key MMG_API_KEY = os.getenv('MMG_API_KEY') @@ -514,6 +516,10 @@ class Development(Config): "SQLALCHEMY_DATABASE_URI", 'postgresql://postgres@localhost/notification_api') + SQLALCHEMY_BINDS = {"read-db": os.getenv( + "SQLALCHEMY_DATABASE_URI_READ", + 'postgresql://postgres@localhost/notification_api')} + ANTIVIRUS_ENABLED = os.getenv('ANTIVIRUS_ENABLED') == '1' for queue in QueueNames.all_queues(): @@ -543,6 +549,9 @@ class Test(Development): 'SQLALCHEMY_DATABASE_URI', 'postgresql://postgres@localhost/test_notification_api' ) + SQLALCHEMY_BINDS = {"read-db": os.getenv( + "SQLALCHEMY_DATABASE_URI_READ", + 'postgresql://postgres@localhost/notification_api')} CELERY_SETTINGS = { 'broker_url': 'you-forgot-to-mock-celery-in-your-tests://' @@ -585,6 +594,10 @@ class Staging(Config): FROM_NUMBER = '+18555420534' SESSION_COOKIE_SECURE = True + SQLALCHEMY_DATABASE_URI = os.getenv('SQLALCHEMY_DATABASE_URI') + SQLALCHEMY_BINDS = {"read-db": os.getenv("SQLALCHEMY_DATABASE_URI_READ")} + if SQLALCHEMY_BINDS['read-db'] is None: + logging.critical("Missing SQLALCHEMY_DATABASE_URI_READ") class Production(Config): @@ -604,6 +617,11 @@ class Production(Config): SESSION_COOKIE_SECURE = True + SQLALCHEMY_DATABASE_URI = os.getenv('SQLALCHEMY_DATABASE_URI') + SQLALCHEMY_BINDS = {"read-db": os.getenv("SQLALCHEMY_DATABASE_URI_READ")} + if SQLALCHEMY_BINDS['read-db'] is None: + logging.critical("Missing SQLALCHEMY_DATABASE_URI_READ") + configs = { 'development': Development, diff --git a/app/dao/api_key_dao.py b/app/dao/api_key_dao.py index d550c9dc0c..52bb65f98f 100644 --- a/app/dao/api_key_dao.py +++ b/app/dao/api_key_dao.py @@ -17,7 +17,8 @@ def save_model_api_key(api_key): if not api_key.id: api_key.id = uuid.uuid4() # must be set now so version history model can use same id - api_key.secret = uuid.uuid4() + if not api_key.secret: + api_key.secret = uuid.uuid4() db.session.add(api_key) diff --git a/app/dao/dao_utils.py b/app/dao/dao_utils.py index e425f56014..ef7f10f714 100644 --- a/app/dao/dao_utils.py +++ b/app/dao/dao_utils.py @@ -2,6 +2,8 @@ from app import db from app.history_meta import create_history from functools import wraps +from contextlib import contextmanager +from sqlalchemy.orm import scoped_session, sessionmaker def transactional(func): @@ -79,3 +81,26 @@ def record_version(*args, **kwargs): def dao_rollback(): db.session.rollback() + + +@contextmanager +def get_reader_session(): + """ + This context manager is used to abstract the connection to the read-only database engine + in order to execute read queries. By using a scoped session, it ensures that the session + is thread-local and can be reused if needed. It ensures proper handling of the session's + lifecycle by closing it when the context is exited. + + Yields: + session (scoped_session): A session connected to the read-only database engine. + + Example Usage: + with get_reader_session() as session: + result = session.query(Service).filter_by(id=service_id).one() + """ + engine = db.engines['read-db'] + session = scoped_session(sessionmaker(bind=engine)) + try: + yield session + finally: + session.close() diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index 3f64e53389..1a9528573f 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -1,15 +1,18 @@ import uuid from datetime import date, datetime, timedelta +from app.service.service_data import ServiceData, ServiceDataException from notifications_utils.statsd_decorators import statsd from notifications_utils.timezones import convert_utc_to_local_timezone from sqlalchemy.sql.expression import asc, case, and_, func from sqlalchemy.orm import joinedload +from sqlalchemy.orm.exc import NoResultFound, MultipleResultsFound from flask import current_app from app import db from app.dao.date_util import get_current_financial_year from app.dao.dao_utils import ( + get_reader_session, transactional, version_class, VersionOptions, @@ -194,16 +197,29 @@ def dao_fetch_service_by_inbound_number(number): def dao_fetch_service_by_id_with_api_keys(service_id, only_active=False): - query = Service.query.filter_by( - id=service_id - ).options( - joinedload('api_keys') - ) - - if only_active: - query = query.filter(Service.active) + with get_reader_session() as session: + query = session.query(Service).filter_by( + id=service_id + ).options( + joinedload('api_keys') + ) - return query.one() + if only_active: + query = query.filter(Service.active) + + try: + result = query.one() + # instead of returning the whole model attached to read-db engine + # extract needed properties and return object that can be + # serialized for caching + return ServiceData(result) + except (ServiceDataException, NoResultFound, MultipleResultsFound) as err: + # we handle this failure in the parent + current_app.logger.error("Could not find unique service with ID %s", service_id) + raise NoResultFound(err) + except Exception: + # we handle this failure in the parent + raise def dao_fetch_all_services_by_user(user_id, only_active=False): diff --git a/app/models.py b/app/models.py index 3289c2a1fe..c31d0d83f4 100644 --- a/app/models.py +++ b/app/models.py @@ -34,7 +34,7 @@ from sqlalchemy.ext.associationproxy import association_proxy from sqlalchemy.ext.declarative import declared_attr from sqlalchemy.ext.hybrid import hybrid_property -from sqlalchemy.orm.collections import attribute_mapped_collection +from sqlalchemy.orm.collections import attribute_mapped_collection, InstrumentedList SMS_TYPE = 'sms' @@ -448,8 +448,20 @@ def get_default_letter_contact(self): default_letter_contact = [x for x in self.letter_contacts if x.is_default] return default_letter_contact[0].contact_block if default_letter_contact else None - def has_permission(self, permission): - return permission in [p.permission for p in self.permissions] + def has_permissions(self, permissions_to_check_for): + if isinstance(permissions_to_check_for, InstrumentedList): + _permissions_to_check_for = [p.permission for p in permissions_to_check_for] + elif not isinstance(permissions_to_check_for, list): + _permissions_to_check_for = [permissions_to_check_for] + else: + _permissions_to_check_for = permissions_to_check_for + + if isinstance(self.permissions, InstrumentedList): + _permissions = [p.permission for p in self.permissions] + else: + _permissions = self.permissions + + return frozenset(_permissions_to_check_for).issubset(frozenset(_permissions)) def serialize_for_org_dashboard(self): return { diff --git a/app/notifications/process_notifications.py b/app/notifications/process_notifications.py index 703254824f..160b291aa4 100644 --- a/app/notifications/process_notifications.py +++ b/app/notifications/process_notifications.py @@ -96,7 +96,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/receive_notifications.py b/app/notifications/receive_notifications.py index 50013df8ae..19a0dac70b 100644 --- a/app/notifications/receive_notifications.py +++ b/app/notifications/receive_notifications.py @@ -223,7 +223,7 @@ def fetch_potential_service(inbound_number: str, provider_name: str) -> Service: current_app.logger.error(message) raise NoSuitableServiceForInboundSms(message) - elif not has_inbound_sms_permissions(service.permissions): + elif not service.has_permissions([INBOUND_SMS_TYPE, SMS_TYPE]): statsd_client.incr(f"inbound.{provider_name}.failed") message = f'Service "{service.id}" does not allow inbound SMS' current_app.logger.error(message) @@ -233,11 +233,6 @@ def fetch_potential_service(inbound_number: str, provider_name: str) -> Service: return service -def has_inbound_sms_permissions(permissions): - str_permissions = [p.permission for p in permissions] - return set([INBOUND_SMS_TYPE, SMS_TYPE]).issubset(set(str_permissions)) - - def strip_leading_forty_four(number): if number.startswith('44'): return number.replace('44', '0', 1) diff --git a/app/notifications/rest.py b/app/notifications/rest.py index d084f66a21..6a0e2e10b9 100644 --- a/app/notifications/rest.py +++ b/app/notifications/rest.py @@ -31,8 +31,7 @@ from app.notifications.validators import ( check_template_is_for_notification_type, check_template_is_active, - check_rate_limiting, - service_has_permission + check_rate_limiting ) from app.schemas import ( email_notification_schema, @@ -122,7 +121,7 @@ def send_notification(notification_type): template_object = create_template_object_for_notification(template, notification_form.get('personalisation', {})) _service_allowed_to_send_to(notification_form, authenticated_service) - if not service_has_permission(notification_type, authenticated_service.permissions): + if not authenticated_service.has_permissions(notification_type): raise InvalidRequest( {'service': ["Cannot send {}".format(get_public_notify_type_text(notification_type, plural=True))]}, status_code=400 @@ -187,10 +186,9 @@ def get_notification_return_data(notification_id, notification, template): def _service_can_send_internationally(service, number): - international_phone_info = get_international_phone_info(number) + 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]: + if phone_info.international and not service.has_permissions(INTERNATIONAL_SMS_TYPE): 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 18934fd049..52640d241d 100644 --- a/app/notifications/validators.py +++ b/app/notifications/validators.py @@ -23,7 +23,6 @@ from app.v2.errors import TooManyRequestsError, BadRequestError, RateLimitError from app import redis_store from app.notifications.process_notifications import create_content_for_notification -from app.utils import get_public_notify_type_text from app.dao.service_email_reply_to_dao import dao_get_reply_to_by_id from app.dao.service_letter_contact_dao import dao_get_letter_contact_by_id @@ -108,16 +107,12 @@ def service_can_send_to_recipient(send_to, key_type, service, allow_whitelisted_ raise BadRequestError(message=message) +# TODO #1410 clean up and remove def service_has_permission(notify_type, permissions): return notify_type in [p.permission for p in permissions] -def check_service_has_permission(notify_type, permissions): - if not service_has_permission(notify_type, permissions): - raise BadRequestError(message="Service is not allowed to send {}".format( - get_public_notify_type_text(notify_type, plural=True))) - - +# TODO #1410 clean up and remove def check_service_can_schedule_notification(permissions, scheduled_for): if scheduled_for: if not service_has_permission(SCHEDULE_NOTIFICATIONS, permissions): @@ -131,15 +126,14 @@ def validate_and_format_recipient(send_to, key_type, service, notification_type, service_can_send_to_recipient(send_to, key_type, service, allow_whitelisted_recipients) if notification_type == SMS_TYPE: - international_phone_info = get_international_phone_info(send_to) + 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]: + if phone_info.international and not service.has_permissions(INTERNATIONAL_SMS_TYPE): raise BadRequestError(message="Cannot send to international mobile numbers") return validate_and_format_phone_number( number=send_to, - international=international_phone_info.international + international=phone_info.international ) elif notification_type == EMAIL_TYPE: return validate_and_format_email_address(email_address=send_to) diff --git a/app/service/send_notification.py b/app/service/send_notification.py index 26b141067f..0c390f2f8c 100644 --- a/app/service/send_notification.py +++ b/app/service/send_notification.py @@ -8,7 +8,6 @@ from app.dao.service_email_reply_to_dao import dao_get_reply_to_by_id from app.dao.service_sms_sender_dao import dao_get_service_sms_sender_by_id from app.notifications.validators import ( - check_service_has_permission, check_service_over_daily_message_limit, validate_and_format_recipient, validate_template @@ -35,6 +34,7 @@ move_uploaded_pdf_to_letters_bucket, ) from app.v2.errors import BadRequestError +from app.utils import get_public_notify_type_text def validate_created_by(service, created_by_id): @@ -135,8 +135,14 @@ 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(UPLOAD_LETTERS, service.permissions) + if not service.has_permissions(LETTER_TYPE): + raise BadRequestError(message="Service is not allowed to send {}".format( + get_public_notify_type_text(LETTER_TYPE, plural=True))) + + if not service.has_permissions(UPLOAD_LETTERS): + raise BadRequestError(message="Service is not allowed to send {}".format( + get_public_notify_type_text(UPLOAD_LETTERS, plural=True))) + check_service_over_daily_message_limit(KEY_TYPE_NORMAL, service) validate_created_by(service, post_data['created_by']) diff --git a/app/service/service_data.py b/app/service/service_data.py new file mode 100644 index 0000000000..10b8afaaeb --- /dev/null +++ b/app/service/service_data.py @@ -0,0 +1,215 @@ +import json +from types import SimpleNamespace +from typing import List, Union + +from app.models import Service + + +class ServiceDataException(Exception): + """ + Custom exception class for handling specific errors related to the ServiceData class. + + Attributes: + message (str): A custom message describing the error. + + Usage: + raise ServiceDataException("Description...") + """ + + def __init__( + self, + message="Unable to create ServiceData object.", + *args, + **kwargs + ): + super().__init__(message, *args, **kwargs) + + +class ServiceDataApiKey: + """ + A class to encapsulate information related to an authenticated service's API key. + + This class serves as a data structure to hold the essential properties of an API key + associated with an authenticated service. It includes a simplified representation of + related service attributes (research mode, restricted, and active status). + + This is needed to deal with circular dependency and multilevel nesting in previously + created SQLAlchemy model. + + Creating simple namespance to preseve dot notation used in calling functions. + + Attributes: + created_at (datetime): The timestamp when the API key was created. + created_by (User): The user who created the API key. + created_by_id (str): The ID of the user who created the API key. + expiry_date (datetime): The expiry date of the API key. + id (str): The unique identifier of the API key. + key_type (str): The type of the API key (e.g., "normal"). + name (str): The name of the API key. + secret (str): The secret string associated with the API key. + service (SimpleNamespace): A namespace containing the 'research_mode', + 'restricted', and 'active' attributes of the related service. + service_id (str): The ID of the related service. + updated_at (datetime): The timestamp when the API key was last updated. + """ + + def __init__(self, key) -> None: + """ + Args: + key (object): The object containing the necessary attributes for the API key. + """ + self.created_at = key.created_at + self.created_by = key.created_by + self.created_by_id = key.created_by_id + self.expiry_date = key.expiry_date + self.id = key.id + self.key_type = key.key_type + self.name = key.name + self.secret = key.secret + + _service = { + "research_mode": key.service.research_mode, + "restricted": key.service.restricted, + "active": key.service.active, + } + self.service = SimpleNamespace(**_service) + self.service_id = key.service_id + self.updated_at = key.updated_at + + def __eq__(self, other: 'ServiceDataApiKey') -> bool: + """ + Determines equality between two instances of ServiceDataApiKey. + + Two instances of ServiceDataApiKey are considered equal if they have the same values for + the following attributes: + - id + - name + - key_type + - created_by_id + - service_id + - secret + + Args: + other (ServiceDataApiKey): The object to compare against. + + Returns: + bool: True if the instances have the same values for the relevant attributes, False otherwise. + """ + if isinstance(other, ServiceDataApiKey): + return self.id == other.id\ + and self.key_type == other.key_type\ + and self.name == other.name\ + and self.service_id == other.service_id\ + and self.created_by_id == other.created_by_id\ + and self.secret == other.secret + return False + + +class ServiceData: + """ + Represents the relevant information extracted from an SQLAlchemy query result. + + This class was created to allow for reading from multiple databases and to facilitate caching. + By extracting only the necessary information from the query result, it ensures compatibility + across different database connections and enables efficient serialization for caching purposes. + The extraction of specific information rather than working with entire query results provides a + more controlled and optimized way to handle the data. + + Attributes: + active (bool): A flag indicating whether the service is active. + permissions (list): A list of permissions associated with the service. + api_keys (list): A list of API keys associated with the service. + id (int): The unique identifier of the service. + research_mode (bool): A flag indicating whether the service is in research mode. + restricted (bool): A flag indicating whether the service is restricted. + rate_limit (int): The rate limit applied to the service. + service_sms_senders (list): A list of SMS senders associated with the service. + message_limit (int): The message limit applied to the service. + users (list): A list of users associated with the service. + whitelist (list): A list of whitelisted entities for the service. + + Methods: + + """ + + def __init__(self, result=None): + self.active = None + self.permissions = None + self.api_keys = None + self.id = None + self.research_mode = None + self.restricted = None + self.rate_limit = None + self.service_sms_senders = None + self.message_limit = None + self.users = None + self.whitelist = None + if result is not None: + try: + self.extract(result) + except Exception as err: + raise ServiceDataException(err) + + def extract(self, result: Service) -> None: + """ + Extracts the necessary data from an authenticated service ORM object. + + Args: + result: The ORM object containing the data. WARNING it's not pure Service + object, but Service augmented with api_keys + + Returns: + None + """ + self.active = result.active + self.permissions = [p.permission for p in result.permissions] + self.api_keys = [ServiceDataApiKey(key) for key in result.api_keys] + self.id = result.id + self.research_mode = result.research_mode + self.restricted = result.restricted + self.rate_limit = result.rate_limit + self.service_sms_senders = result.service_sms_senders + self.message_limit = result.message_limit + self.users = [u for u in result.users] + self.whitelist = [w for w in result.whitelist] + + def serialize(self) -> str: + """ + Serializes the object into a JSON string. + This method might be used for caching, allowing the object to be easily stored and retrieved. + + Returns: + str: A JSON string. + """ + return json.dumps(self.__dict__) + + def has_permissions(self, permissions_to_check_for: Union[str, List[str]]) -> bool: + """ + Checks if the object has the specified permissions. + + Args: + permissions_to_check_for (list): A list or a single permission to check for. + + Returns: + bool: True if all specified permissions are present, False otherwise. + """ + if not isinstance(permissions_to_check_for, list): + tmp = permissions_to_check_for + permissions_to_check_for = [tmp] + return frozenset(permissions_to_check_for).issubset(frozenset(self.permissions)) + + @classmethod + def deserialize(cls, json_string: str) -> 'ServiceData': + """ + Creates a new instance of the class using a JSON string. + This method might be used after retrieval from caching, allowing the object to be reconstructed. + + Args: + json_string (str): A JSON string representing the object. + + Returns: + ServiceData: A new instance of the class populated with the data from the JSON string. + """ + result = cls() + result.__dict__ = json.loads(json_string) + return result diff --git a/app/template/rest.py b/app/template/rest.py index b12ee17261..c89c0e220e 100644 --- a/app/template/rest.py +++ b/app/template/rest.py @@ -98,6 +98,7 @@ def create_template(service_id): errors = {'content': [message]} raise InvalidRequest(errors, status_code=400) + # TODO #1410 clean up validator, use class method instead. if not service_has_permission(new_template.template_type, permissions): message = "Creating {} templates is not allowed".format( get_public_notify_type_text(new_template.template_type)) diff --git a/app/v2/notifications/post_notifications.py b/app/v2/notifications/post_notifications.py index 19424d9d51..b30c4ba53b 100644 --- a/app/v2/notifications/post_notifications.py +++ b/app/v2/notifications/post_notifications.py @@ -17,6 +17,7 @@ from app.feature_flags import accept_recipient_identifiers_enabled, is_feature_enabled, FeatureFlag from app.letters.utils import upload_letter_pdf from app.models import ( + SCHEDULE_NOTIFICATIONS, SMS_TYPE, EMAIL_TYPE, LETTER_TYPE, @@ -42,8 +43,6 @@ from app.notifications.validators import ( validate_and_format_recipient, check_rate_limiting, - check_service_can_schedule_notification, - check_service_has_permission, validate_template, check_service_email_reply_to_id, check_service_sms_sender_id @@ -62,6 +61,7 @@ post_letter_request, post_precompiled_letter_request ) +from app.utils import get_public_notify_type_text # FIXME: POST requests to /v2/notifications/letter match this route and the @@ -74,8 +74,9 @@ def post_precompiled_letter_notification(): form = validate(request.get_json(), post_precompiled_letter_request) - # Check permission to send letters - check_service_has_permission(LETTER_TYPE, authenticated_service.permissions) + if not authenticated_service.has_permissions(LETTER_TYPE): + raise BadRequestError(message="Service is not allowed to send {}".format( + get_public_notify_type_text(LETTER_TYPE, plural=True))) check_rate_limiting(authenticated_service, api_user) @@ -131,11 +132,15 @@ def post_notification(notification_type): # noqa: C901 else: abort(404) - check_service_has_permission(notification_type, authenticated_service.permissions) + if not authenticated_service.has_permissions(notification_type): + raise BadRequestError(message="Service is not allowed to send {}".format( + get_public_notify_type_text(notification_type, plural=True))) scheduled_for = form.get("scheduled_for") - check_service_can_schedule_notification(authenticated_service.permissions, scheduled_for) + if scheduled_for is not None: + if not authenticated_service.has_permissions(SCHEDULE_NOTIFICATIONS): + raise BadRequestError(message="Cannot schedule notifications (this feature is invite-only)") check_rate_limiting(authenticated_service, api_user) @@ -308,7 +313,9 @@ def process_document_uploads(personalisation_data, service, simulated=False): personalisation_data = personalisation_data.copy() - check_service_has_permission(UPLOAD_DOCUMENT, authenticated_service.permissions) + if not authenticated_service.has_permissions(UPLOAD_DOCUMENT): + raise BadRequestError(message="Service is not allowed to send {}".format( + get_public_notify_type_text(UPLOAD_DOCUMENT, plural=True))) if any(personalisation_data[key].get('sending_method') == 'link' for key in file_keys): raise NotImplementedError() diff --git a/app/v2/notifications/rest_push.py b/app/v2/notifications/rest_push.py index 69a6d9a99d..349ef15347 100644 --- a/app/v2/notifications/rest_push.py +++ b/app/v2/notifications/rest_push.py @@ -7,9 +7,6 @@ from app.models import ( PUSH_TYPE ) -from app.notifications.validators import ( - check_service_has_permission -) from app.schema_validation import validate from app.v2.errors import BadRequestError from app.v2.notifications import v2_notification_blueprint @@ -17,6 +14,7 @@ push_notification_request ) from app.va.vetext import (VETextRetryableException, VETextNonRetryableException, VETextBadRequestException) +from app.utils import get_public_notify_type_text @v2_notification_blueprint.route('/push', methods=['POST']) @@ -24,7 +22,10 @@ def send_push_notification(): if not is_feature_enabled(FeatureFlag.PUSH_NOTIFICATIONS_ENABLED): raise NotImplementedError() - check_service_has_permission(PUSH_TYPE, authenticated_service.permissions) + if not authenticated_service.has_permissions(PUSH_TYPE): + raise BadRequestError(message="Service is not allowed to send {}".format( + get_public_notify_type_text(PUSH_TYPE, plural=True))) + req_json = validate(request.get_json(), push_notification_request) registry = MobileAppRegistry() diff --git a/cd/application-deployment/dev/vaec-api-task-definition.json b/cd/application-deployment/dev/vaec-api-task-definition.json index c48da9b809..45ba482f84 100644 --- a/cd/application-deployment/dev/vaec-api-task-definition.json +++ b/cd/application-deployment/dev/vaec-api-task-definition.json @@ -256,6 +256,10 @@ "name": "SQLALCHEMY_DATABASE_URI", "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/dev/notification-api/database/uri" }, + { + "name": "SQLALCHEMY_DATABASE_URI_READ", + "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/dev/notification-api/database/uri_read" + }, { "name": "TWILIO_ACCOUNT_SID", "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/dev/notification-api/twilio/account-sid" diff --git a/cd/application-deployment/dev/vaec-celery-beat-task-definition.json b/cd/application-deployment/dev/vaec-celery-beat-task-definition.json index a1137657a1..d8be2ee5c8 100644 --- a/cd/application-deployment/dev/vaec-celery-beat-task-definition.json +++ b/cd/application-deployment/dev/vaec-celery-beat-task-definition.json @@ -210,6 +210,10 @@ "name": "SQLALCHEMY_DATABASE_URI", "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/dev/notification-api/database/uri" }, + { + "name": "SQLALCHEMY_DATABASE_URI_READ", + "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/dev/notification-api/database/uri_read" + }, { "name": "TWILIO_ACCOUNT_SID", "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/dev/notification-api/twilio/account-sid" diff --git a/cd/application-deployment/dev/vaec-celery-task-definition.json b/cd/application-deployment/dev/vaec-celery-task-definition.json index 39a63316e0..d7a1d1eff4 100644 --- a/cd/application-deployment/dev/vaec-celery-task-definition.json +++ b/cd/application-deployment/dev/vaec-celery-task-definition.json @@ -238,6 +238,10 @@ "name": "SQLALCHEMY_DATABASE_URI", "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/dev/notification-api/database/uri" }, + { + "name": "SQLALCHEMY_DATABASE_URI_READ", + "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/dev/notification-api/database/uri_read" + }, { "name": "TWILIO_ACCOUNT_SID", "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/dev/notification-api/twilio/account-sid" diff --git a/cd/application-deployment/dev/vaec-db-downgrade-task-definition.json b/cd/application-deployment/dev/vaec-db-downgrade-task-definition.json index a8215500ee..037684fb5e 100644 --- a/cd/application-deployment/dev/vaec-db-downgrade-task-definition.json +++ b/cd/application-deployment/dev/vaec-db-downgrade-task-definition.json @@ -45,6 +45,10 @@ "name": "SQLALCHEMY_DATABASE_URI", "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/dev/notification-api/database/uri" }, + { + "name": "SQLALCHEMY_DATABASE_URI_READ", + "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/dev/notification-api/database/uri_read" + }, { "name": "SECRET_KEY", "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/dev/notification-api/secret-key" diff --git a/cd/application-deployment/dev/vaec-db-migrations-task-definition.json b/cd/application-deployment/dev/vaec-db-migrations-task-definition.json index 8adc4353b7..8c65a2ce82 100644 --- a/cd/application-deployment/dev/vaec-db-migrations-task-definition.json +++ b/cd/application-deployment/dev/vaec-db-migrations-task-definition.json @@ -45,6 +45,10 @@ "name": "SQLALCHEMY_DATABASE_URI", "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/dev/notification-api/database/uri" }, + { + "name": "SQLALCHEMY_DATABASE_URI_READ", + "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/dev/notification-api/database/uri_read" + }, { "name": "SECRET_KEY", "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/dev/notification-api/secret-key" diff --git a/cd/application-deployment/perf/vaec-api-task-definition.json b/cd/application-deployment/perf/vaec-api-task-definition.json index 59e1aa84d7..5ea6eb4a48 100644 --- a/cd/application-deployment/perf/vaec-api-task-definition.json +++ b/cd/application-deployment/perf/vaec-api-task-definition.json @@ -204,6 +204,10 @@ "name": "SQLALCHEMY_DATABASE_URI", "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/perf/notification-api/database/uri" }, + { + "name": "SQLALCHEMY_DATABASE_URI_READ", + "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/perf/notification-api/database/uri_read" + }, { "name": "ADMIN_CLIENT_SECRET", "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/perf/notification-api/admin-client-secret" diff --git a/cd/application-deployment/perf/vaec-celery-beat-task-definition.json b/cd/application-deployment/perf/vaec-celery-beat-task-definition.json index 02ce597a51..c7d74bc7ca 100644 --- a/cd/application-deployment/perf/vaec-celery-beat-task-definition.json +++ b/cd/application-deployment/perf/vaec-celery-beat-task-definition.json @@ -178,6 +178,10 @@ "name": "SQLALCHEMY_DATABASE_URI", "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/perf/notification-api/database/uri" }, + { + "name": "SQLALCHEMY_DATABASE_URI_READ", + "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/perf/notification-api/database/uri_read" + }, { "name": "SECRET_KEY", "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/perf/notification-api/secret-key" diff --git a/cd/application-deployment/perf/vaec-celery-task-definition.json b/cd/application-deployment/perf/vaec-celery-task-definition.json index f50496918c..d8798eb7f0 100644 --- a/cd/application-deployment/perf/vaec-celery-task-definition.json +++ b/cd/application-deployment/perf/vaec-celery-task-definition.json @@ -178,6 +178,10 @@ "name": "SQLALCHEMY_DATABASE_URI", "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/perf/notification-api/database/uri" }, + { + "name": "SQLALCHEMY_DATABASE_URI_READ", + "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/perf/notification-api/database/uri_read" + }, { "name": "SECRET_KEY", "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/perf/notification-api/secret-key" diff --git a/cd/application-deployment/perf/vaec-db-downgrade-task-definition.json b/cd/application-deployment/perf/vaec-db-downgrade-task-definition.json index 51a96bf29c..949c95b4f5 100644 --- a/cd/application-deployment/perf/vaec-db-downgrade-task-definition.json +++ b/cd/application-deployment/perf/vaec-db-downgrade-task-definition.json @@ -45,6 +45,10 @@ "name": "SQLALCHEMY_DATABASE_URI", "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/perf/notification-api/database/uri" }, + { + "name": "SQLALCHEMY_DATABASE_URI_READ", + "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/perf/notification-api/database/uri_read" + }, { "name": "SECRET_KEY", "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/perf/notification-api/secret-key" diff --git a/cd/application-deployment/perf/vaec-db-migrations-task-definition.json b/cd/application-deployment/perf/vaec-db-migrations-task-definition.json index 5572250dde..84023c753b 100644 --- a/cd/application-deployment/perf/vaec-db-migrations-task-definition.json +++ b/cd/application-deployment/perf/vaec-db-migrations-task-definition.json @@ -45,6 +45,10 @@ "name": "SQLALCHEMY_DATABASE_URI", "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/perf/notification-api/database/uri" }, + { + "name": "SQLALCHEMY_DATABASE_URI_READ", + "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/perf/notification-api/database/uri_read" + }, { "name": "SECRET_KEY", "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/perf/notification-api/secret-key" diff --git a/cd/application-deployment/prod/vaec-api-task-definition.json b/cd/application-deployment/prod/vaec-api-task-definition.json index c411d10654..bca9bd844c 100644 --- a/cd/application-deployment/prod/vaec-api-task-definition.json +++ b/cd/application-deployment/prod/vaec-api-task-definition.json @@ -220,6 +220,10 @@ "name": "SQLALCHEMY_DATABASE_URI", "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/prod/notification-api/database/uri" }, + { + "name": "SQLALCHEMY_DATABASE_URI_READ", + "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/prod/notification-api/database/uri_read" + }, { "name": "ADMIN_CLIENT_SECRET", "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/prod/notification-api/admin-client-secret" diff --git a/cd/application-deployment/prod/vaec-celery-beat-task-definition.json b/cd/application-deployment/prod/vaec-celery-beat-task-definition.json index af7fe79199..8946b24a77 100644 --- a/cd/application-deployment/prod/vaec-celery-beat-task-definition.json +++ b/cd/application-deployment/prod/vaec-celery-beat-task-definition.json @@ -186,6 +186,10 @@ "name": "SQLALCHEMY_DATABASE_URI", "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/prod/notification-api/database/uri" }, + { + "name": "SQLALCHEMY_DATABASE_URI_READ", + "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/prod/notification-api/database/uri_read" + }, { "name": "SECRET_KEY", "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/prod/notification-api/secret-key" diff --git a/cd/application-deployment/prod/vaec-celery-task-definition.json b/cd/application-deployment/prod/vaec-celery-task-definition.json index 2b4a3ad124..2ee2ddcb36 100644 --- a/cd/application-deployment/prod/vaec-celery-task-definition.json +++ b/cd/application-deployment/prod/vaec-celery-task-definition.json @@ -198,6 +198,10 @@ "name": "SQLALCHEMY_DATABASE_URI", "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/prod/notification-api/database/uri" }, + { + "name": "SQLALCHEMY_DATABASE_URI_READ", + "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/prod/notification-api/database/uri_read" + }, { "name": "SECRET_KEY", "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/prod/notification-api/secret-key" diff --git a/cd/application-deployment/prod/vaec-db-downgrade-task-definition.json b/cd/application-deployment/prod/vaec-db-downgrade-task-definition.json index 9d47fb0c6b..b170d5161e 100644 --- a/cd/application-deployment/prod/vaec-db-downgrade-task-definition.json +++ b/cd/application-deployment/prod/vaec-db-downgrade-task-definition.json @@ -49,6 +49,10 @@ "name": "SQLALCHEMY_DATABASE_URI", "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/prod/notification-api/database/uri" }, + { + "name": "SQLALCHEMY_DATABASE_URI_READ", + "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/prod/notification-api/database/uri_read" + }, { "name": "SECRET_KEY", "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/prod/notification-api/secret-key" diff --git a/cd/application-deployment/prod/vaec-db-migrations-task-definition.json b/cd/application-deployment/prod/vaec-db-migrations-task-definition.json index ff59f041da..ae1c0b001b 100644 --- a/cd/application-deployment/prod/vaec-db-migrations-task-definition.json +++ b/cd/application-deployment/prod/vaec-db-migrations-task-definition.json @@ -49,6 +49,10 @@ "name": "SQLALCHEMY_DATABASE_URI", "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/prod/notification-api/database/uri" }, + { + "name": "SQLALCHEMY_DATABASE_URI_READ", + "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/prod/notification-api/database/uri_read" + }, { "name": "SECRET_KEY", "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/prod/notification-api/secret-key" diff --git a/cd/application-deployment/staging/vaec-api-task-definition.json b/cd/application-deployment/staging/vaec-api-task-definition.json index 321ca8e828..8e7d4cd2de 100644 --- a/cd/application-deployment/staging/vaec-api-task-definition.json +++ b/cd/application-deployment/staging/vaec-api-task-definition.json @@ -236,6 +236,10 @@ "name": "SQLALCHEMY_DATABASE_URI", "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/staging/notification-api/database/uri" }, + { + "name": "SQLALCHEMY_DATABASE_URI_READ", + "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/staging/notification-api/database/uri_read" + }, { "name": "ADMIN_CLIENT_SECRET", "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/staging/notification-api/admin-client-secret" diff --git a/cd/application-deployment/staging/vaec-celery-beat-task-definition.json b/cd/application-deployment/staging/vaec-celery-beat-task-definition.json index a7d1462ac7..7efc862026 100644 --- a/cd/application-deployment/staging/vaec-celery-beat-task-definition.json +++ b/cd/application-deployment/staging/vaec-celery-beat-task-definition.json @@ -202,6 +202,10 @@ "name": "SQLALCHEMY_DATABASE_URI", "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/staging/notification-api/database/uri" }, + { + "name": "SQLALCHEMY_DATABASE_URI_READ", + "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/staging/notification-api/database/uri_read" + }, { "name": "SECRET_KEY", "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/staging/notification-api/secret-key" diff --git a/cd/application-deployment/staging/vaec-celery-task-definition.json b/cd/application-deployment/staging/vaec-celery-task-definition.json index d56fad3855..0d9c3a6900 100644 --- a/cd/application-deployment/staging/vaec-celery-task-definition.json +++ b/cd/application-deployment/staging/vaec-celery-task-definition.json @@ -214,6 +214,10 @@ "name": "SQLALCHEMY_DATABASE_URI", "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/staging/notification-api/database/uri" }, + { + "name": "SQLALCHEMY_DATABASE_URI_READ", + "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/staging/notification-api/database/uri_read" + }, { "name": "SECRET_KEY", "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/staging/notification-api/secret-key" diff --git a/cd/application-deployment/staging/vaec-db-downgrade-task-definition.json b/cd/application-deployment/staging/vaec-db-downgrade-task-definition.json index 62d39d053a..bcfa4dfb49 100644 --- a/cd/application-deployment/staging/vaec-db-downgrade-task-definition.json +++ b/cd/application-deployment/staging/vaec-db-downgrade-task-definition.json @@ -45,6 +45,10 @@ "name": "SQLALCHEMY_DATABASE_URI", "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/staging/notification-api/database/uri" }, + { + "name": "SQLALCHEMY_DATABASE_URI_READ", + "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/staging/notification-api/database/uri_read" + }, { "name": "SECRET_KEY", "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/staging/notification-api/secret-key" diff --git a/cd/application-deployment/staging/vaec-db-migrations-task-definition.json b/cd/application-deployment/staging/vaec-db-migrations-task-definition.json index 57edb851e6..9890e8478c 100644 --- a/cd/application-deployment/staging/vaec-db-migrations-task-definition.json +++ b/cd/application-deployment/staging/vaec-db-migrations-task-definition.json @@ -45,6 +45,10 @@ "name": "SQLALCHEMY_DATABASE_URI", "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/staging/notification-api/database/uri" }, + { + "name": "SQLALCHEMY_DATABASE_URI_READ", + "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/staging/notification-api/database/uri_read" + }, { "name": "SECRET_KEY", "valueFrom": "arn:aws-us-gov:ssm:us-gov-west-1:171875617347:parameter/staging/notification-api/secret-key" diff --git a/ci/.docker-env.example b/ci/.docker-env.example index 291cce6d6e..d37f7db240 100644 --- a/ci/.docker-env.example +++ b/ci/.docker-env.example @@ -2,6 +2,7 @@ TWILIO_ACCOUNT_SID=add value TWILIO_AUTH_TOKEN=add value SQLALCHEMY_DATABASE_URI=postgresql://postgres:LocalPassword@db:5432/notification_api +SQLALCHEMY_DATABASE_URI_READ=postgresql://postgres:LocalPassword@db:5432/notification_api NOTIFY_ENVIRONMENT=development FLASK_APP=application.py @@ -45,4 +46,4 @@ GITHUB_CLIENT_ID=fake-id GITHUB_CLIENT_SECRET=fake-secret REDIS_ENABLED='True' -REDIS_URL=redis://localhost:6379/0 +REDIS_URL=redis://redis:6379/0 diff --git a/ci/README.md b/ci/README.md new file mode 100644 index 0000000000..d27dc3af14 --- /dev/null +++ b/ci/README.md @@ -0,0 +1,119 @@ +# Testing Write and Read instance + +## Fake Replication Setup +NOTE: This setup is enough to test basic route functions with Read & Write instances. Scroll down if you need fully functional / real time replication between read and write instances. +#### 1. Setup Docker-Compose +Update local environment in your `.docker-env`. You should have Read & Write URIs pointing to different instances. +``` +SQLALCHEMY_DATABASE_URI=postgresql://postgres:LocalPassword@db:5432/notification_api +SQLALCHEMY_DATABASE_URI_READ=postgresql://postgres:LocalPassword@db-read:5432/notification_api +``` +Start containers +``` +docker-compose -f ci/docker-compose-local-readwrite.yml up -d +``` + +#### 2. Create snapshot of the write database instance +``` +pg_dump -U postgres -W -F t notification_api > /var/db-data/writedb.dump +``` + +#### 3. Copy snapshot of the write database instance into read instance's mounted volume on your system +``` +cp ci/db-data/writedb.dump ci/db-data-read/writedb.dump +``` + +#### 4. Restore data from write instance on read instance +``` +pg_restore -U postgres -W -d notification_api /var/db-data-read/writedb.dump +``` + +#### 5. Stop containers & remove data when done testing +``` +docker-compose -f ci/docker-compose-local-readwrite.yml down -v +rm -fr db-data* +``` + +## Real Replication Setup +Note: use if you need to test real time synchronization +1. run docker compose file listed below +2. take back up from an instance that has volume mounted +``` +pg_basebackup -h write-db -D /var/lib/postgresql/data_sync -U replicator -P -R -X stream +``` +1. stop only **read-db** +2. replace contents of **data-read** directory with contents of **data-sync** +3. start **read-db** +###### setup-replica.sh +``` +#!/bin/bash +set -e + +cat >> ${PGDATA}/postgresql.conf <> ${PGDATA}/postgresql.conf <> ${PGDATA}/pg_hba.conf + +pg_ctl reload +``` + +##### docker-compose.yml +``` +services: + write-db: + image: postgres:13 # adjust to the version you need + container_name: write-db + environment: + POSTGRES_USER: + POSTGRES_PASSWORD: + POSTGRES_DB: notification_api + volumes: + - ./pgdata-write:/var/lib/postgresql/data + - ./pgdata-sync:/var/lib/postgresql/data_sync + - ./setup-replication.sh:/docker-entrypoint-initdb.d/setup-replication.sh + ports: + - "5432:5432" + command: ["postgres", "-c", "log_statement=all"] + + read-db: + image: postgres:13 # adjust to the version you need + container_name: read-db + depends_on: + - write-db + environment: + POSTGRES_USER: + POSTGRES_PASSWORD: + POSTGRES_DB: notification_api + volumes: + - ./pgdata-read:/var/lib/postgresql/data + - ./setup-replica.sh:/docker-entrypoint-initdb.d/setup-replica.sh + ports: + - "5433:5432" + links: + - write-db + command: ["postgres", "-c", "log_statement=all"] +``` \ No newline at end of file diff --git a/ci/docker-compose-local-readwrite.yml b/ci/docker-compose-local-readwrite.yml new file mode 100644 index 0000000000..e53bee2896 --- /dev/null +++ b/ci/docker-compose-local-readwrite.yml @@ -0,0 +1,100 @@ +version: "3" +services: + app: + build: + context: .. + dockerfile: ci/Dockerfile.local + image: notification_api + volumes: + - "../:/app:ro" + ports: + - 6011:6011 + env_file: + - .docker-env + depends_on: + - migrations + - localstack + + db: + image: postgres:15 + restart: unless-stopped + ports: + - 5432:5432 + environment: + - POSTGRES_PASSWORD=LocalPassword + - POSTGRES_DB=notification_api + volumes: + - ./db-data:/var/db-data + + db-read: + image: postgres:15 + restart: unless-stopped + ports: + - 5433:5432 + environment: + - POSTGRES_PASSWORD=LocalPassword + - POSTGRES_DB=notification_api + volumes: + - ./db-data-read:/var/db-data-read + + migrations: + image: notification_api + env_file: + - .docker-env + volumes: + - "../:/app:ro" + command: ["bash", "-c", "flask db upgrade"] + depends_on: + - db + + localstack: + image: localstack/localstack + container_name: localstack + ports: + - "4566:4566" + env_file: + - .docker-env + environment: + - SERVICES=ec2,iam,lambda,s3,ses,sqs,sts + - DEFAULT_REGION=us-east-2 + - DATA_DIR=/tmp/localstack/data + - START_WEB=0 + - HOSTNAME_EXTERNAL=localstack + volumes: + - './localstack_setup.sh:/docker-entrypoint-initaws.d/setup.sh' + + celery: + image: notification_api + env_file: + - .docker-env + volumes: + - "../:/app:ro" + command: > + sh -c "/app/scripts/wait_for_it.sh localstack:4566 --timeout=30 -- /app/scripts/run_celery.sh" + depends_on: + - app + - localstack + - mountebank + healthcheck: + test: ["CMD", "sh", "-c", "/app/scripts/check_celery.sh"] + interval: 30s + timeout: 10s + retries: 5 + + mountebank: + container_name: mountebank + image: bbyars/mountebank:2.4.0 + volumes: + - ../scripts/mountebank:/mountebank + ports: + - 2525:2525 + - 7005:7005 + - 7006:7006 + - 7007:7007 + - 7008:7008 + command: start --configfile /mountebank/imposters.ejs --allowInjection + + redis: + image: redis + ports: + - "6379:6379" diff --git a/ci/docker-compose-local.yml b/ci/docker-compose-local.yml index a92ec25468..c74a5358d6 100644 --- a/ci/docker-compose-local.yml +++ b/ci/docker-compose-local.yml @@ -14,6 +14,7 @@ services: depends_on: - migrations - localstack + db: image: postgres:15 restart: unless-stopped @@ -22,6 +23,7 @@ services: environment: - POSTGRES_PASSWORD=LocalPassword - POSTGRES_DB=notification_api + migrations: image: notification_api env_file: @@ -31,6 +33,7 @@ services: command: ["bash", "-c", "flask db upgrade"] depends_on: - db + localstack: image: localstack/localstack container_name: localstack @@ -46,6 +49,7 @@ services: - HOSTNAME_EXTERNAL=localstack volumes: - './localstack_setup.sh:/docker-entrypoint-initaws.d/setup.sh' + celery: image: notification_api env_file: @@ -63,6 +67,7 @@ services: interval: 30s timeout: 10s retries: 5 + mountebank: container_name: mountebank image: bbyars/mountebank:2.4.0 @@ -75,9 +80,8 @@ services: - 7007:7007 - 7008:7008 command: start --configfile /mountebank/imposters.ejs --allowInjection + redis: image: redis ports: - "6379:6379" - - diff --git a/ci/docker-compose-test.yml b/ci/docker-compose-test.yml index c37b20ac54..f005d50e40 100644 --- a/ci/docker-compose-test.yml +++ b/ci/docker-compose-test.yml @@ -22,6 +22,7 @@ services: - NOTIFY_ENVIRONMENT=test - RETRY_SQS_URL=RETRY_SQS_URL - SQLALCHEMY_DATABASE_URI=postgresql://postgres:LocalPassword@db:5432/test_notification_api + - SQLALCHEMY_DATABASE_URI_READ=postgresql://postgres:LocalPassword@db:5432/test_notification_api - TIMEOUT=10 - VA_PROFILE_DOMAIN=int.vaprofile.va.gov - VETEXT_API_AUTH_SSM_PATH=test diff --git a/ci/docker-compose.yml b/ci/docker-compose.yml index acb90b0b85..4e97ca7ab0 100644 --- a/ci/docker-compose.yml +++ b/ci/docker-compose.yml @@ -10,6 +10,7 @@ services: - TWILIO_ACCOUNT_SID= - TWILIO_AUTH_TOKEN= - SQLALCHEMY_DATABASE_URI=postgresql://postgres:LocalPassword@db:5432/notification_api + - SQLALCHEMY_DATABASE_URI_READ=postgresql://postgres:LocalPassword@db:5432/notification_api - NOTIFY_ENVIRONMENT=development - FLASK_APP=application.py depends_on: @@ -30,6 +31,7 @@ services: - TWILIO_ACCOUNT_SID= - TWILIO_AUTH_TOKEN= - SQLALCHEMY_DATABASE_URI=postgresql://postgres:LocalPassword@db:5432/notification_api + - SQLALCHEMY_DATABASE_URI_READ=postgresql://postgres:LocalPassword@db:5432/notification_api - NOTIFY_ENVIRONMENT=development - FLASK_APP=application.py depends_on: diff --git a/tests/README.md b/tests/README.md index c086afacab..4b546825f9 100644 --- a/tests/README.md +++ b/tests/README.md @@ -9,9 +9,9 @@ The docker-compose command used to run the full test suite sets environment vari ## Setup 1. Stop all running containers associated with Notification-api. -2. Start the Postgres (ci_db_1) container, and any other containers required by the functionality under test: `docker start ci_db_1`. All migrations should already be applied. -3. Start a test container shell by running `docker run --rm -it -v ":/app" --env-file tests/env_vars ci_test bash`. -4. Add the test container started in the previous step to the default network: `docker network connect ci_default `. +2. Start the Postgres (ci_db_1) container, and any other containers required by the functionality under test: `docker start ci-db-1`. All migrations should already be applied. +3. Start a test container shell by running `docker run --rm -it -v ":/app" --env-file tests/env_vars --name ci-test ci-test bash`. +4. Add the test container started in the previous step to the default network: `docker network connect ci_default ci-test`. 5. In the test container shell, run `pytest -h` to see the syntax for running tests. Without flags, you can run `pytest [file or directory]...`. ## Running Individual Tests diff --git a/tests/app/authentication/test_authentication.py b/tests/app/authentication/test_authentication.py index 9e9ce42055..e4b7774b16 100644 --- a/tests/app/authentication/test_authentication.py +++ b/tests/app/authentication/test_authentication.py @@ -6,7 +6,7 @@ from flask_jwt_extended import create_access_token from jwt import ExpiredSignatureError -from app.dao.services_dao import dao_add_user_to_service +from app.dao.services_dao import dao_add_user_to_service, dao_update_service from tests.app.db import create_user, create_service from tests.conftest import set_config_values @@ -16,7 +16,8 @@ from notifications_python_client.authentication import create_jwt_token from app import api_user -from app.dao.api_key_dao import get_unsigned_secrets, save_model_api_key, get_unsigned_secret, expire_api_key +from app.dao.api_key_dao import get_unsigned_secrets,\ + save_model_api_key, get_unsigned_secret, expire_api_key from app.models import ApiKey, KEY_TYPE_NORMAL, PERMISSION_LIST, Permission from app.authentication.auth import AuthError, validate_admin_auth, validate_service_api_key_auth, \ requires_admin_auth_or_user_in_service, requires_user_in_service_or_admin @@ -278,8 +279,10 @@ def test_authentication_returns_error_when_service_doesnt_exit( 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)) + # we now need to save the model because we'll read using read-db engine + dao_update_service(sample_api_key.service) + token = create_jwt_token(secret=str(sample_api_key.secret), client_id=str(sample_api_key.service_id)) response = client.get('/notifications', headers={'Authorization': 'Bearer {}'.format(token)}) assert response.status_code == 403 @@ -301,21 +304,23 @@ def test_authentication_returns_error_when_service_has_no_secrets(client, assert exc.value.service_id == sample_service.id -def test_should_attach_the_current_api_key_to_current_app(notify_api, sample_service, sample_api_key): +def test_should_attach_the_current_api_key_to_current_app(notify_api, sample_service_data_api_key): with notify_api.test_request_context(), notify_api.test_client() as client: - token = __create_token(sample_api_key.service_id) + _key = sample_service_data_api_key + token = create_jwt_token(secret=_key.secret, client_id=str(_key.service_id)) response = client.get( '/notifications', headers={'Authorization': 'Bearer {}'.format(token)} ) assert response.status_code == 200 - assert api_user == sample_api_key + # api_user is an api key assigned globally after a service is authenticated + assert api_user == _key def test_should_return_403_when_token_is_expired(client, sample_api_key): with freeze_time('2001-01-01T12:00:00'): - token = __create_token(sample_api_key.service_id) + token = create_jwt_token(secret=sample_api_key.secret, client_id=str(sample_api_key.service_id)) with freeze_time('2001-01-01T12:00:40'): with pytest.raises(AuthError) as exc: request.headers = {'Authorization': 'Bearer {}'.format(token)} diff --git a/tests/app/conftest.py b/tests/app/conftest.py index d7ccfc0bcd..e154589d5d 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -1,5 +1,6 @@ import json import os +from app.service.service_data import ServiceDataApiKey import pytest import pytz import requests_mock @@ -581,6 +582,23 @@ def sample_api_key(notify_db, return api_key +@pytest.fixture(scope='function') +def sample_service_data_api_key(service=None, key_type=KEY_TYPE_NORMAL, name=None): + if service is None: + service = create_service(check_if_service_exists=True) + + data = { + 'service': service, + 'name': name or uuid.uuid4(), + 'created_by': service.created_by, + 'key_type': key_type + } + api_key = ApiKey(**data) + save_model_api_key(api_key) + + return ServiceDataApiKey(api_key) + + @pytest.fixture(scope='function') def sample_test_api_key(sample_api_key): service = create_service(check_if_service_exists=True) diff --git a/tests/app/notifications/test_receive_notification.py b/tests/app/notifications/test_receive_notification.py index 6410b5cdaf..301ec74b72 100644 --- a/tests/app/notifications/test_receive_notification.py +++ b/tests/app/notifications/test_receive_notification.py @@ -12,7 +12,6 @@ format_mmg_datetime, create_inbound_sms_object, strip_leading_forty_four, - has_inbound_sms_permissions, unescape_string, fetch_potential_service, NoSuitableServiceForInboundSms, ) @@ -100,7 +99,7 @@ def test_receive_notification_returns_received_to_mmg(client, mocker, sample_ser ]) def test_check_permissions_for_inbound_sms(notify_db, notify_db_session, permissions, expected_response): service = create_service(service_permissions=permissions) - assert has_inbound_sms_permissions(service.permissions) is expected_response + assert service.has_permissions([INBOUND_SMS_TYPE, SMS_TYPE]) is expected_response @pytest.mark.skip(reason="Endpoint disabled and slated for removal") @@ -722,9 +721,13 @@ def test_should_raise_if_no_matching_service(self, notify_api, mocker): fetch_potential_service('some-inbound-number', 'some-provider-name') def test_should_raise_if_service_doesnt_have_permission(self, notify_api, mocker): + # make mocked service execute original code + # just mocking service won't let us execute .has_permissions + # method properly + mock_service_instance = Service(permissions=[]) mocker.patch( 'app.notifications.receive_notifications.dao_fetch_service_by_inbound_number', - return_value=mocker.Mock(Service, permissions=[]) + return_value=mock_service_instance ) with pytest.raises(NoSuitableServiceForInboundSms): diff --git a/tests/app/test_cloudfoundry_config.py b/tests/app/test_cloudfoundry_config.py index ccf238d49c..2304673a44 100644 --- a/tests/app/test_cloudfoundry_config.py +++ b/tests/app/test_cloudfoundry_config.py @@ -36,6 +36,7 @@ def test_extract_cloudfoundry_config_populates_other_vars(): extract_cloudfoundry_config() assert os.environ['SQLALCHEMY_DATABASE_URI'] == 'postgres uri' + assert os.environ['SQLALCHEMY_DATABASE_URI_READ'] == 'postgres uri' assert os.environ['NOTIFY_ENVIRONMENT'] == '🚀🌌' assert os.environ['NOTIFY_LOG_PATH'] == '/home/vcap/logs/app.log' diff --git a/tests/env_vars b/tests/env_vars index 337b4d13e7..dce1cf72f7 100644 --- a/tests/env_vars +++ b/tests/env_vars @@ -1,6 +1,7 @@ # https://docs.docker.com/compose/env-file/ SQLALCHEMY_DATABASE_URI=postgresql://postgres:LocalPassword@db:5432/test_notification_api +SQLALCHEMY_DATABASE_URI_READ=postgresql://postgres:LocalPassword@db:5432/test_notification_api NOTIFY_ENVIRONMENT=test AWS_ACCESS_KEY_ID=test AWS_PINPOINT_APP_ID=AWS_PINPOINT_APP_ID