The logic for AllNotIn and AnyNotIn are the wrong way around

[SpoonMeiser]

Conditions for collections. Take for example, AllNotIn, the documentation says:

none of the members of attribute value collection are members of "values"

Yet the test is:

https://github.com/ketgo/py-abac/blob/master/tests/test_policy/test_conditions/test_collection.py#L80

    @pytest.mark.parametrize("condition, what, result", [
        ...
        (AllNotIn([2]), [1, 2], True),

Of the 2 values for the attribute value collection (1 and 2), 2 is a member of "values", so the condition should return False.

[ketgo]

Hi @SpoonMeise , thanks for pointing this out! I'll have a look and get back to you

[SpoonMeiser]

My particular case, the subject can have multiple roles, and I'm checking if those roles do or do not contain a role I am interested in. It would make the policies easier to read if there were Contains and DoesNotContain conditions. Would you consider that as a feature?

I'm interested in the policies being easier to read because it'll be easier to avoid mistakes, and I'm hoping that I can get non-developers to manage the policies.

[ketgo]

Just had a look, the implementation of AllNotIn and AnyNotIn is correct. The description in the docs is misleading. The AllNotIn condition checks if all the values in a access request attribute is not in the "values" field of the condition. In the test since the condition "values" is just [2] while the access request attribute is [1, 2], since all the values of the request attribute is not in condition "values" so the result should be True. This is what the test is validating.

I believe your use-case can be supported by using the AnyIn and AnyNotIn conditions. The idea here is we are checking if the user has a one or more roles which the policy allows. The AllIn and AllNotIn conditions are useful if you want to make sure all the roles specified in the policy are assigned to the user.

Regarding renaming the conditions, I think its a good idea if it makes the policy easier to read and would be more than happy to include it as a feature. Unfortunately, I might not be able to get on it for while. Would you like to open a PR and give it a try? I think one approach of achieving this is by driving a new named class from AnyIn, AllIn, etc and adding it to the factory in https://github.com/ketgo/py-abac/blob/master/py_abac/policy/conditions/schema.py.

[SpoonMeiser]

If the implementation is correct, the documentation isn't misleading, it's plain wrong.

The documentation for AnyNotIn is

one or more members of the attribute value collection are not members of "values"

Yet the test here is

(AnyNotIn([2]), [1, 2], False)

Again, 1 is one of the attribute values collection, and it is not a member of "values", so why is the condition False?

To my mind, the documentation matches what the names of the conditions intuitively imply, and the implementations don't. If the implementation is what you think it should be, the documentation and the names of the conditions ought to be changed.

[SpoonMeiser]

I'm happy to make a PR, but we ought to agree on what it should change first :)

[ketgo]

Yes, the documentation is wrong and needs to be corrected. The basic idea around these conditions is checking for subsets, i.e. one is subset of another. When writing the documentation I thought that would not be user friendly so decided of not using such. Maybe describing in terms of sets would be more clear? Any suggestions? :)