Deduplicate access permissions list returned to clients

[PaulWay]

It is useful for clients to handle the simplest list of permissions objects, for performance reasons. It reduces the data transferred to the client application, and it reduces the possibility that a client will misinterpret some part of the data. It also reduces the amount of data RBAC stores in its cache, thus saving space and time.

This implements a basic process to deduplicate the list of permissions objects returned to the client. It does two basic optimisations:

1. Two exactly matching permissions - e.g. 'app:*:read' and 'app:*:read'
   • are combined. Their resourceDefinitions are appended without attempting to interpret them.
2. The '*' verb implies all other verbs. Therefore 'app:*:read' will be ignored in favour of 'app:*:*'. Ignored permissions have their resourceDefinitions thrown away because they are superseded.

Further optimisations may be possible but may be app-dependent and we should tread with caution here.

A basic test is included, but probably not yet working.

Checklist

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

Secure Coding Practices Checklist Link

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

Secure Coding Practices Checklist

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

[lpichler]

Thanks @PaulWay for doing it!

it looks that this covers this ticket https://issues.redhat.com/browse/RHCLOUD-27387

Note: It looks that some * in description are hiding during formatting :)

[petrblaho]

@PaulWay I don't think that the idea of throwing resourceDefinitions in the second case is correct.

It is probably not real edge case but I can imagine having app:*:read on one set of resources (identified by resourceDefinitions) and also having app:*:* on different set of resources. So we can't throw away resourceDefinitions in this case.

[skateman]

IMO from our context all these should be merged and no precedence should be applied.

[lpichler]

@PaulWay I believe that we should do only first thing point 1 now and than we need to elaborate and discuss more option 2 about its impact.

Do you have time to separate it or should we take it over ?

[PaulWay]

@lpichler - well spotted, formatting corrected.
@petrblaho - This is the behaviour that @kruai and Rodrigo Antunes described in RHCLOUD-27387 AFAICS. But...
@lpichler I'm pretty busy at the moment, so I'm happy for you to take this up. And I'm happy if we just start with the first deduplication (merging records that are the same) and discuss what other deduplications and merging we can do in the future.

The fundamental point is that we have a large number of apps reading this data. If each of those apps implements different methods of working out which host groups this user is allowed to see, then we'll have inconsistent behaviour and our customers will get confused. We need to provide one simple place that makes sure that these records are consistent, and RBAC is the best place to do it AFAICS.

[fpjrh]

@syncrou and @gmcculloug this has been sitting idle for a while. What steps do we need to take to finish the review?

[syncrou]

@fpjrh We are discussing the solution @lpichler discussed here: #893 (comment)

As far as I know we need to get this reviewed and tested. The tough part has been that Libor and Ashley who have the deepest background on RBAC have another high priority RBAC task they are currently working through ( for the past month. )

I am working at finding resources to get this taken care of, and I'll try to get this escalated.

[syncrou]

@fpjrh We have assigned Doug Donahue to get this moving forward. He is familiar with RBAC and will allow us to apply fresh eyes on this.

[ddonahue007]

I have updated the Jira task RHCLOUD-27387

[PaulWay]

@ddonahue007 @skateman @syncrou just wondering where we're up to with this?

I'm trying to get collaboration on this work so I think it'd be good if we can push to this branch and update the MR as we decide what course to take. E.g. if you want to do deduplication after cache fetch but not before cache push, go right ahead - let's collaborate here.