From cb059347740dcd312cd15f4671178c4465adbfef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicolas=20H=C3=B6ning?= Date: Thu, 25 Aug 2022 14:46:34 +0200 Subject: [PATCH 1/5] validate that user can set the account on an asset; improve UI for adding assets by non-admins MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Nicolas Höning --- flexmeasures/cli/data_show.py | 4 ++-- flexmeasures/data/schemas/generic_assets.py | 9 +++++++++ flexmeasures/ui/crud/assets.py | 19 ++++++++++++++++--- flexmeasures/ui/templates/crud/assets.html | 2 +- 4 files changed, 28 insertions(+), 6 deletions(-) diff --git a/flexmeasures/cli/data_show.py b/flexmeasures/cli/data_show.py index 23f58bd8c..e7f2ecb0f 100644 --- a/flexmeasures/cli/data_show.py +++ b/flexmeasures/cli/data_show.py @@ -109,7 +109,7 @@ def show_account(account): user.username, user.email, naturaltime(user.last_login_at), - "".join([role.name for role in user.roles]), + ",".join([role.name for role in user.roles]), ) for user in users ] @@ -187,7 +187,7 @@ def show_generic_asset(asset): sensor.unit, naturaldelta(sensor.event_resolution), sensor.timezone, - "".join([f"{k}:{v}\n" for k, v in sensor.attributes.items()]), + ",".join([f"{k}:{v}\n" for k, v in sensor.attributes.items()]), ) for sensor in sensors ] diff --git a/flexmeasures/data/schemas/generic_assets.py b/flexmeasures/data/schemas/generic_assets.py index 3b7cc8ed6..41fe42196 100644 --- a/flexmeasures/data/schemas/generic_assets.py +++ b/flexmeasures/data/schemas/generic_assets.py @@ -2,6 +2,7 @@ import json from marshmallow import validates, validates_schema, ValidationError, fields +from flask_security import current_user from flexmeasures.data import ma from flexmeasures.data.models.user import Account @@ -11,6 +12,7 @@ MarshmallowClickMixin, with_appcontext_if_needed, ) +from flexmeasures.auth.policy import user_has_admin_access class JSON(fields.Field): @@ -66,6 +68,13 @@ def validate_account(self, account_id: int): account = Account.query.get(account_id) if not account: raise ValidationError(f"Account with Id {account_id} doesn't exist.") + if ( + not user_has_admin_access(current_user, "update") + and account_id != current_user.account_id + ): + raise ValidationError( + "User is not allowed to create assets for this account." + ) @validates("latitude") def validate_latitude(self, latitude: Optional[float]): diff --git a/flexmeasures/ui/crud/assets.py b/flexmeasures/ui/crud/assets.py index 14ea9dbcf..d36b165df 100644 --- a/flexmeasures/ui/crud/assets.py +++ b/flexmeasures/ui/crud/assets.py @@ -12,6 +12,7 @@ from flexmeasures.data import db from flexmeasures.auth.error_handling import unauthorized_handler +from flexmeasures.auth.policy import check_access from flexmeasures.data.models.generic_assets import ( GenericAssetType, GenericAsset, @@ -48,7 +49,7 @@ class AssetForm(FlaskForm): places=4, render_kw={"placeholder": "--Click the map or enter a longitude--"}, ) - attributes = StringField("Other attributes (JSON)") + attributes = StringField("Other attributes (JSON)", default="{}") def validate_on_submit(self): if ( @@ -148,6 +149,15 @@ def expunge_asset(): return asset_data +def user_can_create_assets() -> bool: + try: + check_access(current_user.account, "create-children") + except Exception as exc: + current_app.logger.error(exc) + return False + return True + + class AssetCrudUI(FlaskView): """ These views help us offer a Jinja2-based UI. @@ -186,7 +196,10 @@ def get_asset_by_account(account_id) -> List[GenericAsset]: assets = get_asset_by_account(current_user.account_id) return render_flexmeasures_template( - "crud/assets.html", assets=assets, message=msg + "crud/assets.html", + assets=assets, + message=msg, + user_can_create_assets=user_can_create_assets(), ) @login_required @@ -218,7 +231,7 @@ def get(self, id: str): """GET from /assets/ where id can be 'new' (and thus the form for asset creation is shown)""" if id == "new": - if not current_user.has_role("admin"): + if not user_can_create_assets(): return unauthorized_handler(None, []) asset_form = with_options(NewAssetForm()) diff --git a/flexmeasures/ui/templates/crud/assets.html b/flexmeasures/ui/templates/crud/assets.html index 0d2dcbf5d..5d4d667af 100644 --- a/flexmeasures/ui/templates/crud/assets.html +++ b/flexmeasures/ui/templates/crud/assets.html @@ -20,7 +20,7 @@

Asset overview

Account Sensors - {% if user_is_admin %} + {% if user_can_create_assets %}
From 82ed72c1d1f828363f465f647a0b83826ae81dbb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicolas=20H=C3=B6ning?= Date: Thu, 25 Aug 2022 15:09:46 +0200 Subject: [PATCH 2/5] add test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Nicolas Höning --- .../api/v3_0/tests/test_assets_api.py | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/flexmeasures/api/v3_0/tests/test_assets_api.py b/flexmeasures/api/v3_0/tests/test_assets_api.py index b8572eb46..6d2be3581 100644 --- a/flexmeasures/api/v3_0/tests/test_assets_api.py +++ b/flexmeasures/api/v3_0/tests/test_assets_api.py @@ -212,6 +212,27 @@ def test_post_an_asset_with_existing_name(client, setup_api_test_data): ) +def test_post_an_asset_with_other_account(client, setup_api_test_data): + """Catch auth error, when account-admin posts an asset for another account""" + with UserContext("test_prosumer_user_2@seita.nl") as account_admin_user: + auth_token = account_admin_user.get_auth_token() + with AccountContext("Test Supplier Account") as supplier: + supplier_id = supplier.id + post_data = get_asset_post_data() + post_data["account_id"] = supplier_id + asset_creation_response = client.post( + url_for("AssetAPI:post"), + json=post_data, + headers={"content-type": "application/json", "Authorization": auth_token}, + ) + print(f"Creation Response: {asset_creation_response.json}") + assert asset_creation_response.status_code == 422 + assert ( + "not allowed to create assets for this account" + in asset_creation_response.json["message"]["json"]["account_id"][0] + ) + + def test_post_an_asset_with_nonexisting_field(client, setup_api_test_data): """Posting a field that is unexpected leads to a 422""" with UserContext("test_admin_user@seita.nl") as prosumer: From 94df04e629717e995bcb406bb75ca7ccb7ecdad9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicolas=20H=C3=B6ning?= Date: Fri, 26 Aug 2022 12:10:36 +0200 Subject: [PATCH 3/5] 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 1f621fc7c..c7fac1647 100644 --- a/documentation/changelog.rst +++ b/documentation/changelog.rst @@ -24,6 +24,7 @@ Bugfixes * The docker-based tutorial now works with UI on all platforms (port 5000 did not expose on MacOS) [see `PR #465 `_] * Fix interpretation of scheduling results in toy tutorial [see `PR #466 `_ and `PR #475 `_] * Avoid formatting datetime.timedelta durations as nominal ISO durations [see `PR #459 `_] +* Account admins cannot add assets to other accounts anymore; and they are shown a button for asset creation in UI [see `PR #488 `_] Infrastructure / Support ---------------------- From adc9e1ea2c06d71d1170abedfa88a5af5e2e2a5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicolas=20H=C3=B6ning?= Date: Fri, 26 Aug 2022 12:26:50 +0200 Subject: [PATCH 4/5] dedidcated check for showing the delete button, apply creation check to all template rendering calls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Nicolas Höning --- flexmeasures/ui/crud/assets.py | 18 ++++++++++++++++-- flexmeasures/ui/templates/crud/asset.html | 4 +++- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/flexmeasures/ui/crud/assets.py b/flexmeasures/ui/crud/assets.py index d36b165df..62cece23c 100644 --- a/flexmeasures/ui/crud/assets.py +++ b/flexmeasures/ui/crud/assets.py @@ -152,8 +152,15 @@ def expunge_asset(): def user_can_create_assets() -> bool: try: check_access(current_user.account, "create-children") - except Exception as exc: - current_app.logger.error(exc) + except Exception: + return False + return True + + +def user_can_delete(asset) -> bool: + try: + check_access(asset, "delete") + except Exception: return False return True @@ -224,6 +231,7 @@ def owned_by(self, account_id: str): account=Account.query.get(account_id), assets=assets, msg=msg, + user_can_create_assets=user_can_create_assets(), ) @login_required @@ -260,6 +268,8 @@ def get(self, id: str): latest_measurement_time_str=latest_measurement_time_str, asset_plot_html=asset_plot_html, mapboxAccessToken=current_app.config.get("MAPBOX_ACCESS_TOKEN", ""), + user_can_create_assets=user_can_create_assets(), + user_can_delete_asset=user_can_delete(asset), ) @login_required @@ -345,6 +355,8 @@ def post(self, id: str): latest_measurement_time_str=latest_measurement_time_str, asset_plot_html=asset_plot_html, mapboxAccessToken=current_app.config.get("MAPBOX_ACCESS_TOKEN", ""), + user_can_create_assets=user_can_create_assets(), + user_can_delete_asset=user_can_delete(asset), ) patch_asset_response = InternalApi().patch( url_for("AssetAPI:patch", id=id), @@ -376,6 +388,8 @@ def post(self, id: str): latest_measurement_time_str=latest_measurement_time_str, asset_plot_html=asset_plot_html, mapboxAccessToken=current_app.config.get("MAPBOX_ACCESS_TOKEN", ""), + user_can_create_assets=user_can_create_assets(), + user_can_delete_asset=user_can_delete(asset), ) @login_required diff --git a/flexmeasures/ui/templates/crud/asset.html b/flexmeasures/ui/templates/crud/asset.html index 992a01855..cfc98ff7c 100644 --- a/flexmeasures/ui/templates/crud/asset.html +++ b/flexmeasures/ui/templates/crud/asset.html @@ -11,12 +11,14 @@
- {% if user_is_admin %} + {% if user_can_create_assets %}
+ {% endif %} + {% if user_can_delete_asset %}
From ddd776a55c2c45401978eaba7f45839f4c975835 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicolas=20H=C3=B6ning?= Date: Fri, 26 Aug 2022 12:27:45 +0200 Subject: [PATCH 5/5] fix count of assets in account for logged-in user view; also the link to those assets on the page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Nicolas Höning --- flexmeasures/ui/templates/admin/logged_in_user.html | 2 +- flexmeasures/ui/templates/crud/assets.html | 7 ++++++- flexmeasures/ui/views/logged_in_user.py | 13 ++++++++----- 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/flexmeasures/ui/templates/admin/logged_in_user.html b/flexmeasures/ui/templates/admin/logged_in_user.html index ce05b0be8..8dfc028c3 100644 --- a/flexmeasures/ui/templates/admin/logged_in_user.html +++ b/flexmeasures/ui/templates/admin/logged_in_user.html @@ -49,7 +49,7 @@

User overview

Assets in account - {{ num_assets }} + {{ num_assets }} diff --git a/flexmeasures/ui/templates/crud/assets.html b/flexmeasures/ui/templates/crud/assets.html index 5d4d667af..a805f86cd 100644 --- a/flexmeasures/ui/templates/crud/assets.html +++ b/flexmeasures/ui/templates/crud/assets.html @@ -10,7 +10,12 @@
-

Asset overview

+

Asset overview + {% if account %} + for account {{ account.name }} + {% endif %} +

+ diff --git a/flexmeasures/ui/views/logged_in_user.py b/flexmeasures/ui/views/logged_in_user.py index 01e3be569..7c69112f4 100644 --- a/flexmeasures/ui/views/logged_in_user.py +++ b/flexmeasures/ui/views/logged_in_user.py @@ -2,20 +2,23 @@ from flask_security import login_required from flexmeasures.ui.views import flexmeasures_ui -from flexmeasures.data.services.resources import get_assets +from flexmeasures.data.models.generic_assets import GenericAsset from flexmeasures.ui.utils.view_utils import render_flexmeasures_template @flexmeasures_ui.route("/logged-in-user", methods=["GET"]) @login_required def logged_in_user_view(): - """TODO: - - Show account name & roles - - Count their assets with a query, link to their (new) list """ + Basic information about the currently logged-in user. + Plus basic actions (logout, reset pwd) + """ + num_assets_in_account = GenericAsset.query.filter( + GenericAsset.account_id == current_user.account_id + ).count() return render_flexmeasures_template( "admin/logged_in_user.html", logged_in_user=current_user, roles=",".join([role.name for role in current_user.roles]), - num_assets=len(get_assets()), + num_assets=num_assets_in_account, )