permissions: allow read access to holding and items for all users

* Adds possibilities to access loan API for users of same organisation.
* Restricts patron API loan search to his own loans.
* Adds organisation to the loan schema.
* Permits API access to holdings and items for all type of users.
* Changes Loan route from /circulation/loans to /loans.
* Sets loan API search sort order to loan transaction_date.

Co-Authored-by: Aly Badr <aly.badr@rero.ch>

rero_ils/config.py

@@ -55,6 +55,8 @@
 from .modules.libraries.permissions import can_create_library_factory, \
     can_delete_library_factory, can_update_library_factory
 from .modules.loans.api import Loan
+from .modules.loans.permissions import can_list_loan_factory, \
+    can_read_loan_factory
 from .modules.loans.utils import can_be_requested, get_default_loan_duration, \
     get_extension_params, is_item_available_for_checkout, \
     loan_satisfy_circ_policies
@@ -398,7 +400,8 @@ def _(x):
         default_media_type='application/json',
         max_result_window=10000,
         search_factory_imp='rero_ils.query:organisation_search_factory',
-        read_permission_factory_imp=can_access_organisation_records_factory,
+        read_permission_factory_imp=allow_all,
+        list_permission_factory_imp=allow_all,
         create_permission_factory_imp=can_create_item_factory,
         update_permission_factory_imp=can_update_delete_item_factory,
         delete_permission_factory_imp=can_update_delete_item_factory,
@@ -462,7 +465,8 @@ def _(x):
         default_media_type='application/json',
         max_result_window=10000,
         search_factory_imp='rero_ils.query:organisation_search_factory',
-        read_permission_factory_imp=can_access_organisation_records_factory,
+        read_permission_factory_imp=allow_all,
+        list_permission_factory_imp=allow_all,
         create_permission_factory_imp=can_create_organisation_records_factory,
         update_permission_factory_imp=can_update_organisation_records_factory,
         delete_permission_factory_imp=can_delete_organisation_records_factory,
@@ -908,7 +912,8 @@ def _(x):
     'libraries',
     'locations',
     'persons',
-    'circ_policies'
+    'circ_policies',
+    'loans'
 ]
 
 RECORDS_REST_SORT_OPTIONS = dict()
@@ -931,6 +936,12 @@ def _(x):
     RECORDS_REST_DEFAULT_SORT[index] = dict(
         query='bestmatch', noquery='mostrecent')
 
+RECORDS_REST_SORT_OPTIONS['loans'] = dict(
+    transactiondate=dict(
+        fields=['-transaction_date'], title='Transaction date',
+        default_order='asc'
+    )
+)
 
 # Detailed View Configuration
 # ===========================
@@ -1105,7 +1116,6 @@ def _(x):
         pid_fetcher=CIRCULATION_LOAN_FETCHER,
         search_class=LoansSearch,
         search_type=None,
-        record_class=Loan,
         record_serializers={
             'application/json': ('invenio_records_rest.serializers'
                                  ':json_v1_response'),
@@ -1114,14 +1124,19 @@ def _(x):
             'application/json': ('invenio_records_rest.serializers'
                                  ':json_v1_search'),
         },
-        list_route='/circulation/loans/',
-        item_route='/circulation/loans/<{0}:pid_value>'.format(
+        record_loaders={
+            'application/json': lambda: Loan(request.get_json()),
+        },
+        record_class='rero_ils.modules.loans.api:Loan',
+        search_factory_imp='rero_ils.query:loans_search_factory',
+        list_route='/loans/',
+        item_route='/loans/<{0}:pid_value>'.format(
             _LOANID_CONVERTER),
         default_media_type='application/json',
         max_result_window=10000,
         error_handlers=dict(),
-        read_permission_factory_imp=deny_all,
-        list_permission_factory_imp=deny_all,
+        read_permission_factory_imp=can_read_loan_factory,
+        list_permission_factory_imp=can_list_loan_factory,
         create_permission_factory_imp=deny_all,
         update_permission_factory_imp=deny_all,
         delete_permission_factory_imp=deny_all

rero_ils/modules/documents/jsonschemas/documents/document-v0.0.1.json

@@ -2318,4 +2318,4 @@
       "default": false
     }
   }
-}
+}

rero_ils/modules/loans/api.py

@@ -73,6 +73,7 @@ def create(cls, data, id_=None, delete_pid=True,
         data['$schema'] = current_jsonschemas.path_to_url(cls._schema)
         if delete_pid and data.get(cls.pid_field):
             del(data[cls.pid_field])
+        cls._loan_build_org_ref(data)
         record = super(Loan, cls).create(
             data=data, id_=id_, delete_pid=delete_pid, dbcommit=dbcommit,
             reindex=reindex, **kwargs)
@@ -99,6 +100,22 @@ def loan_build_item_ref(self, item_pid):
                 pid=item_pid)
         }
 
