Add callback to validate role assignment

[fosterseth]

Some dab-based apps like AWX may wish to add exceptions to the user and team role assignments.

This PR adds logic to the serializer .create method to execute a callback method that is optionally defined on the model.

This callback signature looks like:

validate_role_assignment(self, actor, role_definition)

and should return either

• None
• a string describing the validation error

If a string is returned, the string will be wrapped up in a PermissionDenied error (HTTP 403)

Open questions:

• [x] should validate_role_assignment signature take any other params? maybe **kwargs?
• [x] should validate_role_assignment return a string? We could have it raise a django ValidationError, but then the create serializer method would need to try/catch the exception and raise a 403. Simplest design is to just return a string in my opinion

[AlanCoding]

This should formally solve https://github.com/ansible/django-ansible-base/issues/445. The method signatures might be non-optimal for that particular use case, which is "don't allow _system user to get any permissions". What you have works best for "don't allow _system user to get inventory permissions". If we were to solve the former problem in AWX, then I believe the method would get attached to BaseModel... which again, still solves the problem.

I'm also interested in (later, if not now) UI involvement in this. By having this take arguments, we are not able to surface it in the API in any format. The _system user case might be better as a property on the User model, so that the API could give a true/false reading on whether that particular user was disabled from receiving roles. Likewise the other way, a property on objects that told true/false permissions could be delegated.

When would an object be prohibited from having roles assigned? The AWX control plane EE.

Talking this out, cross-referencing against your AWX PR, I'm leaning in the direction of eventually having both solutions.

[AlanCoding]

should validate_role_assignment signature take any other params? maybe **kwargs?

No, there's no other kwargs to pass it? The caller here is DAB RBAC. It's giving all the parameters for the give_permission interface, which we are being almost religious about.

[AlanCoding]

The _system user case might be better as a property on the User model

I take it back. I've been looking further into the EE case and it's potentially more complicated with a lot of wrinkles We allow some edits for the control plane EE, but not everything. The default EE can be edited, but not deleted(?). That might be (and should be) handled in the validators, so maybe irrelevant. And org-less EEs are another special case.

So I'm leaning towards a boolean being an anti-feature. Even if we had different parameterizations of the callback, returning a message seems pretty critical in the end-game. These are exceptions, and they need to return a reason.

[AlanCoding]

I checked out your branch and used it in https://github.com/ansible/awx/pull/15289 and initially got this, what do you think?

Is AWX running some non-standard logic in its generics that is the problem?

