From 40017cbb1e3f48a4226e65661aa0167a0acf99c3 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Thu, 11 Apr 2024 11:00:59 -0400 Subject: [PATCH] Unit test for email error reporting. --- lib/galaxy/app_unittest_utils/galaxy_mock.py | 2 +- .../tools/error_reports/plugins/email.py | 4 +- lib/galaxy/util/__init__.py | 1 + test/unit/app/tools/test_error_reporting.py | 141 ++++++++++++++++++ 4 files changed, 146 insertions(+), 2 deletions(-) create mode 100644 test/unit/app/tools/test_error_reporting.py diff --git a/lib/galaxy/app_unittest_utils/galaxy_mock.py b/lib/galaxy/app_unittest_utils/galaxy_mock.py index d50842158020..3de37061ebf8 100644 --- a/lib/galaxy/app_unittest_utils/galaxy_mock.py +++ b/lib/galaxy/app_unittest_utils/galaxy_mock.py @@ -213,7 +213,7 @@ def __init__(self, **kwargs): self.password_expiration_period = 0 self.pretty_datetime_format = "pretty_datetime_format" self.redact_username_in_logs = False - self.smtp_server = True + self.smtp_server = None self.terms_url = "terms_url" self.templates_dir = "templates" diff --git a/lib/galaxy/tools/error_reports/plugins/email.py b/lib/galaxy/tools/error_reports/plugins/email.py index 42446c2ecb3e..2db91db9a2b5 100644 --- a/lib/galaxy/tools/error_reports/plugins/email.py +++ b/lib/galaxy/tools/error_reports/plugins/email.py @@ -35,7 +35,9 @@ def submit_report(self, dataset, job, tool, **kwargs): ) return ("Your error report has been sent", "success") except Exception as e: - return (f"An error occurred sending the report by email: {unicodify(e)}", "danger") + msg = f"An error occurred sending the report by email: {unicodify(e)}" + log.exception(msg) + return (msg, "danger") __all__ = ("EmailPlugin",) diff --git a/lib/galaxy/util/__init__.py b/lib/galaxy/util/__init__.py index b688d633c9b8..a43da79a0802 100644 --- a/lib/galaxy/util/__init__.py +++ b/lib/galaxy/util/__init__.py @@ -1628,6 +1628,7 @@ def send_mail(frm, to, subject, body, config, html=None, reply_to=None): email_json = json.to_json_string(email_dict) with open(path, "w") as f: f.write(email_json) + return to = listify(to) if html: diff --git a/test/unit/app/tools/test_error_reporting.py b/test/unit/app/tools/test_error_reporting.py new file mode 100644 index 000000000000..611e298a9f4f --- /dev/null +++ b/test/unit/app/tools/test_error_reporting.py @@ -0,0 +1,141 @@ +import json + +from galaxy import model +from galaxy.app_unittest_utils.tools_support import UsesApp +from galaxy.model.base import transaction +from galaxy.tools.errors import EmailErrorReporter +from galaxy.util.unittest import TestCase + +# The email the user created their account with. +TEST_USER_EMAIL = "mockgalaxyuser@galaxyproject.org" +# The email the user supplied when submitting the error +TEST_USER_SUPPLIED_EMAIL = "fake@example.org" +TEST_SERVER_EMAIL_FROM = "email_from@galaxyproject.org" +TEST_SERVER_ERROR_EMAIL_TO = "admin@email.to" # setup in mock config + + +class TestErrorReporter(TestCase, UsesApp): + + def setUp(self): + self.setup_app() + self.app.config.email_from = TEST_SERVER_EMAIL_FROM + + def test_basic(self, tmp_path): + email_path = tmp_path / "moo.json" + self.app.config.smtp_server = f"mock_emails_to_path://{email_path}" + user, hda = self._setup_model_objects() + + assert not email_path.exists() + error_report = EmailErrorReporter(hda, self.app) + error_report.send_report(user, email=TEST_USER_SUPPLIED_EMAIL, message="My custom message") + assert email_path.exists() + text = email_path.read_text() + email_json = json.loads(text) + assert email_json["from"] == TEST_SERVER_EMAIL_FROM + assert email_json["to"] == f"{TEST_SERVER_ERROR_EMAIL_TO}, {TEST_USER_SUPPLIED_EMAIL}" + assert f"Galaxy tool error report from {TEST_USER_SUPPLIED_EMAIL}" == email_json["subject"] + assert "cat1" in email_json["body"] + assert "cat1" in email_json["html"] + assert TEST_USER_EMAIL == email_json["reply_to"] + + def test_hda_security(self, tmp_path): + email_path = tmp_path / "moo.json" + self.app.config.smtp_server = f"mock_emails_to_path://{email_path}" + user, hda = self._setup_model_objects() + error_report = EmailErrorReporter(hda, self.app) + security_agent = self.app.security_agent + private_role = security_agent.create_private_user_role(user) + access_action = security_agent.permitted_actions.DATASET_ACCESS.action + manage_action = security_agent.permitted_actions.DATASET_MANAGE_PERMISSIONS.action + permissions = {access_action: [private_role], manage_action: [private_role]} + security_agent.set_all_dataset_permissions(hda.dataset, permissions) + + other_user = model.User(email="otheruser@galaxyproject.org", password="mockpass2") + self._commit_objects([other_user]) + security_agent = self.app.security_agent + assert not email_path.exists() + error_report.send_report(other_user, email=TEST_USER_SUPPLIED_EMAIL, message="My custom message") + # Without permissions, the email still gets sent but the supplied email is ignored + # I'm not saying this is the right behavior but it is what the code does at the time of test + # writing -John + assert email_path.exists() + text = email_path.read_text() + email_json = json.loads(text) + assert "otheruser@galaxyproject.org" not in email_json["to"] + + def test_html_sanitization(self, tmp_path): + email_path = tmp_path / "moo.json" + self.app.config.smtp_server = f"mock_emails_to_path://{email_path}" + user, hda = self._setup_model_objects() + + assert not email_path.exists() + error_report = EmailErrorReporter(hda, self.app) + error_report.send_report( + user, email=TEST_USER_SUPPLIED_EMAIL, message='My custom message' + ) + assert email_path.exists() + text = email_path.read_text() + email_json = json.loads(text) + html = email_json["html"] + assert "<a href="http://sneaky.com/">message</a>" in html + + def test_redact_user_details_in_bugreport(self, tmp_path): + email_path = tmp_path / "moo.json" + self.app.config.smtp_server = f"mock_emails_to_path://{email_path}" + user, hda = self._setup_model_objects() + + assert not email_path.exists() + error_report = EmailErrorReporter(hda, self.app) + error_report.send_report( + user, email=TEST_USER_SUPPLIED_EMAIL, message="My custom message", redact_user_details_in_bugreport=True + ) + assert email_path.exists() + text = email_path.read_text() + email_json = json.loads(text) + assert "The user redacted (user: 1) provided the following information:" in email_json["body"] + assert ( + """The user redacted (user: 1) provided the following information:""" + in email_json["html"] + ) + + def test_no_redact_user_details_in_bugreport(self, tmp_path): + email_path = tmp_path / "moo.json" + self.app.config.smtp_server = f"mock_emails_to_path://{email_path}" + user, hda = self._setup_model_objects() + + assert not email_path.exists() + error_report = EmailErrorReporter(hda, self.app) + error_report.send_report( + user, email=TEST_USER_SUPPLIED_EMAIL, message="My custom message", redact_user_details_in_bugreport=False + ) + assert email_path.exists() + text = email_path.read_text() + email_json = json.loads(text) + assert ( + f"The user '{TEST_USER_EMAIL}' (providing preferred contact email '{TEST_USER_SUPPLIED_EMAIL}') provided the following information:" + in email_json["body"] + ) + assert ( + f"""The user '{TEST_USER_EMAIL}' (providing preferred contact email '{TEST_USER_SUPPLIED_EMAIL}') provided the following information:""" + in email_json["html"] + ) + + def _setup_model_objects(self): + user = model.User(email=TEST_USER_EMAIL, password="mockpass") + job = model.Job() + job.tool_id = "cat1" + job.history = model.History() + job.user = user + job.history.user = user + hda = model.HistoryDatasetAssociation(history=job.history) + hda.dataset = model.Dataset() + hda.dataset.state = "ok" + job.add_output_dataset("out1", hda) + self._commit_objects([job, hda, user]) + return user, hda + + def _commit_objects(self, objects): + session = self.app.model.context + session.add_all(objects) + with transaction(session): + session.commit()