Some more reviews from @leplatrem

CHANGELOG.rst

@@ -27,6 +27,11 @@ This document describes changes between each past release.
 - Add a ``validate`` endpoint at ``/accounts/{user id}/validate/{validation
   key}`` which can be used to validate an account when the :ref:`account
   validation <accounts-validate>` option is enabled on the accounts plugin.
+- Add a ``reset-password`` endpoint at ``/accounts/(user
+  id)/reset-password`` which can be used to reset a user's password when the
+  :ref:`account validation <accounts-validate>` option is enabled on the
+  accounts plugin.
+
 
 API is now at version **1.22**. See `API changelog`_.
 

docs/api/1.x/accounts.rst

@@ -153,6 +153,15 @@ Alternatively, accounts can be created using POST.  Supply the user id and passw
 
     Depending on the :ref:`configuration <settings-accounts>`, you may not be allowed to create accounts.
 
+.. note::
+
+    If the :ref:`accounts validation <settings-account-validation>` is enabled,
+    you might also need to provide an ``email-context`` in the ``data``: 
+
+    .. sourcecode:: bash
+
+        $ echo '{"data": {"id": "bob@example.com", "password": "azerty123", "email-context": {"name": "Bob Smith", "form-url": "https://example.com/validate/"}}}' | http POST http://localhost:8888/v1/accounts --verbose
+
 
 .. _accounts-update:
 
@@ -361,12 +370,12 @@ Resetting a forgotten password
 
 If the ``account_validation`` option in :ref:`the settings
 <settings-account-validation>` has been enabled, a temporary reset password may
-be requested through the endpoint available at `/accounts/(user
-id)/reset-password`.
+be requested through the endpoint available at ``/accounts/(user
+id)/reset-password``.
 
 .. http:post:: /accounts/(user_id)/reset-password
 
-    :synopsis: Require a temporary reset password for an account with the ``account_validation`` option enabled.
+    :synopsis: Request a temporary reset password for an account with the ``account_validation`` option enabled.
 
     **Anonymous**
 
@@ -404,5 +413,15 @@ id)/reset-password`.
             "message": "A temporary reset password has been sent by mail"
         }
 
+   .. note::
+
+       You might also need to provide an ``email-context`` in the ``data`` to fill
+       in the holes of the email template defined in the :ref:`settings
+       <settings-account-password-reset>`:
+
+       .. sourcecode:: bash
+
+          $ echo '{"data": {"email-context": {"name": "Bob Smith"}}}' | http POST http://localhost:8888/v1/accounts/bob@example.com/reset-password --verbose
+
 Using this temporary reset password, one can
 :ref:`update the account <accounts-update>` providing the new password.

docs/api/index.rst

@@ -14,11 +14,13 @@ Changelog
 1.22 (unreleased)
 '''''''''''''''''
 
-- new `/accounts/{user id}/validate/{validation key}` endpoint when the
-  `account validation` option is enabled for the accounts plugin. See
-  :ref:`account validation <accounts-validate>` (#1973)
-- new `/accounts/{user id}/reset-password` endpoint to request a temporary
-  reset password by email (#1973)
+- New ``/accounts/{user id}/validate/{validation key}`` endpoint to validate a
+  created account when the ``account validation`` option is enabled for the
+  accounts plugin. See :ref:`account validation <accounts-validate>` (#1973)
+- New ``/accounts/{user id}/reset-password`` endpoint to request a temporary
+  reset password by email when the ``account validation`` option is enabled for
+  the accounts plugin. See :ref:`account validation <accounts-validate>` (#1973)
+
 
 1.21 (2019-01-10)
 '''''''''''''''''

docs/configuration/settings.rst

@@ -587,7 +587,7 @@ email will be sent with an activation key.
 .. note::
 
     Both the account validation and password reset need a properly configured
-    smtp server.
+    SMTP server.
     To use a debug or testing mailer you may use the ``mail.mailer = debug`` or
     ``mail.mailer = testing`` settings. Refer to
     `pyramid_mailer's configuration <https://docs.pylonsproject.org/projects/pyramid_mailer/en/latest/#configuration>`_.
