Skip to content

Commit

Permalink
login: add a change password API
Browse files Browse the repository at this point in the history
* Uses patron permissions without http request.
* Adds a custom invenio-accouts change password API to allow a librarian
  to change a password for a patron.
* Makes the user_id in the patron jsonschema required.
* Gives more weight on the barcode field for the patrons searches.
* Fixes street description message in the patron JSONSchema, closes #1382.
* Fixes email validation in the patron and vendore JSONSchema, closes #1381.

Co-Authored-by: Johnny Mariéthoz <[email protected]>
  • Loading branch information
jma committed Nov 12, 2020
1 parent 7639de6 commit 82dc440
Show file tree
Hide file tree
Showing 8 changed files with 260 additions and 85 deletions.
79 changes: 76 additions & 3 deletions rero_ils/accounts_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,22 @@

"""Invenio Account custom views."""

from flask import current_app
from flask import after_this_request, current_app
from flask import request as flask_request
from invenio_accounts.utils import change_user_password
from invenio_accounts.views.rest import \
ChangePasswordView as BaseChangePasswordView
from invenio_accounts.views.rest import LoginView as CoreLoginView
from invenio_accounts.views.rest import use_kwargs
from invenio_accounts.views.rest import _commit, use_args, use_kwargs
from invenio_userprofiles.models import UserProfile
from marshmallow import Schema, fields
from sqlalchemy.orm.exc import NoResultFound
from webargs import ValidationError, fields
from webargs import ValidationError, fields, validate
from werkzeug.local import LocalProxy

from .modules.patrons.api import Patron, current_patron
from .modules.patrons.permissions import PatronPermission

current_datastore = LocalProxy(
lambda: current_app.extensions['security'].datastore)

Expand Down Expand Up @@ -65,3 +73,68 @@ def post(self, **kwargs):
self.verify_login(user, **kwargs)
self.login_user(user)
return self.success_response(user)


class PasswordPassword(Schema):
"""Args validation when a user want to change his password."""

password = fields.String(
validate=[validate.Length(min=6, max=128)])
new_password = fields.String(
required=True, validate=[validate.Length(min=6, max=128)])


class UsernamePassword(Schema):
"""Args validation when a professional change a password for a user."""

username = fields.String(
validate=[validate.Length(min=1, max=128)])
new_password = fields.String(
required=True, validate=[validate.Length(min=6, max=128)])


def make_password_schema(request):
"""Select the right args validation depending on the context."""
# Filter based on 'fields' query parameter
fields = request.args.get('fields', None)
only = fields.split(',') if fields else None
# Respect partial updates for PATCH requests
partial = request.method == 'PATCH'
if request.json.get('username'):
return UsernamePassword(only=only,
partial=partial, context={"request": request})

# Add current request to the schema's context
return PasswordPassword(only=only,
partial=partial, context={"request": request})


class ChangePasswordView(BaseChangePasswordView):
"""View to change the user password."""

def verify_permission(self, username, **args):
"""Check permissions.
Check if the current user can change a password for an other user.
"""
patron = Patron.get_patron_by_username(username)
if not PatronPermission.can_create(current_patron, patron, patron):
return current_app.login_manager.unauthorized()

def change_password_for_user(self, username, new_password, **kwargs):
"""Perform change password for a specific user."""
after_this_request(_commit)
patron = Patron.get_patron_by_username(username)
change_user_password(user=patron.user,
password=new_password)

