Merge pull request #811 from alphagov/check-service-is-active

Check service is active

app/celery/tasks.py

@@ -25,8 +25,8 @@ from app.models import (
     EMAIL_TYPE,
     SMS_TYPE,
     LETTER_TYPE,
-    KEY_TYPE_NORMAL
-)
+    KEY_TYPE_NORMAL,
+    JOB_STATUS_CANCELLED, JOB_STATUS_PENDING, JOB_STATUS_IN_PROGRESS, JOB_STATUS_FINISHED)
 from app.notifications.process_notifications import persist_notification
 from app.service.utils import service_allowed_to_send_to
 from app.statsd_decorators import statsd
@@ -38,15 +38,22 @@ def process_job(job_id):
     start = datetime.utcnow()
     job = dao_get_job_by_id(job_id)
 
-    if job.job_status != 'pending':
+    if job.job_status != JOB_STATUS_PENDING:
         return
 
     service = job.service
 
+    if not service.active:
+        job.job_status = JOB_STATUS_CANCELLED
+        dao_update_job(job)
+        current_app.logger.warn(
+            "Job {} has been cancelled, service {} is inactive".format(job_id, service.id))
+        return
+
     if __sending_limits_for_job_exceeded(service, job, job_id):
         return
 
-    job.job_status = 'in progress'
+    job.job_status = JOB_STATUS_IN_PROGRESS
     dao_update_job(job)
 
     db_template = dao_get_template_by_id(job.template_id, job.template_version)
@@ -62,7 +69,7 @@ def process_job(job_id):
         process_row(row_number, recipient, personalisation, template, job, service)
 
     finished = datetime.utcnow()
-    job.job_status = 'finished'
+    job.job_status = JOB_STATUS_FINISHED
     job.processing_started = start
     job.processing_finished = finished
     dao_update_job(job)

app/dao/permissions_dao.py

@@ -59,7 +59,8 @@ class PermissionDAO(DAOClass):
                 db.session.commit()
 
     def get_permissions_by_user_id(self, user_id):
-        return self.Meta.model.query.filter_by(user_id=user_id).all()
+        return self.Meta.model.query.filter_by(user_id=user_id)\
+                                    .join(Permission.service).filter_by(active=True).all()
 
 
 permission_dao = PermissionDAO()

app/delivery/send_to_providers.py

@@ -13,19 +13,26 @@ from app.dao.provider_details_dao import (
     get_provider_details_by_notification_type,
     dao_toggle_sms_provider
 )
-from app.dao.services_dao import dao_fetch_service_by_id
 from app.celery.research_mode_tasks import send_sms_response, send_email_response
 from app.dao.templates_dao import dao_get_template_by_id
-from app.models import SMS_TYPE, KEY_TYPE_TEST, BRANDING_ORG, EMAIL_TYPE
+from app.models import SMS_TYPE, KEY_TYPE_TEST, BRANDING_ORG, EMAIL_TYPE, NOTIFICATION_TECHNICAL_FAILURE
 
 
 def send_sms_to_provider(notification):
-    service = dao_fetch_service_by_id(notification.service_id)
-    provider = provider_to_use(SMS_TYPE, notification.id)
-    current_app.logger.info(
-        "Starting sending SMS {} to provider at {}".format(notification.id, datetime.utcnow())
-    )
+    service = notification.service
+    if not service.active:
+        notification.status = NOTIFICATION_TECHNICAL_FAILURE
+        dao_update_notification(notification)
+        current_app.logger.warn(
+            "Send sms for notification id {} to provider is not allowed: service {} is inactive".format(notification.id,
+                                                                                                        service.id))
+        return
+
     if notification.status == 'created':
