Skip to content

Fix auth issues: child asset creation, account page access#2163

Merged
Flix6x merged 8 commits into
mainfrom
copilot/fix-child-assets-auth-issues
May 13, 2026
Merged

Fix auth issues: child asset creation, account page access#2163
Flix6x merged 8 commits into
mainfrom
copilot/fix-child-assets-auth-issues

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 12, 2026 •
edited
Loading

  • Fix 1 & 2: AssetAPI.post - remove decorator, add inline permission check on parent asset or target account (+ null guard for non-existent parent)
  • Fix 3: AccountCrudUI.get - add check_access(account, "read") before rendering (+ null guard for non-existent account)
  • Add test: regular user can create child asset via POST
  • Add test: account-admin cannot create child under cross-account parent
  • Add test: user from different account gets 403 on account page
  • Add test: breadcrumb of child asset with cross-account parent shows parent account name
  • Add changelog entries for bugfixes (PR Fix auth issues: child asset creation, account page access #2163)
  • Fix vague comment (name GenericAssetSchema.validate_account explicitly)
  • Fix auth bug: non-admins could create public assets (account_id=None) when omitting account_id from request body — @validates("account_id") is not called for absent fields; added explicit Forbidden for non-admins
  • Add test: regular user cannot create a public asset (account_id=None)
  • Add test: regular user cannot create a child asset under a public parent
  • Update AGENTS.md: auth concerns must be verified with tests, not just code inspection

Copilot AI requested a review from Flix6x May 12, 2026 13:09
Copilot finished work on behalf of Flix6x May 12, 2026 13:09
@Flix6x Flix6x added the Auth label May 12, 2026
@Flix6x Flix6x added this to the 0.33.0 milestone May 12, 2026
Flix6x
Flix6x requested changes May 12, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@Flix6x Flix6x left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sure to update both agent instructions and all contents of this PR in accordance with my review.

Comment thread documentation/changelog.rst
Comment thread flexmeasures/ui/tests/test_account_crud.py Outdated
Comment thread flexmeasures/api/v3_0/assets.py Outdated
Comment thread flexmeasures/ui/tests/test_asset_crud.py Outdated
Comment thread flexmeasures/api/v3_0/tests/test_assets_api.py Outdated
Comment thread flexmeasures/api/v3_0/tests/test_assets_api.py Outdated
Comment thread flexmeasures/api/v3_0/tests/test_assets_api.py Outdated
Copilot AI and others added 2 commits May 12, 2026 14:55
@Flix6x
fix: address review comments - docstrings, changelog, None account check
58a5f0e 
Agent-Logs-Url: https://github.com/FlexMeasures/flexmeasures/sessions/e5059bde-7621-45d1-a6db-bb2f0025c04d

Co-authored-by: Flix6x <30658763+Flix6x@users.noreply.github.com>
@Flix6x
agents/test-specialist: enforce docstring hygiene for tests (PR #2163 …
8ece770 
…review)

Context:
- PR #2163 reviewer (@Flix6x) requested three specific docstring improvements
- Test docstrings contained 'Bug (on main)' / 'Fix:' / 'Expected: X on main, Y with fix'
  paragraphs describing pre-fix behaviour, which belong in commit messages not source code
- Sentences were hard-wrapped mid-phrase to fit ~79 chars, making docstrings harder to read
- Double spaces after punctuation appeared in docstrings despite the rule existing in AGENTS.md

Change:
- Added 'Docstring Style Rules' section with three enforced sub-rules:
  1. No historical context ('on main', 'before/after fix') in test docstrings
  2. Keep sentences on as few lines as possible; avoid mid-sentence line breaks
  3. No double spaces after punctuation (mirrors AGENTS.md rule, now explicit in this agent)

Co-authored-by: Flix6x <30658763+Flix6x@users.noreply.github.com>
Copilot finished work on behalf of Flix6x May 12, 2026 14:59
Copilot AI requested a review from Flix6x May 12, 2026 14:59
Flix6x
Flix6x requested changes May 12, 2026
View reviewed changes
Comment thread flexmeasures/api/v3_0/assets.py Outdated
Copilot finished work on behalf of Flix6x May 12, 2026 15:29
Copilot AI requested a review from Flix6x May 12, 2026 15:29
@Flix6x Flix6x marked this pull request as ready for review May 13, 2026 12:49
@Flix6x
fix: block non-admins from creating public assets; add regression tests
7fd9cfa 
When account_id is absent from a POST /assets request body, Marshmallow
does not invoke the @validates("account_id") field validator, so the
inline `if account_id is not None:` guard silently skipped the permission
check. Any authenticated user could create a public (account-less) asset.

Fix: raise Forbidden when account_id is None and the caller is neither a
site admin nor CLI.

Add tests:
- test_regular_user_cannot_create_public_asset
- test_regular_user_cannot_create_child_of_public_asset

Update AGENTS.md: auth concerns must be covered by tests, not code
inspection alone.

Agent-Logs-Url: https://github.com/FlexMeasures/flexmeasures/sessions/b55c75f0-c9c5-4d45-9f17-4477d6cb4760

Co-authored-by: Flix6x <30658763+Flix6x@users.noreply.github.com>
Copilot finished work on behalf of Flix6x May 13, 2026 13:21
nhoening
nhoening approved these changes May 13, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@nhoening nhoening left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks.

I only found a few comment lines I think should be deleted.

Comment thread flexmeasures/api/v3_0/tests/test_assets_api.py Outdated
@Flix6x
docs: remove comments about fixes
597b375 
Signed-off-by: F.N. Claessen <claessen@seita.nl>
@Flix6x Flix6x merged commit 508603a into main May 13, 2026
6 checks passed
@Flix6x Flix6x deleted the copilot/fix-child-assets-auth-issues branch May 13, 2026 14:28
@Flix6x Flix6x mentioned this pull request May 14, 2026
Merged
3 tasks
@Flix6x Flix6x modified the milestones: 0.33.0, 0.32.3 May 15, 2026
Flix6x pushed a commit that referenced this pull request May 15, 2026
@Flix6x
Backport PR #2163: Fix auth issues: child asset creation, account pag…
2e8a1bd 
…e access (#2163)

* fix: auth issues for child asset creation, account page access, and breadcrumb test

Agent-Logs-Url: https://github.com/FlexMeasures/flexmeasures/sessions/bbb22573-9b83-4d71-ac1b-f8b9de70f61b

Co-authored-by: Flix6x <30658763+Flix6x@users.noreply.github.com>

* fix: add null checks and changelog entry for auth fixes

Agent-Logs-Url: https://github.com/FlexMeasures/flexmeasures/sessions/bbb22573-9b83-4d71-ac1b-f8b9de70f61b

Co-authored-by: Flix6x <30658763+Flix6x@users.noreply.github.com>

* fix: update changelog with correct PR number #2163

Agent-Logs-Url: https://github.com/FlexMeasures/flexmeasures/sessions/bbb22573-9b83-4d71-ac1b-f8b9de70f61b

Co-authored-by: Flix6x <30658763+Flix6x@users.noreply.github.com>

* fix: address review comments - docstrings, changelog, None account check

Agent-Logs-Url: https://github.com/FlexMeasures/flexmeasures/sessions/e5059bde-7621-45d1-a6db-bb2f0025c04d

Co-authored-by: Flix6x <30658763+Flix6x@users.noreply.github.com>

* agents/test-specialist: enforce docstring hygiene for tests (PR #2163 review)

Context:
- PR #2163 reviewer (@Flix6x) requested three specific docstring improvements
- Test docstrings contained 'Bug (on main)' / 'Fix:' / 'Expected: X on main, Y with fix'
  paragraphs describing pre-fix behaviour, which belong in commit messages not source code
- Sentences were hard-wrapped mid-phrase to fit ~79 chars, making docstrings harder to read
- Double spaces after punctuation appeared in docstrings despite the rule existing in AGENTS.md

Change:
- Added 'Docstring Style Rules' section with three enforced sub-rules:
  1. No historical context ('on main', 'before/after fix') in test docstrings
  2. Keep sentences on as few lines as possible; avoid mid-sentence line breaks
  3. No double spaces after punctuation (mirrors AGENTS.md rule, now explicit in this agent)

Co-authored-by: Flix6x <30658763+Flix6x@users.noreply.github.com>

* fix: clarify public-asset comment - name schema and validator

Agent-Logs-Url: https://github.com/FlexMeasures/flexmeasures/sessions/df191792-501f-4b9e-8502-3c3ec78fa364

Co-authored-by: Flix6x <30658763+Flix6x@users.noreply.github.com>

* fix: block non-admins from creating public assets; add regression tests

When account_id is absent from a POST /assets request body, Marshmallow
does not invoke the @validates("account_id") field validator, so the
inline `if account_id is not None:` guard silently skipped the permission
check. Any authenticated user could create a public (account-less) asset.

Fix: raise Forbidden when account_id is None and the caller is neither a
site admin nor CLI.

Add tests:
- test_regular_user_cannot_create_public_asset
- test_regular_user_cannot_create_child_of_public_asset

Update AGENTS.md: auth concerns must be covered by tests, not code
inspection alone.

Agent-Logs-Url: https://github.com/FlexMeasures/flexmeasures/sessions/b55c75f0-c9c5-4d45-9f17-4477d6cb4760

Co-authored-by: Flix6x <30658763+Flix6x@users.noreply.github.com>

* docs: remove comments about fixes

Signed-off-by: F.N. Claessen <claessen@seita.nl>

---------

Signed-off-by: F.N. Claessen <claessen@seita.nl>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Flix6x <30658763+Flix6x@users.noreply.github.com>
Co-authored-by: F.N. Claessen <claessen@seita.nl>

(cherry picked from commit 508603a)
Signed-off-by: F.N. Claessen <claessen@seita.nl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nhoening nhoening nhoening approved these changes

+1 more reviewer

@Flix6x Flix6x Flix6x approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@Flix6x Flix6x

Labels

AI Infra Auth

Projects

None yet

Milestone

0.32.3

Development

Successfully merging this pull request may close these issues.

3 participants

@nhoening @Flix6x