+    @classmethod
+    def _loan_build_org_ref(cls, data):
+        """Build $ref for the organisation of the Loan."""
+        from ..items.api import Item
+        item_pid = data.get('item_pid')
+        org_pid = Item.get_record_by_pid(item_pid).organisation_pid
+        base_url = current_app.config.get('RERO_ILS_APP_BASE_URL')
+        url_api = '{base_url}/api/{doc_type}/{pid}'
+        org_ref = {
+            '$ref': url_api.format(
+                base_url=base_url,
+                doc_type='organisations',
+                pid=org_pid)
+        }
+        data['organisation'] = org_ref
+
     @property
     def pid(self):
         """Shortcut for pid."""

rero_ils/modules/loans/jsonschemas/loans/loan-ils-v0.0.1.json

@@ -77,6 +77,17 @@
       "format": "date-time",
       "title": "Transaction end date"
     },
+    "organisation": {
+      "title": "Organisation",
+      "type": "object",
+      "properties": {
+        "$ref": {
+          "title": "Organisation URI",
+          "type": "string",
+          "pattern": "^https://ils.rero.ch/api/organisations/.*?$"
+        }
+      }
+    },
     "state": {
       "type": "string",
       "enum": [

rero_ils/modules/loans/mappings/v6/loans/loan-ils-v0.0.1.json

@@ -58,6 +58,13 @@
         "transaction_user_pid": {
           "type": "keyword"
         },
+        "organisation": {
+          "properties": {
+            "pid": {
+              "type": "keyword"
+            }
+          }
+        },
         "trigger": {
           "type": "keyword"
         },

rero_ils/modules/loans/permissions.py

@@ -0,0 +1,53 @@
+# -*- coding: utf-8 -*-
+#
+# RERO ILS
+# Copyright (C) 2019 RERO
+#
+# 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 <http://www.gnu.org/licenses/>.
+
+"""Loan permissions."""
+
+
+from .api import Loan
+from ...permissions import patron_is_authenticated, staffer_is_authenticated, \
+    user_is_authenticated
+
+
+def can_list_loan_factory(record, *args, **kwargs):
+    """Checks if the logged user have access to loans list.
+
+    only authenticated users can place a search on loans.
+    """
+    def can(self):
+        patron = user_is_authenticated()
+        if patron:
+            return True
+        return False
+    return type('Check', (), {'can': can})()
+
+
+def can_read_loan_factory(record, *args, **kwargs):
+    """Checks if the logged user have access to loans of its organisation.
+
+    users with librarian or system_librarian roles can acess all loans.
+    users with patron role can access only its loans.
+    """
+    def can(self):
+        patron = staffer_is_authenticated() or patron_is_authenticated()
+        if patron and patron.organisation_pid == Loan(record).organisation_pid:
+            if patron.is_librarian or patron.is_system_librarian:
+                return True
+            elif patron.is_patron and Loan(record).patron_pid == patron.pid:
+                return True
+        return False
+    return type('Check', (), {'can': can})()

rero_ils/permissions.py

@@ -60,6 +60,22 @@ def staffer_is_authenticated(user=None):
     return None
 
 
+def patron_is_authenticated(user=None):
+    """Checks if user (with role patron) is authenticated.
+
+    returns patron records if user is logged in and authenticated and has
+    the patron role.
+    returns False otherwise.
+    """
+    if not user:
+        user = current_user
+    if user.is_authenticated:
+        patron = Patron.get_patron_by_user(current_user)
+        if patron and patron.is_patron:
+            return patron
+    return None
+
+
 def can_access_organisation_records_factory(record, *args, **kwargs):
     """Checks if the logged user have access to records of its organisation.
 

rero_ils/query.py

@@ -79,6 +79,24 @@ def organisation_search_factory(self, search, query_parser=None):
     return (search, urlkwargs)
 
 
+def loans_search_factory(self, search, query_parser=None):
+    """Loan search factory.
+
+    Restricts results to oraganisation level for librarian and sys_lib.
+    Restricts results to his loans for users with role patron.
+    """
+    search, urlkwargs = search_factory(self, search)
+    if current_patron:
+        if current_patron.is_librarian or current_patron.is_system_librarian:
+            search = search.filter(
+                'term', organisation__pid=current_patron.get_organisation(
+                    )['pid'])
+        if current_patron.is_patron:
+            search = search.filter(
+                'term', patron_pid=current_patron.pid)
+    return (search, urlkwargs)
+
+
 def search_factory(self, search, query_parser=None):
     """Parse query using elasticsearch DSL query.
 

tests/api/test_holdings_rest.py

@@ -34,7 +34,7 @@ def test_holdings_permissions(client, holding_lib_martigny, json_header):
     item_url = url_for('invenio_records_rest.hold_item', pid_value='holding1')
 
     res = client.get(item_url)
-    assert res.status_code == 401
+    assert res.status_code == 200
 
     res, _ = postdata(
         client,
@@ -163,15 +163,14 @@ def test_holding_secure_api(client, json_header, holding_lib_martigny,
                          pid_value=holding_lib_martigny.pid)
 
     res = client.get(record_url)
-    assert res.status_code == 403
+    assert res.status_code == 200
 
 
 def test_holding_secure_api_create(client, json_header, holding_lib_martigny,
                                    librarian_martigny_no_email,
                                    librarian_sion_no_email,
                                    holding_lib_martigny_data):
     """Test holding secure api create."""
-
     # Martigny
     login_user_via_session(client, librarian_martigny_no_email.user)
     post_entrypoint = 'invenio_records_rest.hold_list'

tests/api/test_items_rest.py

@@ -39,7 +39,7 @@ def test_items_permissions(client, item_lib_martigny,
     item_url = url_for('invenio_records_rest.item_item', pid_value='item1')
 
     res = client.get(item_url)
-    assert res.status_code == 401
+    assert res.status_code == 200
 
     res, _ = postdata(
         client,

tests/api/test_items_rest_views.py

@@ -375,7 +375,7 @@ def test_item_secure_api(client, json_header, item_lib_martigny,
                          pid_value=item_lib_martigny.pid)
 
     res = client.get(record_url)
-    assert res.status_code == 403
+    assert res.status_code == 200
 
 
 def test_item_secure_api_create(client, json_header, item_lib_martigny,

tests/api/test_loans_rest.py

@@ -34,8 +34,8 @@
     get_last_transaction_loc_for_item, get_loans_by_patron_pid, \
     get_overdue_loans
 from rero_ils.modules.loans.utils import can_be_requested
-from rero_ils.modules.notifications.api import Notification, \
-    NotificationsSearch, number_of_reminders_sent
+from rero_ils.modules.notifications.api import NotificationsSearch, \
+    number_of_reminders_sent
 
 
 def test_loans_permissions(client, loan_pending_martigny, json_header):
@@ -70,7 +70,7 @@ def test_loans_logged_permissions(client, loan_pending_martigny,
     item_url = url_for('invenio_records_rest.loanid_item', pid_value='1')
 
     res = client.get(item_url)
-    assert res.status_code == 403
+    assert res.status_code == 200
 
     res, _ = postdata(
         client,
@@ -326,3 +326,74 @@ def test_checkout_item_transit(client, item2_lib_martigny,
     loan_after_checkout = get_loan_for_item(item.pid)
     assert loan_after_checkout.get('state') == 'ITEM_ON_LOAN'
     assert loan_before_checkout.get('pid') == loan_after_checkout.get('pid')
+
+
+def test_loan_access_permissions(client, librarian_martigny_no_email,
+                                 patron_martigny_no_email,
+                                 item_lib_sion, patron_sion_no_email,
+                                 librarian_sion_no_email,
+                                 circulation_policies
+                                 ):
+    """Test loans read permissions."""
+    # no access to loans for non authenticated users.
+    loan_list = url_for('invenio_records_rest.loanid_list', q='pid:1')
+    res = client.get(loan_list)
+    assert res.status_code == 401
+
+    # ensure we have loans from the two configured organisation.
+    login_user_via_session(client, librarian_sion_no_email.user)
+    res, _ = postdata(
+        client,
+        'api_item.checkout',
+        dict(
+            item_pid=item_lib_sion.pid,
+            patron_pid=patron_sion_no_email.pid
+        )
+    )
+    assert res.status_code == 200
+
+    loan_pids = Loan.get_all_pids()
+    loans = [Loan.get_record_by_pid(pid) for pid in loan_pids]
+    loans_martigny = [
+        loan for loan in loans if loan.organisation_pid == 'org1']
+    loans_sion = [loan for loan in loans if loan.organisation_pid == 'org2']
+    assert loans
+    assert loan_pids
+    assert loans_martigny
+    assert loans_sion
+    # Test loan list API access.
+    login_user_via_session(client, librarian_martigny_no_email.user)
+    loan_list = url_for('invenio_records_rest.loanid_list', q='pid:1')
+    res = client.get(loan_list)
+    assert res.status_code == 200
+    login_user_via_session(client, patron_martigny_no_email.user)
+    loan_list = url_for('invenio_records_rest.loanid_list', q='pid:1')
+    res = client.get(loan_list)
+    assert res.status_code == 200
+
+    # librarian or system librarian have access all loans of its org
+    user = librarian_martigny_no_email
+    login_user_via_session(client, user.user)
+    for loan in loans:
+        record_url = url_for(
+            'invenio_records_rest.loanid_item', pid_value=loan.pid)
+        res = client.get(record_url)
+        if loan.organisation_pid == user.organisation_pid:
+            assert res.status_code == 200
+        if loan.organisation_pid != user.organisation_pid:
+            assert res.status_code == 403
+
+    # patron can access only its loans
+    user = patron_martigny_no_email
+    login_user_via_session(client, user.user)
+    for loan in loans:
+        record_url = url_for(
+            'invenio_records_rest.loanid_item', pid_value=loan.pid)
+        res = client.get(record_url)
+        if loan.organisation_pid == user.organisation_pid:
+            if loan.patron_pid == user.pid:
+                assert res.status_code == 200
+            else:
+                assert res.status_code == 403
+        if loan.organisation_pid != user.organisation_pid:
+            assert res.status_code == 403