From 05db27f817aa0a512f710c10eafe3ab71903f99a Mon Sep 17 00:00:00 2001 From: Brennan Kinney <5098581+polarathene@users.noreply.github.com> Date: Sat, 4 Feb 2023 11:52:30 +1300 Subject: [PATCH] tests(refactor): Extract mail account management tests from `tests.bats` (#3055) * chore: Extract out accounts test cases from `tests.bats` Standard test file format, the test cases have been copied over unmodified. * chore: Revise test case descriptions * tests(refactor): `accounts.bats` Revised test cases: - Some common test case logic extracted to test methods. - Update direct user management commands to use the `setup email ...` variants. - Improved assertions. - Removed `sleep 2` lines as the need for that is ambiguous (may no longer be relevant?) - Additional commentary for maintaining - Two test cases for missing `postfix-accounts.cf` opted to just run the image without any volumes instead, as the `without-accounts/` folder was empty anyway. 2nd test case can instead use a single `docker run` to check the newly created`postfix-accounts.cf` content. - `test/config/without-accounts/` remains as `open_dkim.bats` presently uses it. * chore: Remove unnecessary account removal assert Traced this back to the original PR where it appears to have been a typo and was probably intended as a cleanup on the `user4` account. Not necessary, removing. * chore: Rename `accounts.bat` -> `account_management.bats` --------- * feedback: Avoid `ls` for detecting directories Replace `ls -d` approach from original test cases Co-authored-by: Georg Lauterbach <44545919+georglauterbach@users.noreply.github.com> * feedback: Remove asserting empty output on failure Co-authored-by: Georg Lauterbach <44545919+georglauterbach@users.noreply.github.com> --- test/helper/common.bash | 3 + .../parallel/set3/mta/account_management.bats | 161 ++++++++++++++++++ test/tests/serial/tests.bats | 136 --------------- 3 files changed, 164 insertions(+), 136 deletions(-) create mode 100644 test/tests/parallel/set3/mta/account_management.bats diff --git a/test/helper/common.bash b/test/helper/common.bash index cc72f3e1..a59d7769 100644 --- a/test/helper/common.bash +++ b/test/helper/common.bash @@ -353,6 +353,9 @@ function _add_mail_account_then_wait_until_ready() { local MAIL_PASS="${2:-password_not_relevant_to_test}" local CONTAINER_NAME=$(__handle_container_name "${3:-}") + # Required to detect a new account and create the maildir: + _wait_for_service changedetector "${CONTAINER_NAME}" + _run_in_container setup email add "${MAIL_ACCOUNT}" "${MAIL_PASS}" assert_success diff --git a/test/tests/parallel/set3/mta/account_management.bats b/test/tests/parallel/set3/mta/account_management.bats new file mode 100644 index 00000000..14cc6525 --- /dev/null +++ b/test/tests/parallel/set3/mta/account_management.bats @@ -0,0 +1,161 @@ +load "${REPOSITORY_ROOT}/test/helper/common" +load "${REPOSITORY_ROOT}/test/helper/setup" + +BATS_TEST_NAME_PREFIX='[Mail Accounts] ' +CONTAINER_NAME='dms-test_accounts' + +function setup_file() { + _init_with_defaults + _common_container_setup + + # Testing account added after start-up is also working correctly: + _add_mail_account_then_wait_until_ready 'added@localhost.localdomain' 'mypassword' + # Testing can create an account with potentially problematic input: + _add_mail_account_then_wait_until_ready 'pass@localhost.localdomain' 'may be \a `p^a.*ssword' +} + +function teardown_file() { _default_teardown ; } + +### Account Setup ### + +@test "should have created all accounts in Dovecot UserDB" { + _run_in_container doveadm user '*' + assert_success + assert_line --index 0 'user1@localhost.localdomain' + assert_line --index 1 'user2@otherdomain.tld' + assert_line --index 2 'user3@localhost.localdomain' + assert_line --index 3 'added@localhost.localdomain' + assert_line --index 4 'pass@localhost.localdomain' + assert_line --index 5 'alias1@localhost.localdomain' + # TODO: Probably not intentional?: + assert_line --index 6 '@localdomain2.com' + _should_output_number_of_lines 7 + + # Relevant log output from scripts/helpers/accounts.sh:_create_dovecot_alias_dummy_accounts(): + # [ DEBUG ] Adding alias 'alias1@localhost.localdomain' for user 'user1@localhost.localdomain' to Dovecot's userdb + # [ DEBUG ] Alias 'alias2@localhost.localdomain' is non-local (or mapped to a non-existing account) and will not be added to Dovecot's userdb + # [ DEBUG ] Adding alias '@localdomain2.com' for user 'user1@localhost.localdomain' to Dovecot's userdb +} + +@test "should have created maildir for 'user1@localhost.localdomain'" { + _run_in_container_bash '[[ -d /var/mail/localhost.localdomain/user1 ]]' + assert_success +} + +@test "should have created maildir for 'user2@otherdomain.tld'" { + _run_in_container_bash '[[ -d /var/mail/otherdomain.tld/user2 ]]' + assert_success +} + +@test "should have created maildir for 'user3@localhost.localdomain'" { + _run_in_container_bash '[[ -d /var/mail/localhost.localdomain/user3 ]]' + assert_success +} + +@test "should have created maildir for 'added@localhost.localdomain'" { + _run_in_container_bash '[[ -d /var/mail/localhost.localdomain/added ]]' + assert_success +} + +@test "should not accidentally parse comments in 'postfix-accounts.cf' as accounts" { + _run_in_container find /var/mail -maxdepth 1 + assert_success + refute_output --partial 'comment' +} + +### Account Management ### + +@test "should fail to create a user when the domain-part ('@example.com') is missing" { + _run_in_container setup email add user_without_domain mypassword + assert_failure + assert_output --partial 'should include the domain (eg: user@example.com)' +} + +@test "should add new user 'user3@domain.tld' into 'postfix-accounts.cf'" { + __should_add_new_user 'user3@domain.tld' +} + +# To catch mistakes from substring matching: +@test "should add new user 'auser3@domain.tld' into 'postfix-accounts.cf'" { + __should_add_new_user 'auser3@domain.tld' +} + +# To catch mistakes from accidental pattern `.` matching as `u`: +@test "should add new user 'a.ser3@domain.tld' into 'postfix-accounts.cf'" { + __should_add_new_user 'a.ser3@domain.tld' +} + +@test "should remove user3 (but not auser3) from 'postfix-accounts.cf'" { + # Waits until change event has created directory but not completed: + _wait_until_account_maildir_exists 'user3@domain.tld' + # Should trigger a new change event: + _exec_in_container setup email del -y 'user3@domain.tld' + + # NOTE: This is only checking `postfix-accounts.cf`, account may still persist + # elsewhere momentarily such as the Dovecot UserDB until change event kicks in. + __check_mail_account_exists 'user3@domain.tld' + assert_failure + + __check_mail_account_exists 'auser3@domain.tld' + assert_success + assert_output 'auser3@domain.tld' +} + +@test "should update password for user4 by modifying entry in 'postfix-accounts.cf'" { + # This change tends to be bundled with change detection event from previous test case + # deleting 'user3@domain.tld', thus both changes are usually applied together. + # NOTE: Technically these two `setup email ...` commands are run async, there is no + # proper file locking applied to `postfix-accounts.cf`, potentially a race condition. + _add_mail_account_then_wait_until_ready 'user4@domain.tld' + + local ORIGINAL_ENTRY UPDATED_ENTRY + ORIGINAL_ENTRY=$(_exec_in_container grep '^user4@domain\.tld' -i /tmp/docker-mailserver/postfix-accounts.cf) + _exec_in_container setup email update 'user4@domain.tld' mynewpassword + UPDATED_ENTRY=$(_exec_in_container grep '^user4@domain\.tld' -i /tmp/docker-mailserver/postfix-accounts.cf) + + assert_not_equal "${ORIGINAL_ENTRY}" "${UPDATED_ENTRY}" +} + +# TODO: Prone to failure sometimes from the change event in previous test case, +# as Dovecot service can be momentarily unavailable during reload? +@test "(ENV ENABLE_QUOTAS=0) 'setup email list' should not display quota information" { + _run_in_container_bash 'echo "ENABLE_QUOTAS=0" >> /etc/dms-settings && setup email list | head -n 1' + assert_success + assert_output '* user1@localhost.localdomain' +} + +@test "(ENV ENABLE_QUOTAS=1) 'setup email list' should display quota information" { + _run_in_container_bash 'sed -i "/ENABLE_QUOTAS=0/d" /etc/dms-settings; setup email list | head -n 1' + assert_success + assert_output '* user1@localhost.localdomain ( 0 / ~ ) [0%]' +} + +@test "(missing postfix-accounts.cf) 'setup email del' should not fail with an error" { + run docker run --rm "${IMAGE_NAME:?}" setup email del -y 'user3@domain.tld' + assert_success + assert_output '' +} + +@test "(missing postfix-accounts.cf) 'setup email add' should create 'postfix-accounts.cf' and populate with new mail account" { + run docker run --rm "${IMAGE_NAME:?}" \ + /bin/bash -c 'setup email add user3@domain.tld mypassword && grep -o "^user3@domain\.tld" /tmp/docker-mailserver/postfix-accounts.cf' + assert_success + assert_output 'user3@domain.tld' +} + +function __should_add_new_user() { + local MAIL_ACCOUNT=${1} + _exec_in_container setup email add "${MAIL_ACCOUNT}" mypassword + + __check_mail_account_exists "${MAIL_ACCOUNT}" + assert_success + assert_output "${MAIL_ACCOUNT}" +} + +# Uses a double grep to avoid test case failures from accidental substring/pattern matching +function __check_mail_account_exists() { + local MAIL_ACCOUNT=${1} + + # Filter out any comment lines, then truncate each line at their first `|` delimiter: + _run_in_container_bash "sed -e '/\s*#/d' -e 's/|.*//' /tmp/docker-mailserver/postfix-accounts.cf | grep '^${MAIL_ACCOUNT}' | grep -F '${MAIL_ACCOUNT}'" +} diff --git a/test/tests/serial/tests.bats b/test/tests/serial/tests.bats index e91f70bd..dfbb49a8 100644 --- a/test/tests/serial/tests.bats +++ b/test/tests/serial/tests.bats @@ -31,7 +31,6 @@ function setup_file() { _common_container_setup 'CONTAINER_ARGS_ENV_CUSTOM' _add_mail_account_then_wait_until_ready 'added@localhost.localdomain' 'mypassword' - _add_mail_account_then_wait_until_ready 'pass@localhost.localdomain' 'may be \a `p^a.*ssword' _wait_for_service postfix _wait_for_smtp_port_in_container @@ -114,44 +113,6 @@ function teardown_file() { _default_teardown ; } assert_success } -# -# accounts -# - -@test "accounts: user accounts" { - _run_in_container doveadm user '*' - assert_success - assert_line --index 0 "user1@localhost.localdomain" - assert_line --index 1 "user2@otherdomain.tld" - assert_line --index 2 "user3@localhost.localdomain" - assert_line --index 3 "added@localhost.localdomain" -} - -@test "accounts: user mail folder for user1" { - _run_in_container_bash "ls -d /var/mail/localhost.localdomain/user1" - assert_success -} - -@test "accounts: user mail folder for user2" { - _run_in_container_bash "ls -d /var/mail/otherdomain.tld/user2" - assert_success -} - -@test "accounts: user mail folder for user3" { - _run_in_container_bash "ls -d /var/mail/localhost.localdomain/user3" - assert_success -} - -@test "accounts: user mail folder for added user" { - _run_in_container_bash "ls -d /var/mail/localhost.localdomain/added" - assert_success -} - -@test "accounts: comments are not parsed" { - _run_in_container_bash "ls /var/mail | grep 'comment'" - assert_failure -} - # # postfix # @@ -283,103 +244,6 @@ zip EOF } -# -# accounts -# -@test "accounts: user_without_domain creation should be rejected since user@domain format is required" { - _run_in_container_bash "addmailuser user_without_domain mypassword" - assert_failure - assert_output --partial 'should include the domain (eg: user@example.com)' -} - -@test "accounts: user3 should have been added to /tmp/docker-mailserver/postfix-accounts.cf" { - _exec_in_container_bash "addmailuser user3@domain.tld mypassword" - - _run_in_container_bash "grep '^user3@domain\.tld|' -i /tmp/docker-mailserver/postfix-accounts.cf" - assert_success - [[ -n ${output} ]] -} - -@test "accounts: auser3 should have been added to /tmp/docker-mailserver/postfix-accounts.cf" { - _exec_in_container_bash "addmailuser auser3@domain.tld mypassword" - - _run_in_container_bash "grep '^auser3@domain\.tld|' -i /tmp/docker-mailserver/postfix-accounts.cf" - assert_success - [[ -n ${output} ]] -} - -@test "accounts: a.ser3 should have been added to /tmp/docker-mailserver/postfix-accounts.cf" { - _exec_in_container_bash "addmailuser a.ser3@domain.tld mypassword" - - _run_in_container_bash "grep '^a\.ser3@domain\.tld|' -i /tmp/docker-mailserver/postfix-accounts.cf" - assert_success - [[ -n ${output} ]] -} - -@test "accounts: user3 should have been removed from /tmp/docker-mailserver/postfix-accounts.cf but not auser3" { - _wait_until_account_maildir_exists 'user3@domain.tld' - - _exec_in_container_bash "delmailuser -y user3@domain.tld" - - _run_in_container_bash "grep '^user3@domain\.tld' -i /tmp/docker-mailserver/postfix-accounts.cf" - assert_failure - [[ -z ${output} ]] - - _run_in_container_bash "grep '^auser3@domain\.tld' -i /tmp/docker-mailserver/postfix-accounts.cf" - assert_success - [[ -n ${output} ]] -} - -@test "user updating password for user in /tmp/docker-mailserver/postfix-accounts.cf" { - _add_mail_account_then_wait_until_ready 'user4@domain.tld' - - initialpass=$(_exec_in_container_bash "grep '^user4@domain\.tld' -i /tmp/docker-mailserver/postfix-accounts.cf") - sleep 2 - _exec_in_container_bash "updatemailuser user4@domain.tld mynewpassword" - sleep 2 - changepass=$(_exec_in_container_bash "grep '^user4@domain\.tld' -i /tmp/docker-mailserver/postfix-accounts.cf") - - [[ ${initialpass} != "${changepass}" ]] - - _run_in_container_bash "delmailuser -y auser3@domain.tld" - assert_success -} - -@test "accounts: listmailuser (quotas disabled)" { - _run_in_container_bash "echo 'ENABLE_QUOTAS=0' >> /etc/dms-settings && listmailuser | head -n 1" - assert_success - assert_output '* user1@localhost.localdomain' -} - -@test "accounts: listmailuser (quotas enabled)" { - _run_in_container_bash "sed -i '/ENABLE_QUOTAS=0/d' /etc/dms-settings; listmailuser | head -n 1" - assert_success - assert_output '* user1@localhost.localdomain ( 0 / ~ ) [0%]' -} - -@test "accounts: no error is generated when deleting a user if /tmp/docker-mailserver/postfix-accounts.cf is missing" { - run docker run --rm \ - -v "$(_duplicate_config_for_container without-accounts/ without-accounts-deleting-user)":/tmp/docker-mailserver/ \ - "${IMAGE_NAME:?}" /bin/sh -c 'delmailuser -y user3@domain.tld' - assert_success - [[ -z ${output} ]] -} - -@test "accounts: user3 should have been added to /tmp/docker-mailserver/postfix-accounts.cf even when that file does not exist" { - local PRIVATE_CONFIG - PRIVATE_CONFIG=$(_duplicate_config_for_container without-accounts/ without-accounts_file_does_not_exist) - run docker run --rm \ - -v "${PRIVATE_CONFIG}/without-accounts/":/tmp/docker-mailserver/ \ - "${IMAGE_NAME:?}" /bin/sh -c 'addmailuser user3@domain.tld mypassword' - assert_success - run docker run --rm \ - -v "${PRIVATE_CONFIG}/without-accounts/":/tmp/docker-mailserver/ \ - "${IMAGE_NAME:?}" /bin/sh -c 'grep user3@domain.tld -i /tmp/docker-mailserver/postfix-accounts.cf' - assert_success - [[ -n ${output} ]] -} - - @test "quota: setquota user must be existing" { _add_mail_account_then_wait_until_ready 'quota_user@domain.tld'