OpenEnergyPlatform / OpenEnergyPlatform/oeplatform

The generated OpenAPI document declares 61 write endpoints as anonymous, including table and row DELETE

Open
#2,481 1 comment 0 reactions 0 assignees View on GitHub

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 decoratorsapi/views.py:

@api_exception
@require_delete_permission          # TableAPIView.delete
def delete(self, request, table): ...

require_delete_permissionpermission_wrapperassert_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 SQL advanced/delete and advanced/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:

  1. 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.
  2. Declare it where the family is uniform. The 31 advanced/ operations are built by
    one factory (create_ajax_handler), so a single extend_schema_view there covers all of
    them — the same place the tags=["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.yaml declares anonymous access ({} in security)
    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.
  • 401 is a documented response where assert_permission can raise it.
  • The artifact is regenerated, committed, and api/tests/test_openapi_schema.py green.
  • 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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.