platform icon indicating copy to clipboard operation
platform copied to clipboard

MatchSubjectMappings should only key off of selector fields and ignore entity representation values of those fields

Open jakedoublev opened this issue 6 months ago • 0 comments

Background

An RPC was added within Policy MatchSubjectMappings when there were two possible logical conditions on the SubjectMappingOperatorEnum as IN and NOT_IN.

When those were the only logical operators, the intended use of the endpoint was to filter down SubjectConditionSets and their relevant SubjectMappings by an entity representations 'subject properties' (the selectable fields on an Entity Representation i.e. '.departments[]' and the entity's values) and only evaluate the entity against the relevant conditions found.

There was a concern about a performance footgun if an admin were to work around the intended use case of ABAC and drive granular access via a field common across every entity in the idP such as .id within every SubjectConditionSet, and that it would fetch every row on every request as a result.

As a result, many comments and logs were added as documentation of the known issue.

At this time, it is now known that more robust logical operators than IN/NOT_IN must be supported. It is known bad practice to embed business logic (which in this case is condition evaluation of expressions in ConditionSets (like CONTAINS, STARTS_WITH, BEFORE, ENDS_WITH, AFTER) within SQL, so it is untenable to try to prevent this footgun.

It is known that MatchSubjectMappings with an admin utilizing a single field value in every single SubjectConditionSet is no worse behavior than just calling ListSubjectMappings within every resolution (such as GetEntitlements).

Acceptance Criteria

The MatchSubjectMappings RPC is not yet utilized within the platform, so we should introduce the breaking changes before that occurs.

  1. update SubjectProperties proto so that entity representation values are not provided (only selectable values found via flattening lib)
  2. update DB query to only query SCS's for rows where the field is found in a matched condition
  3. remove comments and log statements around value evaluation behavior within the SQL query itself
  4. update integration tests

jakedoublev avatar Aug 09 '24 16:08 jakedoublev