permissions: refactoring organisation permissions

* Refactoring the permissions process using a permission factory and
permission classes for any resource instead of resources shared
functions. This way is the same way used by SONAR & Zenodo project
* Creates permission factory method
* Creates basic permission classes
* Create Organisation permission class
* Fixes unitests

Co-Authored-by: Renaud Michotte <renaud.michotte@gmail.com>

rero_ils/config.py

@@ -72,6 +72,7 @@
     can_update_delete_location_factory
 from .modules.notifications.api import Notification
 from .modules.organisations.api import Organisation
+from .modules.organisations.permissions import OrganisationPermission
 from .modules.patron_transaction_events.api import PatronTransactionEvent
 from .modules.patron_transaction_events.permissions import can_list_patron_transaction_event_factory, \
     can_read_patron_transaction_event_factory
@@ -82,6 +83,7 @@
 from .modules.patrons.api import Patron
 from .modules.patrons.permissions import can_delete_patron_factory, \
     can_update_patron_factory
+from .modules.permissions import record_permission_factory
 from .modules.persons.api import Person
 from .modules.vendors.api import Vendor
 from .permissions import can_access_organisation_patrons_factory, \
@@ -386,6 +388,10 @@ def _(x):
 
 # REST API Configuration
 # ======================
+RERO_ILS_APP_DISABLE_PERMISSION_CHECKS = False
+"""Disable permission checks during API calls. Useful when API is test from
+command line or progams like postman."""
+
 RECORDS_REST_DEFAULT_CREATE_PERMISSION_FACTORY = librarian_permission_factory
 """Default create permission factory: reject any request."""
 
@@ -724,11 +730,16 @@ def _(x):
         max_result_window=10000,
         search_factory_imp=('rero_ils.query:'
                             'organisation_organisation_search_factory'),
