login: add a change password API

* Uses patron permissions without http request.
* Adds a custom invenio-accouts change password API to allow a librarian
  to change a password for a patron.
* Makes the user_id in the patron jsonschema required.
* Gives more weight on the barcode field for the patrons searches.
* Fixes street description message in the patron JSONSchema, closes #1382.
* Fixes email validation in the patron and vendore JSONSchema, closes #1381.

Co-Authored-by: Johnny Mariéthoz <[email protected]>

.github/workflows/continuous-integration-test.yml

@@ -3,7 +3,7 @@ on: [push, pull_request, workflow_dispatch]
 
 jobs:
   test:
-    runs-on: ubuntu-latest
+    runs-on: ubuntu-20.04
     strategy:
       matrix:
         tests: ['', 'external']

rero_ils/accounts_views.py

@@ -17,14 +17,22 @@
 
 """Invenio Account custom views."""
 
-from flask import current_app
+from flask import after_this_request, current_app
+from flask import request as flask_request
+from invenio_accounts.utils import change_user_password
+from invenio_accounts.views.rest import \
+    ChangePasswordView as BaseChangePasswordView
 from invenio_accounts.views.rest import LoginView as CoreLoginView
-from invenio_accounts.views.rest import use_kwargs
+from invenio_accounts.views.rest import _commit, use_args, use_kwargs
 from invenio_userprofiles.models import UserProfile
+from marshmallow import Schema, fields
 from sqlalchemy.orm.exc import NoResultFound
-from webargs import ValidationError, fields
+from webargs import ValidationError, fields, validate
 from werkzeug.local import LocalProxy
 
+from .modules.patrons.api import Patron, current_patron
+from .modules.patrons.permissions import PatronPermission
+
 current_datastore = LocalProxy(
     lambda: current_app.extensions['security'].datastore)
 
@@ -65,3 +73,68 @@ def post(self, **kwargs):
         self.verify_login(user, **kwargs)
         self.login_user(user)
         return self.success_response(user)
+
+
+class PasswordPassword(Schema):
+    """Args validation when a user want to change his password."""
+
+    password = fields.String(
+        validate=[validate.Length(min=6, max=128)])
+    new_password = fields.String(
+        required=True, validate=[validate.Length(min=6, max=128)])
+
+
+class UsernamePassword(Schema):
+    """Args validation when a professional change a password for a user."""
+
+    username = fields.String(
+        validate=[validate.Length(min=1, max=128)])
+    new_password = fields.String(
+        required=True, validate=[validate.Length(min=6, max=128)])
+
+
+def make_password_schema(request):
+    """Select the right args validation depending on the context."""
+    # Filter based on 'fields' query parameter
+    fields = request.args.get('fields', None)
+    only = fields.split(',') if fields else None
+    # Respect partial updates for PATCH requests
+    partial = request.method == 'PATCH'
+    if request.json.get('username'):
+        return UsernamePassword(only=only,
+                                partial=partial, context={"request": request})
+
+    # Add current request to the schema's context
+    return PasswordPassword(only=only,
+                            partial=partial, context={"request": request})
+
+
+class ChangePasswordView(BaseChangePasswordView):
+    """View to change the user password."""
+
+    def verify_permission(self, username, **args):
+        """Check permissions.
+
+        Check if the current user can change a password for an other user.
+        """
+        patron = Patron.get_patron_by_username(username)
+        if not PatronPermission.can_create(current_patron, patron, patron):
+            return current_app.login_manager.unauthorized()
+
+    def change_password_for_user(self, username, new_password, **kwargs):
+        """Perform change password for a specific user."""
+        after_this_request(_commit)
+        patron = Patron.get_patron_by_username(username)
+        change_user_password(user=patron.user,
+                             password=new_password)
+
+    @use_args(make_password_schema)
+    def post(self, args):
+        """Change user password."""
+        if flask_request.json.get('username'):
+            self.verify_permission(**args)
+            self.change_password_for_user(**args)
+        else:
+            self.verify_password(**args)
+            self.change_password(**args)
+        return self.success_response()

rero_ils/config.py

@@ -246,7 +246,7 @@ def _(x):
     "register": "invenio_accounts.views.rest:RegisterView",
     "forgot_password": "invenio_accounts.views.rest:ForgotPasswordView",
     "reset_password": "invenio_accounts.views.rest:ResetPasswordView",
-    "change_password": "invenio_accounts.views.rest:ChangePasswordView",
+    "change_password": "rero_ils.accounts_views:ChangePasswordView",
     "send_confirmation":
         "invenio_accounts.views.rest:SendConfirmationEmailView",
     "confirm_email": "invenio_accounts.views.rest:ConfirmEmailView",
@@ -1774,6 +1774,9 @@ def _(x):
         'publicationYearText': 2,
         'freeFormedPublicationDate': 2,
         'subjects.*': 2
+    },
+    'patrons': {
+        'barcode': 3
     }
 }
 

rero_ils/modules/patrons/jsonschemas/patrons/patron-v0.0.1.json

@@ -26,6 +26,7 @@
     "last_name",
     "birth_date",
     "username",
