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

authorization: create role management API #1043

Merged
merged 1 commit into from
Jun 25, 2020
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: 30 additions & 1 deletion rero_ils/modules/patrons/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,22 @@ class Meta:
class Patron(IlsRecord):
"""Define API for patrons mixing."""

ROLE_PATRON = 'patron'
ROLE_LIBRARIAN = 'librarian'
ROLE_SYSTEM_LIBRARIAN = 'system_librarian'

ROLES_HIERARCHY = {
ROLE_PATRON: [],
ROLE_LIBRARIAN: [],
ROLE_SYSTEM_LIBRARIAN: [ROLE_LIBRARIAN]
}

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.
Expand Down Expand Up @@ -209,6 +220,24 @@ def _remove_roles(self):
if role in db_roles:
self.remove_role(role)

@classmethod
def get_reachable_roles(cls, role):
"""Get list of roles depending on role hierarchy."""
if role not in Patron.ROLES_HIERARCHY:
return []
roles = Patron.ROLES_HIERARCHY[role].copy()
roles.append(role)
return roles

@classmethod
def get_all_roles_for_role(cls, role):
"""The list of roles covering given role based on the hierarchy."""
roles = [role]
for key in Patron.ROLES_HIERARCHY:
if role in Patron.ROLES_HIERARCHY[key] and key not in roles:
roles.append(key)
return roles

