Ensure we allow application/problem+json as a valid Accept header value in v2

[coderbydesign]

Link(s) to Jira

• RHCLOUD-35321

Description of Intent of Change(s)

Since we are starting to use application/problem+json as a response content type in v2 APIs [1], we need RBAC/Django to accept this as a valid Accept header value, given clients built off the API spec will be sending this.

Currently, if you send a request to /api/rbac/v2/workspaces/<uuid>/ -XDELETE -H 'Accept: application/problem+json' which is what a client built off the spec will do (with the Accept header), you'll get an error from RBAC: "Not acceptable 406".

We do set the correct content-type to align with the spec [2], however Django complains becasue it can't find the application/problem+json renderer.

This implements a custom renderer in Django Rest Framework [3] on the v2 views, specifically workspaces now, specifying it in the renderer_classes of the view, and setting the media_type (used to match the Accept value(s) to a renderer) and format (response) properties accordingly.

The DELETE test was modified prior to this change to add Accept: application/problem+json to confirm it failed (it did), and now succeeds with the changes in place.

[1] [https://github.com/RedHatInsights/insights-rbac/blob/master/docs/source/specs/v2/openapi.v2.yaml]
[2] https://github.com/RedHatInsights/insights-rbac/blob/master/rbac/api/common/exception_handler.py#L93
[3] [https://www.django-rest-framework.org/api-guide/renderers/#custom-renderers]

Local Testing

To exploit the bug on main, which also presents through a generated API client, send the following request:

curl http://localhost:8000/api/rbac/v2/workspaces/<uuid>/ -XDELETE -H 'Accept: application/problem+json'

You'll see a 406 response. Now check this branch out and run the same curl. You should also see application/json, application/problem+json accordingly, depending on success/failure of the requests.

Checklist

• if API spec changes are required, is the spec updated?
• are there any pre/post merge actions required? if so, document here.
• are theses changes covered by unit tests?
• if warranted, are documentation changes accounted for?
• does this require migration changes?
  • if yes, are they backwards compatible?
• is there known, direct impact to dependent teams/components?
  • if yes, how will this be handled?

Secure Coding Practices Checklist Link

• https://github.com/RedHatInsights/secure-coding-checklist

Secure Coding Practices Checklist

• Input Validation
• Output Encoding
• Authentication and Password Management
• Session Management
• Access Control
• Cryptographic Practices
• Error Handling and Logging
• Data Protection
• Communication Security
• System Configuration
• Database Security
• File Management
• Memory Management
• General Coding Practices

[coderbydesign]

This ensures we pull in the default JSON renderer and just add the custom problem renderer. We'll need to do this for each v2 view, or design a better way to inherit from a v2 viewset, have middleware plug this in based on path, etc. I figured this was probably the easiest for now, and we can revisit if folks think we should refactor when we introduce more views.

[coderbydesign]

Look to see if we can have a v2 viewset to inherit from.

[coderbydesign]

Look at adding a v2 identity request which inherits from v1 to add headers.

[alechenninger]

One interesting thing about this I guess is that the headers are dependent on the request. So like if we added both accept media types unconditionally, I guess we wouldn't have reproduced the bug.

I wonder if we should try and use the generated client in the tests somehow? (If so this is very likely a future reliability topic I guess)

[coderbydesign]

@alechenninger yeah, good point.

that's actually something I've brought up in the past for PR check/IQE tests, around potentially building clients on PR changes and having a small suite that would test client changes based on spec changes and run through some tests.

[vbelchio]

LGTM
QE: Passed