From 33b9ab2581f4adb6c0e9268b5ac551cf2e25a911 Mon Sep 17 00:00:00 2001 From: georglauterbach <44545919+georglauterbach@users.noreply.github.com> Date: Thu, 4 Jan 2024 18:02:57 +0100 Subject: [PATCH] assert success in `_send_email` I added `_send_email_unchecked` which performs no check so we can still use the helper for situations where we check for failure, and `_send_email` is a wrapper for `_send_mail_unchecked` with an additional `assert_success`. I also went through the tests and adjusted them where necessary. --- test/helper/sending.bash | 29 ++++++++++++++++++- .../parallel/set1/dovecot/dovecot_quotas.bats | 3 -- .../parallel/set1/dovecot/dovecot_sieve.bats | 2 -- .../set1/dovecot/mailbox_format_dbox.bats | 2 -- .../set1/dovecot/special_use_folders.bats | 1 - .../parallel/set1/spam_virus/clamav.bats | 1 - .../disabled_clamav_spamassassin.bats | 1 - .../parallel/set1/spam_virus/fail2ban.bats | 2 +- .../set1/spam_virus/postgrey_enabled.bats | 3 +- .../parallel/set1/spam_virus/postscreen.bats | 6 ++-- test/tests/parallel/set3/mta/privacy.bats | 1 - .../parallel/set3/mta/smtp_delivery.bats | 13 ++------- test/tests/serial/mail_with_imap.bats | 4 +-- test/tests/serial/mail_with_ldap.bats | 9 +++--- test/tests/serial/tests.bats | 2 +- 15 files changed, 42 insertions(+), 37 deletions(-) diff --git a/test/helper/sending.bash b/test/helper/sending.bash index 48012178..1b69eb9c 100644 --- a/test/helper/sending.bash +++ b/test/helper/sending.bash @@ -15,6 +15,10 @@ # itself. The `--data` parameter expects a relative path from `emails/` # where the contents will be implicitly provided to `swaks` via STDIN. # +# This functions performs **no** implicit `assert_success` to check whether +# the e-mail transaction was successful. If this is not desirable, use +# `_send_email`. +# # ## Attention # # This function assumes `CONTAINER_NAME` to be properly set (to the container @@ -23,7 +27,7 @@ # This function will just send the email in an "asynchronous" fashion, i.e. it will # send the email but it will not make sure the mail queue is empty after the mail # has been sent. -function _send_email() { +function _send_email_unchecked() { [[ -v CONTAINER_NAME ]] || return 1 # Parameter defaults common to our testing needs: @@ -57,6 +61,29 @@ function _send_email() { done _run_in_container_bash "swaks --server ${SERVER} --port ${PORT} --ehlo ${EHLO} --from ${FROM} --to ${TO} ${ADDITIONAL_SWAKS_OPTIONS[*]} ${FINAL_SWAKS_OPTIONS[*]}" +# Sends a mail from localhost (127.0.0.1) to a container. To send +# a custom email, create a file at `test/files/`, +# and provide `` as an argument to this function. +# +# Parameters include all options that one can supply to `swaks` +# itself. The `--data` parameter expects a relative path from `emails/` +# where the contents will be implicitly provided to `swaks` via STDIN. +# +# This functions performs an implicit `assert_success` to check whether +# the e-mail transaction was successful. If this is not desirable, use +# `_send_email_unchecked`. +# +# ## Attention +# +# This function assumes `CONTAINER_NAME` to be properly set (to the container +# name the command should be executed in)! +# +# This function will just send the email in an "asynchronous" fashion, i.e. it will +# send the email but it will not make sure the mail queue is empty after the mail +# has been sent. +function _send_email() { + _send_email_unchecked "${@}" + assert_success } # Like `_send_email` with two major differences: diff --git a/test/tests/parallel/set1/dovecot/dovecot_quotas.bats b/test/tests/parallel/set1/dovecot/dovecot_quotas.bats index 81cf9bc1..3b07267f 100644 --- a/test/tests/parallel/set1/dovecot/dovecot_quotas.bats +++ b/test/tests/parallel/set1/dovecot/dovecot_quotas.bats @@ -226,11 +226,8 @@ function teardown_file() { _default_teardown ; } # send some big emails _send_email --to 'quotauser@otherdomain.tld' --data 'quota-exceeded' - assert_success _send_email --to 'quotauser@otherdomain.tld' --data 'quota-exceeded' - assert_success _send_email --to 'quotauser@otherdomain.tld' --data 'quota-exceeded' - assert_success # check for quota warn message existence run _repeat_until_success_or_timeout 20 _exec_in_container grep -R 'Subject: quota warning' /var/mail/otherdomain.tld/quotauser/new/ assert_success diff --git a/test/tests/parallel/set1/dovecot/dovecot_sieve.bats b/test/tests/parallel/set1/dovecot/dovecot_sieve.bats index e3e076a5..82bae8d8 100644 --- a/test/tests/parallel/set1/dovecot/dovecot_sieve.bats +++ b/test/tests/parallel/set1/dovecot/dovecot_sieve.bats @@ -27,10 +27,8 @@ function setup_file() { # Single mail sent from 'spam@spam.com' that is handled by User (relocate) and Global (copy) sieves for user1: _send_email --data 'sieve/spam-folder' - assert_success # Mail for user2 triggers the sieve-pipe: _send_email --to 'user2@otherdomain.tld' --data 'sieve/pipe' - assert_success _wait_for_empty_mail_queue_in_container } diff --git a/test/tests/parallel/set1/dovecot/mailbox_format_dbox.bats b/test/tests/parallel/set1/dovecot/mailbox_format_dbox.bats index 033a5bde..ea0af3d5 100644 --- a/test/tests/parallel/set1/dovecot/mailbox_format_dbox.bats +++ b/test/tests/parallel/set1/dovecot/mailbox_format_dbox.bats @@ -27,7 +27,6 @@ function teardown() { _default_teardown ; } _wait_for_smtp_port_in_container _send_email --data 'existing/user1' - assert_success _wait_for_empty_mail_queue_in_container # Mail received should be stored as `u.1` (one file per message) @@ -49,7 +48,6 @@ function teardown() { _default_teardown ; } _wait_for_smtp_port_in_container _send_email --data 'existing/user1' - assert_success _wait_for_empty_mail_queue_in_container # Mail received should be stored in `m.1` (1 or more messages) diff --git a/test/tests/parallel/set1/dovecot/special_use_folders.bats b/test/tests/parallel/set1/dovecot/special_use_folders.bats index fe1f554e..c54f949b 100644 --- a/test/tests/parallel/set1/dovecot/special_use_folders.bats +++ b/test/tests/parallel/set1/dovecot/special_use_folders.bats @@ -15,7 +15,6 @@ function teardown_file() { _default_teardown ; } @test 'normal delivery works' { _send_email --data 'existing/user1' - assert_success _count_files_in_directory_in_container /var/mail/localhost.localdomain/user1/new 1 } diff --git a/test/tests/parallel/set1/spam_virus/clamav.bats b/test/tests/parallel/set1/spam_virus/clamav.bats index 9232f90f..4c8e6ec9 100644 --- a/test/tests/parallel/set1/spam_virus/clamav.bats +++ b/test/tests/parallel/set1/spam_virus/clamav.bats @@ -26,7 +26,6 @@ function setup_file() { _wait_for_service postfix _wait_for_smtp_port_in_container _send_email --from 'virus@external.tld' --data 'amavis/virus' - assert_success _wait_for_empty_mail_queue_in_container } diff --git a/test/tests/parallel/set1/spam_virus/disabled_clamav_spamassassin.bats b/test/tests/parallel/set1/spam_virus/disabled_clamav_spamassassin.bats index f2474cc0..d6205b10 100644 --- a/test/tests/parallel/set1/spam_virus/disabled_clamav_spamassassin.bats +++ b/test/tests/parallel/set1/spam_virus/disabled_clamav_spamassassin.bats @@ -19,7 +19,6 @@ function setup_file() { _wait_for_smtp_port_in_container _send_email --data 'existing/user1' - assert_success _wait_for_empty_mail_queue_in_container } diff --git a/test/tests/parallel/set1/spam_virus/fail2ban.bats b/test/tests/parallel/set1/spam_virus/fail2ban.bats index 8a03ba04..6760202b 100644 --- a/test/tests/parallel/set1/spam_virus/fail2ban.bats +++ b/test/tests/parallel/set1/spam_virus/fail2ban.bats @@ -74,7 +74,7 @@ function teardown_file() { CONTAINER1_IP=$(_get_container_ip "${CONTAINER1_NAME}") # Trigger a ban by failing to login twice: for _ in {1..2}; do - CONTAINER_NAME=${CONTAINER2_NAME} _send_email \ + CONTAINER_NAME=${CONTAINER2_NAME} _send_email_unchecked \ --server "${CONTAINER1_IP}" \ --port 465 \ --auth PLAIN \ diff --git a/test/tests/parallel/set1/spam_virus/postgrey_enabled.bats b/test/tests/parallel/set1/spam_virus/postgrey_enabled.bats index 389fc183..238e0d37 100644 --- a/test/tests/parallel/set1/spam_virus/postgrey_enabled.bats +++ b/test/tests/parallel/set1/spam_virus/postgrey_enabled.bats @@ -51,7 +51,7 @@ function teardown_file() { _default_teardown ; } _reload_postfix # Send test mail (it should fail to deliver): - _send_email --from 'user@external.tld' --port 25 --data 'postgrey' + _send_email_unchecked --from 'user@external.tld' --port 25 --data 'postgrey' assert_failure assert_output --partial 'Recipient address rejected: Delayed by Postgrey' @@ -68,7 +68,6 @@ function teardown_file() { _default_teardown ; } sleep 3 # Retry delivering test mail (it should be trusted this time): _send_email --from 'user@external.tld' --port 25 --data 'postgrey' - assert_success # Confirm postgrey permitted delivery (triplet is now trusted): _should_have_log_entry \ diff --git a/test/tests/parallel/set1/spam_virus/postscreen.bats b/test/tests/parallel/set1/spam_virus/postscreen.bats index 377b2479..750e78b0 100644 --- a/test/tests/parallel/set1/spam_virus/postscreen.bats +++ b/test/tests/parallel/set1/spam_virus/postscreen.bats @@ -56,10 +56,8 @@ function teardown_file() { @test "should successfully pass postscreen and get postfix greeting message (respecting postscreen_greet_wait time)" { # Configure `send_email()` to send from the mail client container (CONTAINER2_NAME) via ENV override, # mail is sent to the DMS server container (CONTAINER1_NAME) via `--server` parameter: - CONTAINER_NAME=${CONTAINER2_NAME} _send_email --server "${CONTAINER1_IP}" --port 25 --data 'postscreen' - # NOTE: Cannot assert_success due to sender address not being resolvable. - # TODO: Uncomment when proper resolution of domain names is possible: - # assert_success + # TODO: Use _send_email when proper resolution of domain names is possible: + CONTAINER_NAME=${CONTAINER2_NAME} _send_email_unchecked --server "${CONTAINER1_IP}" --port 25 --data 'postscreen' # TODO: Prefer this approach when `_send_email_and_get_id()` can support separate client and server containers: # local MAIL_ID=$(_send_email_and_get_id --port 25 --data 'postscreen') diff --git a/test/tests/parallel/set3/mta/privacy.bats b/test/tests/parallel/set3/mta/privacy.bats index 4d4d82ba..999b81b9 100644 --- a/test/tests/parallel/set3/mta/privacy.bats +++ b/test/tests/parallel/set3/mta/privacy.bats @@ -30,7 +30,6 @@ function teardown_file() { _default_teardown ; } --auth-user user1@localhost.localdomain \ --auth-password mypassword \ --data 'privacy' - assert_success _run_until_success_or_timeout 120 _exec_in_container_bash '[[ -d /var/mail/localhost.localdomain/user1/new ]]' assert_success diff --git a/test/tests/parallel/set3/mta/smtp_delivery.bats b/test/tests/parallel/set3/mta/smtp_delivery.bats index 169f374a..5ec19e72 100644 --- a/test/tests/parallel/set3/mta/smtp_delivery.bats +++ b/test/tests/parallel/set3/mta/smtp_delivery.bats @@ -75,7 +75,8 @@ function setup_file() { _send_email --to test123@localhost.localdomain --data 'existing/regexp-alias-local' # Required for 'rejects mail to unknown user': - _send_email --to nouser@localhost.localdomain --data 'non-existing-user' + _send_email_unchecked --to nouser@localhost.localdomain --data 'non-existing-user' + assert_failure # Required for 'redirects mail to external aliases': _send_email --to bounce-always@localhost.localdomain --data 'existing/regexp-alias-external' _send_email --to alias2@localhost.localdomain --data 'existing/alias-local' @@ -84,25 +85,18 @@ function setup_file() { # Required for 'delivers mail to existing account': _send_email --data 'existing/user1' - assert_success _send_email --to user2@otherdomain.tld - assert_success _send_email --to user3@localhost.localdomain - assert_success _send_email --to added@localhost.localdomain --data 'existing/added' - assert_success _send_email --to user1@localhost.localdomain --data 'existing/user-and-cc-local-alias' - assert_success _send_email --data 'sieve/spam-folder' - assert_success _send_email --to user2@otherdomain.tld --data 'sieve/pipe' - assert_success _run_in_container_bash 'sendmail root < /tmp/docker-mailserver-test/emails/sendmail/root-email.txt' assert_success } function _unsuccessful() { - _send_email --port 465 --auth "${1}" --auth-user "${2}" --auth-password wrongpassword + _send_email_unchecked --port 465 --auth "${1}" --auth-user "${2}" --auth-password wrongpassword assert_failure assert_output --partial 'authentication failed' assert_output --partial 'No authentication type succeeded' @@ -110,7 +104,6 @@ function _unsuccessful() { function _successful() { _send_email --port 465 --auth "${1}" --auth-user "${2}" --auth-password mypassword --quit-after AUTH - assert_success assert_output --partial 'Authentication successful' } diff --git a/test/tests/serial/mail_with_imap.bats b/test/tests/serial/mail_with_imap.bats index eeccf888..460fddee 100644 --- a/test/tests/serial/mail_with_imap.bats +++ b/test/tests/serial/mail_with_imap.bats @@ -31,8 +31,8 @@ function teardown_file() { _default_teardown ; } } @test '(SASLauthd) RIMAP SMTP authentication works' { - _send_email \ --auth LOGIN \ + _send_email_unchecked \ --auth-user user1@localhost.localdomain \ --auth-password mypassword \ --quit-after AUTH @@ -45,7 +45,6 @@ function teardown_file() { _default_teardown ; } --auth-user user1@localhost.localdomain \ --auth-password mypassword \ --quit-after AUTH - assert_success assert_output --partial 'Authentication successful' _send_email \ @@ -54,7 +53,6 @@ function teardown_file() { _default_teardown ; } --auth-user user1@localhost.localdomain \ --auth-password mypassword \ --quit-after AUTH - assert_success assert_output --partial 'Authentication successful' } diff --git a/test/tests/serial/mail_with_ldap.bats b/test/tests/serial/mail_with_ldap.bats index f2011d22..7762c972 100644 --- a/test/tests/serial/mail_with_ldap.bats +++ b/test/tests/serial/mail_with_ldap.bats @@ -326,13 +326,14 @@ function teardown() { @test "spoofing (with LDAP): rejects sender forging" { _wait_for_smtp_port_in_container_to_respond dms-test_ldap - _send_email \ --port 465 -tlsc --auth LOGIN \ + _send_email_unchecked \ --auth-user some.user@localhost.localdomain \ --auth-password secret \ --ehlo mail \ --from ldap@localhost.localdomain \ --data 'auth/ldap-smtp-auth-spoofed' + assert_failure assert_output --partial 'Sender address rejected: not owned by user' } @@ -353,20 +354,21 @@ function teardown() { # Template used has invalid AUTH: https://github.com/docker-mailserver/docker-mailserver/pull/3006#discussion_r1073321432 skip 'TODO: This test seems to have been broken from the start (?)' - _send_email \ --port 465 -tlsc --auth LOGIN \ + _send_email_unchecked \ --auth-user some.user.email@localhost.localdomain \ --auth-password secret \ --ehlo mail \ --from randomspoofedaddress@localhost.localdomain \ --to some.user@localhost.localdomain \ --data 'auth/ldap-smtp-auth-spoofed-sender-with-filter-exception' + assert_failure assert_output --partial 'Sender address rejected: not owned by user' } @test "saslauthd: ldap smtp authentication" { - _send_email \ --auth LOGIN \ + _send_email_unchecked \ --auth-user some.user@localhost.localdomain \ --auth-password wrongpassword \ --quit-after AUTH @@ -379,7 +381,6 @@ function teardown() { --auth-user some.user@localhost.localdomain \ --auth-password secret \ --quit-after AUTH - assert_success assert_output --partial 'Authentication successful' _send_email \ diff --git a/test/tests/serial/tests.bats b/test/tests/serial/tests.bats index 26deb541..e286f137 100644 --- a/test/tests/serial/tests.bats +++ b/test/tests/serial/tests.bats @@ -293,8 +293,8 @@ EOF # An authenticated user cannot use an envelope sender (MAIL FROM) # address they do not own according to `main.cf:smtpd_sender_login_maps` lookup - _send_email \ --port 465 -tlsc --auth LOGIN \ + _send_email_unchecked \ --auth-user added@localhost.localdomain \ --auth-password mypassword \ --ehlo mail \