@classmethod
def get_patron_by_user(cls, user):
"""Get patron by user."""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,8 @@
"hideExpression": "!field.parent.model.roles.some(v => (v === 'librarian' || v === 'system_librarian'))",
"expressionProperties": {
"templateOptions.required": "field.parent.model.roles.some(v => (v === 'librarian' || v === 'system_librarian'))"
}
},
"fieldMap": "library"
}
},
"roles": {
Expand Down Expand Up @@ -225,6 +226,9 @@
}
]
}
},
"form": {
"fieldMap": "roles"
}
},
"communication_channel": {
Expand Down
20 changes: 20 additions & 0 deletions rero_ils/modules/patrons/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

from flask import request

from .api import Patron
from ...permissions import staffer_is_authenticated


Expand Down Expand Up @@ -58,6 +59,9 @@ def can_delete_patron_factory(record, *args, **kwargs):
"""
def can(self):
patron = staffer_is_authenticated()
# It should be not possible to remove itself !
if patron and patron.pid == record.pid:
return False
if patron and patron.organisation_pid == record.organisation_pid:
if patron.is_system_librarian:
return True
Expand All @@ -71,3 +75,19 @@ def can(self):
return True
return False
return type('Check', (), {'can': can})()


def get_allowed_roles_management():
"""Get the roles that current logged user could manage.

:return An array of allowed role management.
"""
allowed_roles = []
patron = staffer_is_authenticated()
if patron:
if patron.is_librarian:
allowed_roles.append(Patron.ROLE_PATRON)
allowed_roles.append(Patron.ROLE_LIBRARIAN)
if patron.is_system_librarian:
allowed_roles.append(Patron.ROLE_SYSTEM_LIBRARIAN)
return allowed_roles
10 changes: 10 additions & 0 deletions rero_ils/modules/patrons/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
from werkzeug.exceptions import NotFound

from .api import Patron, PatronsSearch
from .permissions import get_allowed_roles_management
from .utils import user_has_patron
from ..items.api import Item
from ..libraries.api import Library
Expand Down Expand Up @@ -215,6 +216,15 @@ def profile(viewcode):
)


@api_blueprint.route('/roles_management_permissions', methods=['GET'])
@check_permission
def get_roles_management_permissions():
"""Get the roles that current logged user could manage."""
return jsonify({
'allowed_roles': get_allowed_roles_management()
})


@blueprint.app_template_filter('get_patron_from_barcode')
def get_patron_from_barcode(value):
"""Get patron from barcode."""
Expand Down
11 changes: 8 additions & 3 deletions tests/api/test_patrons_rest.py
Original file line number Diff line number Diff line change
Expand Up @@ -356,10 +356,9 @@ def test_patron_secure_api(client, json_header,
# assert res.status_code == 403


def test_patron_secure_api_create(client, json_header,
def test_patron_secure_api_create(client,
patron_martigny_data,
librarian_martigny_no_email,
librarian_sion_no_email):
librarian_martigny_no_email):
"""Test patron secure api create."""
# Martigny
login_user_via_session(client, librarian_martigny_no_email.user)
Expand Down Expand Up @@ -427,6 +426,12 @@ def test_patron_secure_api_delete(client, json_header,
res = client.delete(record_url)
assert res.status_code == 204

# try to delete itself
record_url = url_for('invenio_records_rest.ptrn_item',
pid_value=librarian_martigny_no_email.pid)
res = client.delete(record_url)
assert res.status_code == 403

# Sion
# login_user_via_session(client, librarian_sion_no_email.user)

Expand Down
8 changes: 8 additions & 0 deletions tests/api/test_permissions_librarian.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,14 @@ def test_librarian_permissions(
data = get_json(res)
assert data['hits']['total'] == 3

# can manage all types of patron roles
role_url = url_for('api_patrons.get_roles_management_permissions')
res = client.get(role_url)
assert res.status_code == 200
data = get_json(res)
assert 'librarian' in data['allowed_roles']
assert 'system_librarian' not in data['allowed_roles']

# can create all type of users except system_librarians
post_entrypoint = 'invenio_records_rest.ptrn_list'
system_librarian = deepcopy(record)
Expand Down
5 changes: 5 additions & 0 deletions tests/api/test_permissions_patron.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,11 @@ def test_patron_permissions(
res = client.get(list_url)
assert res.status_code == 403

# can not manage any types of patron roles
role_url = url_for('api_patrons.get_roles_management_permissions')
res = client.get(role_url)
assert res.status_code == 403

# can not create any type of users.
system_librarian = deepcopy(record)
librarian = deepcopy(record)
Expand Down
7 changes: 7 additions & 0 deletions tests/api/test_permissions_sys_librarian.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,13 @@ def test_system_librarian_permissions(
data = get_json(res)
assert data['hits']['total'] == 3

# can manage all types of patron roles
role_url = url_for('api_patrons.get_roles_management_permissions')
res = client.get(role_url)
assert res.status_code == 200
data = get_json(res)
assert 'system_librarian' in data['allowed_roles']

# can create all type of users.
system_librarian = deepcopy(record)
librarian = deepcopy(record)
Expand Down
6 changes: 4 additions & 2 deletions tests/api/test_record_permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,10 +92,11 @@ def test_patrons_permissions(
librarian2_martigny_no_email,
librarian_saxon_no_email,
system_librarian_martigny_no_email,
system_librarian2_martigny_no_email,
system_librarian_sion_no_email,
librarian_sion_no_email
):
"""Test serializers for patrons."""
"""Test permissions for patrons."""

# simple librarian -----------------------------------------------
login_user(client, librarian_martigny_no_email)
Expand Down Expand Up @@ -130,8 +131,9 @@ def test_patrons_permissions(
assert data['update']['can']

# should update and delete a system librarian of the same organisation
# but not itself
data = call_api_permissions(client, 'patrons',
system_librarian_martigny_no_email.pid)
system_librarian2_martigny_no_email.pid)
assert data['delete']['can']
assert data['update']['can']

Expand Down
20 changes: 20 additions & 0 deletions tests/data/data.json
Original file line number Diff line number Diff line change
Expand Up @@ -2340,6 +2340,26 @@
"blocked": true,
"blocked_note": "Lost card"
},
"ptrn12": {
"$schema": "https://ils.rero.ch/schemas/patrons/patron-v0.0.1.json",
"pid": "ptrn12",
"first_name": "Joella",
"last_name": "Dosimonta",
"street": "Grand place, 123",
"postal_code": "1920",
"city": "Martigny",
"birth_date": "1980-06-07",
"library": {
"$ref": "https://ils.rero.ch/api/libraries/lib1"
},
"email": "[email protected]",
"phone": "+32324993585",
"roles": [
"system_librarian",
"librarian"
],
"barcode": "sys_ptrn12"
},
"dummy_notif": {
"$schema": "https://ils.rero.ch/schemas/notifications/notification-v0.0.1.json",
"pid": "notif1",
Expand Down
45 changes: 45 additions & 0 deletions tests/fixtures/circulation.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,51 @@ def system_librarian_martigny_no_email(
return ptrn


@pytest.fixture(scope="module")
def system_librarian2_martigny_data(data):
"""Load Martigny system librarian data."""
return deepcopy(data.get('ptrn12'))


@pytest.fixture(scope="function")
def system_librarian2_martigny_data_tmp(data):
"""Load Martigny system librarian data scope function."""
return deepcopy(data.get('ptrn12'))


@pytest.fixture(scope="module")
def system_librarian2_martigny(
app,
roles,
lib_martigny,
system_librarian2_martigny_data):
"""Create Martigny system librarian record."""
ptrn = Patron.create(
data=system_librarian2_martigny_data,
delete_pid=False,
dbcommit=True,
reindex=True)
flush_index(PatronsSearch.Meta.index)
return ptrn


@pytest.fixture(scope="module")
@mock.patch('rero_ils.modules.patrons.api.send_reset_password_instructions')
def system_librarian2_martigny_no_email(
app,
roles,
lib_martigny,
system_librarian2_martigny_data):
"""Create Martigny system librarian without sending reset password."""
ptrn = Patron.create(
data=system_librarian2_martigny_data,
delete_pid=False,
dbcommit=True,
reindex=True)
flush_index(PatronsSearch.Meta.index)
return ptrn


# ------------ Org: Martigny, Lib: Martigny, Librarian 1 ----------
@pytest.fixture(scope="module")
def librarian_martigny_data(data):
Expand Down
27 changes: 27 additions & 0 deletions tests/ui/patrons/test_patrons_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,3 +175,30 @@ def test_get_patron_blocked_field_absent(patron2_martigny_no_email):
"""Test patron blocked field retrieval."""
patron = Patron.get_patron_by_email(patron2_martigny_no_email.get('email'))
assert 'blocked' not in patron


def test_get_reachable_roles():
"""Test get roles covered by the given role."""
roles = Patron.get_reachable_roles(Patron.ROLE_SYSTEM_LIBRARIAN)
assert len(roles) == 2
assert Patron.ROLE_LIBRARIAN in roles
assert Patron.ROLE_SYSTEM_LIBRARIAN in roles

roles = Patron.get_reachable_roles('unknown_role')
assert not roles


def test_get_all_roles_for_role():
"""Test get roles covering by roles hierarchy."""
roles = Patron.get_all_roles_for_role(Patron.ROLE_PATRON)
assert len(roles) == 1
assert Patron.ROLE_PATRON in roles

roles = Patron.get_all_roles_for_role(Patron.ROLE_SYSTEM_LIBRARIAN)
assert len(roles) == 1
assert Patron.ROLE_SYSTEM_LIBRARIAN in roles

roles = Patron.get_all_roles_for_role(Patron.ROLE_LIBRARIAN)
assert len(roles) == 2
assert Patron.ROLE_LIBRARIAN in roles
assert Patron.ROLE_SYSTEM_LIBRARIAN in roles