diff --git a/.github/workflows/continuous-integration-test.yml b/.github/workflows/continuous-integration-test.yml index 618f637bff..cf8d462789 100644 --- a/.github/workflows/continuous-integration-test.yml +++ b/.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'] diff --git a/rero_ils/accounts_views.py b/rero_ils/accounts_views.py index e2bcd58f9c..447a93196d 100644 --- a/rero_ils/accounts_views.py +++ b/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() diff --git a/rero_ils/config.py b/rero_ils/config.py index bed1aebead..c3bfd88ce3 100644 --- a/rero_ils/config.py +++ b/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 } } diff --git a/rero_ils/modules/patrons/jsonschemas/patrons/patron-v0.0.1.json b/rero_ils/modules/patrons/jsonschemas/patrons/patron-v0.0.1.json index 29a7c34a94..528a15c0f3 100644 --- a/rero_ils/modules/patrons/jsonschemas/patrons/patron-v0.0.1.json +++ b/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 @ and one .." } diff --git a/rero_ils/modules/patrons/permissions.py b/rero_ils/modules/patrons/permissions.py index cccc20cae2..e0e5384c9e 100644 --- a/rero_ils/modules/patrons/permissions.py +++ b/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', []): diff --git a/rero_ils/modules/vendors/jsonschemas/vendors/vendor-v0.0.1.json b/rero_ils/modules/vendors/jsonschemas/vendors/vendor-v0.0.1.json index b27acfcd10..cd2691837b 100644 --- a/rero_ils/modules/vendors/jsonschemas/vendors/vendor-v0.0.1.json +++ b/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": { diff --git a/tests/api/test_user_authentication.py b/tests/api/test_user_authentication.py index d6793c5fdd..1c36801c51 100644 --- a/tests/api/test_user_authentication.py +++ b/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')) diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 2531ab7259..80b6f9ec74 100644 --- a/tests/unit/conftest.py +++ b/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.""" diff --git a/tests/unit/test_patrons_jsonschema.py b/tests/unit/test_patrons_jsonschema.py index 7d542264ba..c626f01b11 100644 --- a/tests/unit/test_patrons_jsonschema.py +++ b/tests/unit/test_patrons_jsonschema.py @@ -24,166 +24,166 @@ from jsonschema.exceptions import ValidationError -def test_required(patron_schema, patron_martigny_data_tmp): +def test_required(patron_schema, patron_martigny_data_tmp_with_id): """Test required for patron jsonschemas.""" - validate(patron_martigny_data_tmp, patron_schema) + validate(patron_martigny_data_tmp_with_id, patron_schema) with pytest.raises(ValidationError): validate({}, patron_schema) - validate(patron_martigny_data_tmp, patron_schema) + validate(patron_martigny_data_tmp_with_id, patron_schema) -def test_pid(patron_schema, patron_martigny_data_tmp): +def test_pid(patron_schema, patron_martigny_data_tmp_with_id): """Test pid for patron jsonschemas.""" - validate(patron_martigny_data_tmp, patron_schema) + validate(patron_martigny_data_tmp_with_id, patron_schema) with pytest.raises(ValidationError): - patron_martigny_data_tmp['pid'] = 25 - validate(patron_martigny_data_tmp, patron_schema) + patron_martigny_data_tmp_with_id['pid'] = 25 + validate(patron_martigny_data_tmp_with_id, patron_schema) -def test_first_name(patron_schema, patron_martigny_data_tmp): +def test_first_name(patron_schema, patron_martigny_data_tmp_with_id): """Test first_name for patron jsonschemas.""" - validate(patron_martigny_data_tmp, patron_schema) + validate(patron_martigny_data_tmp_with_id, patron_schema) with pytest.raises(ValidationError): - patron_martigny_data_tmp['first_name'] = 25 - validate(patron_martigny_data_tmp, patron_schema) + patron_martigny_data_tmp_with_id['first_name'] = 25 + validate(patron_martigny_data_tmp_with_id, patron_schema) -def test_last_name(patron_schema, patron_martigny_data_tmp): +def test_last_name(patron_schema, patron_martigny_data_tmp_with_id): """Test last_name for patron jsonschemas.""" - validate(patron_martigny_data_tmp, patron_schema) + validate(patron_martigny_data_tmp_with_id, patron_schema) with pytest.raises(ValidationError): - patron_martigny_data_tmp['last_name'] = 25 - validate(patron_martigny_data_tmp, patron_schema) + patron_martigny_data_tmp_with_id['last_name'] = 25 + validate(patron_martigny_data_tmp_with_id, patron_schema) -def test_street(patron_schema, patron_martigny_data_tmp): +def test_street(patron_schema, patron_martigny_data_tmp_with_id): """Test street for patron jsonschemas.""" - validate(patron_martigny_data_tmp, patron_schema) + validate(patron_martigny_data_tmp_with_id, patron_schema) with pytest.raises(ValidationError): - patron_martigny_data_tmp['street'] = 25 - validate(patron_martigny_data_tmp, patron_schema) + patron_martigny_data_tmp_with_id['street'] = 25 + validate(patron_martigny_data_tmp_with_id, patron_schema) -def test_postal_code(patron_schema, patron_martigny_data_tmp): +def test_postal_code(patron_schema, patron_martigny_data_tmp_with_id): """Test postal_code for patron jsonschemas.""" - validate(patron_martigny_data_tmp, patron_schema) + validate(patron_martigny_data_tmp_with_id, patron_schema) with pytest.raises(ValidationError): - patron_martigny_data_tmp['postal_code'] = 25 - validate(patron_martigny_data_tmp, patron_schema) + patron_martigny_data_tmp_with_id['postal_code'] = 25 + validate(patron_martigny_data_tmp_with_id, patron_schema) -def test_city(patron_schema, patron_martigny_data_tmp): +def test_city(patron_schema, patron_martigny_data_tmp_with_id): """Test city for patron jsonschemas.""" - validate(patron_martigny_data_tmp, patron_schema) + validate(patron_martigny_data_tmp_with_id, patron_schema) with pytest.raises(ValidationError): - patron_martigny_data_tmp['city'] = 25 - validate(patron_martigny_data_tmp, patron_schema) + patron_martigny_data_tmp_with_id['city'] = 25 + validate(patron_martigny_data_tmp_with_id, patron_schema) -def test_username(patron_schema, patron_martigny_data_tmp): +def test_username(patron_schema, patron_martigny_data_tmp_with_id): """Test username for patron jsonschemas.""" - validate(patron_martigny_data_tmp, patron_schema) - del(patron_martigny_data_tmp['username']) + validate(patron_martigny_data_tmp_with_id, patron_schema) + del(patron_martigny_data_tmp_with_id['username']) with pytest.raises(ValidationError): - validate(patron_martigny_data_tmp, patron_schema) + validate(patron_martigny_data_tmp_with_id, patron_schema) -def test_barcode(patron_schema, patron_martigny_data_tmp): +def test_barcode(patron_schema, patron_martigny_data_tmp_with_id): """Test barcode for patron jsonschemas.""" - validate(patron_martigny_data_tmp, patron_schema) + validate(patron_martigny_data_tmp_with_id, patron_schema) with pytest.raises(ValidationError): - patron_martigny_data_tmp['patron']['barcode'] = 25 - validate(patron_martigny_data_tmp, patron_schema) + patron_martigny_data_tmp_with_id['patron']['barcode'] = 25 + validate(patron_martigny_data_tmp_with_id, patron_schema) -def test_birth_date(patron_schema, patron_martigny_data_tmp): +def test_birth_date(patron_schema, patron_martigny_data_tmp_with_id): """Test birth date for patron jsonschemas.""" - validate(patron_martigny_data_tmp, patron_schema) + validate(patron_martigny_data_tmp_with_id, patron_schema) with pytest.raises(ValidationError): - patron_martigny_data_tmp['birth_date'] = 25 - validate(patron_martigny_data_tmp, patron_schema) + patron_martigny_data_tmp_with_id['birth_date'] = 25 + validate(patron_martigny_data_tmp_with_id, patron_schema) -def test_email(patron_schema, patron_martigny_data_tmp): +def test_email(patron_schema, patron_martigny_data_tmp_with_id): """Test email for patron jsonschemas.""" - validate(patron_martigny_data_tmp, patron_schema) + validate(patron_martigny_data_tmp_with_id, patron_schema) # email is optional - del patron_martigny_data_tmp['email'] - validate(patron_martigny_data_tmp, patron_schema) + del patron_martigny_data_tmp_with_id['email'] + validate(patron_martigny_data_tmp_with_id, patron_schema) # email with a bad format with pytest.raises(ValidationError): - patron_martigny_data_tmp['email'] = 25 - validate(patron_martigny_data_tmp, patron_schema) + patron_martigny_data_tmp_with_id['email'] = 25 + validate(patron_martigny_data_tmp_with_id, patron_schema) -def test_phone(patron_schema, patron_martigny_data_tmp): +def test_phone(patron_schema, patron_martigny_data_tmp_with_id): """Test phone for patron jsonschemas.""" - validate(patron_martigny_data_tmp, patron_schema) + validate(patron_martigny_data_tmp_with_id, patron_schema) with pytest.raises(ValidationError): - patron_martigny_data_tmp['phone'] = 25 - validate(patron_martigny_data_tmp, patron_schema) + patron_martigny_data_tmp_with_id['phone'] = 25 + validate(patron_martigny_data_tmp_with_id, patron_schema) -def test_patron_type(patron_schema, patron_martigny_data_tmp): +def test_patron_type(patron_schema, patron_martigny_data_tmp_with_id): """Test patron_type for patron jsonschemas.""" - validate(patron_martigny_data_tmp, patron_schema) + validate(patron_martigny_data_tmp_with_id, patron_schema) with pytest.raises(ValidationError): - patron_martigny_data_tmp['patron_type_pid'] = 25 - validate(patron_martigny_data_tmp, patron_schema) + patron_martigny_data_tmp_with_id['patron_type_pid'] = 25 + validate(patron_martigny_data_tmp_with_id, patron_schema) -def test_roles(patron_schema, patron_martigny_data_tmp): +def test_roles(patron_schema, patron_martigny_data_tmp_with_id): """Test roles for patron jsonschemas.""" - validate(patron_martigny_data_tmp, patron_schema) + validate(patron_martigny_data_tmp_with_id, patron_schema) with pytest.raises(ValidationError): - patron_martigny_data_tmp['roles'] = 'text' - validate(patron_martigny_data_tmp, patron_schema) + patron_martigny_data_tmp_with_id['roles'] = 'text' + validate(patron_martigny_data_tmp_with_id, patron_schema) -def test_blocked(patron_schema, patron_martigny_data_tmp): +def test_blocked(patron_schema, patron_martigny_data_tmp_with_id): """Test blocked field for patron jsonschemas.""" - validate(patron_martigny_data_tmp, patron_schema) + validate(patron_martigny_data_tmp_with_id, patron_schema) # blocked is a boolean field, should fail with everything except boolean with pytest.raises(ValidationError): - patron_martigny_data_tmp['patron']['blocked'] = 25 - validate(patron_martigny_data_tmp, patron_schema) + patron_martigny_data_tmp_with_id['patron']['blocked'] = 25 + validate(patron_martigny_data_tmp_with_id, patron_schema) with pytest.raises(ValidationError): - patron_martigny_data_tmp['patron']['blocked'] = 'text' - validate(patron_martigny_data_tmp, patron_schema) + patron_martigny_data_tmp_with_id['patron']['blocked'] = 'text' + validate(patron_martigny_data_tmp_with_id, patron_schema) # Should pass with boolean - patron_martigny_data_tmp['patron']['blocked'] = False - validate(patron_martigny_data_tmp, patron_schema) + patron_martigny_data_tmp_with_id['patron']['blocked'] = False + validate(patron_martigny_data_tmp_with_id, patron_schema) -def test_blocked_note(patron_schema, patron_martigny_data_tmp): +def test_blocked_note(patron_schema, patron_martigny_data_tmp_with_id): """Test blocked_note field for patron jsonschemas.""" - validate(patron_martigny_data_tmp, patron_schema) + validate(patron_martigny_data_tmp_with_id, patron_schema) # blocked_note is text field. Should fail except with text. with pytest.raises(ValidationError): - patron_martigny_data_tmp['patron']['blocked_note'] = 25 - validate(patron_martigny_data_tmp, patron_schema) + patron_martigny_data_tmp_with_id['patron']['blocked_note'] = 25 + validate(patron_martigny_data_tmp_with_id, patron_schema) with pytest.raises(ValidationError): - patron_martigny_data_tmp['patron']['blocked_note'] = True - validate(patron_martigny_data_tmp, patron_schema) + patron_martigny_data_tmp_with_id['patron']['blocked_note'] = True + validate(patron_martigny_data_tmp_with_id, patron_schema) - patron_martigny_data_tmp['patron']['blocked_note'] = 'Lost card' - validate(patron_martigny_data_tmp, patron_schema) + patron_martigny_data_tmp_with_id['patron']['blocked_note'] = 'Lost card' + validate(patron_martigny_data_tmp_with_id, patron_schema)