diff --git a/target/bin/addmailuser b/target/bin/addmailuser index 3827c68f..729bb4b8 100755 --- a/target/bin/addmailuser +++ b/target/bin/addmailuser @@ -13,9 +13,9 @@ function _main _manage_accounts_create "${MAIL_ACCOUNT}" "${PASSWD}" - # Change Detection will be triggered from `postfix-accounts.cf` update, - # block until event processed (actual account creation handled there): - _wait_until_account_maildir_exists "${MAIL_ACCOUNT}" + # NOTE: An account has only been added to `postfix-accounts.cf`. + # The `changedetector` service will be triggered from this update + # where the actual account is created in Dovecot. Expect a delay. } function __usage @@ -48,37 +48,4 @@ ${ORANGE}EXIT STATUS${RESET} " } -# TODO: Remove this method or at least it's usage in `addmailuser`. If tests are failing, correct the tests. -# -# This method was added to delay command completion until a change detection event had processed the newly added user, -# confirmed once maildir was created. It was a workaround to accomodate the test suite apparently, but otherwise -# prevents batch adding users (each one would have to go through their own change detection event). -# -# Originally introduced in PR 1980 (afterwards two futher PRs deleted, and then reverted that deletion): -# https://github.com/docker-mailserver/docker-mailserver/pull/1980 -# Not much details/discussion in the PR, these are the specific commits: -# - Initial commit: https://github.com/docker-mailserver/docker-mailserver/pull/1980/commits/2ed402a12cedd412abcf577e8079137ea05204fe#diff-92d2047e4a9a7965f6ef2f029dd781e09265b0ce171b5322a76e35b66ab4cbf4R67 -# - Follow-up commit: https://github.com/docker-mailserver/docker-mailserver/pull/1980/commits/27542867b20c617b63bbec6fdcba421b65a44fbb#diff-92d2047e4a9a7965f6ef2f029dd781e09265b0ce171b5322a76e35b66ab4cbf4R67 -# -# Original reasoning for this method (sounds like a network storage I/O issue): -# Tests fail if the creation of /var/mail/${DOMAIN}/${USER} doesn't happen fast enough after addmailuser executes (check-for-changes.sh race-condition) -# Prevent infinite loop in tests like "checking accounts: user3 should have been added to /tmp/docker-mailserver/postfix-accounts.cf even when that file does not exist" -function _wait_until_account_maildir_exists -{ - local MAIL_ACCOUNT=${1} - - if [[ -f ${CHKSUM_FILE} ]] - then - local USER="${MAIL_ACCOUNT%@*}" - local DOMAIN="${MAIL_ACCOUNT#*@}" - - local MAIL_ACCOUNT_STORAGE_DIR="/var/mail/${DOMAIN}/${USER}" - while [[ ! -d ${MAIL_ACCOUNT_STORAGE_DIR} ]] - do - _log 'info' "Waiting for dovecot to create '${MAIL_ACCOUNT_STORAGE_DIR}'" - sleep 1 - done - fi -} - _main "${@}" diff --git a/test/setup-cli.bats b/test/setup-cli.bats index 0c7848b8..8ead6616 100644 --- a/test/setup-cli.bats +++ b/test/setup-cli.bats @@ -9,8 +9,15 @@ load 'test_helper/common' function setup_file() { # Initializes common default vars to prepare a DMS container with: init_with_defaults - # Creates and starts the container: - common_container_setup + + # Creates and starts the container with additional ENV needed: + # `LOG_LEVEL=debug` required for using `wait_until_change_detection_event_completes()` + # shellcheck disable=SC2034 + local CONTAINER_ARGS_ENV_CUSTOM=( + --env LOG_LEVEL='debug' + ) + + common_container_setup 'CONTAINER_ARGS_ENV_CUSTOM' } function teardown_file() { @@ -29,23 +36,32 @@ function teardown_file() { assert_line --index 0 --partial "The command 'lol troll' is invalid." } +# Create a new account for subsequent tests to depend upon @test "checking setup.sh: setup.sh email add and login" { - wait_for_service "${TEST_NAME}" changedetector + local MAIL_ACCOUNT='user@example.com' + local MAIL_PASS='test_password' + local DATABASE_ACCOUNTS="${TEST_TMP_CONFIG}/postfix-accounts.cf" - run ./setup.sh -c "${TEST_NAME}" email add setup_email_add@example.com test_password + # Create an account + run ./setup.sh -c "${TEST_NAME}" email add "${MAIL_ACCOUNT}" "${MAIL_PASS}" assert_success - value=$(grep setup_email_add@example.com "${TEST_TMP_CONFIG}/postfix-accounts.cf" | awk -F '|' '{print $1}') - assert_equal "${value}" 'setup_email_add@example.com' + # Verify account was added to `postfix-accounts.cf`: + local ACCOUNT + ACCOUNT=$(grep "${MAIL_ACCOUNT}" "${DATABASE_ACCOUNTS}" | awk -F '|' '{print $1}') + assert_equal "${ACCOUNT}" "${MAIL_ACCOUNT}" - wait_for_changes_to_be_detected_in_container "${TEST_NAME}" - - wait_for_service "${TEST_NAME}" postfix + # Wait for change detection event to complete (create maildir and add account to Dovecot UserDB+PassDB) + wait_until_account_maildir_exists "${TEST_NAME}" "${MAIL_ACCOUNT}" + # Dovecot is stopped briefly at the end of processing a change event (should change to reload in future), + # to more accurately use `wait_for_service` ensure you wait until `changedetector` is done. + wait_until_change_detection_event_completes "${TEST_NAME}" wait_for_service "${TEST_NAME}" dovecot - sleep 5 - run docker exec "${TEST_NAME}" /bin/bash -c "doveadm auth test -x service=smtp setup_email_add@example.com 'test_password' | grep 'passdb'" - assert_output "passdb: setup_email_add@example.com auth succeeded" + # Verify account authentication is successful: + local RESPONSE + RESPONSE=$(docker exec "${TEST_NAME}" doveadm auth test "${MAIL_ACCOUNT}" "${MAIL_PASS}" | grep 'passdb') + assert_equal "${RESPONSE}" "passdb: ${MAIL_ACCOUNT} auth succeeded" } @test "checking setup.sh: setup.sh email list" { @@ -53,39 +69,64 @@ function teardown_file() { assert_success } +# Update an existing account @test "checking setup.sh: setup.sh email update" { - run ./setup.sh -c "${TEST_NAME}" email add lorem@impsum.org test_test + local MAIL_ACCOUNT='user@example.com' + local MAIL_PASS='test_password' + local DATABASE_ACCOUNTS="${TEST_TMP_CONFIG}/postfix-accounts.cf" + + # `postfix-accounts.cf` should already have an account with a non-empty hashed password: + local MAIL_PASS_HASH + MAIL_PASS_HASH=$(grep "${MAIL_ACCOUNT}" "${DATABASE_ACCOUNTS}" | awk -F '|' '{print $2}') + assert_not_equal "${MAIL_PASS_HASH}" "" + + # Update the password should be successful: + local NEW_PASS='new_password' + run ./setup.sh -c "${TEST_NAME}" email update "${MAIL_ACCOUNT}" "${NEW_PASS}" + refute_output --partial 'Password must not be empty' assert_success - initialpass=$(grep lorem@impsum.org "${TEST_TMP_CONFIG}/postfix-accounts.cf" | awk -F '|' '{print $2}') - [[ -n ${initialpass} ]] + # `postfix-accounts.cf` should have an updated password hash stored: + local NEW_PASS_HASH + NEW_PASS_HASH=$(grep "${MAIL_ACCOUNT}" "${DATABASE_ACCOUNTS}" | awk -F '|' '{print $2}') + assert_not_equal "${NEW_PASS_HASH}" "" + assert_not_equal "${NEW_PASS_HASH}" "${MAIL_PASS_HASH}" - run ./setup.sh -c "${TEST_NAME}" email update lorem@impsum.org my password - assert_success - - updatepass=$(grep lorem@impsum.org "${TEST_TMP_CONFIG}/postfix-accounts.cf" | awk -F '|' '{print $2}') - [[ ${updatepass} != "" ]] - [[ ${initialpass} != "${updatepass}" ]] - - run docker exec "${TEST_NAME}" doveadm pw -t "${updatepass}" -p 'my password' - assert_output --partial 'verified' + # Verify Dovecot derives NEW_PASS_HASH from NEW_PASS: + run docker exec "${TEST_NAME}" doveadm pw -t "${NEW_PASS_HASH}" -p "${NEW_PASS}" + refute_output 'Fatal: reverse password verification check failed: Password mismatch' + assert_output "${NEW_PASS_HASH} (verified)" } +# Delete an existing account +# WARNING: While this feature works via the internal `setup` command, the external `setup.sh` +# has no support to mount a volume to `/var/mail` (only via `-c` to use a running container), +# thus the `-y` option to delete the account maildir has no effect nor informs the user. +# https://github.com/docker-mailserver/docker-mailserver/issues/949 @test "checking setup.sh: setup.sh email del" { - run ./setup.sh -c "${TEST_NAME}" email del -y lorem@impsum.org + local MAIL_ACCOUNT='user@example.com' + local MAIL_PASS='test_password' + + # Account deletion is successful: + run ./setup.sh -c "${TEST_NAME}" email del -y "${MAIL_ACCOUNT}" assert_success - # TODO - # delmailuser does not work as expected. - # Its implementation is not functional, you cannot delete a user data - # directory in the running container by running a new docker container - # and not mounting the mail folders (persistance is broken). - # The add script is only adding the user to account file. + # Mail storage for account was actually removed by `-y`: + # NOTE: Sometimes the directory still exists, possibly from change detection + # of the previous test (`email udpate`) triggering. If this does not fix it, + # use `wait_until_change_detection_event_completes()` in that test to ensure + # consistency, or create another user to delete. + repeat_in_container_until_success_or_timeout 60 "${TEST_NAME}" bash -c '[[ ! -d /var/mail/example.com/user ]]' + #run docker exec "${TEST_NAME}" bash -c '[[ ! -d /var/mail/example.com/user ]]' + #assert_success - # run docker exec "${TEST_NAME}" ls /var/mail/impsum.org/lorem - # assert_failure - run grep lorem@impsum.org "${TEST_TMP_CONFIG}/postfix-accounts.cf" + # Account is not present in `postfix-accounts.cf`: + run grep "${MAIL_ACCOUNT}" "${TEST_TMP_CONFIG}/postfix-accounts.cf" assert_failure + + # NOTE: Actual account will still exist briefly in Dovecot UserDB+PassDB + # until `changedetector` service is triggered by `postfix-accounts.cf` + # which will rebuild Dovecots accounts from scratch. } @test "checking setup.sh: setup.sh email restrict" { diff --git a/test/test_helper/common.bash b/test/test_helper/common.bash index 821056e4..753124ff 100644 --- a/test/test_helper/common.bash +++ b/test/test_helper/common.bash @@ -173,6 +173,73 @@ function wait_for_changes_to_be_detected_in_container() { repeat_in_container_until_success_or_timeout "${TIMEOUT}" "${CONTAINER_NAME}" bash -c 'source /usr/local/bin/helpers/index.sh; _obtain_hostname_and_domainname; cmp --silent -- <(_monitored_files_checksums) "${CHKSUM_FILE}" >/dev/null' } +# Relies on ENV `LOG_LEVEL=debug` or higher +function wait_until_change_detection_event_completes() { + local CONTAINER_NAME="${1}" + # Ensure early failure if arg is missing: + assert_not_equal "${CONTAINER_NAME}" "" + + # Ensure the container is configured with the required `LOG_LEVEL` ENV: + assert_regex \ + $(docker exec "${CONTAINER_NAME}" env | grep '^LOG_LEVEL=') \ + '=(debug|trace)$' + + local CHANGE_EVENT_START='Change detected' + local CHANGE_EVENT_END='Completed handling of detected change' # debug log + + function __change_event_status() { + docker exec "${CONTAINER_NAME}" \ + grep -oE "${CHANGE_EVENT_START}|${CHANGE_EVENT_END}" /var/log/supervisor/changedetector.log \ + | tail -1 + } + + function __is_changedetector_processing() { + [[ $(__change_event_status) == "${CHANGE_EVENT_START}" ]] + } + + function __is_changedetector_finished() { + [[ $(__change_event_status) == "${CHANGE_EVENT_END}" ]] + } + + if [[ ! $(__is_changedetector_processing) ]] + then + # A new change event is expected, wait for it: + repeat_until_success_or_timeout 60 __is_changedetector_processing + fi + + # Change event is in progress, wait until it finishes: + repeat_until_success_or_timeout 60 __is_changedetector_finished + + # NOTE: Although the change event has completed, services like Postfix and Dovecot + # may still be in the process of restarting. + # You may still want to wait longer if depending on those to be ready. +} + +# An account added to `postfix-accounts.cf` must wait for the `changedetector` service +# to process the update before Dovecot creates the mail account and associated storage dir: +function wait_until_account_maildir_exists() { + local CONTAINER_NAME=$1 + local MAIL_ACCOUNT=$2 + + local LOCAL_PART="${MAIL_ACCOUNT%@*}" + local DOMAIN_PART="${MAIL_ACCOUNT#*@}" + local MAIL_ACCOUNT_STORAGE_DIR="/var/mail/${DOMAIN_PART}/${LOCAL_PART}" + + repeat_in_container_until_success_or_timeout 60 "${CONTAINER_NAME}" bash -c "[[ -d ${MAIL_ACCOUNT_STORAGE_DIR} ]]" +} + +function add_mail_account_then_wait_until_ready() { + local CONTAINER_NAME=$1 + local MAIL_ACCOUNT=$2 + # Password is optional (omit when the password is not needed during the test) + local MAIL_PASS="${3:-password_not_relevant_to_test}" + + run docker exec "${CONTAINER_NAME}" setup email add "${MAIL_ACCOUNT}" "${MAIL_PASS}" + assert_success + + wait_until_account_maildir_exists "${CONTAINER_NAME}" "${MAIL_ACCOUNT}" +} + function wait_for_empty_mail_queue_in_container() { local CONTAINER_NAME="${1}" local TIMEOUT=${TEST_TIMEOUT_IN_SECONDS} diff --git a/test/tests.bats b/test/tests.bats index 2f51bd61..1fbd89a5 100644 --- a/test/tests.bats +++ b/test/tests.bats @@ -10,6 +10,7 @@ setup_file() { PRIVATE_CONFIG=$(duplicate_config_for_container . mail) mv "${PRIVATE_CONFIG}/user-patches/user-patches.sh" "${PRIVATE_CONFIG}/user-patches.sh" + # `LOG_LEVEL=debug` required for using `wait_until_change_detection_event_completes()` docker run --rm -d --name mail \ -v "${PRIVATE_CONFIG}":/tmp/docker-mailserver \ -v "$(pwd)/test/test-files":/tmp/docker-mailserver-test:ro \ @@ -22,6 +23,7 @@ setup_file() { -e ENABLE_SPAMASSASSIN=1 \ -e ENABLE_SRS=1 \ -e ENABLE_UPDATE_CHECK=0 \ + -e LOG_LEVEL='debug' \ -e PERMIT_DOCKER=container \ -e PERMIT_DOCKER=host \ -e PFLOGSUMM_TRIGGER=logrotate \ @@ -49,16 +51,18 @@ setup_file() { # setup sieve docker cp "${PRIVATE_CONFIG}/sieve/dovecot.sieve" mail:/var/mail/localhost.localdomain/user1/.dovecot.sieve - # this relies on the checksum file beeing updated after all changes have been applied - wait_for_changes_to_be_detected_in_container mail - - wait_for_smtp_port_in_container mail + # this relies on the checksum file being updated after all changes have been applied + wait_until_change_detection_event_completes mail # wait for ClamAV to be fully setup or we will get errors on the log repeat_in_container_until_success_or_timeout 60 mail test -e /var/run/clamav/clamd.ctl - # sending test mails - docker exec mail /bin/sh -c "nc 0.0.0.0 25 < /tmp/docker-mailserver-test/email-templates/amavis-spam.txt" + wait_for_smtp_port_in_container mail + + # The first mail sent leverages an assert for better error output if a failure occurs: + run docker exec mail /bin/sh -c "nc 0.0.0.0 25 < /tmp/docker-mailserver-test/email-templates/amavis-spam.txt" + assert_success + docker exec mail /bin/sh -c "nc 0.0.0.0 25 < /tmp/docker-mailserver-test/email-templates/amavis-virus.txt" docker exec mail /bin/sh -c "nc 0.0.0.0 25 < /tmp/docker-mailserver-test/email-templates/existing-alias-external.txt" docker exec mail /bin/sh -c "nc 0.0.0.0 25 < /tmp/docker-mailserver-test/email-templates/existing-alias-local.txt" @@ -97,6 +101,21 @@ teardown_file() { assert_success } +# +# healthcheck +# + +# NOTE: Healthcheck defaults an interval of 30 seconds +# If Postfix is temporarily down (eg: restart triggered by `check-for-changes.sh`), +# it may result in a false-positive `unhealthy` state. +# Be careful with re-locating this test if earlier tests could potentially fail it by +# triggering the `changedetector` service. +@test "checking container healthcheck" { + run bash -c "docker inspect mail | jq -r '.[].State.Health.Status'" + assert_output "healthy" + assert_success +} + # # processes # @@ -627,6 +646,8 @@ EOF } @test "checking accounts: user3 should have been removed from /tmp/docker-mailserver/postfix-accounts.cf but not auser3" { + wait_until_account_maildir_exists mail 'user3@domain.tld' + docker exec mail /bin/sh -c "delmailuser -y user3@domain.tld" run docker exec mail /bin/sh -c "grep '^user3@domain\.tld' -i /tmp/docker-mailserver/postfix-accounts.cf" @@ -639,7 +660,7 @@ EOF } @test "checking user updating password for user in /tmp/docker-mailserver/postfix-accounts.cf" { - docker exec mail /bin/sh -c "addmailuser user4@domain.tld mypassword" + add_mail_account_then_wait_until_ready mail 'user4@domain.tld' initialpass=$(docker exec mail /bin/sh -c "grep '^user4@domain\.tld' -i /tmp/docker-mailserver/postfix-accounts.cf") sleep 2 @@ -689,8 +710,7 @@ EOF @test "checking quota: setquota user must be existing" { - run docker exec mail /bin/sh -c "addmailuser quota_user@domain.tld mypassword" - assert_success + add_mail_account_then_wait_until_ready mail 'quota_user@domain.tld' run docker exec mail /bin/sh -c "setquota quota_user 50M" assert_failure @@ -703,9 +723,9 @@ EOF run docker exec mail /bin/sh -c "delmailuser -y quota_user@domain.tld" assert_success } + @test "checking quota: setquota must be well formatted" { - run docker exec mail /bin/sh -c "addmailuser quota_user@domain.tld mypassword" - assert_success + add_mail_account_then_wait_until_ready mail 'quota_user@domain.tld' run docker exec mail /bin/sh -c "setquota quota_user@domain.tld 26GIGOTS" assert_failure @@ -733,10 +753,8 @@ EOF assert_success } - @test "checking quota: delquota user must be existing" { - run docker exec mail /bin/sh -c "addmailuser quota_user@domain.tld mypassword" - assert_success + add_mail_account_then_wait_until_ready mail 'quota_user@domain.tld' run docker exec mail /bin/sh -c "delquota uota_user@domain.tld" assert_failure @@ -755,9 +773,9 @@ EOF run docker exec mail /bin/sh -c "delmailuser -y quota_user@domain.tld" assert_success } + @test "checking quota: delquota allow when no quota for existing user" { - run docker exec mail /bin/sh -c "addmailuser quota_user@domain.tld mypassword" - assert_success + add_mail_account_then_wait_until_ready mail 'quota_user@domain.tld' run docker exec mail /bin/sh -c "grep -i 'quota_user@domain.tld' /tmp/docker-mailserver/dovecot-quotas.cf" assert_failure @@ -811,8 +829,7 @@ EOF } @test "checking quota: quota directive is removed when mailbox is removed" { - run docker exec mail /bin/sh -c "addmailuser quserremoved@domain.tld mypassword" - assert_success + add_mail_account_then_wait_until_ready mail 'quserremoved@domain.tld' run docker exec mail /bin/sh -c "setquota quserremoved@domain.tld 12M" assert_success @@ -828,16 +845,12 @@ EOF } @test "checking quota: dovecot applies user quota" { - wait_for_changes_to_be_detected_in_container mail - run docker exec mail /bin/sh -c "doveadm quota get -u 'user1@localhost.localdomain' | grep 'User quota STORAGE'" assert_output --partial "- 0" run docker exec mail /bin/sh -c "setquota user1@localhost.localdomain 50M" assert_success - wait_for_changes_to_be_detected_in_container mail - # wait until quota has been updated run repeat_until_success_or_timeout 20 sh -c "docker exec mail sh -c 'doveadm quota get -u user1@localhost.localdomain | grep -oP \"(User quota STORAGE\s+[0-9]+\s+)51200(.*)\"'" assert_success @@ -845,8 +858,6 @@ EOF run docker exec mail /bin/sh -c "delquota user1@localhost.localdomain" assert_success - wait_for_changes_to_be_detected_in_container mail - # wait until quota has been updated run repeat_until_success_or_timeout 20 sh -c "docker exec mail sh -c 'doveadm quota get -u user1@localhost.localdomain | grep -oP \"(User quota STORAGE\s+[0-9]+\s+)-(.*)\"'" assert_success @@ -855,14 +866,11 @@ EOF @test "checking quota: warn message received when quota exceeded" { skip 'disabled as it fails randomly: https://github.com/docker-mailserver/docker-mailserver/pull/2511' - wait_for_changes_to_be_detected_in_container mail - # create user - run docker exec mail /bin/sh -c "addmailuser quotauser@otherdomain.tld mypassword && setquota quotauser@otherdomain.tld 10k" + add_mail_account_then_wait_until_ready mail 'quotauser@otherdomain.tld' + run docker exec mail /bin/sh -c 'setquota quotauser@otherdomain.tld 10k' assert_success - wait_for_changes_to_be_detected_in_container mail - # wait until quota has been updated run repeat_until_success_or_timeout 20 sh -c "docker exec mail sh -c 'doveadm quota get -u quotauser@otherdomain.tld | grep -oP \"(User quota STORAGE\s+[0-9]+\s+)10(.*)\"'" assert_success @@ -967,16 +975,6 @@ EOF assert_failure } -# -# healthcheck -# - -@test "checking container healthcheck" { - run bash -c "docker inspect mail | jq -r '.[].State.Health.Status'" - assert_output "healthy" - assert_success -} - # # supervisor #