* You can now PUT and PATCH on user resources to change the user's display

   name or password.  For passwords, you pass in the clear text password and
   Mailman will hash it before storing.

Also:

 * Major refactoring of validators for PUT and PATCH.  Pull the common logic
   out of configuration.py and put it in a PatchValidator class in
   helpers.py.  Also move GetterSetter to helpers.py
 * Add new exception classes RESTError, UnknownPATCHRequestError,
   ReadOnlyPATCHRequestError.  These are used in the PatchValidator.
 * Added Validator.update() which works nicely for PATCH and PUT.

9 files changed, 320 insertions, 110 deletions

diff --git a/src/mailman/core/errors.py b/src/mailman/core/errors.py
index 529ac86fe..95a318ca6 100644
--- a/src/mailman/core/errors.py
+++ b/src/mailman/core/errors.py
@@ -43,7 +43,10 @@ __all__ = [
     'MemberError',
     'MustDigestError',
     'PasswordError',
+    'RESTError',
+    'ReadOnlyPATCHRequestError',
     'RejectMessage',
+    'UnknownPATCHRequestError',
     ]
 
 
@@ -126,3 +129,22 @@ class BadPasswordSchemeError(PasswordError):
 
     def __str__(self):
         return 'A bad password scheme was given: %s' % self.scheme_name
+
+
+
+class RESTError(MailmanError):
+    """Base class for REST API errors."""
+
+
+class UnknownPATCHRequestError(RESTError):
+    """A PATCH request contained an unknown attribute."""
+
+    def __init__(self, attribute):
+        self.attribute = attribute
+
+
+class ReadOnlyPATCHRequestError(RESTError):
+    """A PATCH request contained a read-only attribute."""
+
+    def __init__(self, attribute):
+        self.attribute = attribute
diff --git a/src/mailman/docs/NEWS.rst b/src/mailman/docs/NEWS.rst
index 984e0d9fb..50eeb0fda 100644
--- a/src/mailman/docs/NEWS.rst
+++ b/src/mailman/docs/NEWS.rst
@@ -20,6 +20,9 @@ REST
    the URL with the list-id.  To reference a mailing list, the list-id url is
    preferred, but for backward compatibility, the posting address is still
    accepted.
+ * You can now PUT and PATCH on user resources to change the user's display
+   name or password.  For passwords, you pass in the clear text password and
+   Mailman will hash it before storing.
 
 
 3.0 beta 2 -- "Freeze"
diff --git a/src/mailman/rest/configuration.py b/src/mailman/rest/configuration.py
index 83d4c74f6..8db23136a 100644
--- a/src/mailman/rest/configuration.py
+++ b/src/mailman/rest/configuration.py
@@ -29,78 +29,17 @@ 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)
 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 PATCH, etag, no_content
-from mailman.rest.validator import Validator, enum_validator
+from mailman.rest.helpers import GetterSetter, PATCH, etag, no_content
+from mailman.rest.validator import PatchValidator, Validator, enum_validator
 
 
 
