Skip to content

Commit f674f41

Browse files
authored
AAP-50803 500 server error when creating a custom role missing the "View" permission (#822)
Fix AAP-50803: prevent 500 on custom roles missing “view” for remote models by adding verbose_name to StandinMeta - a safe fallback in validator; - include regression test. ### Steps to Test 1. Create a new role at [api/access/roles/create](http://localhost:44926/access/roles/create) ### Expected Results Error message: `Permissions for model inventory needs to include view, got ....` ### Root cause Remote stand‑in metadata did not define `verbose_name`, and the validator assumed it existed. ### Fix 1) Remote metadata enhancement - Add `verbose_name` to `StandinMeta` (derived from `model_name`, humanized/title‑cased). Optionally add `verbose_name_plural`. 2) Validator hardening - In `check_view_permission_criteria`, safely obtain a model label via: - `getattr(cls._meta, "verbose_name", getattr(cls._meta, "model_name", "model"))` - Use this label in the error to avoid `AttributeError`. ### Result - The API now returns HTTP 400 with a clear validation message indicating that the `view` permission is required for the target model, instead of a server error. ### Manual verification How to verify in the AAP UI 1. Sign in to the AAP web UI. 2. Navigate to Access Management → Roles. 3. Click “Create role”. 4. Enter values for Name, Display name, and Description. 5. Click “Add permission” but do not specify `view`. 6. Choose a remote content type (e.g., Service “AWX” → Resource “Inventory” or another remote model). 7. For Actions, select only “Change” (or any non‑View action) and do not add the corresponding “View” action for that same model. 8. Click “Save”. 9. Expected: The role is not created and an error is shown indicating the View permission is required for that model. (Optional: In browser DevTools → Network, the POST to /api/gateway/v1/role_definitions/ returns HTTP 400 with the same message.)
1 parent d60e4be commit f674f41

File tree

3 files changed

+63
-3
lines changed

3 files changed

+63
-3
lines changed

ansible_base/rbac/remote.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,12 @@ def __init__(self, ct: models.Model, abstract=False):
5252
self.app_label = ct.app_label
5353
self.abstract = abstract
5454

55+
# Provide Django-like labels for error messaging and display
56+
# e.g., "inventory" -> "inventory" (or title-cased if preferred)
57+
human_label = self.model_name.replace('_', ' ')
58+
self.verbose_name = human_label
59+
self.verbose_name_plural = f"{human_label}s"
60+
5561
self.pk = StandInPK(ct)
5662

5763

ansible_base/rbac/validators.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -132,9 +132,8 @@ def check_view_permission_criteria(codename_set: set[str], permissions_by_model:
132132
model_permissions = set(valid_model_permissions) & codename_set
133133
local_codenames = {codename for codename in model_permissions if not is_add_perm(codename)}
134134
if local_codenames and not any('view' in codename for codename in local_codenames):
135-
raise ValidationError(
136-
{'permissions': f'Permissions for model {cls._meta.verbose_name} needs to include view, got: {prnt_codenames(local_codenames)}'}
137-
)
135+
model_label = getattr(cls._meta, 'verbose_name', getattr(cls._meta, 'model_name', 'model'))
136+
raise ValidationError({'permissions': f'Permissions for model {model_label} needs to include view, got: {prnt_codenames(local_codenames)}'})
138137

139138

140139
def check_has_change_with_delete(codename_set: set[str], permissions_by_model: dict[Union[Type[Model], Type[RemoteObject]], list[str]]):
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
import pytest
2+
from django.test.utils import override_settings
3+
4+
from ansible_base.lib.utils.response import get_relative_url
5+
from ansible_base.rbac.models import DABContentType, DABPermission
6+
from ansible_base.rbac.remote import get_local_resource_prefix
7+
8+
9+
@pytest.mark.django_db
10+
@override_settings(ANSIBLE_BASE_ROLES_REQUIRE_VIEW=True)
11+
def test_create_remote_role_missing_view_returns_400_with_clear_message(admin_api_client):
12+
"""Reproduces AAP-50803: remote stand-in cls lacks verbose_name, causing 500.
13+
14+
Ensure creating a role for a remote content type with change-only permission
15+
returns HTTP 400 and a readable message that mentions view permission is required.
16+
"""
17+
# Ensure a remote content type exists (service not shared/local)
18+
local_service = get_local_resource_prefix()
19+
remote_ct = DABContentType.objects.exclude(service__in=("shared", local_service)).first()
20+
if remote_ct is None:
21+
# Create a representative remote content type (e.g., awx.inventory)
22+
remote_ct, _ = DABContentType.objects.get_or_create(
23+
service="awx",
24+
app_label="awx",
25+
model="inventory",
26+
defaults={"pk_field_type": "integer"},
27+
)
28+
29+
# Ensure both view and change permissions exist so the validator enforces the view requirement
30+
view_perm, _ = DABPermission.objects.get_or_create(
31+
content_type=remote_ct,
32+
codename="view_inventory",
33+
defaults={"name": "Can view inventory"},
34+
)
35+
change_perm, _ = DABPermission.objects.get_or_create(
36+
content_type=remote_ct,
37+
codename="change_inventory",
38+
defaults={"name": "Can change inventory"},
39+
)
40+
41+
url = get_relative_url('roledefinition-list')
42+
response = admin_api_client.post(
43+
url,
44+
data={
45+
'name': 'remote-change-no-view',
46+
'description': '',
47+
'content_type': remote_ct.api_slug,
48+
'permissions': [change_perm.api_slug],
49+
},
50+
)
51+
52+
# Expected: 400 with message indicating view is required
53+
assert response.status_code == 400, response.data
54+
msg = str(response.data).lower()
55+
assert 'view' in msg and ('needs to include view' in msg or 'required' in msg)

0 commit comments

Comments
 (0)