summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--TODO.rst3
-rw-r--r--src/mailman/app/registrar.py11
-rw-r--r--src/mailman/app/tests/test_registrar.py8
-rw-r--r--src/mailman/app/tests/test_subscriptions.py6
-rw-r--r--src/mailman/commands/docs/membership.rst50
-rw-r--r--src/mailman/commands/eml_confirm.py7
-rw-r--r--src/mailman/commands/eml_membership.py49
-rw-r--r--src/mailman/database/transaction.py17
-rw-r--r--src/mailman/interfaces/registrar.py9
-rw-r--r--src/mailman/interfaces/workflow.py17
-rw-r--r--src/mailman/model/docs/registration.rst2
-rw-r--r--src/mailman/model/tests/test_workflow.py84
-rw-r--r--src/mailman/model/workflow.py8
-rw-r--r--src/mailman/runners/docs/command.rst1
-rw-r--r--src/mailman/runners/tests/test_join.py4
15 files changed, 175 insertions, 101 deletions
diff --git a/TODO.rst b/TODO.rst
index 6ff3c3562..6e7774cd8 100644
--- a/TODO.rst
+++ b/TODO.rst
@@ -4,4 +4,5 @@
- workflow for unsubscription
- make sure a user can't double confirm to bypass moderator approval
- make sure registration checks IEmailValidator
- - rosters must return users subscribed via their preferred email
+ - Test all the various options in eml_membership's get_subscriber()
+ - Bump version to 3.0.0
diff --git a/src/mailman/app/registrar.py b/src/mailman/app/registrar.py
index 5c4c46c1f..cc8e42a33 100644
--- a/src/mailman/app/registrar.py
+++ b/src/mailman/app/registrar.py
@@ -27,10 +27,12 @@ import logging
from mailman.app.subscriptions import SubscriptionWorkflow
from mailman.core.i18n import _
+from mailman.database.transaction import flush
from mailman.email.message import UserNotification
from mailman.interfaces.pending import IPendable, IPendings
from mailman.interfaces.registrar import ConfirmationNeededEvent, IRegistrar
from mailman.interfaces.templates import ITemplateLoader
+from mailman.interfaces.workflow import IWorkflowStateManager
from zope.component import getUtility
from zope.interface import implementer
@@ -67,13 +69,16 @@ class Registrar:
"""See `IRegistrar`."""
workflow = SubscriptionWorkflow(self._mlist)
workflow.token = token
- workflow.debug = True
workflow.restore()
list(workflow)
+ return workflow.token is None
def discard(self, token):
- # Throw the record away.
- getUtility(IPendings).confirm(token)
+ """See `IRegistrar`."""
+ with flush():
+ getUtility(IPendings).confirm(token)
+ getUtility(IWorkflowStateManager).discard(
+ SubscriptionWorkflow.__name__, token)
diff --git a/src/mailman/app/tests/test_registrar.py b/src/mailman/app/tests/test_registrar.py
index c8e0044de..3e3c30590 100644
--- a/src/mailman/app/tests/test_registrar.py
+++ b/src/mailman/app/tests/test_registrar.py
@@ -74,7 +74,8 @@ class TestRegistrar(unittest.TestCase):
# happens immediately.
self._mlist.subscription_policy = SubscriptionPolicy.open
self._anne.verified_on = now()
- self._registrar.register(self._anne)
+ status = self._registrar.register(self._anne)
+ self.assertIsNone(status)
member = self._mlist.regular_members.get_member('anne@example.com')
self.assertEqual(member.address, self._anne)
@@ -145,7 +146,10 @@ class TestRegistrar(unittest.TestCase):
self.assertIsNone(member)
# Now confirm the subscription, and wait for the moderator to approve
# the subscription. She is still not subscribed.
- self._registrar.confirm(token)
+ status = self._registrar.confirm(token)
+ # The status is not true because the user has not yet been subscribed
+ # to the mailing list.
+ self.assertFalse(status)
member = self._mlist.regular_members.get_member('anne@example.com')
self.assertIsNone(member)
# Confirm once more, this time as the moderator approving the
diff --git a/src/mailman/app/tests/test_subscriptions.py b/src/mailman/app/tests/test_subscriptions.py
index 7bb635a16..a8eaa4edb 100644
--- a/src/mailman/app/tests/test_subscriptions.py
+++ b/src/mailman/app/tests/test_subscriptions.py
@@ -365,11 +365,11 @@ class TestSubscriptionWorkflow(unittest.TestCase):
self.assertIsNone(workflow.token)
# The pendable associated with the token has been evicted.
self.assertIsNone(getUtility(IPendings).confirm(token, expunge=False))
- # There is no saved workflow associated with the token.
+ # There is no saved workflow associated with the token. This shows up
+ # as an exception when we try to restore the workflow.
new_workflow = SubscriptionWorkflow(self._mlist)
new_workflow.token = token
- new_workflow.restore()
- self.assertIsNone(new_workflow.which)
+ self.assertRaises(LookupError, new_workflow.restore)
def test_moderator_approves(self):
# The workflow runs until moderator approval is required, at which
diff --git a/src/mailman/commands/docs/membership.rst b/src/mailman/commands/docs/membership.rst
index bdebba8f7..49e80511d 100644
--- a/src/mailman/commands/docs/membership.rst
+++ b/src/mailman/commands/docs/membership.rst
@@ -70,7 +70,7 @@ The ``subscribe`` command is an alias.
Joining the sender
------------------
-When the message has a From field, that address will be subscribed.
+When the message has a ``From`` field, that address will be subscribed.
>>> msg = message_from_string("""\
... From: Anne Person <anne@example.com>
@@ -85,13 +85,10 @@ When the message has a From field, that address will be subscribed.
Confirmation email sent to Anne Person <anne@example.com>
<BLANKLINE>
-Anne is not yet a member because she must confirm her subscription request
-first.
+Anne is not yet a member of the mailing list because she must confirm her
+subscription request first.
- >>> from mailman.interfaces.usermanager import IUserManager
- >>> from zope.component import getUtility
- >>> user_manager = getUtility(IUserManager)
- >>> print(user_manager.get_user('anne@example.com'))
+ >>> print(mlist.members.get_member('anne@example.com'))
None
Mailman has sent her the confirmation message.
@@ -118,10 +115,7 @@ Mailman has sent her the confirmation message.
<BLANKLINE>
Before you can start using GNU Mailman at this site, you must first
confirm that this is your email address. You can do this by replying to
- this message, keeping the Subject header intact. Or you can visit this
- web page
- <BLANKLINE>
- http://lists.example.com/confirm/...
+ this message, keeping the Subject header intact.
<BLANKLINE>
If you do not wish to register this email address simply disregard this
message. If you think you are being maliciously subscribed to the list, or
@@ -130,8 +124,7 @@ Mailman has sent her the confirmation message.
alpha-owner@example.com
<BLANKLINE>
-Once Anne confirms her registration, she will be made a member of the mailing
-list.
+Anne confirms her registration.
::
>>> def extract_token(message):
@@ -156,13 +149,7 @@ list.
Confirmed
<BLANKLINE>
- >>> user = user_manager.get_user('anne@example.com')
- >>> print(user.display_name)
- Anne Person
- >>> list(user.addresses)
- [<Address: Anne Person <anne@example.com> [verified] at ...>]
-
-Anne is also now a member of the mailing list.
+Anne is now a member of the mailing list.
>>> mlist.members.get_member('anne@example.com')
<Member: Anne Person <anne@example.com>
@@ -180,12 +167,7 @@ Joining a second list
>>> print(join.process(mlist_2, msg, {}, (), Results()))
ContinueProcessing.yes
-Anne of course, is still registered.
-
- >>> print(user_manager.get_user('anne@example.com'))
- <User "Anne Person" (...) at ...>
-
-But she is not a member of the mailing list.
+Anne is not a member of the mailing list.
>>> print(mlist_2.members.get_member('anne@example.com'))
None
@@ -257,7 +239,9 @@ subscribe with. Any of her registered, linked, and validated email addresses
will do.
::
- >>> anne = user_manager.get_user('anne@example.com')
+ >>> from mailman.interfaces.usermanager import IUserManager
+ >>> from zope.component import getUtility
+ >>> anne = getUtility(IUserManager).get_user('anne@example.com')
>>> address = anne.register('anne.person@example.org')
>>> results = Results()
@@ -333,11 +317,6 @@ message.
... raise AssertionError('No confirmation message')
>>> token = extract_token(item.msg)
-Bart is still not a user.
-
- >>> print(user_manager.get_user('bart@example.com'))
- None
-
Bart replies to the original message, specifically keeping the Subject header
intact except for any prefix. Mailman matches the token and confirms Bart as
a user of the system.
@@ -360,12 +339,7 @@ a user of the system.
Confirmed
<BLANKLINE>
-Now Bart is a user...
-
- >>> print(user_manager.get_user('bart@example.com'))
- <User "Bart Person" (...) at ...>
-
-...and a member of the mailing list.
+Now Bart is now a member of the mailing list.
>>> print(mlist.members.get_member('bart@example.com'))
<Member: Bart Person <bart@example.com>
diff --git a/src/mailman/commands/eml_confirm.py b/src/mailman/commands/eml_confirm.py
index 4c6039aad..2ee48e938 100644
--- a/src/mailman/commands/eml_confirm.py
+++ b/src/mailman/commands/eml_confirm.py
@@ -25,7 +25,6 @@ __all__ = [
from mailman.core.i18n import _
from mailman.interfaces.command import ContinueProcessing, IEmailCommand
from mailman.interfaces.registrar import IRegistrar
-from zope.component import getUtility
from zope.interface import implementer
@@ -53,7 +52,11 @@ class Confirm:
return ContinueProcessing.yes
tokens.add(token)
results.confirms = tokens
- succeeded = IRegistrar(mlist).confirm(token)
+ try:
+ succeeded = IRegistrar(mlist).confirm(token)
+ except LookupError:
+ # The token must not exist in the database.
+ succeeded = False
if succeeded:
print(_('Confirmed'), file=results)
return ContinueProcessing.yes
diff --git a/src/mailman/commands/eml_membership.py b/src/mailman/commands/eml_membership.py
index b42116b74..970fd4429 100644
--- a/src/mailman/commands/eml_membership.py
+++ b/src/mailman/commands/eml_membership.py
@@ -37,6 +37,28 @@ from zope.interface import implementer
+def match_subscriber(email, display_name):
+ # Return something matching the email which should be used as the
+ # subscriber by the IRegistrar interface.
+ manager = getUtility(IUserManager)
+ # Is there a user with a preferred address matching the email?
+ user = manager.get_user(email)
+ if user is not None:
+ preferred = user.preferred_address
+ if preferred is not None and preferred.email == email.lower():
+ return user
+ # Is there an address matching the email?
+ address = manager.get_address(email)
+ if address is not None:
+ return address
+ # Make a new user and subscribe their first (and only) address. We can't
+ # make the first address their preferred address because it hasn't been
+ # verified yet.
+ user = manager.make_user(email, display_name)
+ return list(user.addresses)[0]
+
+
+
@implementer(IEmailCommand)
class Join:
"""The email 'join' command."""
@@ -60,34 +82,35 @@ used.
delivery_mode = self._parse_arguments(arguments, results)
if delivery_mode is ContinueProcessing.no:
return ContinueProcessing.no
- display_name, address = parseaddr(msg['from'])
+ display_name, email = parseaddr(msg['from'])
# Address could be None or the empty string.
- if not address:
- address = msg.sender
- if not address:
+ if not email:
+ email = msg.sender
+ if not email:
print(_('$self.name: No valid address found to subscribe'),
file=results)
return ContinueProcessing.no
- if isinstance(address, bytes):
- address = address.decode('ascii')
+ if isinstance(email, bytes):
+ email = email.decode('ascii')
# Have we already seen one join request from this user during the
# processing of this email?
joins = getattr(results, 'joins', set())
- if address in joins:
+ if email in joins:
# Do not register this join.
return ContinueProcessing.yes
- joins.add(address)
+ joins.add(email)
results.joins = joins
- person = formataddr((display_name, address))
+ person = formataddr((display_name, email))
# Is this person already a member of the list? Search for all
# matching memberships.
members = getUtility(ISubscriptionService).find_members(
- address, mlist.list_id, MemberRole.member)
+ email, mlist.list_id, MemberRole.member)
if len(members) > 0:
print(_('$person is already a member'), file=results)
- else:
- IRegistrar(mlist).register(address)
- print(_('Confirmation email sent to $person'), file=results)
+ return ContinueProcessing.yes
+ subscriber = match_subscriber(email, display_name)
+ IRegistrar(mlist).register(subscriber)
+ print(_('Confirmation email sent to $person'), file=results)
return ContinueProcessing.yes
def _parse_arguments(self, arguments, results):
diff --git a/src/mailman/database/transaction.py b/src/mailman/database/transaction.py
index b96562d3f..fef76b73c 100644
--- a/src/mailman/database/transaction.py
+++ b/src/mailman/database/transaction.py
@@ -19,6 +19,7 @@
__all__ = [
'dbconnection',
+ 'flush',
'transaction',
'transactional',
]
@@ -63,6 +64,22 @@ def transactional(function):
+@contextmanager
+def flush():
+ """Context manager for flushing SQLAlchemy.
+
+ We need this for SA whereas we didn't need it for Storm because the latter
+ did auto-reloads. However, in SA this is needed when we add or delete
+ objects from the database. Use it when you need the id after adding, or
+ when you want to be sure the object won't be found after a delete.
+
+ This is lighter weight than committing the transaction.
+ """
+ yield
+ config.db.store.flush()
+
+
+
def dbconnection(function):
"""Decorator for getting at the database connection.
diff --git a/src/mailman/interfaces/registrar.py b/src/mailman/interfaces/registrar.py
index d67334775..95c8fcd88 100644
--- a/src/mailman/interfaces/registrar.py
+++ b/src/mailman/interfaces/registrar.py
@@ -75,8 +75,9 @@ class IRegistrar(Interface):
:type mlist: `IMailingList`
:param subscriber: The user or address to subscribe.
:type email: ``IUser`` or ``IAddress``
- :return: The confirmation token string.
- :rtype: str
+ :return: The confirmation token string, or None if the workflow
+ completes (i.e. the member has been subscribed).
+ :rtype: str or None
:raises MembershipIsBannedError: when the address being subscribed
appears in the global or list-centric bans.
"""
@@ -89,6 +90,10 @@ class IRegistrar(Interface):
or the moderator must approve the subscription request.
:param token: A token matching a workflow.
+ :type token: string
+ :return: A flag indicating whether the confirmation succeeded in
+ subscribing the user or not.
+ :rtype: bool
:raises LookupError: when no workflow is associated with the token.
"""
diff --git a/src/mailman/interfaces/workflow.py b/src/mailman/interfaces/workflow.py
index b60537652..f80e38547 100644
--- a/src/mailman/interfaces/workflow.py
+++ b/src/mailman/interfaces/workflow.py
@@ -63,9 +63,18 @@ class IWorkflowStateManager(Interface):
:type name: str
:param token: A unique token identifying this workflow instance.
:type token: str
- :raises LookupError: when there's no token associated with the given
- name in the workflow table.
+ :return: The saved state associated with this name/token pair, or None
+ if the pair isn't in the database.
+ :rtype: ``IWorkflowState``
"""
- def count():
- """The number of saved workflows in the database."""
+ def discard(name, token):
+ """Throw away the saved state for a workflow.
+
+ :param name: The name of the workflow.
+ :type name: str
+ :param token: A unique token identifying this workflow instance.
+ :type token: str
+ """
+
+ count = Attribute('The number of saved workflows in the database.')
diff --git a/src/mailman/model/docs/registration.rst b/src/mailman/model/docs/registration.rst
index fc7ad6f1a..5a4935355 100644
--- a/src/mailman/model/docs/registration.rst
+++ b/src/mailman/model/docs/registration.rst
@@ -48,6 +48,7 @@ list. In this case, verifying implies that she also confirms her wish to join
the mailing list.
>>> registrar.confirm(token)
+ True
>>> mlist.members.get_member('anne@example.com')
<Member: Anne Person <anne@example.com> on ant@example.com
as MemberRole.member>
@@ -85,6 +86,7 @@ subscribed to the mailing list.
When the moderator confirms Bart's subscription, he joins the mailing list.
>>> registrar.confirm(token)
+ True
>>> mlist.members.get_member('bart@example.com')
<Member: Bart Person <bart@example.com> on ant@example.com
as MemberRole.member>
diff --git a/src/mailman/model/tests/test_workflow.py b/src/mailman/model/tests/test_workflow.py
index ccf618c2b..88ed506bd 100644
--- a/src/mailman/model/tests/test_workflow.py
+++ b/src/mailman/model/tests/test_workflow.py
@@ -43,11 +43,11 @@ class TestWorkflow(unittest.TestCase):
step = 'cat'
data = 'dog'
self._manager.save(name, token, step, data)
- workflow = self._manager.restore(name, token)
- self.assertEqual(workflow.name, name)
- self.assertEqual(workflow.token, token)
- self.assertEqual(workflow.step, step)
- self.assertEqual(workflow.data, data)
+ state = self._manager.restore(name, token)
+ self.assertEqual(state.name, name)
+ self.assertEqual(state.token, token)
+ self.assertEqual(state.step, step)
+ self.assertEqual(state.data, data)
def test_save_restore_workflow_without_step(self):
# Save and restore a workflow that contains no step.
@@ -55,11 +55,11 @@ class TestWorkflow(unittest.TestCase):
token = 'bee'
data = 'dog'
self._manager.save(name, token, data=data)
- workflow = self._manager.restore(name, token)
- self.assertEqual(workflow.name, name)
- self.assertEqual(workflow.token, token)
- self.assertIsNone(workflow.step)
- self.assertEqual(workflow.data, data)
+ state = self._manager.restore(name, token)
+ self.assertEqual(state.name, name)
+ self.assertEqual(state.token, token)
+ self.assertIsNone(state.step)
+ self.assertEqual(state.data, data)
def test_save_restore_workflow_without_data(self):
# Save and restore a workflow that contains no data.
@@ -67,38 +67,38 @@ class TestWorkflow(unittest.TestCase):
token = 'bee'
step = 'cat'
self._manager.save(name, token, step)
- workflow = self._manager.restore(name, token)
- self.assertEqual(workflow.name, name)
- self.assertEqual(workflow.token, token)
- self.assertEqual(workflow.step, step)
- self.assertIsNone(workflow.data)
+ state = self._manager.restore(name, token)
+ self.assertEqual(state.name, name)
+ self.assertEqual(state.token, token)
+ self.assertEqual(state.step, step)
+ self.assertIsNone(state.data)
def test_save_restore_workflow_without_step_or_data(self):
# Save and restore a workflow that contains no step or data.
name = 'ant'
token = 'bee'
self._manager.save(name, token)
- workflow = self._manager.restore(name, token)
- self.assertEqual(workflow.name, name)
- self.assertEqual(workflow.token, token)
- self.assertIsNone(workflow.step)
- self.assertIsNone(workflow.data)
+ state = self._manager.restore(name, token)
+ self.assertEqual(state.name, name)
+ self.assertEqual(state.token, token)
+ self.assertIsNone(state.step)
+ self.assertIsNone(state.data)
def test_restore_workflow_with_no_matching_name(self):
# Try to restore a workflow that has no matching name in the database.
name = 'ant'
token = 'bee'
self._manager.save(name, token)
- workflow = self._manager.restore('ewe', token)
- self.assertIsNone(workflow)
+ state = self._manager.restore('ewe', token)
+ self.assertIsNone(state)
def test_restore_workflow_with_no_matching_token(self):
# Try to restore a workflow that has no matching token in the database.
name = 'ant'
token = 'bee'
self._manager.save(name, token)
- workflow = self._manager.restore(name, 'fly')
- self.assertIsNone(workflow)
+ state = self._manager.restore(name, 'fly')
+ self.assertIsNone(state)
def test_restore_workflow_with_no_matching_token_or_name(self):
# Try to restore a workflow that has no matching token or name in the
@@ -106,25 +106,43 @@ class TestWorkflow(unittest.TestCase):
name = 'ant'
token = 'bee'
self._manager.save(name, token)
- workflow = self._manager.restore('ewe', 'fly')
- self.assertIsNone(workflow)
+ state = self._manager.restore('ewe', 'fly')
+ self.assertIsNone(state)
def test_restore_removes_record(self):
name = 'ant'
token = 'bee'
- self.assertEqual(self._manager.count(), 0)
+ self.assertEqual(self._manager.count, 0)
self._manager.save(name, token)
- self.assertEqual(self._manager.count(), 1)
+ self.assertEqual(self._manager.count, 1)
self._manager.restore(name, token)
- self.assertEqual(self._manager.count(), 0)
+ self.assertEqual(self._manager.count, 0)
def test_save_after_restore(self):
name = 'ant'
token = 'bee'
- self.assertEqual(self._manager.count(), 0)
+ self.assertEqual(self._manager.count, 0)
self._manager.save(name, token)
- self.assertEqual(self._manager.count(), 1)
+ self.assertEqual(self._manager.count, 1)
self._manager.restore(name, token)
- self.assertEqual(self._manager.count(), 0)
+ self.assertEqual(self._manager.count, 0)
self._manager.save(name, token)
- self.assertEqual(self._manager.count(), 1)
+ self.assertEqual(self._manager.count, 1)
+
+ def test_discard(self):
+ # Discard some workflow state. This is use by IRegistrar.discard().
+ self._manager.save('ant', 'token', 'one')
+ self._manager.save('bee', 'token', 'two')
+ self._manager.save('ant', 'nekot', 'three')
+ self._manager.save('bee', 'nekot', 'four')
+ self.assertEqual(self._manager.count, 4)
+ self._manager.discard('bee', 'token')
+ self.assertEqual(self._manager.count, 3)
+ state = self._manager.restore('ant', 'token')
+ self.assertEqual(state.step, 'one')
+ state = self._manager.restore('bee', 'token')
+ self.assertIsNone(state)
+ state = self._manager.restore('ant', 'nekot')
+ self.assertEqual(state.step, 'three')
+ state = self._manager.restore('bee', 'nekot')
+ self.assertEqual(state.step, 'four')
diff --git a/src/mailman/model/workflow.py b/src/mailman/model/workflow.py
index 6ac3fa76a..392ab0798 100644
--- a/src/mailman/model/workflow.py
+++ b/src/mailman/model/workflow.py
@@ -63,6 +63,14 @@ class WorkflowStateManager:
return state
@dbconnection
+ def discard(self, store, name, token):
+ """See `IWorkflowStateManager`."""
+ state = store.query(WorkflowState).get((name, token))
+ if state is not None:
+ store.delete(state)
+
+ @property
+ @dbconnection
def count(self, store):
"""See `IWorkflowStateManager`."""
return store.query(WorkflowState).count()
diff --git a/src/mailman/runners/docs/command.rst b/src/mailman/runners/docs/command.rst
index 5cfbf7cfb..8fb5c6816 100644
--- a/src/mailman/runners/docs/command.rst
+++ b/src/mailman/runners/docs/command.rst
@@ -140,6 +140,7 @@ address, and the other is the results of his email command.
>>> len(messages)
2
+ >>> from mailman.interfaces.registrar import IRegistrar
>>> registrar = IRegistrar(mlist)
>>> for item in messages:
... subject = item.msg['subject']
diff --git a/src/mailman/runners/tests/test_join.py b/src/mailman/runners/tests/test_join.py
index f968433ba..5742800c2 100644
--- a/src/mailman/runners/tests/test_join.py
+++ b/src/mailman/runners/tests/test_join.py
@@ -197,6 +197,8 @@ join digest=no
self.assertEqual(anne.address.email, 'anne@example.org')
self.assertEqual(anne.delivery_mode, DeliveryMode.regular)
+ # LP: #1444184 - digest=mime is not currently supported.
+ @unittest.expectedFailure
def test_join_with_mime_digests(self):
# Test the digest=mime argument to the join command.
msg = mfs("""\
@@ -211,6 +213,8 @@ join digest=mime
self.assertEqual(anne.address.email, 'anne@example.org')
self.assertEqual(anne.delivery_mode, DeliveryMode.mime_digests)
+ # LP: #1444184 - digest=mime is not currently supported.
+ @unittest.expectedFailure
def test_join_with_plain_digests(self):
# Test the digest=mime argument to the join command.
msg = mfs("""\
generated by cgit v1.2.3-70-g09d2 (git 2.51.2) at 2025-11-08 18:23:23 +0000