Fix organization not showing all galaxy credentials for org admin

[gamuniz]

SUMMARY

Organizations were not properly showing all the associated galaxy credentials for Organization admins. They should be visible even if the credential cannot be accessed by an Org admin.

ISSUE TYPE

• Bug, Docs Fix or other nominal change

COMPONENT NAME

• API

AWX VERSION

make VERSION
awx: 21.12.1.dev95+g122b9e23be.d20230310

ADDITIONAL INFORMATION

Steps to reproduce:

1. create a galaxy credential(in a separate organization as the org admin user)
2. create an org admin
3. associate the galaxy credential with that to which the newly created user is an admin
4. Navigate to the Organization details page

Admin user:

Org admin:

Org Admin with fix:

[AlanCoding]

There's an alternative method of doing this above in this file, which can avoid the contract change of adding the prefetched_items property to the view

awx/awx/api/generics.py

Lines 367 to 372 in 4f6c853

	if self.model in access_registry:
	access_class = access_registry[self.model]
	if access_class.select_related:
	qs = qs.select_related(*access_class.select_related)
	if access_class.prefetch_related:
	qs = qs.prefetch_related(*access_class.prefetch_related)

It would also be good to move the qs definition to after this return because in this code path it's never used.

[gamuniz]

I mostly understood your comments and am going to try something like this:

if not self.filter_read_permission:
        access_class = access_registry[self.model]
        if access_class.prefetch_related:
            return sublist_qs.prefetch_related(*access_class.prefetch_related)
        if access_class.select_related:
            return sublist_qs.select_related(*access_class.select_related)

       I reordered the returns as it seems that prefetch_related if available returns with less DB queries even if it is a little more processing on the api

[AlanCoding]

Many other view should be able to get rid of the get_queryset override by using filter_read_permission = False, which should result in a glorious amount of code deletions.

If you want to formalize that followup, go ahead. If you don't, I never intend to leave a good chunk of unnecessary code un-deleted.

[obaranov]

What is the final behavior? Admin org user should see all the credentials and instance groups? Can the org admin user delete the instance group to which they don't have access from the organization?

[gamuniz]

What is the final behavior? Admin org user should see all the credentials and instance groups? Can the org admin user delete the instance group to which they don't have access from the organization?

@obaranov They should be able to see any credential/instance group associated to the org, but they should not be able to remove without the correct rbac permissions

[obaranov]

@gamuniz I checked, and api works fine. But with UI I have one small concern. User cannot edit org instance groups if at least one in the list has no user role.

The current user does not have access to the ig_restricted group, but it does have an admin role for ig_admin. And when I try to remove the ig_admin group, I'm still getting the forbidden error on save:

The same happens when I try to add a new instance group to the list.

[john-westcott-iv]

Are there any other items we would want to apply this to? i.e. Notifications, whatever else?

[gamuniz]

I would like to add it liberally to remove a lot of the queries that do identical lookups. That was part of the discussion we had last week and came to no conclusion.

[AlanCoding]

Yes, there's a performance argument for using this, if possible. However, we do have a lot of views that are reverse relationships. Even if a JT should list all of its credentials, that doesn't mean a credential should list all the JTs it is used in.

[gamuniz]

@john-westcott-iv @AlanCoding if you would like me to add this to other places in the api just call it out, otherwise I'm okay with just the org endpoints and the job template credentials endpoint for now

[john-westcott-iv]

I'm good with this as a start. We can add other pieces as needed.

[gamuniz]

@obaranov can we merge this guy?