Check read permissions for regressor sensors when triggering forecasts#2096
Check read permissions for regressor sensors when triggering forecasts#2096
Conversation
Documentation build overview
19 files changed ·
|
Agent-Logs-Url: https://github.com/FlexMeasures/flexmeasures/sessions/9b3ff676-cf08-4d79-8cd1-657ef1581d40 Co-authored-by: BelhsanHmida <149331360+BelhsanHmida@users.noreply.github.com>
…mplify asset type name Agent-Logs-Url: https://github.com/FlexMeasures/flexmeasures/sessions/9b3ff676-cf08-4d79-8cd1-657ef1581d40 Co-authored-by: BelhsanHmida <149331360+BelhsanHmida@users.noreply.github.com>
|
@copilot Please use PR #1071 as context for this branch: This branch is addressing a similar permissions issue, but for forecasting regressors. In particular, please compare the authorization semantics and implementation shape:
I’d like something that is consistent with the intent of #1071, while still fitting this branch cleanly. |
Agent-Logs-Url: https://github.com/FlexMeasures/flexmeasures/sessions/564ced47-b413-4ca8-a7d4-3b064d6d8e43 Co-authored-by: BelhsanHmida <149331360+BelhsanHmida@users.noreply.github.com>
... Aligned with PR #1071's pattern in commit
|
|
Manually tested this end-to-end in OpenAPI with a plain non-admin user from the target sensor’s account. Setup:
Results:
This gives the expected behavior: permission to trigger forecasting on the target sensor does not automatically allow indirect access to regressor sensors. Referenced regressors now need to be readable as well. |
Flix6x
left a comment
Flix6x
left a comment
There was a problem hiding this comment.
Looks nice, and thanks for reporting on your tests, too.
Just a changelog entry, please, and one request to enhance the test.
| trigger_res = client.post( | ||
| trigger_url, json=payload, headers={"Authorization": token} | ||
| ) | ||
| assert trigger_res.status_code == 403 |
There was a problem hiding this comment.
Is there anything in the response message that we could check that let's the user know which sensor or which field (e.g. "config", or even better, regressor_field) was to blame?
The
POST /<sensor>/forecasts/triggerendpoint checkedcreate-childrenpermissions for the target sensor but silently skippedreadpermission checks for regressor sensors (future-regressors,past-regressors,regressorsinconfig), allowing users to reference sensors they shouldn't be able to see.Changes
Permission check (
api/v3_0/sensors.py): Added aregressors_loaderfunction (analogous toflex_model_loader/flex_context_loaderin PR 904 read permissions of entities within flex model or flex context #1071) that extracts regressorSensorobjects from the deserialized config. A declarative@permission_required_for_context("read", ctx_arg_name="config", ctx_loader=regressors_loader, pass_ctx_to_loader=True)decorator ontrigger_forecastenforcesreadaccess on each regressor sensor before the job is dispatched.Schema (
data/schemas/forecasting/pipeline.py):ForecastingTriggerSchema.configgainsload_default={}so the kwarg is always present in the decorator chain (mirrors howflex_model/flex_contextare handled in PR 904 read permissions of entities within flex model or flex context #1071).ForecasterParametersSchema.resolve_confignow passes through extra keys from subclass schemas soconfigsurvives the@post_loadtransformation.Auth decorator (
auth/decorators.py):permission_required_for_contextis extended with a_check_access_for_contexthelper that normalises a loader result to a list and callscheck_accessfor each item, enabling multi-sensor permission checks from a single decorator.Tests (
api/v3_0/tests/test_forecasting_api.py): Addedtest_trigger_forecast_with_unreadable_regressor_returns_403, parametrized over all three regressor fields. Creates a target sensor on the requesting user's account (hascreate-children) and a regressor sensor on a different account (noreadaccess), asserts 403.