Skip to content

Commit a0121de

Browse files
MartinCastroAlvarezmartin-castro-laminr-aiclaude
authored
fix(api): apply allowed_permissions in change_actions fallback (#455) (#543)
The fallback path of permitted_action_names (used when a ModelAdmin declares `change_actions` without a `get_change_actions` hook) surfaced declared names as-is, with only the run view's has_change_permission gating the object. Apply each declared action's `allowed_permissions` against has_<perm>_permission(request), mirroring Django's own _filter_actions_by_permissions — defense in depth so an action the user can't run is never offered. Unknown perm / raising check denies. Low severity (the standard mixin provides get_change_actions, which filters internally — the fallback only triggers for a hand-rolled change_actions list), but closes the parity gap with the changelist bulk-action endpoint that already filters this way. Co-authored-by: Martin Castro Laminrs <mcastro@laminr.ai> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 925a966 commit a0121de

2 files changed

Lines changed: 64 additions & 1 deletion

File tree

django_admin_react/api/object_actions.py

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,11 +73,46 @@ def permitted_action_names(
7373

7474
declared = getattr(model_admin, "change_actions", None)
7575
if declared is not None:
76-
return [str(n) for n in (declared or ())]
76+
# Defense in depth (#455): in the fallback path (no
77+
# ``get_change_actions`` hook) filter each declared name by its
78+
# action callable's ``allowed_permissions`` against the admin's
79+
# ``has_<perm>_permission(request)``, mirroring Django's own
80+
# ``_filter_actions_by_permissions``. The run view still gates the
81+
# object via ``has_change_permission``; this just keeps an action
82+
# the user can't run from being surfaced.
83+
out: list[str] = []
84+
for raw in declared or ():
85+
name = str(raw)
86+
callable_obj = getattr(model_admin, name, None)
87+
perms = getattr(callable_obj, "allowed_permissions", None) or ()
88+
if _user_has_action_perms(model_admin, request, perms):
89+
out.append(name)
90+
return out
7791

7892
return None
7993

8094

95+
def _user_has_action_perms(
96+
model_admin: ModelAdmin, request: HttpRequest, perms: Any
97+
) -> bool:
98+
"""Apply each ``perm`` against ``has_<perm>_permission(request)``.
99+
100+
Mirrors Django's ``_filter_actions_by_permissions``. An unknown perm
101+
(no ``has_<perm>_permission`` method) or a raising check denies — we
102+
never grant an action whose permission we can't verify.
103+
"""
104+
for perm in perms:
105+
method = getattr(model_admin, f"has_{perm}_permission", None)
106+
if not callable(method):
107+
return False
108+
try:
109+
if not method(request):
110+
return False
111+
except Exception:
112+
return False
113+
return True
114+
115+
81116
def resolve_action_callable(model_admin: ModelAdmin, name: str) -> Any | None:
82117
"""Resolve one object-action name to its bound callable, or ``None``.
83118

tests/test_object_actions.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,34 @@ def test_detail_change_actions_attr_without_hook(superuser_client: Client) -> No
191191
assert {a["name"] for a in body["object_actions"]} == {"_deactivate"}
192192

193193

194+
@pytest.mark.django_db
195+
def test_detail_fallback_filters_by_allowed_permissions(superuser_client: Client) -> None:
196+
"""Defense in depth (#455): without ``get_change_actions``, the
197+
``change_actions`` fallback also filters each declared action by its
198+
callable's ``allowed_permissions`` against ``has_<perm>_permission``,
199+
mirroring Django's ``_filter_actions_by_permissions``. An action whose
200+
required perm fails is dropped from ``object_actions``."""
201+
u = _make_user()
202+
203+
def _delete_action(self, request, queryset): # noqa: ANN001, ANN202, ARG001
204+
queryset.delete()
205+
206+
_delete_action.allowed_permissions = ("delete",)
207+
208+
def _no_delete(self, request, obj=None): # noqa: ANN001, ANN202, ARG001
209+
return False
210+
211+
with admin_attr(
212+
User,
213+
change_actions=["_delete_action"],
214+
_delete_action=_delete_action,
215+
has_delete_permission=_no_delete,
216+
):
217+
body = superuser_client.get(DETAIL_URL.format(pk=u.pk)).json()
218+
# _delete_action requires `delete`; the admin denies → action dropped.
219+
assert body["object_actions"] == []
220+
221+
194222
@pytest.mark.django_db
195223
def test_detail_filters_unpermitted_actions(superuser_client: Client) -> None:
196224
"""An action the user is not permitted to run (per get_change_actions)

0 commit comments

Comments
 (0)