+    "user_id",
     "roles"
   ],
   "properties": {
@@ -75,7 +76,7 @@
       "title": "Email",
       "type": "string",
       "format": "email",
-      "pattern": "^.*@.*\\..*$",
+      "pattern": "^.*@.*\\..+$",
       "minLength": 6,
       "form": {
         "validation": {
@@ -113,7 +114,7 @@
     },
     "street": {
       "title": "Street",
-      "description": "Street and number of the address, separated by a coma.",
+      "description": "Street and number of the address.",
       "type": "string",
       "minLength": 3
     },
@@ -265,10 +266,10 @@
           "title": "Additional communication email",
           "type": "string",
           "format": "email",
-          "pattern": "^.*@.*\\..*$",
+          "pattern": "^.*@.*\\..+$",
           "minLength": 6,
           "form": {
-            "hideExpression": "model.communication_channel === 'email'",
+            "hideExpression": "field.parent.model && field.parent.model.communication_channel !== 'email'",
             "messages": {
               "patternMessage": "Email should have at least one <code>@</code> and one <code>.</code>."
             }

rero_ils/modules/patrons/permissions.py

@@ -61,7 +61,19 @@ def create(cls, user, record=None):
         """Create permission check.
 
         :param user: Logged user.
-        :param record: Record to check.
+        :param record: pre-existing Record to check.
+        :return: True is action can be done.
+        """
+        incoming_record = request.get_json(silent=True) or {}
+        return cls.can_create(user, record, incoming_record)
+
+    @classmethod
+    def can_create(cls, user, record, incoming_record):
+        """Create permission check.
+
+        :param user: Logged user.
+        :param record: pre-existing Record to check.
+        :param record: new incoming Record data.
         :return: True is action can be done.
         """
         # only staff members (lib, sys_lib) can create patrons ...
@@ -75,7 +87,6 @@ def create(cls, user, record=None):
                     return True
                 # librarian user has some restrictions...
                 if current_patron.is_librarian:
-                    incoming_record = request.get_json(silent=True) or {}
                     # a librarian cannot manage a system_librarian patron
                     if 'system_librarian' in incoming_record.get('roles', [])\
                        or 'system_librarian' in record.get('roles', []):

rero_ils/modules/vendors/jsonschemas/vendors/vendor-v0.0.1.json

@@ -151,7 +151,7 @@
           "title": "Email",
           "type": "string",
           "format": "email",
-          "pattern": "^.*@.*\\..*$",
+          "pattern": "^.*@.*\\..+$",
           "minLength": 6,
           "form": {
             "validation": {
@@ -213,7 +213,7 @@
           "title": "Email",
           "type": "string",
           "format": "email",
-          "pattern": "^.*@.*\\..*$",
+          "pattern": "^.*@.*\\..+$",
           "minLength": 6,
           "form": {
             "validation": {

tests/api/test_user_authentication.py

@@ -17,6 +17,7 @@
 
 """Test User Authentication API."""
 from flask import url_for
+from invenio_accounts.testutils import login_user_via_session
 from utils import get_json, postdata
 
 
@@ -98,3 +99,78 @@ def test_login_without_email(client, patron_sion_without_email):
     assert data.get('id')
     # logout for the next test
     client.post(url_for('invenio_accounts_rest_auth.logout'))
+
+
+def test_change_password(client, patron_martigny_no_email,
+                         librarian_sion_no_email,
+                         librarian_martigny_no_email):
+    """Test login with several scenarios."""
+
+    # try to change password with an anonymous user
+    res, _ = postdata(
+        client,
+        'invenio_accounts_rest_auth.change_password',
+        {
+            'password': patron_martigny_no_email.get('birth_date'),
+            'new_password': 'new'
+        }
+    )
+    data = get_json(res)
+    assert res.status_code == 401
+
+    # with a logged but the password is too short
+    login_user_via_session(client, patron_martigny_no_email.user)
+    res, _ = postdata(
+        client,
+        'invenio_accounts_rest_auth.change_password',
+        {
+            'password': patron_martigny_no_email.get('birth_date'),
+            'new_password': 'new'
+        }
+    )
+    data = get_json(res)
+    assert res.status_code == 400
+    assert data.get('message') == 'Validation error.'
+
+    # with a logged user
+    res, _ = postdata(
+        client,
+        'invenio_accounts_rest_auth.change_password',
+        {
+            'password': patron_martigny_no_email.get('birth_date'),
+            'new_password': 'new_passwd'
+        }
+    )
+    data = get_json(res)
+    assert res.status_code == 200
+    assert data.get('message') == 'You successfully changed your password.'
+
+    # with a librarian of a different organisation
+    login_user_via_session(client, librarian_sion_no_email.user)
+    res, _ = postdata(
+        client,
+        'invenio_accounts_rest_auth.change_password',
+        {
+            'username': patron_martigny_no_email.get('username'),
+            'new_password': 'new_passwd2'
+        }
+    )
+    data = get_json(res)
+    assert res.status_code == 401
+
+    # with a librarian of the same organisation
+    login_user_via_session(client, librarian_martigny_no_email.user)
+    res, _ = postdata(
+        client,
+        'invenio_accounts_rest_auth.change_password',
+        {
+            'username': patron_martigny_no_email.get('username'),
+            'new_password': 'new_passwd2'
+        }
+    )
+    data = get_json(res)
+    assert res.status_code == 200
+    assert data.get('message') == 'You successfully changed your password.'
+
+    # logout for the next test
+    client.post(url_for('invenio_accounts_rest_auth.logout'))

tests/unit/conftest.py

@@ -19,6 +19,8 @@
 
 from __future__ import absolute_import, print_function
 
+from copy import deepcopy
+
 import pytest
 from pkg_resources import resource_string
 from utils import get_schema
@@ -158,6 +160,15 @@ def patron_schema(monkeypatch):
     return get_schema(monkeypatch, schema_in_bytes)
 
 
+@pytest.fixture(scope="function")
+def patron_martigny_data_tmp_with_id(patron_martigny_data_tmp):
+    """Load Martigny patron data scope function with a mocked user_id."""
+    patron = deepcopy(patron_martigny_data_tmp)
+    # mock the user_id which is add by the Patron API.
+    patron['user_id'] = 100
+    return patron
+
+
 @pytest.fixture()
 def persons_schema(monkeypatch):
     """Patron Jsonschema for records."""