-class GetterSetter:
-    """Get and set attributes on mailing lists.
-
-    Most attributes are fairly simple - a getattr() or setattr() on the
-    mailing list does the trick, with the appropriate encoding or decoding on
-    the way in and out.  Encoding doesn't happen here though; the standard
-    JSON library handles most types, but see ExtendedEncoder in
-    mailman.rest.helpers for additional support.
-
-    Others are more complicated since they aren't kept in the model as direct
-    columns in the database.  These will use subclasses of this base class.
-    Read-only attributes will have a decoder which always raises ValueError.
-    """
-
-    def __init__(self, decoder=None):
-        """Create a getter/setter for a specific list attribute.
-
-        :param decoder: The callable for decoding a web request value string
-            into the specific data type needed by the `IMailingList`
-            attribute.  Use None to indicate a read-only attribute.  The
-            callable should raise ValueError when the web request value cannot
-            be converted.
-        :type decoder: callable
-        """
-        self.decoder = decoder
-
-    def get(self, mlist, attribute):
-        """Return the named mailing list attribute value.
-
-        :param mlist: The mailing list.
-        :type mlist: `IMailingList`
-        :param attribute: The attribute name.
-        :type attribute: string
-        :return: The attribute value, ready for JSON encoding.
-        :rtype: object
-        """
-        return getattr(mlist, attribute)
-
-    def put(self, mlist, attribute, value):
-        """Set the named mailing list attribute value.
-
-        :param mlist: The mailing list.
-        :type mlist: `IMailingList`
-        :param attribute: The attribute name.
-        :type attribute: string
-        :param value: The new value for the attribute.
-        :type request_value: object
-        """
-        setattr(mlist, attribute, value)
-
-    def __call__(self, value):
-        """Convert the value to its internal format.
-
-        :param value: The web request value to convert.
-        :type value: string
-        :return: The converted value.
-        :rtype: object
-        """
-        if self.decoder is None:
-            return value
-        return self.decoder(value)
-
-
 class AcceptableAliases(GetterSetter):
     """Resource for the acceptable aliases of a mailing list."""
 
@@ -239,19 +178,6 @@ class ListConfiguration(resource.Resource):
             resource[attribute] = value
         return http.ok([], etag(resource))
 
-    # XXX 2010-09-01 barry: Refactor {put,patch}_configuration() for common
-    # code paths.
-
-    def _set_writable_attributes(self, validator, request):
-        """Common code for setting all attributes given in the request.
-
-        Returns an HTTP 400 when a request tries to write to a read-only
-        attribute.
-        """
-        converted = validator(request)
-        for key, value in converted.items():
-            ATTRIBUTES[key].put(self._mlist, key, value)
-
     @resource.PUT()
     def put_configuration(self, request):
         """Set a mailing list configuration."""
@@ -259,7 +185,7 @@ class ListConfiguration(resource.Resource):
         if attribute is None:
             validator = Validator(**VALIDATORS)
             try:
-                self._set_writable_attributes(validator, request)
+                validator.update(self._mlist, request)
             except ValueError as error:
                 return http.bad_request([], str(error))
         elif attribute not in ATTRIBUTES:
@@ -271,7 +197,7 @@ class ListConfiguration(resource.Resource):
         else:
             validator = Validator(**{attribute: VALIDATORS[attribute]})
             try:
-                self._set_writable_attributes(validator, request)
+                validator.update(self._mlist, request)
             except ValueError as error:
                 return http.bad_request([], str(error))
         return no_content()
@@ -279,20 +205,16 @@ class ListConfiguration(resource.Resource):
     @PATCH()
     def patch_configuration(self, request):
         """Patch the configuration (i.e. partial update)."""
-        # Validate only the partial subset of attributes given in the request.
-        validationators = {}
-        for attribute in request.PATCH:
-            if attribute not in ATTRIBUTES:
-                return http.bad_request(
-                    [], b'Unknown attribute: {0}'.format(attribute))
-            elif ATTRIBUTES[attribute].decoder is None:
-                return http.bad_request(
-                    [], b'Read-only attribute: {0}'.format(attribute))
-            else:
-                validationators[attribute] = VALIDATORS[attribute]
-        validator = Validator(**validationators)
         try:
-            self._set_writable_attributes(validator, request)
+            validator = PatchValidator(request, ATTRIBUTES)
+        except UnknownPATCHRequestError as error:
+            return http.bad_request(
+                [], b'Unknown attribute: {0}'.format(error.attribute))
+        except ReadOnlyPATCHRequestError as error:
+            return http.bad_request(
+                [], b'Read-only attribute: {0}'.format(error.attribute))
+        try:
+            validator.update(self._mlist, request)
         except ValueError as error:
             return http.bad_request([], str(error))
         return no_content()
