OpenEnergyPlatform / OpenEnergyPlatform/oeplatform
OEDB endpoints enforce permissions outside DRF, so authorisation cannot be audited — and there is no deny-by-default
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
The OEDB half of api/v0 enforces permissions in ways that nothing outside the function
body can see. The result is that authorisation cannot be audited, reviewed, or tested as a
property of a view — only by reading each method and every function it calls.
Three mechanisms are in use across api/views.py, and only the third is introspectable:
- Hand-rolled method decorators —
@require_write_permission,
@require_delete_permission,@require_admin_permission(api/helper.py:243-261),
which callassert_permission(user=request.user, …). - Checks inside the action function — the
advanced/family.create_ajax_handler
(api/helper.py:293) builds anAPIViewwith no permission classes and passes
context = {"user": request.user}down; the action does the check, e.g.
data_delete(api/actions.py:1784). - DRF permission classes — used by the dataset endpoints (
IsAuthenticated,
IsAuthenticatedOrReadOnly) and by the OEKG scenario-bundle endpoints
(get_permissions()).
Only the third is visible to anything but a human reader.
Why this is worth an issue of its own
There is no DEFAULT_PERMISSION_CLASSES in REST_FRAMEWORK
(oeplatform/settings.py:417-431). DRF's fallback is therefore AllowAny, so a view is
public unless someone remembered to add one of the three mechanisms above. Nothing fails
when they do not:
- Review cannot catch it — an unguarded write method looks exactly like a guarded one
minus a line, and the decorators sit among@api_exceptionand@method_decorator(...)
that carry no security meaning. - The schema cannot catch it — drf-spectacular reads permission classes, so mechanisms 1
and 2 are invisible to it. This is the root cause of #2481: 61 write operations are
declared anonymous in the generated document because the generator genuinely cannot see
their guards. A deliberately public read and a write whose guard was forgotten
both render as{}, and are indistinguishable in the artifact. - Tests do not catch it — there is no test asserting that writes require
authentication, and six existing tests hedgeassertIn(code, (401, 403))rather than
pinning either, which is defensive rather than evidential.
So the safety of the OEDB API rests entirely on each contributor remembering an unenforced
convention, in a file where the three conventions are mixed.
Ideas of solution
1. Deny by default
Set DEFAULT_PERMISSION_CLASSES to IsAuthenticated and make every genuinely public
endpoint say so explicitly with AllowAny. This inverts the failure mode: forgetting a
guard produces a 401 that a test or a user reports, instead of an open endpoint nobody
notices. It is the single highest-value change here and it is a few lines plus an audit of
which reads are meant to be public — the generated schema already lists exactly those
(28 reads carry {}).
2. Express the existing guards as permission classes
The decorators already compute the right answer; wrapping them as DRF permission classes
(or declaring them with extend_schema(auth=...)) makes them visible to the generator, to
the browsable API, and to anything else reading DRF metadata. The advanced/ family is one
factory (create_ajax_handler), so it is one change for all 31 of its operations.
This is what closes #2481 properly — not by editing the artifact, which is generated and
guarded, but by making the document able to tell the truth.
3. A test that fails when a write is anonymous
Assert over the generated schema that no write operation declares anonymous access unless
it is on a short, explicit allowlist. This is the check that would turn "somebody forgot"
from an invisible state into a red build, and it is cheap because the artifact is already
committed and already regenerated in CI by #2457's guard.
Acceptance criteria
-
DEFAULT_PERMISSION_CLASSESset to a deny-by-default, with every intentionally public
endpoint markedAllowAnyexplicitly. - The existing permission decorators and in-action checks are expressed so that DRF —
and therefore the schema generator — can see them. - A test fails if a write operation in the generated schema permits anonymous access
outside an explicit allowlist. - The 28 genuinely public reads keep their public declaration; this must not be "fixed"
by requiring authentication everywhere. - Artifact regenerated,
api/tests/test_openapi_schema.pygreen, changelog entry.
Relationship to other issues
- #2481 is the symptom in the generated document. This issue is why it happens; fixing
this fixes that. - #2454 / #2475 are the documentation effort, and this is not part of it — it is a
change to howapi/expresses authorisation, not to how the API is described.
Note
Filed after an audit of every write method in api/views.py against its guard. That audit
produced one finding that is not described here and has been reported to the maintainer
separately rather than in a public issue.
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 REST_FRAMEWORK in oeplatform/settings.py:417-431, then compare the permission decorators in api/helper.py:243-261, create_ajax_handler at api/helper.py:293, and the checks in api/actions.py:1784. Review api/views.py and api/tests/test_openapi_schema.py to understand existing endpoint declarations and schema checks. Done means intentional public reads remain explicit, writes are not anonymous, and the schema test and regenerated artifact pass.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- django, python
- Domain
- api, authentication, backend, security
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100