Mask locations of inaccessible assets on map

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 <http://www.github.com/FlexMeasures/flexmeasures/pull/409>`_]
+
+
 v0.9.0 | March 25, 2022
 ===========================
 

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."""

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
 

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

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

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(),
         )
     )
 

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,

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(

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.

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