Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow the possibility of an "Always deny" Permission to facilitate "safe by default" designs. #92

Open
sean102 opened this issue Aug 10, 2024 · 0 comments

Comments

@sean102
Copy link

sean102 commented Aug 10, 2024

In my Flask application, I use class based Views. All views are derived from a base class.
The base class does not have a registered url rule and never receives requests directly.
My desire was to put an "Always Deny" permission on this base class to force any derived view
to explicitly declare permissions, thus creating a "safe by default" pattern.

I was surprised to find that Flask-Principal does not seem to have any (convenient) way of creating such a Permission.
The fundamental problem is that a Permission() without any Needs defaults to 'allow'
This is a little unintuitive given "A Permission is a set of requirements, any of which should be present for access to a resource."
But the chosen design is defensible, so probably an extra note in the documentation is sufficient to specify this corner case.

What is more problematic is that a Denial() without any Needs ALSO defaults to 'allow'. So we have a situation where
Permission() means 'allow' and Denial() also means 'allow'.

PROPOSAL

Part 1) Allow the default disposition (i.e. if permission.perms is an empty set) to be specified.

e.g.

class Permission(object):

    def __init__(self, *needs, default_permit=True):
        self.perms = {n: True for n in needs}
        self.default = default_permit

    def allows(self, identity):
        if self.needs and not self.needs.intersection(identity.provides):
            return False

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

        if self.perms:
            return True

        return self.default

Part 2) Denial() should default to 'deny'.
This is more a restoration of sanity rather than a necessity. The change of Part 1, makes this less necessary.
Alternately, This change would make the change in Part 1 unnecessary as Denial() would be the missing "Always deny" permission.

e.g.

class Denial(Permission):

    def __init__(self, *needs, default_permit=False):  # default_permit=True would be backward compatible

This is a potentially breaking change. However.
The documentation does not say what will happen in this case.
There are no instances of Denial() in the unit tests (There ARE instances of Permission() in the unit tests)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

1 participant