OpenEnergyPlatform / OpenEnergyPlatform/oeplatform
The generated OpenAPI document declares 61 write endpoints as anonymous, including table and row DELETE
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 65
- Forks
- 29
- Avg merge
- 15h 25m
- Merged PRs (30d)
- 32
Description
Description of the issue
No endpoint is actually unprotected — this is a documentation defect, not a security
hole. It is filed because the document understates security, which is the dangerous
direction to be wrong in, and because the obvious "fix" is worse than the defect.
Surfaced while reading the rendered API Reference from #2474: the padlock next to every
GET renders closed and next to every write open, which reads backwards. That part is a
Swagger UI convention (a closed padlock means security already satisfied, so a public
read renders locked) and #2475 covers it in prose. Checking why the icons differ turned
up something else.
61 write operations are declared callable anonymously. Measured on
docs/oeplatform-code/web-api/openapi.yaml at develop — every operation whose security
array contains an empty {}, which in OpenAPI means no authentication is also
acceptable:
| family | operations declared anonymous |
|---|---|
POST /api/v0/advanced/* |
31, incl. advanced/delete, advanced/update, advanced/insert |
/api/v0/tables/{table}/… writes |
15, incl. DELETE /api/v0/tables/{table}/ and DELETE …/rows/{row_id} |
/api/v0/schema/…/tables/{table}/… writes |
15, the same set under the schema-qualified prefix |
The 28 reads that also carry {} are correct — those endpoints are genuinely public.
And all 20 scenario-bundle operations are correct in both directions: the reads carry
{}, the writes do not.
Why, and it is one root cause with two spellings
Every one of the 61 is really guarded. drf-spectacular reads DRF permission_classes; none
of these views has any, because they enforce permissions somewhere the generator cannot
see:
1. Hand-rolled method decorators — api/views.py:
@api_exception
@require_delete_permission # TableAPIView.delete
def delete(self, request, table): ...
require_delete_permission → permission_wrapper → assert_permission(user=request.user, table=…, permission=DELETE_PERM). Same shape as @require_write_permission on the row and
column writes.
2. Checks inside the action function — the advanced/ family. create_ajax_handler
(api/helper.py:293) builds an APIView with no permission classes and passes
context = {"user": request.user} down; the action does the check:
def data_delete(request, context): # api/actions.py:1784
assert_permission(user=context["user"], table=table_obj, permission=DELETE_PERM)
And assert_permission (api/actions.py:133) rejects anonymous callers explicitly:
if user.is_anonymous or not isinstance(user, User):
raise APIError("User is anonymous", 401)
So an anonymous DELETE /api/v0/tables/{table}/ answers 401. The document says it is
allowed. The OEKG endpoints describe correctly for the one reason that has nothing to do
with their being newer: they use get_permissions() / permission_classes, which
drf-spectacular can read.
Why it is worth fixing rather than tolerating
#2454 rules OEDB annotation out of scope and accepts "the OEDB is described twice" as the
cost. That was accepted as duplication. This is a different cost: a wrong security
claim in the artifact a CI guard now protects, so it is stable and reproducible rather
than drifting. Concretely:
- A generated client would be written believing table deletion needs no credentials.
- Anyone auditing the API from the document would conclude 61 write endpoints — including
raw SQLadvanced/deleteandadvanced/update— are open to the world. - The trap: someone "fixing" it could add authentication to endpoints that already have it,
or hand-edit the artifact the drift guard exists to keep generated.
Ideas of solution
Do not hand-edit openapi.yaml — it is generated, and #2457's guard regenerates and
compares it.
Two mechanisms, and they can be mixed per family:
- Make the existing guards visible. A DRF permission class (or
extend_schema(auth=...)on the operation) stating what the decorator already enforces.
For the table and row views the mapping is direct:@require_write_permission→
authenticated-and-write-permitted. - Declare it where the family is uniform. The 31
advanced/operations are built by
one factory (create_ajax_handler), so a singleextend_schema_viewthere covers all of
them — the same place thetags=["Advanced"]decoration already sits, which is prior art
that this hook works.
Whichever is chosen, the artifact is regenerated and committed, and the 401 that
assert_permission actually raises should be a documented response — it is missing on all
61 as well.
Acceptance criteria
- No write operation in
openapi.yamldeclares anonymous access ({}insecurity)
unless it genuinely permits it. - The reads that are public keep their
{}— this must not be "fixed" by declaring
authentication everywhere, which would misdescribe them in the opposite direction. -
401is a documented response whereassert_permissioncan raise it. - The artifact is regenerated, committed, and
api/tests/test_openapi_schema.pygreen. - A test asserting no write declares anonymous access, so this cannot silently return.
- Changelog entry.
Notes
Everything above was read from the code on develop, not inferred: TableAPIView.delete and
TableRowsAPIView.post/put/delete carry the decorators, data_delete carries the in-action
check, and assert_permission raises 401 for anonymous. No route was found where the
document's claim is true.
Scope relative to the OEKG work: the scenario-bundle endpoints need no change here.
#2475 covers stating their auth expectation in prose; this issue is the other 61.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with api/views.py, api/helper.py:293, api/actions.py:133 and :1784, then inspect api/tests/test_openapi_schema.py. Compare the permission decorators and in-action checks with the generated docs, and run the schema test before regenerating docs/oeplatform-code/web-api/openapi.yaml. Done means only genuinely public reads declare anonymous access, protected writes document authentication and 401 responses, the artifact is committed, the regression test passes, and the changelog is updated.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- django, openapi, python
- Domain
- api, documentation, security, testing
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 52/100