From e61f13f7ca22698d19546f666f9e7d7c73470b01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicolas=20H=C3=B6ning?= Date: Sun, 24 Apr 2022 00:20:28 +0200 Subject: [PATCH 1/9] use roles decorator from flexmeasures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Nicolas Höning --- flexmeasures/api/common/routes.py | 3 ++- flexmeasures/api/play/routes.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) 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/play/routes.py b/flexmeasures/api/play/routes.py index a0677822b..1914bbd89 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.deecorators import roles_required from flexmeasures.auth.policy import ADMIN_ROLE from flexmeasures.api.play import ( flexmeasures_api as flexmeasures_api_play, From 1ce5f23139764ddebe3c45987297287463c69f76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicolas=20H=C3=B6ning?= Date: Sun, 24 Apr 2022 00:24:27 +0200 Subject: [PATCH 2/9] fix roles_accepted decorator's passing roles through to flask security MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Nicolas Höning --- flexmeasures/auth/decorators.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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): From b077267501c07b01431547198c2b8a890f23e6f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicolas=20H=C3=B6ning?= Date: Sun, 24 Apr 2022 00:26:19 +0200 Subject: [PATCH 3/9] the two reading User CRUD endpoints give admin-reader a better place. Also use API v3 to count a user's assets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Nicolas Höning --- flexmeasures/ui/crud/users.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/flexmeasures/ui/crud/users.py b/flexmeasures/ui/crud/users.py index 5b4785f2f..b21600ad3 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,16 @@ 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(): + account = [current_user.account] + if current_user.has_role(ADMIN_ROLE) or current_user.has_role( + ADMIN_READER_ROLE + ): + accounts = Account.query.all() + for account in accounts: get_users_response = InternalApi().get( url_for( "UserAPI:index", @@ -97,7 +103,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 +113,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) From 138b9c51453204767442cb8a1ef6c37c0be0db8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicolas=20H=C3=B6ning?= Date: Sun, 24 Apr 2022 00:59:07 +0200 Subject: [PATCH 4/9] make tests work: fix a bug, a mock API route and remove a test we don't need to do anymore after this PR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Nicolas Höning --- flexmeasures/ui/crud/users.py | 2 +- flexmeasures/ui/tests/test_user_crud.py | 14 +------------- 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/flexmeasures/ui/crud/users.py b/flexmeasures/ui/crud/users.py index b21600ad3..c3ba1301f 100644 --- a/flexmeasures/ui/crud/users.py +++ b/flexmeasures/ui/crud/users.py @@ -82,7 +82,7 @@ def index(self): """/users""" include_inactive = request.args.get("include_inactive", "0") != "0" users = [] - account = [current_user.account] + accounts = [current_user.account] if current_user.has_role(ADMIN_ROLE) or current_user.has_role( ADMIN_READER_ROLE ): diff --git a/flexmeasures/ui/tests/test_user_crud.py b/flexmeasures/ui/tests/test_user_crud.py index c4506fa5f..3947c2c99 100644 --- a/flexmeasures/ui/tests/test_user_crud.py +++ b/flexmeasures/ui/tests/test_user_crud.py @@ -1,5 +1,4 @@ from flask import url_for -import pytest from flexmeasures.data.services.users import find_user_by_email from flexmeasures.ui.tests.utils import mock_user_response @@ -10,17 +9,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", @@ -40,7 +28,7 @@ def test_user_page(client, as_admin, requests_mock): "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 ) From 3f2916b6d4fc0990d86289f199501018781e2376 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicolas=20H=C3=B6ning?= Date: Mon, 25 Apr 2022 13:02:28 +0200 Subject: [PATCH 5/9] changelog entry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Nicolas Höning --- documentation/changelog.rst | 1 + 1 file changed, 1 insertion(+) 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 ---------------------- From 981a42b7023548b51d4dba2f5af00dceb0fecf2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicolas=20H=C3=B6ning?= Date: Mon, 25 Apr 2022 13:46:47 +0200 Subject: [PATCH 6/9] let admin-reader use the SensorAPI, also leave TODO for using latest concepts in said API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Nicolas Höning --- flexmeasures/api/dev/sensors.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) 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 ): From 2ddd43bcdf4559f378f751dcfa9540b8d8c39223 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicolas=20H=C3=B6ning?= Date: Wed, 27 Apr 2022 21:39:57 +0200 Subject: [PATCH 7/9] avoid lazy-loading user's account if not needed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Nicolas Höning --- flexmeasures/ui/crud/users.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/flexmeasures/ui/crud/users.py b/flexmeasures/ui/crud/users.py index c3ba1301f..8a39ec97c 100644 --- a/flexmeasures/ui/crud/users.py +++ b/flexmeasures/ui/crud/users.py @@ -82,11 +82,12 @@ def index(self): """/users""" include_inactive = request.args.get("include_inactive", "0") != "0" users = [] - accounts = [current_user.account] 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( From cebe6957eb4ddf4f2cc9c04cc943501dcc950523 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicolas=20H=C3=B6ning?= Date: Wed, 27 Apr 2022 22:08:03 +0200 Subject: [PATCH 8/9] fix typo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Nicolas Höning --- flexmeasures/api/play/routes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flexmeasures/api/play/routes.py b/flexmeasures/api/play/routes.py index 1914bbd89..20788b0f8 100644 --- a/flexmeasures/api/play/routes.py +++ b/flexmeasures/api/play/routes.py @@ -1,6 +1,6 @@ from flask_security import auth_token_required -from flexmeasures.auth.deecorators import roles_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, From 6cb1470afc5c21920269b7b1f943bb1a87430fe1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicolas=20H=C3=B6ning?= Date: Wed, 27 Apr 2022 22:09:42 +0200 Subject: [PATCH 9/9] preserve still-needed parts of test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Nicolas Höning --- flexmeasures/ui/tests/test_user_crud.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/flexmeasures/ui/tests/test_user_crud.py b/flexmeasures/ui/tests/test_user_crud.py index 3947c2c99..fa0dce369 100644 --- a/flexmeasures/ui/tests/test_user_crud.py +++ b/flexmeasures/ui/tests/test_user_crud.py @@ -1,3 +1,4 @@ +import pytest from flask import url_for from flexmeasures.data.services.users import find_user_by_email @@ -22,6 +23,15 @@ 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(