Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix permissions that come from an external auditor role #15291

Merged
merged 7 commits into from
Jun 27, 2024

Conversation

AlanCoding
Copy link
Member

@AlanCoding AlanCoding commented Jun 20, 2024

SUMMARY

RECAP:

  • I tested with a combined development environment with an external resource provider
  • I found that the TEMPLATES tab wasn't showing any entries for a user with an auditor permission provided from that
  • I went to write tests, and at first, found /api/v2/job_templates/ was working fine for this persona
  • Re-tested, and found the problem was /api/v2/unified_job_templates/
  • That led to the obvious problem, which is incompatibility of system roles and the AWX custom implementation of the accessible ids queryset, specifically for polymorphic models which nothing else uses. Considering when this was implemented and when the system roles were implemented this makes complete sense.
  • That established, it was just connecting the dots.

Separately, I was worried about references to is_system_auditor in the access logic, which wouldn't hit for this case. I believe those are effectively defunct. Sometime I want to come back and prune them, along with the rest of our dead code.

AAP-25457

ISSUE TYPE
  • Bug, Docs Fix or other nominal change
COMPONENT NAME
  • API

@AlanCoding
Copy link
Member Author

Needs instance_group fixture from #15284

@AlanCoding
Copy link
Member Author

Got the expected failures:

