fix(security): enforce object-level permissions on admin JSON endpoints by phusi319 · Pull Request #210 · openwisp/django-loci

phusi319 · 2026-03-26T23:08:33Z

Summary

This PR adds explicit object-level authorization checks to the custom admin JSON endpoints for locations.

Security issue

The custom admin endpoints:

/admin/.../location/<uuid>/json/
/admin/.../location/<uuid>/floorplans/json/

retrieved objects and returned data without checking has_view_or_change_permission for the target instance.

Even though these views are wrapped with admin_view, they should still enforce object-level checks before returning potentially sensitive location/floorplan data.

Fix

In django_loci/base/admin.py:
- import PermissionDenied
- add has_view_or_change_permission(request, obj=instance) checks in:
  - json_view
  - floorplans_json_view
- raise PermissionDenied when unauthorized
In django_loci/tests/base/test_admin.py:
- add regression tests verifying users without location view permission receive 403 on both JSON endpoints.

Notes

Could not fully execute project tests locally because this environment misses OpenWISP runtime dependencies (openwisp2) required by the test setup. Added focused regression tests for CI to validate behavior.

Signed-off-by: Phu Si On phusi319@users.noreply.github.com

Signed-off-by: Phu Si On <phusi319@users.noreply.github.com>

coderabbitai · 2026-03-26T23:08:48Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 12eb590d-5398-4136-abae-ea458486c0ad

📥 Commits

Reviewing files that changed from the base of the PR and between 281f613 and cd53db0.

📒 Files selected for processing (2)

django_loci/base/admin.py
django_loci/tests/base/test_admin.py

📜 Recent review details

🧰 Additional context used

📓 Path-based instructions (2)

**/*.{js,ts,py,java,go,rb,php,sh,bash,yml,yaml}

📄 CodeRabbit inference engine (Custom checks)

Cryptic or non-obvious code (regex, complex bash commands, or hard-to-read code) must include a concise comment explaining why it is needed and why the complexity is acceptable

Files:

django_loci/base/admin.py
django_loci/tests/base/test_admin.py

**/*.{js,ts,py,java,go,rb,php}

📄 CodeRabbit inference engine (Custom checks)