___________________________________ test_org_member_required_for_assignment ___________________________________
Traceback (most recent call last):
  File "/home/alancoding/venvs/awx/lib64/python3.12/site-packages/_pytest/runner.py", line 341, in from_call
    result: Optional[TResult] = func()
                                ^^^^^^
  File "/home/alancoding/venvs/awx/lib64/python3.12/site-packages/_pytest/runner.py", line 241, in <lambda>
    lambda: runtest_hook(item=item, **kwds), when=when, reraise=reraise
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/alancoding/venvs/awx/lib64/python3.12/site-packages/pluggy/_hooks.py", line 513, in __call__
    return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/alancoding/venvs/awx/lib64/python3.12/site-packages/pluggy/_manager.py", line 120, in _hookexec
    return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/alancoding/venvs/awx/lib64/python3.12/site-packages/pluggy/_callers.py", line 182, in _multicall
    return outcome.get_result()
           ^^^^^^^^^^^^^^^^^^^^
  File "/home/alancoding/venvs/awx/lib64/python3.12/site-packages/pluggy/_result.py", line 100, in get_result
    raise exc.with_traceback(exc.__traceback__)
  File "/home/alancoding/venvs/awx/lib64/python3.12/site-packages/pluggy/_callers.py", line 167, in _multicall
    teardown.throw(outcome._exception)
  File "/home/alancoding/venvs/awx/lib64/python3.12/site-packages/_pytest/threadexception.py", line 87, in pytest_runtest_call
    yield from thread_exception_runtest_hook()
  File "/home/alancoding/venvs/awx/lib64/python3.12/site-packages/_pytest/threadexception.py", line 63, in thread_exception_runtest_hook
    yield
  File "/home/alancoding/venvs/awx/lib64/python3.12/site-packages/pluggy/_callers.py", line 167, in _multicall
    teardown.throw(outcome._exception)
  File "/home/alancoding/venvs/awx/lib64/python3.12/site-packages/_pytest/unraisableexception.py", line 90, in pytest_runtest_call
    yield from unraisable_exception_runtest_hook()
  File "/home/alancoding/venvs/awx/lib64/python3.12/site-packages/_pytest/unraisableexception.py", line 65, in unraisable_exception_runtest_hook
    yield
  File "/home/alancoding/venvs/awx/lib64/python3.12/site-packages/pluggy/_callers.py", line 167, in _multicall
    teardown.throw(outcome._exception)
  File "/home/alancoding/venvs/awx/lib64/python3.12/site-packages/_pytest/logging.py", line 850, in pytest_runtest_call
    yield from self._runtest_for(item, "call")
  File "/home/alancoding/venvs/awx/lib64/python3.12/site-packages/_pytest/logging.py", line 833, in _runtest_for
    yield
  File "/home/alancoding/venvs/awx/lib64/python3.12/site-packages/pluggy/_callers.py", line 167, in _multicall
    teardown.throw(outcome._exception)
  File "/home/alancoding/venvs/awx/lib64/python3.12/site-packages/_pytest/capture.py", line 878, in pytest_runtest_call
    return (yield)
            ^^^^^
  File "/home/alancoding/venvs/awx/lib64/python3.12/site-packages/pluggy/_callers.py", line 167, in _multicall
    teardown.throw(outcome._exception)
  File "/home/alancoding/venvs/awx/lib64/python3.12/site-packages/_pytest/skipping.py", line 257, in pytest_runtest_call
    return (yield)
            ^^^^^
  File "/home/alancoding/venvs/awx/lib64/python3.12/site-packages/pluggy/_callers.py", line 103, in _multicall
    res = hook_impl.function(*args)
          ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/alancoding/venvs/awx/lib64/python3.12/site-packages/_pytest/runner.py", line 183, in pytest_runtest_call
    raise e
  File "/home/alancoding/venvs/awx/lib64/python3.12/site-packages/_pytest/runner.py", line 173, in pytest_runtest_call
    item.runtest()
  File "/home/alancoding/venvs/awx/lib64/python3.12/site-packages/_pytest/python.py", line 1632, in runtest
    self.ihook.pytest_pyfunc_call(pyfuncitem=self)
  File "/home/alancoding/venvs/awx/lib64/python3.12/site-packages/pluggy/_hooks.py", line 513, in __call__
    return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/alancoding/venvs/awx/lib64/python3.12/site-packages/pluggy/_manager.py", line 120, in _hookexec
    return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/alancoding/venvs/awx/lib64/python3.12/site-packages/pluggy/_callers.py", line 182, in _multicall
    return outcome.get_result()
           ^^^^^^^^^^^^^^^^^^^^
  File "/home/alancoding/venvs/awx/lib64/python3.12/site-packages/pluggy/_result.py", line 100, in get_result
    raise exc.with_traceback(exc.__traceback__)
  File "/home/alancoding/venvs/awx/lib64/python3.12/site-packages/pluggy/_callers.py", line 103, in _multicall
    res = hook_impl.function(*args)
          ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/alancoding/venvs/awx/lib64/python3.12/site-packages/_pytest/python.py", line 162, in pytest_pyfunc_call
    result = testfunction(**testargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/alancoding/repos/awx/awx/main/tests/functional/test_rbac_execution_environment.py", line 58, in test_org_member_required_for_assignment
    r = post(url, {'role_definition': ee_rd.pk, 'user': rando.id, 'object_id': org_ee.pk}, user=admin_user, expect=400)
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/alancoding/repos/awx/awx/main/tests/functional/conftest.py", line 631, in rf
    response = view(request, *view_args, **view_kwargs)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/alancoding/venvs/awx/lib64/python3.12/site-packages/django/views/decorators/csrf.py", line 56, in wrapper_view
    return view_func(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/alancoding/venvs/awx/lib64/python3.12/site-packages/rest_framework/viewsets.py", line 124, in view
    return self.dispatch(request, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/alancoding/repos/awx/awx/api/generics.py", line 348, in dispatch
    return super(APIView, self).dispatch(request, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/alancoding/venvs/awx/lib64/python3.12/site-packages/rest_framework/views.py", line 511, in dispatch
    self.response = self.finalize_response(request, response, *args, **kwargs)
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/alancoding/repos/awx/awx/api/generics.py", line 230, in finalize_response
    msg_data['error'] = ", ".join(list(map(lambda x: x.get('error', response.status_text), response.data)))
                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/alancoding/repos/awx/awx/api/generics.py", line 230, in <lambda>
    msg_data['error'] = ", ".join(list(map(lambda x: x.get('error', response.status_text), response.data)))
                                                     ^^^^^
AttributeError: 'ErrorDetail' object has no attribute 'get'

[AlanCoding]

Resolved by adding a field key to the error:

diff --git a/awx/main/models/execution_environments.py b/awx/main/models/execution_environments.py
index 5d55b55caf..137533d0f4 100644
--- a/awx/main/models/execution_environments.py
+++ b/awx/main/models/execution_environments.py
@@ -60,13 +60,13 @@ class ExecutionEnvironment(CommonModel):

     def validate_role_assignment(self, actor, role_definition):
         if self.managed:
-            raise ValidationError(_('Can not assign object roles to managed Execution Environments'))
+            raise ValidationError({'object_id': _('Can not assign object roles to managed Execution Environments')})
         if self.organization_id is None:
-            raise ValidationError(_('Can not assign object roles to global Execution Environments'))
+            raise ValidationError({'object_id': _('Can not assign object roles to global Execution Environments')})

         if actor._meta.model_name == 'user' and (not actor.has_obj_perm(self.organization, 'view')):
-            raise ValidationError(_('User must have view permission to Execution Environment organization'))
+            raise ValidationError({'user': _('User must have view permission to Execution Environment organization')})
         if actor._meta.model_name == 'team':
             organization_cls = self._meta.get_field('organization').related_model
             if self.orgaanization not in organization_cls.access_qs(actor, 'view'):
-                raise ValidationError(_('Team must have view permission to Execution Environment organization'))
+                raise ValidationError({'team': _('Team must have view permission to Execution Environment organization')})

I'm okay with this, but it should be eventually documented. If not now, then please drop some quick notes in a DAB issue.

[sonarcloud[bot]]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud