From dcbe5af633f47cba5b5804a9017b7062c22f04c1 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. --- .../tools/error_reports/plugins/email.py | 4 +- lib/galaxy/util/__init__.py | 4 +- test/unit/app/tools/test_error_reporting.py | 145 ++++++++++++++++++ 3 files changed, 151 insertions(+), 2 deletions(-) create mode 100644 test/unit/app/tools/test_error_reporting.py 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..a1a3c0e0c8b5 100644 --- a/lib/galaxy/util/__init__.py +++ b/lib/galaxy/util/__init__.py @@ -1615,7 +1615,8 @@ def send_mail(frm, to, subject, body, config, html=None, reply_to=None): :type reply_to: str :param reply_to: Reply-to address (Default None) """ - if config.smtp_server.startswith("mock_emails_to_path://"): + smtp_server = config.smtp_server + if smtp_server and isinstance(smtp_server, str) and config.smtp_server.startswith("mock_emails_to_path://"): path = config.smtp_server[len("mock_emails_to_path://") :] email_dict = { "from": frm, @@ -1628,6 +1629,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..7db9cb4bf60e --- /dev/null +++ b/test/unit/app/tools/test_error_reporting.py @@ -0,0 +1,145 @@ +import json +import shutil +import tempfile +from pathlib import Path + +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 + self.tmp_path = Path(tempfile.mkdtemp()) + self.email_path = self.tmp_path / "email.json" + smtp_server = f"mock_emails_to_path://{self.email_path}" + self.app.config.smtp_server = smtp_server # type: ignore[attr-defined] + + def tearDown(self): + shutil.rmtree(self.tmp_path) + + def test_basic(self): + user, hda = self._setup_model_objects() + + email_path = self.email_path + 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): + 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 + email_path = self.email_path + 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): + user, hda = self._setup_model_objects() + email_path = self.email_path + 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): + user, hda = self._setup_model_objects() + + email_path = self.email_path + 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): + user, hda = self._setup_model_objects() + + email_path = self.email_path + 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()