diff --git a/data/role_policies.json b/data/role_policies.json index 5fdda8c56c..3eb2555852 100644 --- a/data/role_policies.json +++ b/data/role_policies.json @@ -701,12 +701,14 @@ "pro_catalog_manager" ], "tmpl-search": [ + "pro_read_only", "pro_full_permissions", "pro_catalog_manager", "pro_user_manager", "pro_library_administrator" ], "tmpl-read": [ + "pro_read_only", "pro_full_permissions", "pro_catalog_manager", "pro_user_manager", diff --git a/poetry.lock b/poetry.lock index 46e1d0f418..0452cf0871 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1626,6 +1626,29 @@ postgresql = ["invenio-db[postgresql,versioning] (>=1.0.9,<1.1.0)"] sqlite = ["invenio-db[versioning] (>=1.0.9,<1.1.0)"] tests = ["pytest-invenio (>=1.4.1)"] +[[package]] +name = "invenio-records-permissions" +version = "0.13.2" +description = "Permission policies for Invenio records." +category = "main" +optional = false +python-versions = "*" + +[package.dependencies] +invenio-access = ">=1.4.2,<2.0.0" +invenio-i18n = ">=1.2.0" +invenio-records = ">=1.4.0" + +[package.extras] +all = ["Sphinx (==4.2.0)", "invenio-accounts (>=2.0.0)", "invenio-app (>=1.3.0)", "pytest-invenio (>=1.4.13,<2.0.0)", "pytest-mock (>=1.6.0)"] +docs = ["Sphinx (==4.2.0)"] +elasticsearch6 = ["invenio-search[elasticsearch6] (>=1.4.1,<2.0.0)"] +elasticsearch7 = ["invenio-search[elasticsearch7] (>=1.4.1,<2.0.0)"] +mysql = ["invenio-db[mysql,versioning] (>=1.0.9,<2.0.0)"] +postgresql = ["invenio-db[postgresql,versioning] (>=1.0.9,<2.0.0)"] +sqlite = ["invenio-db[versioning] (>=1.0.9,<2.0.0)"] +tests = ["invenio-accounts (>=2.0.0)", "invenio-app (>=1.3.0)", "pytest-invenio (>=1.4.13,<2.0.0)", "pytest-mock (>=1.6.0)"] + [[package]] name = "invenio-records-rest" version = "1.8.0" @@ -3352,7 +3375,7 @@ sip2 = ["invenio-sip2"] [metadata] lock-version = "1.1" python-versions = ">= 3.9, <3.10" -content-hash = "d6313dc83327a95988b754aeceaa80195c99a93aa9d15bf82505029f48d60be0" +content-hash = "ab5ab91812114d376d84da4a086cd1be92f195b8282bb89c8b9d17b5b8ae0d8b" [metadata.files] alabaster = [ @@ -4021,6 +4044,10 @@ invenio-records = [ {file = "invenio-records-1.6.1.tar.gz", hash = "sha256:8eb0f0343ecb8c6f968e9b66162046131eef34739a33f26b2c16b84f2e987353"}, {file = "invenio_records-1.6.1-py2.py3-none-any.whl", hash = "sha256:0dafee31de372969be1f56ed672a535b5cd5d0fdf17c8f1a0b37cc0eab79819e"}, ] +invenio-records-permissions = [ + {file = "invenio-records-permissions-0.13.2.tar.gz", hash = "sha256:a08a818c9dd4e8f94d8e8162490c5a4a780c5d3de3964cac4e28c9ecb528c658"}, + {file = "invenio_records_permissions-0.13.2-py2.py3-none-any.whl", hash = "sha256:168813230c99389b42a9983f4c52ce410e622332fc544ff5ed27a6ae7817e86b"}, +] invenio-records-rest = [ {file = "invenio-records-rest-1.8.0.tar.gz", hash = "sha256:70ba741f19f8c9a1ae14a700d82c632175e881fd786ffdc4692f2718482e8dd1"}, {file = "invenio_records_rest-1.8.0-py2.py3-none-any.whl", hash = "sha256:67fb753131e00bd20aef9d1011d51bcb407d1a30ef2e4af8647bf3b92f2b999e"}, @@ -4447,8 +4474,6 @@ psycopg2-binary = [ {file = "psycopg2_binary-2.9.5-cp311-cp311-musllinux_1_1_i686.whl", hash = "sha256:e67b3c26e9b6d37b370c83aa790bbc121775c57bfb096c2e77eacca25fd0233b"}, {file = "psycopg2_binary-2.9.5-cp311-cp311-musllinux_1_1_ppc64le.whl", hash = "sha256:5fc447058d083b8c6ac076fc26b446d44f0145308465d745fba93a28c14c9e32"}, {file = "psycopg2_binary-2.9.5-cp311-cp311-musllinux_1_1_x86_64.whl", hash = "sha256:d892bfa1d023c3781a3cab8dd5af76b626c483484d782e8bd047c180db590e4c"}, - {file = "psycopg2_binary-2.9.5-cp311-cp311-win32.whl", hash = "sha256:2abccab84d057723d2ca8f99ff7b619285d40da6814d50366f61f0fc385c3903"}, - {file = "psycopg2_binary-2.9.5-cp311-cp311-win_amd64.whl", hash = "sha256:bef7e3f9dc6f0c13afdd671008534be5744e0e682fb851584c8c3a025ec09720"}, {file = "psycopg2_binary-2.9.5-cp36-cp36m-macosx_10_14_x86_64.macosx_10_9_intel.macosx_10_9_x86_64.macosx_10_10_intel.macosx_10_10_x86_64.whl", hash = "sha256:6e63814ec71db9bdb42905c925639f319c80e7909fb76c3b84edc79dadef8d60"}, {file = "psycopg2_binary-2.9.5-cp36-cp36m-manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:212757ffcecb3e1a5338d4e6761bf9c04f750e7d027117e74aa3cd8a75bb6fbd"}, {file = "psycopg2_binary-2.9.5-cp36-cp36m-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:6f8a9bcab7b6db2e3dbf65b214dfc795b4c6b3bb3af922901b6a67f7cb47d5f8"}, diff --git a/rero_ils/config.py b/rero_ils/config.py index 559c81d2c3..da389b8cf6 100644 --- a/rero_ils/config.py +++ b/rero_ils/config.py @@ -899,7 +899,7 @@ def _(x): }, list_route='/patrons/', record_loaders={ - 'application/json': lambda: Patron.load(request.get_json()), + 'application/json': 'rero_ils.modules.patrons.loaders:json_v1' }, record_class='rero_ils.modules.patrons.api:Patron', item_route='/patrons/', diff --git a/rero_ils/modules/api.py b/rero_ils/modules/api.py index 002d74a088..8bc1743933 100644 --- a/rero_ils/modules/api.py +++ b/rero_ils/modules/api.py @@ -198,14 +198,6 @@ def _validate(self, **kwargs): return self json = super()._validate(**kwargs) - - # Check if some record extensions has a validation method. - # DEV NOTES :: Could be part of `invenio-record.extensions` - for extension in self._extensions: - validate_method = getattr(extension, 'validate', None) - if callable(validate_method): - extension.validate(self, **kwargs) - validation_message = self.extended_validation(**kwargs) # We only like to run pids_exist_check if validation_message is True # and not a string with error from extended_validation diff --git a/rero_ils/modules/commons/models.py b/rero_ils/modules/commons/models.py new file mode 100644 index 0000000000..1709206c3d --- /dev/null +++ b/rero_ils/modules/commons/models.py @@ -0,0 +1,26 @@ +# -*- coding: utf-8 -*- +# +# RERO ILS +# Copyright (C) 2019-2023 RERO +# Copyright (C) 2019-2023 UCLouvain +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, version 3 of the License. +# +# This program 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 Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + +"""Common models used into RERO-ILS project.""" + + +class NoteTypes: + """List of note type.""" + + PUBLIC_NOTE = 'public_note' + STAFF_NOTE = 'staff_note' diff --git a/rero_ils/modules/contributions/sync.py b/rero_ils/modules/contributions/sync.py index ac6adb315d..0543f796e7 100644 --- a/rero_ils/modules/contributions/sync.py +++ b/rero_ils/modules/contributions/sync.py @@ -389,14 +389,13 @@ def sync_record(self, pid): f'updated') if not self.dry_run: if old_mef_pid == new_mef_pid: - Contribution.get_record_by_id( - agent.id).replace( - new_mef_data, dbcommit=True, reindex=True) + Contribution.get_record(agent.id).replace( + new_mef_data, dbcommit=True, reindex=True) else: # as we have only the last mef but not the old one # we need get it from the MEF server - # this is important as it can still used by other - # agents + # this is important as it can still be used by + # other agents Contribution.get_record_by_pid( pid).update_online(dbcommit=True, reindex=True) updated = True diff --git a/rero_ils/modules/documents/templates/rero_ils/detailed_view_documents.html b/rero_ils/modules/documents/templates/rero_ils/detailed_view_documents.html index d87a84c323..7ade198e6f 100644 --- a/rero_ils/modules/documents/templates/rero_ils/detailed_view_documents.html +++ b/rero_ils/modules/documents/templates/rero_ils/detailed_view_documents.html @@ -19,6 +19,11 @@ {%- extends 'rero_ils/page.html' %} {% from 'rero_ils/macros/macro.html' import div, dict_values, div_list, dl, dl_row, dl_dict, dl_list, div_json %} +{%- block css %} + {{ super() }} + {{ node_assets('@rero/rero-ils-ui/dist/public-holdings-items', ['styles.*css'], 'css') }} +{%- endblock css %} + {%- block body %}
{{ _('Back') }} diff --git a/rero_ils/modules/notifications/api.py b/rero_ils/modules/notifications/api.py index 74ac1e73a4..b7053df472 100644 --- a/rero_ils/modules/notifications/api.py +++ b/rero_ils/modules/notifications/api.py @@ -227,7 +227,7 @@ def patron_transactions(self): .filter('term', notification__pid=self.pid)\ .source(False).scan() for result in results: - yield PatronTransaction.get_record_by_id(result.meta.id) + yield PatronTransaction.get_record(result.meta.id) # CLASS METHODS =========================================================== def update_process_date(self, sent=False, status=NotificationStatus.DONE): diff --git a/rero_ils/modules/patrons/api.py b/rero_ils/modules/patrons/api.py index 05b9ce6275..175cab6b3e 100644 --- a/rero_ils/modules/patrons/api.py +++ b/rero_ils/modules/patrons/api.py @@ -49,8 +49,7 @@ get_patron_from_arguments, get_ref_for_pid, sorted_pids from rero_ils.utils import create_user_from_data -from .extensions import PatronRoleManagementValidatorExtension, \ - UserDataExtension +from .extensions import UserDataExtension from .models import CommunicationChannel, PatronIdentifier, PatronMetadata from .utils import get_patron_pid_by_email @@ -117,10 +116,10 @@ class Patron(IlsRecord): fetcher = patron_id_fetcher provider = PatronProvider model_cls = PatronMetadata + schema = 'patrons/patron-v0.0.1.json' _extensions = [ - UserDataExtension(), - PatronRoleManagementValidatorExtension() + UserDataExtension() ] # ========================================================================= @@ -500,11 +499,6 @@ def set_keep_history(self, keep_history, dbcommit=True, reindex=True): # ========================================================================= # CLASS METHODS # ========================================================================= - @classmethod - def load(cls, data): - """Load the data and remove the user data.""" - return cls(cls.remove_user_data(data)) - @classmethod def remove_user_data(cls, data): """Remove the user data.""" diff --git a/rero_ils/modules/patrons/extensions.py b/rero_ils/modules/patrons/extensions.py index 1d27577ba6..0e1f029150 100644 --- a/rero_ils/modules/patrons/extensions.py +++ b/rero_ils/modules/patrons/extensions.py @@ -17,10 +17,7 @@ # along with this program. If not, see . """Patron record extensions.""" -from flask import current_app -from flask_login import current_user from invenio_records.extensions import RecordExtension -from jsonschema.exceptions import ValidationError from rero_ils.modules.users.api import User @@ -36,51 +33,6 @@ def pre_dump(self, record, data, dumper=None): :param dumper: Dumper to use when dumping the record. :return the future dumped data. """ - user = User.get_by_id(record.get('user_id')) + user = User.get_record(record.get('user_id')) user_info = user.dumps_metadata() return data.update(user_info) - - -class PatronRoleManagementValidatorExtension(RecordExtension): - """Check about patron role management.""" - - def _validate(self, record, **kwargs): - """Validate changes on the record. - - :param record: the record containing data to validate. - :param **kwargs: any other named arguments. - :raises ValidationError: If an error is detected during the validation - check. This error could be serialized to get the error message. - """ - # First, determine if a user is connected. If not, no check must be - # done about role management (probably it's a console script/user). - if not current_user: - return - - # Now determine the roles difference between original record and - # the record that is updated. If no changes are detected, no more - # check will be done - original_record = record.db_record() or {} - record_roles = set(record.get('roles', [])) - original_roles = set(original_record.get('roles', [])) - role_changes = original_roles.symmetric_difference(record_roles) - if not role_changes: - return - - # Depending on the current logged user roles, determine which roles - # this user can manage reading the configuration setting. If any role - # from `role_changes` are not present in manageable role, an error - # should be raised. - key_config = 'RERO_ILS_PATRON_ROLES_MANAGEMENT_RESTRICTIONS' - config_roles = current_app.config.get(key_config, {}) - manageable_roles = set() - for role in current_user.roles: - manageable_roles = manageable_roles.union( - config_roles.get(role.name, {})) - # If any difference are found between both sets, disallow the operation - if role_diffs := role_changes.difference(manageable_roles): - error_roles = ', '.join(role_diffs) - raise ValidationError(f'Unable to manage role(s) : {error_roles}') - - validate = _validate - pre_delete = _validate diff --git a/rero_ils/modules/patrons/loaders/__init__.py b/rero_ils/modules/patrons/loaders/__init__.py new file mode 100644 index 0000000000..73918bf91a --- /dev/null +++ b/rero_ils/modules/patrons/loaders/__init__.py @@ -0,0 +1,29 @@ +# -*- coding: utf-8 -*- +# +# RERO ILS +# Copyright (C) 2019-2023 RERO +# Copyright (C) 2019-2023 UCLouvain +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, version 3 of the License. +# +# This program 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 Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + +"""Loaders for `Patron` resources.""" + +from invenio_records_rest.loaders.marshmallow import marshmallow_loader + +from ..schemas.json import PatronMetadataSchemaV1 + +json_v1 = marshmallow_loader(PatronMetadataSchemaV1) + +__all__ = ( + 'json_v1', +) diff --git a/rero_ils/modules/patrons/permissions.py b/rero_ils/modules/patrons/permissions.py index 37224c80eb..2ce3c1cb74 100644 --- a/rero_ils/modules/patrons/permissions.py +++ b/rero_ils/modules/patrons/permissions.py @@ -18,7 +18,9 @@ """Permissions for patrons.""" from flask import g -from invenio_access import action_factory +from flask_login import current_user +from invenio_access import action_factory, any_user +from invenio_records_permissions.generators import Generator from rero_ils.modules.permissions import AllowedByAction, \ AllowedByActionRestrictByOrganisation, \ @@ -27,6 +29,7 @@ from rero_ils.modules.users.models import UserRole from .api import Patron, current_librarian +from .utils import validate_role_changes # Actions to control patron permission policy search_action = action_factory('ptrn-search') @@ -67,6 +70,27 @@ def needs(self, record=None, *args, **kwargs): return super().needs(record, *args, **kwargs) +class RestrictDeleteDependOnPatronRolesManagement(Generator): + """Restrict deletion of patron depending on role management restrictions. + + Depending on the current logged user profile, some roles' management + updates could be disallowed. Manageable roles are defined into the + `RERO_ILS_PATRON_ROLES_MANAGEMENT_RESTRICTIONS` configuration attribute. + """ + + def excludes(self, record=None, **kwargs): + """Disallow operation check. + + :param record; the record to check. + :param kwargs: extra named arguments. + :returns: a list of Needs to disable access. + """ + roles = set(record.get('roles', [])) + if not validate_role_changes(current_user, roles, raise_exc=False): + return [any_user] + return [] + + class PatronPermissionPolicy(RecordPermissionPolicy): """Patron Permission Policy used by the CRUD operations.""" @@ -82,7 +106,8 @@ class PatronPermissionPolicy(RecordPermissionPolicy): AllowedByActionRestrictStaffByManageableLibrary(update_action) ] can_delete = [ - AllowedByActionRestrictStaffByManageableLibrary(delete_action) + AllowedByActionRestrictStaffByManageableLibrary(delete_action), + RestrictDeleteDependOnPatronRolesManagement() ] diff --git a/rero_ils/modules/patrons/schemas/__init__.py b/rero_ils/modules/patrons/schemas/__init__.py new file mode 100644 index 0000000000..8216aed9c8 --- /dev/null +++ b/rero_ils/modules/patrons/schemas/__init__.py @@ -0,0 +1,19 @@ +# -*- coding: utf-8 -*- +# +# RERO ILS +# Copyright (C) 2019-2023 RERO +# Copyright (C) 2019-2023 UCLouvain +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, version 3 of the License. +# +# This program 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 Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + +"""Patrons schemas.""" diff --git a/rero_ils/modules/patrons/schemas/json.py b/rero_ils/modules/patrons/schemas/json.py new file mode 100644 index 0000000000..32765284f6 --- /dev/null +++ b/rero_ils/modules/patrons/schemas/json.py @@ -0,0 +1,136 @@ +# -*- coding: utf-8 -*- +# +# RERO ILS +# Copyright (C) 2019-2023 RERO +# Copyright (C) 2019-2023 UCLouvain +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, version 3 of the License. +# +# This program 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 Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + +"""Marshmallow schema for JSON representation of `Patron` resources.""" +from functools import partial + +from flask import abort +from flask_login import current_user +from invenio_records_rest.schemas import StrictKeysMixin +from invenio_records_rest.schemas.fields import GenFunction, SanitizedUnicode +from marshmallow import Schema, fields, pre_load, validates, validates_schema +from marshmallow.validate import OneOf + +from rero_ils.modules.commons.models import NoteTypes +from rero_ils.modules.commons.schemas import NoteSchema, RefSchema, \ + http_applicable_method +from rero_ils.modules.serializers.base import schema_from_context +from rero_ils.modules.users.api import User +from rero_ils.modules.users.models import UserRole + +from ..api import Patron +from ..utils import validate_role_changes + +schema_from_template = partial(schema_from_context, schema=Patron.schema) + + +class PatronAddressSchema(Schema): + """Marshmallow schema for patron address.""" + + street = SanitizedUnicode() + postal_code = SanitizedUnicode() + city = SanitizedUnicode() + country = SanitizedUnicode() + + +class PatronNoteSchema(NoteSchema): + """Marshmallow schema for `notes` on Patron.""" + + ttype = SanitizedUnicode( + validate=OneOf([NoteTypes.PUBLIC_NOTE, NoteTypes.STAFF_NOTE]) + ) + + +class PatronMetadataSchemaV1(StrictKeysMixin): + """Marshmallow schema for the `Template` metadata.""" + + pid = SanitizedUnicode() + schema = GenFunction( + load_only=True, + attribute='$schema', + data_key='$schema', + deserialize=schema_from_template + ) + source = SanitizedUnicode() + local_codes = fields.List(SanitizedUnicode()) + user_id = fields.Integer() + second_address = fields.Nested(PatronAddressSchema()) + patron = fields.Dict() # TODO : stricter implementation + libraries = fields.Nested(RefSchema, many=True) + roles = fields.List(SanitizedUnicode(validate=OneOf(UserRole.ALL_ROLES))) + notes = fields.Nested(PatronNoteSchema, many=True) + + @pre_load + def remove_user_data(self, data, many, **kwargs): + """Removed data concerning User not Patron. + + :param data: the data received from request. + :param many: is the `data` represent an array of schema object. + :param kwargs: any additional named arguments. + :return Data cleared from user profile information. + """ + data = data if many else [data] + profile_fields = set(User.profile_fields + ['username', 'email']) + for record in data: + for field in profile_fields: + record.pop(field, None) + return data if many else data[0] + + @validates('roles') + @http_applicable_method('POST') + def validate_role(self, data, **kwargs): + """Validate `roles` attribute through API request. + + The `roles` attribute must be controlled to restrict some role + attribution/modification depending on the current logged user. + + :param data: the `roles` attribute value. + :param kwargs: any additional named arguments. + :raises ValidationError: if error has detected on `roles` attribute + """ + validate_role_changes(current_user, set(data)) + + @validates_schema + @http_applicable_method('PUT') + def validate_roles_changes(self, data, **kwargs): + """Validate `roles` changes through REST API request. + + Updates on `roles` attribute is subject to restriction depending + on current connected user. Determine if `roles` field changes and if + these changes are allowed or not. + + :param data: the json data to validate. + :param kwargs: additional named arguments. + :raises abort: if corresponding record is not found. + :raises ValidationError: if error has detected on role field + """ + # Load DB record + db_record = Patron.get_record_by_pid(data.get('pid')) + if not db_record: + abort(404) + + # Check if `roles` of the patron changed. If not, we can stop + # the validation process. + original_roles = set(db_record.get('roles', [])) + data_roles = set(data.get('roles', [])) + role_changes = original_roles.symmetric_difference(data_roles) + if not role_changes: + return + + # `roles` field changes, we need to validate this change. + validate_role_changes(current_user, role_changes) diff --git a/rero_ils/modules/patrons/utils.py b/rero_ils/modules/patrons/utils.py index 41e3ee7f26..613959ab31 100644 --- a/rero_ils/modules/patrons/utils.py +++ b/rero_ils/modules/patrons/utils.py @@ -16,9 +16,9 @@ # along with this program. If not, see . """Utilities functions for patrons.""" - - +from flask import current_app from flask_login import current_user +from marshmallow import ValidationError def user_has_patron(user=current_user): @@ -40,15 +40,31 @@ def get_patron_pid_by_email(email): return hit.pid -def clean_record_data(data=None): - """Clean patron data. +def validate_role_changes(user, changes, raise_exc=True): + """Validate `roles` changed depending on application configuration. - :param data: data representing patron as dictionary. - :return: cleaned data + :param user: the user requesting patron role changes. + :param changes: the list of all role changes. + :param raise_exc: Is the function must raise an exception or not. + :return False if validation failed, True otherwise + :raise ValidationError: if error has detected. """ - if data and data.get('barcode'): - data['barcode'] = [d.strip() for d in data.get('barcode')] - if data and data.get('patron', {}).get('barcode'): - data['patron']['barcode'] = [ - d.strip() for d in data['patron']['barcode']] - return data + # Depending on the current logged user roles, determine which roles + # this user can manage reading the configuration setting. If any role + # from `role_changes` are not present in manageable role, an error + # should be raised. + key_config = 'RERO_ILS_PATRON_ROLES_MANAGEMENT_RESTRICTIONS' + config_roles = current_app.config.get(key_config, {}) + manageable_roles = set() + for role in user.roles: + manageable_roles = manageable_roles.union( + config_roles.get(role.name, {})) + # If any difference are found between both sets, disallow the operation + if role_diffs := changes.difference(manageable_roles): + if raise_exc: + error_roles = ', '.join(role_diffs) + raise ValidationError(f'Unable to manage role(s): {error_roles}') + else: + return False + # No problems were detected + return True diff --git a/rero_ils/modules/patrons/views.py b/rero_ils/modules/patrons/views.py index ac0dec3550..54fe17d702 100644 --- a/rero_ils/modules/patrons/views.py +++ b/rero_ils/modules/patrons/views.py @@ -145,7 +145,7 @@ def logged_user(): if not current_user.is_authenticated: return jsonify(data) - user = User.get_by_id(current_user.id).dumps_metadata() + user = User.get_record(current_user.id).dumps_metadata() user['id'] = current_user.id data = {**data, **user, 'patrons': []} for patron in Patron.get_patrons_by_user(current_user): diff --git a/rero_ils/modules/templates/loaders/__init__.py b/rero_ils/modules/templates/loaders/__init__.py index 570bc42343..974ba575cc 100644 --- a/rero_ils/modules/templates/loaders/__init__.py +++ b/rero_ils/modules/templates/loaders/__init__.py @@ -1,8 +1,8 @@ # -*- coding: utf-8 -*- # # RERO ILS -# Copyright (C) 2019-2022 RERO -# Copyright (C) 2019-2022 UCLouvain +# Copyright (C) 2019-2023 RERO +# Copyright (C) 2019-2023 UCLouvain # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU Affero General Public License as published by @@ -17,6 +17,7 @@ # along with this program. If not, see . """Loaders for `Template` resources.""" + from invenio_records_rest.loaders.marshmallow import marshmallow_loader from ..schemas.json import TemplateMetadataSchemaV1 diff --git a/rero_ils/modules/templates/schemas/__init__.py b/rero_ils/modules/templates/schemas/__init__.py index 08c10c0f42..7f87fa2b58 100644 --- a/rero_ils/modules/templates/schemas/__init__.py +++ b/rero_ils/modules/templates/schemas/__init__.py @@ -1,8 +1,8 @@ # -*- coding: utf-8 -*- # # RERO ILS -# Copyright (C) 2019-2022 RERO -# Copyright (C) 2019-2022 UCLouvain +# Copyright (C) 2019-2023 RERO +# Copyright (C) 2019-2023 UCLouvain # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU Affero General Public License as published by diff --git a/rero_ils/modules/templates/schemas/json.py b/rero_ils/modules/templates/schemas/json.py index a978873f72..92849a1bec 100644 --- a/rero_ils/modules/templates/schemas/json.py +++ b/rero_ils/modules/templates/schemas/json.py @@ -16,7 +16,7 @@ # You should have received a copy of the GNU Affero General Public License # along with this program. If not, see . -"""Schema for JSON representation of `Template` resources.""" +"""Marshmallow schema for JSON representation of `Template` resources.""" from functools import partial from flask_babelex import gettext as _ diff --git a/rero_ils/modules/users/api.py b/rero_ils/modules/users/api.py index 2cc7135f5f..fd63f4c6dc 100644 --- a/rero_ils/modules/users/api.py +++ b/rero_ils/modules/users/api.py @@ -195,7 +195,7 @@ def _validate_password(cls, password): raise ValidationError(str(e)) from e @classmethod - def get_by_id(cls, user_id): + def get_record(cls, user_id): """Get a user by a user_id. :param user_id - the user_id diff --git a/rero_ils/modules/users/views.py b/rero_ils/modules/users/views.py index 1e4423d023..bef61291df 100644 --- a/rero_ils/modules/users/views.py +++ b/rero_ils/modules/users/views.py @@ -124,13 +124,13 @@ def __init__(self, **kwargs): @check_user_permission def get(self, id): """Implement the GET.""" - user = User.get_by_id(id) + user = User.get_record(id) return user.dumps() @check_user_permission def put(self, id): """Implement the PUT.""" - user = User.get_by_id(id) + user = User.get_record(id) user = user.update(request.get_json()) editing_own_public_profile = str(current_user.id) == id and \ not ( diff --git a/tests/api/patrons/test_patrons_marshmallow.py b/tests/api/patrons/test_patrons_marshmallow.py new file mode 100644 index 0000000000..2656fab1f1 --- /dev/null +++ b/tests/api/patrons/test_patrons_marshmallow.py @@ -0,0 +1,111 @@ +# -*- coding: utf-8 -*- +# +# RERO ILS +# Copyright (C) 2019-2023 RERO +# Copyright (C) 2019-2023 UCLouvain +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as published by +# the Free Software Foundation, version 3 of the License. +# +# This program 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 Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + +"""Tests Marshmallow schema through REST API for Patrons.""" +import json + +from flask import url_for +from invenio_accounts.testutils import login_user_via_session +from utils import postdata + +from rero_ils.modules.patrons.api import Patron +from rero_ils.modules.users.models import UserRole +from rero_ils.utils import create_user_from_data + + +def test_patrons_marshmallow_loaders( + client, librarian_martigny, system_librarian_martigny_data_tmp, + json_header +): + """Test marshmallow schema/restrictions for Patron resources.""" + + # TEST#1 :: Use console to manage patron/role + # Using 'console' commands, no matter connected user, all operations are + # allowed on a Patron, even changes any roles. + user_data = create_user_from_data(system_librarian_martigny_data_tmp) + patron = Patron.create(user_data, dbcommit=True, reindex=True) + assert patron and patron['roles'] == [UserRole.FULL_PERMISSIONS] + + roles = [UserRole.ACQUISITION_MANAGER, UserRole.CATALOG_MANAGER] + patron['roles'] = roles + patron = patron.update(patron, dbcommit=True, reindex=True) + patron = Patron.get_record_by_pid(patron.pid) + assert patron['roles'] == roles + + patron.delete(dbcommit=True, delindex=True) + patron = Patron.get_record_by_pid(patron.pid) + assert not patron + + # TEST#2 :: Use API to create patron. + # Through the API, the `role` field is controlled by marshmallow + # validation process. A simple staff member isn't allowed to control all + # roles. + login_user_via_session(client, librarian_martigny.user) + + user_data['roles'] = [UserRole.FULL_PERMISSIONS] + user_data = create_user_from_data(system_librarian_martigny_data_tmp) + del (user_data['pid']) + + # Step 1 :: Send POST API to create user with bad roles --> 400 + # Should fail because the current logged user doesn't have authorization + # to deal with `roles` of the user data. + res, response_data = postdata( + client, 'invenio_records_rest.ptrn_list', user_data) + assert res.status_code == 400 + assert 'Validation error' in response_data['message'] + + # Step 2 :: Send POST API to create user with correct roles --> 201 + # Update user data with correct `roles` values and create the + # user. + original_roles = librarian_martigny['roles'] + librarian_martigny['roles'] = [UserRole.LIBRARY_ADMINISTRATOR] + librarian_martigny.update(librarian_martigny, dbcommit=True, reindex=True) + login_user_via_session(client, librarian_martigny.user) + + user_data['roles'] = [UserRole.USER_MANAGER] + res, response_data = postdata( + client, 'invenio_records_rest.ptrn_list', user_data) + assert res.status_code == 201 + pid = response_data['metadata']['pid'] + + # Step 3 :: Send PUT API to update roles --> 400 + # Try to update the created patron to add it some unauthorized roles + item_url = url_for('invenio_records_rest.ptrn_item', pid_value=pid) + user_data['pid'] = pid + user_data['roles'] = [UserRole.FULL_PERMISSIONS] + res = client.put(item_url, data=json.dumps(user_data), headers=json_header) + assert res.status_code == 400 + assert 'Validation error' in res.json['message'] + + # Step 4 :: Update the patron using console + # Force the patron update using console + patron = Patron.get_record_by_pid(pid) + patron['roles'] = [UserRole.FULL_PERMISSIONS] + patron.update(patron, dbcommit=True, reindex=True) + patron = Patron.get_record_by_pid(pid) + assert patron['roles'] == [UserRole.FULL_PERMISSIONS] + + # Step 5 :: Delete patron through API + # This should be disallowed due to role management restrictions. + res = client.delete(item_url) + assert res.status_code == 403 + + # Reset the fixtures + patron.delete(True, True, True) + librarian_martigny['roles'] = original_roles + librarian_martigny.update(librarian_martigny, dbcommit=True, reindex=True) diff --git a/tests/api/patrons/test_patrons_permissions.py b/tests/api/patrons/test_patrons_permissions.py index a0fe140523..3d6ac226c9 100644 --- a/tests/api/patrons/test_patrons_permissions.py +++ b/tests/api/patrons/test_patrons_permissions.py @@ -122,7 +122,7 @@ def test_patrons_permissions( 'read': True, 'create': True, 'update': True, - 'delete': True + 'delete': False # simple librarian cannot delete other librarian }, librarian2_martigny) check_permission(PatronPermissionPolicy, { 'read': True, diff --git a/tests/api/selfcheck/test_selfcheck.py b/tests/api/selfcheck/test_selfcheck.py index 75cfe5c095..54587c1ee8 100644 --- a/tests/api/selfcheck/test_selfcheck.py +++ b/tests/api/selfcheck/test_selfcheck.py @@ -90,7 +90,7 @@ def test_authorize_patron(selfcheck_patron_martigny, default_user_password): assert response # authorize patron without email (using username for authentication) - user = User.get_by_id(selfcheck_patron_martigny.user.id) + user = User.get_record(selfcheck_patron_martigny.user.id) user_metadata = user.dumps_metadata() email = user_metadata.pop('email', None) user.update(user_metadata) @@ -102,7 +102,7 @@ def test_authorize_patron(selfcheck_patron_martigny, default_user_password): assert response # reset user data - user = User.get_by_id(selfcheck_patron_martigny.user.id) + user = User.get_record(selfcheck_patron_martigny.user.id) user_metadata = user.dumps_metadata() user_metadata['email'] = email user.update(user_metadata) diff --git a/tests/api/users/test_users_profile_updates.py b/tests/api/users/test_users_profile_updates.py index fa016c166d..c711f369fa 100644 --- a/tests/api/users/test_users_profile_updates.py +++ b/tests/api/users/test_users_profile_updates.py @@ -40,7 +40,7 @@ def test_user_profile_updates( login_user_via_session(client, patron_martigny.user) # mailbox is empty assert not (len(mailbox)) - user_metadata = User.get_by_id(patron_martigny.user.id).dumps_metadata() + user_metadata = User.get_record(patron_martigny.user.id).dumps_metadata() # changing the email by another does not send any reset_password # notification user_metadata['email'] = 'toto@toto.com' diff --git a/tests/ui/patrons/test_patrons_api.py b/tests/ui/patrons/test_patrons_api.py index 263cbf6f3c..5fe56fae2c 100644 --- a/tests/ui/patrons/test_patrons_api.py +++ b/tests/ui/patrons/test_patrons_api.py @@ -21,24 +21,18 @@ from copy import deepcopy -import mock import pytest from invenio_accounts.models import User from invenio_userprofiles import UserProfile from jsonschema.exceptions import ValidationError -from utils import role_managment_validate_mock from rero_ils.modules.patrons.api import Patron, PatronsSearch, \ patron_id_fetcher -from rero_ils.modules.patrons.extensions import \ - PatronRoleManagementValidatorExtension from rero_ils.modules.patrons.models import CommunicationChannel from rero_ils.modules.users.models import UserRole from rero_ils.utils import create_user_from_data -@mock.patch.object(PatronRoleManagementValidatorExtension, - 'validate', role_managment_validate_mock) def test_patron_create(app, roles, lib_martigny, librarian_martigny_data_tmp, patron_type_adults_martigny, mailbox): """Test Patron creation.""" @@ -339,8 +333,6 @@ def test_get_patron_for_organisation( assert list(pids) -@mock.patch.object(PatronRoleManagementValidatorExtension, - 'validate', role_managment_validate_mock) def test_patron_multiple(patron_sion_multiple, patron2_martigny, lib_martigny): """Test changing roles for multiple patron accounts.""" assert patron2_martigny.user == patron_sion_multiple.user diff --git a/tests/ui/test_api.py b/tests/ui/test_api.py index 911390cfb1..6ee5514a06 100644 --- a/tests/ui/test_api.py +++ b/tests/ui/test_api.py @@ -117,6 +117,8 @@ def test_ilsrecord(app, es_default_index, ils_record, ils_record_2): ) assert record_1.pid == 'ilsrecord_pid' assert record_1.id == RecordTest.get_id_by_pid(record_1.pid) + record_alias = record_1.db_record() + assert record_1.pid == record_alias.pid record_2 = RecordTest.create( data=ils_record_2, @@ -143,6 +145,12 @@ def test_ilsrecord(app, es_default_index, ils_record, ils_record_2): assert sorted(RecordTest.get_all_pids()) == [ '1', 'ilsrecord_pid', 'ilsrecord_pid_2' ] + assert len(list(RecordTest.get_all_pids(limit=None))) == 3 + assert len(list(RecordTest.get_all_ids(limit=None))) == 3 + + assert RecordTest.get_id_by_pid(record_created_pid.pid) == \ + record_created_pid.id + assert not RecordTest.get_record_by_pid('dummy') """Test IlsRecord update.""" record = RecordTest.get_record_by_pid('ilsrecord_pid') diff --git a/tests/utils.py b/tests/utils.py index 7fd26ef7f5..e0d2819011 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -60,11 +60,6 @@ class VerifyRecordPermissionPatch(object): status_code = 200 -def role_managment_validate_mock(self, record, **kwargs): - """Mock method to validate Patron role management extensions.""" - return None - - def check_permission(permission_policy, actions, record): """Check permission.