**/*.{js,ts,py,java,go,rb,php}: Avoid unnecessary comments or docstrings for code that is already clear
Code formatting must be compact and readable; do not add excessive blank lines, especially inside function or method bodies
Variables, functions, classes, and files must have descriptive and consistent names
New code must handle errors properly: log unresolvable errors with error level, log unusual conditions with warning level, log important background actions with info level, and provide user-facing messages for user-solvable errors

Files:

django_loci/base/admin.py
django_loci/tests/base/test_admin.py

🧠 Learnings (2)

📚 Learning: 2026-03-26T22:29:15.066Z

Learnt from: CR
Repo: openwisp/django-loci PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-03-26T22:29:15.066Z
Learning: Bug Fixes: The regression test must fail if the patch is removed or commented out

Applied to files:

django_loci/tests/base/test_admin.py

📚 Learning: 2026-03-26T22:29:15.066Z

Learnt from: CR
Repo: openwisp/django-loci PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-03-26T22:29:15.066Z
Learning: Bug Fixes: A regression test must be included that reproduces the bug scenario

Applied to files:

django_loci/tests/base/test_admin.py

🔇 Additional comments (3)

django_loci/base/admin.py (2)

181-182: Object-level authorization is correctly enforced in json_view.

This guard closes the data exposure path by denying access before returning JSON for unauthorized users.

197-198: floorplans_json_view now correctly applies the same permission gate.

Good consistency with json_view; unauthorized access now returns a proper 403 path.

django_loci/tests/base/test_admin.py (1)

64-81: Regression coverage is solid for both protected admin JSON endpoints.

These tests accurately exercise the unauthorized-user scenario and enforce the expected 403 behavior for both routes. Based on learnings: “Bug Fixes: A regression test must be included that reproduces the bug scenario” and “Bug Fixes: The regression test must fail if the patch is removed or commented out”.

📝 Walkthrough

Walkthrough

This change adds authorization enforcement to two admin JSON endpoints: json_view and floorplans_json_view. After fetching the target instance, each view now calls self.has_view_or_change_permission(request, obj=instance) and raises PermissionDenied when the caller lacks access. PermissionDenied was added to the Django exception imports. Tests were added to assert that unauthorized requests to these endpoints return HTTP 403.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~15 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description includes a summary, explanation of the security issue, detailed fix implementation, test changes, and notes. However, it does not include the required checklist items or link to an existing issue.	Add the complete PR checklist (with check marks for completed items) and include a 'Closes #' reference to link this PR to the corresponding GitHub issue.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly describes the main change—enforcing object-level permissions on admin JSON endpoints—and follows the required format with [fix] prefix and descriptive text.
Bug Fixes	✅ Passed	PR fixes security bug with explicit object-level permission checks in json_view and floorplans_json_view methods, including regression tests.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Copilot

Pull request overview

This PR hardens django_loci’s custom Django admin JSON endpoints for locations by enforcing object-level authorization checks before returning location/floorplan data.

Changes:

Add has_view_or_change_permission(request, obj=instance) checks to the location json_view and floorplans_json_view, raising PermissionDenied when unauthorized.
Add regression tests ensuring users without location permission receive 403 from both JSON endpoints.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
`django_loci/base/admin.py`	Enforces object-level permission checks on the two custom admin JSON endpoints.
`django_loci/tests/base/test_admin.py`	Adds regression tests for unauthorized access returning `403` for both endpoints.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-26T23:15:59Z

+from django.contrib.contenttypes.admin import GenericStackedInline
+from django.core.exceptions import PermissionDenied, ValidationError
+from django.http import JsonResponse
+from django.shortcuts import get_object_or_404
+from django.urls import path


This file now contains CRLF (carriage return) line endings (visible as \r in diffs/search results), which causes the PR to rewrite the whole file and can create noisy diffs/merge conflicts. Please convert the file back to LF-only line endings (e.g., dos2unix or configure core.autocrlf/editor settings) and keep the change limited to the permission check logic.

Copilot · 2026-03-26T23:15:59Z

+import json
+
+import responses
+from django.contrib.auth.models import Permission
+from django.contrib.humanize.templatetags.humanize import ordinal
+from django.urls import reverse


This file appears to have been converted to CRLF line endings (carriage returns), which rewrites the entire file in the diff and can create avoidable churn. Please normalize back to LF line endings and keep the diff focused on the two new regression tests.

Signed-off-by: Phu Si On <phusi319@users.noreply.github.com>

phusi319 · 2026-03-27T07:42:20Z

Addressed both review comments by normalizing reviewed files to LF-only line endings and keeping the diff focused on the permission checks/tests.

No logic changes beyond the intended security fix and regression tests.

phusi319 added 2 commits March 27, 2026 01:08

fix(security): enforce object-level perms on admin JSON endpoints

ca71c5c

Signed-off-by: Phu Si On <phusi319@users.noreply.github.com>

test: add unauthorized access coverage for location JSON views

281f613

Signed-off-by: Phu Si On <phusi319@users.noreply.github.com>

Copilot AI review requested due to automatic review settings March 26, 2026 23:08

Copilot started reviewing on behalf of phusi319 March 26, 2026 23:11 View session

coderabbitai Bot previously approved these changes Mar 26, 2026

View reviewed changes

Copilot AI reviewed Mar 26, 2026

View reviewed changes

chore: normalize reviewed files to LF line endings

cd53db0

Signed-off-by: Phu Si On <phusi319@users.noreply.github.com>

phusi319 dismissed coderabbitai[bot]’s stale review via cd53db0 March 27, 2026 07:41

coderabbitai Bot approved these changes Mar 27, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(security): enforce object-level permissions on admin JSON endpoints#210

fix(security): enforce object-level permissions on admin JSON endpoints#210
phusi319 wants to merge 3 commits into
openwisp:masterfrom
phusi319:fix/admin-json-object-permissions

phusi319 commented Mar 26, 2026

Uh oh!

coderabbitai Bot commented Mar 26, 2026 •

edited

Loading

Walkthrough

Estimated code review effort

❌ Failed checks (1 warning)

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

phusi319 commented Mar 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

phusi319 commented Mar 26, 2026

Summary

Security issue

Fix

Notes

Uh oh!

coderabbitai Bot commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Estimated code review effort

❌ Failed checks (1 warning)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

phusi319 commented Mar 27, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coderabbitai Bot commented Mar 26, 2026 •

edited

Loading