feat(coverage) : improve accuracy for evaluating permission condition

[vijayraghav-io]

Fixes: #837

/claim #837

@tolgaOzen - Please find below the initial findings and approach, will update as i progress.

Action Items

[ ] - Review the current implementation of the 'Coverage' command - InProgress As per the current implementation, only the action names or permission names inside the entity are added to Coverage.Assertions. https://github.com/Permify/permify/blob/60c8fb3ec00f53fc9ebe53d7e056a36c45cf6e52/pkg/development/coverage/coverage.go#L241 This is compared with scenarios->checks->assertions in the input file. Whereas the conditions inside the action/permission are not accounted, hence the coverage for assertions is inaccurate.

[ ] - Redesign the command to incorporate detailed assessments of each condition part - InProgress The starting point for the approach is - the permission's or action's conditions are already captured into permission object, inside the Child attribute while parsing the input file. This permission.Child attribute is made use of to capture the assertions from conditions. Refer to the current commit. Consider this as initial approach to start with. Will refine, redesign and update as I progress with the analysis and considering different scenarios.

Already there is some improvement in the accuracy (with current updates) for the following sample input file: schema.yaml.txt

Before updates : coverage assertions percentage is 50%

After updates from current commit: The coverage assertions percentage is now 37% , as more uncovered assertions are captured from action/permission conditions.

[ ] - Implement tests and quality checks for the revised 'Coverage' command - To be Started

[ ] - Update documentation to reflect the new standards and procedures - To be Started

You can assign the issue #837 to me.

[vijayraghav-io]

Proposed design for incorporating detailed assessments of each condition part :

Considering the following condition as provided in the issue - permission view = system.view or ((is_public or (is_partner and partner) or (viewer or company.maintain or organization.maintain or team.view)) not denied)

Following are the design aspects.

1. Calculate coverage across all scenarios : The new “condition_parts coverage” would be calculated similar to how "relationships coverage" is calculated currently. Not w.r.t each scenario. Because a single scenario cannot cover all parts of the condition.

   Example:
 permission view = follower or non_follower

   In above example, a user cannot be both follower and non_follower.
 So, if there are 2 scenarios - one with user as follower and other as non_follower. Then we can say collectively that the above condition is 100% covered.


2. Splitting the condition parts : The way we split a condition into parts is important. Suppose if we just split straight away into individual parts - system.view, is_public, is_partner, partner,….. Consider there is one scenario that has is_partner as true (among other relations except partner) and if there is another scenario that has partner as true (but not is_partner).
We may conclude that coverage is 100%.

   But actually this is not correct. Because among the input scenarios there is no scenario provided where both is_partner and partner are true and this condition path (is_partner and partner) is never covered. So we need to have logical splits, i.e. we need to split into each nested condition with union (and) as one unit. So the correct splits would be - system.view is_public not denied (is_partner and partner) not denied (viewer or company.maintain or organization.maintain or team.view) not denied And then check whether each split is covered by atleast one of the several input scenarios.

3. Display format :
 Envisioning the output format for coverage command after implementing condition_parts coverage -


@tolgaOzen , Kindly share your thoughts on this.

[tolgaOzen]

Hi @vijayraghav-io, I understand your aspects, but can you clarify with more different examples?

[vijayraghav-io]

Sure @tolgaOzen , will come up with more examples.

[vijayraghav-io]

Hi @tolgaOzen , I am trying provide a simple example as below, but found that i am not able to provide input for an attribute `loggedIn' inside .yaml file. I referred the docs and existing test cases. Can you please show how we can input values to attributes in the schema.yam file (that will be input for the coverage command)

Example:

schema: >-

 entity user {}
    
 entity resource {
    relation viewer  @user  
    relation manager @user

    attribute loggedIn boolean 

    action edit = manager
    action view = (viewer or manager) and loggedIn
 }

relationships:

• “resource:1#viewer@user:1"

scenarios:

• name: "scenario 1" description: "test viewer” checks:
  • entity: “resource:1” subject: "user:1" assertions: view: true

For above check, how can i provide the value for loggedIn attribute as true.

[tolgaOzen]

Hi @vijayraghav-io, there is a field named 'attributes' in the YAML file, you can retrieve it from there. However, what I don't understand is, does it become 'cover' only when it is set to true?

[vijayraghav-io]

Thanks @tolgaOzen for providing your inputs on attributes. Agreed your view that - by default attributes will have a value (false for boolean), so it will be implicitly covered.

May be i was over analysing.

Then, can you throw some more light on what is that we need to check to be covered/not in conditions? The conditions include relations and attributes. If any of relation is not input, then its shown in uncovered relationships.

Does the current implementation done where i am including the condition parts as uncovered assertions (also doing it recursively for multi-level relations) will suffice? In this case the test cases need to be updated in coverage_test.go. Will do this if current implementation is ok.

Can you please explain with above example or any, if there is a gap in my understanding and needs correction.