Cancel job if the service is inactive.

Update the PermissionsDao.get_permissions_by_user_id to only return permissions for active services,
this will make the admin app return a 403 if someone (otherthan platform admin) tries to look at an inactive service.
Removed the active flag in sample_service the dao_create_service overiddes this attribute.
This commit is contained in:
Rebecca Law
2017-02-02 11:34:00 +00:00
parent dab89969cc
commit f56824adde
5 changed files with 56 additions and 8 deletions

View File

@@ -25,8 +25,8 @@ from app.models import (
EMAIL_TYPE, EMAIL_TYPE,
SMS_TYPE, SMS_TYPE,
LETTER_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.notifications.process_notifications import persist_notification
from app.service.utils import service_allowed_to_send_to from app.service.utils import service_allowed_to_send_to
from app.statsd_decorators import statsd from app.statsd_decorators import statsd
@@ -38,15 +38,22 @@ def process_job(job_id):
start = datetime.utcnow() start = datetime.utcnow()
job = dao_get_job_by_id(job_id) job = dao_get_job_by_id(job_id)
if job.job_status != 'pending': if job.job_status != JOB_STATUS_PENDING:
return return
service = job.service 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): if __sending_limits_for_job_exceeded(service, job, job_id):
return return
job.job_status = 'in progress' job.job_status = JOB_STATUS_IN_PROGRESS
dao_update_job(job) dao_update_job(job)
db_template = dao_get_template_by_id(job.template_id, job.template_version) 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) process_row(row_number, recipient, personalisation, template, job, service)
finished = datetime.utcnow() finished = datetime.utcnow()
job.job_status = 'finished' job.job_status = JOB_STATUS_FINISHED
job.processing_started = start job.processing_started = start
job.processing_finished = finished job.processing_finished = finished
dao_update_job(job) dao_update_job(job)

View File

@@ -59,7 +59,8 @@ class PermissionDAO(DAOClass):
db.session.commit() db.session.commit()
def get_permissions_by_user_id(self, user_id): 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() permission_dao = PermissionDAO()

View File

@@ -923,6 +923,22 @@ def test_persist_letter_saves_letter_to_database(sample_letter_job, mocker):
assert notification_db.personalisation == personalisation 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', [ @pytest.mark.parametrize('template_type, expected_class', [
(SMS_TYPE, SMSMessageTemplate), (SMS_TYPE, SMSMessageTemplate),
(EMAIL_TYPE, WithSubjectTemplate), (EMAIL_TYPE, WithSubjectTemplate),

View File

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

View File

@@ -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]