________ TestExternalAuditorRole.test_global_list[WorkflowJobTemplate] _________
[gw0] linux -- Python 3.11.9 /var/lib/awx/venv/awx/bin/python3.11
Traceback (most recent call last):
  File "/var/lib/awx/venv/awx/lib64/python3.11/site-packages/_pytest/runner.py", line 341, in from_call
    result: Optional[TResult] = func()
                                ^^^^^^
  File "/var/lib/awx/venv/awx/lib64/python3.11/site-packages/_pytest/runner.py", line 241, in <lambda>
    lambda: runtest_hook(item=item, **kwds), when=when, reraise=reraise
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/var/lib/awx/venv/awx/lib64/python3.11/site-packages/pluggy/_hooks.py", line 513, in __call__
    return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/var/lib/awx/venv/awx/lib64/python3.11/site-packages/pluggy/_manager.py", line 120, in _hookexec
    return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/var/lib/awx/venv/awx/lib64/python3.11/site-packages/pluggy/_callers.py", line 182, in _multicall
    return outcome.get_result()
           ^^^^^^^^^^^^^^^^^^^^
  File "/var/lib/awx/venv/awx/lib64/python3.11/site-packages/pluggy/_result.py", line 100, in get_result
    raise exc.with_traceback(exc.__traceback__)
  File "/var/lib/awx/venv/awx/lib64/python3.11/site-packages/pluggy/_callers.py", line 167, in _multicall
    teardown.throw(outcome._exception)
  File "/var/lib/awx/venv/awx/lib64/python3.11/site-packages/_pytest/threadexception.py", line 87, in pytest_runtest_call
    yield from thread_exception_runtest_hook()
  File "/var/lib/awx/venv/awx/lib64/python3.11/site-packages/_pytest/threadexception.py", line 63, in thread_exception_runtest_hook
    yield
  File "/var/lib/awx/venv/awx/lib64/python3.11/site-packages/pluggy/_callers.py", line 167, in _multicall
    teardown.throw(outcome._exception)
  File "/var/lib/awx/venv/awx/lib64/python3.11/site-packages/_pytest/unraisableexception.py", line 90, in pytest_runtest_call
    yield from unraisable_exception_runtest_hook()
  File "/var/lib/awx/venv/awx/lib64/python3.11/site-packages/_pytest/unraisableexception.py", line 65, in unraisable_exception_runtest_hook
    yield
  File "/var/lib/awx/venv/awx/lib64/python3.11/site-packages/pluggy/_callers.py", line 167, in _multicall
    teardown.throw(outcome._exception)
  File "/var/lib/awx/venv/awx/lib64/python3.11/site-packages/_pytest/logging.py", line 850, in pytest_runtest_call
    yield from self._runtest_for(item, "call")
  File "/var/lib/awx/venv/awx/lib64/python3.11/site-packages/_pytest/logging.py", line 833, in _runtest_for
    yield
  File "/var/lib/awx/venv/awx/lib64/python3.11/site-packages/pluggy/_callers.py", line 167, in _multicall
    teardown.throw(outcome._exception)
  File "/var/lib/awx/venv/awx/lib64/python3.11/site-packages/_pytest/capture.py", line 878, in pytest_runtest_call
    return (yield)
            ^^^^^
  File "/var/lib/awx/venv/awx/lib64/python3.11/site-packages/pluggy/_callers.py", line 167, in _multicall
    teardown.throw(outcome._exception)
  File "/var/lib/awx/venv/awx/lib64/python3.11/site-packages/_pytest/skipping.py", line 257, in pytest_runtest_call
    return (yield)
            ^^^^^
  File "/var/lib/awx/venv/awx/lib64/python3.11/site-packages/pluggy/_callers.py", line 103, in _multicall
    res = hook_impl.function(*args)
          ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/var/lib/awx/venv/awx/lib64/python3.11/site-packages/_pytest/runner.py", line 183, in pytest_runtest_call
    raise e
  File "/var/lib/awx/venv/awx/lib64/python3.11/site-packages/_pytest/runner.py", line 173, in pytest_runtest_call
    item.runtest()
  File "/var/lib/awx/venv/awx/lib64/python3.11/site-packages/_pytest/python.py", line 1632, in runtest
    self.ihook.pytest_pyfunc_call(pyfuncitem=self)
  File "/var/lib/awx/venv/awx/lib64/python3.11/site-packages/pluggy/_hooks.py", line 513, in __call__
    return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/var/lib/awx/venv/awx/lib64/python3.11/site-packages/pluggy/_manager.py", line 120, in _hookexec
    return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/var/lib/awx/venv/awx/lib64/python3.11/site-packages/pluggy/_callers.py", line 182, in _multicall
    return outcome.get_result()
           ^^^^^^^^^^^^^^^^^^^^
  File "/var/lib/awx/venv/awx/lib64/python3.11/site-packages/pluggy/_result.py", line 100, in get_result
    raise exc.with_traceback(exc.__traceback__)
  File "/var/lib/awx/venv/awx/lib64/python3.11/site-packages/pluggy/_callers.py", line 103, in _multicall
    res = hook_impl.function(*args)
          ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/var/lib/awx/venv/awx/lib64/python3.11/site-packages/_pytest/python.py", line 162, in pytest_pyfunc_call
    result = testfunction(**testargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/awx_devel/awx/main/tests/functional/dab_rbac/test_external_auditor.py", line 68, in test_global_list
    assert r.data['count'] == initial_ct + 1
AssertionError: assert 0 == (0 + 1)

@AlanCoding AlanCoding changed the title Add tests for an external auditor role Fix permissions that come from an external auditor role Jun 25, 2024
@AlanCoding AlanCoding marked this pull request as ready for review June 25, 2024 13:45
@AlanCoding
Copy link
Member Author

Mentioned failure is addressed, remaining failures are general to all PRs.

@AlanCoding AlanCoding merged commit db72c9d into ansible:devel Jun 27, 2024
21 checks passed
djyasin pushed a commit to djyasin/awx that referenced this pull request Sep 16, 2024
* Add tests for external auditor

* Add assertion for unified JTs which fails

* Fix UJT listing bug

* Add test for ad hoc commands just to be sure
djyasin pushed a commit to djyasin/awx that referenced this pull request Nov 11, 2024
* Add tests for external auditor

* Add assertion for unified JTs which fails

* Fix UJT listing bug

* Add test for ad hoc commands just to be sure
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants