[RHCLOUD-27273] Enforce correct value/operation pairings

[mjholder]

BLOCKED BY: #1448

Link(s) to Jira

• https://issues.redhat.com/browse/RHCLOUD-27273

Description of Intent of Change(s)

The what, why and how.
In order to keep JSON parsing working we need to make sure the value types and operations provided by resourceDefinitions are compatible.
Specifically, if a String is provided the operation needs to be equals. Otherwise, if a List is provided, the operation needs to be in.
In order to achieve this I added some extra checks in the ResourceDefinition Serializer to make sure the pairings are correct.

Local Testing

How can the feature be exercised?
How can the bug be exploited and fix confirmed?
Is any special local setup required?

I added a couple tests to the unit test suite. If you want to test locally you can try to create a role with either of the following access definitions:

{
    "permission": "someApp:*:*",
    "resourceDefinitions": [
        {"attributeFilter": {"key": "keyA.id", "operation": "equals", "value": ["value1", "value2"]}}
    ],
}

OR

{
    "permission": "someApp:*:*",
    "resourceDefinitions": [
        {"attributeFilter": {"key": "keyA.id", "operation": "in", "value": "valueA"}}
    ],
}

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

[vbelchio]

@mjholder do we need to make changes in the OpenAPI specs?

[mjholder]

@mjholder do we need to make changes in the OpenAPI specs?

That's a good question. We do have the types that are acceptable listed but not the fact that a string expects equals and a list expects in.
I'm looking to see if there is a way to achieve this in OpenAPI 3

[mjholder]

@mjholder do we need to make changes in the OpenAPI specs?

That's a good question. We do have the types that are acceptable listed but not the fact that a string expects equals and a list expects in. I'm looking to see if there is a way to achieve this in OpenAPI 3

It looks like if we move to OpenAPI 3.1 we can use conditional blocks for schema validation but not in 3.0.0.
This if from the docs on what's added in 3.1: https://datatracker.ietf.org/doc/html/draft-bhutton-json-schema-00#page-42

[vkrizan]

These checks are insufficient, as these check only for string. If for example int was supplied, it would fall through. Why not check the expected type for each operator?

Be aware, that previously a documentation mentioned a string of comma separated values. Have you checked current state of the database? If users would have already stored badly formatted data, they could hit these errors.

I suggest to prefer isinstance over a strong type check. See Duck Typing.