@@ -604,13 +604,9 @@ activation key will be valid:
     # delay the account won't be activable anymore.
     kinto.account_validation.validation_key_cache_ttl_seconds = 604800  # 7 days in seconds.
 
-Once :ref:`created <accounts-create>`, the account will need to be activated
-before being able to authenticate, using the ``validate`` endpoint and the
-``activation-key`` sent by email.
-
-.. sourcecode:: bash
-
-    $ echo '{"data": {"id": "bob@example.com", "password": "azerty123"}}' | http POST http://localhost:8888/v1/accounts --verbose
+Once :ref:`created <accounts-create>`, the account will need to be
+:ref:`activated <accounts-validate>` before being able to authenticate, using
+the ``validate`` endpoint and the ``activation-key`` sent by email.
 
 If the user was created, an email was sent to the user with the activation key,
 which needs to be POSTed to the ``validate`` endpoint.
@@ -648,11 +644,8 @@ The templates for the email subject and body can be customized:
 
 ... and they will be ``String.format``-ted with the content of the user, an
 optional additional ``email-context`` provided alongside the user object, and
-the ``activation-key``:
-
-.. sourcecode:: bash
-
-    $ echo '{"data": {"id": "bob@example.com", "password": "azerty123", "email-context": {"name": "Bob Smith", "form-url": "https://example.com/validate/"}}}' | http POST http://localhost:8888/v1/accounts --verbose
+the ``activation-key`` (see the note in :ref:`account creation
+<accounts-create>` for an example usage).
 
 Whatever the means, a POST to the
 ``/accounts/(user_id)/validate/(activation_key)`` will validate and activate
@@ -666,17 +659,16 @@ rendered using the same ``email-context``.
     kinto.account_validation.email_confirmation_subject_template = "Account active"
     kinto.account_validation.email_confirmation_body_template = "Your account {id} is now active"
 
+.. _settings-account-password-reset:
+
 **About password reset**
 
 When the :ref:`account validation <accounts-validate>` option is enabled, an
 additional endpoint is available at ``/accounts/(user id)/reset-password`` to
-require a temporary reset password by email.
-
-.. sourcecode:: bash
+require a temporary reset password by email (see :ref:`the API docs
+<accounts-reset-password>`).
 
-    $ http POST http://localhost:8888/v1/accounts/bob@example.com/reset-password --verbose
-
-Example email:
+Example email sent to the user with the temporary reset password:
 
 ::
 
@@ -700,7 +692,8 @@ following settings:
 
 Those templates will be rendered using the user record fields, an optional
 additional ``email-context`` provided alongside the user object, and the
-``reset-password``.
+``reset-password`` (see the note in :ref:`resetting a forgotten password
+<accounts-reset-password>` for an example usage).
 
 
 It is the responsability of the administrator to tell the mail recipient how to
@@ -710,11 +703,6 @@ This could be done by providing a link to a webapp that displays a form to the
 user asking for the new password and a call to action, which will POST the
 new password to the ``accounts/(user_id)`` endpoint.
 
-
-.. sourcecode:: bash
-
-    $ echo '{"data": {"email-context": {"name": "Bob Smith"}}}' | http POST http://localhost:8888/v1/accounts/bob@example.com/reset-password --verbose
-
 Using this temporary reset password, one can
 :ref:`update the account <accounts-update>` providing the new password.
 

kinto/plugins/accounts/authentication.py

@@ -7,9 +7,9 @@
 from .utils import (
     ACCOUNT_CACHE_KEY,
     ACCOUNT_POLICY_NAME,
-    ACCOUNT_RESET_PASSWORD_CACHE_KEY,
     is_validated,
     get_cached_reset_password,
+    delete_cached_reset_password,
 )
 
 
@@ -52,7 +52,7 @@ def account_check(username, password, request):
         return True
 
     # Last chance, is this a "reset password" flow?
-    reset_password_flow(username, password, request)
+    return reset_password_flow(username, password, request)
 
 
 def reset_password_flow(username, password, request):
