[Draft] PoC OIDC credential testing#16333
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| # TODO: Make sure requesting user has access to this Job Template | ||
| jt = models.JobTemplate.objects.get(id=int(backend_kwargs['job_template_id'])) |
There was a problem hiding this comment.
See the TODO. I'm not explicitly checking any access here. Outside of this PoC, a request must fail if user does not have access to the JobTemplate
| obj.credential_type.plugin.backend(**backend_kwargs) | ||
| return Response({}, status=status.HTTP_202_ACCEPTED) | ||
| except requests.exceptions.HTTPError: | ||
| return Response(response_dict, status=status.HTTP_202_ACCEPTED) |
There was a problem hiding this comment.
For OIDC-enabled credentials, we return a JSON object containing sent_jwt_payload instead of an empty object:
{
"sent_jwt_payload": {
"aap_controller_organization_name": "Default",
"aap_controller_job_template_name": "Demo Job Template",
"jti": "7180bbbe-343b-4d76-b4b9-8734c23bd461",
"iss": "http://localhost:44927/o/",
"sub": "job::organization:Default:project::job_template:Demo Job Template",
"aud": "aud",
"exp": 1772833430.98862,
"iat": 1772833070.98862
}
}In the PoC this is only returned in the success case, but it's more valuable in the failure case. The intent is to show the user configuring the integration a practical example of the claims we're sending to Vault so they can write secure policies/bindings and have some visibility to debug when things aren't working.
Note that this payload critically must NOT include the signature. The signature would allow the viewer to access Vault. The payload here shows information to which they already have access or generic values.
| found_wit_field = 'workload_identity_token' in [f.get('id') for f in fields] | ||
|
|
||
| # Filter out internal fields from the API response | ||
| value['inputs']['fields'] = [f for f in fields if not f.get('internal')] |
There was a problem hiding this comment.
This change to the serializer ensures that internal fields do not appear in the API response so the user is not prompted to enter them on the form.
| # If workload identity token field exists, add job_template to metadata | ||
| if found_wit_field: | ||
| metadata = value['inputs'].get('metadata', []) | ||
| metadata.append({ | ||
| "id": "job_template_id", | ||
| "label": "ID of a Job Template", | ||
| "type": "string", | ||
| "help_text": "Job template ID to use when generating a token." | ||
| }) | ||
| value['inputs']['metadata'] = metadata |
There was a problem hiding this comment.
This change adds job_template_id to the metadata - the fields a user must provide to complete a test operation (or to configure the credential for lookup). But we only want it in the test operation, so I don't know if we should bother with this here.
There was a problem hiding this comment.
I think the changes in this file are very hacky. I think we should include job_template_id under metadata in awx-plugins, instead of including it at runtime here.
I think the proper abstraction would be to come up with a new field type, such as test_only, so we filter out job_template_id from other views affected by the stuff under metadata.
This way we would keep this credential specific logic away from this very generic serializer.
I am also aware that abstracting a single use case is a waste of time, so I would not fight back if we go with this approach as well.
There was a problem hiding this comment.
Discussed privately. Let's go ahead with this direction 👍
There was a problem hiding this comment.
Honestly I don't think we should hack the serializer either. I think at this point it makes sense for the UI to provide it only when running test operations.
|
|
||
| # Deep copy inputs to avoid modifying the original model data | ||
| if value.get('inputs'): | ||
| value['inputs'] = copy.deepcopy(value['inputs']) |
There was a problem hiding this comment.
This deepcopy here is really important. Without it, workload_identity_token disappears from the persisted model data and job_template_id gets added every time you call the API 😂
| def fetch_workload_identity_token(cred: models.Credential, job_template_name: str): | ||
| from ansible_base.lib.workload_identity.controller import AutomationControllerJobScope | ||
| from ansible_base.resource_registry.workload_identity_client import get_workload_identity_client | ||
| client = get_workload_identity_client() | ||
| claims = { | ||
| AutomationControllerJobScope.CLAIM_ORGANIZATION_NAME: cred.organization.name, | ||
| AutomationControllerJobScope.CLAIM_JOB_TEMPLATE_NAME: job_template_name | ||
| } | ||
| scope = AutomationControllerJobScope.name | ||
| audience = cred.get_input('jwt_aud') | ||
| return client.request_workload_jwt(claims=claims, scope=scope, audience=audience).jwt | ||
|
|
There was a problem hiding this comment.
This would all need to be feature-flagged, plus it duplicates some logic in main/tasks/jobs.py
Draft / PoC of three enhancements for OIDC-enabled credentials that need
workload_identity_tokento authenticate:POST /api/controller/v2/credentials/:id/test/(credential model object exists), view code obtains aworkload_identity_tokenand provides it to the plugin backend callable, allowing auth to succeed.POST /api/controller/v2/credentials/:id/test/, on successful auth, view code returns a JSON response containing the payload of the JWT (no signature, just the claims).CredentialTypeSerializerto suppressinternalfields from rendering (we don't want to prompt user for them) and dynamically addsjob_template_id. More thoughts on this belowThere's a second
testAPI that is used before credentials are saved, but the changes would be similar.