diff options
Diffstat (limited to 'src')
| -rw-r--r-- | src/mailman/app/membership.py | 36 | ||||
| -rw-r--r-- | src/mailman/app/tests/test_membership.py | 26 | ||||
| -rw-r--r-- | src/mailman/bin/docs/master.rst | 8 | ||||
| -rw-r--r-- | src/mailman/bin/mailman.py | 2 | ||||
| -rw-r--r-- | src/mailman/bin/runner.py | 21 | ||||
| -rw-r--r-- | src/mailman/commands/cli_inject.py | 2 | ||||
| -rw-r--r-- | src/mailman/commands/cli_status.py | 2 | ||||
| -rw-r--r-- | src/mailman/commands/cli_withlist.py | 2 | ||||
| -rw-r--r-- | src/mailman/commands/docs/conf.rst | 4 | ||||
| -rw-r--r-- | src/mailman/commands/docs/members.rst | 4 | ||||
| -rw-r--r-- | src/mailman/commands/tests/test_create.py | 4 | ||||
| -rw-r--r-- | src/mailman/config/config.py | 8 | ||||
| -rw-r--r-- | src/mailman/config/schema.cfg | 8 | ||||
| -rw-r--r-- | src/mailman/core/logging.py | 4 | ||||
| -rw-r--r-- | src/mailman/docs/8-miles-high.rst | 6 | ||||
| -rw-r--r-- | src/mailman/docs/NEWS.rst | 51 | ||||
| -rw-r--r-- | src/mailman/docs/START.rst | 7 | ||||
| -rw-r--r-- | src/mailman/rest/tests/test_membership.py | 44 |
18 files changed, 157 insertions, 82 deletions
diff --git a/src/mailman/app/membership.py b/src/mailman/app/membership.py index c50169a7c..996493dc4 100644 --- a/src/mailman/app/membership.py +++ b/src/mailman/app/membership.py @@ -32,7 +32,8 @@ from mailman.core.i18n import _ from mailman.email.message import OwnerNotification from mailman.interfaces.bans import IBanManager from mailman.interfaces.member import ( - MemberRole, MembershipIsBannedError, NotAMemberError, SubscriptionEvent) + AlreadySubscribedError, MemberRole, MembershipIsBannedError, + NotAMemberError, SubscriptionEvent) from mailman.interfaces.usermanager import IUserManager from mailman.utilities.i18n import make from zope.component import getUtility @@ -96,15 +97,32 @@ def add_member(mlist, email, display_name, password, delivery_mode, language, member = mlist.subscribe(address, role) member.preferences.delivery_mode = delivery_mode else: - # The user exists and is linked to the address. + # The user exists and is linked to the case-insensitive address. + # We're looking for two versions of the email address, the case + # preserved version and the case insensitive version. We'll + # subscribe the version with matching case if it exists, otherwise + # we'll use one of the matching case-insensitively ones. It's + # undefined which one we pick. + case_preserved = None + case_insensitive = None for address in user.addresses: - if address.email == email: - break - else: - raise AssertionError( - 'User should have had linked address: {0}'.format(address)) - # Create the member and set the appropriate preferences. - member = mlist.subscribe(address, role) + if address.original_email == email: + case_preserved = address + if address.email == email.lower(): + case_insensitive = address + assert case_preserved is not None or case_insensitive is not None, ( + 'Could not find a linked address for: {}'.format(email)) + address = (case_preserved if case_preserved is not None + else case_insensitive) + # Create the member and set the appropriate preferences. It's + # possible we're subscribing the lower cased version of the address; + # if that's already subscribed re-issue the exception with the correct + # email address (i.e. the one passed in here). + try: + member = mlist.subscribe(address, role) + except AlreadySubscribedError as error: + raise AlreadySubscribedError( + error.fqdn_listname, email, error.role) member.preferences.preferred_language = language member.preferences.delivery_mode = delivery_mode return member diff --git a/src/mailman/app/tests/test_membership.py b/src/mailman/app/tests/test_membership.py index 9b42c21d6..cdf0641ea 100644 --- a/src/mailman/app/tests/test_membership.py +++ b/src/mailman/app/tests/test_membership.py @@ -166,6 +166,32 @@ class TestAddMember(unittest.TestCase): self.assertEqual(member_1.role, MemberRole.member) self.assertEqual(member_2.role, MemberRole.owner) + def test_add_member_with_mixed_case_email(self): + # LP: #1425359 - Mailman is case-perserving, case-insensitive. This + # test subscribes the lower case address and ensures the original + # mixed case address can't be subscribed. + email = 'APerson@example.com' + add_member(self._mlist, email.lower(), 'Ann Person', '123', + DeliveryMode.regular, system_preferences.preferred_language) + with self.assertRaises(AlreadySubscribedError) as cm: + add_member(self._mlist, email, 'Ann Person', '123', + DeliveryMode.regular, + system_preferences.preferred_language) + self.assertEqual(cm.exception.email, email) + + def test_add_member_with_lower_case_email(self): + # LP: #1425359 - Mailman is case-perserving, case-insensitive. This + # test subscribes the mixed case address and ensures the lower cased + # address can't be added. + email = 'APerson@example.com' + add_member(self._mlist, email, 'Ann Person', '123', + DeliveryMode.regular, system_preferences.preferred_language) + with self.assertRaises(AlreadySubscribedError) as cm: + add_member(self._mlist, email.lower(), 'Ann Person', '123', + DeliveryMode.regular, + system_preferences.preferred_language) + self.assertEqual(cm.exception.email, email.lower()) + class TestAddMemberPassword(unittest.TestCase): diff --git a/src/mailman/bin/docs/master.rst b/src/mailman/bin/docs/master.rst index 02c4bd976..5a3a94da6 100644 --- a/src/mailman/bin/docs/master.rst +++ b/src/mailman/bin/docs/master.rst @@ -4,10 +4,10 @@ Mailman runner control Mailman has a number of *runner subprocesses* which perform long-running tasks such as listening on an LMTP port, processing REST API requests, or processing -messages in a queue directory. In normal operation, the ``mailman`` -command is used to start, stop and manage the runners. This is just a wrapper -around the real master watcher, which handles runner starting, stopping, -exiting, and log file reopening. +messages in a queue directory. In normal operation, the ``mailman`` command +is used to start, stop and manage the runners. This is just a wrapper around +the real master watcher, which handles runner starting, stopping, exiting, and +log file reopening. >>> from mailman.testing.helpers import TestableMaster diff --git a/src/mailman/bin/mailman.py b/src/mailman/bin/mailman.py index 1cfff19db..3865fef19 100644 --- a/src/mailman/bin/mailman.py +++ b/src/mailman/bin/mailman.py @@ -36,7 +36,7 @@ from zope.interface.verify import verifyObject def main(): - """mailman""" + """The `mailman` command dispatcher.""" # Create the basic parser and add all globally common options. parser = argparse.ArgumentParser( description=_("""\ diff --git a/src/mailman/bin/runner.py b/src/mailman/bin/runner.py index a981694b8..87d11dbe9 100644 --- a/src/mailman/bin/runner.py +++ b/src/mailman/bin/runner.py @@ -108,19 +108,18 @@ def main(): description=_("""\ Start a runner - The runner named on the command line is started, and it can - either run through its main loop once (for those runners that - support this) or continuously. The latter is how the master - runner starts all its subprocesses. + The runner named on the command line is started, and it can either run + through its main loop once (for those runners that support this) or + continuously. The latter is how the master runner starts all its + subprocesses. - -r is required unless -l or -h is given, and its argument must - be one of the names displayed by the -l switch. + -r is required unless -l or -h is given, and its argument must be one + of the names displayed by the -l switch. - Normally, this script should be started from 'mailman - start'. Running it separately or with -o is generally useful - only for debugging. When run this way, the environment variable - $MAILMAN_UNDER_MASTER_CONTROL will be set which subtly changes - some error handling behavior. + Normally, this script should be started from 'mailman start'. Running + it separately or with -o is generally useful only for debugging. When + run this way, the environment variable $MAILMAN_UNDER_MASTER_CONTROL + will be set which subtly changes some error handling behavior. """)) parser.add_argument( '--version', diff --git a/src/mailman/commands/cli_inject.py b/src/mailman/commands/cli_inject.py index fa71cbed8..1b7f15f7b 100644 --- a/src/mailman/commands/cli_inject.py +++ b/src/mailman/commands/cli_inject.py @@ -15,7 +15,7 @@ # You should have received a copy of the GNU General Public License along with # GNU Mailman. If not, see <http://www.gnu.org/licenses/>. -"""mailman inject""" +"""The `mailman inject` subcommand.""" __all__ = [ 'Inject', diff --git a/src/mailman/commands/cli_status.py b/src/mailman/commands/cli_status.py index 2e8ff0ba2..7faab7941 100644 --- a/src/mailman/commands/cli_status.py +++ b/src/mailman/commands/cli_status.py @@ -15,7 +15,7 @@ # You should have received a copy of the GNU General Public License along with # GNU Mailman. If not, see <http://www.gnu.org/licenses/>. -"""mailman status.""" +"""The `mailman status` subcommand.""" __all__ = [ 'Status', diff --git a/src/mailman/commands/cli_withlist.py b/src/mailman/commands/cli_withlist.py index d5ba35b6e..e3307d7b4 100644 --- a/src/mailman/commands/cli_withlist.py +++ b/src/mailman/commands/cli_withlist.py @@ -15,7 +15,7 @@ # You should have received a copy of the GNU General Public License along with # GNU Mailman. If not, see <http://www.gnu.org/licenses/>. -"""mailman withlist""" +"""The `mailman shell` subcommand.""" __all__ = [ 'Shell', diff --git a/src/mailman/commands/docs/conf.rst b/src/mailman/commands/docs/conf.rst index a76691e63..1db85918f 100644 --- a/src/mailman/commands/docs/conf.rst +++ b/src/mailman/commands/docs/conf.rst @@ -7,8 +7,8 @@ lets you dump one or more Mailman configuration variables to standard output or a file. Mailman's configuration is divided in multiple sections which contain multiple -key-value pairs. The ``mailman conf`` command allows you to display -a specific key-value pair, or several key-value pairs. +key-value pairs. The ``mailman conf`` command allows you to display a +specific key-value pair, or several key-value pairs. >>> class FakeArgs: ... key = None diff --git a/src/mailman/commands/docs/members.rst b/src/mailman/commands/docs/members.rst index fd1c8b1be..c90418181 100644 --- a/src/mailman/commands/docs/members.rst +++ b/src/mailman/commands/docs/members.rst @@ -2,8 +2,8 @@ Managing members ================ -The ``mailman members`` command allows a site administrator to display, -add, and remove members from a mailing list. +The ``mailman members`` command allows a site administrator to display, add, +and remove members from a mailing list. :: >>> mlist1 = create_list('test1@example.com') diff --git a/src/mailman/commands/tests/test_create.py b/src/mailman/commands/tests/test_create.py index a3349fb60..d7e17e5d2 100644 --- a/src/mailman/commands/tests/test_create.py +++ b/src/mailman/commands/tests/test_create.py @@ -15,7 +15,7 @@ # You should have received a copy of the GNU General Public License along with # GNU Mailman. If not, see <http://www.gnu.org/licenses/>. -"""Test `mailman create`.""" +"""Test the `mailman create` subcommand.""" __all__ = [ 'TestCreate', @@ -51,8 +51,6 @@ class FakeParser: class TestCreate(unittest.TestCase): - """Test `mailman create`.""" - layer = ConfigLayer def setUp(self): diff --git a/src/mailman/config/config.py b/src/mailman/config/config.py index 4d0d57cd5..d23bdda13 100644 --- a/src/mailman/config/config.py +++ b/src/mailman/config/config.py @@ -149,10 +149,10 @@ class Configuration: # First, collect all variables in a substitution dictionary. $VAR_DIR # is taken from the environment or from the configuration file if the # environment is not set. Because the var_dir setting in the config - # file could be a relative path, and because 'mailman start' - # chdirs to $VAR_DIR, without this subprocesses bin/master and - # bin/runner will create $VAR_DIR hierarchies under $VAR_DIR when that - # path is relative. + # file could be a relative path, and because 'mailman start' chdirs to + # $VAR_DIR, without this subprocesses bin/master and bin/runner will + # create $VAR_DIR hierarchies under $VAR_DIR when that path is + # relative. var_dir = os.environ.get('MAILMAN_VAR_DIR', category.var_dir) substitutions = dict( cwd = os.getcwd(), diff --git a/src/mailman/config/schema.cfg b/src/mailman/config/schema.cfg index bd3f4f2dd..f8c3a117e 100644 --- a/src/mailman/config/schema.cfg +++ b/src/mailman/config/schema.cfg @@ -66,10 +66,10 @@ filtered_messages_are_preservable: no [shell] -# `mailman shell` (also `withlist`) gives you an interactive prompt that -# you can use to interact with an initialized and configured Mailman system. -# Use --help for more information. This section allows you to configure -# certain aspects of this interactive shell. +# `mailman shell` (also `withlist`) gives you an interactive prompt that you +# can use to interact with an initialized and configured Mailman system. Use +# --help for more information. This section allows you to configure certain +# aspects of this interactive shell. # Customize the interpreter prompt. prompt: >>> diff --git a/src/mailman/core/logging.py b/src/mailman/core/logging.py index 21c86a725..7c80037f6 100644 --- a/src/mailman/core/logging.py +++ b/src/mailman/core/logging.py @@ -103,8 +103,8 @@ def _init_logger(propagate, sub_name, log, logger_config): # Get settings from log configuration file (or defaults). log_format = logger_config.format log_datefmt = logger_config.datefmt - # Propagation to the root logger is how we handle logging to stderr - # when the runners are not run as a subprocess of 'mailman start'. + # Propagation to the root logger is how we handle logging to stderr when + # the runners are not run as a subprocess of 'mailman start'. log.propagate = (as_boolean(logger_config.propagate) if propagate is None else propagate) # Set the logger's level. diff --git a/src/mailman/docs/8-miles-high.rst b/src/mailman/docs/8-miles-high.rst index 96ce3c6f8..ae3074e1c 100644 --- a/src/mailman/docs/8-miles-high.rst +++ b/src/mailman/docs/8-miles-high.rst @@ -162,9 +162,9 @@ when the Mailman daemon starts, and what queue the Runner manages. Shell Commands ============== -`mailman`: This is an ubercommand, with subcommands for all the various -things admins might want to do, similar to Mailman 2's mailmanctl, but with -more functionality. +`mailman`: This is an ubercommand, with subcommands for all the various things +admins might want to do, similar to Mailman 2's mailmanctl, but with more +functionality. `bin/master`: The runner manager: starts, watches, stops the runner daemons. diff --git a/src/mailman/docs/NEWS.rst b/src/mailman/docs/NEWS.rst index 20065b25e..381cd5e03 100644 --- a/src/mailman/docs/NEWS.rst +++ b/src/mailman/docs/NEWS.rst @@ -21,6 +21,9 @@ Bugs (LP: #1418280) * When deleting a user via REST, make sure all linked addresses are deleted. Found by Andrew Stuart. (LP: #1419519) + * When trying to subscribe an address to a mailing list through the REST API + where a case-differing version of the address is already subscribed, return + a 409 error instead of a 500 error. Found by Ankush Sharma. (LP: #1425359) Configuration ------------- @@ -173,7 +176,7 @@ Configuration ------------- * Add support for the Exim 4 MTA. [Stephen Turnbull] * When creating the initial file system layout in ``var``, e.g. via - ``mailman info``, add an ``var/etc/mailman.cfg`` file if one does not + ``bin/mailman info``, add an ``var/etc/mailman.cfg`` file if one does not already exist. Also, when initializing the system, look for that file as the configuration file, just after ``./mailman.cfg`` and before ``~/.mailman.cfg``. (LP: #1157861) @@ -291,10 +294,10 @@ Interfaces Commands -------- - * `mailman aliases` loses the `--output`, `--format`, and `--simple` + * `bin/mailman aliases` loses the `--output`, `--format`, and `--simple` arguments, and adds a `--directory` argument. This is necessary to support the Postfix `relay_domains` support. - * `mailman start` was passing the wrong relative path to its runner + * `bin/mailman start` was passing the wrong relative path to its runner subprocesses when -C was given. (LP: #982551) * `bin/runner` command has been simplified and its command line options reduced. Now, only one `-r/--runner` option may be provided and the @@ -437,7 +440,7 @@ Bug fixes (LP: #953497) * List-Post should be NO when posting is not allowed. (LP: #987563) * Non-unicode values in msgdata broke pending requests. (LP: #1031391) - * Show devmode in `mailman info` output. (LP: #1035028) + * Show devmode in `bin/mailman info` output. (LP: #1035028) * Fix residual references to the old `IMailingList` archive variables. (LP: #1031393) @@ -546,11 +549,11 @@ Interfaces Commands -------- - * IPython support in `mailman shell` contributed by Andrea Crotti. + * IPython support in `bin/mailman shell` contributed by Andrea Crotti. (LP: #949926). * The `mailman.cfg` configuration file will now automatically be detected if it exists in an `etc` directory which is a sibling of argv0. - * `mailman shell` is an alias for `withlist`. + * `bin/mailman shell` is an alias for `withlist`. * The `confirm` email command now properly handles `Re:`-like prefixes, even if they contain non-ASCII characters. (LP: #685261) * The `join` email command no longer accepts an `address=` argument. Its @@ -638,10 +641,10 @@ REST Commands -------- * `bin/qrunner` is renamed to `bin/runner`. - * `mailman aliases` gains `-f` and `-s` options. - * `mailman create` no longer allows a list to be created with bogus owner + * `bin/mailman aliases` gains `-f` and `-s` options. + * `bin/mailman create` no longer allows a list to be created with bogus owner addresses. (LP: #778687) - * `mailman start --force` option is fixed. (LP: #869317) + * `bin/mailman start --force` option is fixed. (LP: #869317) Documentation ------------- @@ -715,11 +718,11 @@ Configuration Commands -------- - * 'mailman start' does a better job of producing an error when Mailman is + * 'bin/mailman start' does a better job of producing an error when Mailman is already running. - * 'mailman status' added for providing command line status on the master + * 'bin/mailman status' added for providing command line status on the master queue runner watcher process. - * 'mailman info' now prints the REST root url and credentials. + * 'bin/mailman info' now prints the REST root url and credentials. * mmsitepass removed; there is no more site password. REST @@ -763,8 +766,8 @@ Bugs fixed Commands -------- * The functionality of 'bin/list_members' has been moved to - 'mailman members'. - * 'mailman info' -v/--verbose output displays the file system + 'bin/mailman members'. + * 'bin/mailman info' -v/--verbose output displays the file system layout paths Mailman is currently configured to use. Configuration @@ -825,8 +828,8 @@ REST Commands -------- - * 'bin/dumpdb' is now 'mailman qfile' - * 'bin/unshunt' is now 'mailman unshunt' + * 'bin/dumpdb' is now 'bin/mailman qfile' + * 'bin/unshunt' is now 'bin/mailman unshunt' * Mailman now properly handles the '-join', '-leave', and '-confirm' email commands and sub-addresses. '-subscribe' and '-unsubscribe' are aliases for '-join' and '-leave' respectively. @@ -850,16 +853,16 @@ Configuration Commands -------- - * 'inject' is now 'mailman inject', with some changes - * 'mailmanctl' is now 'mailman start|stop|reopen|restart' - * 'mailman version' is added (output same as 'mailman --version') - * 'mailman members' command line arguments have changed. It also + * 'bin/inject' is now 'bin/mailman inject', with some changes + * 'bin/mailmanctl' is now 'bin/mailman start|stop|reopen|restart' + * 'bin/mailman version' is added (output same as 'bin/mailman --version') + * 'bin/mailman members' command line arguments have changed. It also now ignores blank lines and lines that start with #. It also no longer quits when it sees an address that's already subscribed. - * 'bin/withlist' is now 'mailman withlist', and its command line + * 'bin/withlist' is now 'bin/mailman withlist', and its command line arguments have changed. - * 'mailman lists' command line arguments have changed. - * 'bin/genaliases' is now 'mailman aliases' + * 'bin/mailman lists' command line arguments have changed. + * 'bin/genaliases' is now 'bin/mailman aliases' Architecture ------------ @@ -901,7 +904,7 @@ Configuration Architecture ------------ - * 'mailman' is a new super-command for managing Mailman from the command + * 'bin/mailman' is a new super-command for managing Mailman from the command line. Some older bin scripts have been converted, with more to come. * Mailman now has an administrative REST interface which can be used to get information from and manage Mailman remotely. diff --git a/src/mailman/docs/START.rst b/src/mailman/docs/START.rst index c604299e7..3ca4460b4 100644 --- a/src/mailman/docs/START.rst +++ b/src/mailman/docs/START.rst @@ -188,10 +188,9 @@ The first existing file found wins. * ``/etc/mailman.cfg`` * ``argv[0]/../../etc/mailman.cfg`` -Run the ``mailman info`` command to see which configuration file Mailman -will use, and where it will put its database file. The first time you run -this, Mailman will also create any necessary run-time directories and log -files. +Run the ``mailman info`` command to see which configuration file Mailman will +use, and where it will put its database file. The first time you run this, +Mailman will also create any necessary run-time directories and log files. Try ``mailman --help`` for more details. You can use the commands ``mailman start`` to start the runner subprocess daemons, and of course diff --git a/src/mailman/rest/tests/test_membership.py b/src/mailman/rest/tests/test_membership.py index e1bff833b..a77dea3b5 100644 --- a/src/mailman/rest/tests/test_membership.py +++ b/src/mailman/rest/tests/test_membership.py @@ -39,6 +39,12 @@ from urllib.error import HTTPError from zope.component import getUtility +def _set_preferred(user): + preferred = list(user.addresses)[0] + preferred.verified_on = now() + user.preferred_address = preferred + + class TestMembership(unittest.TestCase): layer = RESTLayer @@ -98,6 +104,36 @@ class TestMembership(unittest.TestCase): self.assertEqual(cm.exception.code, 409) self.assertEqual(cm.exception.reason, b'Member already subscribed') + def test_add_member_with_mixed_case_email(self): + # LP: #1425359 - Mailman is case-perserving, case-insensitive. This + # test subscribes the lower case address and ensures the original mixed + # case address can't be subscribed. + with transaction(): + anne = self._usermanager.create_address('anne@example.com') + self._mlist.subscribe(anne) + with self.assertRaises(HTTPError) as cm: + call_api('http://localhost:9001/3.0/members', { + 'list_id': 'test.example.com', + 'subscriber': 'ANNE@example.com', + }) + self.assertEqual(cm.exception.code, 409) + self.assertEqual(cm.exception.reason, b'Member already subscribed') + + def test_add_member_with_lower_case_email(self): + # LP: #1425359 - Mailman is case-perserving, case-insensitive. This + # test subscribes the mixed case address and ensures the lower cased + # address can't be added. + with transaction(): + anne = self._usermanager.create_address('ANNE@example.com') + self._mlist.subscribe(anne) + with self.assertRaises(HTTPError) as cm: + call_api('http://localhost:9001/3.0/members', { + 'list_id': 'test.example.com', + 'subscriber': 'anne@example.com', + }) + self.assertEqual(cm.exception.code, 409) + self.assertEqual(cm.exception.reason, b'Member already subscribed') + def test_join_with_invalid_delivery_mode(self): with self.assertRaises(HTTPError) as cm: call_api('http://localhost:9001/3.0/members', { @@ -129,9 +165,7 @@ class TestMembership(unittest.TestCase): def test_join_as_user_with_preferred_address(self): with transaction(): anne = self._usermanager.create_user('anne@example.com') - preferred = list(anne.addresses)[0] - preferred.verified_on = now() - anne.preferred_address = preferred + _set_preferred(anne) self._mlist.subscribe(anne) content, response = call_api('http://localhost:9001/3.0/members') self.assertEqual(response.status, 200) @@ -150,9 +184,7 @@ class TestMembership(unittest.TestCase): def test_member_changes_preferred_address(self): with transaction(): anne = self._usermanager.create_user('anne@example.com') - preferred = list(anne.addresses)[0] - preferred.verified_on = now() - anne.preferred_address = preferred + _set_preferred(anne) self._mlist.subscribe(anne) # Take a look at Anne's current membership. content, response = call_api('http://localhost:9001/3.0/members') |