@use_args(make_password_schema)
def post(self, args):
"""Change user password."""
if flask_request.json.get('username'):
self.verify_permission(**args)
self.change_password_for_user(**args)
else:
self.verify_password(**args)
self.change_password(**args)
return self.success_response()
5 changes: 4 additions & 1 deletion rero_ils/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ def _(x):
"register": "invenio_accounts.views.rest:RegisterView",
"forgot_password": "invenio_accounts.views.rest:ForgotPasswordView",
"reset_password": "invenio_accounts.views.rest:ResetPasswordView",
"change_password": "invenio_accounts.views.rest:ChangePasswordView",
"change_password": "rero_ils.accounts_views:ChangePasswordView",
"send_confirmation":
"invenio_accounts.views.rest:SendConfirmationEmailView",
"confirm_email": "invenio_accounts.views.rest:ConfirmEmailView",
Expand Down Expand Up @@ -1774,6 +1774,9 @@ def _(x):
'publicationYearText': 2,
'freeFormedPublicationDate': 2,
'subjects.*': 2
},
'patrons': {
'barcode': 3
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
"last_name",
"birth_date",
"username",
"user_id",
"roles"
],
"properties": {
Expand Down Expand Up @@ -75,7 +76,7 @@
"title": "Email",
"type": "string",
"format": "email",
"pattern": "^.*@.*\\..*$",
"pattern": "^.*@.*\\..+$",
"minLength": 6,
"form": {
"validation": {
Expand Down Expand Up @@ -113,7 +114,7 @@
},
"street": {
"title": "Street",
"description": "Street and number of the address, separated by a coma.",
"description": "Street and number of the address.",
"type": "string",
"minLength": 3
},
Expand Down Expand Up @@ -265,10 +266,10 @@
"title": "Additional communication email",
"type": "string",
"format": "email",
"pattern": "^.*@.*\\..*$",
"pattern": "^.*@.*\\..+$",
"minLength": 6,
"form": {
"hideExpression": "model.communication_channel === 'email'",
"hideExpression": "field.parent.model && field.parent.model.communication_channel !== 'email'",
"messages": {
"patternMessage": "Email should have at least one <code>@</code> and one <code>.</code>."
}
Expand Down
15 changes: 13 additions & 2 deletions rero_ils/modules/patrons/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,19 @@ def create(cls, user, record=None):
"""Create permission check.
:param user: Logged user.
:param record: Record to check.
:param record: pre-existing Record to check.
:return: True is action can be done.
"""
incoming_record = request.get_json(silent=True) or {}
return cls.can_create(user, record, incoming_record)

@classmethod
def can_create(cls, user, record, incoming_record):
"""Create permission check.
:param user: Logged user.
:param record: pre-existing Record to check.
:param record: new incoming Record data.
:return: True is action can be done.
"""
# only staff members (lib, sys_lib) can create patrons ...
Expand All @@ -75,7 +87,6 @@ def create(cls, user, record=None):
return True
# librarian user has some restrictions...
if current_patron.is_librarian:
incoming_record = request.get_json(silent=True) or {}
# a librarian cannot manage a system_librarian patron
if 'system_librarian' in incoming_record.get('roles', [])\
or 'system_librarian' in record.get('roles', []):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@
"title": "Email",
"type": "string",
"format": "email",
"pattern": "^.*@.*\\..*$",
"pattern": "^.*@.*\\..+$",
"minLength": 6,
"form": {
"validation": {
Expand Down Expand Up @@ -213,7 +213,7 @@
"title": "Email",
"type": "string",
"format": "email",
"pattern": "^.*@.*\\..*$",
"pattern": "^.*@.*\\..+$",
"minLength": 6,
"form": {
"validation": {
Expand Down
76 changes: 76 additions & 0 deletions tests/api/test_user_authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

"""Test User Authentication API."""
from flask import url_for
from invenio_accounts.testutils import login_user_via_session
from utils import get_json, postdata


Expand Down Expand Up @@ -98,3 +99,78 @@ def test_login_without_email(client, patron_sion_without_email):
assert data.get('id')
# logout for the next test
client.post(url_for('invenio_accounts_rest_auth.logout'))


def test_change_password(client, patron_martigny_no_email,
librarian_sion_no_email,
librarian_martigny_no_email):
"""Test login with several scenarios."""

# try to change password with an anonymous user
res, _ = postdata(
client,
'invenio_accounts_rest_auth.change_password',
{
'password': patron_martigny_no_email.get('birth_date'),
'new_password': 'new'
}
)
data = get_json(res)
assert res.status_code == 401

# with a logged but the password is too short
login_user_via_session(client, patron_martigny_no_email.user)
res, _ = postdata(
client,
'invenio_accounts_rest_auth.change_password',
{
'password': patron_martigny_no_email.get('birth_date'),
'new_password': 'new'
}
)
data = get_json(res)
assert res.status_code == 400
assert data.get('message') == 'Validation error.'

# with a logged user
res, _ = postdata(
client,
'invenio_accounts_rest_auth.change_password',
{
'password': patron_martigny_no_email.get('birth_date'),
'new_password': 'new_passwd'
}
)
data = get_json(res)
assert res.status_code == 200
assert data.get('message') == 'You successfully changed your password.'

# with a librarian of a different organisation
login_user_via_session(client, librarian_sion_no_email.user)
res, _ = postdata(
client,
'invenio_accounts_rest_auth.change_password',
{
'username': patron_martigny_no_email.get('username'),
'new_password': 'new_passwd2'
}
)
data = get_json(res)
assert res.status_code == 401

# with a librarian of the same organisation
login_user_via_session(client, librarian_martigny_no_email.user)
res, _ = postdata(
client,
'invenio_accounts_rest_auth.change_password',
{
'username': patron_martigny_no_email.get('username'),
'new_password': 'new_passwd2'
}
)
data = get_json(res)
assert res.status_code == 200
assert data.get('message') == 'You successfully changed your password.'

# logout for the next test
client.post(url_for('invenio_accounts_rest_auth.logout'))
11 changes: 11 additions & 0 deletions tests/unit/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

from __future__ import absolute_import, print_function

from copy import deepcopy

import pytest
from pkg_resources import resource_string
from utils import get_schema
Expand Down Expand Up @@ -158,6 +160,15 @@ def patron_schema(monkeypatch):
return get_schema(monkeypatch, schema_in_bytes)


@pytest.fixture(scope="function")
def patron_martigny_data_tmp_with_id(patron_martigny_data_tmp):
"""Load Martigny patron data scope function with a mocked user_id."""
patron = deepcopy(patron_martigny_data_tmp)
# mock the user_id which is add by the Patron API.
patron['user_id'] = 100
return patron


@pytest.fixture()
def persons_schema(monkeypatch):
"""Patron Jsonschema for records."""
Expand Down
Loading

0 comments on commit 82dc440

Please sign in to comment.