diff --git a/documentation/changelog.rst b/documentation/changelog.rst index f1e2d53c9..6bb5911ee 100644 --- a/documentation/changelog.rst +++ b/documentation/changelog.rst @@ -16,6 +16,14 @@ Infrastructure / Support ---------------------- +v0.9.1 | March 31, 2022 +=========================== + +Bugfixes +-------- +* Fix auth bug not masking locations of inaccessible assets on map [see `PR #409 `_] + + v0.9.0 | March 25, 2022 =========================== diff --git a/flexmeasures/api/common/responses.py b/flexmeasures/api/common/responses.py index 143512458..7b30465c4 100644 --- a/flexmeasures/api/common/responses.py +++ b/flexmeasures/api/common/responses.py @@ -11,6 +11,19 @@ ResponseTuple = Tuple[dict, int] +def is_response_tuple(value) -> bool: + """Check if an object qualifies as a ResponseTuple""" + if not isinstance(value, tuple): + return False + if not len(value) == 2: + return False + if not isinstance(value[0], dict): + return False + if not isinstance(value[1], int): + return False + return True + + class BaseMessage: """Set a base message to which extra info can be added by calling the wrapped function with additional string arguments. This is a decorator implemented as a class.""" diff --git a/flexmeasures/api/v1_1/implementations.py b/flexmeasures/api/v1_1/implementations.py index 0b94c382b..37a59f270 100644 --- a/flexmeasures/api/v1_1/implementations.py +++ b/flexmeasures/api/v1_1/implementations.py @@ -13,8 +13,8 @@ from flexmeasures.api.common.responses import ( invalid_domain, invalid_unit, - ResponseTuple, invalid_horizon, + is_response_tuple, ) from flexmeasures.api.common.utils.api_utils import ( save_and_enqueue, @@ -98,7 +98,7 @@ def post_price_data_response( # Look for the Sensor object sensor = get_sensor_by_unique_name(market_name, ["day_ahead", "tou_tariff"]) - if type(sensor) == ResponseTuple: + if is_response_tuple(sensor): # Error message telling the user what to do return sensor if unit != sensor.unit: @@ -185,7 +185,7 @@ def post_weather_data_response( # noqa: C901 sensor = get_sensor_by_generic_asset_type_and_location( weather_sensor_type_name, latitude, longitude ) - if type(sensor) == ResponseTuple: + if is_response_tuple(sensor): # Error message telling the user about the nearest weather sensor they can post to return sensor diff --git a/flexmeasures/cli/__init__.py b/flexmeasures/cli/__init__.py index 8718348bb..20f98d921 100644 --- a/flexmeasures/cli/__init__.py +++ b/flexmeasures/cli/__init__.py @@ -1,4 +1,6 @@ -from flask import Flask +import sys + +from flask import Flask, current_app def register_at(app: Flask): @@ -13,3 +15,27 @@ def register_at(app: Flask): import flexmeasures.cli.data_delete import flexmeasures.cli.db_ops import flexmeasures.cli.testing # noqa: F401 + + +def is_running() -> bool: + """ + True if we are running one of the custom FlexMeasures CLI commands. + + We use this in combination with authorization logic, e.g. we assume that only sysadmins run commands there, + but also we consider forecasting & scheduling jobs to be in that realm, as well. + + This tooling might not live forever, as we could evolve into a more sophisticated auth model for these cases. + For instance, these jobs are queued by the system, but caused by user actions (sending data), and then they are run by the system. + + See also: the run_as_cli test fixture, which uses the (non-public) PRETEND_RUNNING_AS_CLI env setting. + + TODO: How can plugins add their CLI set here, should they need that? + """ + cli_sets = ("add", "delete", "show", "monitor", "jobs", "db-ops") + command_line = " ".join(sys.argv) + for cli_set in cli_sets: + if f"flexmeasures {cli_set}" in command_line: + return True + if current_app.config.get("PRETEND_RUNNING_AS_CLI", False): + return True + return False diff --git a/flexmeasures/conftest.py b/flexmeasures/conftest.py index 29af4f134..8438cb1c6 100644 --- a/flexmeasures/conftest.py +++ b/flexmeasures/conftest.py @@ -739,6 +739,17 @@ def battery_soc_sensor(db: SQLAlchemy, setup_generic_assets): return soc_sensor +@pytest.fixture +def run_as_cli(app, monkeypatch): + """ + Use this to run your test as if it is run from the CLI. + This is useful where some auth restrictions (e.g. for querying) are in place. + FlexMeasures is more lenient with them if the CLI is running, as it considers + the user a sysadmin. + """ + monkeypatch.setitem(app.config, "PRETEND_RUNNING_AS_CLI", True) + + @pytest.fixture(scope="function") def clean_redis(app): failed = app.queues["forecasting"].failed_job_registry diff --git a/flexmeasures/data/queries/utils.py b/flexmeasures/data/queries/utils.py index 0817debbb..5a7e7b9b6 100644 --- a/flexmeasures/data/queries/utils.py +++ b/flexmeasures/data/queries/utils.py @@ -1,18 +1,19 @@ from typing import List, Optional, Type, Tuple, Union from datetime import datetime, timedelta -from flask import current_app from flask_security import current_user from werkzeug.exceptions import Forbidden import pandas as pd import timely_beliefs as tb from sqlalchemy.orm import Query, Session -from sqlalchemy.sql.elements import BinaryExpression +from sqlalchemy.sql.elements import BinaryExpression, or_ +from sqlalchemy.sql.expression import null from flexmeasures.data.config import db from flexmeasures.data.models.generic_assets import GenericAsset from flexmeasures.data.models.data_sources import DataSource from flexmeasures.utils import flexmeasures_inflection +from flexmeasures.cli import is_running as running_as_cli import flexmeasures.data.models.time_series as ts # noqa: F401 from flexmeasures.auth.policy import ADMIN_ROLE, ADMIN_READER_ROLE @@ -49,10 +50,10 @@ def potentially_limit_query_to_account_assets( :param account_id: if set, all assets that are not in the given account will be filtered out (only works for admins and CLI users). For querying public assets in particular, don't use this function. """ - if not current_app.cli and not current_user.is_authenticated: + if not running_as_cli() and not current_user.is_authenticated: raise Forbidden("Unauthenticated user cannot list assets.") user_is_admin = ( - current_app.cli + running_as_cli() or current_user.has_role(ADMIN_ROLE) or current_user.has_role(ADMIN_READER_ROLE) ) @@ -68,9 +69,9 @@ def potentially_limit_query_to_account_assets( account_id if account_id is not None else current_user.account_id ) return query.filter( - ( - GenericAsset.account_id == account_id_to_filter - or GenericAsset.account_id is None + or_( + GenericAsset.account_id == account_id_to_filter, + GenericAsset.account_id == null(), ) ) diff --git a/flexmeasures/data/services/asset_grouping.py b/flexmeasures/data/services/asset_grouping.py index d6a73c756..b037f662c 100644 --- a/flexmeasures/data/services/asset_grouping.py +++ b/flexmeasures/data/services/asset_grouping.py @@ -28,7 +28,7 @@ def get_asset_group_queries( group_by_location: bool = False, ) -> Dict[str, Query]: """ - An asset group is defined by Asset queries. Each query has a name, and we prefer pluralised names. + An asset group is defined by Asset queries. Each query has a name, and we prefer pluralised names. They still need an executive call, like all(), count() or first(). This function limits the assets to be queried to the current user's account, diff --git a/flexmeasures/data/tests/test_forecasting_jobs.py b/flexmeasures/data/tests/test_forecasting_jobs.py index a8815cd55..c664059c8 100644 --- a/flexmeasures/data/tests/test_forecasting_jobs.py +++ b/flexmeasures/data/tests/test_forecasting_jobs.py @@ -46,7 +46,7 @@ def check_aggregate(overall_expected: int, horizon: timedelta, sensor_id: int): assert all([not np.isnan(f.event_value) for f in all_forecasts]) -def test_forecasting_an_hour_of_wind(db, app, setup_test_data): +def test_forecasting_an_hour_of_wind(db, run_as_cli, app, setup_test_data): """Test one clean run of one job: - data source was made, - forecasts have been made @@ -84,7 +84,9 @@ def test_forecasting_an_hour_of_wind(db, app, setup_test_data): check_aggregate(4, horizon, wind_device_1.id) -def test_forecasting_two_hours_of_solar_at_edge_of_data_set(db, app, setup_test_data): +def test_forecasting_two_hours_of_solar_at_edge_of_data_set( + db, run_as_cli, app, setup_test_data +): solar_device1: Sensor = Sensor.query.filter_by(name="solar-asset-1").one_or_none() last_power_datetime = ( @@ -168,7 +170,9 @@ def check_failures( assert job.meta["model_identifier"] == model_identifiers[job_idx] -def test_failed_forecasting_insufficient_data(app, clean_redis, setup_test_data): +def test_failed_forecasting_insufficient_data( + app, run_as_cli, clean_redis, setup_test_data +): """This one (as well as the fallback) should fail as there is no underlying data. (Power data is in 2015)""" solar_device1: Sensor = Sensor.query.filter_by(name="solar-asset-1").one_or_none() @@ -183,7 +187,9 @@ def test_failed_forecasting_insufficient_data(app, clean_redis, setup_test_data) check_failures(app.queues["forecasting"], 2 * ["NotEnoughDataException"]) -def test_failed_forecasting_invalid_horizon(app, clean_redis, setup_test_data): +def test_failed_forecasting_invalid_horizon( + app, run_as_cli, clean_redis, setup_test_data +): """This one (as well as the fallback) should fail as the horizon is invalid.""" solar_device1: Sensor = Sensor.query.filter_by(name="solar-asset-1").one_or_none() create_forecasting_jobs( diff --git a/flexmeasures/data/tests/test_forecasting_jobs_fresh_db.py b/flexmeasures/data/tests/test_forecasting_jobs_fresh_db.py index b1660b334..a01b1cbfc 100644 --- a/flexmeasures/data/tests/test_forecasting_jobs_fresh_db.py +++ b/flexmeasures/data/tests/test_forecasting_jobs_fresh_db.py @@ -18,7 +18,9 @@ from flexmeasures.utils.time_utils import as_server_time -def test_forecasting_three_hours_of_wind(app, setup_fresh_test_data, clean_redis): +def test_forecasting_three_hours_of_wind( + app, run_as_cli, setup_fresh_test_data, clean_redis +): wind_device2: Sensor = Sensor.query.filter_by(name="wind-asset-2").one_or_none() # makes 12 forecasts @@ -47,7 +49,9 @@ def test_forecasting_three_hours_of_wind(app, setup_fresh_test_data, clean_redis check_aggregate(12, horizon, wind_device2.id) -def test_forecasting_two_hours_of_solar(app, setup_fresh_test_data, clean_redis): +def test_forecasting_two_hours_of_solar( + app, run_as_cli, setup_fresh_test_data, clean_redis +): solar_device1: Sensor = Sensor.query.filter_by(name="solar-asset-1").one_or_none() wind_device2: Sensor = Sensor.query.filter_by(name="wind-asset-2").one_or_none() print(solar_device1) @@ -82,7 +86,12 @@ def test_forecasting_two_hours_of_solar(app, setup_fresh_test_data, clean_redis) "model_to_start_with, model_version", [("failing-test", 1), ("linear-OLS", 2)] ) def test_failed_model_with_too_much_training_then_succeed_with_fallback( - setup_fresh_test_data, app, clean_redis, model_to_start_with, model_version + app, + run_as_cli, + clean_redis, + setup_fresh_test_data, + model_to_start_with, + model_version, ): """ Here we fail once - because we start with a model that needs too much training. diff --git a/flexmeasures/data/tests/test_sensor_queries.py b/flexmeasures/data/tests/test_sensor_queries.py index 241bb0bf9..0cdedd4ee 100644 --- a/flexmeasures/data/tests/test_sensor_queries.py +++ b/flexmeasures/data/tests/test_sensor_queries.py @@ -1,7 +1,7 @@ from flexmeasures.data.models.time_series import Sensor -def test_closest_sensor(add_nearby_weather_sensors): +def test_closest_sensor(run_as_cli, add_nearby_weather_sensors): """Check that the closest temperature sensor to our wind sensor returns the one that is on the same spot as the wind sensor itself. (That's where we set it up in our conftest.)