diff --git a/src/mailman/rest/docs/domains.rst b/src/mailman/rest/docs/domains.rst
index c890af7fa..92c73ffbf 100644
--- a/src/mailman/rest/docs/domains.rst
+++ b/src/mailman/rest/docs/domains.rst
@@ -131,7 +131,7 @@ example.com domain does not contain any mailing lists.
     ...           })
     content-length: 0
     date: ...
-    location: http://localhost:9001/3.0/lists/test-domains@example.com
+    location: http://localhost:9001/3.0/lists/test-domains.example.com
     ...
 
     >>> dump_json('http://localhost:9001/3.0/domains/example.com/lists')
@@ -141,7 +141,7 @@ example.com domain does not contain any mailing lists.
         http_etag: "..."
         ...
         member_count: 0
-        self_link: http://localhost:9001/3.0/lists/test-domains@example.com
+        self_link: http://localhost:9001/3.0/lists/test-domains.example.com
         volume: 1
     http_etag: "..."
     start: 0
diff --git a/src/mailman/rest/docs/users.rst b/src/mailman/rest/docs/users.rst
index cdede10ee..888bc43fd 100644
--- a/src/mailman/rest/docs/users.rst
+++ b/src/mailman/rest/docs/users.rst
@@ -40,7 +40,7 @@ The user ids match.
     >>> json['entries'][0]['user_id'] == anne.user_id.int
     True
 
-A user might not have a real name, in which case, the attribute will not be
+A user might not have a display name, in which case, the attribute will not be
 returned in the REST API.
 
     >>> dave = user_manager.create_user('dave@example.com')
@@ -66,7 +66,8 @@ Creating users via the API
 ==========================
 
 New users can be created through the REST API.  To do so requires the initial
-email address for the user, a password, and optionally the user's full name.
+email address for the user, a password, and optionally the user's display
+name.
 ::
 
     >>> transaction.abort()
@@ -134,6 +135,75 @@ therefore cannot be retrieved.  It can be reset though.
     user_id: 4
 
 
+Updating users
+==============
+
+Users have a password and a display name.  The display name can be changed
+through the REST API.
+
+    >>> dump_json('http://localhost:9001/3.0/users/4', {
+    ...           'display_name': 'Chrissy Person',
+    ...           }, method='PATCH')
+    content-length: 0
+    date: ...
+    server: ...
+    status: 204
+
+Cris's display name has been updated.
+
+    >>> dump_json('http://localhost:9001/3.0/users/4')
+    created_on: 2005-08-01T07:49:23
+    display_name: Chrissy Person
+    http_etag: "..."
+    password: {plaintext}...
+    self_link: http://localhost:9001/3.0/users/4
+    user_id: 4
+
+You can also change the user's password by passing in the new clear text
+password.  Mailman will hash this before it is stored internally.
+
+    >>> dump_json('http://localhost:9001/3.0/users/4', {
+    ...           'cleartext_password': 'clockwork angels',
+    ...           }, method='PATCH')
+    content-length: 0
+    date: ...
+    server: ...
+    status: 204
+
+Even though you see *{plaintext}clockwork angels* below, it has still been
+hashed before storage.  The default hashing algorithm for the test suite is a
+plain text hash, but you can see that it works by the addition of the
+algorithm prefix.
+
+    >>> dump_json('http://localhost:9001/3.0/users/4')
+    created_on: 2005-08-01T07:49:23
+    display_name: Chrissy Person
+    http_etag: "..."
+    password: {plaintext}clockwork angels
+    self_link: http://localhost:9001/3.0/users/4
+    user_id: 4
+
+You can change both the display name and the password by PUTing the full
+resource.
+
+    >>> dump_json('http://localhost:9001/3.0/users/4', {
+    ...           'display_name': 'Christopherson Person',
+    ...           'cleartext_password': 'the garden',
+    ...           }, method='PUT')
+    content-length: 0
+    date: ...
+    server: ...
+    status: 204
+
+    >>> dump_json('http://localhost:9001/3.0/users/4')
+    created_on: 2005-08-01T07:49:23
+    display_name: Christopherson Person
+    http_etag: "..."
+    password: {plaintext}the garden
+    self_link: http://localhost:9001/3.0/users/4
+    user_id: 4
+
+
 Deleting users via the API
 ==========================
 
