5 files changed, 129 insertions, 125 deletions

diff --git a/setup.py b/setup.py
index 4a2b178df..94d56af43 100644
--- a/setup.py
+++ b/setup.py
@@ -105,7 +105,7 @@ case second `m'.  Any other spelling is incorrect.""",
         },
     install_requires = [
         'alembic',
-        'falcon>=0.3rc1,<1.0',
+        'falcon>=0.3rc1',
         'flufl.bounce',
         'flufl.i18n',
         'flufl.lock',
diff --git a/src/mailman/rest/lists.py b/src/mailman/rest/lists.py
index 7315d805a..417db4839 100644
--- a/src/mailman/rest/lists.py
+++ b/src/mailman/rest/lists.py
@@ -43,7 +43,7 @@ from mailman.rest.validator import Validator
 from zope.component import getUtility
 
 
-def member_matcher(request, segments):
+def member_matcher(segments):
     """A matcher of member URLs inside mailing lists.
 
     e.g. /<role>/aperson@example.org
@@ -58,7 +58,7 @@ def member_matcher(request, segments):
     return (), dict(role=role, email=segments[1]), ()
 
 
-def roster_matcher(request, segments):
+def roster_matcher(segments):
     """A matcher of all members URLs inside mailing lists.
 
     e.g. /roster/<role>
@@ -72,7 +72,7 @@ def roster_matcher(request, segments):
         return None
 
 
-def config_matcher(request, segments):
+def config_matcher(segments):
     """A matcher for a mailing list's configuration resource.
 
     e.g. /config
diff --git a/src/mailman/rest/root.py b/src/mailman/rest/root.py
index 366dc3d08..c07be5388 100644
--- a/src/mailman/rest/root.py
+++ b/src/mailman/rest/root.py
@@ -17,9 +17,6 @@
 
 """The root of the REST API."""
 
-import falcon
-
-from base64 import b64decode
 from mailman import public
 from mailman.config import config
 from mailman.core.api import API30, API31
@@ -55,48 +52,24 @@ class Root:
     """
 
     @child('3.0')
-    def api_version_30(self, request, segments):
+    def api_version_30(self, context, segments):
         # API version 3.0 was introduced in Mailman 3.0.
-        request.context['api'] = API30
-        return self._check_authorization(request, segments)
+        context['api'] = API30
+        return TopLevel()
 
     @child('3.1')
-    def api_version_31(self, request, segments):
+    def api_version_31(self, context, segments):
         # API version 3.1 was introduced in Mailman 3.1.  Primary backward
         # incompatible difference is that uuids are represented as hex strings
         # instead of 128 bit integers.  The latter is not compatible with all
         # versions of JavaScript.
-        request.context['api'] = API31
-        return self._check_authorization(request, segments)
-
-    def _check_authorization(self, request, segments):
-        # We have to do this here instead of in a @falcon.before() handler
-        # because those handlers are not compatible with our custom traversal
-        # logic.  Specifically, falcon's before/after handlers will call the
-        # responder, but the method we're wrapping isn't a responder, it's a
-        # child traversal method.  There's no way to cause the thing that
-        # calls the before hook to follow through with the child traversal in
-        # the case where no error is raised.
-        if request.auth is None:
-            raise falcon.HTTPUnauthorized(
-                '401 Unauthorized',
-                'The REST API requires authentication')
-        if request.auth.startswith('Basic '):
-            # b64decode() returns bytes, but we require a str.
-            credentials = b64decode(request.auth[6:]).decode('utf-8')
-            username, password = credentials.split(':', 1)
-            if (username != config.webservice.admin_user or
-                    password != config.webservice.admin_pass):
-                # Not authorized.
-                raise falcon.HTTPUnauthorized(
-                    '401 Unauthorized',
-                    'User is not authorized for the REST API')
+        context['api'] = API31
         return TopLevel()
 
 
 @public
 class Versions:
-    def on_get(self, request, response):
+    def on_get(self, context, response):
         """/<api>/system/versions"""
         resource = dict(
             mailman_version=system.mailman_version,
@@ -112,7 +85,7 @@ class SystemConfiguration:
     def __init__(self, section=None):
         self._section = section
 
-    def on_get(self, request, response):
+    def on_get(self, context, response):
         if self._section is None:
             resource = dict(
                 sections=sorted(section.name for section in config))
@@ -131,14 +104,14 @@ class SystemConfiguration:
 
 @public
 class Pipelines:
-    def on_get(self, request, response):
+    def on_get(self, context, response):
         resource = dict(pipelines=sorted(config.pipelines))
         okay(response, etag(resource))
 
 
 @public
 class Chains:
-    def on_get(self, request, response):
+    def on_get(self, context, response):
         resource = dict(chains=sorted(config.chains))
         okay(response, etag(resource))
 
@@ -155,7 +128,7 @@ class Reserved:
     def __init__(self, segments):
         self._resource_path = SLASH.join(segments)
 
-    def on_delete(self, request, response):
+    def on_delete(self, context, response):
         if self._resource_path != 'uids/orphans':
             not_found(response)
             return
@@ -168,7 +141,7 @@ class TopLevel:
     """Top level collections and entries."""
 
     @child()
-    def system(self, request, segments):
+    def system(self, context, segments):
         """/<api>/system"""
         if len(segments) == 0:
             # This provides backward compatibility; see /system/versions.
@@ -197,22 +170,22 @@ class TopLevel:
             return NotFound(), []
 
     @child()
-    def addresses(self, request, segments):
+    def addresses(self, context, segments):
         """/<api>/addresses
            /<api>/addresses/<email>
         """
         if len(segments) == 0:
             resource = AllAddresses()
-            resource.api = request.context['api']
+            resource.api = context['api']
             return resource
         else:
             email = segments.pop(0)
             resource = AnAddress(email)
-            resource.api = request.context['api']
+            resource.api = context['api']
             return resource, segments
 
     @child()
-    def domains(self, request, segments):
+    def domains(self, context, segments):
         """/<api>/domains
            /<api>/domains/<domain>
         """
@@ -223,7 +196,7 @@ class TopLevel:
             return ADomain(domain), segments
 
     @child()
-    def lists(self, request, segments):
+    def lists(self, context, segments):
         """/<api>/lists
            /<api>/lists/styles
            /<api>/lists/<list>
@@ -240,9 +213,9 @@ class TopLevel:
             return AList(list_identifier), segments
 
     @child()
-    def members(self, request, segments):
+    def members(self, context, segments):
         """/<api>/members"""
-        api = request.context['api']
+        api = context['api']
         if len(segments) == 0:
             resource = AllMembers()
             resource.api = api
@@ -258,9 +231,9 @@ class TopLevel:
         return resource, segments
 
     @child()
-    def users(self, request, segments):
+    def users(self, context, segments):
         """/<api>/users"""
-        api = request.context['api']
+        api = context['api']
         if len(segments) == 0:
             resource = AllUsers()
             resource.api = api
@@ -270,17 +243,17 @@ class TopLevel:
             return AUser(api, user_id), segments
 
     @child()
-    def owners(self, request, segments):
+    def owners(self, context, segments):
         """/<api>/owners"""
         if len(segments) != 0:
             return BadRequest(), []
         else:
             resource = ServerOwners()
-            resource.api = request.context['api']
+            resource.api = context['api']
             return resource, segments
 
     @child()
-    def templates(self, request, segments):
+    def templates(self, context, segments):
         """/<api>/templates/<fqdn_listname>/<template>/[<language>]
 
         Use content negotiation to request language and suffix (content-type).
@@ -295,13 +268,13 @@ class TopLevel:
         mlist = getUtility(IListManager).get(fqdn_listname)
         if mlist is None:
             return NotFound(), []
-        # XXX dig out content-type from request
+        # XXX dig out content-type from request.
         content_type = None
         return TemplateFinder(
             fqdn_listname, template, language, content_type)
 
     @child()
-    def queues(self, request, segments):
+    def queues(self, context, segments):
         """/<api>/queues[/<name>[/file]]"""
         if len(segments) == 0:
             return AllQueues()
@@ -313,7 +286,7 @@ class TopLevel:
             return BadRequest(), []
 
     @child()
-    def bans(self, request, segments):
+    def bans(self, context, segments):
         """/<api>/bans
            /<api>/bans/<email>
         """
@@ -324,6 +297,6 @@ class TopLevel:
             return BannedEmail(None, email), segments
 
     @child()
-    def reserved(self, request, segments):
+    def reserved(self, context, segments):
         """/<api>/reserved/[...]"""
         return Reserved(segments), []
diff --git a/src/mailman/rest/users.py b/src/mailman/rest/users.py
index 19ebba413..fe5641ddc 100644
--- a/src/mailman/rest/users.py
+++ b/src/mailman/rest/users.py
@@ -80,7 +80,7 @@ CREATION_FIELDS = dict(
     )
 
 
-def create_user(arguments, request, response):
+def create_user(api, arguments, response):
     """Create a new user."""
     # We can't pass the 'password' argument to the user creation method, so
     # strip that out (if it exists), then create the user, adding the password
@@ -109,9 +109,8 @@ def create_user(arguments, request, response):
         password = generate(int(config.passwords.password_length))
     user.password = config.password_context.encrypt(password)
     user.is_server_owner = is_server_owner
-    api = request.context['api']
     user_id = api.from_uuid(user.user_id)
-    location = request.context.get('api').path_to('users/{}'.format(user_id))
+    location = api.path_to('users/{}'.format(user_id))
     created(response, location)
     return user
 
@@ -162,7 +161,7 @@ class AllUsers(_UserBase):
         except ValueError as error:
             bad_request(response, str(error))
             return
-        create_user(arguments, request, response)
+        create_user(self.api, arguments, response)
 
 
 @public
@@ -204,7 +203,7 @@ class AUser(_UserBase):
             okay(response, self._resource_as_json(self._user))
 
     @child()
-    def addresses(self, request, segments):
+    def addresses(self, context, segments):
         """/users/<uid>/addresses"""
         if self._user is None:
             return NotFound(), []
@@ -222,7 +221,7 @@ class AUser(_UserBase):
         no_content(response)
 
     @child()
-    def preferences(self, request, segments):
+    def preferences(self, context, segments):
         """/users/<id>/preferences"""
         if len(segments) != 0:
             return BadRequest(), []
@@ -270,7 +269,7 @@ class AUser(_UserBase):
             no_content(response)
 
     @child()
-    def login(self, request, segments):
+    def login(self, context, segments):
         """Log the user in, sort of, by verifying a given password."""
         if self._user is None:
             return NotFound(), []
@@ -302,17 +301,17 @@ class AddressUser(_UserBase):
 
     def on_post(self, request, response):
         """Link a user to the address, and create it if needed."""
+        import pdb; pdb.set_trace()
         if self._user:
             conflict(response)
             return
-        api = request.context['api']
         # When creating a linked user by POSTing, the user either must already
         # exist, or it can be automatically created, if the auto_create flag
         # is given and true (if missing, it defaults to true).  However, in
         # this case we do not accept 'email' as a POST field.
         fields = CREATION_FIELDS.copy()
         del fields['email']
-        fields['user_id'] = api.to_uuid
+        fields['user_id'] = self.api.to_uuid
         fields['auto_create'] = as_boolean
         fields['_optional'] = fields['_optional'] + (
             'user_id', 'auto_create', 'is_server_owner')
@@ -335,7 +334,7 @@ class AddressUser(_UserBase):
             auto_create = arguments.pop('auto_create', True)
             if auto_create:
                 # This sets the 201 or 400 status.
-                user = create_user(arguments, request, response)
+                user = create_user(self.api, arguments, response)
                 if user is None:
                     return
             else:
@@ -368,7 +367,7 @@ class AddressUser(_UserBase):
                 return
             okay(response)
         else:
-            user = create_user(arguments, request, response)
+            user = create_user(self.api, arguments, response)
             if user is None:
                 return
         user.link(self._address)
diff --git a/src/mailman/rest/wsgiapp.py b/src/mailman/rest/wsgiapp.py
index 2c22948bb..c7e7bfea0 100644
--- a/src/mailman/rest/wsgiapp.py
+++ b/src/mailman/rest/wsgiapp.py
@@ -20,8 +20,8 @@
 import re
 import logging
 
-from falcon import API
-from falcon.responders import path_not_found
+from base64 import b64decode
+from falcon import API, HTTPUnauthorized
 from falcon.routing import create_http_method_map
 from mailman import public
 from mailman.config import config
@@ -32,9 +32,11 @@ from wsgiref.simple_server import (
 
 
 log = logging.getLogger('mailman.http')
-_missing = object()
+
+MISSING = object()
 SLASH = '/'
 EMPTYSTRING = ''
+REALM = 'mailman3-rest'
 
 
 class AdminWSGIServer(WSGIServer):
@@ -73,53 +75,68 @@ class AdminWebServiceWSGIRequestHandler(WSGIRequestHandler):
         return StderrLogger()
 
 
-class SetAPIVersion:
-    """Falcon middleware object that sets the API on resources."""
+class Middleware:
+    """Falcon middleware object for Mailman's REST API.
 
-    def process_resource(self, request, response, resource):
-        # Set this attribute on the resource right before it is dispatched
-        # too.  This can be used by the resource to provide different
-        # responses based on the API version, and for path_to() to provide an
-        # API version-specific path.
-        #
-        # Note that it's possible that resource is None, e.g. such as when a
-        # resource path does not exist.  This middleware method will still get
-        # called, but there's nothing to set the api_version on.
-        if resource is not None:
-            resource.api = request.context.get('api')
+    This does two things.  It sets the API version on the resource object so
+    that it is acceptable to all http mapped methods, and it verifies that the
+    proper authentication has been performed.
+    """
+    def process_resource(self, request, response, resource, params):
+        # Set this attribute on the resource right before it is dispatched to.
+        # This can be used by the resource to provide different responses
+        # based on the API version, and for path_to() to provide an API
+        # version-specific path.
+        resource.api = params.pop('api')
+        # We have to do this here instead of in a @falcon.before() handler
+        # because those handlers are not compatible with our custom traversal
+        # logic.  Specifically, falcon's before/after handlers will call the
+        # responder, but the method we're wrapping isn't a responder, it's a
+        # child traversal method.  There's no way to cause the thing that
+        # calls the before hook to follow through with the child traversal in
+        # the case where no error is raised.
+        if request.auth is None:
+            raise HTTPUnauthorized(
+                '401 Unauthorized',
+                'The REST API requires authentication',
+                challenges=['Basic realm=Mailman3'])
+        if request.auth.startswith('Basic '):
+            # b64decode() returns bytes, but we require a str.
+            credentials = b64decode(request.auth[6:]).decode('utf-8')
+            username, password = credentials.split(':', 1)
+            if (username != config.webservice.admin_user or
+                    password != config.webservice.admin_pass):
+                # Not authorized.
+                raise HTTPUnauthorized(
+                    '401 Unauthorized',
+                    'User is not authorized for the REST API',
+                    challenges=['Basic realm=Mailman3'])
 
 
-class RootedAPI(API):
-    def __init__(self, root, *args, **kws):
+class ObjectRouter:
+    def __init__(self, root):
         self._root = root
-        super().__init__(*args, middleware=SetAPIVersion(), **kws)
 
-    @transactional
-    def __call__(self, environ, start_response):
-        # Override the base class implementation to wrap a transactional
-        # handler around the call, such that the current transaction is
-        # committed if no errors occur, and aborted otherwise.
-        return super().__call__(environ, start_response)
+    def add_route(self, uri_template, method_map, resource):
+        raise NotImplementedError
 
-    def _get_responder(self, req):
-        path = req.path
-        method = req.method
-        path_segments = path.split('/')
+    def find(self, uri):
+        segments = uri.split(SLASH)
         # Since the path is always rooted at /, skip the first segment, which
         # will always be the empty string.
-        path_segments.pop(0)
-        this_segment = path_segments.pop(0)
+        segments.pop(0)
+        this_segment = segments.pop(0)
         resource = self._root
+        context = {}
         while True:
-            # See if there's a child matching the current segment.
             # See if any of the resource's child links match the next segment.
             for name in dir(resource):
                 if name.startswith('__') and name.endswith('__'):
                     continue
-                attribute = getattr(resource, name, _missing)
-                assert attribute is not _missing, name
-                matcher = getattr(attribute, '__matcher__', _missing)
-                if matcher is _missing:
+                attribute = getattr(resource, name, MISSING)
+                assert attribute is not MISSING, name
+                matcher = getattr(attribute, '__matcher__', MISSING)
+                if matcher is MISSING:
                     continue
                 result = None
                 if isinstance(matcher, str):
@@ -128,15 +145,15 @@ class RootedAPI(API):
                     if matcher.startswith('^'):
                         cre = re.compile(matcher)
                         # Search against the entire remaining path.
-                        tmp_path_segments = path_segments[:]
-                        tmp_path_segments.insert(0, this_segment)
-                        remaining_path = SLASH.join(tmp_path_segments)
+                        tmp_segments = segments[:]
+                        tmp_segments.insert(0, this_segment)
+                        remaining_path = SLASH.join(tmp_segments)
                         mo = cre.match(remaining_path)
                         if mo:
                             result = attribute(
-                                req, path_segments, **mo.groupdict())
+                                context, segments, **mo.groupdict())
                     elif matcher == this_segment:
-                        result = attribute(req, path_segments)
+                        result = attribute(context, segments)
                 else:
                     # The matcher is a callable.  It returns None if it
                     # doesn't match, and if it does, it returns a 3-tuple
@@ -145,39 +162,54 @@ class RootedAPI(API):
                     # then called with these arguments.  Note that the matcher
                     # wants to see the full remaining path components, which
                     # includes the current hop.
-                    tmp_path_segments = path_segments[:]
-                    tmp_path_segments.insert(0, this_segment)
-                    matcher_result = matcher(req, tmp_path_segments)
+                    tmp_segments = segments[:]
+                    tmp_segments.insert(0, this_segment)
+                    matcher_result = matcher(tmp_segments)
                     if matcher_result is not None:
-                        positional, keyword, path_segments = matcher_result
+                        positional, keyword, segments = matcher_result
                         result = attribute(
-                            req, path_segments, *positional, **keyword)
+                            context, segments, *positional, **keyword)
                 # The attribute could return a 2-tuple giving the resource and
-                # remaining path segments, or it could just return the
-                # result.  Of course, if the result is None, then the matcher
-                # did not match.
+                # remaining path segments, or it could just return the result.
+                # Of course, if the result is None, then the matcher did not
+                # match.
                 if result is None:
                     continue
                 elif isinstance(result, tuple):
-                    resource, path_segments = result
+                    resource, segments = result
                 else:
                     resource = result
                 # The method could have truncated the remaining segments,
                 # meaning, it's consumed all the path segments, or this is the
                 # last path segment.  In that case the resource we're left at
                 # is the responder.
-                if len(path_segments) == 0:
+                if len(segments) == 0:
                     # We're at the end of the path, so the root must be the
                     # responder.
-                    method_map = create_http_method_map(resource, None, None)
-                    responder = method_map[method]
-                    return responder, {}, resource
-                this_segment = path_segments.pop(0)
+                    method_map = create_http_method_map(resource)
+                    return resource, method_map, context
+                this_segment = segments.pop(0)
                 break
             else:
                 # None of the attributes matched this path component, so the
                 # response is a 404.
-                return path_not_found, {}, None
+                return None, None, None
+
+
+class RootedAPI(API):
+    def __init__(self, root, *args, **kws):
+        super().__init__(
+            *args,
+            middleware=Middleware(),
+            router=ObjectRouter(root),
+            **kws)
+
+    @transactional
+    def __call__(self, environ, start_response):
+        # Override the base class implementation to wrap a transactional
+        # handler around the call, such that the current transaction is
+        # committed if no errors occur, and aborted otherwise.
+        return super().__call__(environ, start_response)
 
 
 @public