Fix export with system auditor user

[tanganellilore]

SUMMARY

If we use a system auditor user to export elements like schedules or worklows (that have schedule) we receive a general error from awx.awx.module (because is wrapped and is not clear what is behind).

Going down in the error itself I notice that issue was the Insufficient privileges on %s, inferring POST fields from description. and in the parse_description function that is called whtn POST action is not visible to "unpriviledge" user.

Inside the parse_description i notice that argument passed is referring to desc[desc.index('POST') :] but if the description of API not have any POST section (like schedule) extraction go in error.

I think that we can check if POST is present in the description and if not use the description itself to extract required data.

Otherwhise is better to return an error to end user (or add the POST descriptio where is missing)

Let me know if i need to modify something
This will fix #14635

ISSUE TYPE

• Bug, Docs Fix or other nominal change

COMPONENT NAME

• API
• Collection
• CLI

AWX VERSION

23.3.1

ADDITIONAL INFORMATION

[chrismeyersfsu]

Hey @tanganellilore thanks for the bug issues and the follow up PR, it helps me get up to speed on the issue.

I think what you have here is better than before so I'm good with approving and merging it. I'll note that it does not fix the underlying issue, but that issue is harder to fix.

[chrismeyersfsu]

Digging into this further. This PR avoids the error in question but the fields are not discovered correctly from the description text blob. Below is what an successful export of the schedule roughly looks like with an admin user vs system auditor.

Maybe we should NOT merge this PR. I'm thinking maybe we change it to fail but more gracefully. We don't want users thinking their schedules backed up without actually backing them up.

Admin user

{
  "rrule": "DTSTART:20151117T050000Z RRULE:FREQ=DAILY;INTERVAL=1;COUNT=1",
  "name": "test-sched",
  "description": "",
  "extra_data": {},
  "scm_branch": null,
  "job_type": null,
  "job_tags": null,
  "skip_tags": null,
  "limit": null,
  "diff_mode": null,
  "verbosity": null,
  "execution_environment": null,
  "forks": null,
  "job_slice_count": null,
  "timeout": null,
  "enabled": true,
  "inventory": {
    "organization": {
      "name": "Default",
      "type": "organization"
    },
    "name": "test-inv",
    "type": "inventory"
  },
  "unified_job_template": {
    "organization": {
      "name": "Default",
      "type": "organization"
    },
    "name": "test-jt",
    "type": "job_template"
  },
  "related": {
    "credentials": []
  },
  "natural_key": {
    "unified_job_template": {
      "organization": {
        "name": "Default",
        "type": "organization"
      },
      "name": "test-jt",
      "type": "job_template"
    },
    "name": "test-sched",
    "type": "schedule"
  }
}

System Auditor

{
  "assets": {
    "schedules": [
      {
        "related": {
          "credentials": []
        },
        "natural_key": {
          "unified_job_template": {
            "organization": {
              "name": "Default",
              "type": "organization"
            },
            "name": "test-jt",
            "type": "job_template"
          },
          "name": "test-sched",
          "type": "schedule"
        }
      }
    ]
  },

Dug deeper. We need to decide how we want to handle this case. I say fail, but more gracefully.

[tanganellilore]

Nice highligtht.
My opinion is to have an export fully compatibile with system Auditore role, not only partial.

Probably is required some more changes on api level, but better approach.

Fields are missing because are not present in the description and system Auditore user is not able to see post method.

This happen on all api that require extract io of post method

[chrismeyersfsu]

I share your goal. I wonder why we require POST data and fields when doing an export. I would have thought we should rely on the GET data and fields.

[tanganellilore]

Yes, this nlis not clear to me.... I checked the conde and probably the reason is related to check the permission and/or role and /or reuse some function.

Tomorrow if i have time I will check if i can change Thi beaviour

[jbradberry]

@chrismeyersfsu @tanganellilore the POST fields were needed because we provide a number of read-only fields, and my goal when writing this functionality was to avoid loading up the output with extraneous data. Maybe it's ok to fall back to just dumping everything (I don't see much reason to hit OPTIONS if POST isn't going to do the job)...? But how then are you going to distinguish the case when the exporting user legit doesn't have access to the resources?

[jbradberry]

Rummaging through the code now, I'll document the sharp edges I see here:

• for-real insufficient privileges
• deprecated endpoints
• some foreign keys are required on import, and that info is in the POST fields (awxkit/api/pages/api.py, L114-5)
• POST fields are used to distinguish between create-only and attach api relational endpoints
• WorkflowApprovalTemplates are very special-cased; it'll take some effort to remember what all is going on with those and how many of their uses of POST fields can be thrown away

[tanganellilore]

@jbradberry you are correct, your post remeber me when i digging into the code ans see why it's used post instead get .

The question now could be... Why we need to "hide" the post key in options to user that is read only like system auditor?

[jbradberry]

@jbradberry you are correct, your post remeber me when i digging into the code ans see why it's used post instead get .

The question now could be... Why we need to "hide" the post key in options to user that is read only like system auditor?

It's a good point, I was thinking myself that we're conflating two things: metadata about the fields and permissions for the current user. It'd be nice to decouple them, but I'll point out that any changes we make to the API won't entirely remove the headaches for us since there is an expectation that different versions of the CLI can work with different versions of the API. The same holds (but worse, since devs frequently forget that we need to keep awxkit in sync with changes in the API) if we solve this problem by hard-coding information in awxkit about what fields are important.

[chrismeyersfsu]

It's a good point, I was thinking myself that we're conflating two things: metadata about the fields and permissions for the current user. It'd be nice to decouple them

👍 Philosophically I share this view and direction.

• some foreign keys are required on import, and that info is in the POST fields (awxkit/api/pages/api.py, L114-5)
• POST fields are used to distinguish between create-only and attach api relational endpoints

Hmm, this changes my mental model. I was considering export in isolation, but it seems that when I think about export I need to think about import at the same time.

Am I correct that the two reasons I quoted from you above is why we can't simply fall back to getting the fields from GET after we try POST ?

[jbradberry]

@chrismeyersfsu yeah, I think those are the critical reasons.

[jbradberry]

I think this is fine.

[jbradberry]

@tanganellilore Can you please update awx_collection/test/awx/test_export.py::test_export_system_auditor? Your changes are making the test fail, since it presumes that exporting as a system auditor will fail. We're also getting a linting failure.

[jbradberry]

I went ahead and pushed some quick fix commits that I think will pass CI. Will merge if so.