mitchos/pyZscaler

[BUG]: Unable to create complex conditions with OR/AND statement in access policy rules

martinkiska opened this issue · 6 comments

Describe the bug

With current SDK and functions to create or update access policy rules we are unable to push OR/AND statements in POSTURE,SCIM,SCIM_GROUPS,APP conditions (probably also in others). With you current flat design there is unexpected results.

Current description of functions:

   """[('app', 'id', '926196382959075416'),
  ('app', 'id', '926196382959075417'),
  ('app_group', 'id', '926196382959075332),...]"""

kwargs passing to function:

  arg = {
      "conditions": [
          ("app", "id", "216197915188658453"),
          ("app", "id", "216197915188658455"),
          ("scim", "216197915188658304", "martin.kiska@xxxx"),
          ("scim", "216197915188658304", "xxxxx"),
          ("posture", "fc92ead2-4046-428d-bf3f-6e534a53194b", "TRUE"),
          ("posture", "490db9b4-96d8-4035-9b5e-935daa697f45", "TRUE"),
      ]
  }

Template function process them in a way that they are standalone with AND conditions which results in nonsense condition, which can be seen in GUI:
image

Expected behavior

Expected behavior is that I can for example push multiple posture conditions or multiple application segments (GUI allows that) with option to select OR/AND operator for such condition.

Proposed solution

to update/create function we can pass list of lists where we can add extra argument "OR", "AND" which will allow us to select the behavior. Still code will accepting old mechanism accepting list of tuples.

   arg = {
       "conditions": [
           [
               ("app", "id", "216197915188658453"),
               ("app", "id", "216197915188658455"),
               "OR",
           ],
           [
               ("scim", "216197915188658304", "martin.kiska@xxxxx"),
               ("scim", "216197915188658304", "xxxx"),
               "OR",
           ],
           ("scim_group", "216197915188658303", "241874"),   ###backward compatibility
           [
               ("posture", "fc92ead2-4046-428d-bf3f-6e534a53194b", "TRUE"),
               ("posture", "490db9b4-96d8-4035-9b5e-935daa697f45", "TRUE"),
               "AND",
           ],
       ]
   }

Template creation code:

   final_template = []
   for condition in conditions:
       operator = 0
       if isinstance(condition, list):
           template = []
           for parameter in condition:
               if isinstance(parameter, str) and (
                   parameter.upper() == "OR" or parameter.upper() == "AND"
               ):
                   operator = parameter
           for parameter in condition:
               if isinstance(parameter, tuple) and len(parameter) == 3:
                   operand = {
                       "objectType": parameter[0].upper(),
                       "lhs": parameter[1],
                       "rhs": parameter[2],
                   }
                   template.append(operand)
           if operator:
               one_condition = {"operands": template, "operator": operator.upper()}
           else:
               one_condition = {"operands": template}
           final_template.append(one_condition)
       ### for backward compatibility:
       if isinstance(condition, tuple) and len(condition) == 3:
           operand = {
               "operands": [
                   {
                       "objectType": condition[0].upper(),
                       "lhs": condition[1],
                       "rhs": condition[2],
                   }
               ]
           }
           final_template.append(operand)
   return final_template

image

Screenshots

Shared in text

Hi @martinkiska thank you for the comprehensive overview - problem is clear.

When I implemented the template logic, I have to admit that I only tested with the following criteria:

  • App Segments
  • Client Types
  • Client Connector Trusted Networks.

In these cases, the logic is automatic and doesn't need to be selected so my (flawed) assumption was that this carried through to the others.

I can see that the following criteria allow logic selection:

  • Client Connector Posture Profiles
  • SAML and SCIM Attributes

Looking back at the Zscaler API reference, it looks like the policy controller endpoint has changed a bit and now we have some other fields like negate (although I can't find where this can actually apply in any of the criterion in the UI). Also looks like it can accept an Idp ID in the conditions for the SCIM/SAML attributes.

Given that, we'll probably need to look at using a dict structure for the template as it's starting to get beyond the simplicity of a tuple. Of course it makes sense to also accept the simplified tuple condition, as you've suggested, for those object types that don't accept any other parameters.

I'll go through and test every policy condition and evaluate what's required for each object type, as well as the additional policy types that didn't exist when I implemented the policy controller (Isolation and Inspection). I'll be able to close this Issue off as
part of that piece of work.

Thanks for being patient, I just want to evaluate a few approaches to get this right and maintain simplicity.

Hi @mitchos, thanks for your reply and I really appreciate your work on this SDK!

Let mi highlight one more thing which was also badly implemented from my side as I didn't check properly SAML and SCIM attributes OR/AND behavior in GUI. While checking it when I click on OR/AND button in UI it changes conditions for all IDPs conditions (we have two different), so there should be some master "operator" statement for whole SCIM and SAML access rules configuration (not sure if it might be bug in UI).

image

Also you are mentioning posture and SAML and SCIM logic, but very important is app segment logic, where I expect people will push multiple app segments where OR logic is also very important.

Thank you...

Hi @mitchos,
It has been a while and I am still using my custom fix of _create_condition function and I am building on the top of that. Have you somehow consider my fix mentioned above to add it into main branch? Are you considering some other options?

Thx for your update!

Hi @mitchos and @martinkiska, I used the above-mentioned code & example (thanks!) to modify it, split it, add docs, tests and prepared a PR #166.

Please take a look at it. I believe it fits it purpose and covers what @martinkiska needed to cover (complex conditions with operands) although I'm not sure that it covers 100 % of cases of all types of conditions that could be created but it definitely expands the current capability for a rule creation.

WDYT? :-)

Thank you @david-kn for your work and standardization of code.

Hi @mitchos - is there some update from your side? Current limitation of access-policy method is blocking further deployment. We want to build automation based on this, but still we are forced to use custom branch with modification mentioned in this bug report.

We would really appreciate if you can look into it so it can be merged to main branch afterwards.

Hi @mitchos, let me close this issue as it was already covered by #166 last year