@@ -80,7 +80,7 @@ def reset_password_flow(username, password, request):
 
     try:
         data = request.json["data"]
-    except ValueError:
+    except (ValueError, KeyError):
         return None
 
     # Request one and only one data field: the `password`.
@@ -90,10 +90,7 @@ def reset_password_flow(username, password, request):
     cached_password_str = cached_password.encode(encoding="utf-8")
     if bcrypt.checkpw(pwd_str, cached_password_str):
         # Remove the temporary reset password from the cache.
-        reset_password_cache_key = utils.hmac_digest(
-            hmac_secret, ACCOUNT_RESET_PASSWORD_CACHE_KEY.format(username)
-        )
-        cache.delete(reset_password_cache_key)
+        delete_cached_reset_password(username, request.registry)
         cache.set(cache_key, hashed_password, ttl=cache_ttl)
         return True
 

kinto/plugins/accounts/utils.py

@@ -7,6 +7,8 @@
 ACCOUNT_POLICY_NAME = "account"
 ACCOUNT_RESET_PASSWORD_CACHE_KEY = "accounts:{}:reset-password"
 ACCOUNT_VALIDATION_CACHE_KEY = "accounts:{}:validation-key"
+DEFAULT_RESET_PASSWORD_CACHE_TTL_SECONDS = 7 * 24 * 60 * 60
+DEFAULT_VALIDATION_KEY_CACHE_TTL_SECONDS = 7 * 24 * 60 * 60
 
 
 def hash_password(password):
@@ -25,6 +27,23 @@ def is_validated(user):
     return user.get("validated", True)
 
 
+def cache_reset_password(reset_password, username, registry):
+    """Store a reset-password in the cache."""
+    settings = registry.settings
+    hmac_secret = settings["userid_hmac_secret"]
+    cache_key = utils.hmac_digest(hmac_secret, ACCOUNT_RESET_PASSWORD_CACHE_KEY.format(username))
+    # Store a reset password for 7 days by default.
+    cache_ttl = int(
+        settings.get(
+            "account_validation.reset_password_cache_ttl_seconds",
+            DEFAULT_RESET_PASSWORD_CACHE_TTL_SECONDS,
+        )
+    )
+
+    cache = registry.cache
+    cache.set(cache_key, reset_password, ttl=cache_ttl)
+
+
 def get_cached_reset_password(username, registry):
     """Given a username, get the reset-password from the cache."""
     hmac_secret = registry.settings["userid_hmac_secret"]
@@ -33,3 +52,57 @@ def get_cached_reset_password(username, registry):
     cache = registry.cache
     cache_result = cache.get(cache_key)
     return cache_result
+
+
+def delete_cached_reset_password(username, registry):
+    """Given a username, delete the reset-password from the cache."""
+    hmac_secret = registry.settings["userid_hmac_secret"]
+    cache_key = utils.hmac_digest(hmac_secret, ACCOUNT_RESET_PASSWORD_CACHE_KEY.format(username))
+
+    cache = registry.cache
+    cache_result = cache.delete(cache_key)
+    return cache_result
+
+
+def cache_validation_key(activation_key, username, registry):
+    """Store a validation_key in the cache."""
+    settings = registry.settings
+    hmac_secret = settings["userid_hmac_secret"]
+    cache_key = utils.hmac_digest(hmac_secret, ACCOUNT_VALIDATION_CACHE_KEY.format(username))
+    # Store an activation key for 7 days by default.
+    cache_ttl = int(
+        settings.get(
+            "account_validation.validation_key_cache_ttl_seconds",
+            DEFAULT_VALIDATION_KEY_CACHE_TTL_SECONDS,
+        )
+    )
+
+    cache = registry.cache
+    cache.set(cache_key, activation_key, ttl=cache_ttl)
+
+
+def get_cached_validation_key(username, registry):
+    """Given a username, get the validation key from the cache."""
+    hmac_secret = registry.settings["userid_hmac_secret"]
+    cache_key = utils.hmac_digest(hmac_secret, ACCOUNT_VALIDATION_CACHE_KEY.format(username))
+    cache = registry.cache
+    activation_key = cache.get(cache_key)
+    return activation_key
+
+
+def delete_cached_validation_key(username, registry):
+    """Given a username, delete the validation key from the cache."""
+    hmac_secret = registry.settings["userid_hmac_secret"]
+    cache_key = utils.hmac_digest(hmac_secret, ACCOUNT_VALIDATION_CACHE_KEY.format(username))
+    cache = registry.cache
+    cache_result = cache.delete(cache_key)
+    return cache_result
+
+
+def delete_cached_account_key(username, registry):
+    """Given a username, delete the account key from the cache."""
+    hmac_secret = registry.settings["userid_hmac_secret"]
+    cache_key = utils.hmac_digest(hmac_secret, ACCOUNT_CACHE_KEY.format(username))
+    cache = registry.cache
+    cache_result = cache.delete(cache_key)
+    return cache_result

