From 13217be939278a483e77e46fd1e1dd5081d7a829 Mon Sep 17 00:00:00 2001 From: Kevin Morris Date: Tue, 8 Mar 2022 17:49:21 -0800 Subject: [PATCH 1/3] fix: don't check suspension for ownership changes People can change comaintainer ownership to suspended users if they want to. Suspended users cannot login, so there is no breach of security here. It does make sense to allow ownership to be changed, imo. Closes #339 Signed-off-by: Kevin Morris --- aurweb/scripts/notify.py | 5 +---- aurweb/testing/email.py | 9 +++++++++ test/test_notify.py | 15 +++++++++++++++ 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/aurweb/scripts/notify.py b/aurweb/scripts/notify.py index c823b09e..dbef3aa5 100755 --- a/aurweb/scripts/notify.py +++ b/aurweb/scripts/notify.py @@ -399,10 +399,7 @@ class ComaintainershipEventNotification(Notification): self._pkgbase = db.query(PackageBase.Name).filter( PackageBase.ID == pkgbase_id).first().Name - user = db.query(User).filter( - and_(User.ID == uid, - User.Suspended == 0) - ).with_entities( + user = db.query(User).filter(User.ID == uid).with_entities( User.Email, User.LangPreference ).first() diff --git a/aurweb/testing/email.py b/aurweb/testing/email.py index c0be2797..b3e3990b 100644 --- a/aurweb/testing/email.py +++ b/aurweb/testing/email.py @@ -37,6 +37,15 @@ class Email: if autoparse: self._parse() + @staticmethod + def reset() -> None: + # Cleanup all email files for this test suite. + prefix = Email.email_prefix(suite=True) + files = os.listdir(Email.TEST_DIR) + for file in files: + if file.startswith(prefix): + os.remove(os.path.join(Email.TEST_DIR, file)) + @staticmethod def email_prefix(suite: bool = False) -> str: """ diff --git a/test/test_notify.py b/test/test_notify.py index a8e994c5..2009e3a8 100644 --- a/test/test_notify.py +++ b/test/test_notify.py @@ -299,6 +299,21 @@ You were removed from the co-maintainer list of {pkgbase.Name} [1]. assert email.body == expected +def test_suspended_ownership_change(user: User, pkgbases: List[PackageBase]): + with db.begin(): + user.Suspended = 1 + + pkgbase = pkgbases[0] + notif = notify.ComaintainerAddNotification(user.ID, pkgbase.ID) + notif.send() + assert Email.count() == 1 + + Email.reset() # Clear the Email pool + notif = notify.ComaintainerRemoveNotification(user.ID, pkgbase.ID) + notif.send() + assert Email.count() == 1 + + def test_delete(user: User, user2: User, pkgbases: List[PackageBase]): pkgbase = pkgbases[0] notif = notify.DeleteNotification(user2.ID, pkgbase.ID) From e00cf5f1249b522e58fb0651ae8b00e7b74c6ab2 Mon Sep 17 00:00:00 2001 From: Kevin Morris Date: Tue, 8 Mar 2022 17:51:44 -0800 Subject: [PATCH 2/3] test: use smtplib.SMTP[_SSL] timeout = notifications.smtp-timeout A new option has been added for configuration of SMTP timeout: - notifications.smtp-timeout During tests, we can change this timeout to be small, so we aren't depending on hardware-based RNG to pass the timeout. Without a timeout, users can run into a long-running test for no particular reason. Signed-off-by: Kevin Morris --- aurweb/scripts/notify.py | 5 +++- aurweb/testing/smtp.py | 3 +++ conf/config.defaults | 1 + test/test_notify.py | 49 ++++++++++++++++++++++++---------------- 4 files changed, 37 insertions(+), 21 deletions(-) diff --git a/aurweb/scripts/notify.py b/aurweb/scripts/notify.py index dbef3aa5..6afa65ae 100755 --- a/aurweb/scripts/notify.py +++ b/aurweb/scripts/notify.py @@ -104,7 +104,10 @@ class Notification: False: smtplib.SMTP, True: smtplib.SMTP_SSL, } - server = classes[use_ssl](server_addr, server_port) + smtp_timeout = aurweb.config.getint("notifications", + "smtp-timeout") + server = classes[use_ssl](server_addr, server_port, + timeout=smtp_timeout) if use_starttls: server.ehlo() diff --git a/aurweb/testing/smtp.py b/aurweb/testing/smtp.py index da64c93f..e5d67991 100644 --- a/aurweb/testing/smtp.py +++ b/aurweb/testing/smtp.py @@ -36,6 +36,9 @@ class FakeSMTP: def quit(self) -> None: self.quit_count += 1 + def __call__(self, *args, **kwargs) -> "FakeSMTP": + return self + class FakeSMTP_SSL(FakeSMTP): """ A fake version of smtplib.SMTP_SSL used for testing. """ diff --git a/conf/config.defaults b/conf/config.defaults index 371c99b2..722802cc 100644 --- a/conf/config.defaults +++ b/conf/config.defaults @@ -65,6 +65,7 @@ smtp-use-ssl = 0 smtp-use-starttls = 0 smtp-user = smtp-password = +smtp-timeout = 60 sender = notify@aur.archlinux.org reply-to = noreply@aur.archlinux.org diff --git a/test/test_notify.py b/test/test_notify.py index 2009e3a8..fdec5ed7 100644 --- a/test/test_notify.py +++ b/test/test_notify.py @@ -547,18 +547,18 @@ def test_smtp(user: User): with db.begin(): user.ResetKey = "12345678901234567890123456789012" - SMTP = FakeSMTP() + smtp = FakeSMTP() get = "aurweb.config.get" getboolean = "aurweb.config.getboolean" with mock.patch(get, side_effect=mock_smtp_config(str)): with mock.patch(getboolean, side_effect=mock_smtp_config(bool)): - with mock.patch("smtplib.SMTP", side_effect=lambda a, b: SMTP): + with mock.patch("smtplib.SMTP", side_effect=smtp): config.rehash() notif = notify.WelcomeNotification(user.ID) notif.send() config.rehash() - assert len(SMTP.emails) == 1 + assert len(smtp.emails) == 1 def mock_smtp_starttls_config(cls): @@ -586,25 +586,25 @@ def test_smtp_starttls(user: User): user.ResetKey = "12345678901234567890123456789012" user.BackupEmail = "backup@example.org" - SMTP = FakeSMTP() + smtp = FakeSMTP() get = "aurweb.config.get" getboolean = "aurweb.config.getboolean" with mock.patch(get, side_effect=mock_smtp_starttls_config(str)): with mock.patch( getboolean, side_effect=mock_smtp_starttls_config(bool)): - with mock.patch("smtplib.SMTP", side_effect=lambda a, b: SMTP): + with mock.patch("smtplib.SMTP", side_effect=smtp): notif = notify.WelcomeNotification(user.ID) notif.send() - assert SMTP.starttls_enabled - assert SMTP.user - assert SMTP.passwd + assert smtp.starttls_enabled + assert smtp.user + assert smtp.passwd - assert len(SMTP.emails) == 2 - to = SMTP.emails[0][1] + assert len(smtp.emails) == 2 + to = smtp.emails[0][1] assert to == [user.Email] - to = SMTP.emails[1][1] + to = smtp.emails[1][1] assert to == [user.BackupEmail] @@ -629,19 +629,19 @@ def test_smtp_ssl(user: User): with db.begin(): user.ResetKey = "12345678901234567890123456789012" - SMTP = FakeSMTP_SSL() + smtp = FakeSMTP_SSL() get = "aurweb.config.get" getboolean = "aurweb.config.getboolean" with mock.patch(get, side_effect=mock_smtp_ssl_config(str)): with mock.patch(getboolean, side_effect=mock_smtp_ssl_config(bool)): - with mock.patch("smtplib.SMTP_SSL", side_effect=lambda a, b: SMTP): + with mock.patch("smtplib.SMTP_SSL", side_effect=smtp): notif = notify.WelcomeNotification(user.ID) notif.send() - assert len(SMTP.emails) == 1 - assert SMTP.use_ssl - assert SMTP.user - assert SMTP.passwd + assert len(smtp.emails) == 1 + assert smtp.use_ssl + assert smtp.user + assert smtp.passwd def test_notification_defaults(): @@ -655,6 +655,7 @@ def test_notification_oserror(user: User, caplog: pytest.LogCaptureFixture): """ Try sending a notification with a bad SMTP configuration. """ caplog.set_level(ERROR) config_get = config.get + config_getint = config.getint mocked_options = { "sendmail": str(), @@ -662,8 +663,9 @@ def test_notification_oserror(user: User, caplog: pytest.LogCaptureFixture): "smtp-port": "587", "smtp-user": "notify@server.xyz", "smtp-password": "notify_server_xyz", + "smtp-timeout": 1, "sender": "notify@server.xyz", - "reply-to": "no-reply@server.xyz" + "reply-to": "no-reply@server.xyz", } def mock_config_get(section: str, key: str) -> str: @@ -672,9 +674,16 @@ def test_notification_oserror(user: User, caplog: pytest.LogCaptureFixture): return mocked_options.get(key) return config_get(section, key) + def mock_config_getint(section: str, key: str) -> str: + if section == "notifications": + if key in mocked_options: + return mocked_options.get(key) + return config_getint(section, key) + notif = notify.WelcomeNotification(user.ID) - with mock.patch("aurweb.config.get", side_effect=mock_config_get): - notif.send() + with mock.patch("aurweb.config.getint", side_effect=mock_config_getint): + with mock.patch("aurweb.config.get", side_effect=mock_config_get): + notif.send() expected = "Unable to emit notification due to an OSError" assert expected in caplog.text From 2a393f95faa8a4952faf22b1cbcb4e0c8e8318ae Mon Sep 17 00:00:00 2001 From: Kevin Morris Date: Tue, 8 Mar 2022 17:59:00 -0800 Subject: [PATCH 3/3] upgrade: bump to v6.0.23 Signed-off-by: Kevin Morris --- aurweb/config.py | 2 +- pyproject.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/aurweb/config.py b/aurweb/config.py index d0b095f0..637024de 100644 --- a/aurweb/config.py +++ b/aurweb/config.py @@ -6,7 +6,7 @@ from typing import Any # Publicly visible version of aurweb. This is used to display # aurweb versioning in the footer and must be maintained. # Todo: Make this dynamic/automated. -AURWEB_VERSION = "v6.0.22" +AURWEB_VERSION = "v6.0.23" _parser = None diff --git a/pyproject.toml b/pyproject.toml index f2401b88..e930a331 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -8,7 +8,7 @@ # [tool.poetry] name = "aurweb" -version = "v6.0.22" +version = "v6.0.23" license = "GPL-2.0-only" description = "Source code for the Arch User Repository's website" homepage = "https://aur.archlinux.org"