@@ -145,11 +215,21 @@ Users can also be deleted via the API.
     date: ...
     server: ...
     status: 204
+
+Cris's resource cannot be retrieved either by email address...
+
     >>> dump_json('http://localhost:9001/3.0/users/cris@example.com')
     Traceback (most recent call last):
     ...
     HTTPError: HTTP Error 404: 404 Not Found
 
+...or user id.
+
+    >>> dump_json('http://localhost:9001/3.0/users/4')
+    Traceback (most recent call last):
+    ...
+    HTTPError: HTTP Error 404: 404 Not Found
+
 
 Missing users
 =============
@@ -171,6 +251,23 @@ user id...
     ...
     HTTPError: HTTP Error 404: 404 Not Found
 
+You also can't update a missing user.
+
+    >>> dump_json('http://localhost:9001/3.0/users/zed@example.org', {
+    ...           'display_name': 'Is Dead',
+    ...           }, method='PATCH')
+    Traceback (most recent call last):
+    ...
+    HTTPError: HTTP Error 404: 404 Not Found
+
+    >>> dump_json('http://localhost:9001/3.0/users/zed@example.org', {
+    ...           'display_name': 'Is Dead',
+    ...           'cleartext_password': 'vroom',
+    ...           }, method='PUT')
+    Traceback (most recent call last):
+    ...
+    HTTPError: HTTP Error 404: 404 Not Found
+
 
 User addresses
 ==============
