Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Nov 20, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 23 additions & 8 deletions rero_ils/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -908,7 +912,8 @@ def _(x):
'libraries',
'locations',
'persons',
'circ_policies'
'circ_policies',
'loans'
]

RECORDS_REST_SORT_OPTIONS = dict()
Expand All @@ -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
# ===========================
Expand Down Expand Up @@ -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'),
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2318,4 +2318,4 @@
"default": false
}
}
}
}
17 changes: 17 additions & 0 deletions rero_ils/modules/loans/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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."""
Expand Down
11 changes: 11 additions & 0 deletions rero_ils/modules/loans/jsonschemas/loans/loan-ils-v0.0.1.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": [
Expand Down
7 changes: 7 additions & 0 deletions rero_ils/modules/loans/mappings/v6/loans/loan-ils-v0.0.1.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,13 @@
"transaction_user_pid": {
"type": "keyword"
},
"organisation": {
"properties": {
"pid": {
"type": "keyword"
}
}
},
"trigger": {
"type": "keyword"
},
Expand Down
53 changes: 53 additions & 0 deletions rero_ils/modules/loans/permissions.py
Original file line number Diff line number Diff line change
@@ -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})()
24 changes: 20 additions & 4 deletions rero_ils/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@
def user_is_authenticated(user=None):
"""Checks if user is authenticated.

returns True if user is logged in and authenticated.
returns False if user is not logged or not authenticated.
:returns: True if user is logged in and authenticated.
:returns False if user is not logged or not authenticated.
"""
if not user:
user = current_user
Expand All @@ -47,9 +47,9 @@ def user_is_authenticated(user=None):
def staffer_is_authenticated(user=None):
"""Checks if user (librarian or system_librarian) is authenticated.

returns patron records if user is logged in and authenticated and has
:returns: patron records if user is logged in and authenticated and has
librarian or system_librarian role.
returns False otherwise.
:returns False otherwise.
"""
if not user:
user = current_user
Expand All @@ -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.
BadrAly marked this conversation as resolved.
Show resolved Hide resolved

: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.

Expand Down
18 changes: 18 additions & 0 deletions rero_ils/query.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
5 changes: 2 additions & 3 deletions tests/api/test_holdings_rest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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'
Expand Down
2 changes: 1 addition & 1 deletion tests/api/test_items_rest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion tests/api/test_items_rest_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading