diff --git a/documentation/changelog.rst b/documentation/changelog.rst index d29556cb5..7b9cf5e0a 100644 --- a/documentation/changelog.rst +++ b/documentation/changelog.rst @@ -12,6 +12,7 @@ New features Bugfixes ----------- +* Fix small problems in support for the admin-reader role & role-based authorization [see `PR #422 `_] Infrastructure / Support ---------------------- diff --git a/flexmeasures/api/common/routes.py b/flexmeasures/api/common/routes.py index 7a4b0d818..148ed7984 100644 --- a/flexmeasures/api/common/routes.py +++ b/flexmeasures/api/common/routes.py @@ -1,5 +1,6 @@ -from flask_security import auth_token_required, roles_required +from flask_security import auth_token_required +from flexmeasures.auth.decorators import roles_required from flexmeasures.api.common import flexmeasures_api as flexmeasures_api_ops from flexmeasures.api.common import implementations as ops_impl diff --git a/flexmeasures/api/dev/sensors.py b/flexmeasures/api/dev/sensors.py index f17e01b2e..8e269a6d7 100644 --- a/flexmeasures/api/dev/sensors.py +++ b/flexmeasures/api/dev/sensors.py @@ -6,7 +6,7 @@ from webargs.flaskparser import use_kwargs from werkzeug.exceptions import abort -from flexmeasures.auth.policy import ADMIN_ROLE +from flexmeasures.auth.policy import ADMIN_ROLE, ADMIN_READER_ROLE from flexmeasures.data.schemas.times import AwareDateTimeField from flexmeasures.data.models.time_series import Sensor @@ -15,6 +15,8 @@ class SensorAPI(FlaskView): """ This view exposes sensor attributes through API endpoints under development. These endpoints are not yet part of our official API, but support the FlexMeasures UI. + + TODO: use permission-based auth and marshmallow (SensorIDField). """ route_base = "/sensor" @@ -73,11 +75,15 @@ def get(self, id: int): def get_sensor_or_abort(id: int) -> Sensor: + """ + Util function to help the GET requests. Will be obsolete, see TODO above. + """ sensor = Sensor.query.filter(Sensor.id == id).one_or_none() if sensor is None: raise abort(404, f"Sensor {id} not found") if not ( current_user.has_role(ADMIN_ROLE) + or current_user.has_role(ADMIN_READER_ROLE) or sensor.generic_asset.owner is None # public or sensor.generic_asset.owner == current_user.account # private but authorized ): diff --git a/flexmeasures/api/play/routes.py b/flexmeasures/api/play/routes.py index a0677822b..20788b0f8 100644 --- a/flexmeasures/api/play/routes.py +++ b/flexmeasures/api/play/routes.py @@ -1,5 +1,6 @@ -from flask_security import auth_token_required, roles_required +from flask_security import auth_token_required +from flexmeasures.auth.decorators import roles_required from flexmeasures.auth.policy import ADMIN_ROLE from flexmeasures.api.play import ( flexmeasures_api as flexmeasures_api_play, diff --git a/flexmeasures/auth/decorators.py b/flexmeasures/auth/decorators.py index 653c49fe7..d3db61095 100644 --- a/flexmeasures/auth/decorators.py +++ b/flexmeasures/auth/decorators.py @@ -20,7 +20,7 @@ def roles_accepted(*roles): """As in Flask-Security, but also accept admin""" if ADMIN_ROLE not in roles: roles = roles + (ADMIN_ROLE,) - return roles_accepted_fs(roles) + return roles_accepted_fs(*roles) def roles_required(*roles): diff --git a/flexmeasures/ui/crud/users.py b/flexmeasures/ui/crud/users.py index 5b4785f2f..8a39ec97c 100644 --- a/flexmeasures/ui/crud/users.py +++ b/flexmeasures/ui/crud/users.py @@ -3,12 +3,14 @@ from flask import request, url_for from flask_classful import FlaskView +from flask_login import current_user +from flask_security import login_required from flask_wtf import FlaskForm from wtforms import StringField, FloatField, DateTimeField, BooleanField from wtforms.validators import DataRequired -from flask_security import roles_required, login_required -from flexmeasures.auth.policy import ADMIN_ROLE +from flexmeasures.auth.policy import ADMIN_READER_ROLE, ADMIN_ROLE +from flexmeasures.auth.decorators import roles_required, roles_accepted from flexmeasures.data import db from flexmeasures.data.models.user import User, Role, Account from flexmeasures.data.services.users import ( @@ -76,12 +78,17 @@ class UserCrudUI(FlaskView): route_base = "/users" trailing_slash = False - @roles_required(ADMIN_ROLE) def index(self): """/users""" include_inactive = request.args.get("include_inactive", "0") != "0" users = [] - for account in Account.query.all(): + if current_user.has_role(ADMIN_ROLE) or current_user.has_role( + ADMIN_READER_ROLE + ): + accounts = Account.query.all() + else: + accounts = [current_user.account] + for account in accounts: get_users_response = InternalApi().get( url_for( "UserAPI:index", @@ -97,7 +104,7 @@ def index(self): "crud/users.html", users=users, include_inactive=include_inactive ) - @roles_required(ADMIN_ROLE) + @roles_accepted(ADMIN_ROLE, ADMIN_READER_ROLE) def get(self, id: str): """GET from /users/""" get_user_response = InternalApi().get(url_for("UserAPI:get", id=id)) @@ -107,7 +114,7 @@ def get(self, id: str): asset_count = 0 if user: get_users_assets_response = InternalApi().get( - url_for("flexmeasures_api_v2_0.get_assets", owner_id=user.id) + url_for("AssetAPI:index", account_id=user.account_id) ) asset_count = len(get_users_assets_response.json()) return render_user(user, asset_count=asset_count) diff --git a/flexmeasures/ui/tests/test_user_crud.py b/flexmeasures/ui/tests/test_user_crud.py index c4506fa5f..fa0dce369 100644 --- a/flexmeasures/ui/tests/test_user_crud.py +++ b/flexmeasures/ui/tests/test_user_crud.py @@ -1,5 +1,5 @@ -from flask import url_for import pytest +from flask import url_for from flexmeasures.data.services.users import find_user_by_email from flexmeasures.ui.tests.utils import mock_user_response @@ -10,17 +10,6 @@ """ -@pytest.mark.parametrize("view", ["index", "get", "toggle_active"]) -def test_user_crud_as_non_admin(client, as_prosumer_user1, view): - user_index = client.get(url_for("UserCrudUI:index"), follow_redirects=True) - assert user_index.status_code == 403 - user2_id = find_user_by_email("test_prosumer_user_2@seita.nl").id - user_page = client.get( - url_for(f"UserCrudUI:{view}", id=user2_id), follow_redirects=True - ) - assert user_page.status_code == 403 - - def test_user_list(client, as_admin, requests_mock): requests_mock.get( "http://localhost//api/v3_0/users", @@ -34,13 +23,22 @@ def test_user_list(client, as_admin, requests_mock): assert b"bert@seita.nl" in user_index.data +@pytest.mark.parametrize("view", ["get", "toggle_active"]) +def test_user_page_as_nonadmin(client, as_prosumer_user1, view): + user2_id = find_user_by_email("test_prosumer_user_2@seita.nl").id + user_page = client.get( + url_for(f"UserCrudUI:{view}", id=user2_id), follow_redirects=True + ) + assert user_page.status_code == 403 + + def test_user_page(client, as_admin, requests_mock): mock_user = mock_user_response(as_list=False) requests_mock.get( "http://localhost//api/v3_0/users/2", status_code=200, json=mock_user ) requests_mock.get( - "http://localhost//api/v2_0/assets", + "http://localhost//api/v3_0/assets", status_code=200, json=[{}, {}, {}], # we only care about the length )