Intersection of required and provided needs is not sufficient

[kahlertl]

The test for access in the allows method of the Permission class using the intersection of the required and provided needs in insufficient. The documentation says the needs in a permission "Represents needs, any of which must be present to access a resource". But with the intersection, if you have at least one need in common, the intersection will be not empty and the test succeeds.

I think we should use issubset instead:

def allows(self, identity):
        """Whether the identity can access this permission.

        :param identity: The identity
        """
        if not self.needs.issubset(identity.provides):
            return False

        if self.excludes and self.excludes.intersection(identity.provides):
            return False

        return True

[aop]

I think you are understanding it wrong. "any of which must be present to access a resource" means that only one must match to gain access. This means they are evaluated as OR, not AND.

This makes the issue obsolete

[kahlertl]

Yeah, you are right. I have got a little bit confused with the English grammar. Sorry :)

[nealormsbee]

Hi,

I wonder if you'd consider reopening this issue as an enhancement, rather than a bug. Permissions behave as the docs describe, but it seems insufficient to only allow the OR'ing of Needs in Permission requirements.

It would be useful to be able to define complex OR/AND combinations of needs in Permission's constructor, rather than creating a new Need type for every desired AND of requirements, and then using only ORs at the top level.

[kahlertl]

Hey @SheepGotoHeaven :) We use Flask-Principal in our project with the following improvement:

import flask.ext.principal

class Permission(flask.ext.principal.Permission):

    def __init__(self, *needs):
        """A set of needs, any of which must be present in an identity to have
        access.
        """
        self.needs = set()

        for need in needs:
            if isinstance(need, str): 
                self.needs.add(flask.ext.principal.ActionNeed(need))
            else:
                self.needs.add(need)

        self.excludes = set()

    def __and__(self, permission):
        return AndPermission((self, permission))

    def __or__(self, permission):
        return OrPermission((self, permission))

class AndPermission(Permission):

    def __init__(self, *permissions):
        self.permissions = set(*permissions)

    def __and__(self, permission):
        return Permission(set(self.permissions).add(permission))

    def allows(self, identity):
        for permission in self.permissions:
            # If any permission fails, the whole or-permission fails
            if not permission.allows(identity):
                return False

        return True

class OrPermission(Permission):

    def __init__(self, *permissions):
        self.permissions = set(*permissions)

    def __or__(self, permission):
        return Permission(set(self.permissions).add(permission))

    def allows(self, identity):
        for permission in self.permissions:
            # If any permission succeeds, the whole or-permission
            # succeeds
            if permission.allows(identity):
                return True

        return False

delete_all_quests_permission         = Permission('delete_all_quests')
delete_quests_without_raw_permission = Permission('delete_quests_without_raw')

# Composite permission, indicating that the user is allowed to delete
# some questionnaires
delete_quests_permission = delete_all_quests_permission | delete_quests_without_raw_permission

Do you think, I should start a pull request for this?

[metatoaster]

@f3anaro I've done something like this already and included the required unittests for this in pull request #21. It's been over 2 years with no further activity.

[kahlertl]

@metatoaster You haved looked into your pull request and it seems to be perfect! Nicly done ... but why it is still open and not merged?