Better support for the admin-reader role and various smaller fixes in…

… role-based authorization (#422)

* use roles decorator from flexmeasures

Signed-off-by: Nicolas Höning <nicolas@seita.nl>

* fix roles_accepted decorator's passing roles through to flask security

Signed-off-by: Nicolas Höning <nicolas@seita.nl>

* the two reading User CRUD endpoints give admin-reader a better place. Also use API v3 to count a user's assets

Signed-off-by: Nicolas Höning <nicolas@seita.nl>

* make tests work: fix a bug, a mock API route and remove a test we don't need to do anymore after this PR

Signed-off-by: Nicolas Höning <nicolas@seita.nl>

* changelog entry

Signed-off-by: Nicolas Höning <nicolas@seita.nl>

* let admin-reader use the SensorAPI, also leave TODO for using latest concepts in said API

Signed-off-by: Nicolas Höning <nicolas@seita.nl>

* avoid lazy-loading user's account if not needed

Signed-off-by: Nicolas Höning <nicolas@seita.nl>

* fix typo

Signed-off-by: Nicolas Höning <nicolas@seita.nl>

* preserve still-needed parts of test

Signed-off-by: Nicolas Höning <nicolas@seita.nl>

documentation/changelog.rst

@@ -13,6 +13,7 @@ New features
 
 Bugfixes
 -----------
+* Fix small problems in support for the admin-reader role & role-based authorization [see `PR #422 <http://www.github.com/FlexMeasures/flexmeasures/pull/422>`_]
 
 Infrastructure / Support
 ----------------------

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
 

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

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,

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

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/<id>"""
         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)

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
     )