diff options
30 files changed, 1011 insertions, 609 deletions
@@ -95,6 +95,7 @@ case second `m'. Any other spelling is incorrect.""", install_requires = [ 'alembic', 'enum34', + 'falcon', 'flufl.bounce', 'flufl.i18n', 'flufl.lock', @@ -104,7 +105,6 @@ case second `m'. Any other spelling is incorrect.""", 'mock', 'nose2', 'passlib', - 'restish', 'sqlalchemy', 'zope.component', 'zope.configuration', diff --git a/src/mailman/config/schema.cfg b/src/mailman/config/schema.cfg index ef158d6c5..c7a63e794 100644 --- a/src/mailman/config/schema.cfg +++ b/src/mailman/config/schema.cfg @@ -149,7 +149,7 @@ testing: no # Time-outs for starting up various test subprocesses, such as the LMTP and # REST servers. This is only used for the test suite, so if you're seeing # test failures, try increasing the wait time. -wait: 10s +wait: 60s [passwords] diff --git a/src/mailman/docs/NEWS.rst b/src/mailman/docs/NEWS.rst index 057dcb4ff..6223dc9d0 100644 --- a/src/mailman/docs/NEWS.rst +++ b/src/mailman/docs/NEWS.rst @@ -49,6 +49,8 @@ Interfaces REST ---- + * The Falcon Framework has replaced restish as the REST layer. This is an + internal change only. * The JSON representation `http_etag` key uses an algorithm that is insensitive to Python's dictionary sort order. diff --git a/src/mailman/rest/addresses.py b/src/mailman/rest/addresses.py index 92772d6f3..fa3d099b6 100644 --- a/src/mailman/rest/addresses.py +++ b/src/mailman/rest/addresses.py @@ -28,13 +28,14 @@ __all__ = [ from operator import attrgetter -from restish import http, resource from zope.component import getUtility from mailman.interfaces.address import ( ExistingAddressError, InvalidEmailAddressError) from mailman.interfaces.usermanager import IUserManager -from mailman.rest.helpers import CollectionMixin, etag, no_content, path_to +from mailman.rest.helpers import ( + BadRequest, CollectionMixin, NotFound, bad_request, child, created, etag, + no_content, not_found, okay, path_to) from mailman.rest.members import MemberCollection from mailman.rest.preferences import Preferences from mailman.rest.validator import Validator @@ -42,7 +43,7 @@ from mailman.utilities.datetime import now -class _AddressBase(resource.Resource, CollectionMixin): +class _AddressBase(CollectionMixin): """Shared base class for address representations.""" def _resource_as_dict(self, address): @@ -72,15 +73,14 @@ class _AddressBase(resource.Resource, CollectionMixin): class AllAddresses(_AddressBase): """The addresses.""" - @resource.GET() - def collection(self, request): + def on_get(self, request, response): """/addresses""" resource = self._make_collection(request) - return http.ok([], etag(resource)) + okay(response, etag(resource)) -class _VerifyResource(resource.Resource): +class _VerifyResource: """A helper resource for verify/unverify POSTS.""" def __init__(self, address, action): @@ -88,14 +88,13 @@ class _VerifyResource(resource.Resource): self._action = action assert action in ('verify', 'unverify') - @resource.POST() - def verify(self, request): + def on_post(self, request, response): # We don't care about the POST data, just do the action. if self._action == 'verify' and self._address.verified_on is None: self._address.verified_on = now() elif self._action == 'unverify': self._address.verified_on = None - return no_content() + no_content(response) class AnAddress(_AddressBase): @@ -109,51 +108,51 @@ class AnAddress(_AddressBase): """ self._address = getUtility(IUserManager).get_address(email) - @resource.GET() - def address(self, request): + def on_get(self, request, response): """Return a single address.""" if self._address is None: - return http.not_found() - return http.ok([], self._resource_as_json(self._address)) + not_found(response) + else: + okay(response, self._resource_as_json(self._address)) - @resource.child() + @child() def memberships(self, request, segments): """/addresses/<email>/memberships""" if len(segments) != 0: - return http.bad_request() + return BadRequest(), [] if self._address is None: - return http.not_found() + return NotFound(), [] return AddressMemberships(self._address) - @resource.child() + @child() def preferences(self, request, segments): """/addresses/<email>/preferences""" if len(segments) != 0: - return http.bad_request() + return NotFound(), [] if self._address is None: - return http.not_found() + return NotFound(), [] child = Preferences( self._address.preferences, 'addresses/{0}'.format(self._address.email)) return child, [] - @resource.child() + @child() def verify(self, request, segments): """/addresses/<email>/verify""" if len(segments) != 0: - return http.bad_request() + return BadRequest(), [] if self._address is None: - return http.not_found() + return NotFound(), [] child = _VerifyResource(self._address, 'verify') return child, [] - @resource.child() + @child() def unverify(self, request, segments): """/addresses/<email>/verify""" if len(segments) != 0: - return http.bad_request() + return BadRequest(), [] if self._address is None: - return http.not_found() + return NotFound(), [] child = _VerifyResource(self._address, 'unverify') return child, [] @@ -171,20 +170,21 @@ class UserAddresses(_AddressBase): return sorted(self._user.addresses, key=attrgetter('original_email')) - @resource.GET() - def collection(self, request): + def on_get(self, request, response): """/addresses""" - resource = self._make_collection(request) - return http.ok([], etag(resource)) + if self._user is None: + not_found(response) + else: + okay(response, etag(self._make_collection(request))) - @resource.POST() - def create(self, request): + def on_post(self, request, response): """POST to /addresses Add a new address to the user record. """ if self._user is None: - return http.not_found() + not_found(response) + return user_manager = getUtility(IUserManager) validator = Validator(email=unicode, display_name=unicode, @@ -192,16 +192,15 @@ class UserAddresses(_AddressBase): try: address = user_manager.create_address(**validator(request)) except ValueError as error: - return http.bad_request([], str(error)) + bad_request(response, str(error)) except InvalidEmailAddressError: - return http.bad_request([], b'Invalid email address') + bad_request(response, b'Invalid email address') except ExistingAddressError: - return http.bad_request([], b'Address already exists') + bad_request(response, b'Address already exists') else: # Link the address to the current user and return it. address.user = self._user - location = path_to('addresses/{0}'.format(address.email)) - return http.created(location, [], None) + created(response, path_to('addresses/{0}'.format(address.email))) diff --git a/src/mailman/rest/configuration.py b/src/mailman/rest/configuration.py index db0918703..b432268c7 100644 --- a/src/mailman/rest/configuration.py +++ b/src/mailman/rest/configuration.py @@ -26,8 +26,6 @@ __all__ = [ from lazr.config import as_boolean, as_timedelta -from restish import http, resource - from mailman.config import config from mailman.core.errors import ( ReadOnlyPATCHRequestError, UnknownPATCHRequestError) @@ -35,7 +33,8 @@ from mailman.interfaces.action import Action from mailman.interfaces.archiver import ArchivePolicy from mailman.interfaces.autorespond import ResponseAction from mailman.interfaces.mailinglist import IAcceptableAliasSet, ReplyToMunging -from mailman.rest.helpers import GetterSetter, PATCH, etag, no_content +from mailman.rest.helpers import ( + GetterSetter, bad_request, etag, no_content, okay) from mailman.rest.validator import PatchValidator, Validator, enum_validator @@ -46,7 +45,7 @@ class AcceptableAliases(GetterSetter): def get(self, mlist, attribute): """Return the mailing list's acceptable aliases.""" assert attribute == 'acceptable_aliases', ( - 'Unexpected attribute: {0}'.format(attribute)) + 'Unexpected attribute: {}'.format(attribute)) aliases = IAcceptableAliasSet(mlist) return sorted(aliases.aliases) @@ -58,7 +57,7 @@ class AcceptableAliases(GetterSetter): ignored. """ assert attribute == 'acceptable_aliases', ( - 'Unexpected attribute: {0}'.format(attribute)) + 'Unexpected attribute: {}'.format(attribute)) alias_set = IAcceptableAliasSet(mlist) alias_set.clear() for alias in value: @@ -73,7 +72,7 @@ def pipeline_validator(pipeline_name): """Convert the pipeline name to a string, but only if it's known.""" if pipeline_name in config.pipelines: return unicode(pipeline_name) - raise ValueError('Unknown pipeline: {0}'.format(pipeline_name)) + raise ValueError('Unknown pipeline: {}'.format(pipeline_name)) def list_of_unicode(values): @@ -156,15 +155,14 @@ for attribute, gettersetter in VALIDATORS.items(): -class ListConfiguration(resource.Resource): +class ListConfiguration: """A mailing list configuration resource.""" def __init__(self, mailing_list, attribute): self._mlist = mailing_list self._attribute = attribute - @resource.GET() - def get_configuration(self, request): + def on_get(self, request, response): """Get a mailing list configuration.""" resource = {} if self._attribute is None: @@ -173,16 +171,16 @@ class ListConfiguration(resource.Resource): value = ATTRIBUTES[attribute].get(self._mlist, attribute) resource[attribute] = value elif self._attribute not in ATTRIBUTES: - return http.bad_request( - [], b'Unknown attribute: {0}'.format(self._attribute)) + bad_request( + response, b'Unknown attribute: {}'.format(self._attribute)) + return else: attribute = self._attribute value = ATTRIBUTES[attribute].get(self._mlist, attribute) resource[attribute] = value - return http.ok([], etag(resource)) + okay(response, etag(resource)) - @resource.PUT() - def put_configuration(self, request): + def on_put(self, request, response): """Set a mailing list configuration.""" attribute = self._attribute if attribute is None: @@ -190,34 +188,39 @@ class ListConfiguration(resource.Resource): try: validator.update(self._mlist, request) except ValueError as error: - return http.bad_request([], str(error)) + bad_request(response, str(error)) + return elif attribute not in ATTRIBUTES: - return http.bad_request( - [], b'Unknown attribute: {0}'.format(attribute)) + bad_request(response, b'Unknown attribute: {}'.format(attribute)) + return elif ATTRIBUTES[attribute].decoder is None: - return http.bad_request( - [], b'Read-only attribute: {0}'.format(attribute)) + bad_request( + response, b'Read-only attribute: {}'.format(attribute)) + return else: validator = Validator(**{attribute: VALIDATORS[attribute]}) try: validator.update(self._mlist, request) except ValueError as error: - return http.bad_request([], str(error)) - return no_content() + bad_request(response, str(error)) + return + no_content(response) - @PATCH() - def patch_configuration(self, request): + def on_patch(self, request, response): """Patch the configuration (i.e. partial update).""" try: validator = PatchValidator(request, ATTRIBUTES) except UnknownPATCHRequestError as error: - return http.bad_request( - [], b'Unknown attribute: {0}'.format(error.attribute)) + bad_request( + response, b'Unknown attribute: {}'.format(error.attribute)) + return except ReadOnlyPATCHRequestError as error: - return http.bad_request( - [], b'Read-only attribute: {0}'.format(error.attribute)) + bad_request( + response, b'Read-only attribute: {}'.format(error.attribute)) + return try: validator.update(self._mlist, request) except ValueError as error: - return http.bad_request([], str(error)) - return no_content() + bad_request(response, str(error)) + else: + no_content(response) diff --git a/src/mailman/rest/docs/basic.rst b/src/mailman/rest/docs/basic.rst index 51b287c90..aa0205874 100644 --- a/src/mailman/rest/docs/basic.rst +++ b/src/mailman/rest/docs/basic.rst @@ -7,70 +7,48 @@ Mailman exposes a REST HTTP server for administrative control. The server listens for connections on a configurable host name and port. It is always protected by HTTP basic authentication using a single global -username and password. The credentials are set in the webservice section -of the config using the admin_user and admin_pass properties. +user name and password. The credentials are set in the `[webservice]` section +of the configuration using the `admin_user` and `admin_pass` properties. Because the REST server has full administrative access, it should always be run only on localhost, unless you really know what you're doing. In addition -you should set the username and password to secure values and distribute them +you should set the user name and password to secure values and distribute them to any REST clients with reasonable precautions. The Mailman major and minor version numbers are in the URL. -System information can be retrieved from the server. By default JSON is -returned. - >>> dump_json('http://localhost:9001/3.0/system') - http_etag: "..." - mailman_version: GNU Mailman 3.0... (...) - python_version: ... - self_link: http://localhost:9001/3.0/system - - -Non-existent links -================== - -When you try to access a link that doesn't exist, you get the appropriate HTTP -404 Not Found error. - - >>> dump_json('http://localhost:9001/3.0/does-not-exist') - Traceback (most recent call last): - ... - HTTPError: HTTP Error 404: 404 Not Found - - -Invalid credentials -=================== +Credentials +=========== -When you try to access the REST server using invalid credentials you will get -an appropriate HTTP 401 Unauthorized error. -:: +When the `Authorization` header contains the proper credentials, the request +succeeds. >>> from base64 import b64encode - >>> auth = b64encode('baduser:badpass') - - >>> url = 'http://localhost:9001/3.0/system' + >>> from httplib2 import Http + >>> auth = b64encode('{0}:{1}'.format(config.webservice.admin_user, + ... config.webservice.admin_pass)) >>> headers = { ... 'Content-Type': 'application/x-www-form-urlencode', ... 'Authorization': 'Basic ' + auth, ... } - - >>> from httplib2 import Http - >>> response, content = Http().request(url, 'GET', None, headers) - >>> print(content) - 401 Unauthorized - <BLANKLINE> - User is not authorized for the REST API - <BLANKLINE> - -But with the right headers, the request succeeds. - - >>> auth = b64encode('{0}:{1}'.format(config.webservice.admin_user, - ... config.webservice.admin_pass)) - >>> headers['Authorization'] = 'Basic ' + auth + >>> url = 'http://localhost:9001/3.0/system' >>> response, content = Http().request(url, 'GET', None, headers) >>> print(response.status) 200 +Basic information +================= + +System information can be retrieved from the server, in the form of a JSON +encoded response. + + >>> dump_json('http://localhost:9001/3.0/system') + http_etag: "..." + mailman_version: GNU Mailman 3.0... (...) + python_version: ... + self_link: http://localhost:9001/3.0/system + + .. _REST: http://en.wikipedia.org/wiki/REST diff --git a/src/mailman/rest/docs/configuration.rst b/src/mailman/rest/docs/configuration.rst index 53ad172e9..841ab3c27 100644 --- a/src/mailman/rest/docs/configuration.rst +++ b/src/mailman/rest/docs/configuration.rst @@ -73,9 +73,8 @@ Not all of the readable attributes can be set through the web interface. The ones that can, can either be set via ``PUT`` or ``PATCH``. ``PUT`` changes all the writable attributes in one request. -When using PUT, all writable attributes must be included. +When using ``PUT``, all writable attributes must be included. - >>> from mailman.interfaces.action import Action >>> dump_json('http://localhost:9001/3.0/lists/' ... 'ant@example.com/config', ... dict( diff --git a/src/mailman/rest/docs/domains.rst b/src/mailman/rest/docs/domains.rst index 92c73ffbf..b28326f73 100644 --- a/src/mailman/rest/docs/domains.rst +++ b/src/mailman/rest/docs/domains.rst @@ -110,13 +110,6 @@ The information for a single domain is available by following one of the self_link: http://localhost:9001/3.0/domains/lists.example.net url_host: example.net -But we get a 404 for a non-existent domain. - - >>> dump_json('http://localhost:9001/3.0/domains/does-not-exist') - Traceback (most recent call last): - ... - HTTPError: HTTP Error 404: 404 Not Found - You can also list all the mailing lists for a given domain. At first, the example.com domain does not contain any mailing lists. :: diff --git a/src/mailman/rest/docs/helpers.rst b/src/mailman/rest/docs/helpers.rst index 0acbb5f45..5bcf5cad4 100644 --- a/src/mailman/rest/docs/helpers.rst +++ b/src/mailman/rest/docs/helpers.rst @@ -62,19 +62,19 @@ dictionary after tagging, since that's almost always what you want. neil : drums -POST unpacking -============== +POST and PUT unpacking +====================== -Another helper unpacks ``POST`` request variables, validating and converting -their values. +Another helper unpacks ``POST`` and ``PUT`` request variables, validating and +converting their values. :: >>> from mailman.rest.validator import Validator >>> validator = Validator(one=int, two=unicode, three=bool) >>> class FakeRequest: - ... POST = {} - >>> FakeRequest.POST = dict(one='1', two='two', three='yes') + ... params = None + >>> FakeRequest.params = dict(one='1', two='two', three='yes') On valid input, the validator can be used as a ``**keyword`` argument. @@ -85,7 +85,7 @@ On valid input, the validator can be used as a ``**keyword`` argument. On invalid input, an exception is raised. - >>> FakeRequest.POST['one'] = 'hello' + >>> FakeRequest.params['one'] = 'hello' >>> print_request(**validator(FakeRequest)) Traceback (most recent call last): ... @@ -93,7 +93,7 @@ On invalid input, an exception is raised. On missing input, an exception is raised. - >>> del FakeRequest.POST['one'] + >>> del FakeRequest.params['one'] >>> print_request(**validator(FakeRequest)) Traceback (most recent call last): ... @@ -101,7 +101,7 @@ On missing input, an exception is raised. If more than one key is missing, it will be reflected in the error message. - >>> del FakeRequest.POST['two'] + >>> del FakeRequest.params['two'] >>> print_request(**validator(FakeRequest)) Traceback (most recent call last): ... @@ -109,8 +109,8 @@ If more than one key is missing, it will be reflected in the error message. Extra keys are also not allowed. - >>> FakeRequest.POST = dict(one='1', two='two', three='yes', - ... four='', five='') + >>> FakeRequest.params = dict(one='1', two='two', three='yes', + ... four='', five='') >>> print_request(**validator(FakeRequest)) Traceback (most recent call last): ... @@ -123,25 +123,25 @@ However, if optional keys are missing, it's okay. ... four=int, five=int, ... _optional=('four', 'five')) - >>> FakeRequest.POST = dict(one='1', two='two', three='yes', - ... four='4', five='5') + >>> FakeRequest.params = dict(one='1', two='two', three='yes', + ... four='4', five='5') >>> def print_request(one, two, three, four=None, five=None): ... print(repr(one), repr(two), repr(three), repr(four), repr(five)) >>> print_request(**validator(FakeRequest)) 1 u'two' True 4 5 - >>> del FakeRequest.POST['four'] + >>> del FakeRequest.params['four'] >>> print_request(**validator(FakeRequest)) 1 u'two' True None 5 - >>> del FakeRequest.POST['five'] + >>> del FakeRequest.params['five'] >>> print_request(**validator(FakeRequest)) 1 u'two' True None None But if the optional values are present, they must of course also be valid. - >>> FakeRequest.POST = dict(one='1', two='two', three='yes', - ... four='no', five='maybe') + >>> FakeRequest.params = dict(one='1', two='two', three='yes', + ... four='no', five='maybe') >>> print_request(**validator(FakeRequest)) Traceback (most recent call last): ... @@ -157,12 +157,17 @@ such form data. Specifically, when a key shows up multiple times in the form data, a list is given to the validator. :: - # Of course we can't use a normal dictionary, but webob has a useful data - # type we can use. - >>> from webob.multidict import MultiDict - >>> form_data = MultiDict(one='1', many='3') - >>> form_data.add('many', '4') - >>> form_data.add('many', '5') + # We can't use a normal dictionary because we'll have multiple keys, but + # the validator only wants to call .items() on the object. + >>> class MultiDict: + ... def __init__(self, *params): self.values = list(params) + ... def items(self): return iter(self.values) + >>> form_data = MultiDict( + ... ('one', '1'), + ... ('many', '3'), + ... ('many', '4'), + ... ('many', '5'), + ... ) This is a validation function that ensures the value is a list. @@ -181,7 +186,7 @@ This is a validation function that ensure the value is *not* a list. And a validator to pull it all together. >>> validator = Validator(one=must_be_scalar, many=must_be_list) - >>> FakeRequest.POST = form_data + >>> FakeRequest.params = form_data >>> values = validator(FakeRequest) >>> print(values['one']) 1 @@ -191,11 +196,12 @@ And a validator to pull it all together. The list values are guaranteed to be in the same order they show up in the form data. - >>> from webob.multidict import MultiDict - >>> form_data = MultiDict(one='1', many='3') - >>> form_data.add('many', '5') - >>> form_data.add('many', '4') - >>> FakeRequest.POST = form_data + >>> FakeRequest.params = MultiDict( + ... ('one', '1'), + ... ('many', '3'), + ... ('many', '5'), + ... ('many', '4'), + ... ) >>> values = validator(FakeRequest) >>> print(values['one']) 1 diff --git a/src/mailman/rest/docs/preferences.rst b/src/mailman/rest/docs/preferences.rst index f4839b1f4..b9332c954 100644 --- a/src/mailman/rest/docs/preferences.rst +++ b/src/mailman/rest/docs/preferences.rst @@ -199,21 +199,12 @@ Preferences accessed through this interface are always read only. receive_own_postings: False self_link: http://localhost:9001/3.0/members/1/all/preferences -These preferences cannot be changed. - - >>> dump_json('http://localhost:9001/3.0/members/1/all/preferences', { - ... 'delivery_status': 'enabled', - ... }, method='PATCH') - Traceback (most recent call last): - ... - HTTPError: HTTP Error 405: 405 Method Not Allowed - System preferences ================== The Mailman system itself has a default set of preference. All preference -lookups fall back to these values, which are read-only. +look ups fall back to these values, which are read-only. >>> dump_json('http://localhost:9001/3.0/system/preferences') acknowledge_posts: False @@ -225,12 +216,3 @@ lookups fall back to these values, which are read-only. receive_list_copy: True receive_own_postings: True self_link: http://localhost:9001/3.0/system/preferences - -These preferences cannot be changed. - - >>> dump_json('http://localhost:9001/3.0/system/preferences', { - ... 'delivery_status': 'enabled', - ... }, method='PATCH') - Traceback (most recent call last): - ... - HTTPError: HTTP Error 405: 405 Method Not Allowed diff --git a/src/mailman/rest/domains.py b/src/mailman/rest/domains.py index 1a260ea3d..5d36dcab9 100644 --- a/src/mailman/rest/domains.py +++ b/src/mailman/rest/domains.py @@ -26,18 +26,18 @@ __all__ = [ ] -from restish import http, resource -from zope.component import getUtility - from mailman.interfaces.domain import ( BadDomainSpecificationError, IDomainManager) -from mailman.rest.helpers import CollectionMixin, etag, no_content, path_to +from mailman.rest.helpers import ( + BadRequest, CollectionMixin, NotFound, bad_request, child, created, etag, + no_content, not_found, okay, path_to) from mailman.rest.lists import ListsForDomain from mailman.rest.validator import Validator +from zope.component import getUtility -class _DomainBase(resource.Resource, CollectionMixin): +class _DomainBase(CollectionMixin): """Shared base class for domain representations.""" def _resource_as_dict(self, domain): @@ -62,41 +62,40 @@ class ADomain(_DomainBase): def __init__(self, domain): self._domain = domain - @resource.GET() - def domain(self, request): + def on_get(self, request, response): """Return a single domain end-point.""" domain = getUtility(IDomainManager).get(self._domain) if domain is None: - return http.not_found() - return http.ok([], self._resource_as_json(domain)) + not_found(response) + else: + okay(response, self._resource_as_json(domain)) - @resource.DELETE() - def delete(self, request): + def on_delete(self, request, response): """Delete the domain.""" try: getUtility(IDomainManager).remove(self._domain) except KeyError: # The domain does not exist. - return http.not_found() - return no_content() + not_found(response) + else: + no_content(response) - @resource.child() + @child() def lists(self, request, segments): """/domains/<domain>/lists""" if len(segments) == 0: domain = getUtility(IDomainManager).get(self._domain) if domain is None: - return http.not_found() + return NotFound() return ListsForDomain(domain) else: - return http.bad_request() + return BadRequest(), [] class AllDomains(_DomainBase): """The domains.""" - @resource.POST() - def create(self, request): + def on_post(self, request, response): """Create a new domain.""" domain_manager = getUtility(IDomainManager) try: @@ -108,15 +107,13 @@ class AllDomains(_DomainBase): 'contact_address')) domain = domain_manager.add(**validator(request)) except BadDomainSpecificationError: - return http.bad_request([], b'Domain exists') + bad_request(response, b'Domain exists') except ValueError as error: - return http.bad_request([], str(error)) - location = path_to('domains/{0}'.format(domain.mail_host)) - # Include no extra headers or body. - return http.created(location, [], None) + bad_request(response, str(error)) + else: + created(response, path_to('domains/{0}'.format(domain.mail_host))) - @resource.GET() - def collection(self, request): + def on_get(self, request, response): """/domains""" resource = self._make_collection(request) - return http.ok([], etag(resource)) + okay(response, etag(resource)) diff --git a/src/mailman/rest/helpers.py b/src/mailman/rest/helpers.py index db3e43af0..0bc312b1f 100644 --- a/src/mailman/rest/helpers.py +++ b/src/mailman/rest/helpers.py @@ -21,30 +21,32 @@ from __future__ import absolute_import, print_function, unicode_literals __metaclass__ = type __all__ = [ + 'BadRequest', + 'ChildError', 'GetterSetter', - 'PATCH', + 'NotFound', + 'bad_request', + 'child', + 'conflict', + 'created', 'etag', + 'forbidden', 'no_content', + 'not_found', + 'okay', 'path_to', - 'restish_matcher', ] -import cgi import json +import falcon import hashlib -from cStringIO import StringIO from datetime import datetime, timedelta from enum import Enum from lazr.config import as_boolean -from pprint import pformat -from restish import http -from restish.http import Response -from restish.resource import MethodDecorator -from webob.multidict import MultiDict - from mailman.config import config +from pprint import pformat @@ -123,22 +125,15 @@ def paginate(method): arguments. """ def wrapper(self, request, *args, **kwargs): - try: - count = int(request.GET['count']) - page = int(request.GET['page']) - if count < 0 or page < 0: - return http.bad_request([], b'Invalid parameters') - # Wrong parameter types or no GET attribute in request object. - except (AttributeError, ValueError, TypeError): - return http.bad_request([], b'Invalid parameters') - # No count/page params. - except KeyError: - count = page = None + # Allow falcon's HTTPBadRequest exceptions to percolate up. They'll + # get turned into HTTP 400 errors. + count = request.get_param_as_int('count', min=0) + page = request.get_param_as_int('page', min=1) result = method(self, request, *args, **kwargs) if count is None and page is None: return result - list_start = int((page - 1) * count) - list_end = int(page * count) + list_start = (page - 1) * count + list_end = page * count return result[list_start:list_end] return wrapper @@ -168,14 +163,14 @@ class CollectionMixin: This must be implemented by subclasses. - :param request: A restish request. + :param request: An http request. :return: The collection :rtype: list """ raise NotImplementedError def _make_collection(self, request): - """Provide the collection to restish.""" + """Provide the collection to the REST layer.""" collection = self._get_collection(request) if len(collection) == 0: return dict(start=0, total_size=0) @@ -193,59 +188,6 @@ class CollectionMixin: -# XXX 2010-02-24 barry Seems like contrary to the documentation, matchers -# cannot be plain functions, because matchers must have a .score attribute. -# OTOH, I think they support regexps, so that might be a better way to go. -def restish_matcher(function): - """Decorator for restish matchers.""" - function.score = () - return function - - -# restish doesn't support HTTP response code 204. -def no_content(): - """204 No Content.""" - return Response('204 No Content', [], None) - - -# These two classes implement an ugly, dirty hack to work around the fact that -# neither WebOb nor really the stdlib cgi module support non-standard HTTP -# verbs such as PATCH. Note that restish handles it just fine in the sense -# that the right method gets called, but without the following kludge, the -# body of the request will never get decoded, so the method won't see any -# data. -# -# Stuffing the MultiDict on request.PATCH is pretty ugly, but it mirrors -# WebOb's use of request.POST and request.PUT for those standard verbs. -# Besides, WebOb refuses to allow us to set request.POST. This does make -# validators.py a bit more complicated. :( - -class PATCHWrapper: - """Hack to decode the request body for PATCH.""" - def __init__(self, func): - self.func = func - - def __call__(self, resource, request): - # We can't use request.body_file because that's a socket that's - # already had its data read off of. IOW, if we use that directly, - # we'll block here. - field_storage = cgi.FieldStorage( - fp=StringIO(request.body), - # Yes, lie about the method so cgi will do the right thing. - environ=dict(REQUEST_METHOD='POST'), - keep_blank_values=True) - request.PATCH = MultiDict.from_fieldstorage(field_storage) - return self.func(resource, request) - - -class PATCH(MethodDecorator): - method = 'PATCH' - - def __call__(self, func): - really_wrapped_func = PATCHWrapper(func) - return super(PATCH, self).__call__(really_wrapped_func) - - class GetterSetter: """Get and set attributes on an object. @@ -304,3 +246,79 @@ class GetterSetter: if self.decoder is None: return value return self.decoder(value) + + + +# Falcon REST framework add-ons. + +def child(matcher=None): + def decorator(func): + if matcher is None: + func.__matcher__ = func.__name__ + else: + func.__matcher__ = matcher + return func + return decorator + + +class ChildError: + def __init__(self, status): + self._status = status + + def _oops(self, request, response): + raise falcon.HTTPError(self._status, None) + + on_get = _oops + on_post = _oops + on_put = _oops + on_patch = _oops + on_delete = _oops + + +class BadRequest(ChildError): + def __init__(self): + super(BadRequest, self).__init__(falcon.HTTP_400) + + +class NotFound(ChildError): + def __init__(self): + super(NotFound, self).__init__(falcon.HTTP_404) + + +def okay(response, body=None): + response.status = falcon.HTTP_200 + if body is not None: + response.body = body + + +def no_content(response): + response.status = falcon.HTTP_204 + + +def not_found(response, body=b'404 Not Found'): + response.status = falcon.HTTP_404 + if body is not None: + response.body = body + + +def bad_request(response, body='400 Bad Request'): + response.status = falcon.HTTP_400 + if body is not None: + response.body = body + + +def created(response, location): + response.status = falcon.HTTP_201 + response.location = location + + +def conflict(response, body=b'409 Conflict'): + response.status = falcon.HTTP_409 + if body is not None: + response.body = body + + +def forbidden(response, body=b'403 Forbidden'): + response.status = falcon.HTTP_403 + if body is not None: + response.body = body diff --git a/src/mailman/rest/lists.py b/src/mailman/rest/lists.py index 552824927..580b6e898 100644 --- a/src/mailman/rest/lists.py +++ b/src/mailman/rest/lists.py @@ -26,32 +26,33 @@ __all__ = [ 'ListArchivers', 'ListConfiguration', 'ListsForDomain', + 'Styles', ] from lazr.config import as_boolean from operator import attrgetter -from restish import http, resource from zope.component import getUtility from mailman.app.lifecycle import create_list, remove_list +from mailman.config import config from mailman.interfaces.domain import BadDomainSpecificationError from mailman.interfaces.listmanager import ( IListManager, ListAlreadyExistsError) from mailman.interfaces.mailinglist import IListArchiverSet from mailman.interfaces.member import MemberRole +from mailman.interfaces.styles import IStyleManager from mailman.interfaces.subscriptions import ISubscriptionService from mailman.rest.configuration import ListConfiguration from mailman.rest.helpers import ( - CollectionMixin, GetterSetter, PATCH, etag, no_content, paginate, path_to, - restish_matcher) + CollectionMixin, GetterSetter, NotFound, bad_request, child, created, + etag, no_content, not_found, okay, paginate, path_to) from mailman.rest.members import AMember, MemberCollection from mailman.rest.moderation import HeldMessages, SubscriptionRequests from mailman.rest.validator import Validator -@restish_matcher def member_matcher(request, segments): """A matcher of member URLs inside mailing lists. @@ -64,13 +65,9 @@ def member_matcher(request, segments): except KeyError: # Not a valid role. return None - # No more segments. - # XXX 2010-02-25 barry Matchers are undocumented in restish; they return a - # 3-tuple of (match_args, match_kws, segments). return (), dict(role=role, email=segments[1]), () -@restish_matcher def roster_matcher(request, segments): """A matcher of all members URLs inside mailing lists. @@ -85,7 +82,6 @@ def roster_matcher(request, segments): return None -@restish_matcher def config_matcher(request, segments): """A matcher for a mailing list's configuration resource. @@ -103,7 +99,7 @@ def config_matcher(request, segments): -class _ListBase(resource.Resource, CollectionMixin): +class _ListBase(CollectionMixin): """Shared base class for mailing list representations.""" def _resource_as_dict(self, mlist): @@ -138,66 +134,66 @@ class AList(_ListBase): else: self._mlist = manager.get_by_list_id(list_identifier) - @resource.GET() - def mailing_list(self, request): + def on_get(self, request, response): """Return a single mailing list end-point.""" if self._mlist is None: - return http.not_found() - return http.ok([], self._resource_as_json(self._mlist)) + not_found(response) + else: + okay(response, self._resource_as_json(self._mlist)) - @resource.DELETE() - def delete_list(self, request): + def on_delete(self, request, response): """Delete the named mailing list.""" if self._mlist is None: - return http.not_found() - remove_list(self._mlist) - return no_content() + not_found(response) + else: + remove_list(self._mlist) + no_content(response) - @resource.child(member_matcher) + @child(member_matcher) def member(self, request, segments, role, email): """Return a single member representation.""" if self._mlist is None: - return http.not_found() + return NotFound(), [] members = getUtility(ISubscriptionService).find_members( email, self._mlist.list_id, role) if len(members) == 0: - return http.not_found() + return NotFound(), [] assert len(members) == 1, 'Too many matches' return AMember(members[0].member_id) - @resource.child(roster_matcher) + @child(roster_matcher) def roster(self, request, segments, role): """Return the collection of all a mailing list's members.""" if self._mlist is None: - return http.not_found() + return NotFound(), [] return MembersOfList(self._mlist, role) - @resource.child(config_matcher) + @child(config_matcher) def config(self, request, segments, attribute=None): """Return a mailing list configuration object.""" if self._mlist is None: - return http.not_found() + return NotFound(), [] return ListConfiguration(self._mlist, attribute) - @resource.child() + @child() def held(self, request, segments): """Return a list of held messages for the mailing list.""" if self._mlist is None: - return http.not_found() + return NotFound(), [] return HeldMessages(self._mlist) - @resource.child() + @child() def requests(self, request, segments): """Return a list of subscription/unsubscription requests.""" if self._mlist is None: - return http.not_found() + return NotFound(), [] return SubscriptionRequests(self._mlist) - @resource.child() + @child() def archivers(self, request, segments): """Return a representation of mailing list archivers.""" if self._mlist is None: - return http.not_found() + return NotFound(), [] return ListArchivers(self._mlist) @@ -205,8 +201,7 @@ class AList(_ListBase): class AllLists(_ListBase): """The mailing lists.""" - @resource.POST() - def create(self, request): + def on_post(self, request, response): """Create a new mailing list.""" try: validator = Validator(fqdn_listname=unicode, @@ -214,22 +209,20 @@ class AllLists(_ListBase): _optional=('style_name',)) mlist = create_list(**validator(request)) except ListAlreadyExistsError: - return http.bad_request([], b'Mailing list exists') + bad_request(response, b'Mailing list exists') except BadDomainSpecificationError as error: - return http.bad_request([], b'Domain does not exist: {0}'.format( - error.domain)) + bad_request( + response, + b'Domain does not exist: {0}'.format(error.domain)) except ValueError as error: - return http.bad_request([], str(error)) - # wsgiref wants headers to be bytes, not unicodes. - location = path_to('lists/{0}'.format(mlist.list_id)) - # Include no extra headers or body. - return http.created(location, [], None) + bad_request(response, str(error)) + else: + created(response, path_to('lists/{0}'.format(mlist.list_id))) - @resource.GET() - def collection(self, request): + def on_get(self, request, response): """/lists""" resource = self._make_collection(request) - return http.ok([], etag(resource)) + okay(response, etag(resource)) @@ -257,11 +250,10 @@ class ListsForDomain(_ListBase): def __init__(self, domain): self._domain = domain - @resource.GET() - def collection(self, request): + def on_get(self, request, response): """/domains/<domain>/lists""" resource = self._make_collection(request) - return http.ok([], etag(resource)) + okay(response, etag(resource)) @paginate def _get_collection(self, request): @@ -287,21 +279,20 @@ class ArchiverGetterSetter(GetterSetter): archiver.is_enabled = as_boolean(value) -class ListArchivers(resource.Resource): +class ListArchivers: """The archivers for a list, with their enabled flags.""" def __init__(self, mlist): self._mlist = mlist - @resource.GET() - def statuses(self, request): + def on_get(self, request, response): """Get all the archiver statuses.""" archiver_set = IListArchiverSet(self._mlist) resource = {archiver.name: archiver.is_enabled for archiver in archiver_set.archivers} - return http.ok([], etag(resource)) + okay(response, etag(resource)) - def patch_put(self, request, is_optional): + def patch_put(self, request, response, is_optional): archiver_set = IListArchiverSet(self._mlist) kws = {archiver.name: ArchiverGetterSetter(self._mlist) for archiver in archiver_set.archivers} @@ -311,15 +302,29 @@ class ListArchivers(resource.Resource): try: Validator(**kws).update(self._mlist, request) except ValueError as error: - return http.bad_request([], str(error)) - return no_content() + bad_request(response, str(error)) + else: + no_content(response) - @resource.PUT() - def put_statuses(self, request): + def on_put(self, request, response): """Update all the archiver statuses.""" - return self.patch_put(request, is_optional=False) + self.patch_put(request, response, is_optional=False) - @PATCH() - def patch_statuses(self, request): + def on_patch(self, request, response): """Patch some archiver statueses.""" - return self.patch_put(request, is_optional=True) + self.patch_put(request, response, is_optional=True) + + + +class Styles: + """Simple resource representing all list styles.""" + + def __init__(self): + manager = getUtility(IStyleManager) + style_names = sorted(style.name for style in manager.styles) + self._resource = dict( + style_names=style_names, + default=config.styles.default) + + def on_get(self, request, response): + okay(response, etag(self._resource)) diff --git a/src/mailman/rest/members.py b/src/mailman/rest/members.py index 51cbbd888..4d1c87b73 100644 --- a/src/mailman/rest/members.py +++ b/src/mailman/rest/members.py @@ -30,7 +30,6 @@ __all__ = [ from uuid import UUID from operator import attrgetter -from restish import http, resource from zope.component import getUtility from mailman.app.membership import delete_member @@ -43,14 +42,15 @@ from mailman.interfaces.subscriptions import ISubscriptionService from mailman.interfaces.user import UnverifiedAddressError from mailman.interfaces.usermanager import IUserManager from mailman.rest.helpers import ( - CollectionMixin, PATCH, etag, no_content, paginate, path_to) + CollectionMixin, NotFound, bad_request, child, conflict, created, etag, + no_content, not_found, okay, paginate, path_to) from mailman.rest.preferences import Preferences, ReadOnlyPreferences from mailman.rest.validator import ( Validator, enum_validator, subscriber_validator) -class _MemberBase(resource.Resource, CollectionMixin): +class _MemberBase(CollectionMixin): """Shared base class for member representations.""" def _resource_as_dict(self, member): @@ -94,11 +94,10 @@ class MemberCollection(_MemberBase): """See `CollectionMixin`.""" raise NotImplementedError - @resource.GET() - def container(self, request): + def on_get(self, request, response): """roster/[members|owners|moderators]""" resource = self._make_collection(request) - return http.ok([], etag(resource)) + okay(response, etag(resource)) @@ -117,90 +116,93 @@ class AMember(_MemberBase): service = getUtility(ISubscriptionService) self._member = service.get_member(member_id) - @resource.GET() - def member(self, request): + def on_get(self, request, response): """Return a single member end-point.""" if self._member is None: - return http.not_found() - return http.ok([], self._resource_as_json(self._member)) + not_found(response) + else: + okay(response, self._resource_as_json(self._member)) - @resource.child() + @child() def preferences(self, request, segments): """/members/<id>/preferences""" if len(segments) != 0: - return http.bad_request() + return NotFound(), [] if self._member is None: - return http.not_found() + return NotFound(), [] child = Preferences( self._member.preferences, 'members/{0}'.format(self._member.member_id.int)) return child, [] - @resource.child() + @child() def all(self, request, segments): """/members/<id>/all/preferences""" if len(segments) == 0: - return http.not_found() + return NotFound(), [] if self._member is None: - return http.not_found() + return NotFound(), [] child = ReadOnlyPreferences( self._member, 'members/{0}/all'.format(self._member.member_id.int)) return child, [] - @resource.DELETE() - def delete(self, request): + def on_delete(self, request, response): """Delete the member (i.e. unsubscribe).""" # Leaving a list is a bit different than deleting a moderator or # owner. Handle the former case first. For now too, we will not send # an admin or user notification. if self._member is None: - return http.not_found() + not_found(response) + return mlist = getUtility(IListManager).get_by_list_id(self._member.list_id) if self._member.role is MemberRole.member: try: delete_member(mlist, self._member.address.email, False, False) except NotAMemberError: - return http.not_found() + not_found(response) + return else: self._member.unsubscribe() - return no_content() + no_content(response) - @PATCH() - def patch_membership(self, request): + def on_patch(self, request, response): """Patch the membership. This is how subscription changes are done. """ if self._member is None: - return http.not_found() + not_found(response) + return try: values = Validator( address=unicode, delivery_mode=enum_validator(DeliveryMode), _optional=('address', 'delivery_mode'))(request) except ValueError as error: - return http.bad_request([], str(error)) + bad_request(response, str(error)) + return if 'address' in values: email = values['address'] address = getUtility(IUserManager).get_address(email) if address is None: - return http.bad_request([], b'Address not registered') + bad_request(response, b'Address not registered') + return try: self._member.address = address except (MembershipError, UnverifiedAddressError) as error: - return http.bad_request([], str(error)) + bad_request(response, str(error)) + return if 'delivery_mode' in values: self._member.preferences.delivery_mode = values['delivery_mode'] - return no_content() + no_content(response) class AllMembers(_MemberBase): """The members.""" - @resource.POST() - def create(self, request): + def on_post(self, request, response): """Create a new member.""" service = getUtility(ISubscriptionService) try: @@ -213,25 +215,24 @@ class AllMembers(_MemberBase): _optional=('delivery_mode', 'display_name', 'role')) member = service.join(**validator(request)) except AlreadySubscribedError: - return http.conflict([], b'Member already subscribed') + conflict(response, b'Member already subscribed') except NoSuchListError: - return http.bad_request([], b'No such list') + bad_request(response, b'No such list') except InvalidEmailAddressError: - return http.bad_request([], b'Invalid email address') + bad_request(response, b'Invalid email address') except ValueError as error: - return http.bad_request([], str(error)) - # The member_id are UUIDs. We need to use the integer equivalent in - # the URL. - member_id = member.member_id.int - location = path_to('members/{0}'.format(member_id)) - # Include no extra headers or body. - return http.created(location, [], None) + bad_request(response, str(error)) + else: + # The member_id are UUIDs. We need to use the integer equivalent + # in the URL. + member_id = member.member_id.int + location = path_to('members/{0}'.format(member_id)) + created(response, location) - @resource.GET() - def container(self, request): + def on_get(self, request, response): """/members""" resource = self._make_collection(request) - return http.ok([], etag(resource)) + okay(response, etag(resource)) @@ -251,8 +252,7 @@ class _FoundMembers(MemberCollection): class FindMembers(_MemberBase): """/members/find""" - @resource.POST() - def find(self, request): + def on_post(self, request, response): """Find a member""" service = getUtility(ISubscriptionService) validator = Validator( @@ -260,10 +260,10 @@ class FindMembers(_MemberBase): subscriber=unicode, role=enum_validator(MemberRole), _optional=('list_id', 'subscriber', 'role')) - members = service.find_members(**validator(request)) - # We can't just return the _FoundMembers instance, because - # CollectionMixins have only a GET method, which is incompatible with - # this POSTed resource. IOW, without doing this here, restish would - # throw a 405 Method Not Allowed. - resource = _FoundMembers(members)._make_collection(request) - return http.ok([], etag(resource)) + try: + members = service.find_members(**validator(request)) + except ValueError as error: + bad_request(response, str(error)) + else: + resource = _FoundMembers(members)._make_collection(request) + okay(response, etag(resource)) diff --git a/src/mailman/rest/moderation.py b/src/mailman/rest/moderation.py index 1a302ff3c..0bdc50688 100644 --- a/src/mailman/rest/moderation.py +++ b/src/mailman/rest/moderation.py @@ -28,16 +28,15 @@ __all__ = [ ] -from restish import http, resource -from zope.component import getUtility - from mailman.app.moderator import ( handle_message, handle_subscription, handle_unsubscription) from mailman.interfaces.action import Action from mailman.interfaces.messages import IMessageStore from mailman.interfaces.requests import IListRequests, RequestType -from mailman.rest.helpers import CollectionMixin, etag, no_content +from mailman.rest.helpers import ( + CollectionMixin, bad_request, child, etag, no_content, not_found, okay) from mailman.rest.validator import Validator, enum_validator +from zope.component import getUtility HELD_MESSAGE_REQUESTS = (RequestType.held_message,) @@ -101,45 +100,48 @@ class _HeldMessageBase(_ModerationBase): return resource -class HeldMessage(_HeldMessageBase, resource.Resource): +class HeldMessage(_HeldMessageBase): """Resource for moderating a held message.""" def __init__(self, mlist, request_id): self._mlist = mlist self._request_id = request_id - @resource.GET() - def details(self, request): + def on_get(self, request, response): try: request_id = int(self._request_id) except ValueError: - return http.bad_request() + bad_request(response) + return resource = self._make_resource(request_id) if resource is None: - return http.not_found() - return http.ok([], etag(resource)) + not_found(response) + else: + okay(response, etag(resource)) - @resource.POST() - def moderate(self, request): + def on_post(self, request, response): try: validator = Validator(action=enum_validator(Action)) arguments = validator(request) except ValueError as error: - return http.bad_request([], str(error)) + bad_request(response, str(error)) + return requests = IListRequests(self._mlist) try: request_id = int(self._request_id) except ValueError: - return http.bad_request() + bad_request(response) + return results = requests.get_request(request_id, RequestType.held_message) if results is None: - return http.not_found() - handle_message(self._mlist, request_id, **arguments) - return no_content() + not_found(response) + else: + handle_message(self._mlist, request_id, **arguments) + no_content(response) -class HeldMessages(_HeldMessageBase, resource.Resource, CollectionMixin): +class HeldMessages(_HeldMessageBase, CollectionMixin): """Resource for messages held for moderation.""" def __init__(self, mlist): @@ -155,70 +157,72 @@ class HeldMessages(_HeldMessageBase, resource.Resource, CollectionMixin): self._requests = requests return list(requests.of_type(RequestType.held_message)) - @resource.GET() - def requests(self, request): + def on_get(self, request, response): """/lists/listname/held""" - # `request` is a restish.http.Request object. resource = self._make_collection(request) - return http.ok([], etag(resource)) + okay(response, etag(resource)) - @resource.child('{id}') + @child(r'^(?P<id>[^/]+)') def message(self, request, segments, **kw): return HeldMessage(self._mlist, kw['id']) -class MembershipChangeRequest(resource.Resource, _ModerationBase): +class MembershipChangeRequest(_ModerationBase): """Resource for moderating a membership change.""" def __init__(self, mlist, request_id): self._mlist = mlist self._request_id = request_id - @resource.GET() - def details(self, request): + def on_get(self, request, response): try: request_id = int(self._request_id) except ValueError: - return http.bad_request() + bad_request(response) + return resource = self._make_resource(request_id, MEMBERSHIP_CHANGE_REQUESTS) if resource is None: - return http.not_found() - # Remove unnecessary keys. - del resource['key'] - return http.ok([], etag(resource)) + not_found(response) + else: + # Remove unnecessary keys. + del resource['key'] + okay(response, etag(resource)) - @resource.POST() - def moderate(self, request): + def on_post(self, request, response): try: validator = Validator(action=enum_validator(Action)) arguments = validator(request) except ValueError as error: - return http.bad_request([], str(error)) + bad_request(response, str(error)) + return requests = IListRequests(self._mlist) try: request_id = int(self._request_id) except ValueError: - return http.bad_request() + bad_request(response) + return results = requests.get_request(request_id) if results is None: - return http.not_found() + not_found(response) + return key, data = results try: request_type = RequestType[data['_request_type']] except ValueError: - return http.bad_request() + bad_request(response) + return if request_type is RequestType.subscription: handle_subscription(self._mlist, request_id, **arguments) elif request_type is RequestType.unsubscription: handle_unsubscription(self._mlist, request_id, **arguments) else: - return http.bad_request() - return no_content() + bad_request(response) + return + no_content(response) -class SubscriptionRequests( - _ModerationBase, resource.Resource, CollectionMixin): +class SubscriptionRequests(_ModerationBase, CollectionMixin): """Resource for membership change requests.""" def __init__(self, mlist): @@ -241,13 +245,11 @@ class SubscriptionRequests( items.append(request) return items - @resource.GET() - def requests(self, request): + def on_get(self, request, response): """/lists/listname/requests""" - # `request` is a restish.http.Request object. resource = self._make_collection(request) - return http.ok([], etag(resource)) + okay(response, etag(resource)) - @resource.child('{id}') + @child(r'^(?P<id>[^/]+)') def subscription(self, request, segments, **kw): return MembershipChangeRequest(self._mlist, kw['id']) diff --git a/src/mailman/rest/preferences.py b/src/mailman/rest/preferences.py index bbc6b1769..b85388ec9 100644 --- a/src/mailman/rest/preferences.py +++ b/src/mailman/rest/preferences.py @@ -27,11 +27,9 @@ __all__ = [ from lazr.config import as_boolean -from restish import http, resource - from mailman.interfaces.member import DeliveryMode, DeliveryStatus from mailman.rest.helpers import ( - GetterSetter, PATCH, etag, no_content, path_to) + GetterSetter, bad_request, etag, no_content, not_found, okay, path_to) from mailman.rest.validator import ( Validator, enum_validator, language_validator) @@ -48,15 +46,14 @@ PREFERENCES = ( -class ReadOnlyPreferences(resource.Resource): +class ReadOnlyPreferences: """.../<object>/preferences""" def __init__(self, parent, base_url): self._parent = parent self._base_url = base_url - @resource.GET() - def preferences(self, segments): + def on_get(self, request, response): resource = dict() for attr in PREFERENCES: # Handle this one specially. @@ -72,16 +69,17 @@ class ReadOnlyPreferences(resource.Resource): # Add the self link. resource['self_link'] = path_to( '{0}/preferences'.format(self._base_url)) - return http.ok([], etag(resource)) + okay(response, etag(resource)) class Preferences(ReadOnlyPreferences): """Preferences which can be changed.""" - def patch_put(self, request, is_optional): + def patch_put(self, request, response, is_optional): if self._parent is None: - return http.not_found() + not_found(response) + return kws = dict( acknowledge_posts=GetterSetter(as_boolean), hide_address = GetterSetter(as_boolean), @@ -97,23 +95,21 @@ class Preferences(ReadOnlyPreferences): try: Validator(**kws).update(self._parent, request) except ValueError as error: - return http.bad_request([], str(error)) - return no_content() + bad_request(response, str(error)) + else: + no_content(response) - @PATCH() - def patch_preferences(self, request): + def on_patch(self, request, response): """Patch the preferences.""" - return self.patch_put(request, is_optional=True) + self.patch_put(request, response, is_optional=True) - @resource.PUT() - def put_preferences(self, request): + def on_put(self, request, response): """Change all preferences.""" - return self.patch_put(request, is_optional=False) + self.patch_put(request, response, is_optional=False) - @resource.DELETE() - def delete_preferences(self, request): + def on_delete(self, request, response): """Delete all preferences.""" for attr in PREFERENCES: if hasattr(self._parent, attr): setattr(self._parent, attr, None) - return no_content() + no_content(response) diff --git a/src/mailman/rest/root.py b/src/mailman/rest/root.py index ec0c9c93b..93a8f0086 100644 --- a/src/mailman/rest/root.py +++ b/src/mailman/rest/root.py @@ -25,19 +25,20 @@ __all__ = [ ] +import falcon + from base64 import b64decode -from restish import guard, http, resource from zope.component import getUtility from mailman.config import config from mailman.core.constants import system_preferences from mailman.core.system import system from mailman.interfaces.listmanager import IListManager -from mailman.interfaces.styles import IStyleManager from mailman.rest.addresses import AllAddresses, AnAddress from mailman.rest.domains import ADomain, AllDomains -from mailman.rest.helpers import etag, path_to -from mailman.rest.lists import AList, AllLists +from mailman.rest.helpers import ( + BadRequest, NotFound, child, etag, okay, path_to) +from mailman.rest.lists import AList, AllLists, Styles from mailman.rest.members import AMember, AllMembers, FindMembers from mailman.rest.preferences import ReadOnlyPreferences from mailman.rest.templates import TemplateFinder @@ -45,20 +46,7 @@ from mailman.rest.users import AUser, AllUsers -def webservice_auth_checker(request, obj): - auth = request.environ.get('HTTP_AUTHORIZATION', '') - if auth.startswith('Basic '): - credentials = b64decode(auth[6:]) - username, password = credentials.split(':', 1) - if (username != config.webservice.admin_user or - password != config.webservice.admin_pass): - # Not authorized. - raise guard.GuardError(b'User is not authorized for the REST API') - else: - raise guard.GuardError(b'The REST API requires authentication') - - -class Root(resource.Resource): +class Root: """The RESTful root resource. At the root of the tree are the API version numbers. Everything else @@ -67,33 +55,58 @@ class Root(resource.Resource): always be the case though. """ - @resource.child(config.webservice.api_version) - @guard.guard(webservice_auth_checker) + @child(config.webservice.api_version) def api_version(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( + b'401 Unauthorized', + b'The REST API requires authentication') + if request.auth.startswith('Basic '): + credentials = b64decode(request.auth[6:]) + username, password = credentials.split(':', 1) + if (username != config.webservice.admin_user or + password != config.webservice.admin_pass): + # Not authorized. + raise falcon.HTTPUnauthorized( + b'401 Unauthorized', + b'User is not authorized for the REST API') return TopLevel() -class TopLevel(resource.Resource): +class System: + def on_get(self, request, response): + """/<api>/system""" + resource = dict( + mailman_version=system.mailman_version, + python_version=system.python_version, + self_link=path_to('system'), + ) + okay(response, etag(resource)) + + +class TopLevel: """Top level collections and entries.""" - @resource.child() + @child() def system(self, request, segments): """/<api>/system""" if len(segments) == 0: - resource = dict( - mailman_version=system.mailman_version, - python_version=system.python_version, - self_link=path_to('system'), - ) + return System() elif len(segments) > 1: - return http.bad_request() + return BadRequest(), [] elif segments[0] == 'preferences': return ReadOnlyPreferences(system_preferences, 'system'), [] else: - return http.bad_request() - return http.ok([], etag(resource)) + return NotFound(), [] - @resource.child() + @child() def addresses(self, request, segments): """/<api>/addresses /<api>/addresses/<email> @@ -104,7 +117,7 @@ class TopLevel(resource.Resource): email = segments.pop(0) return AnAddress(email), segments - @resource.child() + @child() def domains(self, request, segments): """/<api>/domains /<api>/domains/<domain> @@ -115,7 +128,7 @@ class TopLevel(resource.Resource): domain = segments.pop(0) return ADomain(domain), segments - @resource.child() + @child() def lists(self, request, segments): """/<api>/lists /<api>/lists/<list> @@ -124,18 +137,14 @@ class TopLevel(resource.Resource): if len(segments) == 0: return AllLists() elif len(segments) == 1 and segments[0] == 'styles': - manager = getUtility(IStyleManager) - style_names = sorted(style.name for style in manager.styles) - resource = dict(style_names=style_names, - default=config.styles.default) - return http.ok([], etag(resource)) + return Styles(), [] else: # list-id is preferred, but for backward compatibility, # fqdn_listname is also accepted. list_identifier = segments.pop(0) return AList(list_identifier), segments - @resource.child() + @child() def members(self, request, segments): """/<api>/members""" if len(segments) == 0: @@ -148,7 +157,7 @@ class TopLevel(resource.Resource): else: return AMember(segment), segments - @resource.child() + @child() def users(self, request, segments): """/<api>/users""" if len(segments) == 0: @@ -157,7 +166,7 @@ class TopLevel(resource.Resource): user_id = segments.pop(0) return AUser(user_id), segments - @resource.child() + @child() def templates(self, request, segments): """/<api>/templates/<fqdn_listname>/<template>/[<language>] @@ -169,10 +178,10 @@ class TopLevel(resource.Resource): fqdn_listname, template = segments language = 'en' else: - return http.bad_request() + return BadRequest(), [] mlist = getUtility(IListManager).get(fqdn_listname) if mlist is None: - return http.not_found() + return NotFound(), [] # XXX dig out content-type from request content_type = None return TemplateFinder( diff --git a/src/mailman/rest/templates.py b/src/mailman/rest/templates.py index bee21d7de..44dcdefc5 100644 --- a/src/mailman/rest/templates.py +++ b/src/mailman/rest/templates.py @@ -25,11 +25,7 @@ __all__ = [ ] -import os - -from restish import http, resource - -from mailman.config import config +from mailman.rest.helpers import not_found from mailman.utilities.i18n import TemplateNotFoundError, find @@ -41,7 +37,7 @@ EXTENSIONS = { -class TemplateFinder(resource.Resource): +class TemplateFinder: """Template finder resource.""" def __init__(self, mlist, template, language, content_type): @@ -50,19 +46,20 @@ class TemplateFinder(resource.Resource): self.language = language self.content_type = content_type - @resource.GET() - def find_template(self, request): + def on_get(self, request, response): # XXX We currently only support .txt and .html files. extension = EXTENSIONS.get(self.content_type) if extension is None: - return http.not_found() + not_found(response) + return template = self.template + extension fp = None try: try: path, fp = find(template, self.mlist, self.language) except TemplateNotFoundError: - return http.not_found() + not_found(response) + return else: return fp.read() finally: diff --git a/src/mailman/rest/tests/test_addresses.py b/src/mailman/rest/tests/test_addresses.py index 198674b22..f4aeb3013 100644 --- a/src/mailman/rest/tests/test_addresses.py +++ b/src/mailman/rest/tests/test_addresses.py @@ -46,6 +46,13 @@ class TestAddresses(unittest.TestCase): with transaction(): self._mlist = create_list('test@example.com') + def test_no_addresses(self): + # At first, there are no addresses. + url = 'http://localhost:9001/3.0/addresses' + json, response = call_api(url) + self.assertEqual(json['start'], 0) + self.assertEqual(json['total_size'], 0) + def test_membership_of_missing_address(self): # Try to get the memberships of a missing address. with self.assertRaises(HTTPError) as cm: @@ -111,7 +118,7 @@ class TestAddresses(unittest.TestCase): self.assertEqual(cm.exception.code, 400) def test_address_added_to_user(self): - # Address is added to a user record. + # An address is added to a user record. user_manager = getUtility(IUserManager) with transaction(): anne = user_manager.create_user('anne@example.com') @@ -184,6 +191,13 @@ class TestAddresses(unittest.TestCase): self.assertEqual(cm.exception.code, 400) self.assertEqual(cm.exception.reason, 'Missing parameters: email') + def test_get_addresses_of_missing_user(self): + # There is no user associated with the given address. + with self.assertRaises(HTTPError) as cm: + call_api( + 'http://localhost:9001/3.0/users/anne@example.com/addresses') + self.assertEqual(cm.exception.code, 404) + def test_add_address_to_missing_user(self): # The user that the address is being added to must exist. with self.assertRaises(HTTPError) as cm: diff --git a/src/mailman/rest/tests/test_configuration.py b/src/mailman/rest/tests/test_configuration.py new file mode 100644 index 000000000..93171ec4b --- /dev/null +++ b/src/mailman/rest/tests/test_configuration.py @@ -0,0 +1,94 @@ +# Copyright (C) 2014 by the Free Software Foundation, Inc. +# +# This file is part of GNU Mailman. +# +# GNU Mailman is free software: you can redistribute it and/or modify it under +# the terms of the GNU General Public License as published by the Free +# Software Foundation, either version 3 of the License, or (at your option) +# any later version. +# +# GNU Mailman is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# 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 list configuration via the REST API.""" + +from __future__ import absolute_import, print_function, unicode_literals + +__metaclass__ = type +__all__ = [ + 'TestConfiguration', + ] + + +import unittest + +from mailman.app.lifecycle import create_list +from mailman.database.transaction import transaction +from mailman.interfaces.mailinglist import IAcceptableAliasSet +from mailman.testing.helpers import call_api +from mailman.testing.layers import RESTLayer + + + +class TestConfiguration(unittest.TestCase): + """Test list configuration via the REST API.""" + + layer = RESTLayer + + def setUp(self): + with transaction(): + self._mlist = create_list('test@example.com') + + def test_put_configuration(self): + aliases = [ + 'ant@example.com', + 'bee@example.com', + 'cat@example.com', + ] + # When using PUT, all writable attributes must be included. + resource, response = call_api( + 'http://localhost:9001/3.0/lists/test@example.com/config', + dict( + acceptable_aliases=aliases, + admin_immed_notify=False, + admin_notify_mchanges=True, + administrivia=False, + advertised=False, + anonymous_list=True, + archive_policy='never', + autorespond_owner='respond_and_discard', + autorespond_postings='respond_and_continue', + autorespond_requests='respond_and_discard', + autoresponse_grace_period='45d', + autoresponse_owner_text='the owner', + autoresponse_postings_text='the mailing list', + autoresponse_request_text='the robot', + display_name='Fnords', + description='This is my mailing list', + include_rfc2369_headers=False, + allow_list_posts=False, + digest_size_threshold=10.5, + posting_pipeline='virgin', + filter_content=True, + first_strip_reply_to=True, + convert_html_to_plaintext=True, + collapse_alternatives=False, + reply_goes_to_list='point_to_list', + reply_to_address='bee@example.com', + send_welcome_message=False, + subject_prefix='[ant]', + welcome_message_uri='mailman:///welcome.txt', + default_member_action='hold', + default_nonmember_action='discard', + ), + 'PUT') + self.assertEqual(response.status, 204) + self.assertEqual(self._mlist.display_name, 'Fnords') + # All three acceptable aliases were set. + self.assertEqual(set(IAcceptableAliasSet(self._mlist).aliases), + set(aliases)) diff --git a/src/mailman/rest/tests/test_domains.py b/src/mailman/rest/tests/test_domains.py index ec08b749d..44cf11ef3 100644 --- a/src/mailman/rest/tests/test_domains.py +++ b/src/mailman/rest/tests/test_domains.py @@ -66,3 +66,17 @@ class TestDomains(unittest.TestCase): 'http://localhost:9001/3.0/domains/example.com', method='DELETE') self.assertEqual(response.status, 204) self.assertEqual(getUtility(IListManager).get('ant@example.com'), None) + + def test_missing_domain(self): + # You get a 404 if you try to access a nonexisting domain. + with self.assertRaises(HTTPError) as cm: + call_api('http://localhost:9001/3.0/domains/does-not-exist.com') + self.assertEqual(cm.exception.code, 404) + + def test_missing_domain_lists(self): + # You get a 404 if you try to access the mailing lists of a + # nonexisting domain. + with self.assertRaises(HTTPError) as cm: + call_api( + 'http://localhost:9001/3.0/domains/does-not-exist.com/lists') + self.assertEqual(cm.exception.code, 404) diff --git a/src/mailman/rest/tests/test_lists.py b/src/mailman/rest/tests/test_lists.py index f4cafa3f3..ba6f6ea59 100644 --- a/src/mailman/rest/tests/test_lists.py +++ b/src/mailman/rest/tests/test_lists.py @@ -22,6 +22,7 @@ from __future__ import absolute_import, print_function, unicode_literals __metaclass__ = type __all__ = [ 'TestListArchivers', + 'TestListPagination', 'TestLists', 'TestListsMissing', ] @@ -161,6 +162,24 @@ class TestLists(unittest.TestCase): method='DELETE') self.assertEqual(cm.exception.code, 404) + def test_roster(self): + # Lists have rosters which can be accessed by role. + with transaction(): + anne = self._usermanager.create_address('anne@example.com') + bart = self._usermanager.create_address('bart@example.com') + self._mlist.subscribe(anne) + self._mlist.subscribe(bart) + resource, response = call_api( + 'http://localhost:9001/3.0/lists/test@example.com/roster/member') + self.assertEqual(resource['start'], 0) + self.assertEqual(resource['total_size'], 2) + member = resource['entries'][0] + self.assertEqual(member['email'], 'anne@example.com') + self.assertEqual(member['role'], 'member') + member = resource['entries'][1] + self.assertEqual(member['email'], 'bart@example.com') + self.assertEqual(member['role'], 'member') + class TestListArchivers(unittest.TestCase): @@ -228,3 +247,84 @@ class TestListArchivers(unittest.TestCase): method='PATCH') self.assertEqual(cm.exception.code, 400) self.assertEqual(cm.exception.reason, 'Invalid boolean value: sure') + + + +class TestListPagination(unittest.TestCase): + """Test mailing list pagination functionality. + + We create a bunch of mailing lists within a domain. When we want to + get all the lists in that domain via the REST API, we need to + paginate over them, otherwise there could be too many for display. + """ + + layer = RESTLayer + + def setUp(self): + with transaction(): + # Create a bunch of mailing lists in the example.com domain. + create_list('ant@example.com') + create_list('bee@example.com') + create_list('cat@example.com') + create_list('dog@example.com') + create_list('emu@example.com') + create_list('fly@example.com') + + def test_first_page(self): + resource, response = call_api( + 'http://localhost:9001/3.0/domains/example.com/lists' + '?count=1&page=1') + # There are 6 total lists, but only the first one in the page. + self.assertEqual(resource['total_size'], 1) + self.assertEqual(resource['start'], 0) + self.assertEqual(len(resource['entries']), 1) + entry = resource['entries'][0] + self.assertEqual(entry['fqdn_listname'], 'ant@example.com') + + def test_second_page(self): + resource, response = call_api( + 'http://localhost:9001/3.0/domains/example.com/lists' + '?count=1&page=2') + # There are 6 total lists, but only the first one in the page. + self.assertEqual(resource['total_size'], 1) + self.assertEqual(resource['start'], 0) + self.assertEqual(len(resource['entries']), 1) + entry = resource['entries'][0] + self.assertEqual(entry['fqdn_listname'], 'bee@example.com') + + def test_last_page(self): + resource, response = call_api( + 'http://localhost:9001/3.0/domains/example.com/lists' + '?count=1&page=6') + # There are 6 total lists, but only the first one in the page. + self.assertEqual(resource['total_size'], 1) + self.assertEqual(resource['start'], 0) + self.assertEqual(len(resource['entries']), 1) + entry = resource['entries'][0] + self.assertEqual(entry['fqdn_listname'], 'fly@example.com') + + def test_zeroth_page(self): + # Page numbers start at one. + with self.assertRaises(HTTPError) as cm: + resource, response = call_api( + 'http://localhost:9001/3.0/domains/example.com/lists' + '?count=1&page=0') + self.assertEqual(cm.exception.code, 400) + + def test_negative_page(self): + # Negative pages are not allowed. + with self.assertRaises(HTTPError) as cm: + resource, response = call_api( + 'http://localhost:9001/3.0/domains/example.com/lists' + '?count=1&page=-1') + self.assertEqual(cm.exception.code, 400) + + def test_past_last_page(self): + # The 7th page doesn't exist so the collection is empty. + resource, response = call_api( + 'http://localhost:9001/3.0/domains/example.com/lists' + '?count=1&page=7') + # There are 6 total lists, but only the first one in the page. + self.assertEqual(resource['total_size'], 0) + self.assertEqual(resource['start'], 0) + self.assertNotIn('entries', resource) diff --git a/src/mailman/rest/tests/test_membership.py b/src/mailman/rest/tests/test_membership.py index 503d68a94..3c7d0520b 100644 --- a/src/mailman/rest/tests/test_membership.py +++ b/src/mailman/rest/tests/test_membership.py @@ -69,14 +69,12 @@ class TestMembership(unittest.TestCase): '/member/nobody@example.com', method='DELETE') self.assertEqual(cm.exception.code, 404) - self.assertEqual(cm.exception.msg, '404 Not Found') def test_try_to_leave_list_with_bogus_address(self): # Try to leave a mailing list using an invalid membership address. with self.assertRaises(HTTPError) as cm: call_api('http://localhost:9001/3.0/members/1', method='DELETE') self.assertEqual(cm.exception.code, 404) - self.assertEqual(cm.exception.msg, '404 Not Found') def test_try_to_leave_a_list_twice(self): with transaction(): @@ -91,7 +89,6 @@ class TestMembership(unittest.TestCase): with self.assertRaises(HTTPError) as cm: call_api(url, method='DELETE') self.assertEqual(cm.exception.code, 404) - self.assertEqual(cm.exception.msg, '404 Not Found') def test_try_to_join_a_list_twice(self): with transaction(): diff --git a/src/mailman/rest/tests/test_paginate.py b/src/mailman/rest/tests/test_paginate.py index 0774125bb..e267100c7 100644 --- a/src/mailman/rest/tests/test_paginate.py +++ b/src/mailman/rest/tests/test_paginate.py @@ -27,6 +27,7 @@ __all__ = [ import unittest +from falcon import HTTPInvalidParam, Request from mailman.app.lifecycle import create_list from mailman.database.transaction import transaction from mailman.rest.helpers import paginate @@ -34,15 +35,13 @@ from mailman.testing.layers import RESTLayer -class _FakeRequest: - """Fake restish.http.Request object.""" - +class _FakeRequest(Request): def __init__(self, count=None, page=None): - self.GET = {} + self._params = {} if count is not None: - self.GET['count'] = count + self._params['count'] = count if page is not None: - self.GET['page'] = page + self._params['page'] = page @@ -105,41 +104,29 @@ class TestPaginateHelper(unittest.TestCase): @paginate def get_collection(self, request): return [] - response = get_collection(None, _FakeRequest('two', 1)) - self.assertEqual(response.status, '400 Bad Request') - - def test_no_get_attr_returns_bad_request(self): - # ?count=two&page=2 are not valid values so a bad request is returned. - @paginate - def get_collection(self, request): - return [] - request = _FakeRequest() - del request.GET - # The request object has no GET attribute. - self.assertIsNone(getattr(request, 'GET', None)) - response = get_collection(None, request) - self.assertEqual(response.status, '400 Bad Request') + self.assertRaises(HTTPInvalidParam, get_collection, + None, _FakeRequest('two', 1)) def test_negative_count(self): # ?count=-1&page=1 @paginate def get_collection(self, request): return ['one', 'two', 'three', 'four', 'five'] - response = get_collection(None, _FakeRequest(-1, 1)) - self.assertEqual(response.status, '400 Bad Request') + self.assertRaises(HTTPInvalidParam, get_collection, + None, _FakeRequest(-1, 1)) def test_negative_page(self): # ?count=1&page=-1 @paginate def get_collection(self, request): return ['one', 'two', 'three', 'four', 'five'] - response = get_collection(None, _FakeRequest(1, -1)) - self.assertEqual(response.status, '400 Bad Request') + self.assertRaises(HTTPInvalidParam, get_collection, + None, _FakeRequest(1, -1)) def test_negative_page_and_count(self): # ?count=1&page=-1 @paginate def get_collection(self, request): return ['one', 'two', 'three', 'four', 'five'] - response = get_collection(None, _FakeRequest(-1, -1)) - self.assertEqual(response.status, '400 Bad Request') + self.assertRaises(HTTPInvalidParam, get_collection, + None, _FakeRequest(-1, -1)) diff --git a/src/mailman/rest/tests/test_preferences.py b/src/mailman/rest/tests/test_preferences.py new file mode 100644 index 000000000..91a066cff --- /dev/null +++ b/src/mailman/rest/tests/test_preferences.py @@ -0,0 +1,67 @@ +# Copyright (C) 2014 by the Free Software Foundation, Inc. +# +# This file is part of GNU Mailman. +# +# GNU Mailman is free software: you can redistribute it and/or modify it under +# the terms of the GNU General Public License as published by the Free +# Software Foundation, either version 3 of the License, or (at your option) +# any later version. +# +# GNU Mailman is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# 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 various preference functionality.""" + +from __future__ import absolute_import, print_function, unicode_literals + +__metaclass__ = type +__all__ = [ + 'TestPreferences', + ] + + +import unittest + +from mailman.app.lifecycle import create_list +from mailman.database.transaction import transaction +from mailman.interfaces.usermanager import IUserManager +from mailman.testing.helpers import call_api +from mailman.testing.layers import RESTLayer +from urllib2 import HTTPError +from zope.component import getUtility + + +class TestPreferences(unittest.TestCase): + """Test various preference functionality.""" + + layer = RESTLayer + + def setUp(self): + user_manager = getUtility(IUserManager) + with transaction(): + self._mlist = create_list('test@example.com') + anne = user_manager.create_address('anne@example.com') + self._member = self._mlist.subscribe(anne) + + def test_read_only_member_all_preferences(self): + # A member's combined preferences are read-only. + with self.assertRaises(HTTPError) as cm: + call_api('http://localhost:9001/3.0/members/1/all/preferences', { + 'delivery_status': 'enabled', + }, method='PATCH') + # The resource at this endpoint doesn't even have a PATCH method. + self.assertEqual(cm.exception.code, 405) + + def test_read_only_system_preferences(self): + # The system preferences are read-only. + with self.assertRaises(HTTPError) as cm: + call_api('http://localhost:9001/3.0/system/preferences', { + 'delivery_status': 'enabled', + }, method='PATCH') + # The resource at this endpoint doesn't even have a PATCH method. + self.assertEqual(cm.exception.code, 405) diff --git a/src/mailman/rest/tests/test_root.py b/src/mailman/rest/tests/test_root.py index 42ea2f6f9..d4d25ede0 100644 --- a/src/mailman/rest/tests/test_root.py +++ b/src/mailman/rest/tests/test_root.py @@ -21,24 +21,41 @@ from __future__ import absolute_import, print_function, unicode_literals __metaclass__ = type __all__ = [ - 'TestSystem', + 'TestRoot', ] -import unittest import os +import json +import unittest -from urllib2 import HTTPError - +from base64 import b64encode +from httplib2 import Http from mailman.config import config +from mailman.core.system import system from mailman.testing.helpers import call_api from mailman.testing.layers import RESTLayer +from urllib2 import HTTPError -class TestSystem(unittest.TestCase): +class TestRoot(unittest.TestCase): layer = RESTLayer + def test_root_system(self): + # You can get the system preferences via the root path. + url = 'http://localhost:9001/3.0/system' + json, response = call_api(url) + self.assertEqual(json['mailman_version'], system.mailman_version) + self.assertEqual(json['python_version'], system.python_version) + self.assertEqual(json['self_link'], url) + + def test_path_under_root_does_not_exist(self): + # Accessing a non-existent path under root returns a 404. + with self.assertRaises(HTTPError) as cm: + call_api('http://localhost:9001/3.0/does-not-exist') + self.assertEqual(cm.exception.code, 404) + def test_system_url_too_long(self): # /system/foo/bar is not allowed. with self.assertRaises(HTTPError) as cm: @@ -49,7 +66,7 @@ class TestSystem(unittest.TestCase): # /system/foo where `foo` is not `preferences`. with self.assertRaises(HTTPError) as cm: call_api('http://localhost:9001/3.0/system/foo') - self.assertEqual(cm.exception.code, 400) + self.assertEqual(cm.exception.code, 404) def test_system_preferences_are_read_only(self): # /system/preferences are read-only. @@ -76,3 +93,31 @@ class TestSystem(unittest.TestCase): # directory in var/queue. queue_directory = os.path.join(config.QUEUE_DIR, 'rest') self.assertFalse(os.path.isdir(queue_directory)) + + def test_no_basic_auth(self): + # If Basic Auth credentials are missing, it is a 401 error. + url = 'http://localhost:9001/3.0/system' + headers = { + 'Content-Type': 'application/x-www-form-urlencode', + } + response, raw_content = Http().request(url, 'GET', None, headers) + self.assertEqual(response.status, 401) + content = json.loads(raw_content) + self.assertEqual(content['title'], '401 Unauthorized') + self.assertEqual(content['description'], + 'The REST API requires authentication') + + def test_unauthorized(self): + # Bad Basic Auth credentials results in a 401 error. + auth = b64encode('baduser:badpass') + url = 'http://localhost:9001/3.0/system' + headers = { + 'Content-Type': 'application/x-www-form-urlencode', + 'Authorization': 'Basic ' + auth, + } + response, raw_content = Http().request(url, 'GET', None, headers) + self.assertEqual(response.status, 401) + content = json.loads(raw_content) + self.assertEqual(content['title'], '401 Unauthorized') + self.assertEqual(content['description'], + 'User is not authorized for the REST API') diff --git a/src/mailman/rest/users.py b/src/mailman/rest/users.py index bf9668564..798fe699d 100644 --- a/src/mailman/rest/users.py +++ b/src/mailman/rest/users.py @@ -27,7 +27,6 @@ __all__ = [ from passlib.utils import generate_password as generate -from restish import http, resource from uuid import UUID from zope.component import getUtility @@ -38,7 +37,8 @@ from mailman.interfaces.address import ExistingAddressError from mailman.interfaces.usermanager import IUserManager from mailman.rest.addresses import UserAddresses from mailman.rest.helpers import ( - CollectionMixin, GetterSetter, PATCH, etag, no_content, paginate, path_to) + BadRequest, CollectionMixin, GetterSetter, NotFound, bad_request, child, + created, etag, forbidden, no_content, not_found, okay, paginate, path_to) from mailman.rest.preferences import Preferences from mailman.rest.validator import PatchValidator, Validator @@ -63,7 +63,7 @@ ATTRIBUTES = dict( -class _UserBase(resource.Resource, CollectionMixin): +class _UserBase(CollectionMixin): """Shared base class for user representations.""" def _resource_as_dict(self, user): @@ -96,14 +96,12 @@ class _UserBase(resource.Resource, CollectionMixin): class AllUsers(_UserBase): """The users.""" - @resource.GET() - def collection(self, request): + def on_get(self, request, response): """/users""" resource = self._make_collection(request) - return http.ok([], etag(resource)) + okay(response, etag(resource)) - @resource.POST() - def create(self, request): + def on_post(self, request, response): """Create a new user.""" try: validator = Validator(email=unicode, @@ -112,7 +110,8 @@ class AllUsers(_UserBase): _optional=('display_name', 'password')) arguments = validator(request) except ValueError as error: - return http.bad_request([], str(error)) + bad_request(response, str(error)) + return # 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 after the fact if successful. @@ -120,14 +119,15 @@ class AllUsers(_UserBase): try: user = getUtility(IUserManager).create_user(**arguments) except ExistingAddressError as error: - return http.bad_request( - [], b'Address already exists: {0}'.format(error.address)) + bad_request( + response, b'Address already exists: {0}'.format(error.address)) + return if password is None: # This will have to be reset since it cannot be retrieved. password = generate(int(config.passwords.password_length)) user.password = config.password_context.encrypt(password) location = path_to('users/{0}'.format(user.user_id.int)) - return http.created(location, [], None) + created(response, location) @@ -157,84 +157,98 @@ class AUser(_UserBase): else: self._user = user_manager.get_user_by_id(user_id) - @resource.GET() - def user(self, request): + def on_get(self, request, response): """Return a single user end-point.""" if self._user is None: - return http.not_found() - return http.ok([], self._resource_as_json(self._user)) + not_found(response) + else: + okay(response, self._resource_as_json(self._user)) - @resource.child() + @child() def addresses(self, request, segments): """/users/<uid>/addresses""" if self._user is None: - return http.not_found() + return NotFound(), [] return UserAddresses(self._user) - @resource.DELETE() - def delete_user(self, request): + def on_delete(self, request, response): """Delete the named user, all her memberships, and addresses.""" if self._user is None: - return http.not_found() + not_found(response) + return for member in self._user.memberships.members: member.unsubscribe() user_manager = getUtility(IUserManager) for address in self._user.addresses: user_manager.delete_address(address) user_manager.delete_user(self._user) - return no_content() + no_content(response) - @resource.child() + @child() def preferences(self, request, segments): """/addresses/<email>/preferences""" if len(segments) != 0: - return http.bad_request() + return BadRequest(), [] if self._user is None: - return http.not_found() + return NotFound(), [] child = Preferences( self._user.preferences, 'users/{0}'.format(self._user.user_id.int)) return child, [] - @PATCH() - def patch_update(self, request): + def on_patch(self, request, response): """Patch the user's configuration (i.e. partial update).""" if self._user is None: - return http.not_found() + not_found(response) + return try: validator = PatchValidator(request, ATTRIBUTES) except UnknownPATCHRequestError as error: - return http.bad_request( - [], b'Unknown attribute: {0}'.format(error.attribute)) + bad_request( + response, b'Unknown attribute: {0}'.format(error.attribute)) except ReadOnlyPATCHRequestError as error: - return http.bad_request( - [], b'Read-only attribute: {0}'.format(error.attribute)) - validator.update(self._user, request) - return no_content() + bad_request( + response, b'Read-only attribute: {0}'.format(error.attribute)) + else: + validator.update(self._user, request) + no_content(response) - @resource.PUT() - def put_update(self, request): + def on_put(self, request, response): """Put the user's configuration (i.e. full update).""" if self._user is None: - return http.not_found() + not_found(response) + return validator = Validator(**ATTRIBUTES) try: validator.update(self._user, request) except UnknownPATCHRequestError as error: - return http.bad_request( - [], b'Unknown attribute: {0}'.format(error.attribute)) + bad_request( + response, b'Unknown attribute: {0}'.format(error.attribute)) except ReadOnlyPATCHRequestError as error: - return http.bad_request( - [], b'Read-only attribute: {0}'.format(error.attribute)) + bad_request( + response, b'Read-only attribute: {0}'.format(error.attribute)) except ValueError as error: - return http.bad_request([], str(error)) - return no_content() + bad_request(response, str(error)) + else: + no_content(response) - @resource.child('login') + @child() def login(self, request, segments): """Log the user in, sort of, by verifying a given password.""" if self._user is None: - return http.not_found() + return NotFound(), [] + return Login(self._user) + + + +class Login: + """<api>/users/<uid>/login""" + + def __init__(self, user): + assert user is not None + self._user = user + + def on_post(self, request, response): # We do not want to encrypt the plaintext password given in the POST # data. That would hash the password, but we need to have the # plaintext in order to pass into passlib. @@ -242,11 +256,13 @@ class AUser(_UserBase): try: values = validator(request) except ValueError as error: - return http.bad_request([], str(error)) + bad_request(response, str(error)) + return is_valid, new_hash = config.password_context.verify( values['cleartext_password'], self._user.password) if is_valid: if new_hash is not None: self._user.password = new_hash - return no_content() - return http.forbidden() + no_content(response) + else: + forbidden(response) diff --git a/src/mailman/rest/validator.py b/src/mailman/rest/validator.py index 8fe1a6078..cbcc5f652 100644 --- a/src/mailman/rest/validator.py +++ b/src/mailman/rest/validator.py @@ -62,7 +62,7 @@ def subscriber_validator(subscriber): try: return UUID(int=int(subscriber)) except ValueError: - return subscriber + return unicode(subscriber) def language_validator(code): @@ -90,11 +90,7 @@ class Validator: # in the pre-converted dictionary. All keys which show up more than # once get a list value. missing = object() - # This is a gross hack to allow PATCH. See helpers.py for details. - try: - items = request.PATCH.items() - except AttributeError: - items = request.POST.items() + items = request.params.items() for key, new_value in items: old_value = form_data.get(key, missing) if old_value is missing: @@ -166,7 +162,7 @@ class PatchValidator(Validator): that is defined as read-only. """ validationators = {} - for attribute in request.PATCH: + for attribute in request.params: if attribute not in converters: raise UnknownPATCHRequestError(attribute) if converters[attribute].decoder is None: diff --git a/src/mailman/rest/wsgiapp.py b/src/mailman/rest/wsgiapp.py index b7ad3d698..698c4269d 100644 --- a/src/mailman/rest/wsgiapp.py +++ b/src/mailman/rest/wsgiapp.py @@ -26,18 +26,22 @@ __all__ = [ ] +import re import logging -from restish.app import RestishApp -from wsgiref.simple_server import WSGIRequestHandler -from wsgiref.simple_server import make_server as wsgi_server - +from falcon import API +from falcon.responders import path_not_found +from falcon.routing import create_http_method_map from mailman.config import config from mailman.database.transaction import transactional from mailman.rest.root import Root +from wsgiref.simple_server import WSGIRequestHandler +from wsgiref.simple_server import make_server as wsgi_server log = logging.getLogger('mailman.http') +_missing = object() +SLASH = '/' @@ -49,15 +53,96 @@ class AdminWebServiceWSGIRequestHandler(WSGIRequestHandler): log.info('%s - - %s', self.address_string(), format % args) -class AdminWebServiceApplication(RestishApp): - """Connect the restish WSGI application to Mailman's database.""" +class RootedAPI(API): + def __init__(self, root, *args, **kws): + self._root = root + super(RootedAPI, self).__init__(*args, **kws) @transactional def __call__(self, environ, start_response): """See `RestishApp`.""" - return super(AdminWebServiceApplication, self).__call__( + return super(RootedAPI, self).__call__( environ, start_response) + def _get_responder(self, req): + path = req.path + method = req.method + path_segments = path.split('/') + # 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) + resource = self._root + 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: + continue + result = None + if isinstance(matcher, basestring): + # Is the matcher string a regular expression or plain + # string? If it starts with a caret, it's a regexp. + 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) + mo = cre.match(remaining_path) + if mo: + result = attribute( + req, path_segments, **mo.groupdict()) + elif matcher == this_segment: + result = attribute(req, path_segments) + else: + # The matcher is a callable. It returns None if it + # doesn't match, and if it does, it returns a 3-tuple + # containing the positional arguments, the keyword + # arguments, and the remaining segments. The attribute is + # 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) + if matcher_result is not None: + positional, keyword, path_segments = matcher_result + result = attribute( + req, path_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. + if result is None: + continue + elif isinstance(result, tuple): + resource, path_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: + # We're at the end of the path, so the root must be the + # responder. + method_map = create_http_method_map( + resource, None, None, None) + responder = method_map[method] + return responder, {}, resource + this_segment = path_segments.pop(0) + break + else: + # None of the attributes matched this path component, so the + # response is a 404. + return path_not_found, {}, None + def make_application(): @@ -66,7 +151,7 @@ def make_application(): Use this if you want to integrate Mailman's REST server with your own WSGI server. """ - return AdminWebServiceApplication(Root()) + return RootedAPI(Root()) def make_server(): diff --git a/src/mailman/testing/helpers.py b/src/mailman/testing/helpers.py index 1ef9e964e..aab23ab75 100644 --- a/src/mailman/testing/helpers.py +++ b/src/mailman/testing/helpers.py @@ -313,8 +313,9 @@ def call_api(url, data=None, method=None, username=None, password=None): :param password: The HTTP Basic Auth password. None means use the value from the configuration. :type username: str - :return: The response object and the JSON decoded content, if there is - any. If not, the second tuple item will be None. + :return: A 2-tuple containing the JSON decoded content (if there is any, + else None) and the response object. + :rtype: 2-tuple of (dict, response) :raises HTTPError: when a non-2xx return code is received. """ headers = {} |