kinto/plugins/accounts/views/__init__.py

@@ -9,20 +9,20 @@
 
 
 from kinto.views import NameGenerator
-from kinto.core import resource, utils
+from kinto.core import resource
 from kinto.core.errors import raise_invalid, http_error
 from kinto.core.events import ResourceChanged, ACTIONS
 
 from ..mails import Emailer
 from ..utils import (
+    cache_validation_key,
+    delete_cached_account_key,
+    get_cached_validation_key,
     hash_password,
-    ACCOUNT_CACHE_KEY,
     ACCOUNT_POLICY_NAME,
-    ACCOUNT_VALIDATION_CACHE_KEY,
 )
 
 DEFAULT_EMAIL_REGEXP = "^[a-zA-Z0-9_.+-]+@[a-zA-Z0-9-]+\\.[a-zA-Z0-9-.]+$"
-DEFAULT_VALIDATION_KEY_CACHE_TTL_SECONDS = 7 * 24 * 60 * 60
 
 
 def _extract_posted_body_id(request):
@@ -39,23 +39,6 @@ def _extract_posted_body_id(request):
         raise http_error(httpexceptions.HTTPUnauthorized(), error=error_msg)
 
 
-def cache_validation_key(activation_key, username, registry):
-    """Store a validation_key in the cache."""
-    settings = registry.settings
-    hmac_secret = settings["userid_hmac_secret"]
-    cache_key = utils.hmac_digest(hmac_secret, ACCOUNT_VALIDATION_CACHE_KEY.format(username))
-    # Store an activation key for 7 days by default.
-    cache_ttl = int(
-        settings.get(
-            "account_validation.validation_key_cache_ttl_seconds",
-            DEFAULT_VALIDATION_KEY_CACHE_TTL_SECONDS,
-        )
-    )
-
-    cache = registry.cache
-    cache.set(cache_key, activation_key, ttl=cache_ttl)
-
-
 class AccountIdGenerator(NameGenerator):
     """Allow @ signs in account IDs."""
 
@@ -187,16 +170,12 @@ def process_object(self, new, old=None):
 )
 def on_account_changed(event):
     request = event.request
-    cache = request.registry.cache
-    settings = request.registry.settings
-    hmac_secret = settings["userid_hmac_secret"]
 
     for obj in event.impacted_objects:
         # Extract username and password from current user
         username = obj["old"]["id"]
-        cache_key = utils.hmac_digest(hmac_secret, ACCOUNT_CACHE_KEY.format(username))
         # Delete cache
-        cache.delete(cache_key)
+        delete_cached_account_key(username, request.registry)
 
 
 # Send activation code by email on account creation if account validation is enabled.
@@ -207,14 +186,10 @@ def on_account_created(event):
     if not settings.get("account_validation", False):
         return
 
-    cache = request.registry.cache
-    hmac_secret = settings["userid_hmac_secret"]
-
     for impacted_object in event.impacted_objects:
         account = impacted_object["new"]
         user_email = account["id"]
-        cache_key = utils.hmac_digest(hmac_secret, ACCOUNT_VALIDATION_CACHE_KEY.format(user_email))
-        activation_key = cache.get(cache_key)
+        activation_key = get_cached_validation_key(user_email, request.registry)
         if activation_key is None:
             continue