+        provider = provider_to_use(SMS_TYPE, notification.id)
+        current_app.logger.info(
+            "Starting sending SMS {} to provider at {}".format(notification.id, datetime.utcnow())
+        )
         template_model = dao_get_template_by_id(notification.template_id, notification.template_version)
         template = SMSMessageTemplate(
             template_model.__dict__,
@@ -61,12 +68,20 @@ def send_sms_to_provider(notification):
 
 
 def send_email_to_provider(notification):
-    service = dao_fetch_service_by_id(notification.service_id)
-    provider = provider_to_use(EMAIL_TYPE, notification.id)
-    current_app.logger.info(
-        "Starting sending EMAIL {} to provider at {}".format(notification.id, datetime.utcnow())
-    )
+    service = notification.service
+    if not service.active:
+        notification.status = NOTIFICATION_TECHNICAL_FAILURE
+        dao_update_notification(notification)
+        current_app.logger.warn(
+            "Send email for notification id {} to provider is not allowed: service {} is inactive".format(
+                notification.id,
+                service.id))
+        return
     if notification.status == 'created':
+        provider = provider_to_use(EMAIL_TYPE, notification.id)
+        current_app.logger.info(
+            "Starting sending EMAIL {} to provider at {}".format(notification.id, datetime.utcnow())
+        )
         template_dict = dao_get_template_by_id(notification.template_id, notification.template_version).__dict__
 
         html_email = HTMLEmailTemplate(

app/job/rest.py

@@ -110,7 +110,9 @@ def get_jobs_by_service(service_id):
 
 @job.route('', methods=['POST'])
 def create_job(service_id):
-    dao_fetch_service_by_id(service_id)
+    service = dao_fetch_service_by_id(service_id)
+    if not service.active:
+        raise InvalidRequest("Create job is not allowed: service is inactive ", 403)
 
     data = request.get_json()
 

app/service/rest.py

@@ -353,20 +353,6 @@ def update_whitelist(service_id):
         return '', 204
 
 
-# Renaming this endpoint to archive
-@service_blueprint.route('/<uuid:service_id>/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_archive_service(service.id)
-
-    return '', 204
-
-
 @service_blueprint.route('/<uuid:service_id>/archive', methods=['POST'])
 def archive_service(service_id):
     """

tests/app/celery/test_tasks.py

@@ -923,6 +923,22 @@ def test_persist_letter_saves_letter_to_database(sample_letter_job, mocker):
     assert notification_db.personalisation == personalisation
 
 
+def test_should_cancel_job_if_service_is_inactive(sample_service,
+                                                  sample_job,
+                                                  mocker):
+    sample_service.active = False
+
+    mocker.patch('app.celery.tasks.s3.get_job_from_s3')
+    mocker.patch('app.celery.tasks.process_row')
+
+    process_job(sample_job.id)
+
+    job = jobs_dao.dao_get_job_by_id(sample_job.id)
+    assert job.job_status == 'cancelled'
+    s3.get_job_from_s3.assert_not_called()
+    tasks.process_row.assert_not_called()
+
+
 @pytest.mark.parametrize('template_type, expected_class', [
     (SMS_TYPE, SMSMessageTemplate),
     (EMAIL_TYPE, WithSubjectTemplate),

tests/app/conftest.py

@@ -121,7 +121,6 @@ def sample_service(notify_db,
                    notify_db_session,
                    service_name="Sample service",
                    user=None,
-                   active=True,
                    restricted=False,
                    limit=1000,
                    email_from=None):
@@ -132,7 +131,6 @@ def sample_service(notify_db,
     data = {
         'name': service_name,
         'message_limit': limit,
-        'active': active,
         'restricted': restricted,
         'email_from': email_from,
         'created_by': user

tests/app/dao/test_permissionDAO.py

@@ -0,0 +1,26 @@
+from app.dao.permissions_dao import permission_dao
+from tests.app.conftest import sample_service as create_service
+
+
+def test_get_permissions_by_user_id_returns_all_permissions(sample_service):
+    permissions = permission_dao.get_permissions_by_user_id(user_id=sample_service.users[0].id)
+    assert len(permissions) == 8
+    assert sorted(["manage_users",
+                   "manage_templates",
+                   "manage_settings",
+                   "send_texts",
+                   "send_emails",
+                   "send_letters",
+                   "manage_api_keys",
+                   "view_activity"]) == sorted([i.permission for i in permissions])
+
+
+def test_get_permissions_by_user_id_returns_only_active_service(notify_db, notify_db_session, sample_user):
+    active_service = create_service(notify_db, notify_db_session, service_name="Active service", user=sample_user)
+    inactive_service = create_service(notify_db, notify_db_session, service_name="Inactive service",
+                                      user=sample_user)
+    inactive_service.active = False
+    permissions = permission_dao.get_permissions_by_user_id(user_id=sample_user.id)
+    assert len(permissions) == 8
+    assert active_service in [i.service for i in permissions]
+    assert inactive_service not in [i.service for i in permissions]

tests/app/delivery/test_send_to_providers.py

@@ -117,6 +117,20 @@ def test_should_send_personalised_template_to_correct_email_provider_and_persist
     assert notification.personalisation == {"name": "Jo"}
 
 
+@pytest.mark.parametrize("client_send",
+                         ["app.aws_ses_client.send_email",
+                          "app.mmg_client.send_sms",
+                          "app.firetext_client.send_sms"])
+def test_should_not_send_message_when_service_is_inactive_notiifcation_is_in_tech_failure(
+        sample_service, sample_notification, mocker, client_send):
+    sample_service.active = False
+    send_mock = mocker.patch(client_send, return_value='reference')
+
+    send_to_providers.send_email_to_provider(sample_notification)
+    send_mock.assert_not_called()
+    assert Notification.query.get(sample_notification.id).status == 'technical-failure'
+
+
 def test_send_sms_should_use_template_version_from_notification_not_latest(
         notify_db,
         notify_db_session,

tests/app/job/test_rest.py

@@ -170,6 +170,21 @@ def test_create_scheduled_job(notify_api, sample_template, mocker, fake_uuid):
                 assert resp_json['data']['original_file_name'] == 'thisisatest.csv'
 
 
+def test_create_job_returns_403_if_service_is_not_active(client, fake_uuid, sample_service, mocker):
+    sample_service.active = False
+    mock_job_dao = mocker.patch("app.dao.jobs_dao.dao_create_job")
+    auth_header = create_authorization_header()
+    response = client.post('/service/{}/job'.format(sample_service.id),
+                           data="",
+                           headers=[('Content-Type', 'application/json'), auth_header])
+
+    assert response.status_code == 403
+    resp_json = json.loads(response.get_data(as_text=True))
+    assert resp_json['result'] == 'error'
+    assert resp_json['message'] == "Create job is not allowed: service is inactive "
+    mock_job_dao.assert_not_called()
+
+
 def test_should_not_create_scheduled_job_more_then_24_hours_hence(notify_api, sample_template, mocker, fake_uuid):
     with notify_api.test_request_context():
         with notify_api.test_client() as client:

tests/app/service/test_deactivate.py

@@ -1,123 +0,0 @@
-import uuid
-from datetime import datetime
-
-import pytest
-from freezegun import freeze_time
-
-from app import db
-from app.models import Service
-from app.dao.services_dao import dao_archive_service
-from app.dao.api_key_dao import expire_api_key
-from app.dao.templates_dao import dao_update_template
-
-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
-
-
-@pytest.fixture
-def deactivated_service_with_deleted_stuff(client, notify_db, notify_db_session, sample_service):
-    with freeze_time('2001-01-01'):
-        template = create_template(notify_db, notify_db_session, template_name='a')
-        api_key = create_api_key(notify_db, notify_db_session)
-
-        expire_api_key(sample_service.id, api_key.id)
-
-        template.archived = True
-        dao_update_template(template)
-
-    with freeze_time('2002-02-02'):
-        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_doesnt_affect_existing_archived_templates(deactivated_service_with_deleted_stuff):
-    assert deactivated_service_with_deleted_stuff.templates[0].archived is True
-    assert deactivated_service_with_deleted_stuff.templates[0].updated_at == datetime(2001, 1, 1, 0, 0, 0)
-    assert deactivated_service_with_deleted_stuff.templates[0].version == 2
-
-
-def test_deactivating_service_doesnt_affect_existing_revoked_api_keys(deactivated_service_with_deleted_stuff):
-    assert deactivated_service_with_deleted_stuff.api_keys[0].expiry_date == datetime(2001, 1, 1, 0, 0, 0)
-    assert deactivated_service_with_deleted_stuff.api_keys[0].version == 2
-
-
-def test_deactivating_service_rolls_back_everything_on_error(sample_service, sample_api_key, sample_template):
-    unwrapped_deactive_service = unwrap_function(dao_archive_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