[RBAC] Proposal: Remove HTTP 404 handling from permission class

[AlanCoding]

After looking at some related-ish issues with @Dostonbek1 I was curious so I looked at our SonarCloud coverage:

https://sonarcloud.io/component_measures?id=ansible_django-ansible-base&metric=new_coverage&selected=ansible_django-ansible-base%3Aansible_base%2Frbac%2Fapi%2Fpermissions.py

I have another issue #221 requesting some changes around this code.

This method structure is ultimately taken from DRF.

https://github.com/encode/django-rest-framework/blob/4f10c4e43ee57f4a2e387e0c8d44d28d21a3621c/rest_framework/permissions.py#L286-L311

Those 2 cases of Http404 are not only not covered, but they are unreachable if you are doing anything sane. I'll mention again here that the only 2 cases using this permission class are eda-server and test_app. There is no practical case in either of those where you could hit this. To it it, you would have to:

• define a viewset that used model.objects.all() (incorrect)
• do a GET to the detail endpoint

Even if this incorrect scenario happened, giving a 404 on the detail endpoint, but still listing the object in the list view is not defensible. DAB RBAC centers around querysets, and individual evaluations are an afterthought. Not the other way around.

Clinging to standards is the reason for writing it in this way to start with... but we have a very complete and coherent way of doing things, and this doesn't fit.

For more academic reading, see encode/django-rest-framework#8009, and you can see that checking view permission for an object isn't really a thing, and that ultimately got reverted. If DRF thought checking object view permission should be done the heck, that still wouldn't even affect this proposal! Go check it.

I think the deep-seated concern is a PATCH to the detail endpoint where user has "change" permission but not "view" permission, and that we might accidentally allow this action. But...

1. No, this won't ever happen because get_object() uses the view querset, and lacking "view" permission removes the object from that --> 404, as expected
2. RoleDefinition validation is already on this problem, enforcing "view" permission for any model the role gives "change" permission for. Some apps might not use this validation, but if not...
3. It's not clear if this expectation is reasonable in the first place. Perhaps you could argue "change" permission is a backdoor to viewing.. but no. I will be pigheaded about "change" and "view" being directionally inseparable. We have enforcement with both (1) and (2) here and if you go so far as to fight both of those, the app should relent and give you what you asked for.