-        list_permission_factory_imp=can_access_organisation_patrons_factory,
-        create_permission_factory_imp=deny_all,
-        update_permission_factory_imp=is_system_librarian_organisation_record_factory,
-        delete_permission_factory_imp=deny_all,
-        read_permission_factory_imp=can_access_organisation_records_factory,
+        list_permission_factory_imp=lambda record: record_permission_factory(
+            action='list', record=record, cls=OrganisationPermission),
+        read_permission_factory_imp=lambda record: record_permission_factory(
+            action='read', record=record, cls=OrganisationPermission),
+        create_permission_factory_imp=lambda record: record_permission_factory(
+            action='create', record=record, cls=OrganisationPermission),
+        update_permission_factory_imp=lambda record: record_permission_factory(
+            action='update', record=record, cls=OrganisationPermission),
+        delete_permission_factory_imp=lambda record: record_permission_factory(
+            action='delete',record=record, cls=OrganisationPermission),
     ),
     lib=dict(
         pid_type='lib',

rero_ils/modules/organisations/api.py

@@ -18,9 +18,11 @@
 
 """API for manipulating organisation."""
 
-
 from functools import partial
 
+from flask_login import current_user
+from werkzeug.local import LocalProxy
+
 from .models import OrganisationIdentifier, OrganisationMetadata
 from ..api import IlsRecord, IlsRecordsIndexer, IlsRecordsSearch
 from ..fetchers import id_fetcher
@@ -29,6 +31,9 @@
 from ..minters import id_minter
 from ..providers import Provider
 
+current_organisation = LocalProxy(
+    lambda: Organisation.get_record_by_user(current_user))
+
 # provider
 OrganisationProvider = type(
     'OrganisationProvider',
@@ -44,7 +49,7 @@
 class OrganisationsSearch(IlsRecordsSearch):
     """Organisation search."""
 
-    class Meta():
+    class Meta:
         """Meta class."""
 
         index = 'organisations'
@@ -59,42 +64,6 @@ class Organisation(IlsRecord):
     provider = OrganisationProvider
     model_cls = OrganisationMetadata
 
-    def get_libraries_pids(self):
-        """Get all libraries pids related to the organisation."""
-        results = LibrariesSearch().source(['pid'])\
-            .filter('term', organisation__pid=self.pid)\
-            .scan()
-        for result in results:
-            yield result.pid
-
-    def get_libraries(self):
-        """Get all libraries related to the organisation."""
-        pids = self.get_libraries_pids()
-        for pid in pids:
-            yield Library.get_record_by_pid(pid)
-
-    def get_number_of_libraries(self):
-        """Get number of libraries."""
-        results = LibrariesSearch().filter(
-            'term', organisation__pid=self.pid).source().count()
-        return results
-
-    def get_links_to_me(self):
-        """Get number of links."""
-        links = {}
-        libraries = self.get_number_of_libraries()
-        if libraries:
-            links['libraries'] = libraries
-        return links
-
-    def reasons_not_to_delete(self):
-        """Get reasons not to delete record."""
-        cannot_delete = {}
-        links = self.get_links_to_me()
-        if links:
-            cannot_delete['links'] = links
-        return cannot_delete
-
     @classmethod
     def get_all(cls):
         """Get all organisations."""
@@ -121,6 +90,28 @@ def get_record_by_viewcode(cls, viewcode):
 
         return result['hits']['hits'][0]['_source']
 
+    @classmethod
+    def get_record_by_user(cls, user):
+        """Return organisation associated with patron.
+
+        :param user: the user to check.
+        :return: Organisation record or None.
+        """
+        from ..patrons.api import current_patron, Patron
+        patron = Patron.get_patron_by_user(user) or current_patron
+        if patron:
+            return patron.get_organisation()
+
+    @classmethod
+    def get_record_by_online_harvested_source(cls, source):
+        """Get record by online harvested source."""
+        results = OrganisationsSearch().filter(
+            'term', online_harvested_source=source).scan()
+        try:
+            return Organisation.get_record_by_pid(next(results).pid)
+        except StopIteration:
+            return None
+
     @property
     def organisation_pid(self):
         """Get organisation pid ."""
@@ -136,21 +127,48 @@ def online_circulation_category(self):
         except StopIteration:
             return None
 
-    @classmethod
-    def get_record_by_online_harvested_source(cls, source):
-        """Get record by online harvested source."""
-        results = OrganisationsSearch().filter(
-            'term', online_harvested_source=source).scan()
-        try:
-            return Organisation.get_record_by_pid(next(results).pid)
-        except StopIteration:
-            return None
-
     def get_online_locations(self):
         """Get list of online locations."""
         return [library.online_location
                 for library in self.get_libraries() if library.online_location]
 
+    def get_libraries_pids(self):
+        """Get all libraries pids related to the organisation."""
+        results = LibrariesSearch().source(['pid'])\
+            .filter('term', organisation__pid=self.pid)\
+            .scan()
+        for result in results:
+            yield result.pid
+
+    def get_libraries(self):
+        """Get all libraries related to the organisation."""
+        pids = self.get_libraries_pids()
+        for pid in pids:
+            yield Library.get_record_by_pid(pid)
+
+    def get_number_of_libraries(self):
+        """Get number of libraries."""
+        return LibrariesSearch()\
+            .filter('term', organisation__pid=self.pid)\
+            .source()\
+            .count()
+
+    def get_links_to_me(self):
+        """Get number of links."""
+        links = {}
+        libraries = self.get_number_of_libraries()
+        if libraries:
+            links['libraries'] = libraries
+        return links
+
+    def reasons_not_to_delete(self):
+        """Get reasons not to delete record."""
+        cannot_delete = {}
+        links = self.get_links_to_me()
+        if links:
+            cannot_delete['links'] = links
+        return cannot_delete
+
 
 class OrganisationsIndexer(IlsRecordsIndexer):
     """Holdings indexing class."""

rero_ils/modules/organisations/permissions.py

@@ -0,0 +1,96 @@
+# -*- 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/>.
+
+"""Permissions for organisations."""
+
+from rero_ils.modules.organisations.api import current_organisation
+from rero_ils.modules.permissions import RecordPermission
+from rero_ils.modules.patrons.api import current_patron
+
+
+class OrganisationPermission(RecordPermission):
+    """Organisations permissions."""
+
+    @classmethod
+    def list(cls, user, record=None):
+        """List permission check.
+        :param user: Logged user.
+        :param record: Record to check.
+        :returns: True is action can be done.
+        """
+        # List organisation allowed only for staff members (lib, sys_lib)
+        if not current_patron or not current_patron.is_librarian:
+            return False
+        return True
+
+    @classmethod
+    def create(cls, user, record=None):
+        """Create permission check.
+        :param user: Logged user.
+        :param record: Record to check.
+        :returns: True is action can be done.
+        """
+        # Nobody can create an organisation. To create a new organisation, use
+        # the CLI interface
+        return False
+
+    @classmethod
+    def read(cls, user, record):
+        """Read permission check.
+        :param user: Logged user.
+        :param record: Record to check.
+        :returns: True is action can be done.
+        """
+        # user should be authenticated
+        if not current_patron:
+            return False
+        # Super user is allowed
+        if current_patron.is_superuser:
+            return True
+        # only staff members (lib, sys_lib) are allowed to read an organisation
+        if not current_patron.is_librarian:
+            return False
+        # For staff users, they can read only their own organisation.
+        return current_organisation['pid'] == record['pid']
+
+    @classmethod
+    def update(cls, user, record):
+        """Update permission check.
+        :param user: Logged user.
+        :param record: Record to check.
+        :returns: True is action can be done.
+        """
+        # user should be authenticated
+        if not current_patron or not current_patron.is_system_librarian:
+            return False
+        # super user is allowed
+        if current_patron.is_superuser:
+            return True
+        # only 'system_librarian' user allowed to update their own organisation
+        if not current_patron.is_system_librarian:
+            return False
+        return current_organisation['pid'] == record['pid']
+
+    @classmethod
+    def delete(cls, user, record):
+        """Delete permission check.
+        :param user: Logged user.
+        :param record: Record to check.
+        :returns: True if action can be done.
+        """
+        # Nobody can remove an organisation
+        return False

rero_ils/modules/patrons/api.py

@@ -71,11 +71,15 @@ class Meta:
 class Patron(IlsRecord):
     """Define API for patrons mixing."""
 
+    ROLE_SYSTEM_LIBRARIAN = 'system_librarian'
+    ROLE_LIBRARIAN = 'librarian'
+    ROLE_PATRON = 'patron'
+
     minter = patron_id_minter
     fetcher = patron_id_fetcher
     provider = PatronProvider
     model_cls = PatronMetadata
-    available_roles = ['system_librarian', 'librarian', 'patron']
+    available_roles = [ROLE_SYSTEM_LIBRARIAN, ROLE_LIBRARIAN, ROLE_PATRON]
 
     def validate(self, **kwargs):
         """Validate record against schema.
@@ -152,7 +156,7 @@ def update(self, data, dbcommit=False, reindex=False):
     def get_pid_exist_test(cls, data):
         """Test if library or patron type $ref pid exist.
 
-        ;param data: Data to find the information for the pids to test.
+        :param data: Data to find the information for the pids to test.
         :return: dictionair with pid types to test.
         """
         roles = data.get('roles', [])
@@ -389,6 +393,28 @@ def reasons_not_to_delete(self):
             cannot_delete['links'] = links
         return cannot_delete
 
+    @property
+    def is_superuser(self):
+        """Shortcut to check if user is a superuser."""
+        # At this time, there are no 'superuser' role
+        return False
+
+    @property
+    def is_system_librarian(self):
+        """Shortcut to check if user has system_librarian role."""
+        return Patron.ROLE_SYSTEM_LIBRARIAN in self.get('roles', [])
+
+    @property
+    def is_librarian(self):
+        """Shortcut to check if user has librarian role."""
+        librarian_roles = [Patron.ROLE_SYSTEM_LIBRARIAN, Patron.ROLE_LIBRARIAN]
+        return any(role in librarian_roles for role in self.get('roles', []))
+
+    @property
+    def is_patron(self):
+        """Shortcut to check if user has patron role."""
+        return Patron.ROLE_PATRON in self.get('roles', [])
+
     def get_organisation(self):
         """Return organisation."""
         return Organisation.get_record_by_pid(self.organisation_pid)
@@ -401,27 +427,6 @@ def library_pid(self):
         else:
             return None
 
-    @property
-    def is_librarian(self):
-        """Shortcut to check if user has librarian role."""
-        if self.is_system_librarian or 'librarian' in self.get('roles'):
-            return True
-        return False
-
-    @property
-    def is_system_librarian(self):
-        """Shortcut to check if user has system_librarian role."""
-        if 'system_librarian' in self.get('roles'):
-            return True
-        return False
-
-    @property
-    def is_patron(self):
-        """Shortcut to check if user has patron role."""
-        if 'patron' in self.get('roles'):
-            return True
-        return False
-
     @property
     def organisation_pid(self):
         """Get organisation pid for patron."""