diff --git a/src/mailman/rest/helpers.py b/src/mailman/rest/helpers.py
index 2824a894e..72040c848 100644
--- a/src/mailman/rest/helpers.py
+++ b/src/mailman/rest/helpers.py
@@ -21,6 +21,7 @@ from __future__ import absolute_import, unicode_literals
 
 __metaclass__ = type
 __all__ = [
+    'GetterSetter',
     'PATCH',
     'etag',
     'no_content',
@@ -205,3 +206,63 @@ class PATCH(MethodDecorator):
     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.
+
+    Most attributes are fairly simple - a getattr() or setattr() on the object
+    does the trick, with the appropriate encoding or decoding on the way in
+    and out.  Encoding doesn't happen here though; the standard JSON library
+    handles most types, but see ExtendedEncoder for additional support.
+
+    Others are more complicated since they aren't kept in the model as direct
+    columns in the database.  These will use subclasses of this base class.
+    Read-only attributes will have a decoder which always raises ValueError.
+    """
+
+    def __init__(self, decoder=None):
+        """Create a getter/setter for a specific attribute.
+
+        :param decoder: The callable for decoding a web request value string
+            into the specific data type needed by the object's attribute.  Use
+            None to indicate a read-only attribute.  The callable should raise
+            ValueError when the web request value cannot be converted.
+        :type decoder: callable
+        """
+        self.decoder = decoder
+
+    def get(self, obj, attribute):
+        """Return the named object attribute value.
+
+        :param obj: The object to access.
+        :type obj: object
+        :param attribute: The attribute name.
+        :type attribute: string
+        :return: The attribute value, ready for JSON encoding.
+        :rtype: object
+        """
+        return getattr(obj, attribute)
+
+    def put(self, obj, attribute, value):
+        """Set the named object attribute value.
+
+        :param obj: The object to change.
+        :type obj: object
+        :param attribute: The attribute name.
+        :type attribute: string
+        :param value: The new value for the attribute.
+        """
+        setattr(obj, attribute, value)
+
+    def __call__(self, value):
+        """Convert the value to its internal format.
+
+        :param value: The web request value to convert.
+        :type value: string
+        :return: The converted value.
+        :rtype: object
+        """
+        if self.decoder is None:
+            return value
+        return self.decoder(value)
diff --git a/src/mailman/rest/preferences.py b/src/mailman/rest/preferences.py
index be598458e..1961c8b84 100644
--- a/src/mailman/rest/preferences.py
+++ b/src/mailman/rest/preferences.py
@@ -30,7 +30,8 @@ from lazr.config import as_boolean
 from restish import http, resource
 
 from mailman.interfaces.member import DeliveryMode, DeliveryStatus
-from mailman.rest.helpers import PATCH, etag, no_content, path_to
+from mailman.rest.helpers import (
+    GetterSetter, PATCH, etag, no_content, path_to)
 from mailman.rest.validator import (
     Validator, enum_validator, language_validator)
 
@@ -72,7 +73,7 @@ class ReadOnlyPreferences(resource.Resource):
         resource['self_link'] = path_to(
             '{0}/preferences'.format(self._base_url))
         return http.ok([], etag(resource))
-    
+
 
 
 class Preferences(ReadOnlyPreferences):
@@ -82,22 +83,20 @@ class Preferences(ReadOnlyPreferences):
         if self._parent is None:
             return http.not_found()
         kws = dict(
-            acknowledge_posts=as_boolean,
-            delivery_mode=enum_validator(DeliveryMode),
-            delivery_status=enum_validator(DeliveryStatus),
-            preferred_language=language_validator,
-            receive_list_copy=as_boolean,
-            receive_own_postings=as_boolean,
+            acknowledge_posts=GetterSetter(as_boolean),
+            delivery_mode=GetterSetter(enum_validator(DeliveryMode)),
+            delivery_status=GetterSetter(enum_validator(DeliveryStatus)),
+            preferred_language=GetterSetter(language_validator),
+            receive_list_copy=GetterSetter(as_boolean),
+            receive_own_postings=GetterSetter(as_boolean),
             )
         if is_optional:
             # For a PUT, all attributes are optional.
             kws['_optional'] = kws.keys()
         try:
-            values = Validator(**kws)(request)
+            Validator(**kws).update(self._parent, request)
         except ValueError as error:
             return http.bad_request([], str(error))
-        for key, value in values.items():
-            setattr(self._parent, key, value)
         return no_content()
 
     @PATCH()
diff --git a/src/mailman/rest/users.py b/src/mailman/rest/users.py
index bce541ae5..94817da49 100644
--- a/src/mailman/rest/users.py
+++ b/src/mailman/rest/users.py
@@ -32,12 +32,34 @@ from uuid import UUID
 from zope.component import getUtility
 
 from mailman.config import config
+from mailman.core.errors import (
+    ReadOnlyPATCHRequestError, UnknownPATCHRequestError)
 from mailman.interfaces.address import ExistingAddressError
 from mailman.interfaces.usermanager import IUserManager
 from mailman.rest.addresses import UserAddresses
-from mailman.rest.helpers import CollectionMixin, etag, no_content, path_to
+from mailman.rest.helpers import (
+    CollectionMixin, GetterSetter, PATCH, etag, no_content, path_to)
 from mailman.rest.preferences import Preferences
-from mailman.rest.validator import Validator
+from mailman.rest.validator import PatchValidator, Validator
+
+
+# Attributes of a user which can be changed via the REST API.
+class PasswordEncrypterGetterSetter(GetterSetter):
+    def __init__(self):
+        super(PasswordEncrypterGetterSetter, self).__init__(
+            config.password_context.encrypt)
+    def get(self, obj, attribute):
+        assert attribute == 'cleartext_password'
+        super(PasswordEncrypterGetterSetter, self).get(obj, 'password')
+    def put(self, obj, attribute, value):
+        assert attribute == 'cleartext_password'
+        super(PasswordEncrypterGetterSetter, self).put(obj, 'password', value)
+
+
+ATTRIBUTES = dict(
+    display_name=GetterSetter(unicode),
+    cleartext_password=PasswordEncrypterGetterSetter(),
+    )
 
 
 
@@ -165,3 +187,37 @@ class AUser(_UserBase):
             self._user.preferences,
             'users/{0}'.format(self._user.user_id.int))
         return child, []
+
+    @PATCH()
+    def patch_update(self, request):
+        """Patch the user's configuration (i.e. partial update)."""
+        if self._user is None:
+            return http.not_found()
+        try:
+            validator = PatchValidator(request, ATTRIBUTES)
+        except UnknownPATCHRequestError as error:
+            return http.bad_request(
+                [], 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()
+
+    @resource.PUT()
+    def put_update(self, request):
+        """Put the user's configuration (i.e. full update)."""
+        if self._user is None:
+            return http.not_found()
+        validator = Validator(**ATTRIBUTES)
+        try:
+            validator.update(self._user, request)
+        except UnknownPATCHRequestError as error:
+            return http.bad_request(
+                [], b'Unknown attribute: {0}'.format(error.attribute))
+        except ReadOnlyPATCHRequestError as error:
+            return http.bad_request(
+                [], b'Read-only attribute: {0}'.format(error.attribute))
+        except ValueError as error:
+            return http.bad_request([], str(error))
+        return no_content()
diff --git a/src/mailman/rest/validator.py b/src/mailman/rest/validator.py
index 7484aa260..f6f0cd7e6 100644
--- a/src/mailman/rest/validator.py
+++ b/src/mailman/rest/validator.py
@@ -21,6 +21,7 @@ from __future__ import absolute_import, unicode_literals
 
 __metaclass__ = type
 __all__ = [
+    'PatchValidator',
     'Validator',
     'enum_validator',
     'language_validator',
@@ -31,6 +32,8 @@ __all__ = [
 from uuid import UUID
 from zope.component import getUtility
 
+from mailman.core.errors import (
+    ReadOnlyPATCHRequestError, UnknownPATCHRequestError)
 from mailman.interfaces.languages import ILanguageManager
 
 
@@ -119,3 +122,50 @@ class Validator:
             missing = COMMASPACE.join(sorted(required_keys - value_keys))
             raise ValueError('Missing parameters: {0}'.format(missing))
         return values
+
+    def update(self, obj, request):
+        """Update the object with the values in the request.
+
+        This first validates and converts the attributes in the request, then
+        updates the given object with the newly converted values.
+
+        :param obj: The object to update.
+        :type obj: object
+        :param request: The HTTP request.
+        :raises ValueError: if conversion failed for some attribute.
+        """
+        for key, value in self.__call__(request).items():
+            self._converters[key].put(obj, key, value)
+
+
+
+class PatchValidator(Validator):
+    """Create a special validator for PATCH requests.
+
+    PATCH is different than PUT because with the latter, you're changing the
+    entire resource, so all expected attributes must exist.  With the former,
+    you're only changing a subset of the attributes, so you only validate the
+    ones that exist in the request.
+    """
+    def __init__(self, request, converters):
+        """Create a validator for the PATCH request.
+
+        :param request: The request object, which must have a .PATCH
+            attribute.
+        :param converters: A mapping of attribute names to the converter for
+            that attribute's type.  Generally, this will be a GetterSetter
+            instance, but it might be something more specific for custom data
+            types (e.g. non-basic types like unicodes).
+        :raises UnknownPATCHRequestError: if the request contains an unknown
+            attribute, i.e. one that is not in the `attributes` mapping.
+        :raises ReadOnlyPATCHRequest: if the requests contains an attribute
+            that is defined as read-only.
+        """
+        validationators = {}
+        for attribute in request.PATCH:
+            if attribute not in converters:
+                raise UnknownPATCHRequestError(attribute)
+            if converters[attribute].decoder is None:
+                raise ReadOnlyPATCHRequestError(attribute)
+            validationators[attribute] = converters[attribute]
+        super(PatchValidator, self).__init__(**validationators)