Skip to content

Commit

Permalink
Fix/template-platform-admin-check (#1909)
Browse files Browse the repository at this point in the history 
* fix(create template): update security check for when FF is ON

* test: Add tests for creating email templates with different process types

* test: update template_folders tests

* test: update templates tests

---------

Co-authored-by: Jumana B <[email protected]>
Co-authored-by: William B <[email protected]>
  • Loading branch information
@andrewleith @jzbahrai @whabanks
3 people authored Jul 29, 2024
1 parent 51dbc78 commit 61aaa4e
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 14 deletions.
8 changes: 6 additions & 2 deletions app/main/views/templates.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -776,8 +776,12 @@ def add_service_template(service_id, template_type, template_folder_id=None): #
form = form_objects[template_type](**template) # TODO: remove when FF_TEMPLATE_CATEGORY removed

if form.validate_on_submit():
if form.process_type.data != TemplateProcessTypes.BULK.value:
abort_403_if_not_admin_user()
if current_app.config["FF_TEMPLATE_CATEGORY"]: # TODO: remove when FF_TEMPLATE_CATEGORY removed
if form.process_type.data != TC_PRIORITY_VALUE:
abort_403_if_not_admin_user()
else: # TODO: remove when FF_TEMPLATE_CATEGORY removed
if form.process_type.data != TemplateProcessTypes.BULK.value: # TODO: remove when FF_TEMPLATE_CATEGORY removed
abort_403_if_not_admin_user() # TODO: remove when FF_TEMPLATE_CATEGORY removed
subject = form.subject.data if hasattr(form, "subject") else None
if request.form.get("button_pressed") == "preview":
preview_template_data = {
Expand Down
5 changes: 3 additions & 2 deletions tests/app/main/views/test_template_folders.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from flask import abort, url_for
from notifications_python_client.errors import HTTPError

from app.main.forms import TC_PRIORITY_VALUE
from app.models.enum.template_process_types import TemplateProcessTypes
from app.models.service import Service
from app.models.user import User
Expand Down Expand Up @@ -385,7 +386,7 @@ def test_can_create_email_template_with_parent_folder(
"template_type": "email",
"template_category_id": TESTING_TEMPLATE_CATEGORY,
"service": SERVICE_ONE_ID,
"process_type": TemplateProcessTypes.BULK.value,
"process_type": TC_PRIORITY_VALUE if app_.config["FF_TEMPLATE_CATEGORY"] else TemplateProcessTypes.BULK.value,
"parent_folder_id": PARENT_FOLDER_ID,
}
client_request.post(
Expand All @@ -402,7 +403,7 @@ def test_can_create_email_template_with_parent_folder(
data["template_content"],
SERVICE_ONE_ID,
data["subject"],
data["process_type"],
None if app_.config["FF_TEMPLATE_CATEGORY"] else data["process_type"],
data["parent_folder_id"],
data["template_category_id"] if app_.config["FF_TEMPLATE_CATEGORY"] else None,
)
Expand Down
96 changes: 86 additions & 10 deletions tests/app/main/views/test_templates.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from freezegun import freeze_time
from notifications_python_client.errors import HTTPError

from app.main.forms import TC_PRIORITY_VALUE
from app.main.views.templates import (
delete_preview_data,
get_human_readable_delta,
Expand Down Expand Up @@ -129,7 +130,7 @@ def test_create_email_template_cat_other_to_freshdesk(
"template_type": "email",
"template_category_id": TESTING_TEMPLATE_CATEGORY,
"service": SERVICE_ONE_ID,
"process_type": DEFAULT_PROCESS_TYPE,
"process_type": None if app_.config["FF_TEMPLATE_CATEGORY"] else DEFAULT_PROCESS_TYPE,
"button_pressed": "save",
"template_category_other": "hello",
},
Expand Down Expand Up @@ -1642,15 +1643,15 @@ def test_should_not_update_if_template_name_too_long(

@pytest.mark.parametrize("template_type", ["sms", "email"])
def test_should_not_create_if_template_name_too_long(
client_request, template_type, mock_create_service_template_400_name_too_long, mock_get_template_categories
client_request, template_type, mock_create_service_template_400_name_too_long, mock_get_template_categories, app_
):
template_data = {
"name": "new name",
"template_content": "template content",
"template_type": template_type,
"template_category_id": TESTING_TEMPLATE_CATEGORY,
"service": SERVICE_ONE_ID,
"process_type": DEFAULT_PROCESS_TYPE,
"process_type": None if app_.config["FF_TEMPLATE_CATEGORY"] else DEFAULT_PROCESS_TYPE,
}
if template_type == "email":
template_data.update({"subject": "subject"})
Expand All @@ -1671,6 +1672,7 @@ def test_should_not_create_too_big_template(
mock_create_service_template_content_too_big,
mock_get_template_categories,
fake_uuid,
app_,
):
page = client_request.post(
".add_service_template",
Expand All @@ -1682,7 +1684,7 @@ def test_should_not_create_too_big_template(
"template_type": "sms",
"template_category_id": TESTING_TEMPLATE_CATEGORY,
"service": SERVICE_ONE_ID,
"process_type": DEFAULT_PROCESS_TYPE,
"process_type": None if app_.config["FF_TEMPLATE_CATEGORY"] else DEFAULT_PROCESS_TYPE,
},
_expected_status=200,
)
Expand Down Expand Up @@ -2299,6 +2301,7 @@ def test_can_create_email_template_with_emoji(
mock_get_template_folders,
mock_get_service_template_when_no_template_exists,
mock_get_template_categories,
app_,
):
page = client_request.post(
".add_service_template",
Expand All @@ -2311,7 +2314,7 @@ def test_can_create_email_template_with_emoji(
"template_type": "email",
"template_category_id": TESTING_TEMPLATE_CATEGORY,
"service": SERVICE_ONE_ID,
"process_type": DEFAULT_PROCESS_TYPE,
"process_type": None if app_.config["FF_TEMPLATE_CATEGORY"] else DEFAULT_PROCESS_TYPE,
"button_pressed": "save",
},
_follow_redirects=True,
Expand All @@ -2322,11 +2325,84 @@ def test_can_create_email_template_with_emoji(
assert flash_banner == "'new name' template saved"


def test_should_not_create_sms_template_with_emoji(
# params


@pytest.mark.parametrize(
"PRIORITY_FF_ON, PRIORITY_FF_OFF, expect_success",
[ # TODO: Remove `PRIORITY_FF_OFF`, rename `PRIORITY_FF_ON` to simply `priority`
(TC_PRIORITY_VALUE, TemplateProcessTypes.BULK.value, True),
(TemplateProcessTypes.BULK.value, TC_PRIORITY_VALUE, False),
(TemplateProcessTypes.NORMAL.value, TemplateProcessTypes.NORMAL.value, False),
(TemplateProcessTypes.PRIORITY.value, TemplateProcessTypes.PRIORITY.value, False),
],
)
def test_create_template_with_process_types(
client_request,
service_one,
mock_create_service_template,
mock_get_template_folders,
mock_get_service_template_when_no_template_exists,
mock_get_template_categories,
app_,
mocker,
PRIORITY_FF_ON,
PRIORITY_FF_OFF,
expect_success,
):
with set_config(app_, "FF_TEMPLATE_CATEGORY", False): # TODO: Remove this block when FF_TEMPLATE_CATEGORY is removed
# mock /workspaces/notification-admin/app/main/views/templates.py/abort_403_if_not_admin_user
raise_403 = mocker.patch("app.main.views.templates.abort_403_if_not_admin_user", return_value=None)

page = client_request.post(
".add_service_template",
service_id=SERVICE_ONE_ID,
template_type="email",
_data={
"name": "new name",
"subject": "Food incoming!",
"template_content": "here's a burrito 🌯",
"template_type": "email",
"template_category_id": TESTING_TEMPLATE_CATEGORY,
"service": SERVICE_ONE_ID,
"process_type": PRIORITY_FF_OFF,
"button_pressed": "save",
},
_follow_redirects=True,
)

if expect_success:
assert mock_create_service_template.called

flash_banner = page.select_one(".banner-default-with-tick").string.strip()
assert flash_banner == "'new name' template saved"
else:
assert raise_403.called

with set_config(app_, "FF_TEMPLATE_CATEGORY", True):
page = client_request.post(
".add_service_template",
service_id=SERVICE_ONE_ID,
template_type="email",
_data={
"name": "new name",
"subject": "Food incoming!",
"template_content": "here's a burrito 🌯",
"template_type": "email",
"template_category_id": TESTING_TEMPLATE_CATEGORY,
"service": SERVICE_ONE_ID,
"process_type": PRIORITY_FF_ON,
"button_pressed": "save",
},
_follow_redirects=True,
)
assert mock_create_service_template.called is True

flash_banner = page.select_one(".banner-default-with-tick").string.strip()
assert flash_banner == "'new name' template saved"


def test_should_not_create_sms_template_with_emoji(
client_request, service_one, mock_create_service_template, mock_get_template_categories, app_
):
page = client_request.post(
".add_service_template",
Expand All @@ -2338,7 +2414,7 @@ def test_should_not_create_sms_template_with_emoji(
"template_type": "sms",
"template_category_id": DEFAULT_TEMPLATE_CATEGORY_LOW,
"service": SERVICE_ONE_ID,
"process_type": DEFAULT_PROCESS_TYPE,
"process_type": None if app_.config["FF_TEMPLATE_CATEGORY"] else DEFAULT_PROCESS_TYPE,
},
_expected_status=200,
)
Expand Down Expand Up @@ -2373,7 +2449,7 @@ def test_should_not_update_sms_template_with_emoji(


def test_should_create_sms_template_without_downgrading_unicode_characters(
client_request, mock_create_service_template, mock_get_template_categories
client_request, mock_create_service_template, mock_get_template_categories, app_
):
msg = "here:\tare some “fancy quotes” and non\u200Bbreaking\u200Bspaces"

Expand All @@ -2386,7 +2462,7 @@ def test_should_create_sms_template_without_downgrading_unicode_characters(
"template_content": msg,
"template_type": "sms",
"service": SERVICE_ONE_ID,
"process_type": DEFAULT_PROCESS_TYPE,
"process_type": None if app_.config["FF_TEMPLATE_CATEGORY"] else DEFAULT_PROCESS_TYPE,
"template_category_id": TESTING_TEMPLATE_CATEGORY,
},
expected_status=302,
Expand Down

0 comments on commit 61aaa4e

Please sign in to comment.